test: Fix intermittent issues in feature_assumevalid.py #34571

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2602-test-av-int-fail changing 1 files +51 −63
  1. maflcko commented at 10:29 am on February 12, 2026: member

    The test has many issues:

    • The send_blocks_until_disconnected seems to imply to wait until a disconnect happens. However, in reality it will blindly send all blocks and then return early, when done or when a disconnect happens. This will cause test failures when python is running faster than Bitcoin Core, for example when using sanitizers.
    • The assert_debug_log scopes are bloated, which makes finding the above issue harder. This is, because a test failure will basically print the 1000+ line debug log excerpt twice, with the second instance stripped of useful python test logs. Also, the checks are less precise, if they happen over a larger scope/snippet.

    Fix all issues, by:

    • Removing send_blocks_until_disconnected and just sending the blocks that are needed to get the disconnect and then explicitly waiting for the disconnect.
    • Reduce the scopes of the debug log checks. This can be reviewed with the git options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space.
  2. test: Fix intermittent issues in feature_assumevalid.py fa90d44a22
  3. DrahtBot added the label Tests on Feb 12, 2026
  4. DrahtBot commented at 10:29 am on February 12, 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 l0rinc, achow101

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

  5. maflcko commented at 10:36 am on February 12, 2026: member

    Locally, I can reproduce the intermittent issue via while ./bld-cmake/test/functional/feature_assumevalid.py --timeout-factor=4 --valgrind ; do echo >> /tmp/runs_fa ; done.

    An alternative could be:

     0diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
     1index c7be6abc3a..aacb7b6563 100644
     2--- a/src/validationinterface.cpp
     3+++ b/src/validationinterface.cpp
     4@@ -14,2 +14,3 @@
     5 #include <primitives/transaction.h>
     6+#include <random.h>
     7 #include <util/check.h>
     8@@ -156,2 +157,4 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
     9 
    10+static FastRandomContext g_rnd{};
    11+
    12 // Use a macro instead of a function for conditional logging to prevent
    13@@ -166,2 +169,3 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
    14             LOG_EVENT(fmt, local_name, __VA_ARGS__);           \
    15+            if(g_rnd.randrange(2)<1)UninterruptibleSleep(15ms);\
    16             event();                                           \
    
  6. fanquake added this to the milestone 31.0 on Feb 12, 2026
  7. maflcko commented at 1:24 pm on February 17, 2026: member
    cc @l0rinc as the author of the code, you may be qualified to review this
  8. fanquake referenced this in commit 9e4567b17a on Feb 18, 2026
  9. l0rinc commented at 5:35 pm on February 18, 2026: contributor

    ACK fa90d44a22838591e47eec206c23f87d69ad842a

    The test has many issues

    As the author of this test with many issues, thanks for the ping and for fixing the racy tests.

    I had to separate the change into smaller commits to understand what was being changed. I pushed it to https://github.com/l0rinc/bitcoin/pull/120/commits in case you decide to take anything from it:

    • send the first 103 blocks explicitly, then wait for disconnect
    • convert inline comments to logs
    • move-only P2P/header/chain setup outside the assert to minimize scope
    • check the first block explicitly, then process the rest
    • replace residual wait_until polling with synchronized sends and direct assert_equal
    • add the missing node2 invalid-chaintip assertion, and simplify the node1 height check to avoid an unnecessary RPC call

    I can confirm this passes with the UninterruptibleSleep hack for cmake -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build -j && build/test/functional/test_runner.py feature_assumevalid.py, and fails reliably without these fixes.

  10. maflcko commented at 8:49 am on February 19, 2026: member
    Thanks. I think I’ll leave it as-is for now, but reviewers are welcome to review your branch instead, if that is easier for them. Afterward, they can confirm there is no diff ($ git diff fa90d44a22838591e47eec206c23f87d69ad842a de739aaa8a3d4ae9218cf808efe9de73c47b6087 -- test/functional/feature_assumevalid.py) and ack the single commit here.
  11. achow101 commented at 10:03 pm on February 19, 2026: member
    ACK fa90d44a22838591e47eec206c23f87d69ad842a
  12. achow101 merged this on Feb 20, 2026
  13. achow101 closed this on Feb 20, 2026

  14. maflcko deleted the branch on Feb 20, 2026


maflcko DrahtBot l0rinc achow101

Labels
Tests

Milestone
31.0


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-09 18:13 UTC

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