instagibbs
commented at 12:58 PM on October 5, 2026:
member
The first commit adds p2p_1p1c_liveness, an end-to-end fuzz target for parent and child relay. It
drives a real PeerManager with real messages and validation, and checks that if an honest peer announces
the child of an acceptable parent and child, the child reaches the mempool within a bounded time,
whatever other peers do. Known liveness holes are marked in the target as flags that turn the check off
once opened, including this one. 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).
The second commit hands reconsideration over to a remaining announcer when the marked announcement is
erased, and removes the flag.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
#36015 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36015.svg"></sub> (txorphanage: bound orphan memory by storing transactions serialized by brunoerg)
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-->
instagibbs marked this as ready for review on Oct 5, 2026
instagibbs
commented at 1:12 PM on October 5, 2026:
member
Prior harness found this PR's issue that is fixed, and this harness development found "R11", so I think it's already paid for itself
<details>
<summary>Concept review notes</summary>
Diff. Two commits.
294d6ffcd0 changes TxOrphanageImpl::Erase in about 20 lines. When the erased announcement was the
one marked for reconsideration, the mark moves to the remaining announcer with the lowest NodeId. The
comment in AddChildrenToWorkSet and the doc comment for EraseForPeer are updated, and
txorphanage_sim and orphanage_tests are updated to match.
345497515a adds the p2p_1p1c_liveness fuzz target (547 lines, test-only).
The description matches the diff.
Problem. The node is sent a parent and a child (for example a CPFP), and the child does not reach the
mempool until the orphan expires or is evicted. The trigger is one announcer leaving at the wrong
moment, and an adversary can try to cause it. The evidence is a fuzzer reproduction on master. There is
no reported incident. The goal one level up is that 1p1c relay gets a child accepted in bounded time
whatever adversarial peers do.
Baseline. The child is delayed until orphan expiry or eviction. The attacker needs to be the randomly
chosen announcer, which is likelier with many sybil connections, and it must disconnect before its next
ProcessMessages turn. In that turn ProcessOrphanTx runs first, and the turn is skipped once
fDisconnect is set. If the attacker also supplies the parent and then closes the socket, the race is
plausible. The cost is delay, not loss.
Payoff. The fix closes the hole for both the disconnect and the eviction path. The new unit and sim
tests demonstrate it, and the liveness harness checks it end to end. The payoff is modest but real.
Risks.
The invariant: "an orphan whose inputs became available keeps a reconsiderable announcement while any
announcement exists." It is enforced by construction, because every removal of an announcement goes
through Erase<Tag>.
The change stays inside txorphanage. net_processing and txdownloadman are untouched.
Blast radius is relay liveness and, at worst, extra orphan-validation work. No consensus or fund
exposure, and no new Asserts.
The change is internal and easy to reverse.
Review cost is mostly the harness. It drives a real PeerManager, so it is a possible source of
maintenance work and flakiness.
Prior discussion. This came up in #31666, which introduced the single random announcer:
That proposal was deferred, not rejected. This PR implements it, and the fuzzer reproduction is new
evidence against the impracticality point. No closed PRs attempt this fix.
Alternatives.
Kept, both trades rather than improvements:
Wtxid-keyed reconsider queue that any announcer drains. Removes per-peer ownership structurally, but
changes the TxOrphanage interface and moves the cost of other peers' work into each peer's processing
turn.
Detect-and-heal sweep for orphans with available inputs and no reconsider mark. Catches causes not
yet known, but adds a second mechanism and CPU cost.
Rejected:
Mark every announcer reconsiderable. It undoes ed24e01696 (at most one reconsiderable announcement
per wtxid) and wastes work. This is glozow's argument from #31666.
Process one workset item at disconnect. An attacker defeats it by putting two items in the workset.
Also glozow's argument from #31666.
Process children synchronously after the parent is accepted. It changes net_processing's per-peer
processing budget, a larger change in a costlier place.
Docs-only or tests-only. They leave the hole open.
Re-request the child via txrequest. Adds a new bandwidth path an attacker can trigger.
The PR takes the shallowest placement of the candidates.
Dominance: none.
Verdict:Concept ACK. The fix delivers the payoff without the harness. The harness is test-only and
has already found bugs, so splitting it out would not change the verdict.
instagibbs force-pushed on Oct 5, 2026
DrahtBot added the label CI failed on Oct 5, 2026
instagibbs
commented at 2:25 PM on October 5, 2026:
member
DrahtBot removed the label CI failed on Oct 5, 2026
w0xlt
commented at 1:34 AM on October 8, 2026:
contributor
Concept ACK
instagibbs force-pushed on Oct 11, 2026
fuzz: add end-to-end liveness target for parent and child relay
Check that if an honest peer announces the child of an acceptable
parent and child, the child ends up in the mempool within a bounded
time, whatever other peers do. The target drives a real PeerManager
with real messages and validation, which the existing targets cannot,
as they model validation results.
After an adversarial prefix the honest peer only answers requests. With
an outbound honest peer the adversaries keep acting; with an inbound
one they go quiet. Adversary connections are limited, and adversaries
do not refill outbound slots.
Known liveness holes are marked in the target and turn the check off
once opened, including the lost orphan reconsideration fixed in the
next commit.
47776a909e
txorphanage: hand reconsideration over when its announcement is erased
A child that arrives before its parent is kept as an orphan. Once the
parent is accepted on its own, the orphan is put in one randomly chosen
announcer's work set to be retried. 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.
Hand reconsideration over to the remaining announcer with the lowest
NodeId instead.
Remove its hole from the liveness target.
65b48ba1f1
instagibbs force-pushed on Oct 12, 2026
instagibbs
commented at 9:24 PM on October 12, 2026:
member
added coinbase-based censorship cast stuff to the PR. Does not add any new holes per se, but covers more ground.
@FlashWayne this PR and the follow-up in the OP should cover known holes. If we can get this in first, then subtracting holes just becomes a matter of implementation debate, and which are worth filling. parent_stripped does seem worth filling, for sure.
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-10-24 07:51 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
