feat(rpc/wallet): allow skipping/forcing blockchain rescan on upgradetohd (#4822)

* wallet: allow skipping/forcing blockchain rescan on upgradetohd

* tests: test new use cases in wallet_upgradetohd.py

* add release notes

doc/release-notes-4822.md

@@ -0,0 +1,3 @@
+Miscellaneous RPC Changes
+-------------------------
+- In rpc `upgradetohd` new parameter `rescan` was added which allows users to skip or force blockchain rescan. This params defaults to `false` when `mnemonic` parameter is empty and `true` otherwise.

src/rpc/client.cpp

@@ -196,6 +196,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "rescanblockchain", 1, "stop_height"},
     { "createwallet", 1, "disable_private_keys"},
     { "createwallet", 2, "blank"},
+    { "upgradetohd", 3, "rescan"},
     { "getnodeaddresses", 0, "count"},
     { "stop", 0, "wait" },
 };

src/wallet/rpcwallet.cpp

@@ -2567,6 +2567,7 @@ static UniValue upgradetohd(const JSONRPCRequest& request)
             {"mnemonic", RPCArg::Type::STR, /* default */ "", "Mnemonic as defined in BIP39 to use for the new HD wallet. Use an empty string \"\" to generate a new random mnemonic."},
             {"mnemonicpassphrase", RPCArg::Type::STR, /* default */ "", "Optional mnemonic passphrase as defined in BIP39"},
             {"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted specifying wallet passphrase will trigger wallet encryption."},
+            {"rescan", RPCArg::Type::BOOL, /* default */ "false if mnemonic is empty", "Whether to rescan the blockchain for missing transactions or not"},
         },
         RPCResult{
             RPCResult::Type::BOOL, "", "true if successful"
@@ -2650,7 +2651,8 @@ static UniValue upgradetohd(const JSONRPCRequest& request)
     }
 
     // If you are generating new mnemonic it is assumed that the addresses have never gotten a transaction before, so you don't need to rescan for transactions
-    if (!generate_mnemonic) {
+    bool rescan = request.params[3].isNull() ? !generate_mnemonic : request.params[3].get_bool();
+    if (rescan) {
         WalletRescanReserver reserver(pwallet);
         if (!reserver.reserve()) {
             throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
@@ -3926,7 +3928,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "signmessage",                      &signmessage,                   {"address","message"} },
     { "wallet",             "signrawtransactionwithwallet",     &signrawtransactionwithwallet,  {"hexstring","prevtxs","sighashtype"} },
     { "wallet",             "unloadwallet",                     &unloadwallet,                  {"wallet_name"} },
-    { "wallet",             "upgradetohd",                      &upgradetohd,                   {"mnemonic", "mnemonicpassphrase", "walletpassphrase"} },
+    { "wallet",             "upgradetohd",                      &upgradetohd,                   {"mnemonic", "mnemonicpassphrase", "walletpassphrase", "rescan"} },
     { "wallet",             "walletlock",                       &walletlock,                    {} },
     { "wallet",             "walletpassphrasechange",           &walletpassphrasechange,        {"oldpassphrase","newpassphrase"} },
     { "wallet",             "walletpassphrase",                 &walletpassphrase,              {"passphrase","timeout","mixingonly"} },

test/functional/wallet_upgradetohd.py

@@ -90,6 +90,17 @@ class WalletUpgradeToHDTest(BitcoinTestFramework):
 
         self.recover_non_hd()
 
+        self.log.info("No mnemonic, no mnemonic passphrase, no wallet passphrase, should result in completely different keys")
+        self.stop_node(0)
+        self.start_node(0, extra_args=['-keypool=10'])
+        assert node.upgradetohd("", "", "", True)
+        # Completely different keys, no HD coins should be recovered
+        assert mnemonic != node.dumphdinfo()['mnemonic']
+        assert chainid != node.getwalletinfo()['hdchainid']
+        assert_equal(balance_non_HD, node.getbalance())
+
+        self.recover_non_hd()
+
         self.log.info("Same mnemonic, another mnemonic passphrase, no wallet passphrase, should result in a different set of keys")
         new_mnemonic_passphrase = "somewords"
         assert node.upgradetohd(mnemonic, new_mnemonic_passphrase)
@@ -123,6 +134,7 @@ class WalletUpgradeToHDTest(BitcoinTestFramework):
         assert_equal(mnemonic, node.dumphdinfo()['mnemonic'])
         assert_equal(chainid, node.getwalletinfo()['hdchainid'])
         node.keypoolrefill(5)
+        assert balance_after != node.getbalance()
         node.rescanblockchain()
         assert_equal(balance_after, node.getbalance())
 
@@ -139,6 +151,19 @@ class WalletUpgradeToHDTest(BitcoinTestFramework):
 
         self.recover_non_hd()
 
+        self.log.info("Same mnemonic, no mnemonic passphrase, no wallet passphrase, large enough keepool, rescan is skipped initially, should recover all coins after rescanblockchain")
+        self.stop_node(0)
+        self.start_node(0, extra_args=['-keypool=10'])
+        assert node.upgradetohd(mnemonic, "", "", False)
+        assert_equal(mnemonic, node.dumphdinfo()['mnemonic'])
+        assert_equal(chainid, node.getwalletinfo()['hdchainid'])
+        assert balance_after != node.getbalance()
+        node.rescanblockchain()
+        # All coins should be recovered
+        assert_equal(balance_after, node.getbalance())
+
+        self.recover_non_hd()
+
         self.log.info("Same mnemonic, same mnemonic passphrase, encrypt wallet on upgrade, should recover all coins after rescan")
         walletpass = "111pass222"
         assert node.upgradetohd(mnemonic, "", walletpass)