Merge #2337: [Wallet] Basic multiwallet support

4734a84 [Cleanup][Tests] Fix chainparams-change in librust tests (random-zebra)
ff4cee0 [Trivial] Add wallet filename to backup errors/warning (random-zebra)
dc596f3 [Doc] Add multiwallet section to release notes (random-zebra)
36796f2 [Cleanup] Fix formatting in wallet and walletdb (random-zebra)
b6dbbf3 wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet with multiple wallets (random-zebra)
60f9b4b wallet: Base backup filenames on original wallet filename (Luke Dashjr)
e6efa6b wallet: Include actual backup filename in recovery warning message (Luke Dashjr)
647fbc9 Wallet: Move multiwallet sanity checks to CWallet::Verify, and do other checks on all wallets (Luke Dashjr)
b27dcfe Wallet: Support loading multiple wallets if -wallet used more than once (Luke Dashjr)
d10acd5 [Tests] move pwalletMain to wallet test fixture + use smart pointer (random-zebra)
d6cf608 [Refactor] Remove CWalletDBWrapper::GetUpdateCounter() (random-zebra)
4bbad5c [Wallet] Replace pwalletMain with a vector of wallet pointers (random-zebra)
100d67c Wallet: Sanitise -wallet parameter (Luke Dashjr)
3bfa7d8 Wallet/RPC: Use filename rather than CWallet pointer, for lockwallet RPCRunLater job name (Luke Dashjr)
ca6a62d [MOVE-ONLY] Move wallet RPC declarations to rpcwallet.h (random-zebra)
687c2fd RPC: Pass on JSONRPCRequest metadata (URI/user/etc) for "help" method (Luke Dashjr)
cc965fe Move nWalletUnlockTime to CWallet::nRelockTime, and name timed task unique per CWallet (Luke Dashjr)
22f8507 [Trivial] Rename pwalletMain --> pwallet for local variables in RPC (random-zebra)
325baaa RPC: Do all wallet access through new GetWalletForJSONRPCRequest (random-zebra)
0e21e09 [Cleanup] Remove un-used printAddresses() function in rpcwallet (random-zebra)
a8dd236 RPC/Wallet: Pass CWallet as pointer to helper functions (random-zebra)
fb64bbc [Doc] Remove ThreadFlushWalletDB from developer notes (random-zebra)
dc2e022 [Refactor] fix WalletTestingSetup fixture (random-zebra)
f9d7fe1 refactor: move bdb (bitdb) interaction from init.cpp to wallet.cpp (random-zebra)
9cfb711 CWalletDB: Store the update counter per wallet (Luke Dashjr)
8edb74f Bug: increment counter when writing minversion (random-zebra)
8aa93b9 Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races (Luke Dashjr)

Pull request description:

  Quite a bit of refactoring to bring us a little closer to upstream in the wallet/walletdb area, introducing basic support for multiple wallets.
  The PIVX client can now be started with more than one `-wallet` argument (either as startup flags, or as multiple lines in pivx.conf). The wallets will be all loaded and kept separated, with individual balances, keys and received transactions.

  Even though only the first wallet will be used in the GUI/RPC for the moment (selectable wallets will be added later), all other loaded wallets will remain synchronized to the node's current tip and update their internal data.

  Bulk of changes coming from:
  - bitcoin#8775 RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest
  - bitcoin#8694 Basic multiwallet support
  - bitcoin#11713 Fix for mismatched extern definition in wallet tests
  - bitcoin#11781 tests: move pwalletMain to wallet test fixture

ACKs for top commit:
  furszy:
    Code ACK 4734a84 after rebase and inclusion of 4734a84.

Tree-SHA512: 483fc34310c86070ffde08487605b25da957e55fbc8ef1b9cc1f682c6af0789619bb40839e915618d603097a1bdcf6b2110c7fabf017f8351b3a426976ca77a9

Author: random-zebra

doc/developer-notes.md

@@ -301,8 +301,6 @@ Threads
 
 - DumpAddresses : Dumps IP addresses of nodes to peers.dat.
 
-- ThreadFlushWalletDB : Close the wallet.dat file if it hasn't been used in 500ms.
-
 - ThreadRPCServer : Remote procedure call handler, listens on port 8332 for connections and services them.
 
 - BitcoinMiner : Generates PIVs (if wallet is enabled).

doc/release-notes.md

@@ -49,14 +49,26 @@ Cold-Staking Re-Activation
 PIVX Core v6.0.0 includes a fix for the vulnerability identified within the cold-staking protocol (see PR [#2258](https://github.com/PIVX-Project/PIVX/pull/2258)).
 Therefore the feature will be re-enabled on the network, via `SPORK_19`, shortly after the upgrade enforcement.
 
-#### Protocol changes
+### Protocol changes
 
 A new opcode (`0xd2`) is introduced (see PR [#2275](https://github.com/PIVX-Project/PIVX/pull/2275)). It enforces the same rules as the legacy cold-staking opcode, but without allowing a "free" script for the last output of the transaction.
 This is in accord with the consensus change introduced with the "Deterministic Masternodes" update, as masternode/budget payments are now outputs of the *coinbase* transaction (rather than the *coinstake*), therefore a "free" output for the coinstake is no longer needed.
 The new opcode takes the name of `OP_CHECKCOLDSTAKEVERIFY`, and the legacy opcode (`0xd1`) is renamed to `OP_CHECKCOLDSTAKEVERIFY_LOF` (last-output-free).
 Scripts with the old opcode are still accepted on the network (the restriction on the last-output is enforced after the script validation in this case), but the client creates new delegations with the new opcode, by default, after the upgrade enforcement.
 
 
+Multi-wallet support
+--------------------
+
+PIVX Core now supports loading multiple, separate wallets (See [PR 2337](https://github.com/PIVX-Project/PIVX/pull/2337)). The wallets are completely separated, with individual balances, keys and received transactions.
+
+Multi-wallet is enabled by using more than one `-wallet` argument when starting PIVX client, either on the command line or in the pivx.conf config file.
+
+**In pivx-qt, only the first wallet will be displayed and accessible for creating and signing transactions.** GUI selectable multiple wallets will be supported in a future version. However, even in 6.0 other loaded wallets will remain synchronized to the node's current tip in the background.
+
+!TODO: update after endpoint support and multi-wallet RPC support
+
+
 GUI changes
 -----------
 
@@ -101,6 +113,7 @@ Low-level RPC changes
   - `maximumCount` - a number specifying the minimum number of UTXOs
   - `minimumSumAmount` - a number specifying the minimum sum value of all UTXOs
 
+
 #### Show wallet's auto-combine settings in getwalletinfo
 
 `getwalletinfo` now has two additional return fields. `autocombine_enabled` (boolean) and `autocombine_threshold` (numeric) that will show the auto-combine threshold and whether or not it is currently enabled.

src/budget/budgetmanager.cpp

@@ -107,13 +107,17 @@ uint256 CBudgetManager::SubmitFinalBudget()
         // create the collateral tx, send it to the network and return
         CTransactionRef wtx;
         // Get our change address
-        CReserveKey keyChange(pwalletMain);
-        if (!pwalletMain->CreateBudgetFeeTX(wtx, budgetHash, keyChange, true)) {
+        if (vpwallets.empty() || !vpwallets[0]) {
+            LogPrint(BCLog::MNBUDGET,"%s: Wallet not found\n", __func__);
+            return UINT256_ZERO;
+        }
+        CReserveKey keyChange(vpwallets[0]);
+        if (!vpwallets[0]->CreateBudgetFeeTX(wtx, budgetHash, keyChange, true)) {
             LogPrint(BCLog::MNBUDGET,"%s: Can't make collateral transaction\n", __func__);
             return UINT256_ZERO;
         }
         // Send the tx to the network
-        const CWallet::CommitResult& res = pwalletMain->CommitTransaction(wtx, keyChange, g_connman.get());
+        const CWallet::CommitResult& res = vpwallets[0]->CommitTransaction(wtx, keyChange, g_connman.get());
         if (res.status == CWallet::CommitStatus::OK) {
             const uint256& collateraltxid = wtx->GetHash();
             mapUnconfirmedFeeTx.emplace(budgetHash, collateraltxid);

src/init.cpp

@@ -1,10 +1,10 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2015 The Bitcoin developers
+// Copyright (c) 2009-2021 The Bitcoin developers
 // Copyright (c) 2014-2015 The Dash developers
 // Copyright (c) 2011-2013 The PPCoin developers
 // Copyright (c) 2013-2014 The NovaCoin Developers
 // Copyright (c) 2014-2018 The BlackCoin Developers
-// Copyright (c) 2015-2020 The PIVX developers
+// Copyright (c) 2015-2021 The PIVX developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -58,7 +58,6 @@
 #include "zpivchain.h"
 
 #ifdef ENABLE_WALLET
-#include "wallet/db.h"
 #include "wallet/wallet.h"
 #include "wallet/rpcwallet.h"
 
@@ -222,8 +221,9 @@ void PrepareShutdown()
     StopRPC();
     StopHTTPServer();
 #ifdef ENABLE_WALLET
-    if (pwalletMain)
-        bitdb.Flush(false);
+    for (CWalletRef pwallet : vpwallets) {
+        pwallet->Flush(false);
+    }
     GenerateBitcoins(false, NULL, 0);
 #endif
     StopMapPort();
@@ -302,8 +302,9 @@ void PrepareShutdown()
         evoDb.reset();
     }
 #ifdef ENABLE_WALLET
-    if (pwalletMain)
-        bitdb.Flush(true);
+    for (CWalletRef pwallet : vpwallets) {
+        pwallet->Flush(true);
+    }
 #endif
 
     if (pEvoNotificationInterface) {
@@ -355,8 +356,10 @@ void Shutdown()
         PrepareShutdown();
     }
 #ifdef ENABLE_WALLET
-    delete pwalletMain;
-    pwalletMain = NULL;
+    for (CWalletRef pwallet : vpwallets) {
+        delete pwallet;
+    }
+    vpwallets.clear();
 #endif
     globalVerifyHandle.reset();
     ECC_Stop();
@@ -933,24 +936,6 @@ void InitParameterInteraction()
         if (gArgs.SoftSetBoolArg("-discover", false))
             LogPrintf("%s : parameter interaction: -externalip set -> setting -discover=0\n", __func__);
     }
-
-    if (gArgs.GetBoolArg("-salvagewallet", false)) {
-        // Rewrite just private keys: rescan to find transactions
-        if (gArgs.SoftSetBoolArg("-rescan", true))
-            LogPrintf("%s : parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__);
-    }
-
-    int zapwallettxes = gArgs.GetArg("-zapwallettxes", 0);
-    // -zapwallettxes implies dropping the mempool on startup
-    if (zapwallettxes != 0 && gArgs.SoftSetBoolArg("-persistmempool", false)) {
-        LogPrintf("%s: parameter interaction: -zapwallettxes=%s -> setting -persistmempool=0\n", __func__, zapwallettxes);
-    }
-
-    // -zapwallettxes implies a rescan
-    if (zapwallettxes != 0) {
-        if (gArgs.SoftSetBoolArg("-rescan", true))
-            LogPrintf("%s : parameter interaction: -zapwallettxes=%s -> setting -rescan=1\n", __func__, zapwallettxes);
-    }
 }
 
 bool InitNUParams()
@@ -1334,11 +1319,8 @@ bool AppInitMain()
         }
     }
 
-// ********************************************************* Step 5: Backup wallet and verify wallet database integrity
+// ********************************************************* Step 5: Verify wallet database integrity
 #ifdef ENABLE_WALLET
-    if (!InitAutoBackupWallet()) {
-        return false;
-    }
     if (!CWallet::Verify()) {
         return false;
     }
@@ -1726,7 +1708,7 @@ bool AppInitMain()
         mempool.ReadFeeEstimates(est_filein);
     fFeeEstimatesInitialized = true;
 
-// ********************************************************* Step 8: load wallet
+// ********************************************************* Step 8: Backup and Load wallet
 #ifdef ENABLE_WALLET
     if (!CWallet::InitLoadWallet())
         return false;
@@ -1878,14 +1860,15 @@ bool AppInitMain()
     strBudgetMode = gArgs.GetArg("-budgetvotemode", "auto");
 
 #ifdef ENABLE_WALLET
-    if (gArgs.GetBoolArg("-mnconflock", DEFAULT_MNCONFLOCK) && pwalletMain) {
-        LOCK(pwalletMain->cs_wallet);
+    // use only the first wallet here. This section can be removed after transition to DMN
+    if (gArgs.GetBoolArg("-mnconflock", DEFAULT_MNCONFLOCK) && !vpwallets.empty() && vpwallets[0]) {
+        LOCK(vpwallets[0]->cs_wallet);
         LogPrintf("Locking Masternodes collateral utxo:\n");
         uint256 mnTxHash;
         for (const auto& mne : masternodeConfig.getEntries()) {
             mnTxHash.SetHex(mne.getTxHash());
             COutPoint outpoint = COutPoint(mnTxHash, (unsigned int) std::stoul(mne.getOutputIndex()));
-            pwalletMain->LockCoin(outpoint);
+            vpwallets[0]->LockCoin(outpoint);
             LogPrintf("Locked collateral, MN: %s, tx hash: %s, output index: %s\n",
                       mne.getAlias(), mne.getTxHash(), mne.getOutputIndex());
         }
@@ -1919,11 +1902,13 @@ bool AppInitMain()
 
 #ifdef ENABLE_WALLET
     {
-        if (pwalletMain) {
-            LOCK(pwalletMain->cs_wallet);
-            LogPrintf("setKeyPool.size() = %u\n", pwalletMain ? pwalletMain->GetKeyPoolSize() : 0);
-            LogPrintf("mapWallet.size() = %u\n", pwalletMain ? pwalletMain->mapWallet.size() : 0);
-            LogPrintf("mapAddressBook.size() = %u\n", pwalletMain ? pwalletMain->GetAddressBookSize() : 0);
+        int idx = 0;
+        for (CWalletRef pwallet : vpwallets) {
+            LogPrintf("Wallet %d\n", idx++);
+            LOCK(pwallet->cs_wallet);
+            LogPrintf("setKeyPool.size() = %u\n", pwallet->GetKeyPoolSize());
+            LogPrintf("mapWallet.size() = %u\n", pwallet->mapWallet.size());
+            LogPrintf("mapAddressBook.size() = %u\n", pwallet->GetAddressBookSize());
         }
     }
 #endif
@@ -1954,22 +1939,21 @@ bool AppInitMain()
         return UIError(strNodeError);
 
 #ifdef ENABLE_WALLET
-    // Generate coins in the background
-    if (pwalletMain)
-        GenerateBitcoins(gArgs.GetBoolArg("-gen", DEFAULT_GENERATE), pwalletMain, gArgs.GetArg("-genproclimit", DEFAULT_GENERATE_PROCLIMIT));
+    // Generate coins in the background (disabled on mainnet. use only wallet 0)
+    if (!vpwallets.empty())
+        GenerateBitcoins(gArgs.GetBoolArg("-gen", DEFAULT_GENERATE), vpwallets[0], gArgs.GetArg("-genproclimit", DEFAULT_GENERATE_PROCLIMIT));
 #endif
 
     // ********************************************************* Step 12: finished
 
 #ifdef ENABLE_WALLET
-    if (pwalletMain) {
-        uiInterface.InitMessage(_("Reaccepting wallet transactions..."));
-        pwalletMain->postInitProcess(scheduler);
-
-        // StakeMiner thread disabled by default on regtest
-        if (gArgs.GetBoolArg("-staking", !Params().IsRegTestNet() && DEFAULT_STAKING)) {
-            threadGroup.create_thread(std::bind(&ThreadStakeMinter));
-        }
+    uiInterface.InitMessage(_("Reaccepting wallet transactions..."));
+    for (CWalletRef pwallet : vpwallets) {
+        pwallet->postInitProcess(scheduler);
+    }
+    // StakeMiner thread disabled by default on regtest
+    if (!vpwallets.empty() && gArgs.GetBoolArg("-staking", !Params().IsRegTestNet() && DEFAULT_STAKING)) {
+        threadGroup.create_thread(std::bind(&ThreadStakeMinter));
     }
 #endif
 

src/masternode.cpp

@@ -251,7 +251,8 @@ bool CMasternodeBroadcast::Create(const std::string& strService,
     }
 
     std::string strError;
-    if (!pwalletMain->GetMasternodeVinAndKeys(txin, pubKeyCollateralAddressNew, keyCollateralAddressNew, strTxHash, strOutputIndex, strError)) {
+    // Use wallet-0 here. Legacy mnb creation can be removed after transition to DMN
+    if (vpwallets.empty() || !vpwallets[0]->GetMasternodeVinAndKeys(txin, pubKeyCollateralAddressNew, keyCollateralAddressNew, strTxHash, strOutputIndex, strError)) {
         strErrorRet = strError; // GetMasternodeVinAndKeys logs this error. Only returned for GUI error notification.
         LogPrint(BCLog::MASTERNODE,"CMasternodeBroadcast::Create -- %s\n", strprintf("Could not allocate txin %s:%s for masternode %s", strTxHash, strOutputIndex, strService));
         return false;

src/miner.cpp

@@ -306,8 +306,8 @@ void GenerateBitcoins(bool fGenerate, CWallet* pwallet, int nThreads)
 void ThreadStakeMinter()
 {
     boost::this_thread::interruption_point();
-    LogPrintf("ThreadStakeMinter started\n");
-    CWallet* pwallet = pwalletMain;
+    LogPrintf("ThreadStakeMinter started. Using wallet-0\n");
+    CWallet* pwallet = vpwallets[0];
     try {
         BitcoinMiner(pwallet, true);
         boost::this_thread::interruption_point();

src/qt/pivx.cpp

@@ -487,8 +487,9 @@ void BitcoinApplication::initializeResult(int retval)
         window->setClientModel(clientModel);
 
 #ifdef ENABLE_WALLET
-        if (pwalletMain) {
-            walletModel = new WalletModel(pwalletMain, optionsModel);
+        // TODO: Expose secondary wallets
+        if (!vpwallets.empty()) {
+            walletModel = new WalletModel(vpwallets[0], optionsModel);
             walletModel->setClientModel(clientModel);
 
             window->addWallet(PIVXGUI::DEFAULT_WALLET, walletModel);

src/rest.cpp

@@ -16,6 +16,7 @@
 #include "utilstrencodings.h"
 #include "validation.h"
 #include "version.h"
+#include "wallet/wallet.h"
 
 #include <boost/algorithm/string.hpp>
 
@@ -60,7 +61,7 @@ struct CCoin {
     }
 };
 
-extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
+extern void TxToJSON(CWallet* const pwallet, const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
 extern UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false);
 extern UniValue mempoolInfoToJSON();
 extern UniValue mempoolToJSON(bool fVerbose = false);
@@ -373,7 +374,7 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart)
 
     case RF_JSON: {
         UniValue objTx(UniValue::VOBJ);
-        TxToJSON(*tx, hashBlock, objTx);
+        TxToJSON(nullptr, *tx, hashBlock, objTx);
         std::string strJSON = objTx.write() + "\n";
         req->WriteHeader("Content-Type", "application/json");
         req->WriteReply(HTTP_OK, strJSON);

src/rpc/blockchain.cpp

@@ -42,7 +42,7 @@ static std::mutex cs_blockchange;
 static std::condition_variable cond_blockchange;
 static CUpdatedBlock latestblock;
 
-extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
+extern void TxToJSON(CWallet* const pwallet, const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
 
 UniValue syncwithvalidationinterfacequeue(const JSONRPCRequest& request)
 {
@@ -147,7 +147,7 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx
         const CTransaction& tx = *txIn;
         if (txDetails) {
             UniValue objTx(UniValue::VOBJ);
-            TxToJSON(tx, UINT256_ZERO, objTx);
+            TxToJSON(nullptr, tx, UINT256_ZERO, objTx);
             txs.push_back(objTx);
         } else
             txs.push_back(tx.GetHash().GetHex());

src/rpc/budget.cpp

@@ -15,6 +15,9 @@
 #include "messagesigner.h"
 #include "rpc/server.h"
 #include "utilmoneystr.h"
+#ifdef ENABLE_WALLET
+#include "wallet/rpcwallet.h"
+#endif
 
 #include <univalue.h>
 
@@ -97,6 +100,11 @@ void checkBudgetInputs(const UniValue& params, std::string &strProposalName, std
 
 UniValue preparebudget(const JSONRPCRequest& request)
 {
+    CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
+
+    if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
+        return NullUniValue;
+
     if (request.fHelp || request.params.size() != 6)
         throw std::runtime_error(
             "preparebudget \"proposal-name\" \"url\" payment-count block-start \"pivx-address\" monthy-payment\n"
@@ -117,13 +125,9 @@ UniValue preparebudget(const JSONRPCRequest& request)
             HelpExampleCli("preparebudget", "\"test-proposal\" \"https://forum.pivx.org/t/test-proposal\" 2 820800 \"D9oc6C3dttUbv8zd7zGNq1qKBGf4ZQ1XEE\" 500") +
             HelpExampleRpc("preparebudget", "\"test-proposal\" \"https://forum.pivx.org/t/test-proposal\" 2 820800 \"D9oc6C3dttUbv8zd7zGNq1qKBGf4ZQ1XEE\" 500"));
 
-    if (!pwalletMain) {
-        throw JSONRPCError(RPC_IN_WARMUP, "Try again after active chain is loaded");
-    }
-
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, pwallet->cs_wallet);
 
-    EnsureWalletIsUnlocked();
+    EnsureWalletIsUnlocked(pwallet);
 
     std::string strProposalName;
     std::string strURL;
@@ -145,19 +149,19 @@ UniValue preparebudget(const JSONRPCRequest& request)
 
     CTransactionRef wtx;
     // make our change address
-    CReserveKey keyChange(pwalletMain);
-    if (!pwalletMain->CreateBudgetFeeTX(wtx, nHash, keyChange, false)) { // 50 PIV collateral for proposal
+    CReserveKey keyChange(pwallet);
+    if (!pwallet->CreateBudgetFeeTX(wtx, nHash, keyChange, false)) { // 50 PIV collateral for proposal
         throw std::runtime_error("Error making collateral transaction for proposal. Please check your wallet balance.");
     }
 
     //send the tx to the network
-    const CWallet::CommitResult& res = pwalletMain->CommitTransaction(wtx, keyChange, g_connman.get());
+    const CWallet::CommitResult& res = pwallet->CommitTransaction(wtx, keyChange, g_connman.get());
     if (res.status != CWallet::CommitStatus::OK)
         throw JSONRPCError(RPC_WALLET_ERROR, res.ToString());
 
     // Store proposal name as a comment
-    assert(pwalletMain->mapWallet.count(wtx->GetHash()));
-    pwalletMain->mapWallet.at(wtx->GetHash()).SetComment("Proposal: " + strProposalName);
+    assert(pwallet->mapWallet.count(wtx->GetHash()));
+    pwallet->mapWallet.at(wtx->GetHash()).SetComment("Proposal: " + strProposalName);
 
     return wtx->GetHash().ToString();
 }