From 95a812b5993afe69bfe0fd796428b98ab6282e66 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 31 Jan 2019 17:42:56 -0500 Subject: Rename ScanResult stop_block field Avoid confusion with stop_block argument as suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252038449 --- src/qt/test/wallettests.cpp | 4 ++-- src/wallet/rpcwallet.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 30 +++++++++++++++--------------- src/wallet/wallet.cpp | 10 +++++----- src/wallet/wallet.h | 6 +++--- 5 files changed, 26 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index ee84da0cdf..2b50a2ba81 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -149,8 +149,8 @@ void TestGUI() reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions(locked_chain->getBlockHash(0), {} /* stop_block */, reserver, true /* fUpdate */); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); - QCOMPARE(result.stop_block, chainActive.Tip()->GetBlockHash()); - QVERIFY(result.failed_block.IsNull()); + QCOMPARE(result.last_scanned_block, chainActive.Tip()->GetBlockHash()); + QVERIFY(result.last_failed_block.IsNull()); } wallet->SetBroadcastTransactions(true); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c96a9b0aff..f2898250ca 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3478,7 +3478,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) } UniValue response(UniValue::VOBJ); response.pushKV("start_height", start_height); - response.pushKV("stop_height", result.stop_height ? *result.stop_height : UniValue()); + response.pushKV("stop_height", result.last_scanned_height ? *result.last_scanned_height : UniValue()); return response; } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 8c380f1257..fcb34c3706 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -54,9 +54,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* start_block */, {} /* stop_block */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK(result.failed_block.IsNull()); - BOOST_CHECK(result.stop_block.IsNull()); - BOOST_CHECK(!result.stop_height); + BOOST_CHECK(result.last_failed_block.IsNull()); + BOOST_CHECK(result.last_scanned_block.IsNull()); + BOOST_CHECK(!result.last_scanned_height); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 0); } @@ -69,9 +69,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK(result.failed_block.IsNull()); - BOOST_CHECK_EQUAL(result.stop_block, newTip->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.stop_height, newTip->nHeight); + BOOST_CHECK(result.last_failed_block.IsNull()); + BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); + BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); } @@ -88,9 +88,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); - BOOST_CHECK_EQUAL(result.failed_block, oldTip->GetBlockHash()); - BOOST_CHECK_EQUAL(result.stop_block, newTip->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.stop_height, newTip->nHeight); + BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash()); + BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); + BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); } @@ -106,9 +106,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) reserver.reserve(); CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); - BOOST_CHECK_EQUAL(result.failed_block, newTip->GetBlockHash()); - BOOST_CHECK(result.stop_block.IsNull()); - BOOST_CHECK(!result.stop_height); + BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash()); + BOOST_CHECK(result.last_scanned_block.IsNull()); + BOOST_CHECK(!result.last_scanned_height); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 0); } } @@ -345,9 +345,9 @@ public: reserver.reserve(); CWallet::ScanResult result = wallet->ScanForWalletTransactions(chainActive.Genesis()->GetBlockHash(), {} /* stop_block */, reserver, false /* update */); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK_EQUAL(result.stop_block, chainActive.Tip()->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.stop_height, chainActive.Height()); - BOOST_CHECK(result.failed_block.IsNull()); + BOOST_CHECK_EQUAL(result.last_scanned_block, chainActive.Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(*result.last_scanned_height, chainActive.Height()); + BOOST_CHECK(result.last_failed_block.IsNull()); } ~ListCoinsTestingSetup() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9b643be69a..784cb51cc2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1603,7 +1603,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r ScanResult result = ScanForWalletTransactions(start_block, {} /* stop_block */, reserver, update); if (result.status == ScanResult::FAILURE) { int64_t time_max; - if (!chain().findBlock(result.failed_block, nullptr /* block */, nullptr /* time */, &time_max)) { + if (!chain().findBlock(result.last_failed_block, nullptr /* block */, nullptr /* time */, &time_max)) { throw std::logic_error("ScanForWalletTransactions returned invalid block hash"); } return time_max + TIMESTAMP_WINDOW + 1; @@ -1678,7 +1678,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc // 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.failed_block = block_hash; + result.last_failed_block = block_hash; result.status = ScanResult::FAILURE; break; } @@ -1686,11 +1686,11 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc SyncTransaction(block.vtx[posInBlock], block_hash, posInBlock, fUpdate); } // scan succeeded, record block as most recent successfully scanned - result.stop_block = block_hash; - result.stop_height = *block_height; + result.last_scanned_block = block_hash; + result.last_scanned_height = *block_height; } else { // could not scan block, keep scanning but record this block as the most recent failure - result.failed_block = block_hash; + result.last_failed_block = block_hash; result.status = ScanResult::FAILURE; } if (block_hash == stop_block) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4776b0eacc..cf3fa0aced 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -912,14 +912,14 @@ public: //! Hash and height of most recent block that was successfully scanned. //! Unset if no blocks were scanned due to read errors or the chain //! being empty. - uint256 stop_block; - Optional stop_height; + uint256 last_scanned_block; + Optional last_scanned_height; //! Height of the most recent block that could not be scanned due to //! read errors or pruning. Will be set if status is FAILURE, unset if //! status is SUCCESS, and may or may not be set if status is //! USER_ABORT. - uint256 failed_block; + uint256 last_failed_block; }; ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate); void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; -- cgit v1.2.3 From a8d645c934ff878773332c34347ba2d91b4dee05 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 31 Jan 2019 17:48:51 -0500 Subject: Update ScanForWalletTransactions result comment Suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252038666 --- src/wallet/wallet.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 784cb51cc2..deb4289f17 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1622,10 +1622,11 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r * @param[in] stop_block if not null, the scan will stop at this block instead * of the chain tip * - * @return ScanResult indicating success or failure of the scan. SUCCESS if - * scan was successful. FAILURE if a complete rescan was not possible (due to - * pruning or corruption). USER_ABORT if the rescan was aborted before it - * could complete. + * @return ScanResult returning scan information and indicating success or + * failure. Return status will be set to SUCCESS if scan was + * successful. FAILURE if a complete rescan was not possible (due to + * pruning or corruption). USER_ABORT if the rescan was aborted before + * it could complete. * * @pre Caller needs to make sure start_block (and the optional stop_block) are on * the main chain after to the addition of any new keys you want to detect -- cgit v1.2.3 From db2d0932336552c6d2df15ed82751ca8dee7e37f Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 31 Jan 2019 17:51:41 -0500 Subject: Add suggested rescanblockchain comments From https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252043990 --- src/wallet/rpcwallet.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f2898250ca..94d92a65b6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3459,6 +3459,11 @@ UniValue rescanblockchain(const JSONRPCRequest& request) if (tip_height) { start_block = locked_chain->getBlockHash(start_height); + // If called with a stop_height, set the stop_height here to + // trigger a rescan to that height. + // If called without a stop height, leave stop_height as null here + // so rescan continues to the tip (even if the tip advances during + // rescan). if (stop_height) { stop_block = locked_chain->getBlockHash(*stop_height); } -- cgit v1.2.3 From 2efa66b46429bb1d0f237fcaaff38d9e4d9c5d4e Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 1 Feb 2019 16:11:05 -0500 Subject: Document rescanblockchain returned stop_height being null Suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252031485 --- src/wallet/rpcwallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 94d92a65b6..8a7323b58c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3412,8 +3412,8 @@ UniValue rescanblockchain(const JSONRPCRequest& request) }, RPCResult{ "{\n" - " \"start_height\" (numeric) The block height where the rescan has started.\n" - " \"stop_height\" (numeric) The height of the last rescanned block.\n" + " \"start_height\" (numeric) The block height where the rescan started (the requested height or 0)\n" + " \"stop_height\" (numeric) The height of the last rescanned block. May be null in rare cases if there was a reorg and the call didn't scan any blocks because they were already scanned in the background.\n" "}\n" }, RPCExamples{ -- cgit v1.2.3 From 84adb206fc0ae12c4dd3ea8bd514aff0128dc539 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 1 Feb 2019 16:15:13 -0500 Subject: Fix ScanForWalletTransactions start_block comment Suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252036436 --- src/wallet/wallet.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index deb4289f17..0006396abe 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1617,10 +1617,11 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r * from or to us. If fUpdate is true, found transactions that already * exist in the wallet will be updated. * - * @param[in] start_block if not null, the scan will start at this block instead - * of the genesis block - * @param[in] stop_block if not null, the scan will stop at this block instead - * of the chain tip + * @param[in] start_block Scan starting block. If block is not on the active + * chain, the scan will return SUCCESS immediately. + * @param[in] stop_block Scan ending block. If block is not on the active + * chain, the scan will continue until it reaches the + * chain tip. * * @return ScanResult returning scan information and indicating success or * failure. Return status will be set to SUCCESS if scan was -- cgit v1.2.3 From 2c1fbaa771959f42078d2ca68b8705e92a4ad023 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 1 Feb 2019 16:16:16 -0500 Subject: Drop redundant get_value_or Suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252041954 --- src/wallet/wallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0006396abe..98e6f2f9bf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1722,10 +1722,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc } ShowProgress(strprintf("%s " + _("Rescanning..."), GetDisplayName()), 100); // hide progress dialog in GUI if (block_height && fAbortRescan) { - WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height.get_value_or(0), progress_current); + WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", *block_height, progress_current); result.status = ScanResult::USER_ABORT; } else if (block_height && ShutdownRequested()) { - WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", block_height.get_value_or(0), progress_current); + WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", *block_height, progress_current); result.status = ScanResult::USER_ABORT; } } -- cgit v1.2.3 From aebafd0edf81b546d4b7db9e7f53e9eef2c0073e Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 1 Feb 2019 16:17:43 -0500 Subject: Rename Chain getLocator -> getTipLocator Suggested https://github.com/bitcoin/bitcoin/pull/14711#discussion_r252044389 --- src/interfaces/chain.cpp | 2 +- src/interfaces/chain.h | 2 +- src/wallet/wallet.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 38888be8a3..da810bc5e6 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -123,7 +123,7 @@ class LockImpl : public Chain::Lock CBlockIndex* block = LookupBlockIndex(hash); return block && block->GetAncestor(::chainActive.Height()) == ::chainActive.Tip(); } - CBlockLocator getLocator() override { return ::chainActive.GetLocator(); } + CBlockLocator getTipLocator() override { return ::chainActive.GetLocator(); } Optional findLocatorFork(const CBlockLocator& locator) override { LockAnnotation lock(::cs_main); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 735d5b60df..3a54b9164e 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -96,7 +96,7 @@ public: virtual bool isPotentialTip(const uint256& hash) = 0; //! Get locator for the current chain tip. - virtual CBlockLocator getLocator() = 0; + virtual CBlockLocator getTipLocator() = 0; //! Return height of the latest block common to locator and chain, which //! is guaranteed to be an ancestor of the block used to create the diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 98e6f2f9bf..bdddcd718b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4086,7 +4086,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } auto locked_chain = chain.assumeLocked(); // Temporary. Removed in upcoming lock cleanup - walletInstance->ChainStateFlushed(locked_chain->getLocator()); + walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation InitError(strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile)); @@ -4233,7 +4233,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } walletInstance->WalletLogPrintf("Rescan completed in %15dms\n", GetTimeMillis() - nStart); - walletInstance->ChainStateFlushed(locked_chain->getLocator()); + walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); walletInstance->database->IncrementUpdateCounter(); // Restore wallet transaction metadata after -zapwallettxes=1 -- cgit v1.2.3