LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #15342: Suggested wallet code cleanups from #14711

Browse source 
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
This commit is contained in:
MarcoFalke 2019-02-05 10:38:10 -05:00
commit bbdcc0b0ff
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
7 changed files with 49 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/interfaces/chain.cpp
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);

2
src/interfaces/chain.h
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

4
src/qt/test/wallettests.cpp
View file

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

11
src/wallet/rpcwallet.cpp
View file

@ -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{
@ -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);
}
@ -3478,7 +3483,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;
}

30
src/wallet/test/wallet_tests.cpp
View file

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

36
src/wallet/wallet.cpp
View file

@ -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;
@ -1617,15 +1617,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
@ -1678,7 +1680,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 +1688,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) {
@ -1720,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;
}
}
@ -4084,7 +4086,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());
} 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));
@ -4231,7 +4233,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

6
src/wallet/wallet.h
View file

@ -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<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) override;