p2p: TxOrphanage revamp cleanups #32941

pull glozow wants to merge 27 commits into bitcoin:master from glozow:2025-07-orphanage-followups changing 25 files +2645 −905
  1. glozow commented at 4:05 pm on July 10, 2025: member

    Followup to #31829:

    • Release notes
    • Have the orphanage auto-trim itself whenever necessary (and test changes) #31829 (review)
    • Reduce duplicate reconsiderations by keeping track of which txns are already reconsiderable so we only mark it for reconsideration for 1 peer at a time
    • Rename OrphanTxBase to OrphanInfo
    • Get rid of Size() method by replacing all calls with CountUniqueOrphans
  2. [txorphanage] change type of usage to int64_t
    Since this field holds a total number of bytes, overflow is within the
    realm of possibility. Use int64 to be safe.
    ef801dd625
  3. [prep/refactor] move txorphanage to node namespace and directory
    This is move-only.
    ff3c693467
  4. [prep/rpc] remove entry and expiry time from getorphantxs
    Expiry is going away in a later commit.
    This is only an RPC change. Behavior of the orphanage does not change.
    Note that getorphantxs is marked experimental.
    7ac253436b
  5. [fuzz] add SeedRandomStateForTest(SeedRand::ZEROS) to txorphan 0b54034705
  6. [prep/test] modify test to not access TxOrphanage internals
    These internals should and will be private.
    4aff35d107
  7. [prep/config] remove -maxorphantx
    The orphanage will no longer have a maximum number of unique orphans.
    22a1a1b1d4
  8. [prep/refactor] move DEFAULT_MAX_ORPHAN_TRANSACTIONS to txorphanage.h
    This is move only.
    33e2cc34c2
  9. [prep/test] have TxOrphanage remember its own limits in LimitOrphans
    Move towards a model where TxOrphanage is initialized with limits that
    it remembers throughout its lifetime.
    Remove the param. Limiting by number of unique orphans will be removed
    in a later commit.
    Now that -maxorphantx is gone, this does not change the node behavior.
    The parameter is only used in tests.
    50a53f5e11
  10. [prep/refactor] make TxOrphanage a virtual class implemented by TxOrphanageImpl f2f33c3ec7
  11. [prep] change return type of EraseTx to bool
    This function only ever returns 0 or 1 (number of unique orphans
    erased).
    d4376eab81
  12. [refactor] create aliases for TxOrphanage Count and Usage d1032fc115
  13. glozow added the label P2P on Jul 10, 2025
  14. DrahtBot commented at 4:05 pm on July 10, 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/32941.

    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:

    • #32827 (mempool: Avoid needless vtx iteration during IBD by l0rinc)
    • #32730 (p2p: avoid traversing blocks (twice) during IBD by furszy)
    • #32631 (refactor: Convert GenTxid to std::variant by marcofleon)
    • #32430 (test: Add and use ElapseTime helper by maflcko)
    • #32189 (refactor: Txid type safety (parent PR) by marcofleon)
    • #29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
    • #28690 (build: Introduce internal kernel library by TheCharlatan)

    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.

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • tranaction -> transaction [spelling error in comment “actual memory usage of the tranaction”]
    • comphehensive -> comprehensive [spelling error in “comphehensive simulation fuzz test”]

    drahtbot_id_4_m

  15. glozow renamed this:
    TxOrphanage revamp cleanups
    p2p: TxOrphanage revamp cleanups
    on Jul 10, 2025
  16. [p2p] overhaul TxOrphanage with smarter limits
    This is largely a reimplementation using boost::multi_index_container.
    All the same public methods are available. It has an index by outpoint,
    per-peer tracking, peer worksets, etc.
    
    A few differences:
    - Limits have changed: instead of a global limit of 100 unique orphans,
      we have a maximum number of announcements (which can include duplicate
    orphans) and a global memory limit which scales with the number of
    peers.
    - The maximum announcements limit is 100 to match the original limit,
      but this is actually a stricter limit because the announcement count
    is not de-duplicated.
    - Eviction strategy: when global limits are reached, a per-peer limit
      comes into play. While limits are exceeded, we choose the peer whose
    “DoS score” (max usage / limit ratio for announcements and memory
    limits) is highest and evict announcements by entry time, sorting
    non-reconsiderable ones before reconsiderable ones. Since announcements
    are unique by (wtxid, peer), as long as 1 announcement remains for a
    transaction, it remains in the orphanage.
    - This eviction strategy means no peer can influence the eviction of
      another peer’s orphans.
    - Also, since global limits are a multiple of per-peer limits, as long
      as a peer does not exceed its limits, its orphans are protected from
    eviction.
    - Orphans no longer expire, since older announcements are generally
      removed before newer ones.
    - GetChildrenFromSamePeer returns the transactions from newest to
      oldest.
    
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    0e5ab863c0
  17. [cleanup] remove unused rng param from LimitOrphans 3489f8e693
  18. [unit test] basic TxOrphanage eviction and protection 1560bab989
  19. [unit test] strengthen GetChildrenFromSamePeer tests: results are in recency order 588aa9a4c4
  20. [fuzz] TxOrphanage protects peers that don't go over limit
    Co-authored-by: Greg Sanders <gsanders87@gmail.com>
    2e34a5c250
  21. [p2p] bump DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE to 3,000
    For the default number of peers (125), allows each to relay a default
    descendant package (up to 25-1=24 can be missing inputs) of small (9
    inputs or fewer) transactions out of order.
    
    This limit also gives acceptable bounds for worst case LimitOrphans iterations.
    
    Functional tests aren't changed to check for larger cap because it would
    make the runtime too long.
    
    Also deletes the now-unused DEFAULT_MAX_ORPHAN_TRANSACTIONS.
    ee438b3d0a
  22. [prep] Return the made-reconsiderable announcements in AddChildrenToWorkSet
    This is preparation for the simulation fuzz test added in a later commit. Since
    AddChildrenToWorkSet consumes randomness, there is no way for the simulator to
    exactly predict its behavior. By returning the set of made-reconsiderable announcements
    instead, the simulator can instead test that it is *a* valid choice, and then
    apply it to its own data structures.
    78be3a0bff
  23. [fuzz] Add simulation fuzz test for TxOrphanage
    This adds a large simulation fuzz test for all TxOrphanage public interface
    functions, using a mix of comparison with expected behavior (in case it is
    fully specified), and testing of properties exhibited otherwise.
    ef2fac9315
  24. [prep/test] restart instead of bumpmocktime between p2p_orphan_handling subtests
    If we want to restart at all during the tests, we can't have future timestamps.
    3f4aebaf5d
  25. [functional test] orphan resolution works in the presence of DoSy peers
    Co-authored-by: Greg Sanders <gsanders87@gmail.com>
    0d82ef4a1f
  26. [bench] worst case LimitOrphans and EraseForBlock
    Co-authored-by: Greg Sanders <gsanders87@gmail.com>
    76e65b8d29
  27. [doc] 31829 release note 46997b2de9
  28. [refactor] make TxOrphanage keep itself trimmed 90e10ea167
  29. [optimization] Maintain at most 1 reconsiderable announcement per wtxid
    This introduces an invariant that TxOrphanageImpl never holds more than one
    announcement with m_reconsider=true for a given wtxid. This avoids duplicate
    work, both in the caller might otherwise reconsider the same transaction multiple
    times before it is ready, and internally in AddChildrenToWorkSet, which might
    otherwise iterate over all announcements multiple times.
    b2f1a8dfe8
  30. [cleanup] replace TxOrphanage::Size() with CountUniqueOrphans e25588bbfa
  31. [cleanup] rename OrphanTxBase to OrphanInfo
    This struct was previously used by the underlying data strcuture. Now,
    it is only used to return information to external callers, so "Base"
    does not make sense anymore.
    f6e9fc5d54
  32. glozow force-pushed on Jul 10, 2025
  33. DrahtBot added the label CI failed on Jul 10, 2025
  34. DrahtBot commented at 9:04 pm on July 10, 2025: contributor

    🚧 At least one of the CI tasks failed. Task fuzzer,address,undefined,integer, no depends: https://github.com/bitcoin/bitcoin/runs/45743710188 LLM reason (✨ experimental): Fuzz test failed due to an assertion failure causing a deadly signal crash.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.


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-07-11 12:13 UTC

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