Repository navigation
mempool: admit a cluster replacement of several conflicts - #1091
Conversation
Core replaces many children by spending the parents they already spend. An unrelated unconfirmed input is still refused, and an over-size cluster is still too-large-cluster. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Review
Adversarial pass on 223c3b3b. Question: does dropping the one-conflict limit reopen finding 096, or only the Core cluster-replacement case?
Disposition: comment. No change requested. Finding 096 stays closed. The overnight mempool_cluster.py failure is the case this admits.
What the patch actually does
reject_new_unconfirmed still refuses a replacement input whose parent transaction is in the mempool unless that parent is an input of a direct conflict. Spending the conflict's own output is still bad-txns-spends-conflicting-tx. The deleted branch is only direct.len() > 1.
That branch was why test_cluster_limit_rbf failed. Each of the 63 inputs is a parent of exactly one child. The transaction has many direct conflicts, so the old check rejected it before the cluster count was considered. With the branch gone, those children leave and the parents plus the new transaction are one cluster, which is what Core asserts.
The check then moved to after the cluster cap. An over-cap transaction that also spends an unrelated mempool output is too-large-cluster and is not admitted. Under the cap, that same shape is still adds unconfirmed input (high in replacement_unconfirmed_inputs_follow_libre_rule).
Finding 096
Disposition: closed. The hole was a replacement spending a mempool output that was not a parent of the conflicts, which ties an unrelated transaction into the replacement. That path still returns adds unconfirmed input, and the test still checks the unrelated output stays in the pool.
The parent test is by transaction id, not by outpoint. A replacement may spend a second output of a parent the conflict already spends. That does not pull in a new ancestor transaction. The parent is already in the conflict's cluster, and the cluster cap still applies. That is inside the rule the finding now states, not a bypass of it.
Ancestor closure is not accepted. Only inputs of the direct conflicts count. A grandparent the conflict does not itself spend is still adds unconfirmed input.
Fee rule times many conflicts
Disposition: accept, pre-existing scale. rbf_allows_replacement is still an or: pay the full conflict set, or pay 1.25× the direct conflicts only (pure_rbfr_pays). Several direct conflicts can now be unconfirmed-parent spends, so one transaction can evict several descendant chains while paying the direct-set rate.
That eviction shape already existed for confirmed inputs. reject_new_unconfirmed skips inputs that are not in the mempool, so a replacement of many confirmed outputs was already allowed to use pure RBFR. This patch extends that to parents that are still unconfirmed. Each resulting cluster is still capped at 64. Finding 051 already records the direct-set rule as won't-fix. This patch does not widen that rule.
Work before the refusal
Disposition: accept. The unrelated-input refusal used to run before conflict_set and the cluster walk. It now runs after both, so a doomed replacement pays for those walks. The walks are the direct conflicts and their descendants, and each cluster is capped at 64. That is not the unbounded “attach any mempool output” cost from 096.
Error name for a descendant output
Disposition: pre-existing, not this patch. If the replacement spends an output of a descendant of a direct conflict, reject_new_unconfirmed returns adds unconfirmed input because that descendant is not itself in direct. The later conflict_set check would have said bad-txns-spends-conflicting-tx. The same order existed when the refusal ran first. The transaction is still not admitted.
Tests
Disposition: sufficient for the deleted branch. The journey now admits one parent with two children and keeps the unrelated-input refusal, the conflicting-output refusal, and the over-cap error. The deleted direct.len() > 1 branch did not care whether the parents were one transaction or many. The nightly shape (63 separate parents) is what mempool_cluster.py passed locally. The in-tree journey does not build that shape. A later special case could tell them apart and the journey would stay green. I would not block on it.
The life-journey helper no longer attempts the two-parent merger. That attempt was only the rejection pin. Leaving it in would have replaced the children before the single-conflict bumps that follow.
Notes
Disposition: nits, not blocking.
changelog.d/cluster-rbf.mdis underFixed. The behavior change is a relaxation toward Core.Changedmatches the release headings more closely.changelog.d/bobberb-remote.mdwas edited so the unreleased note no longer claims the one-conflict limit. That keeps the two fragments from contradicting each other when they are folded.
Not reviewed as defects
Package RBF in the same Core script (confirmed inputs only, then a sponsor child) does not go through the new multi-conflict unconfirmed path. Local mempool_cluster.py passed after the reorder, including the size-limit case that wanted too-large-cluster instead of adds unconfirmed input.
Summary
mempool_cluster.pytest_cluster_limit_rbfsubmits, and the case finding 096's one-conflict limit rejected.bad-txns-spends-conflicting-tx.too-large-cluster, which is what Core reports for the same transaction.Test plan
cargo test -p rbitcoin-mempool --libcargo test --workspace --exclude rbitcoin-bench(two pre-existingrbitcoin-netreorg tests also fail on unmodified master here)./scripts/core-functional/run.sh mempool_cluster.pypassed locallyqc,test,windows,macos,coverage)Made with Cursor