not sure I understand why we’ve moved this out of the condition.
Could we split this into two commits, one removing the need for getActiveChainLocator, something like:
0diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
1index f45cb8e598..82e626e35d 100644
2--- a/src/interfaces/chain.h
3+++ b/src/interfaces/chain.h
4@@ -143,10 +143,6 @@ public:
5 //! pruned), and contains transactions.
6 virtual bool haveBlockOnDisk(int height) = 0;
7
8- //! Return a locator that refers to a block in the active chain.
9- //! If specified block is not in the active chain, return locator for the latest ancestor that is in the chain.
10- virtual CBlockLocator getActiveChainLocator(const uint256& block_hash) = 0;
11-
12 //! Return height of the highest block on chain in common with the locator,
13 //! which will either be the original block used to create the locator,
14 //! or one of its ancestors.
15diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
16index d7ada5ae75..106c961173 100644
17--- a/src/node/interfaces.cpp
18+++ b/src/node/interfaces.cpp
19@@ -559,12 +559,6 @@ public:
20 const CBlockIndex* block{chainman().ActiveChain()[height]};
21 return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
22 }
23- CBlockLocator getActiveChainLocator(const uint256& block_hash) override
24- {
25- LOCK(::cs_main);
26- const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash);
27- return GetLocator(index);
28- }
29 std::optional<int> findLocatorFork(const CBlockLocator& locator) override
30 {
31 LOCK(::cs_main);
32diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
33index 21de98862b..fe82383ebd 100644
34--- a/src/wallet/wallet.cpp
35+++ b/src/wallet/wallet.cpp
36@@ -1839,7 +1839,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
37 if (fetch_block) {
38 // Read block data
39 CBlock block;
40- chain().findBlock(block_hash, FoundBlock().data(block));
41+ auto found_block{FoundBlock().data(block)};
42+ chain().findBlock(block_hash, found_block);
43
44 if (!block.IsNull()) {
45 LOCK(cs_wallet);
46@@ -1858,7 +1859,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
47 result.last_scanned_height = block_height;
48
49 if (save_progress && next_interval) {
50- CBlockLocator loc = m_chain->getActiveChainLocator(block_hash);
51+ CBlockLocator loc;
52+ found_block.locator(loc);
53
54 if (!loc.IsNull()) {
55 WalletLogPrintf("Saving scan progress %d.\n", block_height);
Which is easy to understand and a second one, moving the locator assignment before the block null check, i.e. something like:
0diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
1index fe82383ebd..8f2bbc23c9 100644
2--- a/src/wallet/wallet.cpp
3+++ b/src/wallet/wallet.cpp
4@@ -1837,9 +1837,12 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
5 chain().findBlock(block_hash, FoundBlock().inActiveChain(block_still_active).nextBlock(FoundBlock().inActiveChain(next_block).hash(next_block_hash)));
6
7 if (fetch_block) {
8- // Read block data
9 CBlock block;
10 auto found_block{FoundBlock().data(block)};
11+
12+ CBlockLocator loc;
13+ if (save_progress && next_interval) found_block.locator(loc);
14+
15 chain().findBlock(block_hash, found_block);
16
17 if (!block.IsNull()) {
18@@ -1858,15 +1861,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
19 result.last_scanned_block = block_hash;
20 result.last_scanned_height = block_height;
21
22- if (save_progress && next_interval) {
23- CBlockLocator loc;
24- found_block.locator(loc);
25-
26- if (!loc.IsNull()) {
27- WalletLogPrintf("Saving scan progress %d.\n", block_height);
28- WalletBatch batch(GetDatabase());
29- batch.WriteBestBlock(loc);
30- }
31+ if (!loc.IsNull()) {
32+ WalletLogPrintf("Saving scan progress %d.\n", block_height);
33+ WalletBatch batch(GetDatabase());
34+ batch.WriteBestBlock(loc);
35 }
36 } else {
37 // could not scan block, keep scanning but record this block as the most recent failure
Which I’m not yet sure I fully understand. The commit message doesn’t help me, but after the split the commit messages could explain the separately with more context, maybe that would help.
