#36506 p2p: plug "tier 1" 1p1c liveness holes
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36506 p2p: plug "tier 1" 1p1c liveness holes | Review #36437 first | 1 | Strong | 110 + 993 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
Plugs several liveness holes in 1-parent-1-child (1p1c) package relay and orphan handling, and introduces an end-to-end fuzz harness (`p2p_1p1c_liveness`) that tests bounded CPFP relay propagation under adversarial conditions. It hands over orphan reconsideration to another peer when the assigned peer disconnects, prevents `TX_CONFLICT` and `DIFFERENT_WITNESS` results from entering the reject filter, retains orphans after package feerate failures, and enables type-specific forgetting in `TxRequestTracker` to prevent witnessless parent rejections from cancelling txid orphan parent requests.
Problem: Peers can exploit orphan management and reject filter edge cases to stall or block honest CPFP fee-bump packages from entering the mempool. An announcer assigned to reconsider an orphan can disconnect without the work being reassigned, reject filters can be poisoned by witnessless copies (where txid equals wtxid) of known transactions, temporary package feerate failures with malleated parents erase the child orphan for all announcers, and rejecting a witnessless parent accidentally cancels pending txid-based parent requests.
Evidence of importance:
- PR description: 'Resolves two fuzz harness "holes" and narrows a third. This is a proposed stopping point on resolving the holes, as it's unclear if further work is worth the churn vs a sender-initiated package relay protocol update that would resolve this directly.'
- 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.'
- Commit feff499de9: 'When a 1p1c package fails the package feerate check, the child gets a TX_RECONSIDERABLE result, and ProcessPackageResult passed it to ProcessInvalidTx, which erased the child from the orphanage for all of its announcers.'
Scope: The PR adds a major new fuzz target (`p2p_1p1c_liveness.cpp`) of over 500 lines that drives a real `PeerManager` end-to-end to verify transaction liveness invariants.
Risk of a mistake: medium (p2p, mempool_policy). Mistakes in orphan lifecycle management, reject filtering, or TxRequestTracker could cause memory leaks in the orphanage or prevent honest transactions from being downloaded and accepted.
Changed since the previous assessment: Dropped earlier changes that introduced a dedicated txid cache for nonstandard-input rejections and re-requested missing parents on failed orphan retry; focused the patch set specifically on the peer handover, reject filter handling of conflicts, orphan retention on package feerate failure, and typed announcements forgetting in TxRequestTracker.
Reviewability
Ready · Review #36437 first
Stacked on #36437; the base PR's changes are included in this diff.
Waiting on (from the thread read):
- nothing: all items addressed and PR has substantive review
Agreement
Strong
substantive support, no open objection (fametrano, fametrano, fametrano)
Thread notes: fametrano reACKed after the DIFFERENT_WITNESS gap was addressed in 8d44ac8909 with a test.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fametrano (contributor) | suggestion, scope | Address in a follow-up the remaining gap where DIFFERENT_WITNESS still goes through ProcessInvalidTx and puts the parent txid in the reject filter | accepted · fix pushed · fixed in push 8d44ac8909 and confirmed by fametrano | answered | yes | 2026-10-14: 'One gap remains for a follow-up: in the 1p1c package path, a `DIFFERENT_WITNESS` result still goes through `ProcessInvalidTx`' Status from: 2026-10-15: fametrano: 'The new `DIFFERENT_WITNESS` case closes the 1p1c gap I noted at c8abd9ee41' |
Support:
- fametrano (contributor) (Tested ACK, 2026-10-14) [tested]: each of the four fixes is caught by a test this PR adds or extends
- fametrano (contributor) (ACK, 2026-10-14) [read]: reviewed the four fixes commit by commit and found nothing blocking
- fametrano (contributor) (ACK, 2026-10-15) [both: txdownload_tests, orphanage_tests, txpackage_tests]: the new DIFFERENT_WITNESS case resolves the remaining 1p1c gap and passes unit tests
Participants:
- fametrano (contributor, support, 3 statements 2026-10-14 to 2026-10-15): reACKed after the DIFFERENT_WITNESS gap was fixed and covered by unit tests
Dependencies
Files
1001 lines under test/bench/ci.
- src/test/fuzz/p2p_1p1c_liveness.cpp +513/-0
- src/test/txdownload_tests.cpp +191/-1
- test/functional/p2p_opportunistic_1p1c.py +118/-2
- src/test/txrequest_tests.cpp +82/-0
- src/test/fuzz/txrequest.cpp +38/-1
- src/txrequest.cpp +39/-0
- src/test/orphanage_tests.cpp +28/-2
- src/net_processing.cpp +21/-3
- src/node/txorphanage.cpp +19/-5
- src/test/fuzz/txorphan.cpp +22/-2
- src/txrequest.h +15/-0
- src/node/txdownloadman_impl.cpp +10/-2
- src/node/txdownloadman_impl.h +4/-3
- src/node/txorphanage.h +2/-1
- src/test/fuzz/CMakeLists.txt +1/-0
Card
The short text the ranking pass compares across PRs.
This PR fixes several liveness and pinning vulnerabilities in 1-parent-1-child package relay and adds an end-to-end fuzz target verifying bounded CPFP relay propagation against adversarial peers. Adversarial or disconnecting peers could prevent CPFP transactions from reaching the mempool by stranding orphans when assigned announcers disconnect, poisoning reject filters using witnessless copies of known transactions, purging orphans globally on package feerate failures caused by malleated parents, or wiping pending orphan parent requests when txid equals wtxid. The patch reassigns orphan reconsideration upon announcer eviction, excludes conflict results from reject filters, keeps orphans on reconsiderable package errors, and implements typed forgetting in TxRequestTracker. It depends on PR 36437.
Data
thread read through 2026-10-15 (5 statements), head 8d44ac8909
labels: P2P
Categories
P2P
P1 · DoS protection (assessed alone as P2; Promoted to P1: it contains #36437's fix plus several further exploitable 1p1c liveness/pinning holes, so it cannot sit a band below its base.)
Plugs multiple adversarial liveness holes where malicious or disconnecting peers can stall or block honest CPFP fee-bump packages. As noted in the description, adversaries could cause a child orphan of an honest fee-paying parent to be withheld until eviction or dropped from all announcers entirely, undermining opportunistic package relay reliability.
Membership: Labeled P2P and modifies orphan management, reject filter handling, and transaction request tracking in src/net_processing.cpp, src/node/txorphanage.cpp, and src/txrequest.cpp.
Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 1