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:
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index f45cb8e598..82e626e35d 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -143,10 +143,6 @@ public:
//! pruned), and contains transactions.
virtual bool haveBlockOnDisk(int height) = 0;
- //! Return a locator that refers to a block in the active chain.
- //! If specified block is not in the active chain, return locator for the latest ancestor that is in the chain.
- virtual CBlockLocator getActiveChainLocator(const uint256& block_hash) = 0;
-
//! Return height of the highest block on chain in common with the locator,
//! which will either be the original block used to create the locator,
//! or one of its ancestors.
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index d7ada5ae75..106c961173 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -559,12 +559,6 @@ public:
const CBlockIndex* block{chainman().ActiveChain()[height]};
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
}
- CBlockLocator getActiveChainLocator(const uint256& block_hash) override
- {
- LOCK(::cs_main);
- const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash);
- return GetLocator(index);
- }
std::optional<int> findLocatorFork(const CBlockLocator& locator) override
{
LOCK(::cs_main);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 21de98862b..fe82383ebd 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1839,7 +1839,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
if (fetch_block) {
// Read block data
CBlock block;
- chain().findBlock(block_hash, FoundBlock().data(block));
+ auto found_block{FoundBlock().data(block)};
+ chain().findBlock(block_hash, found_block);
if (!block.IsNull()) {
LOCK(cs_wallet);
@@ -1858,7 +1859,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
result.last_scanned_height = block_height;
if (save_progress && next_interval) {
- CBlockLocator loc = m_chain->getActiveChainLocator(block_hash);
+ CBlockLocator loc;
+ found_block.locator(loc);
if (!loc.IsNull()) {
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:
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index fe82383ebd..8f2bbc23c9 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1837,9 +1837,12 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
chain().findBlock(block_hash, FoundBlock().inActiveChain(block_still_active).nextBlock(FoundBlock().inActiveChain(next_block).hash(next_block_hash)));
if (fetch_block) {
- // Read block data
CBlock block;
auto found_block{FoundBlock().data(block)};
+
+ CBlockLocator loc;
+ if (save_progress && next_interval) found_block.locator(loc);
+
chain().findBlock(block_hash, found_block);
if (!block.IsNull()) {
@@ -1858,15 +1861,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
result.last_scanned_block = block_hash;
result.last_scanned_height = block_height;
- if (save_progress && next_interval) {
- CBlockLocator loc;
- found_block.locator(loc);
-
- if (!loc.IsNull()) {
- WalletLogPrintf("Saving scan progress %d.\n", block_height);
- WalletBatch batch(GetDatabase());
- batch.WriteBestBlock(loc);
- }
+ if (!loc.IsNull()) {
+ WalletLogPrintf("Saving scan progress %d.\n", block_height);
+ WalletBatch batch(GetDatabase());
+ batch.WriteBestBlock(loc);
}
} else {
// 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.
