Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: --recover to get mnemonic from file #137

Merged
merged 20 commits into from
Apr 3, 2024
Merged

fix: --recover to get mnemonic from file #137

merged 20 commits into from
Apr 3, 2024

Conversation

dckc
Copy link
Member

@dckc dckc commented Apr 2, 2024

maybe this is all it takes?

fixes: #136

cc @ivanlei

@dckc dckc requested review from turadg and LuqiPan April 2, 2024 16:06
@dckc
Copy link
Member Author

dckc commented Apr 2, 2024

@ivanlei suggests a test that the generated address is as expected. Seems reasonable. hm.

@LuqiPan
Copy link
Contributor

LuqiPan commented Apr 2, 2024

Thank you for fixing my mistakes

Re: CI failure, I made another mistake and copied the same mnemonics for gov3, user1, and validator

I can push some commits to fix it if you want

proposals/16:upgrade-8/sanity.test.js Outdated Show resolved Hide resolved
@@ -5,6 +5,8 @@ set -e

source /usr/src/upgrade-test-scripts/env_setup.sh

yarn ava keys.test.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this just debugging? tests don't belong in use.sh. the ones here are tech debt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I noticed keys.test.js was not run and found an example of running it in use.sh

I've since moved it to test.sh

@@ -4,7 +4,7 @@ set -e

source /usr/src/upgrade-test-scripts/env_setup.sh

yarn ava addresses.test.js
YARN_IGNORE_NODE=1 yarn ava addresses.test.js
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LuqiPan
Copy link
Contributor

LuqiPan commented Apr 3, 2024

Given #136 has customer visible impact, I'm prioritizing merging this PR in and opened a separate issue in #138

@LuqiPan LuqiPan merged commit bf97fac into main Apr 3, 2024
2 checks passed
@LuqiPan LuqiPan deleted the dckc-patch-1 branch April 3, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fixed mnemonics are not being used
3 participants