test: ensure FastWalletRescanFilter is correctly updated during scanning #34667

Eunovo commented at 10:26 AM on February 25, 2026: contributor

On master, the wallet_fast_rescan.py test will pass even if the fast rescan filters are not updated during wallet rescan. This PR improves the wallet_fast_rescan.py test so that this no longer happens. Testers can verify by applying this diff

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 63dab29972..f7d6c5f84a 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1898,7 +1898,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
 
         bool fetch_block{true};
         if (fast_rescan_filter) {
-            fast_rescan_filter->UpdateIfNeeded();
+            // fast_rescan_filter->UpdateIfNeeded();
             auto matches_block{fast_rescan_filter->MatchesBlock(block_hash)};
             if (matches_block.has_value()) {
                 if (*matches_block) {

and running the wallet_fast_rescan.py test. The test will pass on master but fail on this PR.

DrahtBot added the label Tests on Feb 25, 2026

DrahtBot commented at 10:27 AM on February 25, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Bortlesboat, rkrux, ismaelsadeeq, achow101
Concept ACK	theStack

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34400 (wallet: parallel fast rescan (approx 5x speed up with 16 threads) by Eunovo)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Feb 25, 2026

Eunovo commented at 11:50 AM on February 25, 2026: contributor

CI failure is unrelated

DrahtBot removed the label CI failed on Feb 25, 2026

in test/functional/wallet_fast_rescan.py:68 in 90d347b40b

  68 | +            if 'range' not in desc_info:
  69 | +                spk = bytes.fromhex(fixed_key.p2wpkh_script)
  70 | +                self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
  71 | +            wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
  72 | +            num_txs += 1
  73 | +        self.generate(node, 1)

rkrux commented at 2:27 PM on February 26, 2026:

While it seems fine to mine the transaction related to the non-ranged descriptor in a separate block, I think it'd be thorough to mine a block with transaction(s) unrelated to the wallet at all so that the fast rescan doesn't fetch it, which can be asserted for in the debug log. Consider the following diff:

diff --git a/test/functional/wallet_fast_rescan.py b/test/functional/wallet_fast_rescan.py
index a917a9db32..93078386df 100755
--- a/test/functional/wallet_fast_rescan.py
+++ b/test/functional/wallet_fast_rescan.py
@@ -14,7 +15,7 @@ from test_framework.wallet_util import get_generate_key
 
 
 KEYPOOL_SIZE = 100   # smaller than default size to speed-up test
-NUM_BLOCKS = 6       # number of blocks to mine
+NUM_BLOCKS = 7       # number of blocks to mine
 
 
 class WalletFastRescanTest(BitcoinTestFramework):
@@ -32,7 +33,7 @@ class WalletFastRescanTest(BitcoinTestFramework):
 
     def run_test(self):
         node = self.nodes[0]
-        wallet = MiniWallet(node)
+        funding_wallet = MiniWallet(node)
 
         self.log.info("Create descriptor wallet with backup")
         WALLET_BACKUP_FILENAME = node.datadir_path / 'wallet.bak'
@@ -45,7 +46,8 @@ class WalletFastRescanTest(BitcoinTestFramework):
 
         self.log.info("Create txs sending to end range address of each descriptor, triggering top-ups")
         num_txs = 0
-        for i in range(NUM_BLOCKS-1):
+        fast_rescan_messages = []
+        for i in range(NUM_BLOCKS-2):
             self.log.info(f"Block {i+1}/{NUM_BLOCKS}")
             for desc_info in w.listdescriptors()['descriptors']:
                 if 'range' in desc_info:
@@ -53,28 +55,39 @@ class WalletFastRescanTest(BitcoinTestFramework):
                     addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
                     spk = address_to_scriptpubkey(addr)
                     self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
-                wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
+                funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
                 num_txs += 1
             self.generate(node, 1)
+            chain_info = self.nodes[0].getblockchaininfo()
+            fast_rescan_messages.append(f"Fast rescan: inspect block {chain_info['blocks']} [{chain_info['bestblockhash']}] (filter matched)")
 
         self.log.info("Create txs sending to non-ranged descriptors")
-        self.log.info(f"Block {NUM_BLOCKS}/{NUM_BLOCKS}")
+        self.log.info(f"Block {NUM_BLOCKS-1}/{NUM_BLOCKS}")
         for desc_info in w.listdescriptors()['descriptors']:
             if 'range' not in desc_info:
                 spk = bytes.fromhex(fixed_key.p2wpkh_script)
                 self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
-            wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
+            funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
             num_txs += 1
         self.generate(node, 1)
+        chain_info = self.nodes[0].getblockchaininfo()
+        fast_rescan_messages.append(f"Fast rescan: inspect block {chain_info['blocks']} [{chain_info['bestblockhash']}] (filter matched)")
+
+        self.log.info("Create tx unrelated to the wallet under consideration")
+        self.log.info(f"Block {NUM_BLOCKS}/{NUM_BLOCKS}")
+        funding_wallet.send_self_transfer(from_node=node)
+        self.generate(node, 1)
 
         self.log.info("Import wallet backup with block filter index")
-        with node.assert_debug_log(['fast variant using block filters']):
+        with node.assert_debug_log(['fast variant using block filters', *fast_rescan_messages]):
             node.restorewallet('rescan_fast', WALLET_BACKUP_FILENAME)
         txids_fast = self.get_wallet_txids(node, 'rescan_fast')
 
         self.log.info("Import non-active descriptors with block filter index")
         node.createwallet(wallet_name='rescan_fast_nonactive', disable_private_keys=True, blank=True)
-        with node.assert_debug_log(['fast variant using block filters']):
+        with node.assert_debug_log(['fast variant using block filters', *fast_rescan_messages]):
             w = node.get_wallet_rpc('rescan_fast_nonactive')
             w.importdescriptors([{"desc": descriptor['desc'], "timestamp": 0} for descriptor in descriptors])
         txids_fast_nonactive = self.get_wallet_txids(node, 'rescan_fast_nonactive')

rkrux commented at 3:01 PM on February 26, 2026:

The above diff also renames the MiniWallet instance to funding_wallet to avoid confusion with the other wallets under consideration.

Eunovo commented at 11:03 AM on March 2, 2026:

While it seems fine to mine the transaction related to the non-ranged descriptor in a separate block, I think it'd be thorough to mine a block with transaction(s) unrelated to the wallet at all so that the fast rescan doesn't fetch it, which can be asserted for in the debug log.

This was a good idea, but there is the possibility of false positives with block filters, so a block can be fetched(with a low probability) even if the wallet spk is not in the set.

The above diff also renames the MiniWallet instance to funding_wallet to avoid confusion with the other wallets under consideration.

Done

rkrux commented at 12:08 PM on March 2, 2026:

to mine a block with transaction(s) unrelated to the wallet at all so that the fast rescan doesn't fetch it, which can be asserted for in the debug log.

This was a good idea, but there is the possibility of false positives with block filters, so a block can be fetched(with a low probability) even if the wallet spk is not in the set.

I see that the last portion of my comment was misleading. I didn't mean to suggest that it should be asserted that the block is not being fetched when the filter didn't match, something that the suggested diff also doesn't do. Although the reason for not suggesting it was not because of false positives but because of the lack of such a debug log (there is no log when fetch_block is set to false). Thus, the suggested diff limits the assertions to only filter matches.

https://github.com/bitcoin/bitcoin/blob/9cad97f6cdf1bd660732cd10e844a6a7e0771ea0/src/wallet/wallet.cpp#L1900-L1914

in test/functional/wallet_fast_rescan.py:57 in 90d347b40b

  57 |                      self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
  58 | -                else:
  59 | -                    spk = bytes.fromhex(fixed_key.p2wpkh_script)
  60 | -                    self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
  61 |                  wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
  62 | +                num_txs += 1

rkrux commented at 2:45 PM on February 26, 2026:

    self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)

Why do the transactions need to be created and sent in the case range property is not present? It will end up creating a transaction sending to spk of the previous iteration in case range property is not present, which seems incorrect. Consider the following which reads easier too:

             self.log.info(f"Block {i+1}/{NUM_BLOCKS}")
-            for desc_info in w.listdescriptors()['descriptors']:
-                if 'range' in desc_info:
-                    start_range, end_range = desc_info['range']
-                    addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
-                    spk = address_to_scriptpubkey(addr)
-                    self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
+            descs = [desc_info for desc_info in w.listdescriptors()['descriptors'] if 'range' in desc_info]
+            for desc_info in descs:
+                start_range, end_range = desc_info['range']
+                addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
+                spk = address_to_scriptpubkey(addr)
+                self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
                 wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
                 num_txs += 1
             self.generate(node, 1)

Eunovo commented at 11:03 AM on March 2, 2026:

Thanks for bringing this to my attention. Fixed.

in test/functional/wallet_fast_rescan.py:67 in 90d347b40b

  67 | +        for desc_info in w.listdescriptors()['descriptors']:
  68 | +            if 'range' not in desc_info:
  69 | +                spk = bytes.fromhex(fixed_key.p2wpkh_script)
  70 | +                self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
  71 | +            wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
  72 | +            num_txs += 1

rkrux commented at 2:49 PM on February 26, 2026:

Same point here as well. num_txs will also be reduced with the following diff because unnecessary transactions are being created at the moment.

Eunovo commented at 11:05 AM on March 2, 2026:

Fixed.

in test/functional/wallet_fast_rescan.py:60 in 90d347b40b

  60 | -                    self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
  61 |                  wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
  62 | +                num_txs += 1
  63 |              self.generate(node, 1)
  64 |  
  65 | +        self.log.info("Create txs sending to non-ranged descriptors")

rkrux commented at 2:50 PM on February 26, 2026:

- self.log.info("Create txs sending to non-ranged descriptors")
+ self.log.info("Create tx sending to non-ranged descriptor")

There should be only one such desc.

Eunovo commented at 11:05 AM on March 2, 2026:

Fixed.

in test/functional/wallet_fast_rescan.py:65 in 90d347b40b

  65 | +        self.log.info("Create txs sending to non-ranged descriptors")
  66 | +        self.log.info(f"Block {NUM_BLOCKS}/{NUM_BLOCKS}")
  67 | +        for desc_info in w.listdescriptors()['descriptors']:
  68 | +            if 'range' not in desc_info:
  69 | +                spk = bytes.fromhex(fixed_key.p2wpkh_script)
  70 | +                self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")

rkrux commented at 2:59 PM on February 26, 2026:

This doesn't actually use the values from the descriptor and instead uses the fixed key, which is not ideal.

-  spk = bytes.fromhex(fixed_key.p2wpkh_script)
-  self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
+  addr = w.deriveaddresses(desc_info['desc'])[0]
+  spk = address_to_scriptpubkey(addr)
+  self.log.info(f"-> fixed non-range descriptor address {addr}")

Eunovo commented at 11:05 AM on March 2, 2026:

This is unrelated to the test, so I decided to leave it as is.

in test/functional/wallet_fast_rescan.py:42 in 90d347b40b

  40 | @@ -42,24 +41,32 @@ def run_test(self):
  41 |          fixed_key = get_generate_key()
  42 |          print(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))

rkrux commented at 3:00 PM on February 26, 2026:

Unrelated to the PR but can consider. I don't see a reason why the output needs to be printed.

Eunovo commented at 11:05 AM on March 2, 2026:

The print statement has been removed.

rkrux changes_requested

rkrux commented at 3:02 PM on February 26, 2026: contributor

Concept ACK, very nice figuring out the issue in the test. I suggest few changes.

I also wanted to add an assertion checking that the fast rescan is indeed faster than the slow rescan by adding the following time calculation in both restoration with and without blockfilter index. I tried it for multiple extra non-matching blocks so that the fast rescan can bypass fetching them, but I didn't get deterministic results, so unfortunately had to abandon the idea.

@@ -82,7 +96,9 @@ class WalletFastRescanTest(BitcoinTestFramework):
         self.restart_node(0, [f'-keypool={KEYPOOL_SIZE}', '-blockfilterindex=0'])
         self.log.info("Import wallet backup w/o block filter index")
         with node.assert_debug_log(['slow variant inspecting all blocks']):
+            start_time = time.time()
             node.restorewallet("rescan_slow", WALLET_BACKUP_FILENAME)
+            print(time.time() - start_time)
         txids_slow = self.get_wallet_txids(node, 'rescan_slow')

Eunovo commented at 4:28 PM on February 26, 2026: contributor

I also wanted to add an assertion checking that the fast rescan is indeed faster than the slow rescan by adding the following time calculation in both restoration with and without blockfilter index. I tried it for multiple extra non-matching blocks so that the fast rescan can bypass fetching them, but I didn't get deterministic results, so unfortunately had to abandon the idea.

Fast rescan is not always strictly faster. See #25957 (comment). The time to check the filter is significant, and this will make fast rescan slower if the blocks are not big enough.

Eunovo force-pushed on Mar 2, 2026

in test/functional/wallet_fast_rescan.py:54 in ceb943dc71 outdated

  52 | +        self.log.info(f"Block 1/{NUM_BLOCKS}")
  53 | +        spk = bytes.fromhex(fixed_key.p2wpkh_script)
  54 | +        self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
  55 | +        funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
  56 | +        num_txs += 1
  57 | +        self.generate(node, 1)

rkrux commented at 12:09 PM on March 2, 2026:

Is it intentional to not assert for the filter match log for this block?

--- a/test/functional/wallet_fast_rescan.py
+++ b/test/functional/wallet_fast_rescan.py
@@ -44,6 +44,7 @@ class WalletFastRescanTest(BitcoinTestFramework):
         w.backupwallet(WALLET_BACKUP_FILENAME)
 
         num_txs = 0
+        fast_rescan_messages = []
 
         self.log.info("Create tx sending to non-ranged descriptors")
         self.log.info(f"Block 1/{NUM_BLOCKS}")
@@ -52,9 +53,11 @@ class WalletFastRescanTest(BitcoinTestFramework):
         funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
         num_txs += 1
         self.generate(node, 1)
+        chain_info = self.nodes[0].getblockchaininfo()
+        fast_rescan_messages.append(f"Fast rescan: inspect block {chain_info['blocks']} [{chain_info['bestblockhash']}] (filter matched)")
 
         self.log.info("Create txs sending to end range address of each descriptor, triggering top-ups")
-        fast_rescan_messages = []
         for i in range(1, NUM_BLOCKS):
             self.log.info(f"Block {i+1}/{NUM_BLOCKS}")
             ranged_descs = [desc for desc in descriptors if 'range' in desc]

Eunovo commented at 1:42 PM on March 2, 2026:

Fixed!

DrahtBot added the label CI failed on Mar 2, 2026

Eunovo force-pushed on Mar 2, 2026

DrahtBot removed the label CI failed on Mar 2, 2026

rkrux approved

rkrux commented at 10:21 AM on March 3, 2026: contributor

~lgtm A-C-K e7796a5d55cf9c7a5e7f5ff59c261d051acd102b~

Thanks for addressing the comments.

theStack commented at 7:04 PM on March 10, 2026: contributor

Concept ACK

in test/functional/wallet_fast_rescan.py:70 in e7796a5d55 outdated

  79 | +            ranged_descs = [desc for desc in descriptors if 'range' in desc]
  80 | +            for desc_info in ranged_descs:
  81 | +                start_range, end_range = desc_info['range']
  82 | +                addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
  83 | +                spk = address_to_scriptpubkey(addr)
  84 | +                self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")

ismaelsadeeq commented at 2:43 PM on March 13, 2026:

nit: the range is currently static on each loop iteration run [0, 99]. Adding the descriptor and then the range in debug logs (to avoid verbosity) could be more useful.

                self.log.debug(f"-> {desc_info['desc']} range [{start_range},{end_range}], last address {addr}")

rkrux commented at 2:13 PM on March 14, 2026:

Ah, this is incorrect and needs to be fixed. Same end ranges and therefore same addresses are being used, no top-ups are happening unlike in the master branch.

Testers can verify by commenting out https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L1901 and running the wallet_fast_rescan.py test. The test will pass on master but fail on this PR.

This test passes on this PR if I comment out the mentioned line: https://github.com/bitcoin/bitcoin/blob/92a3d30f382f2c72886cf4a0cdae43bed600148f/src/wallet/wallet.cpp#L1915

I believe this defeats the purpose of this PR unfortunately.

2026-03-14T14:10:59.925651Z TestFramework (INFO): Create txs sending to end range address of each descriptor, triggering top-ups
2026-03-14T14:10:59.925812Z TestFramework (INFO): Block 2/6
2026-03-14T14:10:59.926794Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
2026-03-14T14:10:59.971362Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
2026-03-14T14:10:59.990679Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
2026-03-14T14:11:00.034311Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
2026-03-14T14:11:00.055303Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
2026-03-14T14:11:00.115288Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
2026-03-14T14:11:00.145300Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
2026-03-14T14:11:00.192296Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
2026-03-14T14:11:00.219601Z TestFramework (INFO): Block 3/6
2026-03-14T14:11:00.220740Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
2026-03-14T14:11:00.229588Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
2026-03-14T14:11:00.240024Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
2026-03-14T14:11:00.246762Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
2026-03-14T14:11:00.254477Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
2026-03-14T14:11:00.263729Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
2026-03-14T14:11:00.274483Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
2026-03-14T14:11:00.280881Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
2026-03-14T14:11:00.300429Z TestFramework (INFO): Block 4/6
2026-03-14T14:11:00.304186Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
2026-03-14T14:11:00.311862Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
2026-03-14T14:11:00.319270Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
2026-03-14T14:11:00.328106Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
2026-03-14T14:11:00.339337Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
2026-03-14T14:11:00.346269Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
2026-03-14T14:11:00.353791Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
2026-03-14T14:11:00.363796Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
2026-03-14T14:11:00.383322Z TestFramework (INFO): Block 5/6
2026-03-14T14:11:00.385387Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
2026-03-14T14:11:00.394171Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
2026-03-14T14:11:00.405695Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
2026-03-14T14:11:00.412206Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
2026-03-14T14:11:00.420359Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
2026-03-14T14:11:00.431522Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
2026-03-14T14:11:00.442100Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
2026-03-14T14:11:00.447409Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
2026-03-14T14:11:00.469185Z TestFramework (INFO): Block 6/6
2026-03-14T14:11:00.472373Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
2026-03-14T14:11:00.479604Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
2026-03-14T14:11:00.487203Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
2026-03-14T14:11:00.497411Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
2026-03-14T14:11:00.508043Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
2026-03-14T14:11:00.513448Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
2026-03-14T14:11:00.520729Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
2026-03-14T14:11:00.531293Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn

</details>

Eunovo commented at 10:54 AM on March 16, 2026:

I accidentally introduced the issue during the previous refactoring. It has been fixed

Eunovo commented at 10:55 AM on March 16, 2026:

Adding the descriptor and then the range in debug logs (to avoid verbosity) could be more useful.

I agree. Changed the log level to "debug".

ismaelsadeeq commented at 2:45 PM on March 13, 2026: member

light code review ACK e7796a5d55cf9c7a5e7f5ff59c261d051acd102b

Testers can verify by commenting out https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L1901 and running the wallet_fast_rescan.py test. The test will pass on master but fail on this PR.

This link now points to a closing brace, which might even change when you see this. It would be better if you specified a diff and the master commit hash you used for better reproducibility, because a master permalink can easily go stale like this, because "master" will keep getting updates.

DrahtBot requested review from theStack on Mar 13, 2026

rkrux changes_requested

rkrux commented at 2:34 PM on March 14, 2026: contributor

Removing a-c-k because of an issue.

DrahtBot requested review from rkrux on Mar 14, 2026

Bortlesboat commented at 10:14 PM on March 15, 2026: none

ACK e7796a5d55cf9c7a5e7f5ff59c261d051acd102b. Ran wallet_fast_rescan.py locally on Ubuntu 24.04, test passes.

in test/functional/wallet_fast_rescan.py:50 in e7796a5d55 outdated

  48 |  
  49 | +        num_txs = 0
  50 | +
  51 | +        fast_rescan_messages = []
  52 | +        def append_fast_rescan_message():
  53 | +            chain_info = self.nodes[0].getblockchaininfo()

Bortlesboat commented at 10:14 PM on March 15, 2026:

The original code re-read w.listdescriptors() inside each loop iteration, which would capture the updated ranges after top-ups. This refactored version reads descriptors once before the loop and filters from that snapshot. Is this intentional? If the test is specifically meant to exercise cascading top-ups (where each block extends the keypool range further), using the initial snapshot means subsequent blocks send to the same end-of-range addresses rather than the newly extended end-of-range. The test still passes because the wallet finds the transactions at those addresses regardless, but it may test a different code path than before.

Eunovo commented at 10:53 AM on March 16, 2026:

Fixed!

Eunovo force-pushed on Mar 16, 2026

test/wallet: ensure FastWalletRescanFilter is updated during scanning

The fixed non-range descriptor address ensured that the FastWalletRescanFilter would match all Blocks even if the filter wasn't properly updated.
This commit moves the non-range descriptor tx to a different block, so that the filters must be updated after each TopUp for the test to pass.

Co-authored-by: rkrux <rkrux.connect@gmail.com>

92287ae753

Eunovo force-pushed on Mar 16, 2026

DrahtBot added the label CI failed on Mar 16, 2026

Eunovo commented at 10:57 AM on March 16, 2026: contributor

This link now points to a closing brace, which might even change when you see this. It would be better if you specified a diff and the master commit hash you used for better reproducibility, because a master permalink can easily go stale like this, because "master" will keep getting updates.

Thanks for the suggestion, I changed the description to use a diff instead.

DrahtBot removed the label CI failed on Mar 16, 2026

Bortlesboat commented at 6:41 PM on March 16, 2026: none

re-ACK 92287ae75397755343f957ccfdda1933e9169d02

DrahtBot requested review from ismaelsadeeq on Mar 16, 2026

rkrux approved

rkrux commented at 11:36 AM on March 21, 2026: contributor

tACK 92287ae

Ran the test again with and without the mentioned diff in the PR description. Also checked with loglevel DEBUG that the end ranges are being updated & topups being done.

Range diff contains the fix of the previously highlighted issue #34667 (review) and loglevel info to debug changes.

git range-diff e7796a5...92287ae

ismaelsadeeq commented at 5:04 PM on March 23, 2026: member

Tested ACK 92287ae75397755343f957ccfdda1933e9169d02

I did what the OP asked by applying the diff to master versus this PR. Master passes, while this PR fails.

I wanted to know why that happens. At first, I thought it was because we now append more verbose fast rescan messages after each block connection. So I added something like append_fast_rescan_message approach to master, and the test still passes.

Since that was not the reason, I then moved the non-ranged descriptor creation outside the ranged loop, created one fixed non-ranged descriptor address, and mined a block.

After that, the test started failing. Even without assert_debug_log checking for the vector of fast_rescan_message in the logs, the test still fails. assert_equal(len(txids_fast), num_txs) only returns 9 transactions from the fast scan rather than 41.

achow101 commented at 12:27 AM on March 24, 2026: member

ACK 92287ae75397755343f957ccfdda1933e9169d02

achow101 merged this on Mar 24, 2026

achow101 closed this on Mar 24, 2026