test: add -alertnotify test for large work invalid chain warning #33893

1. 
   theStack commented at 9:57 pm on November 17, 2025: contributor

   This PR adds missing test coverage for the LARGE_WORK_INVALID_CHAIN fork warning, checked with the -alertnotify option: https://github.com/bitcoin/bitcoin/blob/ead849c9f177a3a175a22b35fa864b4b37fb9934/src/validation.cpp#L2033-L2040 Found that this is missing during review of #32587. The test works by first creating a bunch of invalid blocks, that are first announced by headers and then submitted fully in reverse (invalid tip first), in order to set m_best_invalid to that value, finally leading to the best chain / invalid chain gap of >= 6 blocks. I’d be curious if there are other (more realistic?) ways to test this. One simple alternative is just to call invalidateblock twice (once at the tip, once at the base of the invalid chain).

   Note that the written warning doesn’t include the exclamation mark, as it is removed via SanitizeString in the AlertNotify function.

2. DrahtBot added the label Tests on Nov 17, 2025
3. 
   DrahtBot commented at 9:57 pm on November 17, 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/33893.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	mzumsande, brunoerg
   Concept ACK	TheCharlatan
   Stale ACK	maflcko

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

4. theStack force-pushed on Nov 17, 2025
5. DrahtBot added the label CI failed on Nov 17, 2025
6. 
   DrahtBot commented at 10:01 pm on November 17, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19445926854/job/55640320613 LLM reason (✨ experimental): Python linter failed due to missing encoding in open() usage (lint-python-utf8-encoding).

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

7. theStack force-pushed on Nov 17, 2025
8. theStack force-pushed on Nov 18, 2025
9. DrahtBot removed the label CI failed on Nov 18, 2025
10. 
    maflcko commented at 6:44 pm on November 18, 2025: member

    The test works by first creating a bunch of invalid blocks, that are first announced by headers and then submitted fully in reverse (invalid tip first), in order to set m_best_invalid to that value, finally leading to the best chain / invalid chain gap of >= 6 blocks. I’d be curious if there are other (more realistic?) ways to test this. One simple alternative is just to call invalidateblock twice (once at the tip, once at the base of the invalid chain).

    Another alternative could be to create and submit one invalid block, then submit a longer chain forking enough blocks before the invalid block, but this seems even less likely to happen in a realistic scenario.

    Found that this is missing during review of #32587.

    corecheck does not show any new coverage here, so I guess there is a test (via invalidateblock?) which triggers this as a side-effect?

11. 
    brunoerg commented at 10:59 pm on November 18, 2025: contributor

    corecheck does not show any new coverage here, so I guess there is a test (via invalidateblock?) which triggers this as a side-effect?

    Exactly, it’s in feature_bip68_sequence. I guess it happens at: https://github.com/bitcoin/bitcoin/blob/53b72372da91c5e90df865bc15961e16feb4a983/test/functional/feature_bip68_sequence.py#L341-L344

12. 
    theStack commented at 12:05 pm on November 19, 2025: contributor

    In addition to feature_bip68_sequence.py, it seems that the functional tests feature_coinstatsindex.py, feature_pruning.py and mempool_reorg.py hit this code-path as well (tested experimentally by putting an assert(false) into the branch to provoke a crash), most likely all of them in the course of an invalidateblock call, though I didn’t check in detail. However, none of these use the -alertnotify option, so the function AlertNotify returns early in those cases.
13. in test/functional/feature_notifications.py:174 in 8a40e10c1e

    170+        tip = self.nodes[0].getbestblockhash()
    171+        height = self.nodes[0].getblockcount()
    172+        block_time = self.nodes[0].getblock(tip)['time'] + 1
    173+
    174+        invalid_blocks = []
    175+        for i in range(7):  # invalid chain must be at least 6 blocks longer to trigger warning
    

    ---

    ---

    

    brunoerg commented at 2:45 pm on November 19, 2025:

    8a40e10c1ef4cfdd5fe19796131af2b5a34531ce: nit: i is not used.

    0        for _ in range(7):  # invalid chain must be at least 6 blocks longer to trigger warning
    

14. in test/functional/feature_notifications.py:175 in 8a40e10c1e

    171+        height = self.nodes[0].getblockcount()
    172+        block_time = self.nodes[0].getblock(tip)['time'] + 1
    173+
    174+        invalid_blocks = []
    175+        for i in range(7):  # invalid chain must be at least 6 blocks longer to trigger warning
    176+            block = create_block(int(tip, 16), create_coinbase(height + 1), block_time)
    

    ---

    ---

    

    maflcko commented at 3:59 pm on November 19, 2025:

    0        height = self.nodes[0].getblockcount() + 1
    1        block_time = self.nodes[0].getblock(tip)['time'] + 1
    2
    3        invalid_blocks = []
    4        for i in range(7):  # invalid chain must be at least 6 blocks longer to trigger warning
    5            block = create_block(int(tip, 16), create_coinbase(height), block_time)
    

    small nit: Could ensure time and height are incremented in the same places?

15. maflcko approved
16. 
    maflcko commented at 4:00 pm on November 19, 2025: member

    review ACK 8a40e10c1ef4cfdd5fe19796131af2b5a34531ce 🌃

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: review ACK 8a40e10c1ef4cfdd5fe19796131af2b5a34531ce 🌃
    3z9Kn9hOppf0Zvgnc5/T8ijsN/kBcTnuRyg+GpdqeiKUNivwTtHzHV1KTkJDFxY8kZCqfAfHgT46+XMD8y8T3Ag==
    

17. in test/functional/feature_notifications.py:27 in 8a40e10c1e

    23@@ -20,6 +24,9 @@
    24 FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/'
    25 UNCONFIRMED_HASH_STRING = 'unconfirmed'
    26 
    27+LARGE_WORK_INVALID_CHAIN_WARNING = "Warning: We do not appear to fully agree with our peers You may need to upgrade, or other nodes may need to upgrade."
    

    ---

    ---

    

    maflcko commented at 4:10 pm on November 19, 2025:

    nit: Could move the comment from the pull description to a code comment?

    0LARGE_WORK_INVALID_CHAIN_WARNING = (
    1    "Warning: We do not appear to fully agree with our peers "  # Exclamation mark removed by SanitizeString in AlertNotify
    2    "You may need to upgrade, or other nodes may need to upgrade."
    3)
    

18. in test/functional/feature_notifications.py:189 in 8a40e10c1e

    185+
    186+        # submit headers of invalid blocks
    187+        for invalid_block in invalid_blocks:
    188+            self.nodes[0].submitheader(invalid_block.serialize().hex())
    189+        # submit invalid blocks in reverse order (tip first, to set m_best_invalid)
    190+        for invalid_block in invalid_blocks[::-1]:
    

    ---

    ---

    

    brunoerg commented at 4:52 pm on November 19, 2025:

    8a40e10c1ef4cfdd5fe19796131af2b5a34531ce: nit: I think that using reversed would be clearer.

    0        for invalid_block in reversed(invalid_blocks):
    

19. brunoerg approved
20. 
    brunoerg commented at 5:05 pm on November 19, 2025: contributor

    ACK 8a40e10c1ef4cfdd5fe19796131af2b5a34531ce

    Code lgtm. I verified that reducing one block doesn’t trigger the warning and makes the test to fail.

21. 
    mzumsande commented at 7:08 pm on November 19, 2025: contributor

    Concept ACK

    FYI I proposed to reword the warning message in #33553 (7f284835be87c4d1a8a56804992043e12dae1ea1).

22. test: add `-alertnotify` test for large work invalid chain warning c5b301f6ac
23. theStack force-pushed on Nov 19, 2025
24. 
    theStack commented at 9:11 pm on November 19, 2025: contributor

    @brunoerg @maflcko: Thanks for reviewing, I addressed all the nits.

    FYI I proposed to reword the warning message in #33553 (7f28483).

    Ah nice, makes sense to unify, was already wondering why there are two different messages, each one only mentioning one possible cause.

25. 
    TheCharlatan commented at 9:24 pm on November 19, 2025: contributor

    Nice, Concept ACK
26. in test/functional/feature_notifications.py:177 in c5b301f6ac

    173+        tip = self.nodes[0].getbestblockhash()
    174+        height = self.nodes[0].getblockcount() + 1
    175+        block_time = self.nodes[0].getblock(tip)['time'] + 1
    176+
    177+        invalid_blocks = []
    178+        for _ in range(7):  # invalid chain must be at least 6 blocks longer to trigger warning
    

    ---

    ---

    

    mzumsande commented at 9:48 pm on November 19, 2025:

    nit: isn’t “at least” imprecise, and you need more than 6 blocks? The validation codes uses > instead of >= and the test fails if I create just 6 blocks on top of the tip.
27. 
    mzumsande commented at 10:12 pm on November 19, 2025: contributor

    ACK c5b301f6ac0d3f882cc55397e3fb0cf95058704b

    Fun fact: The 6-block warning is very old, introduced by Satoshi in https://github.com/bitcoin/bitcoin/commit/94cfec07fd302c9ff9b6a80c47418d4fe56596ae

28. DrahtBot requested review from TheCharlatan on Nov 19, 2025
29. DrahtBot requested review from brunoerg on Nov 19, 2025
30. DrahtBot requested review from maflcko on Nov 19, 2025
31. brunoerg approved
32. 
    brunoerg commented at 10:52 pm on November 19, 2025: contributor

    reACK c5b301f6ac0d3f882cc55397e3fb0cf95058704b

Contributors
theStack DrahtBot maflcko brunoerg mzumsande TheCharlatan

Review Requested
maflcko TheCharlatan

Labels
Tests