From ccf84bb9c10b4397f1a2aed6cf83fa0172c5cf7f Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 2 Mar 2017 16:14:39 -0500 Subject: [PATCH 1/4] Move birthday optimization out of ScanForWalletTransactions This change has no effect on wallet behavior. On wallet startup, the transaction scan avoids reading any blocks with timestamps older than the wallet birthday (less than nTimeFirstKey - TIMESTAMP_WINDOW). This block skipping code currently resides in CWallet::ScanForWalletTransactions but it doesn't really belong there because it makes the implementation unnecessarily fragile and hard to understand, and it never has any effect except at startup (because all other callers do their rescans based on timestamps other than, but always greater or equal to, nTimeFirstKey). --- src/wallet/wallet.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3e000d2a9..880a6a8bd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1479,11 +1479,6 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f fAbortRescan = false; fScanningWallet = true; - // no need to read and scan block, if block was created before - // our wallet birthday (as adjusted for block time variability) - while (pindex && nTimeFirstKey && (pindex->GetBlockTime() < (nTimeFirstKey - TIMESTAMP_WINDOW))) - pindex = chainActive.Next(pindex); - 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()); @@ -3880,6 +3875,13 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) uiInterface.InitMessage(_("Rescanning...")); LogPrintf("Rescanning last %i blocks (from block %i)...\n", chainActive.Height() - pindexRescan->nHeight, pindexRescan->nHeight); + + // No need to read and scan block if block was created before + // our wallet birthday (as adjusted for block time variability) + while (pindexRescan && walletInstance->nTimeFirstKey && (pindexRescan->GetBlockTime() < (walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW))) { + pindexRescan = chainActive.Next(pindexRescan); + } + nStart = GetTimeMillis(); walletInstance->ScanForWalletTransactions(pindexRescan, true); LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); From 9bb66ab66013fc3024fad00bceeb22a330b5bc1b Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 2 Mar 2017 15:24:50 -0500 Subject: [PATCH 2/4] Add RescanFromTime method and use from rpcdump No change in behavior. --- src/wallet/rpcdump.cpp | 26 +++++++++----------------- src/wallet/wallet.cpp | 32 ++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 3 +++ 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 3efbeafe4..9359887e0 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -149,7 +149,7 @@ UniValue importprivkey(const JSONRPCRequest& request) pwallet->UpdateTimeFirstKey(1); if (fRescan) { - pwallet->ScanForWalletTransactions(chainActive.Genesis(), true); + pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); } } @@ -279,7 +279,7 @@ UniValue importaddress(const JSONRPCRequest& request) if (fRescan) { - pwallet->ScanForWalletTransactions(chainActive.Genesis(), true); + pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -437,7 +437,7 @@ UniValue importpubkey(const JSONRPCRequest& request) if (fRescan) { - pwallet->ScanForWalletTransactions(chainActive.Genesis(), true); + pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -537,11 +537,7 @@ UniValue importwallet(const JSONRPCRequest& request) file.close(); pwallet->ShowProgress("", 100); // hide progress dialog in GUI pwallet->UpdateTimeFirstKey(nTimeBegin); - - CBlockIndex *pindex = chainActive.FindEarliestAtLeast(nTimeBegin - TIMESTAMP_WINDOW); - - LogPrintf("Rescanning last %i blocks\n", pindex ? chainActive.Height() - pindex->nHeight + 1 : 0); - pwallet->ScanForWalletTransactions(pindex); + pwallet->RescanFromTime(nTimeBegin - TIMESTAMP_WINDOW, false /* update */); pwallet->MarkDirty(); if (!fGood) @@ -1117,14 +1113,10 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } if (fRescan && fRunScan && requests.size()) { - CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(std::max(nLowestTimestamp - TIMESTAMP_WINDOW, 0)) : chainActive.Genesis(); - CBlockIndex* scanFailed = nullptr; - if (pindex) { - scanFailed = pwallet->ScanForWalletTransactions(pindex, true); - pwallet->ReacceptWalletTransactions(); - } + int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp - TIMESTAMP_WINDOW, true /* update */); + pwallet->ReacceptWalletTransactions(); - if (scanFailed) { + if (scannedTime > nLowestTimestamp - TIMESTAMP_WINDOW) { std::vector results = response.getValues(); response.clear(); response.setArray(); @@ -1134,7 +1126,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) // range, or if the import result already has an error set, let // the result stand unmodified. Otherwise replace the result // with an error message. - if (GetImportTimestamp(request, now) - TIMESTAMP_WINDOW > scanFailed->GetBlockTimeMax() || results.at(i).exists("error")) { + if (scannedTime <= GetImportTimestamp(request, now) - TIMESTAMP_WINDOW || results.at(i).exists("error")) { response.push_back(results.at(i)); } else { UniValue result = UniValue(UniValue::VOBJ); @@ -1150,7 +1142,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " "and -rescan options).", - GetImportTimestamp(request, now), scanFailed->GetBlockTimeMax(), TIMESTAMP_WINDOW))); + GetImportTimestamp(request, now), scannedTime - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } ++i; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 880a6a8bd..06f8fc045 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -221,6 +221,10 @@ bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector& listReceived, } +/** + * Scan active chain for relevant transactions after importing keys. This should + * be called whenever new keys are added to the wallet, with the oldest key + * creation time minus TIMESTAMP_WINDOW. + * + * @return Earliest timestamp that could be successfully scanned from. Timestamp + * returned may be higher than startTime if some blocks could not be read. + */ +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); + LogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0); + + if (startBlock) { + const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, update); + if (failedBlock) { + return failedBlock->GetBlockTimeMax() + 1; + } + } + return startTime; +} + /** * Scan the block chain (starting in pindexStart) for transactions * from or to us. If fUpdate is true, found transactions that already diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a3974bf00..b6459f765 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -67,6 +67,8 @@ static const bool DEFAULT_USE_HD_WALLET = true; extern const char * DEFAULT_WALLET_DAT; +static const int64_t TIMESTAMP_MIN = 0; + class CBlockIndex; class CCoinControl; class COutput; @@ -918,6 +920,7 @@ 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, bool fUpdate = false); void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; From 5b2be2b787f26ce8a87b742890e43b02cc4779d1 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 22 Jun 2017 17:14:40 -0400 Subject: [PATCH 3/4] Make CWallet::RescanFromTime comment less ambiguous --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 06f8fc045..d65a31ee9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1468,7 +1468,7 @@ void CWalletTx::GetAmounts(std::list& listReceived, * creation time minus TIMESTAMP_WINDOW. * * @return Earliest timestamp that could be successfully scanned from. Timestamp - * returned may be higher than startTime if some blocks could not be read. + * returned will be higher than startTime if relevant blocks could not be read. */ int64_t CWallet::RescanFromTime(int64_t startTime, bool update) { From deaf48b046e573f6774d19e74b58918ed777cb14 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 22 Jun 2017 17:16:24 -0400 Subject: [PATCH 4/4] Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime This way CWallet::RescanFromTime callers don't need to subtract TIMESTAMP_WINDOW themselves. This is pure refactoring, there is no change in behavior. --- src/wallet/rpcdump.cpp | 10 +++++----- src/wallet/wallet.cpp | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 9359887e0..d8fdbd391 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -537,7 +537,7 @@ UniValue importwallet(const JSONRPCRequest& request) file.close(); pwallet->ShowProgress("", 100); // hide progress dialog in GUI pwallet->UpdateTimeFirstKey(nTimeBegin); - pwallet->RescanFromTime(nTimeBegin - TIMESTAMP_WINDOW, false /* update */); + pwallet->RescanFromTime(nTimeBegin, false /* update */); pwallet->MarkDirty(); if (!fGood) @@ -1113,10 +1113,10 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } if (fRescan && fRunScan && requests.size()) { - int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp - TIMESTAMP_WINDOW, true /* update */); + int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, true /* update */); pwallet->ReacceptWalletTransactions(); - if (scannedTime > nLowestTimestamp - TIMESTAMP_WINDOW) { + if (scannedTime > nLowestTimestamp) { std::vector results = response.getValues(); response.clear(); response.setArray(); @@ -1126,7 +1126,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) // range, or if the import result already has an error set, let // the result stand unmodified. Otherwise replace the result // with an error message. - if (scannedTime <= GetImportTimestamp(request, now) - TIMESTAMP_WINDOW || results.at(i).exists("error")) { + if (scannedTime <= GetImportTimestamp(request, now) || results.at(i).exists("error")) { response.push_back(results.at(i)); } else { UniValue result = UniValue(UniValue::VOBJ); @@ -1142,7 +1142,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " "and -rescan options).", - GetImportTimestamp(request, now), scannedTime - 1, TIMESTAMP_WINDOW))); + GetImportTimestamp(request, now), scannedTime - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } ++i; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d65a31ee9..a4197caeb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1465,7 +1465,7 @@ void CWalletTx::GetAmounts(std::list& listReceived, /** * Scan active chain for relevant transactions after importing keys. This should * be called whenever new keys are added to the wallet, with the oldest key - * creation time minus TIMESTAMP_WINDOW. + * creation time. * * @return Earliest timestamp that could be successfully scanned from. Timestamp * returned will be higher than startTime if relevant blocks could not be read. @@ -1478,13 +1478,13 @@ int64_t CWallet::RescanFromTime(int64_t startTime, 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 // to be scanned. - CBlockIndex* const startBlock = chainActive.FindEarliestAtLeast(startTime); + CBlockIndex* const 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, update); if (failedBlock) { - return failedBlock->GetBlockTimeMax() + 1; + return failedBlock->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1; } } return startTime;