diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -177,6 +177,13 @@ findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock &block = {}) = 0; + //! Find next block if block is part of current chain. Also flag if + //! there was a reorg and the specified block hash is no longer in the + //! current chain, and optionally return block information. + virtual bool findNextBlock(const BlockHash &block_hash, int block_height, + const FoundBlock &next = {}, + bool *reorg = nullptr) = 0; + //! Find ancestor of block at specified height and optionally return //! ancestor information. virtual bool findAncestorByHeight(const BlockHash &block_hash, diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -275,6 +275,19 @@ ChainActive().FindEarliestAtLeast(min_time, min_height), block, lock); } + bool findNextBlock(const BlockHash &block_hash, int block_height, + const FoundBlock &next, bool *reorg) override { + WAIT_LOCK(cs_main, lock); + CBlockIndex *block = ChainActive()[block_height]; + if (block && block->GetBlockHash() != block_hash) { + block = nullptr; + } + if (reorg) { + *reorg = !block; + } + return FillBlock(block ? ChainActive()[block_height + 1] : nullptr, + next, lock); + } bool findAncestorByHeight(const BlockHash &block_hash, int ancestor_height, const FoundBlock &ancestor_out) override { diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -135,8 +135,8 @@ WalletRescanReserver reserver(*wallet); reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions( - Params().GetConsensus().hashGenesisBlock, {} /* max_height */, - reserver, true /* fUpdate */); + Params().GetConsensus().hashGenesisBlock, 0 /* block height */, + {} /* max height */, reserver, true /* fUpdate */); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); QCOMPARE(result.last_scanned_block, ::ChainActive().Tip()->GetBlockHash()); diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp --- a/src/test/interfaces_tests.cpp +++ b/src/test/interfaces_tests.cpp @@ -68,6 +68,22 @@ /* min_height= */ 0)); } +BOOST_AUTO_TEST_CASE(findNextBlock) { + auto chain = interfaces::MakeChain(m_node, Params()); + auto &active = ChainActive(); + bool reorg; + BlockHash hash; + BOOST_CHECK(chain->findNextBlock(active[20]->GetBlockHash(), 20, + FoundBlock().hash(hash), &reorg)); + BOOST_CHECK_EQUAL(hash, active[21]->GetBlockHash()); + BOOST_CHECK_EQUAL(reorg, false); + BOOST_CHECK(!chain->findNextBlock(BlockHash(), 20, {}, &reorg)); + BOOST_CHECK_EQUAL(reorg, true); + BOOST_CHECK(!chain->findNextBlock(active.Tip()->GetBlockHash(), + active.Height(), {}, &reorg)); + BOOST_CHECK_EQUAL(reorg, false); +} + BOOST_AUTO_TEST_CASE(findAncestorByHeight) { auto chain = interfaces::MakeChain(m_node, Params()); auto &active = ChainActive(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4036,7 +4036,7 @@ } CWallet::ScanResult result = pwallet->ScanForWalletTransactions( - start_block, stop_height, reserver, true /* fUpdate */); + start_block, start_height, stop_height, reserver, true /* fUpdate */); switch (result.status) { case CWallet::ScanResult::SUCCESS: break; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -46,7 +46,7 @@ auto locked_chain = chain->lock(); LockAssertion lock(::cs_main); - // Verify ScanForWalletTransactions accommodates a null start block. + // Verify ScanForWalletTransactions fails to read an unknown start block. { CWallet wallet(Params(), chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); @@ -59,8 +59,9 @@ WalletRescanReserver reserver(wallet); reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions( - BlockHash(), {} /* max_height */, reserver, false /* update */); - BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); + BlockHash() /* start_block */, 0 /* start_height */, + {} /* max_height */, reserver, false /* update */); + BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK(result.last_scanned_block.IsNull()); BOOST_CHECK(!result.last_scanned_height); @@ -81,8 +82,8 @@ WalletRescanReserver reserver(wallet); reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions( - oldTip->GetBlockHash(), {} /* max_height */, reserver, - false /* update */); + oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, + reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); @@ -108,8 +109,8 @@ WalletRescanReserver reserver(wallet); reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions( - oldTip->GetBlockHash(), {} /* max_height */, reserver, - false /* update */); + oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, + reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); @@ -134,8 +135,8 @@ WalletRescanReserver reserver(wallet); reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions( - oldTip->GetBlockHash(), {} /* max_height */, reserver, - false /* update */); + oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, + reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash()); BOOST_CHECK(result.last_scanned_block.IsNull()); @@ -518,8 +519,8 @@ WalletRescanReserver reserver(*wallet); reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions( - ::ChainActive().Genesis()->GetBlockHash(), {} /* max_height */, - reserver, false /* update */); + ::ChainActive().Genesis()->GetBlockHash(), 0 /* start_height */, + {} /* max_height */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK_EQUAL(result.last_scanned_block, ::ChainActive().Tip()->GetBlockHash()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1068,7 +1068,8 @@ //! USER_ABORT. BlockHash last_failed_block; }; - ScanResult ScanForWalletTransactions(const BlockHash &first_block, + ScanResult ScanForWalletTransactions(const BlockHash &start_block, + int start_height, Optional max_height, const WalletRescanReserver &reserver, bool fUpdate); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1721,7 +1721,7 @@ if (start) { // TODO: this should take into account failure by ScanResult::USER_ABORT ScanResult result = ScanForWalletTransactions( - start_block, {} /* max_height */, reserver, update); + start_block, start_height, {} /* max_height */, reserver, update); if (result.status == ScanResult::FAILURE) { int64_t time_max; CHECK_NONFATAL(chain().findBlock(result.last_failed_block, @@ -1739,6 +1739,7 @@ * * @param[in] start_block Scan starting block. If block is not on the active * chain, the scan will return SUCCESS immediately. + * @param[in] start_height Height of start_block * @param[in] max_height Optional max scanning height. If unset there is * no maximum and scanning can continue to the tip * @@ -1753,7 +1754,7 @@ * transactions for. */ CWallet::ScanResult CWallet::ScanForWalletTransactions( - const BlockHash &start_block, Optional max_height, + const BlockHash &start_block, int start_height, Optional max_height, const WalletRescanReserver &reserver, bool fUpdate) { int64_t nNow = GetTime(); int64_t start_time = GetTimeMillis(); @@ -1767,56 +1768,49 @@ start_block.ToString()); fAbortRescan = false; - - // Show rescan progress in GUI as dialog or on splashscreen, if -rescan - // on startup. + // Show rescan progress in GUI as dialog or on splashscreen, if -rescan on + // startup. ShowProgress( strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), 0); - BlockHash tip_hash; - // The way the 'block_height' is initialized is just a workaround for - // the gcc bug #47679 since version 4.6.0. - Optional block_height = MakeOptional(false, int()); - double progress_begin; - double progress_end; - { - auto locked_chain = chain().lock(); - if (Optional tip_height = locked_chain->getHeight()) { - tip_hash = locked_chain->getBlockHash(*tip_height); - } - block_height = locked_chain->getBlockHeight(block_hash); - BlockHash end_hash = tip_hash; - if (max_height) { - chain().findAncestorByHeight(tip_hash, *max_height, - FoundBlock().hash(end_hash)); - } - progress_begin = chain().guessVerificationProgress(block_hash); - progress_end = chain().guessVerificationProgress(end_hash); - } + BlockHash tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash()); + BlockHash end_hash = tip_hash; + if (max_height) { + chain().findAncestorByHeight(tip_hash, *max_height, + FoundBlock().hash(end_hash)); + } + double progress_begin = chain().guessVerificationProgress(block_hash); + double progress_end = chain().guessVerificationProgress(end_hash); double progress_current = progress_begin; - while (block_height && !fAbortRescan && !chain().shutdownRequested()) { + int block_height = start_height; + while (!fAbortRescan && !chain().shutdownRequested()) { m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin); - if (*block_height % 100 == 0 && progress_end - progress_begin > 0.0) { + if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) { ShowProgress( strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), - std::max(1, std::min(99, int(m_scanning_progress * 100)))); + std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); } if (GetTime() >= nNow + 60) { nNow = GetTime(); WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", - *block_height, progress_current); + block_height, progress_current); } CBlock block; + bool next_block; + BlockHash next_block_hash; + bool reorg = false; if (chain().findBlock(block_hash, FoundBlock().data(block)) && !block.IsNull()) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - if (!locked_chain->getBlockHeight(block_hash)) { - // Abort scan if current block is no longer active, to - // prevent marking transactions as coming from the wrong - // block. + next_block = chain().findNextBlock( + block_hash, block_height, FoundBlock().hash(next_block_hash), + &reorg); + if (reorg) { + // Abort scan if current block is no longer active, to prevent + // marking transactions as coming from the wrong block. // TODO: This should return success instead of failure, see // https://github.com/bitcoin/bitcoin/pull/14711#issuecomment-458342518 result.last_failed_block = block_hash; @@ -1826,40 +1820,42 @@ for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, - *block_height, block_hash, + block_height, block_hash, posInBlock); SyncTransaction(block.vtx[posInBlock], confirm, fUpdate); } // scan succeeded, record block as most recent successfully // scanned result.last_scanned_block = block_hash; - result.last_scanned_height = *block_height; + result.last_scanned_height = block_height; } else { // could not scan block, keep scanning but record this block as // the most recent failure result.last_failed_block = block_hash; result.status = ScanResult::FAILURE; + next_block = chain().findNextBlock( + block_hash, block_height, FoundBlock().hash(next_block_hash), + &reorg); } - if (max_height && *block_height >= *max_height) { + if (max_height && block_height >= *max_height) { break; } { auto locked_chain = chain().lock(); - Optional tip_height = locked_chain->getHeight(); - if (!tip_height || *tip_height <= block_height || - !locked_chain->getBlockHeight(block_hash)) { + if (!next_block || reorg) { // break successfully when rescan has reached the tip, or // previous block is no longer on the chain due to a reorg break; } // increment block and verification progress - block_hash = locked_chain->getBlockHash(++*block_height); + block_hash = next_block_hash; + ++block_height; progress_current = chain().guessVerificationProgress(block_hash); // handle updated tip hash const BlockHash prev_tip_hash = tip_hash; - tip_hash = locked_chain->getBlockHash(*tip_height); + tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash()); if (!max_height && prev_tip_hash != tip_hash) { // in case the tip has changed, update progress max progress_end = chain().guessVerificationProgress(tip_hash); @@ -1873,12 +1869,12 @@ 100); if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", - *block_height, progress_current); + block_height, progress_current); result.status = ScanResult::USER_ABORT; } else if (block_height && chain().shutdownRequested()) { - WalletLogPrintf("Rescan interrupted by shutdown request at block " - "%d. Progress=%f\n", - *block_height, progress_current); + WalletLogPrintf( + "Rescan interrupted by shutdown request at block %d. Progress=%f\n", + block_height, progress_current); result.status = ScanResult::USER_ABORT; } else { WalletLogPrintf("Rescan completed in %15dms\n", @@ -4428,7 +4424,8 @@ walletInstance ->ScanForWalletTransactions( locked_chain->getBlockHash(rescan_height), - {} /* max_height */, reserver, true /* update */) + rescan_height, {} /* max height */, reserver, + true /* update */) .status)) { error = _("Failed to rescan the wallet during initialization"); return nullptr;