Refactor ChainTip wallet locks

src/wallet/rpcwallet.cpp

@@ -2335,7 +2335,7 @@ UniValue keypoolrefill(const UniValue& params, bool fHelp, const CPubKey& mypk)
 
 static void LockWallet(CWallet* pWallet)
 {
-    LOCK(cs_nWalletUnlockTime);
+    LOCK2(pWallet->cs_wallet, cs_nWalletUnlockTime);
     nWalletUnlockTime = 0;
     pWallet->Lock();
 }

src/wallet/wallet.cpp

@@ -472,6 +472,7 @@ void CWallet::ChainTip(const CBlockIndex *pindex,
                        const CBlock *pblock,
                        boost::optional<std::pair<SproutMerkleTree, SaplingMerkleTree>> added)
 {
+    LOCK2(cs_main, cs_wallet);
     if (added) {
         bool initialDownloadCheck = IsInitialBlockDownload();
         // Prevent witness cache building && consolidation transactions
@@ -517,25 +518,15 @@ void CWallet::ChainTip(const CBlockIndex *pindex,
     if (++nSetChainUpdates >= WITNESS_WRITE_UPDATES || nLastSetChain + (int64_t)WITNESS_WRITE_INTERVAL * 1000000 < nNow) {
         nLastSetChain = nNow;
         nSetChainUpdates = 0;
-        CBlockLocator loc;
+        // The locator must be derived from the pindex used to increment
-        {
+        // the witnesses above; pindex can be behind chainActive.Tip().
-            // The locator must be derived from the pindex used to increment
+        CBlockLocator loc = chainActive.GetLocator(pindex);
-            // the witnesses above; pindex can be behind chainActive.Tip().
+        SetBestChain(loc);
-            LOCK(cs_main);
+        LogPrintf("%s: wrote bestchain to wallet at height=%d\n", __func__, pindex->GetHeight() );
-            loc = chainActive.GetLocator(pindex);
-        }
-        {
-            LOCK(cs_wallet);
-            SetBestChain(loc);
-        }
     }
 }
 
 void CWallet::RunSaplingSweep(int blockHeight) {
-    // Sapling is always active since height=1 of HUSH+HSCs
-    // if (!NetworkUpgradeActive(blockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
-    //     return;
-    // }
     AssertLockHeld(cs_wallet);
     if (!fSweepEnabled) {
         return;
@@ -573,12 +564,7 @@ void CWallet::RunSaplingSweep(int blockHeight) {
 }
 
 void CWallet::RunSaplingConsolidation(int blockHeight) {
-    // Sapling is always active on HUSH+HSCs
+    AssertLockHeld(cs_wallet);
-    //if (!NetworkUpgradeActive(blockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
-    //    return;
-    //}
-
-    LOCK(cs_wallet);
 
     if (!fSaplingConsolidationEnabled) {
         return;
@@ -617,6 +603,7 @@ bool CWallet::CommitAutomatedTx(const CTransaction& tx) {
 
 void CWallet::SetBestChain(const CBlockLocator& loc)
 {
+    AssertLockHeld(cs_wallet);
     CWalletDB walletdb(strWalletFile);
     SetBestChainINTERNAL(walletdb, loc);
 }
@@ -999,7 +986,7 @@ void CWallet::ClearNoteWitnessCache()
 
 void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex)
 {
-    LOCK(cs_wallet);
+    AssertLockHeld(cs_wallet);
 
     extern int32_t HUSH_REWIND;
 
@@ -1041,7 +1028,8 @@ int CWallet::SaplingWitnessMinimumHeight(const uint256& nullifier, int nWitnessH
 
 int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessOnly)
 {
-  LOCK2(cs_main, cs_wallet);
+  AssertLockHeld(cs_main);
+  AssertLockHeld(cs_wallet);
 
   int nWitnessTxIncrement = 0;
   int nWitnessTotalTxCount = mapWallet.size();
@@ -1156,8 +1144,8 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO
 
 void CWallet::BuildWitnessCache(const CBlockIndex* pindex, bool witnessOnly)
 {
-
+  AssertLockHeld(cs_main);
-  LOCK2(cs_main, cs_wallet);
+  AssertLockHeld(cs_wallet);
 
   int startHeight = VerifyAndSetInitialWitness(pindex, witnessOnly) + 1;
 
@@ -1783,7 +1771,8 @@ void CWallet::RescanWallet()
  */
 std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> CWallet::FindMySaplingNotes(const CTransaction &tx) const
 {
-    LOCK(cs_SpendingKeyStore);
+    // LOCK(cs_SpendingKeyStore);
+    LOCK(cs_wallet);
     uint256 hash = tx.GetHash();
     uint32_t nZouts = tx.vShieldedOutput.size();
     if(fDebug && (nZouts > 0)) {
@@ -2453,7 +2442,8 @@ bool CWalletTx::WriteToDisk(CWalletDB *pwalletdb)
 
 void CWallet::ReorderWalletTransactions(std::map<std::pair<int,int>, CWalletTx*> &mapSorted, int64_t &maxOrderPos)
 {
-    LOCK2(cs_main, cs_wallet);
+    AssertLockHeld(cs_main);
+    AssertLockHeld(cs_wallet);
     if(fZdebug)
         fprintf(stderr,"%s: maxOrderPos=%li\n",__func__, maxOrderPos);
 
@@ -2478,11 +2468,11 @@ void CWallet::ReorderWalletTransactions(std::map<std::pair<int,int>, CWalletTx*>
     if(fZdebug)
         fprintf(stderr,"%s: mapSorted.size=%lu\n",__func__, mapSorted.size());
 }
- /**Update the nOrderPos with passed in ordered map.
- */
 
+ /**Update the nOrderPos with passed in ordered map.  */
 void CWallet::UpdateWalletTransactionOrder(std::map<std::pair<int,int>, CWalletTx*> &mapSorted, bool resetOrder) {
-  LOCK2(cs_main, cs_wallet);
+  AssertLockHeld(cs_main);
+  AssertLockHeld(cs_wallet);
 
   int64_t previousPosition = 0;
   std::map<const uint256, CWalletTx*> mapUpdatedTxs;
@@ -2529,11 +2519,9 @@ void CWallet::UpdateWalletTransactionOrder(std::map<std::pair<int,int>, CWalletT
   LogPrintf("%s: Total Transactions Reordered %i, Next Position %i\n ", __func__, mapUpdatedTxs.size(), nOrderPosNext);
 }
 
-/**
+// Delete transactions from the Wallet
- * Delete transactions from the Wallet
- */
 void CWallet::DeleteTransactions(std::vector<uint256> &removeTxs) {
-    LOCK(cs_wallet);
+    AssertLockHeld(cs_wallet);
 
     int numTx = removeTxs.size();
     if(fZdebug)
@@ -2562,7 +2550,8 @@ void CWallet::DeleteTransactions(std::vector<uint256> &removeTxs) {
 }
 
 void CWallet::DeleteWalletTransactions(const CBlockIndex* pindex) {
-      LOCK2(cs_main, cs_wallet);
+      AssertLockHeld(cs_main);
+      AssertLockHeld(cs_wallet);
 
       int nDeleteAfter = (int)fDeleteTransactionsAfterNBlocks;
       bool runCompact = false;

src/wallet/walletdb.cpp

@@ -855,13 +855,13 @@ static bool IsKeyType(string strType)
 
 DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
 {
+    LOCK2(cs_main, pwallet->cs_wallet);
     pwallet->vchDefaultKey = CPubKey();
     CWalletScanState wss;
     bool fNoncriticalErrors = false;
     DBErrors result = DB_LOAD_OK;
 
     try {
-        LOCK(pwallet->cs_wallet);
         int nMinVersion = 0;
         if (Read((string)"minversion", nMinVersion))
         {