[tests] Fix feature_block flakiness #13048

pull jnewbery wants to merge 1 commits into bitcoin:master from jnewbery:fix_feature_block_flakiness changing 1 files +7 −1
  1. jnewbery commented at 7:51 PM on April 20, 2018: member

    feature_block.py occasionally fails on Travis. I believe this is due to a a race condition when reconnecting to bitcoind after a subtest that expects disconnection. If the test runs ahead and sends the INV for the subsequent test before we've received the initial sync getheaders, then we may end up sending two headers messages - one as a response to the initial sync getheaders and one in response to the INV getheaders. If both of those headers fail validation with a DoS score of 50 or higher, then we'll unexpectedly be disconnected.

    There is only one validation failure that has a DoS score bewteen 50 and 100, which is high-hash. That's why the test is failing immediately after the "Reject a block with invalid work" subtest.

    Fix is to wait for the initial getheaders from the peer before we start populating our blockstore. That way we won't have any invalid headers to respond to it with.

  2. [tests] Fix feature_block flakiness
    feature_block.py occasionally fails on Travis. I believe this is due to
    a a race condition when reconnecting to bitcoind after a subtest that
    expects disconnection. If the test runs ahead and sends the INV for the
    subsequent test before we've received the initial sync getheaders, then
    we may end up sending two headers messages - one as a response to the
    initial sync getheaders and one in response to the INV getheaders. If
    both of those headers fail validation with a DoS score of 50 or higher,
    then we'll unexpectedly be disconnected.
    
    There is only one validation failure that has a DoS score bewteen 50 and
    100, which is high-hash. That's why the test is failing immediately
    after the "Reject a block with invalid work" subtest.
    
    Fix is to wait for the initial getheaders from the peer before we
    start populating our blockstore. That way we won't have any invalid
    headers to respond to it with.
    c1d742025c
  3. jnewbery commented at 7:53 PM on April 20, 2018: member

    This test bug would disappear with #11639, since that changes the DoS disconnect/ban behaviour. However, this change is probably still useful to ensure that there's no cross-contamination between subtests.

  4. jnewbery commented at 7:54 PM on April 20, 2018: member

    Longer term, can we just have proper unit tests for validation please :grin:

  5. ryanofsky commented at 8:07 PM on April 20, 2018: member

    utACK c1d742025caec62cf38cea1166fa6c4d0f7e207b. I'm not very familiar with this test but the explanation and fix make sense. Is a similar fix needed for the new invalid_tx test in #13003 that is now getting disconnected?

  6. jnewbery commented at 8:25 PM on April 20, 2018: member

    Is a similar fix needed for the new invalid_tx test in #13003 that is now getting disconnected?

    No - this is specific to invalid block headers (and more specifically - only to block headers that fail PoW validation).

  7. fanquake added the label Tests on Apr 20, 2018
  8. PierreRochard commented at 12:33 AM on April 21, 2018: contributor

    utACK c1d7420

  9. MarcoFalke merged this on Apr 22, 2018
  10. MarcoFalke closed this on Apr 22, 2018

  11. MarcoFalke referenced this in commit cac6d1184d on Apr 22, 2018
  12. UdjinM6 referenced this in commit 12568388cd on Oct 2, 2019
  13. UdjinM6 referenced this in commit bf712c5c5c on Oct 3, 2019
  14. codablock referenced this in commit d21031d428 on Oct 3, 2019
  15. codablock referenced this in commit 69caee2e09 on Jan 5, 2020
  16. codablock referenced this in commit 4e0016c562 on Jan 5, 2020
  17. codablock referenced this in commit c00e23d1e7 on Jan 7, 2020
  18. codablock referenced this in commit 91fad6198a on Jan 8, 2020
  19. UdjinM6 referenced this in commit 91b4a38398 on Jan 11, 2020
  20. ckti referenced this in commit 3170bf03b0 on Mar 28, 2021
  21. TheArbitrator referenced this in commit 9a316791fc on Jun 4, 2021
  22. TheArbitrator referenced this in commit 4c2d3bbb57 on Jun 9, 2021
  23. TheArbitrator referenced this in commit 454ad73b46 on Jun 21, 2021
  24. gades referenced this in commit 211cda13b3 on Jun 30, 2021
  25. MarcoFalke locked this on Sep 8, 2021

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-04-30 12:15 UTC

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