#36015 txorphanage: bound orphan memory by storing transactions serialized
https://github.com/bitcoin/bitcoin/pull/36015 · · +434/-146 in 10 files, 7 commits · labels: none
Goal
- Prevent peers from consuming excessive node memory by sending crafted orphan transactions
- Ensure orphan memory limits cannot be bypassed by transactions with massive witness stacks
Bounds memory usage in the transaction orphanage by storing orphan transactions in serialized form instead of deserialized CTransactionRef objects. It deserializes transactions only when they are evaluated or handed out, and updates 1P1C candidate package selection to operate on txids and wtxids before materializing the selected child. This ensures that transaction weight functions as a true upper bound on memory usage without rejecting complex transactions with large witness element counts.
Problem: The orphanage relies on transaction weight as a proxy for memory usage when bounding allocations per peer and globally. Because deserialized witness stack elements incur individual heap allocation overheads, a standard-weight transaction containing many 1-byte witness elements can consume over 11MB of memory (28 times its accounted weight), allowing peers to bypass intended orphanage memory ceilings.
Category: P2P (#1 of 65)
P2 · DoS protection
- P1 because it closes a substantial memory DoS amplification vector in P2P transaction relay
- Peers could allocate 28 times more heap memory than accounted for, risking node crashes
- Protects resource-constrained nodes from remote memory exhaustion during orphan handling
Fixes a substantial resource accounting flaw in peer-to-peer transaction relay. Adversaries could abuse standard transactions containing many witness items to inflate orphan memory 28x past peer quotas (up to ~1GB across default peer connections). The solution prevents DoS attacks while preserving relay functionality for standard witness-heavy protocols.
Membership: Modifies TxOrphanage and TxDownloadManagerImpl to control memory consumption of unconfirmed transactions held during peer-to-peer relay.
Factors: security/stability 2, bug 2, performance 1, user value 0, leverage 1
Reviewability: Ready
- Ready for review as the branch merges cleanly, CI passes, and earlier performance issues are resolved
The PR has passing CI, clean merge status, and incorporates all requested design adjustments and benchmarks.
Author status: active, addressed all review feedback and suggestions in recent force pushes
Resolved concerns:
- Initial metric-replacement design could reject standard-weight transactions with many small witness items like BitVM transactions, resolved by adopting serialized storage instead.
- Initial serialized implementation caused a ~1s CPU delay and ~1GB memory spike during 1P1C package search by eagerly deserializing candidate children, resolved by filtering by txid/wtxid first.
- Test asserting object reference difference between input and output transaction in unit tests was added.
Agreement: Strong
- Strong consensus on storing serialized orphans rather than altering weight metrics
- Proposed serialized storage design and contributed package selection optimizations (instagibbs)
- Verified memory and CPU impacts through benchmarking across updates (jeanpablojp)
- Concept approval without stated reasons (w0xlt, l0rinc, Crypt-iQ)
Strong consensus on serialized approach; earlier concerns regarding BitVM support and 1P1C evaluation overhead have been resolved.
The author worked closely with reviewers (especially instagibbs and jeanpablojp) to overhaul the design, address relay compatibility, and resolve a temporary memory spike in 1P1C package selection. No objections remain open.
- instagibbs suggested the serialized orphan approach on 2026-08-24, which author adopted on 2026-08-25.
- jeanpablojp confirmed memory calculations and highlighted 1P1C deserialization cost on 2026-08-26.
- instagibbs provided a patch to only deserialize selected 1P1C candidates on 2026-09-08, which the author incorporated on 2026-09-09.
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| instagibbs | approach | Changing the admission metric would break relay for standard witness-heavy transactions like BitVM | resolved | yes | yes | 2026-08-24: 'I\'m pretty worried that the new behavior would cause subtle user breakage, epsecially considering something BitVM-like' Settled: 2026-08-25: 'Force-pushed addressing @instagibbs\' approach.' |
| jeanpablojp | safety | Find1P1CPackage materializing all child orphans creates a ~1GB memory spike | resolved | no | yes | 2026-08-26: 'Every matching child is rebuilt before Find1P1CPackage looks at the first one... that\'s 1,047.6 MB live against the 37.4 MB accounted for it' Settled: 2026-09-09: 'Addressed suggestion from @instagibbs - fixing Find1P1CPackage.' |
| instagibbs | safety | Find1P1CPackage deserializing many orphans causes ~1s CPU delay and ~1GB memory usage | resolved | yes | yes | 2026-09-08: 'we\'re now deserializing potentially many orphans, which on my machine can cause ~1s of cpu time at ~1GB of memory usage' Settled: 2026-09-09: 'Addressed suggestion from @instagibbs - fixing Find1P1CPackage.' |
| l0rinc | scope | Mixing refactors and hardening increases review friction and conflicts with concurrent PRs | resolved | no | yes | 2026-08-18: '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?' Settled: 2026-08-18: 'forgot to push since my latest local change. Just did it.' |
Support:
- instagibbs: Proposed serialized storage solution to enforce upper bound without discarding valid transactions, co-authoring child lookup optimization
- jeanpablojp: Tested and verified that memory-heavy orphans trigger eviction on head and reproduced memory metrics
- w0xlt: Concept ACK [not substantive]
- Crypt-iQ: Concept ACK [not substantive]
- l0rinc: Concept ACK [not substantive]
Participants: w0xlt (support), l0rinc (objection), jeanpablojp (objection), instagibbs (objection), Crypt-iQ (support)
State derived from the lists: substantive support, no open objection (instagibbs, jeanpablojp)
Review verdicts (DrahtBot): 0 (+1)
- Stale ACK: jeanpablojp
- Concept ACK: w0xlt, l0rinc, Crypt-iQ
Files
316 lines under test/bench/ci.
- src/node/txorphanage.cpp +146/-54
- src/test/orphanage_tests.cpp +118/-37
- src/test/txdownload_tests.cpp +62/-0
- src/bench/txorphanage.cpp +33/-22
- src/test/fuzz/txorphan.cpp +21/-16
- src/node/txorphanage.h +26/-8
- src/node/txdownloadman_impl.cpp +10/-7
- src/policy/packages.cpp +8/-2
- src/test/txpackage_tests.cpp +7/-0
- src/policy/packages.h +3/-0
Card
PR 36015 modifies TxOrphanage to store orphan transactions in serialized format so that weight acts as an exact upper bound on memory usage. Under the previous model, deserialized transactions with many small witness elements could consume 28 times their accounted weight in heap memory, exposing nodes to memory exhaustion DoS attacks from peers sending standard-weight BitVM-like transactions. The implementation avoids deserializing candidate children during 1P1C package lookup unless selected, keeping memory and CPU costs minimal. Reviewers strongly support the approach following collaborative fixes for potential performance overheads, and the PR is ready for final review.