interfaces, chain, refactor: Remove inaccurate getActiveChainLocator

pablomartin4btc · ryanofsky · furszy · pablomartin4btc · commit 2b00030af84e · 2025-08-13T00:21:17.000-03:00
The getActiveChainLocator method name was misleading, and its functionality duplicated `Chain::findBlock`. This commit removes the method and replaces all its usages with direct `Chain::findBlock` calls. Additionally, the comment of getActiveChainLocator has been outdated since commit ed47094 from #25717. Finally, in CWallet::ScanForWalletTransactions, the findBlock calls are now unified into a single call at the start of the function. Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: Matias Furszyfer <mfurszy@protonmail.com>
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -143,10 +143,6 @@ class Chain
     //! 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
@@ -559,12 +559,6 @@ class ChainImpl : public Chain
         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
@@ -1837,9 +1837,13 @@ 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
+            // Read block data and locator if needed (the locator is usually null unless we need to save progress)
             CBlock block;
-            chain().findBlock(block_hash, FoundBlock().data(block));
+            CBlockLocator loc;
+            // Find block
+            FoundBlock found_block{FoundBlock().data(block)};
+            if (save_progress && next_interval) found_block.locator(loc);
+            chain().findBlock(block_hash, found_block);
 
             if (!block.IsNull()) {
                 LOCK(cs_wallet);
@@ -1857,14 +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 = m_chain->getActiveChainLocator(block_hash);
-
-                    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

Original file line number	Diff line number	Diff line change
`@@ -559,12 +559,6 @@ class ChainImpl : public Chain`
`559`	`559`	`const CBlockIndex* block{chainman().ActiveChain()[height]};`
`560`	`560`	`return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;`
`561`	`561`	`}`
`562`		`- CBlockLocator getActiveChainLocator(const uint256& block_hash) override`
`563`		`- {`
`564`		`- LOCK(::cs_main);`
`565`		`- const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash);`
`566`		`- return GetLocator(index);`
`567`		`- }`
`568`	`562`	`std::optional<int> findLocatorFork(const CBlockLocator& locator) override`
`569`	`563`	`{`
`570`	`564`	`LOCK(::cs_main);`