Merge dashpay#5991: fix: various fixes for HD wallets implementation

e5129e6 fix: if hdseed is wrong - do not setup random seed, user can lost his fund (Konstantin Akimov)
e52498b chore: add todoes to UpgradeToHD function (Konstantin Akimov)
0d12ea9 fix: wrong if/else branches for key refill in wallet creation (Konstantin Akimov)
d2c3dcb refactor: move list of function in rpcwallet to the end (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Reviewed our implementation of HD wallets and compare it to bitcoin's for aiming to backport or re-implement `sethdseed` rpc.
  Noticed some strange things in our implementation, which this PR is aim to fix.

  ## What was done?
  See each commit for detailed changes.

  ## How Has This Been Tested?
  Run unit/functional tests

  ## Breaking Changes

  `-hdseed` doesn't assign a random seed anymore if an user provided an invalid hex string.

  ## 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

ACKs for top commit:
  PastaPastaPasta:
    utACK e5129e6

Tree-SHA512: b404dbc0762777abf421a847f58cd243e0aa00151ba3f036835c9ff54c1109b6159921ec24e29455e975797f49d54832c55f7876188da90f37dd1e4a811a21e0

Author: PastaPastaPasta

src/wallet/rpcwallet.cpp

@@ -4272,21 +4272,6 @@ static RPCHelpMan send()
     };
 }
 
-RPCHelpMan abortrescan();
-RPCHelpMan dumpprivkey();
-RPCHelpMan importprivkey();
-RPCHelpMan importaddress();
-RPCHelpMan importpubkey();
-RPCHelpMan dumpwallet();
-RPCHelpMan importwallet();
-RPCHelpMan importprunedfunds();
-RPCHelpMan removeprunedfunds();
-RPCHelpMan importmulti();
-RPCHelpMan importdescriptors();
-RPCHelpMan listdescriptors();
-RPCHelpMan dumphdinfo();
-RPCHelpMan importelectrumwallet();
-
 RPCHelpMan walletprocesspsbt()
 {
     return RPCHelpMan{"walletprocesspsbt",
@@ -4520,6 +4505,21 @@ static RPCHelpMan upgradewallet()
     };
 }
 
+RPCHelpMan abortrescan();
+RPCHelpMan dumpprivkey();
+RPCHelpMan importprivkey();
+RPCHelpMan importaddress();
+RPCHelpMan importpubkey();
+RPCHelpMan dumpwallet();
+RPCHelpMan importwallet();
+RPCHelpMan importprunedfunds();
+RPCHelpMan removeprunedfunds();
+RPCHelpMan importmulti();
+RPCHelpMan importdescriptors();
+RPCHelpMan listdescriptors();
+RPCHelpMan dumphdinfo();
+RPCHelpMan importelectrumwallet();
+
 Span<const CRPCCommand> GetWalletRPCCommands()
 {
 // clang-format off

src/wallet/wallet.cpp

@@ -4609,7 +4609,8 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, interfaces::C
                     newHdChain.AddAccount();
                 } else {
                     if (gArgs.IsArgSet("-hdseed") && !IsHex(strSeed)) {
-                        walletInstance->WalletLogPrintf("%s -- Incorrect seed, generating a random mnemonic instead\n", __func__);
+                        error = strprintf(_("%s -- Incorrect seed, it should be a hex string"), __func__);
+                        return nullptr;
                     }
                     SecureString secureMnemonic = gArgs.GetArg("-mnemonic", "").c_str();
                     SecureString secureMnemonicPassphrase = gArgs.GetArg("-mnemonicpassphrase", "").c_str();
@@ -4623,18 +4624,13 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, interfaces::C
                 gArgs.ForceRemoveArg("hdseed");
                 gArgs.ForceRemoveArg("mnemonic");
                 gArgs.ForceRemoveArg("mnemonicpassphrase");
-            }
+            } // Otherwise, do not create a new HD chain
+
             LOCK(walletInstance->cs_wallet);
             if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
                 walletInstance->SetupDescriptorScriptPubKeyMans();
                 // SetupDescriptorScriptPubKeyMans already calls SetupGeneration for us so we don't need to call SetupGeneration separately
-            }
-        } // Otherwise, do not create a new HD chain
-
-        // Top up the keypool
-        {
-            LOCK(walletInstance->cs_wallet);
-            if (!walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
+            } else { // Top up the keypool
                 // Legacy wallets need SetupGeneration here.
                 if (auto spk_man = walletInstance->GetLegacyScriptPubKeyMan()) {
                     if (spk_man->CanGenerateKeys() && !spk_man->TopUp()) {
@@ -4935,8 +4931,10 @@ bool CWallet::UpgradeToHD(const SecureString& secureMnemonic, const SecureString
     WalletLogPrintf("Upgrading wallet to HD\n");
     SetMinVersion(FEATURE_HD);
 
+    // TODO: replace to GetLegacyScriptPubKeyMan() when `sethdseed` is backported
     auto spk_man = GetOrCreateLegacyScriptPubKeyMan();
     bool prev_encrypted = IsCrypted();
+    // TODO: unify encrypted and plain chains usages here
     if (prev_encrypted) {
         if (!GenerateNewHDChainEncrypted(secureMnemonic, secureMnemonicPassphrase, secureWalletPassphrase)) {
             error = Untranslated("Failed to generate encrypted HD wallet");