txorphanage: bound orphan memory by storing transactions serialized #36015

pull brunoerg wants to merge 7 commits into bitcoin:master from brunoerg:2026-08-orphanage-mem-bug changing 10 files +434 −146
  1. brunoerg commented at 9:11 PM on August 18, 2026: contributor

    The orphanage limits the "usage" of the orphans it stores, per peer and globally, to bound the amount of memory an attacker can make us hold on to. It uses weight as a proxy for that memory, on the assumption that weight is "often higher than the actual memory usage of the transaction".

    That assumption does not hold for a deserialized transaction. Every witness stack element is an individually heap-allocated vector, costing its 24-byte slot in the stack vector plus a 32-byte minimum allocation, while only weighing 2WU. A transaction of 199,000 1-byte witness elements weighs 398,247WU (i.e. it is of standard weight, and witness standardness cannot be checked while the inputs are missing), but uses 11.1MB of memory: 28 times what is accounted for it, and one such orphan can be retained per peer.

    Rather than change the accounting metric, keep orphans in serialized form, deserializing them again on the paths that hand them back out. Serialized, a transaction's memory usage is bounded by its weight, so the existing weight-based accounting becomes a true upper bound on memory and the worst case is the peers' combined allowances.

    Admission, eviction and accounting behavior are unchanged: no transaction that was previously accepted is refused, so orphan resolution (and thus 1p1c package relay) keeps working for standard-weight transactions whose witnesses consist of many small elements, such as BitVM-style transactions.

  2. DrahtBot commented at 9:11 PM on August 18, 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/36015.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    ACK nervana21
    Concept ACK w0xlt, l0rinc, Crypt-iQ
    Stale ACK jeanpablojp

    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.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35919 (p2p: avoid orphanage abort at high peer counts by l0rinc)
    • #35569 (Encapsulation for CTransaction by purpleKarrot)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. w0xlt commented at 9:15 PM on August 18, 2026: contributor

    Concept ACK.

  4. brunoerg marked this as a draft on Aug 18, 2026
  5. DrahtBot added the label CI failed on Aug 18, 2026
  6. DrahtBot commented at 10:42 PM on August 18, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/32186423022/job/95870987922</sub> <sub>LLM reason (✨ experimental): CI failed due to an assertion failure in the fuzz test txorphan (txorphan.cpp:182), causing the fuzzer to exit with code 1.</sub>

    <details><summary>Hints</summary>

    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.

    </details>

  7. l0rinc commented at 11:08 PM on August 18, 2026: contributor

    Concept ACK The failing orphanage tests seem related (the musl one is the new test, the fuzz ones need a rebase, iwyu is the bench) and the change mixes refactors and hardening - can you check if it's possible to simplify and do refactors that aren't strictly related in follow-ups instead? Especially since #35923 is related and complements this change, and #35919 is also touching the same area.

  8. brunoerg force-pushed on Aug 18, 2026
  9. brunoerg commented at 11:22 PM on August 18, 2026: contributor

    The failing orphanage tests seem related (the musl one is the new test, the fuzz ones need a rebase, iwyu is the bench) and the change mixes refactors and hardening - can you check if it's possible to simplify and do refactors that aren't strictly related in follow-ups instead?

    My bad on that, forgot to push since my latest local change. Just did it.

  10. brunoerg force-pushed on Aug 19, 2026
  11. DrahtBot removed the label CI failed on Aug 19, 2026
  12. brunoerg marked this as ready for review on Aug 19, 2026
  13. brunoerg commented at 11:33 AM on August 19, 2026: contributor

    Ready for review.

  14. jeanpablojp commented at 3:03 PM on August 21, 2026: contributor

    tACK d48b8afcaee6d08cc5e0c00262d0b7defdc6009f

    Reverted txorphanage back to master and the new functional test fails like it should, "no orphan was evicted". With 10 peers sending one memory-heavy orphan each, master keeps all 10 and the head none.

    I also checked the two numbers in the MAX_ORPHAN_TX_USAGE comment and got 1.24x and 1.47x, matching.

    Left a comment on send_orphan() in the new test, nothing blocking.

  15. in test/functional/p2p_orphan_memory_accounting.py:85 in d48b8afcae
      80 | +    def send_orphan(self, peer, tx):
      81 | +        """Announce tx by wtxid, then serve it when it is requested."""
      82 | +        node = self.nodes[0]
      83 | +        peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(tx.wtxid_hex, 16))]))
      84 | +        node.bumpmocktime(TXREQUEST_TIME_SKIP)
      85 | +        peer.wait_until(lambda: peer.last_message.get("getdata"))
    


    jeanpablojp commented at 3:03 PM on August 21, 2026:

    This wait_until matches the getdata from the previous call, since last_message isn't cleared. From the second send_orphan() on, the getdata doesn't hold the current tx's wtxid, so the tx goes out unrequested and the inv/getdata path is only exercised on the first call.

    wait_for_getdata([int(tx.wtxid_hex, 16)]) fixes it, which is what p2p_orphan_handling.py does. Swapped it here and it still passes.

  16. DrahtBot requested review from l0rinc on Aug 21, 2026
  17. instagibbs commented at 9:28 AM on August 24, 2026: member

    I'm pretty worried that the new behavior would cause subtle user breakage, epsecially considering something BitVM-like (with lots of 20b and 1b elements in witness data i.e. winternitz sigs).

    Rather than change the metrics entirely, could you consider an alternative where the stored transactions are in serialized form, and deserialized only when required? https://github.com/instagibbs/bitcoin/tree/2026-08-orphanage-serialized-weight

    It's significantly less code, doesn't require the weight metric to be swapped out (it would truly be an overestimation now modulo small constant), limiting the scope of the change. The one cost is the serialize<->deserialize that would be added on round-tripping. From my benchmarks it looks like a non-adversarial large tx would take ~1.5ms on ReconsiderTx, and a BitVM-like one ~3ms. The other costs look negligible.

    Let me know what you think. I'm happy for you to take the code, or to open my own PR.

  18. jeanpablojp commented at 9:05 PM on August 24, 2026: contributor

    @instagibbs I was curious so I built your branch, and I'm sharing some numbers I got in my tests.

    The timing below is for rebuilding the transaction, one input with N items of k bytes.

    witness stack         weight    deserialize   with teardown
    100 x 80B              8,343        0.07 ms         0.08 ms
    1 x 380,000B         380,248         2.4 ms          2.4 ms
    8,000 x 20B          168,245         2.2 ms          2.8 ms
    199,000 x 1B         398,247          28 ms           42 ms
    

    Your ~3ms checks out. It's the item count driving this and not the bytes, and it's paid once, both of the paths a peer can trigger consume the orphan.

    The limit here is 600,000 bytes of memory, not weight. Varying the item size, this is the weight at which it's reached.

    item size    stored up to    % of the range to 400,000 WU lost
      1 byte       21,661 WU                  94.6%
     20 bytes     175,133 WU                  56.2%
     64 bytes     375,035 WU                   6.2%
     65 bytes     330,047 WU                  17.5%
    

    64 to 65 is a MallocUsage step, so the threshold tracks the allocator's buckets and not the transaction.

    I tested on x86-64, best case of several runs.

  19. instagibbs commented at 11:01 AM on August 25, 2026: member

    Note that it's a pretty expensive "attack" in that to cause deserialization in the orphanage you'd have to enter in a valid parent tx into the mempool.

  20. brunoerg commented at 12:11 PM on August 25, 2026: contributor

    I'm pretty worried that the new behavior would cause subtle user breakage, epsecially considering something BitVM-like (with lots of 20b and 1b elements in witness data i.e. winternitz sigs).

    Rather than change the metrics entirely, could you consider an alternative where the stored transactions are in serialized form, and deserialized only when required? https://github.com/instagibbs/bitcoin/tree/2026-08-orphanage-serialized-weight

    It's significantly less code, doesn't require the weight metric to be swapped out (it would truly be an overestimation now modulo small constant), limiting the scope of the change. The one cost is the serialize<->deserialize that would be added on round-tripping. From my benchmarks it looks like a non-adversarial large tx would take ~1.5ms on ReconsiderTx, and a BitVM-like one ~3ms. The other costs look negligible.

    Let me know what you think. I'm happy for you to take the code, or to open my own PR.

    Good point. I haven't tried that approach, but it seems simpler and achieves the same. I'll take a look at your branch, but you can open the PR and move on, no problem. Happy to review.

  21. instagibbs commented at 12:12 PM on August 25, 2026: member

    @brunoerg I'm a little busy, please take it on

  22. brunoerg force-pushed on Aug 25, 2026
  23. brunoerg commented at 5:24 PM on August 25, 2026: contributor

    Force-pushed addressing @instagibbs' approach.

  24. instagibbs commented at 7:45 PM on August 25, 2026: member

    title and OP will need updating :+1:

  25. brunoerg renamed this:
    txorphanage: account memory usage instead of weight
    txorphanage: bound orphan memory by storing transactions serialized
    on Aug 25, 2026
  26. brunoerg commented at 8:15 PM on August 25, 2026: contributor

    title and OP will need updating 👍

    Done.

  27. jeanpablojp commented at 11:00 AM on August 26, 2026: contributor

    Built the merge with master and tested again.

  28. in src/node/txorphanage.cpp:763 in c377e4af30
     757 | @@ -666,9 +758,9 @@ std::vector<CTransactionRef> TxOrphanageImpl::GetChildrenFromSamePeer(const CTra
     758 |          --it_upper;
     759 |          if (!Assume(it_upper->m_announcer == peer)) break;
     760 |          // Check if this tx spends from parent.
     761 | -        for (const auto& input : it_upper->m_tx->vin) {
     762 | -            if (input.prevout.hash == parent_txid) {
     763 | -                children_found.emplace_back(it_upper->m_tx);
     764 | +        for (const auto& prevout : it_upper->m_tx_data->m_prevouts) {
     765 | +            if (prevout.hash == parent_txid) {
     766 | +                children_found.emplace_back(it_upper->m_tx_data->MakeTxRef());
    


    jeanpablojp commented at 11:00 AM on August 26, 2026:

    Every matching child is rebuilt before Find1P1CPackage looks at the first one. With 94 orphans of 199,000 1-byte items, one per peer, all announced by a single peer, that's 1,047.6 MB live against the 37.4 MB accounted for it, where one at a time would be 11.1 MB.

    The trigger costs no fee, min relay fee not met already returns TX_RECONSIDERABLE.

  29. in src/test/orphanage_tests.cpp:749 in c377e4af30 outdated
     744 | +    BOOST_CHECK_LE(orphanage->UsageByPeer(0), orphanage->ReservedPeerUsage());
     745 | +
     746 | +    // The transaction handed back out deserializes to the original.
     747 | +    const auto ptx_out{orphanage->GetTx(ptx->GetWitnessHash())};
     748 | +    BOOST_REQUIRE(ptx_out != nullptr);
     749 | +    BOOST_CHECK(ptx_out->GetWitnessHash() == ptx->GetWitnessHash());
    


    jeanpablojp commented at 11:00 AM on August 26, 2026:

    This test passes without the change. Reverting just src/node/txorphanage.cpp to before 493561a34285, the whole suite still passes, since the only difference the API exposes is the identity of the returned object.

        BOOST_CHECK(ptx_out != ptx);
        BOOST_CHECK(ptx_out->GetWitnessHash() == ptx->GetWitnessHash());
    

    Fails without the change and passes with it.


    brunoerg commented at 5:46 PM on September 9, 2026:

    Good point, I could add a BOOST_CHECK(ptx_out != ptx) right after it.

  30. Crypt-iQ commented at 7:24 AM on August 29, 2026: contributor

    Concept ACK

  31. l0rinc referenced this in commit 7045787c0c on Sep 3, 2026
  32. l0rinc referenced this in commit 9d159b717a on Sep 3, 2026
  33. instagibbs commented at 9:43 PM on September 8, 2026: member

    When finding potential 1P1C packages, we're now deserializing potentially many orphans, which on my machine can cause ~1s of cpu time at ~1GB of memory usage on top of normal. astra slop for your consideration:

    https://github.com/instagibbs/bitcoin/commit/847d2dc2a0dc76340e269f3f60977b8a5d6bab2a

  34. brunoerg commented at 2:19 PM on September 9, 2026: contributor

    When finding potential 1P1C packages, we're now deserializing potentially many orphans, which on my machine can cause ~1s of cpu time at ~1GB of memory usage on top of normal. astra slop for your consideration:

    instagibbs@847d2dc

    Interesting, I benchmarked it (with and without your suggestion) and got +700 MB of peak (I think around 40MB of stored orphans) without and basically zero with. Will review the code and address it here.

  35. brunoerg force-pushed on Sep 9, 2026
  36. brunoerg force-pushed on Sep 9, 2026
  37. DrahtBot added the label CI failed on Sep 9, 2026
  38. brunoerg commented at 6:48 PM on September 9, 2026: contributor

    Force-pushed:

    • Addressed suggestion from @instagibbs - fixing Find1P1CPackage. I got instagibbs@847d2dc but changed some things; most nits that I found and test improvements. I also changed Find1P1CPackage to store the GetTx result and skips the candidate under Assume if it is null.

    • Addressed #36015 (review)

  39. brunoerg force-pushed on Sep 9, 2026
  40. brunoerg commented at 7:19 PM on September 9, 2026: contributor

    Missing rebase, doing it now.

  41. brunoerg force-pushed on Sep 9, 2026
  42. brunoerg force-pushed on Sep 9, 2026
  43. brunoerg commented at 3:17 AM on September 10, 2026: contributor

    CI failure is unrelated

  44. iwyu: fix includes in node/txorphanage.h
    Drop includes the header does not use, add the ones it does, and forward
    declare FastRandomContext. The IWYU job checks the files a change touches,
    and the following commits modify this header.
    9d6e93f5c7
  45. scripted-diff: rename DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER
    This constant is the default for TxOrphanage::m_reserved_usage_per_peer,
    returned by ReservedPeerUsage() and compared against UsageByPeer(), all
    of which are named after the "usage" they bound (of type
    TxOrphanage::Usage). Name the constant after that same quantity rather
    than after weight, the metric that happens to measure it, so that it
    matches the field it initializes and the rest of the usage-based API.
    
    The orphanage keeps accounting usage by weight, so this is a pure
    naming change with no change in behavior.
    
    -BEGIN VERIFY SCRIPT-
    sed -i 's/DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER/DEFAULT_RESERVED_ORPHAN_USAGE_PER_PEER/g' $(git grep -l DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER)
    -END VERIFY SCRIPT-
    f9643f9347
  46. txorphanage: add GetOrphanUsage() and cache it per announcement
    The orphanage's notion of an orphan's "usage" is currently duplicated in
    the unit tests, the benchmarks and the fuzz targets, all of which call
    GetTransactionWeight() to predict what the orphanage will account. Move
    that knowledge into a single function so that a change of metric only
    has to happen in one place, and so that all users agree on it.
    
    Also cache the value in the Announcement instead of recomputing it on
    every operation. Announcements hold an immutable CTransactionRef, so the
    value never changes; caching it guarantees that PeerDoSInfo::Add() and
    ::Subtract() always use the same number, and makes Erase() (and therefore
    LimitOrphans()) independent of the transaction's size.
    
    No behavior change.
    f4d43b530a
  47. txorphanage: add GetParentTxids() and use it for orphan resolution candidates
    AddTxAnnouncement() only needs the deduplicated prevout txids of an
    announced orphan to consider a peer as an orphan resolution candidate,
    but obtains them by retrieving the full transaction with GetTx(). Serve
    them from the orphanage directly, so that handling an announcement of a
    known orphan does not depend on how the orphanage stores the
    transaction. The following commit stores orphans in serialized form,
    which would otherwise make every such announcement deserialize the
    orphan.
    87a2791f54
  48. txorphanage: return orphan ids from GetChildrenFromSamePeer
    Find1P1CPackage only needs each candidate child's txid and wtxid to
    check the reject filters, and materializes at most one of them. Have
    GetChildrenFromSamePeer() return those ids instead of a CTransactionRef
    per child, add GetPackageHashFromWtxids() so the package-reject check
    can be done from ids, and look up the selected child with GetTx().
    
    This prepares for storing orphans serialized: with this change, finding
    a 1P1C package deserializes only the child that is actually selected.
    Otherwise a peer holding ~40MB of witness-heavy orphans (many tiny
    witness elements, each a separate heap allocation once deserialized)
    could make a single call cost ~0.9s of CPU and ~700MB of memory.
    
    Co-authored-by: Greg Sanders <gsanders87@gmail.com>
    0557bc81d5
  49. txorphanage: store orphans serialized, making weight bound their memory
    The orphanage limits the "usage" of the orphans it stores, per peer and
    globally, to bound the amount of memory an attacker can make us hold on
    to, using weight as a proxy for that memory on the assumption that it
    is "often higher than the actual memory usage of the transaction".
    
    That assumption does not hold for a deserialized transaction: every
    witness stack element is an individually heap-allocated vector, costing
    its 24-byte slot in the stack vector plus a 32-byte minimum allocation,
    while only weighing 2WU. A transaction of 199,000 1-byte witness
    elements weighs 398,247WU (i.e. it is of standard weight, and witness
    standardness cannot be checked while the inputs are missing), but uses
    11.1MB of memory: 28 times what is accounted for it, and one such
    orphan can be retained per peer.
    
    Keep orphans in serialized form instead, deserializing them again on
    the paths that hand them back out, all of which feed into full
    (re)validation or RPC whose cost dwarfs a deserialization. Serialized,
    a transaction's memory usage is bounded by its weight, so the existing
    weight-based accounting becomes a true upper bound on memory and the
    worst case is the peers' combined allowances. Admission, eviction and
    accounting behavior are unchanged: no transaction that was previously
    accepted is refused, so orphan resolution (and thus 1p1c package relay)
    keeps working for standard-weight transactions whose witnesses consist
    of many small elements, such as BitVM-style transactions.
    
    The remaining per-orphan overhead not covered by weight (the entry in
    m_orphans, the entries in m_outpoint_to_orphan_wtxids, and the cached
    prevouts and hashes) is bounded by the latency score limits.
    933a9f95af
  50. test: cover storing a witness-heavy orphan at its serialized cost
    A standard-weight transaction of 199,000 1-byte witness elements, which
    would use over 10 times its weight in memory if stored deserialized, is
    stored (accounted its weight, which now bounds its memory), fits within
    the announcer's reservation alongside normal orphans, and round-trips
    through the orphanage intact.
    e9381aac15
  51. brunoerg force-pushed on Sep 10, 2026
  52. DrahtBot removed the label CI failed on Sep 10, 2026
  53. nervana21 commented at 1:59 PM on September 23, 2026: contributor

    Concept ACK

  54. in src/bench/txorphanage.cpp:72 in e9381aac15
      68 | @@ -70,33 +69,40 @@ static void OrphanageSinglePeerEviction(benchmark::Bench& bench)
      69 |      for (unsigned int i{0}; i < NUM_TINY_TRANSACTIONS; ++i) {
      70 |          tiny_txs.emplace_back(MakeTransactionBulkedTo(1, TINY_TX_WEIGHT, det_rand));
      71 |      }
      72 | +    // All of the tiny transactions are accounted the same usage.
    


    nervana21 commented at 2:39 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    nit.

        // All of the tiny transactions are attributed the same usage.
    
  55. in src/bench/txorphanage.cpp:142 in e9381aac15
     138 | @@ -132,11 +139,14 @@ static void OrphanageMultiPeerEviction(benchmark::Bench& bench)
     139 |      for (unsigned int i{0}; i < NUM_UNIQUE_TXNS; ++i) {
     140 |          shared_txs.emplace_back(MakeTransactionBulkedTo(9, LARGE_TX_WEIGHT, det_rand));
     141 |      }
     142 | +    // All of the shared transactions are accounted the same usage.
    


    nervana21 commented at 2:40 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    nit.

        // All of the shared transactions are attributed the same usage.
    
  56. in src/node/txorphanage.h:122 in e9381aac15


    nervana21 commented at 2:48 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    nit.

         * announcers, its usage will be accounted for in each PeerOrphanInfo, so the total of all
    

    nervana21 commented at 2:51 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    Can add the usage metric

                const auto tx_weight{GetTransactionWeight(*tx)};
                const auto tx_usage{node::GetOrphanUsage(tx)};
    

    nervana21 commented at 3:30 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    Can use usage instead of weight.

                                Assert(orphanage->UsageByPeer(peer_id) <= bytes_from_peer_before - tx_usage);
    
  57. in src/node/txorphanage.h:164 in e9381aac15
     159 | @@ -147,5 +160,10 @@ class TxOrphanage {
     160 |  /** Create a new TxOrphanage instance */
     161 |  std::unique_ptr<TxOrphanage> MakeTxOrphanage() noexcept;
     162 |  std::unique_ptr<TxOrphanage> MakeTxOrphanage(TxOrphanage::Count max_global_latency_score, TxOrphanage::Usage reserved_peer_usage) noexcept;
     163 | +
     164 | +/** Get the amount TxOrphanage accounts for this transaction, i.e. its contribution to
    


    nervana21 commented at 2:48 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    nit. Match the usage API this comment documents.

    /** Get the usage TxOrphanage attributes to this transaction, i.e. its contribution to
    
  58. in src/policy/packages.h:92 in e9381aac15
      88 | @@ -89,4 +89,7 @@ bool IsChildWithParentsTree(const Package& package);
      89 |   */
      90 |  uint256 GetPackageHash(const std::vector<CTransactionRef>& transactions);
      91 |  
      92 | +/** Same as GetPackageHash(), but computed from the transactions' wtxids without needing the transactions. */
    


    nervana21 commented at 2:50 AM on September 24, 2026:

    0557bc81d577ab3ed4354e79dfa6bf5b5a2f0c6c: txorphanage: return orphan ids from GetChildrenFromSamePeer

    nit. Mirror GetPackageHash above

    /** Get the hash of the concatenated wtxids, with wtxids
     * treated as little-endian numbers and sorted in ascending numeric order.
     * Same as GetPackageHash(), but takes wtxids directly without needing the transactions. */
    
  59. in src/test/orphanage_tests.cpp:717 in e9381aac15
     712 | +    FastRandomContext det_rand{true};
     713 | +
     714 | +    // A transaction whose witness is a stack of many 1-byte elements is of standard weight (and its full
     715 | +    // witness standardness cannot be checked, since its inputs are missing). Deserialized, it would use ~28 times more
     716 | +    // memory than its weight suggests, as every element is an individually heap-allocated vector. Since orphans
     717 | +    // are stored in serialized form, its memory usage is bounded by the weight that is accounted for it.
    


    nervana21 commented at 3:04 AM on September 24, 2026:

    e9381aac1570875f395c1f8f9169179726534e06: test: cover storing a witness-heavy orphan at its serialized cost

    nit.

        // are stored in serialized form, its memory usage is bounded by the usage attributed to it.
    
  60. in src/test/orphanage_tests.cpp:726 in e9381aac15
     721 | +    mtx.vout.resize(1);
     722 | +    mtx.vin[0].scriptWitness.stack.assign(199'000, std::vector<unsigned char>{1});
     723 | +    const auto ptx{MakeTransactionRef(mtx)};
     724 | +    const auto weight{GetTransactionWeight(*ptx)};
     725 | +    BOOST_CHECK_LE(weight, MAX_STANDARD_TX_WEIGHT);
     726 | +    // The serialized transaction, which is what the orphanage keeps, is smaller than the accounted weight.
    


    nervana21 commented at 3:04 AM on September 24, 2026:

    e9381aac1570875f395c1f8f9169179726534e06: test: cover storing a witness-heavy orphan at its serialized cost

    nit.

        // The serialized transaction, which is what the orphanage keeps, is smaller than the usage attributed to it.
    
  61. in src/test/orphanage_tests.cpp:731 in e9381aac15
     726 | +    // The serialized transaction, which is what the orphanage keeps, is smaller than the accounted weight.
     727 | +    BOOST_CHECK_LT(static_cast<int64_t>(GetSerializeSize(TX_WITH_WITNESS(*ptx))), weight);
     728 | +
     729 | +    auto orphanage{node::MakeTxOrphanage()};
     730 | +
     731 | +    // It is stored and accounted its weight.
    


    nervana21 commented at 3:05 AM on September 24, 2026:

    e9381aac1570875f395c1f8f9169179726534e06: test: cover storing a witness-heavy orphan at its serialized cost

    nit.

        // It is stored and its usage is accounted for.
    
  62. in src/node/txorphanage.cpp:47 in e9381aac15
      42 | +    const Wtxid m_wtxid;
      43 | +    const Txid m_txid;
      44 | +    /** The transaction's prevouts, cached so that maintaining m_outpoint_to_orphan_wtxids, EraseForBlock and latency
      45 | +     * scores do not require deserializing. */
      46 | +    const std::vector<COutPoint> m_prevouts;
      47 | +    /** Memory accounted for this orphan (see GetOrphanUsage), cached at construction. Caching guarantees that the
    


    nervana21 commented at 3:16 AM on September 24, 2026:

    933a9f95af2ec52e0804dd8f66d229e49e53c426: txorphanage: store orphans serialized, making weight bound their memory

    nit.

        /** Usage attributed to this orphan (see GetOrphanUsage), cached at construction. Caching guarantees that the
    
  63. in src/test/orphanage_tests.cpp:113 in e9381aac15
     111 |          auto ptx = MakeTransactionSpending({}, det_rand);
     112 |          txns.emplace_back(ptx);
     113 | -        BOOST_CHECK_EQUAL(TX_SIZE, GetTransactionWeight(*ptx));
     114 | +        BOOST_CHECK_EQUAL(TX_WEIGHT, GetTransactionWeight(*ptx));
     115 |      }
     116 | +    // The orphanage accounts the same usage for each of them. Don't hardcode the value, as it is
    


    nervana21 commented at 3:17 AM on September 24, 2026:

    f4d43b530a142fea0db9488028c5e510bf78d1ee: txorphanage: add GetOrphanUsage() and cache it per announcement

    nit.

        // The orphanage attributes the same usage to each orphan transaction. Don't hardcode the value, as it is
    
  64. nervana21 commented at 5:45 PM on September 24, 2026: contributor

    Nits for the commit message themselves

    87a2791f5438672cba63de2c10b5da1d82425d51: txorphanage: add GetParentTxids() and use it for orphan resolution candidates Says "The following commit" Should read "In a later commit"

    9d6e93f5c7dc5afd08e872d9c7e39623c487a9c8: iwyu: fix includes in node/txorphanage.h Says "the following commits modify this header" Should read "later commits modify this header"

    e9381aac1570875f395c1f8f9169179726534e06: test: cover storing a witness-heavy orphan at its serialized cost Says "stored (accounted its weight, which now bounds its memory)" Should read "stored (its usage is accounted for, which now bounds its memory)"

  65. nervana21 commented at 5:47 PM on September 24, 2026: contributor

    tACK e9381aac1570875f395c1f8f9169179726534e06

    left a slew of non blocking nits, mostly about wording the logic is sound based on my review and testing

  66. DrahtBot requested review from jeanpablojp on Sep 24, 2026
  67. DrahtBot requested review from Crypt-iQ on Sep 24, 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-09-30 16:51 UTC

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