Skip to content

Commit 7ba0850

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#25036: wallet: Save wallet scan progress
230a2f4 wallet test: Add unit test for wallet scan save_progress option (Ryan Ofsky) a89ddfb wallet: Save wallet scan progress (w0xlt) Pull request description: Currently, the wallet scan progress is not saved. If it is interrupted, it will be necessary to start from scratch on the next load. This PR changes this and the progress is saved right after checking a block. Close bitcoin/bitcoin#25010 ACKs for top commit: furszy: re-ACK 230a2f4 achow101: ACK 230a2f4 ryanofsky: Code review ACK 230a2f4. Only change since last review is tweaking whitespace and adding log print Tree-SHA512: 1a9dec207ed22b3443fb06a4daf967637bc02bcaf71c070b7dc33605d0cab959551e4014c9e92293a63f54c5cbcc98bb9f8844a8c60bc32a1482b1c4130fab32
2 parents c92eb6c + 230a2f4 commit 7ba0850

File tree

8 files changed

+68
-24
lines changed

8 files changed

+68
-24
lines changed

‎src/interfaces/chain.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ class Chain
111111
//! Get locator for the current chain tip.
112112
virtual CBlockLocator getTipLocator() = 0;
113113

114+
//! Return a locator that refers to a block in the active chain.
115+
//! If specified block is not in the active chain, return locator for the latest ancestor that is in the chain.
116+
virtual CBlockLocator getActiveChainLocator(const uint256& block_hash) = 0;
117+
114118
//! Return height of the highest block on chain in common with the locator,
115119
//! which will either be the original block used to create the locator,
116120
//! or one of its ancestors.

‎src/node/interfaces.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -524,20 +524,27 @@ class ChainImpl : public Chain
524524
}
525525
bool haveBlockOnDisk(int height) override
526526
{
527-
LOCK(cs_main);
527+
LOCK(::cs_main);
528528
const CChain& active = Assert(m_node.chainman)->ActiveChain();
529529
CBlockIndex* block = active[height];
530530
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
531531
}
532532
CBlockLocator getTipLocator() override
533533
{
534-
LOCK(cs_main);
534+
LOCK(::cs_main);
535535
const CChain& active = Assert(m_node.chainman)->ActiveChain();
536536
return active.GetLocator();
537537
}
538+
CBlockLocator getActiveChainLocator(const uint256& block_hash) override
539+
{
540+
LOCK(::cs_main);
541+
const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash);
542+
if (!index) return {};
543+
return chainman().ActiveChain().GetLocator(index);
544+
}
538545
std::optional<int> findLocatorFork(const CBlockLocator& locator) override
539546
{
540-
LOCK(cs_main);
547+
LOCK(::cs_main);
541548
const CChainState& active = Assert(m_node.chainman)->ActiveChainstate();
542549
if (const CBlockIndex* fork = active.FindForkInGlobalIndex(locator)) {
543550
return fork->nHeight;
@@ -593,7 +600,7 @@ class ChainImpl : public Chain
593600
void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
594601
double guessVerificationProgress(const uint256& block_hash) override
595602
{
596-
LOCK(cs_main);
603+
LOCK(::cs_main);
597604
return GuessVerificationProgress(chainman().GetParams().TxData(), chainman().m_blockman.LookupBlockIndex(block_hash));
598605
}
599606
bool hasBlocks(const uint256& block_hash, int min_height, std::optional<int> max_height) override
@@ -693,7 +700,7 @@ class ChainImpl : public Chain
693700
CFeeRate relayDustFee() override { return ::dustRelayFee; }
694701
bool havePruned() override
695702
{
696-
LOCK(cs_main);
703+
LOCK(::cs_main);
697704
return m_node.chainman->m_blockman.m_have_pruned;
698705
}
699706
bool isReadyToBroadcast() override { return !node::fImporting && !node::fReindex && !isInitialBlockDownload(); }

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ void TestGUI(interfaces::Node& node)
173173
{
174174
WalletRescanReserver reserver(*wallet);
175175
reserver.reserve();
176-
CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, 0 /* block height */, {} /* max height */, reserver, true /* fUpdate */);
176+
CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/false);
177177
QCOMPARE(result.status, CWallet::ScanResult::SUCCESS);
178178
QCOMPARE(result.last_scanned_block, node.context()->chainman->ActiveChain().Tip()->GetBlockHash());
179179
QVERIFY(result.last_failed_block.IsNull());

‎src/wallet/rpc/transactions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ RPCHelpMan rescanblockchain()
889889
}
890890

891891
CWallet::ScanResult result =
892-
pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, true /* fUpdate */);
892+
pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, /*fUpdate=*/true, /*save_progress=*/false);
893893
switch (result.status) {
894894
case CWallet::ScanResult::SUCCESS:
895895
break;

‎src/wallet/test/util.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
3838
}
3939
WalletRescanReserver reserver(*wallet);
4040
reserver.reserve();
41-
CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), 0 /* start_height */, {} /* max_height */, reserver, false /* update */);
41+
CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
4242
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
4343
BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash());
4444
BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height());

‎src/wallet/test/wallet_tests.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
112112
AddKey(wallet, coinbaseKey);
113113
WalletRescanReserver reserver(wallet);
114114
reserver.reserve();
115-
CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* start_block */, 0 /* start_height */, {} /* max_height */, reserver, false /* update */);
115+
CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/{}, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
116116
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE);
117117
BOOST_CHECK(result.last_failed_block.IsNull());
118118
BOOST_CHECK(result.last_scanned_block.IsNull());
@@ -123,21 +123,36 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
123123
// Verify ScanForWalletTransactions picks up transactions in both the old
124124
// and new block files.
125125
{
126-
CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase());
126+
CWallet wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
127127
{
128128
LOCK(wallet.cs_wallet);
129129
wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
130130
wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
131131
}
132132
AddKey(wallet, coinbaseKey);
133133
WalletRescanReserver reserver(wallet);
134+
std::chrono::steady_clock::time_point fake_time;
135+
reserver.setNow([&] { fake_time += 60s; return fake_time; });
134136
reserver.reserve();
135-
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */);
137+
138+
{
139+
CBlockLocator locator;
140+
BOOST_CHECK(!WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator));
141+
BOOST_CHECK(locator.IsNull());
142+
}
143+
144+
CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/true);
136145
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
137146
BOOST_CHECK(result.last_failed_block.IsNull());
138147
BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash());
139148
BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight);
140149
BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 100 * COIN);
150+
151+
{
152+
CBlockLocator locator;
153+
BOOST_CHECK(WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator));
154+
BOOST_CHECK(!locator.IsNull());
155+
}
141156
}
142157

143158
// Prune the older block file.
@@ -161,7 +176,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
161176
AddKey(wallet, coinbaseKey);
162177
WalletRescanReserver reserver(wallet);
163178
reserver.reserve();
164-
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */);
179+
CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
165180
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE);
166181
BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash());
167182
BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash());
@@ -188,7 +203,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
188203
AddKey(wallet, coinbaseKey);
189204
WalletRescanReserver reserver(wallet);
190205
reserver.reserve();
191-
CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */);
206+
CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
192207
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE);
193208
BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash());
194209
BOOST_CHECK(result.last_scanned_block.IsNull());

‎src/wallet/wallet.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,7 +1665,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
16651665

16661666
if (start) {
16671667
// TODO: this should take into account failure by ScanResult::USER_ABORT
1668-
ScanResult result = ScanForWalletTransactions(start_block, start_height, {} /* max_height */, reserver, update);
1668+
ScanResult result = ScanForWalletTransactions(start_block, start_height, /*max_height=*/{}, reserver, /*fUpdate=*/update, /*save_progress=*/false);
16691669
if (result.status == ScanResult::FAILURE) {
16701670
int64_t time_max;
16711671
CHECK_NONFATAL(chain().findBlock(result.last_failed_block, FoundBlock().maxTime(time_max)));
@@ -1696,12 +1696,11 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
16961696
* the main chain after to the addition of any new keys you want to detect
16971697
* transactions for.
16981698
*/
1699-
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate)
1699+
CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress)
17001700
{
1701-
using Clock = std::chrono::steady_clock;
1702-
constexpr auto LOG_INTERVAL{60s};
1703-
auto current_time{Clock::now()};
1704-
auto start_time{Clock::now()};
1701+
constexpr auto INTERVAL_TIME{60s};
1702+
auto current_time{reserver.now()};
1703+
auto start_time{reserver.now()};
17051704

17061705
assert(reserver.isReserved());
17071706

@@ -1728,8 +1727,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
17281727
if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) {
17291728
ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
17301729
}
1731-
if (Clock::now() >= current_time + LOG_INTERVAL) {
1732-
current_time = Clock::now();
1730+
1731+
bool next_interval = reserver.now() >= current_time + INTERVAL_TIME;
1732+
if (next_interval) {
1733+
current_time = reserver.now();
17331734
WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", block_height, progress_current);
17341735
}
17351736

@@ -1759,6 +1760,16 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
17591760
// scan succeeded, record block as most recent successfully scanned
17601761
result.last_scanned_block = block_hash;
17611762
result.last_scanned_height = block_height;
1763+
1764+
if (save_progress && next_interval) {
1765+
CBlockLocator loc = m_chain->getActiveChainLocator(block_hash);
1766+
1767+
if (!loc.IsNull()) {
1768+
WalletLogPrintf("Saving scan progress %d.\n", block_height);
1769+
WalletBatch batch(GetDatabase());
1770+
batch.WriteBestBlock(loc);
1771+
}
1772+
}
17621773
} else {
17631774
// could not scan block, keep scanning but record this block as the most recent failure
17641775
result.last_failed_block = block_hash;
@@ -1796,7 +1807,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
17961807
WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", block_height, progress_current);
17971808
result.status = ScanResult::USER_ABORT;
17981809
} else {
1799-
auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - start_time);
1810+
auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(reserver.now() - start_time);
18001811
WalletLogPrintf("Rescan completed in %15dms\n", duration_milliseconds.count());
18011812
}
18021813
return result;
@@ -3069,7 +3080,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
30693080

30703081
{
30713082
WalletRescanReserver reserver(*walletInstance);
3072-
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) {
3083+
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/true).status)) {
30733084
error = _("Failed to rescan the wallet during initialization");
30743085
return false;
30753086
}

‎src/wallet/wallet.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
528528
//! USER_ABORT.
529529
uint256 last_failed_block;
530530
};
531-
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
531+
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress);
532532
void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override;
533533
void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
534534
void ResendWalletTransactions();
@@ -919,8 +919,11 @@ void MaybeResendWalletTxs(WalletContext& context);
919919
class WalletRescanReserver
920920
{
921921
private:
922+
using Clock = std::chrono::steady_clock;
923+
using NowFn = std::function<Clock::time_point()>;
922924
CWallet& m_wallet;
923925
bool m_could_reserve;
926+
NowFn m_now;
924927
public:
925928
explicit WalletRescanReserver(CWallet& w) : m_wallet(w), m_could_reserve(false) {}
926929

@@ -941,6 +944,10 @@ class WalletRescanReserver
941944
return (m_could_reserve && m_wallet.fScanningWallet);
942945
}
943946

947+
Clock::time_point now() const { return m_now ? m_now() : Clock::now(); };
948+
949+
void setNow(NowFn now) { m_now = std::move(now); }
950+
944951
~WalletRescanReserver()
945952
{
946953
if (m_could_reserve) {

0 commit comments

Comments
 (0)