#36437 orphanage: Fix orphan reconsideration disconnect hole, add 1p1c invariant harness
| Reviewability | Reviews | Agreement | Size | |||||||
|---|---|---|---|---|---|---|---|---|---|---|
| #36437 orphanage: Fix orphan reconsideration disconnect hole, add 1p1c invariant harness | Ready | 1 | Strong | 21 + 619 tests | ||||||
|
|
|
| |||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | ||||||||||
Goal
Fixes a transaction relay liveness bug in TxOrphanage by reassigning orphan reconsideration to another announcer when the currently assigned announcer's entry is erased due to disconnect or eviction. It also adds an end-to-end P2P fuzz target (`p2p_1p1c_liveness`) that drives PeerManager with real messages and validation to verify 1-parent-1-child relay under adversarial conditions.
Problem: When a missing parent transaction is accepted, its orphan children are assigned to a single announcer's work set for reconsideration. If that announcer disconnects or gets evicted before the node retries the orphan, the transaction remains stranded in the orphanage without being processed; because the node already holds the orphan, re-announcements from other peers do not re-fetch it, allowing peers to stall CPFP child relay until the orphan is evicted.
Evidence of importance:
- Commit 65b48ba1f1: 'If that announcer disconnects or is evicted first, the orphan is never retried, and re-announcements by the other announcers don't fetch it again since it is already held. This keeps e.g. a CPFP child of a parent that pays for itself out of the mempool until the orphan is evicted, and an announcer can cause it on purpose.'
- PR description: 'With that flag turned off, the target finds this bug on master from an empty corpus in under an hour on one core; an earlier version found it first (https://github.com/bitcoin/bitcoin/pull/36371#issuecomment-5937814761).'
Scope: The PR contains a small 20-line logic fix in src/node/txorphanage.cpp along with a large (~570-line) new fuzz harness in src/test/fuzz/p2p_1p1c_liveness.cpp.
Risk of a mistake: medium (p2p). A flaw in orphan reconsideration or peer work set assignment could stall orphan transaction processing or corrupt TxOrphanage multi-index state.
Changed since the previous assessment: Rebased and marked as the base for follow-up PR #36506.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- nothing: no pending items
Agreement
Strong
substantive support, no open objection (fametrano)
Thread notes: Added Tested ACK from fametrano with extensive reproduction testing and code analysis.
Objections, suggestions, and questions: none recorded.
Support:
- w0xlt (contributor) (Concept ACK, 2026-10-08): no reason given [not substantive]
- fametrano (contributor) (Tested ACK, 2026-10-25) [both: EraseForPeer, EraseTxInternal, LimitOrphans, Erase, peer_worksets, p2p_1p1c_liveness, txorphanage_sim]: Reproduced the orphan disconnect hole end-to-end and confirmed this PR fixes it, verified iterator validity during announcement handover, and ran unit and fuzz test suites
Participants:
- w0xlt (contributor, support, 1 statements 2026-10-08 to 2026-10-08): Concept ACK without detailed rationale
- fametrano (contributor, support, 1 statements 2026-10-25 to 2026-10-25): Tested ACK with reproduction test case and iterator safety review
Dependencies
Files
623 lines under test/bench/ci.
- src/test/fuzz/p2p_1p1c_liveness.cpp +568/-0
- src/test/orphanage_tests.cpp +28/-2
- src/node/txorphanage.cpp +19/-5
- src/test/fuzz/txorphan.cpp +22/-2
- src/node/txorphanage.h +2/-1
- src/test/fuzz/CMakeLists.txt +1/-0
Card
The short text the ranking pass compares across PRs.
This pull request fixes a transaction relay liveness hole in TxOrphanage where an orphan queued for reconsideration is abandoned if its assigned announcer disconnects or is evicted before retry. Because the transaction remains held in the orphanage, subsequent announcements from honest peers are ignored, allowing peers to deliberately delay transactions such as CPFP children until eviction. TxOrphanage now reassigns reconsideration to a remaining announcer on erasure. The PR also introduces an end-to-end fuzz harness that reproduces this vulnerability on master in under an hour and validates parent-child relay under adversarial conditions, unblocking follow-up hole fixes in #36506.
Data
thread read through 2026-10-25 (5 statements), head 65b48ba1f1
labels: none
Categories
P2P
P1 · bug fix
Fixes an exploitable relay liveness hole where orphans stuck awaiting reconsideration are abandoned if an announcer disconnects. As commit e974d21b4e notes, this allows an announcer to deliberately keep CPFP child transactions out of the mempool until the orphan expires.
Membership: Touches TxOrphanage in net_processing to prevent dropped orphan reconsiderations when peers disconnect, and adds a P2P liveness fuzz harness.
Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 1
Test infrastructure
P1 · test coverage (assessed alone as P2; Raised to P1: the harness found a real relay liveness bug from an empty corpus and unblocks follow-up hole fixes in #36506 — end-to-end p2p+validation fuzzing is the highest-value coverage gap here.)
The PR introduces an end-to-end P2P fuzz harness (p2p_1p1c_liveness) that drives PeerManager with real messages and validation to verify 1p1c relay invariants under adversarial conditions. The description highlights that the target covers high-risk P2P message handling paths and catches realistic regressions, noting that it 'finds this bug on master from an empty corpus in under an hour on one core' while also enabling #36506.
Membership: Adds a new 568-line end-to-end fuzzing harness (src/test/fuzz/p2p_1p1c_liveness.cpp) and updates fuzz build configuration in src/test/fuzz/CMakeLists.txt.
Factors: security/stability 1, bug 2, performance 0, user value 1, leverage 1