fuzz: Add check in p2p_headers_presync that chain work never exceeds minimum work #30918

pull marcofleon wants to merge 2 commits into bitcoin:master from marcofleon:2024/09/headers-presync-fuzz-followup changing 2 files +26 −9
  1. marcofleon commented at 5:21 pm on September 17, 2024: contributor

    A followup to #30661

    The added assertion just makes sure that the fuzz test is working as intended. If we’re sure that the total work of the test chain is never more than minimum chain work, then we can be sure that the later assertion failure would actually mean that a bug in the headers presync logic was found.

    This PR also addresses: #30661 (review) #30661 (review) #30661 (review)

  2. DrahtBot commented at 5:21 pm on September 17, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK instagibbs, maflcko, achow101

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

  3. DrahtBot added the label Tests on Sep 17, 2024
  4. in src/test/fuzz/p2p_headers_presync.cpp:207 in 7b85fd1443 outdated
    201             });
    202+
    203+        total_work += CalculateClaimedHeadersWork(all_headers);
    204+
    205+        // This test should never create a chain with more work than MinimumChainWork.
    206+        assert(total_work < chainman.MinimumChainWork());
    


    instagibbs commented at 1:10 pm on September 18, 2024:

    IIUC this would be a conservative overestimate, since the compact block and block messages don’t move base forward?

    If so, probably should note that here.


    marcofleon commented at 2:08 pm on September 20, 2024:
  5. instagibbs approved
  6. instagibbs commented at 1:14 pm on September 18, 2024: member

    ACK 7b85fd1443488afe60235e4c33bd956cb9c8d562

    Thanks for the quick follow-up on suggestion! Makes the test significantly easier to read.

  7. add clarification in comment 9aa5d1c3fc
  8. add check that chainwork doesn't exceed minimum work 284bd17309
  9. marcofleon force-pushed on Sep 20, 2024
  10. marcofleon commented at 2:12 pm on September 20, 2024: contributor
    Updated the PR description to include some additional changes.
  11. instagibbs commented at 2:26 pm on September 20, 2024: member
    reACK 284bd17309ab3b124d9dcddfec62f5506383343b
  12. maflcko commented at 2:46 pm on September 20, 2024: member
    review ACK 284bd17309ab3b124d9dcddfec62f5506383343b
  13. achow101 commented at 6:27 pm on September 20, 2024: member
    ACK 284bd17309ab3b124d9dcddfec62f5506383343b
  14. achow101 merged this on Sep 20, 2024
  15. achow101 closed this on Sep 20, 2024


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: 2024-10-08 16:12 UTC

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