forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(wallet): make mnemonic bits tweakable, default to 128 bit / 12 words #5457
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
99f0e71
feat(wallet): Make mnemonic bits tweakable
UdjinM6 b0731e4
feat(wallet): Set mnemonic bits default to 128 bits / 12 words
UdjinM6 be0ddb4
test: add tests for `-mnemonicbits`
UdjinM6 bd92c5b
fix: tweak help/error texts
UdjinM6 8a8e3e3
fix: adjust bip39 implementation to match bip39 spec
UdjinM6 3542ce6
fix: adjust `-mnemonicbits` help text/error
UdjinM6 112634f
fix `wallet_mnemonicbits.py` to use new error text, test more values,…
UdjinM6 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| #!/usr/bin/env python3 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing executable flag on |
||
| # Copyright (c) 2023 The Dash Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
| """Test -mnemonicbits wallet option.""" | ||
|
|
||
| from test_framework.test_framework import BitcoinTestFramework | ||
| from test_framework.util import ( | ||
| assert_equal, | ||
| ) | ||
|
|
||
| class WalletMnemonicbitsTest(BitcoinTestFramework): | ||
| def set_test_params(self): | ||
| self.setup_clean_chain = True | ||
| self.num_nodes = 1 | ||
| self.extra_args = [['-usehd=1']] | ||
|
|
||
| def skip_test_if_missing_module(self): | ||
| self.skip_if_no_wallet() | ||
|
|
||
| def run_test(self): | ||
| self.log.info("Test -mnemonicbits") | ||
|
|
||
| self.log.info("Invalid -mnemonicbits should rise an error") | ||
| self.stop_node(0) | ||
| self.nodes[0].assert_start_raises_init_error(self.extra_args[0] + ['-mnemonicbits=123'], "Error: Invalid '-mnemonicbits'. Allowed values: 128, 192, 256.") | ||
| self.start_node(0) | ||
| assert_equal(len(self.nodes[0].dumphdinfo()["mnemonic"].split()), 12) # 12 words by default | ||
|
|
||
| self.log.info("Can have multiple wallets with different mnemonic length loaded at the same time") | ||
| self.restart_node(0, extra_args=self.extra_args[0] + ["-mnemonicbits=192"]) | ||
| self.nodes[0].createwallet("wallet_192") | ||
| self.restart_node(0, extra_args=self.extra_args[0] + ["-mnemonicbits=256"]) | ||
| self.nodes[0].loadwallet("wallet_192") | ||
| self.nodes[0].createwallet("wallet_256", False, True) # blank | ||
| self.nodes[0].get_wallet_rpc("wallet_256").upgradetohd() | ||
| assert_equal(len(self.nodes[0].get_wallet_rpc(self.default_wallet_name).dumphdinfo()["mnemonic"].split()), 12) # 12 words by default | ||
| assert_equal(len(self.nodes[0].get_wallet_rpc("wallet_192").dumphdinfo()["mnemonic"].split()), 18) # 18 words | ||
| assert_equal(len(self.nodes[0].get_wallet_rpc("wallet_256").dumphdinfo()["mnemonic"].split()), 24) # 24 words | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| WalletMnemonicbitsTest().main () | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
isn't 160 also accepted as a correct value?
but when it's generated:
Probably should be updated
32to 64: otherwise Generate() is happy with 160 bits, 224 bits butCMnemonic::Checklater fails with error: mnemonic with 15 words are invalid mnemonic.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 tested it with modified script:
...
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.
Good catch! Indeed more values should be allowed https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#generating-the-mnemonic