fees: Return false for incompatible fee estimates #35830

pull HowHsu wants to merge 1 commits into bitcoin:master from HowHsu:fuzz-policy-estimator-io-stability changing 1 files +36 −34
  1. HowHsu commented at 2:49 PM on July 28, 2026: contributor

    policy_estimator_io deliberately reuses a CBlockPolicyEstimator because constructing one for every fuzz input severely reduces throughput. However, Read() returns true for an incompatible old fee estimates file without replacing the estimator state. The target then calls Write() with state loaded by a previous input, making coverage depend on corpus order.

    Return false for incompatible files so the target skips Write() when no state was loaded. This keeps the estimator reuse optimization instead of resetting the expensive object before every fuzz input.

    For the in-tree production caller, incompatible files remain non-fatal and the estimator still starts from its default state. Read() now reports failure, so startup emits one additional non-fatal warning. Node startup and estimator state are unchanged, as is RPC behavior.

  2. DrahtBot added the label TX fees and policy on Jul 28, 2026
  3. DrahtBot commented at 2:49 PM on July 28, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    Stale ACK ismaelsadeeq

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. ismaelsadeeq approved
  5. ismaelsadeeq commented at 9:52 AM on July 29, 2026: member

    ACK f23debfee28169388ee6a2e9f7d38ee99fbfda40

    Also seems misleading that we do not warn that the block estimator file was not read.

  6. fees: Return false for incompatible fee estimates
    policy_estimator_io deliberately reuses a CBlockPolicyEstimator because
    constructing one for every fuzz input severely reduces throughput.
    However, Read() returns true for an incompatible old fee estimates file
    without replacing the estimator state. The target then calls Write()
    with state loaded by a previous input, making coverage depend on corpus
    order.
    
    Return false for incompatible files so the target skips Write() when no
    state was loaded. This keeps the estimator reuse optimization instead of
    resetting the expensive object before every fuzz input.
    
    For the in-tree production caller, incompatible files remain non-fatal
    and the estimator still starts from its default state. Read() now
    reports failure, so startup emits one additional non-fatal warning.
    Node startup and estimator state are unchanged, as is RPC behavior.
    80299ac874
  7. in src/policy/fees/block_policy_estimator.cpp:1019 in f23debfee2 outdated
    1015 | @@ -1016,6 +1016,7 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein)
    1016 |  
    1017 |          if (nVersionRequired < CURRENT_FEES_FILE_VERSION) {
    1018 |              LogWarning("Incompatible old fee estimation data (non-fatal). Version: %d", nVersionRequired);
    1019 | +            return false;
    


    sedited commented at 9:56 AM on July 29, 2026:

    Is there still a need for the else case after this?


    HowHsu commented at 12:50 PM on July 29, 2026:

    Thanks for the review, I've updated this.

  8. HowHsu force-pushed on Jul 29, 2026

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-07-31 20:50 UTC

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