feat(wallet): make mnemonic bits tweakable, default to 128 bit / 12 words (#5457)

## Issue being fixed or feature implemented
Allow generating 12, 18, 24 words mnemonics. Default to 12 words as it's
the most popular option/de-facto a standard now imo.

## What was done?
Add `-mnemonicbits` option, add tests

## How Has This Been Tested?
run tests, play with wallets on regtest

## Breaking Changes
n/a, old wallets should not be affected

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

Author: UdjinM6

src/bip39.cpp

@@ -91,7 +91,7 @@ bool CMnemonic::Check(SecureString mnemonic)
     }
     nWordCount++;
     // check number of words
-    if (nWordCount != 12 && nWordCount != 18 && nWordCount != 24) {
+    if (nWordCount % 3 != 0 || nWordCount < 12 || nWordCount > 24) {
         return false;
     }
 
@@ -133,18 +133,10 @@ bool CMnemonic::Check(SecureString mnemonic)
     bits[32] = bits[nWordCount * 4 / 3];
     CSHA256().Write(bits.data(), nWordCount * 4 / 3).Finalize(bits.data());
 
-    bool fResult = 0;
-    if (nWordCount == 12) {
-        fResult = (bits[0] & 0xF0) == (bits[32] & 0xF0); // compare first 4 bits
-    } else
-    if (nWordCount == 18) {
-        fResult = (bits[0] & 0xFC) == (bits[32] & 0xFC); // compare first 6 bits
-    } else
-    if (nWordCount == 24) {
-        fResult = bits[0] == bits[32]; // compare 8 bits
-    }
+    const char checksum_length = nWordCount / 3;
+    const char mask = (2 ^ checksum_length) << (8 - checksum_length);
 
-    return fResult;
+    return (bits[0] & mask) == (bits[32] & mask);
 }
 
 // passphrase must be at most 256 characters otherwise it would be truncated

src/hdchain.cpp

@@ -88,7 +88,7 @@ bool CHDChain::SetMnemonic(const SecureString& ssMnemonic, const SecureString& s
 
         // empty mnemonic i.e. "generate a new one"
         if (ssMnemonic.empty()) {
-            ssMnemonicTmp = CMnemonic::Generate(256);
+            ssMnemonicTmp = CMnemonic::Generate(gArgs.GetArg("-mnemonicbits", DEFAULT_MNEMONIC_BITS));
         }
         // NOTE: default mnemonic passphrase is an empty string
 

src/hdchain.h

@@ -42,6 +42,8 @@ class CHDChain
     std::map<uint32_t, CHDAccount> GUARDED_BY(cs) mapAccounts;
 
 public:
+    /** Default for -mnemonicbits */
+    static constexpr int DEFAULT_MNEMONIC_BITS = 128; // 128 bits == 12 words
 
     CHDChain() = default;
     CHDChain(const CHDChain& other) :

src/wallet/init.cpp

@@ -24,8 +24,10 @@
 #include <wallet/wallet.h>
 #include <walletinitinterface.h>
 
+#include <bip39.h>
 #include <coinjoin/client.h>
 #include <coinjoin/options.h>
+#include <hdchain.h>
 
 class WalletInit : public WalletInitInterface
 {
@@ -85,6 +87,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
 
     argsman.AddArg("-hdseed=<hex>", "User defined seed for HD wallet (should be in hex). Only has effect during wallet creation/first start (default: randomly generated)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::WALLET_HD);
     argsman.AddArg("-mnemonic=<text>", "User defined mnemonic for HD wallet (bip39). Only has effect during wallet creation/first start (default: randomly generated)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::WALLET_HD);
+    argsman.AddArg("-mnemonicbits=<n>", strprintf("User defined mnemonic security for HD wallet in bits (BIP39). Only has effect during wallet creation/first start (allowed values: 128, 160, 192, 224, 256; default: %u)", CHDChain::DEFAULT_MNEMONIC_BITS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_HD);
     argsman.AddArg("-mnemonicpassphrase=<text>", "User defined mnemonic passphrase for HD wallet (BIP39). Only has effect during wallet creation/first start (default: empty string)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::WALLET_HD);
     argsman.AddArg("-usehd", strprintf("Use hierarchical deterministic key generation (HD) after BIP39/BIP44. Only has effect during wallet creation/first start (default: %u)", DEFAULT_USE_HD_WALLET), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_HD);
 
@@ -168,6 +171,10 @@ bool WalletInit::ParameterInteraction() const
         return InitError(strprintf(_("%s can't be lower than %s"), "-coinjoindenomshardcap", "-coinjoindenomsgoal"));
     }
 
+    if (CMnemonic::Generate(gArgs.GetArg("-mnemonicbits", CHDChain::DEFAULT_MNEMONIC_BITS)) == SecureString()) {
+        return InitError(strprintf(_("Invalid '%s'. Allowed values: 128, 160, 192, 224, 256."), "-mnemonicbits"));
+    }
+
     return true;
 }
 

test/functional/test_runner.py

@@ -213,6 +213,7 @@
     'wallet_import_rescan.py',
     'wallet_import_with_label.py',
     'wallet_upgradewallet.py',
+    'wallet_mnemonicbits.py',
     'rpc_bind.py --ipv4',
     'rpc_bind.py --ipv6',
     'rpc_bind.py --nonloopback',

test/functional/wallet_mnemonicbits.py

@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+# 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, 160, 192, 224, 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=160"])
+        self.nodes[0].createwallet("wallet_160")
+        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=224"])
+        self.nodes[0].createwallet("wallet_224")
+        self.restart_node(0, extra_args=self.extra_args[0] + ["-mnemonicbits=256"])
+        self.nodes[0].loadwallet("wallet_160")
+        self.nodes[0].loadwallet("wallet_192")
+        self.nodes[0].loadwallet("wallet_224")
+        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_160").dumphdinfo()["mnemonic"].split()), 15)              # 15 words
+        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_224").dumphdinfo()["mnemonic"].split()), 21)              # 21 words
+        assert_equal(len(self.nodes[0].get_wallet_rpc("wallet_256").dumphdinfo()["mnemonic"].split()), 24)              # 24 words
+
+
+if __name__ == '__main__':
+    WalletMnemonicbitsTest().main ()