mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 15:04:44 +01:00
Code Issues Projects Releases Wiki Activity

wallet: Avoid use of Chain::Lock in CWallet::ScanForWalletTransactions

Browse source 
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change affects behavior in a few small ways.

- If there's no max_height specified, percentage progress is measured ending at
  wallet last processed block instead of node tip

- More consistent error reporting: Early check to see if start_block is on the
  active chain is removed, so start_block is always read and the triggers an
  error if it's unavailable
This commit is contained in:
Russell Yanofsky 2020-01-22 16:53:42 -05:00
parent 1be8ff280c
commit c0d07dc4cb
8 changed files with 65 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/interfaces/chain.cpp
View file

@ -251,6 +251,13 @@ public:
WAIT_LOCK(cs_main, lock);
return FillBlock(ChainActive().FindEarliestAtLeast(min_time, min_height), block, lock);
}
bool findNextBlock(const uint256& 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 uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out) override
{
WAIT_LOCK(cs_main, lock);

5
src/interfaces/chain.h
View file

@ -148,6 +148,11 @@ public:
//! information.
virtual bool 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 uint256& 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 uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out={}) = 0;

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

@ -153,7 +153,7 @@ void TestGUI(interfaces::Node& node)
{
WalletRescanReserver reserver(wallet.get());
reserver.reserve();
CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, {} /* stop_block */, reserver, true /* fUpdate */);
CWallet::ScanResult result = wallet->ScanForWalletTransactions(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());
QVERIFY(result.last_failed_block.IsNull());

15
src/test/interfaces_tests.cpp
View file

@ -59,6 +59,21 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
BOOST_CHECK(!chain->findFirstBlockWithTimeAndHeight(/* min_time= */ active.Tip()->GetBlockTimeMax() + 1, /* min_height= */ 0));
}
BOOST_AUTO_TEST_CASE(findNextBlock)
{
auto chain = interfaces::MakeChain(m_node);
auto& active = ChainActive();
bool reorg;
uint256 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(uint256(), 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);

2
src/wallet/rpcwallet.cpp
View file

@ -3569,7 +3569,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
}
CWallet::ScanResult result =
pwallet->ScanForWalletTransactions(start_block, stop_height, reserver, true /* fUpdate */);
pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, true /* fUpdate */);
switch (result.status) {
case CWallet::ScanResult::SUCCESS:
break;

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

@ -46,7 +46,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
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(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
{
@ -56,8 +56,8 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* start_block */, {} /* stop_block */, reserver, false /* update */);
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* 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);
@ -75,7 +75,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */);
CWallet::ScanResult result = wallet.ScanForWalletTransactions(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());
@ -98,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */);
CWallet::ScanResult result = wallet.ScanForWalletTransactions(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());
@ -120,7 +120,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), {} /* stop_block */, reserver, false /* update */);
CWallet::ScanResult result = wallet.ScanForWalletTransactions(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());
@ -465,7 +465,7 @@ public:
AddKey(*wallet, coinbaseKey);
WalletRescanReserver reserver(wallet.get());
reserver.reserve();
CWallet::ScanResult result = wallet->ScanForWalletTransactions(::ChainActive().Genesis()->GetBlockHash(), {} /* stop_block */, reserver, false /* update */);
CWallet::ScanResult result = wallet->ScanForWalletTransactions(::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());
BOOST_CHECK_EQUAL(*result.last_scanned_height, ::ChainActive().Height());

60
src/wallet/wallet.cpp
View file

@ -1598,7 +1598,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
if (start) {
// TODO: this should take into account failure by ScanResult::USER_ABORT
ScanResult result = ScanForWalletTransactions(start_block, {} /* stop_block */, reserver, update);
ScanResult result = ScanForWalletTransactions(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, FoundBlock().maxTime(time_max)));
@ -1615,6 +1615,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
*
* @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
*
@ -1628,7 +1629,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
* the main chain after to the addition of any new keys you want to detect
* transactions for.
*/
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate)
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate)
{
int64_t nNow = GetTime();
int64_t start_time = GetTimeMillis();
@ -1642,38 +1643,32 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
fAbortRescan = false;
ShowProgress(strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup
uint256 tip_hash;
// The way the 'block_height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0.
Optional<int> block_height = MakeOptional(false, int());
double progress_begin;
double progress_end;
{
auto locked_chain = chain().lock();
if (Optional<int> tip_height = locked_chain->getHeight()) {
tip_hash = locked_chain->getBlockHash(*tip_height);
}
block_height = locked_chain->getBlockHeight(block_hash);
uint256 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);
}
uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash());
uint256 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))));
}
if (GetTime() >= nNow + 60) {
nNow = GetTime();
WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", *block_height, progress_current);
WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", block_height, progress_current);
}
CBlock block;
bool next_block;
uint256 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)) {
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
@ -1683,36 +1678,37 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
break;
}
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *block_height, block_hash, posInBlock);
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, 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<int> 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 uint256 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);
@ -1721,10 +1717,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
}
ShowProgress(strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), 100); // hide progress dialog in GUI
if (block_height && fAbortRescan) {
WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", *block_height, progress_current);
WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", 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", GetTimeMillis() - start_time);
@ -4049,7 +4045,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
{
WalletRescanReserver reserver(walletInstance.get());
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), {} /* stop block */, reserver, true /* update */).status)) {
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) {
error = _("Failed to rescan the wallet during initialization").translated;
return nullptr;
}

2
src/wallet/wallet.h
View file

@ -894,7 +894,7 @@ public:
//! USER_ABORT.
uint256 last_failed_block;
};
ScanResult ScanForWalletTransactions(const uint256& first_block, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, Optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
void transactionRemovedFromMempool(const CTransactionRef &ptx) override;
void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void ResendWalletTransactions();