Avoid pemanent cs_main/cs_wallet lock during wallet rescans

src/wallet/rpcdump.cpp

@@ -101,6 +101,8 @@ UniValue importprivkey(const JSONRPCRequest& request)
         );
 
 
+    bool fRescan = true;
+    {
         LOCK2(cs_main, pwallet->cs_wallet);
 
         EnsureWalletIsUnlocked(pwallet);
@@ -111,7 +113,6 @@ UniValue importprivkey(const JSONRPCRequest& request)
             strLabel = request.params[1].get_str();
 
         // Whether to perform rescan after import
-    bool fRescan = true;
         if (!request.params[2].isNull())
             fRescan = request.params[2].get_bool();
 
@@ -131,7 +132,6 @@ UniValue importprivkey(const JSONRPCRequest& request)
         CKeyID vchAddress = pubkey.GetID();
         {
             pwallet->MarkDirty();
-
             // We don't know which corresponding address will be used; label them all
             for (const auto& dest : GetAllDestinationsForKey(pubkey)) {
                 pwallet->SetAddressBook(dest, strLabel, "receive");
@@ -142,20 +142,19 @@ UniValue importprivkey(const JSONRPCRequest& request)
                 return NullUniValue;
             }
 
+            // whenever a key is imported, we need to scan the whole chain
+            pwallet->UpdateTimeFirstKey(1);
             pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1;
 
             if (!pwallet->AddKeyPubKey(key, pubkey)) {
                 throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
             }
             pwallet->LearnAllRelatedScripts(pubkey);
-
-        // whenever a key is imported, we need to scan the whole chain
-        pwallet->UpdateTimeFirstKey(1);
-
+        }
+    }
     if (fRescan) {
         pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */);
     }
-    }
 
     return NullUniValue;
 }
@@ -268,6 +267,7 @@ UniValue importaddress(const JSONRPCRequest& request)
     if (!request.params[3].isNull())
         fP2SH = request.params[3].get_bool();
 
+    {
         LOCK2(cs_main, pwallet->cs_wallet);
 
         CTxDestination dest = DecodeDestination(request.params[0].get_str());
@@ -282,7 +282,7 @@ UniValue importaddress(const JSONRPCRequest& request)
         } else {
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script");
         }
-
+    }
     if (fRescan)
     {
         pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */);
@@ -436,6 +436,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
     if (!pubKey.IsFullyValid())
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
 
+    {
         LOCK2(cs_main, pwallet->cs_wallet);
 
         for (const auto& dest : GetAllDestinationsForKey(pubKey)) {
@@ -443,7 +444,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
         }
         ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false);
         pwallet->LearnAllRelatedScripts(pubKey);
-
+    }
     if (fRescan)
     {
         pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */);
@@ -479,18 +480,19 @@ UniValue importwallet(const JSONRPCRequest& request)
     if (fPruneMode)
         throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode");
 
+    int64_t nTimeBegin = 0;
+    bool fGood = true;
+    {
         LOCK2(cs_main, pwallet->cs_wallet);
 
         EnsureWalletIsUnlocked(pwallet);
 
         std::ifstream file;
         file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate);
-    if (!file.is_open())
+        if (!file.is_open()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
-
-    int64_t nTimeBegin = chainActive.Tip()->GetBlockTime();
-
-    bool fGood = true;
+        }
+        nTimeBegin = chainActive.Tip()->GetBlockTime();
 
         int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg());
         file.seekg(0, file.beg);
@@ -563,6 +565,7 @@ UniValue importwallet(const JSONRPCRequest& request)
         file.close();
         pwallet->ShowProgress("", 100); // hide progress dialog in GUI
         pwallet->UpdateTimeFirstKey(nTimeBegin);
+    }
     pwallet->RescanFromTime(nTimeBegin, false /* update */);
     pwallet->MarkDirty();
 
@@ -1135,18 +1138,21 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
         }
     }
 
+    int64_t now = 0;
+    bool fRunScan = false;
+    int64_t nLowestTimestamp = 0;
+    UniValue response(UniValue::VARR);
+    {
         LOCK2(cs_main, pwallet->cs_wallet);
         EnsureWalletIsUnlocked(pwallet);
 
         // Verify all timestamps are present before importing any keys.
-    const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0;
+        now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0;
         for (const UniValue& data : requests.getValues()) {
             GetImportTimestamp(data, now);
         }
 
-    bool fRunScan = false;
         const int64_t minimumTimestamp = 1;
-    int64_t nLowestTimestamp = 0;
 
         if (fRescan && chainActive.Tip()) {
             nLowestTimestamp = chainActive.Tip()->GetBlockTime();
@@ -1154,8 +1160,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
             fRescan = false;
         }
 
-    UniValue response(UniValue::VARR);
-
         for (const UniValue& data : requests.getValues()) {
             const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
             const UniValue result = ProcessImport(pwallet, data, timestamp);
@@ -1175,7 +1179,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
                 nLowestTimestamp = timestamp;
             }
         }
-
+    }
     if (fRescan && fRunScan && requests.size()) {
         int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, true /* update */);
         pwallet->ReacceptWalletTransactions();

src/wallet/rpcwallet.cpp

@@ -3398,10 +3398,18 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
             );
     }
 
-    LOCK2(cs_main, pwallet->cs_wallet);
+    if (pwallet->IsScanning()) {
+        throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
+    }
 
-    CBlockIndex *pindexStart = chainActive.Genesis();
+    CBlockIndex *pindexStart = nullptr;
     CBlockIndex *pindexStop = nullptr;
+    CBlockIndex *pChainTip = nullptr;
+    {
+        LOCK(cs_main);
+        pindexStart = chainActive.Genesis();
+        pChainTip = chainActive.Tip();
+
         if (!request.params[0].isNull()) {
             pindexStart = chainActive[request.params[0].get_int()];
             if (!pindexStart) {
@@ -3418,10 +3426,12 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
                 throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height");
             }
         }
+    }
 
     // We can't rescan beyond non-pruned blocks, stop and throw an error
     if (fPruneMode) {
-        CBlockIndex *block = pindexStop ? pindexStop : chainActive.Tip();
+        LOCK(cs_main);
+        CBlockIndex *block = pindexStop ? pindexStop : pChainTip;
         while (block && block->nHeight >= pindexStart->nHeight) {
             if (!(block->nStatus & BLOCK_HAVE_DATA)) {
                 throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.");
@@ -3436,12 +3446,11 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
             throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted.");
         }
         // if we got a nullptr returned, ScanForWalletTransactions did rescan up to the requested stopindex
-        stopBlock = pindexStop ? pindexStop : chainActive.Tip();
+        stopBlock = pindexStop ? pindexStop : pChainTip;
     }
     else {
         throw JSONRPCError(RPC_MISC_ERROR, "Rescan failed. Potentially corrupted data files.");
     }
-
     UniValue response(UniValue::VOBJ);
     response.pushKV("start_height", pindexStart->nHeight);
     response.pushKV("stop_height", stopBlock->nHeight);

src/wallet/wallet.cpp

@@ -1614,14 +1614,15 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
  */
 int64_t CWallet::RescanFromTime(int64_t startTime, bool update)
 {
-    AssertLockHeld(cs_main);
-    AssertLockHeld(cs_wallet);
-
     // Find starting block. May be null if nCreateTime is greater than the
     // highest blockchain timestamp, in which case there is nothing that needs
     // to be scanned.
-    CBlockIndex* const startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW);
+    CBlockIndex* startBlock = nullptr;
+    {
+        LOCK(cs_main);
+        startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW);
         LogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0);
+    }
 
     if (startBlock) {
         const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, nullptr, update);
@@ -1643,6 +1644,10 @@ int64_t CWallet::RescanFromTime(int64_t startTime, bool update)
  *
  * If pindexStop is not a nullptr, the scan will stop at the block-index
  * defined by pindexStop
+ *
+ * Caller needs to make sure pindexStop (and the optional pindexStart) are on
+ * the main chain after to the addition of any new keys you want to detect
+ * transactions for.
  */
 CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, bool fUpdate)
 {
@@ -1656,24 +1661,49 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock
     CBlockIndex* pindex = pindexStart;
     CBlockIndex* ret = nullptr;
     {
-        LOCK2(cs_main, cs_wallet);
         fAbortRescan = false;
         fScanningWallet = true;
 
         ShowProgress(_("Rescanning..."), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup
-        double dProgressStart = GuessVerificationProgress(chainParams.TxData(), pindex);
-        double dProgressTip = GuessVerificationProgress(chainParams.TxData(), chainActive.Tip());
+        CBlockIndex* tip = nullptr;
+        double dProgressStart;
+        double dProgressTip;
+        {
+            LOCK(cs_main);
+            tip = chainActive.Tip();
+            dProgressStart = GuessVerificationProgress(chainParams.TxData(), pindex);
+            dProgressTip = GuessVerificationProgress(chainParams.TxData(), tip);
+        }
         while (pindex && !fAbortRescan)
         {
-            if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0)
-                ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((GuessVerificationProgress(chainParams.TxData(), pindex) - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
+            if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) {
+                double gvp = 0;
+                {
+                    LOCK(cs_main);
+                    gvp = GuessVerificationProgress(chainParams.TxData(), pindex);
+                }
+                ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((gvp - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
+            }
             if (GetTime() >= nNow + 60) {
                 nNow = GetTime();
+                LOCK(cs_main);
                 LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex));
             }
 
+            bool readRet = false;
             CBlock block;
-            if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) {
+            {
+                LOCK(cs_main);
+                readRet = ReadBlockFromDisk(block, pindex, Params().GetConsensus());
+            }
+            if (readRet) {
+                LOCK2(cs_main, cs_wallet);
+                if (pindex && !chainActive.Contains(pindex)) {
+                    // Abort scan if current block is no longer active, to prevent
+                    // marking transactions as coming from the wrong block.
+                    ret = pindex;
+                    break;
+                }
                 for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
                     AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate);
                 }
@@ -1683,7 +1713,15 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock
             if (pindex == pindexStop) {
                 break;
             }
+            {
+                LOCK(cs_main);
                 pindex = chainActive.Next(pindex);
+                if (tip != chainActive.Tip()) {
+                    tip = chainActive.Tip();
+                    // in case the tip has changed, update progress max
+                    dProgressTip = GuessVerificationProgress(chainParams.TxData(), tip);
+                }
+            }
         }
         if (pindex && fAbortRescan) {
             LogPrintf("Rescan aborted at block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex));