refactor: Header sync optimisations & simplifications #32740

pull danielabrozzoni wants to merge 8 commits into bitcoin:master from danielabrozzoni:upforgrabs/25968 changing 15 files +100 −110
  1. danielabrozzoni commented at 8:20 pm on June 12, 2025: contributor

    This is a partial* revival of #25968

    It contains a list of most-unrelated simplifications and optimizations to the code merged in #25717:

    • Make ProcessNextHeaders use the headers argument as in/out: This allows reusing the same vector storage for input and output headers to HeadersSync::ProcessNextHeaders. It’s also natural in the sense that this argument just represents the headers-to-be-processed, both in the caller and the callee, and both before and after the calls.
    • Avoid an IsAncestorOfBestHeaderOrTip call: Just don’t call this function when it won’t have any effect.
    • Compute work from headers without CBlockIndex: Avoid the need to construct a CBlockIndex object just to compute work for a header, when its nBits value suffices for that. Also use some Spans where possible.
    • Remove useless CBlock::GetBlockHeader: There is no need for a function to convert a CBlock to a CBlockHeader, as it’s a child class of it.

    It also contains the following optimizations, which were suggested by reviewers in #25968:

    • Remove redundant parameter from CheckHeadersPoW: No need to pass consensusParams, as CheckHeadersPow already has access to m_chainparams.GetConsensus()
    • Remove unused parameter in ReportHeadersPresync
    • Use initializer list in CompressedHeader, also make GetFullHeader const
    • Use reference for chain_start in HeadersSyncState: chain_start can never be null, so it’s better to pass it as a reference rather than a raw pointer

    *I decided to leave out two commits that were in #25968 (ab52fb4e95aa2732d1a1391331ea01362e035984, 7f1cf440ca1a9c86085716745ca64d3ac26957c0), since they’re a bit more involved, and I’m a new contributor. If this PR gets merged, I’ll comment under #25968 to note that these two commits are still up for grabs :)

  2. refactor: Make ProcessNextHeaders use the headers argument as in/out
    This allows reusing the same vector storage for input and output headers to
    HeadersSync::ProcessNextHeaders. It's also natural in the sense that this argument
    just represents the headers-to-be-processed, both in the caller and the callee, and
    both before and after the calls.
    
    Co-Authored-By: Daniela Brozzoni <danielabrozzoni@protonmail.com>
    16d16f3e08
  3. p2p: Avoid an IsAncestorOfBestHeaderOrTip call
    Just don't call this function when it won't have any effect.
    98f6b49dae
  4. refactor: Compute work from headers without CBlockIndex
    Avoid the need to construct a CBlockIndex object just to compute work for a header,
    when its nBits value suffices for that. Also use some Spans where possible.
    2dcf2e4a85
  5. refactor: Remove useless CBlock::GetBlockHeader
    There is no need for a function to convert a CBlock to a CBlockHeader, as it's a child
    class of it.
    dd40ce1e2e
  6. refactor: Remove redundant parameter from CheckHeadersPoW
    No need to pass consensusParams, as CheckHeadersPow already has access
    to m_chainparams.GetConsensus()
    
    Co-Authored-By: maflcko <6399679+maflcko@users.noreply.github.com>
    c6beb267ef
  7. refactor: Remove unused parameter in ReportHeadersPresync
    Co-Authored-By: Aurèle Oulès <aurele@oules.com>
    7fa83b43d7
  8. refactor: Use initializer list in CompressedHeader
    Also mark GetFullHeader as const
    
    Co-Authored-By: Aurèle Oulès <aurele@oules.com>
    3ff26e5c06
  9. refactor: Use reference for chain_start in HeadersSyncState
    chain_start can never be null, so it's better to pass it as a reference
    rather than a raw pointer
    
    Co-Authored-By: maflcko <6399679+maflcko@users.noreply.github.com>
    13e123e7cc
  10. DrahtBot commented at 8:20 pm on June 12, 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/32740.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32579 (headerssync: Preempt unrealistic unit test behavior by hodlinator)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  11. DrahtBot added the label Refactoring on Jun 12, 2025
  12. ajtowns commented at 2:43 am on June 13, 2025: contributor
    Maybe change the title to describe what’s actually going on for those of us who don’t memorise every pr number? :) (“Header sync optimisations/simplifications” ?)
  13. danielabrozzoni renamed this:
    refactor: Optimizations & simplifications following #25717
    refactor: Header sync optimisations & simplifications #25717
    on Jun 13, 2025
  14. danielabrozzoni renamed this:
    refactor: Header sync optimisations & simplifications #25717
    refactor: Header sync optimisations & simplifications
    on Jun 13, 2025

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: 2025-06-15 06:13 UTC

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