-
Notifications
You must be signed in to change notification settings - Fork 88
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
[gh-2022] update address data in mnemonic data. #2091
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
password: Option<String>, | ||
) -> Result<GeneratedKeyPair, anyhow::Error> { | ||
// load mnemonic phrase from keystore | ||
let mnemonic = self.get_mnemonic(password.clone())?; | ||
let account_index = mnemonic.mnemonic_data.addresses.len() as u32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the mnemonic_data.addresses.len()
incorrect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the
mnemonic_data.addresses.len()
incorrect?
See #2022 for details.
Cause there's only one address in mnemonic_data
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only one address in mnemonic_data? It also should be fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only one address in mnemonic_data? It also should be fixed
We should discuss it with @baichuan3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use keystore.addresses()
as the account index is ok. If do this, we can remove MnemonicData.addresses
field to simplify the MnemonicData definition.
Or when creating an account, need to update the new address to MnemonicData.addresses
, and can keep MnemonicData.addresses
.
Both implementations can work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
keystore.addresses()
as the account index is ok. If do this, we can removeMnemonicData.addresses
field to simplify the MnemonicData definition. Or when creating an account, need to update the new address toMnemonicData.addresses
, and can keepMnemonicData.addresses
. Both implementations can work.
It sounds like that we can remove MnemonicData.addresses as mentioned in #950 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the process to add the Rooch addresses to mnemonic data while using the current length of keystores as account index to create keys as the addresses from keys are needed to be generated from account index before adding them to mnemonic data. |
14780b2
to
45142a7
Compare
Summary
Update address data in mnemonic.
See discussion in #2022 for details.
rooch account create
can not create more account #2022