test: fix intermittent failure in rpc_invalidateblock.py #32968

1. 
   stratospher commented at 9:27 am on July 14, 2025: contributor

   resolves #32965.

   node1 (with 24 blocks) causes node0 (with 6 blocks + 1 extra header) to silently reorg. so move the subtest to a point before the 20 blocks are generated so that node1’s state doesn’t cause node0 to silently reorg.

2. DrahtBot added the label Tests on Jul 14, 2025
3. 
   DrahtBot commented at 9:27 am on July 14, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32968.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, mzumsande

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • “Since header doesn’t have block data, it can’t be chain tip” -> “Since the header doesn’t have block data, it can’t be the chain tip” [missing definite articles]

   drahtbot_id_4_m

4. in test/functional/rpc_invalidateblock.py:104 in 4aa0e4d9ce outdated

   100@@ -101,6 +101,7 @@ def run_test(self):
   101         assert_equal(self.nodes[0].getbestblockhash(), blockhash_3)
   102         assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 3)
   103         assert_equal(self.nodes[0].getblockchaininfo()['headers'], 3)
   104+        self.disconnect_nodes(0, 1)
   

   ---

   ---

   

   maflcko commented at 9:53 am on July 14, 2025:

   This means in the remainder of the test, the state of the two nodes shouldn’t affect each other. So the reconsiderblock+test

   0        assert_equal(self.nodes[0].getbestblockhash(), blockhash_6)
   1        assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6)
   2        assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7)
   

   Can be moved after this line to disconnect the nodes? This would bundle the tests more nicely, but no strong opinion.

   ---

   

   stratospher commented at 1:18 pm on July 14, 2025:

   yeah makes sense, I guess this was the reason some of the test was moved up in #30479 (comment)
5. 
   maflcko commented at 9:53 am on July 14, 2025: member

   lgtm ACK 4aa0e4d9ce6283665b1aac185da67543b8df606e

   Also, left a nit/questsion

6. test: fix intermittent failure in rpc_invalidateblock.py

   node1 (with 24 blocks) causes node0 (with 6 blocks) to silently
   reorg. so move the subtest to a point before the 20 blocks are
   generated so that node1's state doesn't cause node0 to silently
   reorg.

   28416f367a
7. stratospher force-pushed on Jul 14, 2025
8. in test/functional/rpc_invalidateblock.py:107 in 28416f367a

   101@@ -102,6 +102,14 @@ def run_test(self):
   102         assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 3)
   103         assert_equal(self.nodes[0].getblockchaininfo()['headers'], 3)
   104 
   105+        # Reconsider the header
   106+        self.nodes[0].reconsiderblock(block.hash)
   107+        # Since header doesn't have block data, it can't be chain tip
   

   ---

   ---

   

   maflcko commented at 2:39 pm on July 14, 2025:

   “Since header doesn't have block data, it can't be chain tip” -> “Since the header doesn't have block data, it can't be the chain tip” [missing definite articles]
   

9. 
   maflcko commented at 2:40 pm on July 14, 2025: member

   lgtm ACK 28416f367a5d720d89214aa8ef94013caa1d1a40

   Can be tested via by moving the lines after self.nodes[0].reconsiderblock(block.hash) down again and observing a failure.

10. 
    mzumsande commented at 3:29 pm on July 14, 2025: contributor

    Code Review ACK 28416f367a5d720d89214aa8ef94013caa1d1a40
11. fanquake merged this on Jul 15, 2025
12. fanquake closed this on Jul 15, 2025

    ---

Contributors
stratospher DrahtBot maflcko mzumsande

Labels
Tests