test: ensure FastWalletRescanFilter is correctly updated during scanning #34667

pull Eunovo wants to merge 1 commits into bitcoin:master from Eunovo:impr-rescan-test changing 1 files +36 −22
  1. Eunovo commented at 10:26 am on February 25, 2026: contributor

    Part of #34400

    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

     0diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
     1index 63dab29972..f7d6c5f84a 100644
     2--- a/src/wallet/wallet.cpp
     3+++ b/src/wallet/wallet.cpp
     4@@ -1898,7 +1898,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
     5 
     6         bool fetch_block{true};
     7         if (fast_rescan_filter) {
     8-            fast_rescan_filter->UpdateIfNeeded();
     9+            // fast_rescan_filter->UpdateIfNeeded();
    10             auto matches_block{fast_rescan_filter->MatchesBlock(block_hash)};
    11             if (matches_block.has_value()) {
    12                 if (*matches_block) {
    

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

  2. DrahtBot added the label Tests on Feb 25, 2026
  3. 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 <!–meta-tag:bot-skip–> 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.

  4. DrahtBot added the label CI failed on Feb 25, 2026
  5. Eunovo commented at 11:50 am on February 25, 2026: contributor
    CI failure is unrelated
  6. DrahtBot removed the label CI failed on Feb 25, 2026
  7. 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:

     0diff --git a/test/functional/wallet_fast_rescan.py b/test/functional/wallet_fast_rescan.py
     1index a917a9db32..93078386df 100755
     2--- a/test/functional/wallet_fast_rescan.py
     3+++ b/test/functional/wallet_fast_rescan.py
     4@@ -14,7 +15,7 @@ from test_framework.wallet_util import get_generate_key
     5 
     6 
     7 KEYPOOL_SIZE = 100   # smaller than default size to speed-up test
     8-NUM_BLOCKS = 6       # number of blocks to mine
     9+NUM_BLOCKS = 7       # number of blocks to mine
    10 
    11 
    12 class WalletFastRescanTest(BitcoinTestFramework):
    13@@ -32,7 +33,7 @@ class WalletFastRescanTest(BitcoinTestFramework):
    14 
    15     def run_test(self):
    16         node = self.nodes[0]
    17-        wallet = MiniWallet(node)
    18+        funding_wallet = MiniWallet(node)
    19 
    20         self.log.info("Create descriptor wallet with backup")
    21         WALLET_BACKUP_FILENAME = node.datadir_path / 'wallet.bak'
    22@@ -45,7 +46,8 @@ class WalletFastRescanTest(BitcoinTestFramework):
    23 
    24         self.log.info("Create txs sending to end range address of each descriptor, triggering top-ups")
    25         num_txs = 0
    26-        for i in range(NUM_BLOCKS-1):
    27+        fast_rescan_messages = []
    28+        for i in range(NUM_BLOCKS-2):
    29             self.log.info(f"Block {i+1}/{NUM_BLOCKS}")
    30             for desc_info in w.listdescriptors()['descriptors']:
    31                 if 'range' in desc_info:
    32@@ -53,28 +55,39 @@ class WalletFastRescanTest(BitcoinTestFramework):
    33                     addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
    34                     spk = address_to_scriptpubkey(addr)
    35                     self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
    36-                wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
    37+                funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
    38                 num_txs += 1
    39             self.generate(node, 1)
    40+            chain_info = self.nodes[0].getblockchaininfo()
    41+            fast_rescan_messages.append(f"Fast rescan: inspect block {chain_info['blocks']} [{chain_info['bestblockhash']}] (filter matched)")
    42 
    43         self.log.info("Create txs sending to non-ranged descriptors")
    44-        self.log.info(f"Block {NUM_BLOCKS}/{NUM_BLOCKS}")
    45+        self.log.info(f"Block {NUM_BLOCKS-1}/{NUM_BLOCKS}")
    46         for desc_info in w.listdescriptors()['descriptors']:
    47             if 'range' not in desc_info:
    48                 spk = bytes.fromhex(fixed_key.p2wpkh_script)
    49                 self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
    50-            wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
    51+            funding_wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
    52             num_txs += 1
    53         self.generate(node, 1)
    54+        chain_info = self.nodes[0].getblockchaininfo()
    55+        fast_rescan_messages.append(f"Fast rescan: inspect block {chain_info['blocks']} [{chain_info['bestblockhash']}] (filter matched)")
    56+
    57+        self.log.info("Create tx unrelated to the wallet under consideration")
    58+        self.log.info(f"Block {NUM_BLOCKS}/{NUM_BLOCKS}")
    59+        funding_wallet.send_self_transfer(from_node=node)
    60+        self.generate(node, 1)
    61 
    62         self.log.info("Import wallet backup with block filter index")
    63-        with node.assert_debug_log(['fast variant using block filters']):
    64+        with node.assert_debug_log(['fast variant using block filters', *fast_rescan_messages]):
    65             node.restorewallet('rescan_fast', WALLET_BACKUP_FILENAME)
    66         txids_fast = self.get_wallet_txids(node, 'rescan_fast')
    67 
    68         self.log.info("Import non-active descriptors with block filter index")
    69         node.createwallet(wallet_name='rescan_fast_nonactive', disable_private_keys=True, blank=True)
    70-        with node.assert_debug_log(['fast variant using block filters']):
    71+        with node.assert_debug_log(['fast variant using block filters', *fast_rescan_messages]):
    72             w = node.get_wallet_rpc('rescan_fast_nonactive')
    73             w.importdescriptors([{"desc": descriptor['desc'], "timestamp": 0} for descriptor in descriptors])
    74         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

  8. 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:
    0    self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
    1wallet.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:

     0             self.log.info(f"Block {i+1}/{NUM_BLOCKS}")
     1-            for desc_info in w.listdescriptors()['descriptors']:
     2-                if 'range' in desc_info:
     3-                    start_range, end_range = desc_info['range']
     4-                    addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
     5-                    spk = address_to_scriptpubkey(addr)
     6-                    self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
     7+            descs = [desc_info for desc_info in w.listdescriptors()['descriptors'] if 'range' in desc_info]
     8+            for desc_info in descs:
     9+                start_range, end_range = desc_info['range']
    10+                addr = w.deriveaddresses(desc_info['desc'], [end_range, end_range])[0]
    11+                spk = address_to_scriptpubkey(addr)
    12+                self.log.info(f"-> range [{start_range},{end_range}], last address {addr}")
    13                 wallet.send_to(from_node=node, scriptPubKey=spk, amount=10000)
    14                 num_txs += 1
    15             self.generate(node, 1)
    

    Eunovo commented at 11:03 am on March 2, 2026:
    Thanks for bringing this to my attention. Fixed.
  9. 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.
  10. 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:
    0- self.log.info("Create txs sending to non-ranged descriptors")
    1+ 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.
  11. 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.

    0-  spk = bytes.fromhex(fixed_key.p2wpkh_script)
    1-  self.log.info(f"-> fixed non-range descriptor address {fixed_key.p2wpkh_addr}")
    2+  addr = w.deriveaddresses(desc_info['desc'])[0]
    3+  spk = address_to_scriptpubkey(addr)
    4+  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.
  12. 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.
  13. rkrux changes_requested
  14. 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.

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

  16. Eunovo force-pushed on Mar 2, 2026
  17. 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?

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

    Eunovo commented at 1:42 pm on March 2, 2026:
    Fixed!
  18. DrahtBot added the label CI failed on Mar 2, 2026
  19. Eunovo force-pushed on Mar 2, 2026
  20. DrahtBot removed the label CI failed on Mar 2, 2026
  21. rkrux approved
  22. rkrux commented at 10:21 am on March 3, 2026: contributor

    lgtm A-C-K e7796a5d55cf9c7a5e7f5ff59c261d051acd102b

    Thanks for addressing the comments.

  23. theStack commented at 7:04 pm on March 10, 2026: contributor
    Concept ACK
  24. 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.

    0                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.

     02026-03-14T14:10:59.925651Z TestFramework (INFO): Create txs sending to end range address of each descriptor, triggering top-ups
     12026-03-14T14:10:59.925812Z TestFramework (INFO): Block 2/6
     22026-03-14T14:10:59.926794Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
     32026-03-14T14:10:59.971362Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
     42026-03-14T14:10:59.990679Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
     52026-03-14T14:11:00.034311Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
     62026-03-14T14:11:00.055303Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
     72026-03-14T14:11:00.115288Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
     82026-03-14T14:11:00.145300Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
     92026-03-14T14:11:00.192296Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
    102026-03-14T14:11:00.219601Z TestFramework (INFO): Block 3/6
    112026-03-14T14:11:00.220740Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
    122026-03-14T14:11:00.229588Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
    132026-03-14T14:11:00.240024Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
    142026-03-14T14:11:00.246762Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
    152026-03-14T14:11:00.254477Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
    162026-03-14T14:11:00.263729Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
    172026-03-14T14:11:00.274483Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
    182026-03-14T14:11:00.280881Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
    192026-03-14T14:11:00.300429Z TestFramework (INFO): Block 4/6
    202026-03-14T14:11:00.304186Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
    212026-03-14T14:11:00.311862Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
    222026-03-14T14:11:00.319270Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
    232026-03-14T14:11:00.328106Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
    242026-03-14T14:11:00.339337Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
    252026-03-14T14:11:00.346269Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
    262026-03-14T14:11:00.353791Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
    272026-03-14T14:11:00.363796Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
    282026-03-14T14:11:00.383322Z TestFramework (INFO): Block 5/6
    292026-03-14T14:11:00.385387Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
    302026-03-14T14:11:00.394171Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
    312026-03-14T14:11:00.405695Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
    322026-03-14T14:11:00.412206Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
    332026-03-14T14:11:00.420359Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
    342026-03-14T14:11:00.431522Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
    352026-03-14T14:11:00.442100Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
    362026-03-14T14:11:00.447409Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
    372026-03-14T14:11:00.469185Z TestFramework (INFO): Block 6/6
    382026-03-14T14:11:00.472373Z TestFramework (INFO): -> range [0,99], last address mjAkjWRCvqYhC19St4Kd8FiBsAB1ZPKMXc
    392026-03-14T14:11:00.479604Z TestFramework (INFO): -> range [0,99], last address mfcZh9XmUq131ziU4GK3NcwknEwwtn3Lg6
    402026-03-14T14:11:00.487203Z TestFramework (INFO): -> range [0,99], last address 2MxpNQE4tcoDEYfyQzuXbeW2fqwcKMbgW6k
    412026-03-14T14:11:00.497411Z TestFramework (INFO): -> range [0,99], last address 2Mw6n7anjw4cBnwVAsTnSJzoBkARvoXNEPV
    422026-03-14T14:11:00.508043Z TestFramework (INFO): -> range [0,99], last address bcrt1p2fdjfxah28q62z4nwyfeh0y92pfxe0yfxdwwkll0ym85myreycyqxkau2x
    432026-03-14T14:11:00.513448Z TestFramework (INFO): -> range [0,99], last address bcrt1pfrttk0ll80lzyj77hae83gg5ph4hetk0nkrctn2qdgk7k3q04nzq9kuhfs
    442026-03-14T14:11:00.520729Z TestFramework (INFO): -> range [0,99], last address bcrt1q9cn8pruzkdvjaqsmul9ywftwrlyu025ssu5x04
    452026-03-14T14:11:00.531293Z TestFramework (INFO): -> range [0,99], last address bcrt1qut6r25ltj42efca9xj7v8wlrefpcpevfqa5tfn
    

    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”.

  25. 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.

  26. DrahtBot requested review from theStack on Mar 13, 2026
  27. rkrux changes_requested
  28. rkrux commented at 2:34 pm on March 14, 2026: contributor
    Removing a-c-k because of an issue.
  29. DrahtBot requested review from rkrux on Mar 14, 2026
  30. 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.
  31. 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!
  32. Eunovo force-pushed on Mar 16, 2026
  33. 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
  34. Eunovo force-pushed on Mar 16, 2026
  35. DrahtBot added the label CI failed on Mar 16, 2026
  36. 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.

  37. DrahtBot removed the label CI failed on Mar 16, 2026
  38. Bortlesboat commented at 6:41 pm on March 16, 2026: none
    re-ACK 92287ae75397755343f957ccfdda1933e9169d02
  39. DrahtBot requested review from ismaelsadeeq on Mar 16, 2026
  40. rkrux approved
  41. 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.

    0git range-diff e7796a5...92287ae
    
  42. 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.

  43. achow101 commented at 0:27 am on March 24, 2026: member
    ACK 92287ae75397755343f957ccfdda1933e9169d02
  44. achow101 merged this on Mar 24, 2026
  45. achow101 closed this on Mar 24, 2026


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-03-24 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me