Merge #15342: Suggested wallet code cleanups from #14711

aebafd0edf Rename Chain getLocator -> getTipLocator (Russell Yanofsky)
2c1fbaa771 Drop redundant get_value_or (Russell Yanofsky)
84adb206fc Fix ScanForWalletTransactions start_block comment (Russell Yanofsky)
2efa66b464 Document rescanblockchain returned stop_height being null (Russell Yanofsky)
db2d093233 Add suggested rescanblockchain comments (Russell Yanofsky)
a8d645c934 Update ScanForWalletTransactions result comment (Russell Yanofsky)
95a812b599 Rename ScanResult stop_block field (Russell Yanofsky)

Pull request description:

  This implements suggested changes from #14711 review comments that didn't make make it in before merging.

  There are no changes in behavior in this PR, just documentation updates, simplifications, and variable renames.

Tree-SHA512: 39f1a5718195732b70b5e427c3b3e4295ea5af6328a5991763a422051212dfb95383186db0c0504ce2c2782fb61998dfd2fe9851645b7cb4e75d849049483cc8

# Conflicts:
#	src/interfaces/chain.cpp
#	src/qt/test/wallettests.cpp
#	src/wallet/test/wallet_tests.cpp
#	src/wallet/wallet.cpp
This commit is contained in:
MarcoFalke 2019-02-05 10:38:10 -05:00 committed by Pasta
parent 48826b429d
commit dfa040e9b9
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
7 changed files with 49 additions and 43 deletions

View File

@ -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<int> findLocatorFork(const CBlockLocator& locator) override
{
LockAnnotation lock(::cs_main);

View File

@ -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

View File

@ -124,8 +124,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);

View File

@ -3563,8 +3563,8 @@ static UniValue rescanblockchain(const JSONRPCRequest& request)
.ToString() +
"\nResult:\n"
"{\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"
"\nExamples:\n"
+ HelpExampleCli("rescanblockchain", "100000 120000")
@ -3608,6 +3608,11 @@ static 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);
}
@ -3627,7 +3632,7 @@ static 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;
}

View File

@ -56,9 +56,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.GetBalance().m_mine_immature, 0);
}
@ -71,9 +71,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.GetBalance().m_mine_immature, 1000 * COIN);
}
@ -90,9 +90,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.GetBalance().m_mine_immature, 500 * COIN);
}
@ -108,9 +108,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.GetBalance().m_mine_immature, 0);
}
}
@ -353,9 +353,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()

View File

@ -2145,7 +2145,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;
@ -2159,15 +2159,17 @@ 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 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
@ -2220,7 +2222,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;
}
@ -2228,11 +2230,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) {
@ -2262,10 +2264,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.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.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;
}
}
@ -5019,7 +5021,7 @@ std::shared_ptr<CWallet> 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());
// Try to create wallet backup right after new wallet was created
std::string strBackupWarning;
@ -5032,7 +5034,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
return error(strBackupError);
}
}
} 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));
@ -5186,7 +5187,7 @@ std::shared_ptr<CWallet> 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

View File

@ -1007,14 +1007,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<int> stop_height;
uint256 last_scanned_block;
Optional<int> 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, MemPoolRemovalReason reason) override;