From 8d0b610fe8d0916404aa9158c525b80b1c581c0e Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 7 Sep 2017 16:29:59 -0700 Subject: [PATCH 1/5] Avoid pemanent cs_main/cs_wallet lock during wallet rescans --- src/wallet/rpcdump.cpp | 334 ++++++++++++++++++++------------------- src/wallet/rpcwallet.cpp | 43 +++-- src/wallet/wallet.cpp | 62 ++++++-- 3 files changed, 245 insertions(+), 194 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 645e776b6..1711b400e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -101,61 +101,60 @@ UniValue importprivkey(const JSONRPCRequest& request) ); - LOCK2(cs_main, pwallet->cs_wallet); - - EnsureWalletIsUnlocked(pwallet); - - std::string strSecret = request.params[0].get_str(); - std::string strLabel = ""; - if (!request.params[1].isNull()) - 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(); + { + LOCK2(cs_main, pwallet->cs_wallet); - if (fRescan && fPruneMode) - throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + EnsureWalletIsUnlocked(pwallet); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strSecret); + std::string strSecret = request.params[0].get_str(); + std::string strLabel = ""; + if (!request.params[1].isNull()) + strLabel = request.params[1].get_str(); - if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); + // Whether to perform rescan after import + if (!request.params[2].isNull()) + fRescan = request.params[2].get_bool(); - CKey key = vchSecret.GetKey(); - if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + if (fRescan && fPruneMode) + throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - CKeyID vchAddress = pubkey.GetID(); - { - pwallet->MarkDirty(); + CBitcoinSecret vchSecret; + bool fGood = vchSecret.SetString(strSecret); - // We don't know which corresponding address will be used; label them all - for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - pwallet->SetAddressBook(dest, strLabel, "receive"); - } + if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - // Don't throw error in case a key is already there - if (pwallet->HaveKey(vchAddress)) { - return NullUniValue; - } + CKey key = vchSecret.GetKey(); + if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); - pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1; + CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); + 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"); + } - if (!pwallet->AddKeyPubKey(key, pubkey)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); - } - pwallet->LearnAllRelatedScripts(pubkey); + // Don't throw error in case a key is already there + if (pwallet->HaveKey(vchAddress)) { + return NullUniValue; + } - // whenever a key is imported, we need to scan the whole chain - pwallet->UpdateTimeFirstKey(1); + // whenever a key is imported, we need to scan the whole chain + pwallet->UpdateTimeFirstKey(1); + pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1; - if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + if (!pwallet->AddKeyPubKey(key, pubkey)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); + } + pwallet->LearnAllRelatedScripts(pubkey); } } + if (fRescan) { + pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + } return NullUniValue; } @@ -268,21 +267,22 @@ UniValue importaddress(const JSONRPCRequest& request) if (!request.params[3].isNull()) fP2SH = request.params[3].get_bool(); - LOCK2(cs_main, pwallet->cs_wallet); + { + LOCK2(cs_main, pwallet->cs_wallet); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); - if (IsValidDestination(dest)) { - if (fP2SH) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); + CTxDestination dest = DecodeDestination(request.params[0].get_str()); + if (IsValidDestination(dest)) { + if (fP2SH) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); + } + ImportAddress(pwallet, dest, strLabel); + } else if (IsHex(request.params[0].get_str())) { + std::vector data(ParseHex(request.params[0].get_str())); + ImportScript(pwallet, CScript(data.begin(), data.end()), strLabel, fP2SH); + } else { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } - ImportAddress(pwallet, dest, strLabel); - } else if (IsHex(request.params[0].get_str())) { - std::vector data(ParseHex(request.params[0].get_str())); - ImportScript(pwallet, CScript(data.begin(), data.end()), strLabel, fP2SH); - } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } - if (fRescan) { pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); @@ -436,14 +436,15 @@ 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); + { + LOCK2(cs_main, pwallet->cs_wallet); - for (const auto& dest : GetAllDestinationsForKey(pubKey)) { - ImportAddress(pwallet, dest, strLabel); + for (const auto& dest : GetAllDestinationsForKey(pubKey)) { + ImportAddress(pwallet, dest, strLabel); + } + ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); + pwallet->LearnAllRelatedScripts(pubKey); } - ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); - pwallet->LearnAllRelatedScripts(pubKey); - if (fRescan) { pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); @@ -479,90 +480,92 @@ UniValue importwallet(const JSONRPCRequest& request) if (fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); - LOCK2(cs_main, pwallet->cs_wallet); + int64_t nTimeBegin = 0; + bool fGood = true; + { + LOCK2(cs_main, pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(pwallet); - std::ifstream file; - file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate); - if (!file.is_open()) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + std::ifstream file; + file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate); + if (!file.is_open()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + } + nTimeBegin = chainActive.Tip()->GetBlockTime(); - int64_t nTimeBegin = chainActive.Tip()->GetBlockTime(); + int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); + file.seekg(0, file.beg); - bool fGood = true; + pwallet->ShowProgress(_("Importing..."), 0); // show progress dialog in GUI + while (file.good()) { + pwallet->ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100)))); + std::string line; + std::getline(file, line); + if (line.empty() || line[0] == '#') + continue; - int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); - file.seekg(0, file.beg); - - pwallet->ShowProgress(_("Importing..."), 0); // show progress dialog in GUI - while (file.good()) { - pwallet->ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100)))); - std::string line; - std::getline(file, line); - if (line.empty() || line[0] == '#') - continue; - - std::vector vstr; - boost::split(vstr, line, boost::is_any_of(" ")); - if (vstr.size() < 2) - continue; - CBitcoinSecret vchSecret; - if (vchSecret.SetString(vstr[0])) { - CKey key = vchSecret.GetKey(); - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - CKeyID keyid = pubkey.GetID(); - if (pwallet->HaveKey(keyid)) { - LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid)); + std::vector vstr; + boost::split(vstr, line, boost::is_any_of(" ")); + if (vstr.size() < 2) continue; - } - int64_t nTime = DecodeDumpTime(vstr[1]); - std::string strLabel; - bool fLabel = true; - for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { - if (boost::algorithm::starts_with(vstr[nStr], "#")) - break; - if (vstr[nStr] == "change=1") - fLabel = false; - if (vstr[nStr] == "reserve=1") - fLabel = false; - if (boost::algorithm::starts_with(vstr[nStr], "label=")) { - strLabel = DecodeDumpString(vstr[nStr].substr(6)); - fLabel = true; + CBitcoinSecret vchSecret; + if (vchSecret.SetString(vstr[0])) { + CKey key = vchSecret.GetKey(); + CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); + CKeyID keyid = pubkey.GetID(); + if (pwallet->HaveKey(keyid)) { + LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid)); + continue; } + int64_t nTime = DecodeDumpTime(vstr[1]); + std::string strLabel; + bool fLabel = true; + for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { + if (boost::algorithm::starts_with(vstr[nStr], "#")) + break; + if (vstr[nStr] == "change=1") + fLabel = false; + if (vstr[nStr] == "reserve=1") + fLabel = false; + if (boost::algorithm::starts_with(vstr[nStr], "label=")) { + strLabel = DecodeDumpString(vstr[nStr].substr(6)); + fLabel = true; + } + } + LogPrintf("Importing %s...\n", EncodeDestination(keyid)); + if (!pwallet->AddKeyPubKey(key, pubkey)) { + fGood = false; + continue; + } + pwallet->mapKeyMetadata[keyid].nCreateTime = nTime; + if (fLabel) + pwallet->SetAddressBook(keyid, strLabel, "receive"); + nTimeBegin = std::min(nTimeBegin, nTime); + } else if(IsHex(vstr[0])) { + std::vector vData(ParseHex(vstr[0])); + CScript script = CScript(vData.begin(), vData.end()); + if (pwallet->HaveCScript(script)) { + LogPrintf("Skipping import of %s (script already present)\n", vstr[0]); + continue; + } + if(!pwallet->AddCScript(script)) { + LogPrintf("Error importing script %s\n", vstr[0]); + fGood = false; + continue; + } + int64_t birth_time = DecodeDumpTime(vstr[1]); + if (birth_time > 0) { + pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time; + nTimeBegin = std::min(nTimeBegin, birth_time); + } } - LogPrintf("Importing %s...\n", EncodeDestination(keyid)); - if (!pwallet->AddKeyPubKey(key, pubkey)) { - fGood = false; - continue; - } - pwallet->mapKeyMetadata[keyid].nCreateTime = nTime; - if (fLabel) - pwallet->SetAddressBook(keyid, strLabel, "receive"); - nTimeBegin = std::min(nTimeBegin, nTime); - } else if(IsHex(vstr[0])) { - std::vector vData(ParseHex(vstr[0])); - CScript script = CScript(vData.begin(), vData.end()); - if (pwallet->HaveCScript(script)) { - LogPrintf("Skipping import of %s (script already present)\n", vstr[0]); - continue; - } - if(!pwallet->AddCScript(script)) { - LogPrintf("Error importing script %s\n", vstr[0]); - fGood = false; - continue; - } - int64_t birth_time = DecodeDumpTime(vstr[1]); - if (birth_time > 0) { - pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time; - nTimeBegin = std::min(nTimeBegin, birth_time); - } } + file.close(); + pwallet->ShowProgress("", 100); // hide progress dialog in GUI + pwallet->UpdateTimeFirstKey(nTimeBegin); } - file.close(); - pwallet->ShowProgress("", 100); // hide progress dialog in GUI - pwallet->UpdateTimeFirstKey(nTimeBegin); pwallet->RescanFromTime(nTimeBegin, false /* update */); pwallet->MarkDirty(); @@ -685,7 +688,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) file << strprintf("# mined on %s\n", EncodeDumpTime(chainActive.Tip()->GetBlockTime())); file << "\n"; - // add the base58check encoded extended master if the wallet uses HD + // add the base58check encoded extended master if the wallet uses HD CKeyID masterKeyID = pwallet->GetHDChain().masterKeyID; if (!masterKeyID.IsNull()) { @@ -1135,47 +1138,48 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } } - 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; - for (const UniValue& data : requests.getValues()) { - GetImportTimestamp(data, now); - } - + int64_t now = 0; bool fRunScan = false; - const int64_t minimumTimestamp = 1; int64_t nLowestTimestamp = 0; - - if (fRescan && chainActive.Tip()) { - nLowestTimestamp = chainActive.Tip()->GetBlockTime(); - } else { - fRescan = false; - } - UniValue response(UniValue::VARR); + { + LOCK2(cs_main, pwallet->cs_wallet); + EnsureWalletIsUnlocked(pwallet); - for (const UniValue& data : requests.getValues()) { - const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp); - const UniValue result = ProcessImport(pwallet, data, timestamp); - response.push_back(result); - - if (!fRescan) { - continue; + // Verify all timestamps are present before importing any keys. + now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0; + for (const UniValue& data : requests.getValues()) { + GetImportTimestamp(data, now); } - // If at least one request was successful then allow rescan. - if (result["success"].get_bool()) { - fRunScan = true; + const int64_t minimumTimestamp = 1; + + if (fRescan && chainActive.Tip()) { + nLowestTimestamp = chainActive.Tip()->GetBlockTime(); + } else { + fRescan = false; } - // Get the lowest timestamp. - if (timestamp < nLowestTimestamp) { - nLowestTimestamp = timestamp; + for (const UniValue& data : requests.getValues()) { + const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp); + const UniValue result = ProcessImport(pwallet, data, timestamp); + response.push_back(result); + + if (!fRescan) { + continue; + } + + // If at least one request was successful then allow rescan. + if (result["success"].get_bool()) { + fRunScan = true; + } + + // Get the lowest timestamp. + if (timestamp < nLowestTimestamp) { + nLowestTimestamp = timestamp; + } } } - if (fRescan && fRunScan && requests.size()) { int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, true /* update */); pwallet->ReacceptWalletTransactions(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e307623fd..bc0d544aa 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3398,30 +3398,40 @@ 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; - if (!request.params[0].isNull()) { - pindexStart = chainActive[request.params[0].get_int()]; - if (!pindexStart) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid start_height"); - } - } + CBlockIndex *pChainTip = nullptr; + { + LOCK(cs_main); + pindexStart = chainActive.Genesis(); + pChainTip = chainActive.Tip(); - if (!request.params[1].isNull()) { - pindexStop = chainActive[request.params[1].get_int()]; - if (!pindexStop) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); + if (!request.params[0].isNull()) { + pindexStart = chainActive[request.params[0].get_int()]; + if (!pindexStart) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid start_height"); + } } - else if (pindexStop->nHeight < pindexStart->nHeight) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height"); + + if (!request.params[1].isNull()) { + pindexStop = chainActive[request.params[1].get_int()]; + if (!pindexStop) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); + } + else if (pindexStop->nHeight < pindexStart->nHeight) { + 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); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index da721ea00..450ef4c49 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1614,14 +1614,15 @@ void CWalletTx::GetAmounts(std::list& 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); - LogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0); + 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; } - pindex = chainActive.Next(pindex); + { + 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)); From dbf8556b4d6a2484ad4c03d0b4e41c1db0133997 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Fri, 8 Dec 2017 11:07:37 -1000 Subject: [PATCH 2/5] Add RAII wallet rescan reserver --- src/wallet/rpcdump.cpp | 20 ++++++++++++++++++++ src/wallet/rpcwallet.cpp | 3 ++- src/wallet/wallet.h | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 1711b400e..a0b57ecd2 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -262,6 +262,11 @@ UniValue importaddress(const JSONRPCRequest& request) if (fRescan && fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + // Whether to import a p2sh version, too bool fP2SH = false; if (!request.params[3].isNull()) @@ -429,6 +434,11 @@ UniValue importpubkey(const JSONRPCRequest& request) if (fRescan && fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + if (!IsHex(request.params[0].get_str())) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); std::vector data(ParseHex(request.params[0].get_str())); @@ -480,6 +490,11 @@ UniValue importwallet(const JSONRPCRequest& request) if (fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); + WalletRescanReserver reserver(pwallet); + if (!reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + int64_t nTimeBegin = 0; bool fGood = true; { @@ -1138,6 +1153,11 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } } + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + int64_t now = 0; bool fRunScan = false; int64_t nLowestTimestamp = 0; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index bc0d544aa..50642e6b8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3398,7 +3398,8 @@ UniValue rescanblockchain(const JSONRPCRequest& request) ); } - if (pwallet->IsScanning()) { + WalletRescanReserver reserver(pwallet); + if (!reserver.reserve()) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 53a2c6b9d..5d782faa6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -669,6 +669,9 @@ private: static std::atomic fFlushScheduled; std::atomic fAbortRescan; std::atomic fScanningWallet; + std::mutex mutexScanning; + friend class WalletRescanReserver; + /** * Select a set of coins such that nValueRet >= nTargetValue and at least @@ -1263,4 +1266,34 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType); /** Get all destinations (potentially) supported by the wallet for the given key. */ std::vector GetAllDestinationsForKey(const CPubKey& key); +/** RAII object to check and reserve a wallet rescan */ +class WalletRescanReserver +{ +private: + CWalletRef m_wallet; + bool m_could_reserve; +public: + explicit WalletRescanReserver(CWalletRef w) : m_wallet(w), m_could_reserve(false) {} + + bool reserve() + { + assert(!m_could_reserve); + std::lock_guard lock(m_wallet->mutexScanning); + if (m_wallet->fScanningWallet) { + return false; + } + m_wallet->fScanningWallet = true; + m_could_reserve = true; + return true; + } + + ~WalletRescanReserver() + { + std::lock_guard lock(m_wallet->mutexScanning); + if (m_could_reserve) { + m_wallet->fScanningWallet = false; + } + } +}; + #endif // BITCOIN_WALLET_WALLET_H From bc356b4268e222ac57d9e9297d2a986bb6e09de8 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 12 Dec 2017 13:13:58 -1000 Subject: [PATCH 3/5] Make sure WalletRescanReserver has successfully reserved the rescan --- src/qt/test/wallettests.cpp | 4 +++- src/wallet/rpcdump.cpp | 15 ++++++++++----- src/wallet/rpcwallet.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 12 +++++++++--- src/wallet/wallet.cpp | 20 ++++++++++++-------- src/wallet/wallet.h | 12 +++++++++--- 6 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index a270e5de5..cd4929213 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -169,7 +169,9 @@ void TestGUI() } { LOCK(cs_main); - wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true); } wallet.SetBroadcastTransactions(true); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index a0b57ecd2..936432bac 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -101,6 +101,7 @@ UniValue importprivkey(const JSONRPCRequest& request) ); + WalletRescanReserver reserver(pwallet); bool fRescan = true; { LOCK2(cs_main, pwallet->cs_wallet); @@ -119,6 +120,10 @@ UniValue importprivkey(const JSONRPCRequest& request) if (fRescan && fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + CBitcoinSecret vchSecret; bool fGood = vchSecret.SetString(strSecret); @@ -153,7 +158,7 @@ UniValue importprivkey(const JSONRPCRequest& request) } } if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); } return NullUniValue; @@ -290,7 +295,7 @@ UniValue importaddress(const JSONRPCRequest& request) } if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -457,7 +462,7 @@ UniValue importpubkey(const JSONRPCRequest& request) } if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -581,7 +586,7 @@ UniValue importwallet(const JSONRPCRequest& request) pwallet->ShowProgress("", 100); // hide progress dialog in GUI pwallet->UpdateTimeFirstKey(nTimeBegin); } - pwallet->RescanFromTime(nTimeBegin, false /* update */); + pwallet->RescanFromTime(nTimeBegin, reserver, false /* update */); pwallet->MarkDirty(); if (!fGood) @@ -1201,7 +1206,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } } if (fRescan && fRunScan && requests.size()) { - int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, true /* update */); + int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); if (scannedTime > nLowestTimestamp) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 50642e6b8..7188bb40b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3441,7 +3441,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) } } - CBlockIndex *stopBlock = pwallet->ScanForWalletTransactions(pindexStart, pindexStop, true); + CBlockIndex *stopBlock = pwallet->ScanForWalletTransactions(pindexStart, pindexStop, reserver, true); if (!stopBlock) { if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted."); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7b3c283f3..7e0881afd 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -384,7 +384,9 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { CWallet wallet; AddKey(wallet, coinbaseKey); - BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip, nullptr)); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip, nullptr, reserver)); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); } @@ -397,7 +399,9 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { CWallet wallet; AddKey(wallet, coinbaseKey); - BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip, nullptr)); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip, nullptr, reserver)); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); } @@ -608,7 +612,9 @@ public: bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); - wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr); + WalletRescanReserver reserver(wallet.get()); + reserver.reserve(); + wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver); } ~ListCoinsTestingSetup() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 450ef4c49..3bf649f26 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1612,7 +1612,7 @@ void CWalletTx::GetAmounts(std::list& listReceived, * @return Earliest timestamp that could be successfully scanned from. Timestamp * returned will be higher than startTime if relevant blocks could not be read. */ -int64_t CWallet::RescanFromTime(int64_t startTime, bool update) +int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update) { // Find starting block. May be null if nCreateTime is greater than the // highest blockchain timestamp, in which case there is nothing that needs @@ -1625,7 +1625,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, bool update) } if (startBlock) { - const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, nullptr, update); + const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, nullptr, reserver, update); if (failedBlock) { return failedBlock->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1; } @@ -1649,11 +1649,12 @@ int64_t CWallet::RescanFromTime(int64_t startTime, bool update) * 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) +CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver &reserver, bool fUpdate) { int64_t nNow = GetTime(); const CChainParams& chainParams = Params(); + assert(reserver.isReserved()); if (pindexStop) { assert(pindexStop->nHeight >= pindexStart->nHeight); } @@ -1662,8 +1663,6 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock CBlockIndex* ret = nullptr; { fAbortRescan = false; - fScanningWallet = true; - ShowProgress(_("Rescanning..."), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup CBlockIndex* tip = nullptr; double dProgressStart; @@ -1727,8 +1726,6 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock LogPrintf("Rescan aborted at block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex)); } ShowProgress(_("Rescanning..."), 100); // hide progress dialog in GUI - - fScanningWallet = false; } return ret; } @@ -4039,7 +4036,14 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) } nStart = GetTimeMillis(); - walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, true); + { + WalletRescanReserver reserver(walletInstance); + if (!reserver.reserve()) { + InitError(_("Failed to rescan the wallet during initialization")); + return nullptr; + } + walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, true); + } LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); walletInstance->SetBestChain(chainActive.GetLocator()); walletInstance->dbw->IncrementUpdateCounter(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5d782faa6..70ced30e4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -659,6 +659,7 @@ private: }; +class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime /** * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. @@ -668,7 +669,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface private: static std::atomic fFlushScheduled; std::atomic fAbortRescan; - std::atomic fScanningWallet; + std::atomic fScanningWallet; //controlled by WalletRescanReserver std::mutex mutexScanning; friend class WalletRescanReserver; @@ -948,8 +949,8 @@ public: void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; void BlockDisconnected(const std::shared_ptr& pblock) override; bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); - int64_t RescanFromTime(int64_t startTime, bool update); - CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, bool fUpdate = false); + int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); + CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver& reserver, bool fUpdate = false); void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; @@ -1287,6 +1288,11 @@ public: return true; } + bool isReserved() const + { + return (m_could_reserve && m_wallet->fScanningWallet); + } + ~WalletRescanReserver() { std::lock_guard lock(m_wallet->mutexScanning); From ccd8ef65f93ed82a87cee634660bed3ac17d9eb5 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 13 Dec 2017 11:06:51 -1000 Subject: [PATCH 4/5] Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock --- src/validation.cpp | 8 +++++++- src/wallet/wallet.cpp | 7 +------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 14d60bb26..8cee0dfac 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1121,7 +1121,13 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus: bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams) { - if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), consensusParams)) + CDiskBlockPos blockPos; + { + LOCK(cs_main); + blockPos = pindex->GetBlockPos(); + } + + if (!ReadBlockFromDisk(block, blockPos, consensusParams)) return false; if (block.GetHash() != pindex->GetBlockHash()) return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s", diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3bf649f26..dd9bc6472 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1689,13 +1689,8 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex)); } - bool readRet = false; CBlock block; - { - LOCK(cs_main); - readRet = ReadBlockFromDisk(block, pindex, Params().GetConsensus()); - } - if (readRet) { + if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { LOCK2(cs_main, cs_wallet); if (pindex && !chainActive.Contains(pindex)) { // Abort scan if current block is no longer active, to prevent From 7f812502b78b76653bbef19169e8a05873ee3b8d Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Fri, 5 Jan 2018 10:43:31 -1000 Subject: [PATCH 5/5] Mention that other RPC calls report keys as "imported" while txns are still missing --- src/wallet/rpcdump.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 936432bac..0b021f9fe 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -86,7 +86,8 @@ UniValue importprivkey(const JSONRPCRequest& request) "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "\nExamples:\n" "\nDump a private key\n" + HelpExampleCli("dumpprivkey", "\"myaddress\"") + @@ -241,7 +242,8 @@ UniValue importaddress(const JSONRPCRequest& request) "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" "4. p2sh (boolean, optional, default=false) Add the P2SH version of the script as well\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" "as change, and not show up in many RPCs.\n" @@ -416,7 +418,8 @@ UniValue importpubkey(const JSONRPCRequest& request) "1. \"pubkey\" (string, required) The hex-encoded public key\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "\nExamples:\n" "\nImport a public key with rescan\n" + HelpExampleCli("importpubkey", "\"mypubkey\"") + @@ -1133,6 +1136,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) " {\n" " \"rescan\": , (boolean, optional, default: true) Stating if should rescan the blockchain after all imports\n" " }\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported keys, addresses or scripts exists but related transactions are still missing.\n" "\nExamples:\n" + HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"\" }, \"timestamp\":1455191478 }, " "{ \"scriptPubKey\": { \"address\": \"\" }, \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") +