diff --git a/changelog.d/bobberb-remote.md b/changelog.d/bobberb-remote.md index b0eb5dcef4..680b39475b 100644 --- a/changelog.d/bobberb-remote.md +++ b/changelog.d/bobberb-remote.md @@ -21,10 +21,10 @@ Fixed - **An SV2 session goes idle after setup.** The setup timeout also closes a client that sent one `CoinbaseOutputConstraints` and then stopped sending. Tip and fee pushes do not refresh it. -- **A replacement may spend an unconfirmed input only from its one - direct conflict.** That parent stays a valid child-pays-for-parent. - Spending the conflict's own output is refused as - `bad-txns-spends-conflicting-tx`. Direct-set feerate is unchanged. +- **A replacement may spend an unconfirmed input only when that input + is a parent of a transaction it replaces.** That parent stays a valid + child-pays-for-parent. Spending the conflict's own output is refused + as `bad-txns-spends-conflicting-tx`. Direct-set feerate is unchanged. - **The invalid-txid set stops at 4,096.** A newer id does not clear the ones already stored. - **Onion and I2P misbehavior is remembered for a day.** The key is diff --git a/changelog.d/cluster-rbf.md b/changelog.d/cluster-rbf.md new file mode 100644 index 0000000000..78ab8c37b4 --- /dev/null +++ b/changelog.d/cluster-rbf.md @@ -0,0 +1,7 @@ +Fixed + +- **A replacement may evict several mempool transactions that spend its + unconfirmed inputs.** That is Core's cluster replacement. An unrelated + unconfirmed input is still refused, and spending a conflict's own + output is still `bad-txns-spends-conflicting-tx`. A replacement that + would exceed the cluster cap is `too-large-cluster`. diff --git a/crates/rbitcoin-mempool/src/accept.rs b/crates/rbitcoin-mempool/src/accept.rs index 556fd8b914..3c6afcdc0b 100644 --- a/crates/rbitcoin-mempool/src/accept.rs +++ b/crates/rbitcoin-mempool/src/accept.rs @@ -1281,9 +1281,9 @@ impl ActiveMempool { }) } - /// Unconfirmed inputs on a replacement must already be parents of the - /// direct conflicts, and there is only one direct conflict. Spending an - /// output of a direct conflict is `bad-txns-spends-conflicting-tx`. + /// An unconfirmed input on a replacement must be a parent of the direct + /// conflicts. Spending an output of a direct conflict is + /// `bad-txns-spends-conflicting-tx`. fn reject_new_unconfirmed( &self, tx: &Transaction, @@ -1313,11 +1313,6 @@ impl ActiveMempool { } return Err(AcceptError::Policy("adds unconfirmed input")); } - if direct.len() > 1 { - return Err(AcceptError::Policy( - "unconfirmed input has multiple conflicts", - )); - } } Ok(()) } @@ -1357,10 +1352,7 @@ impl ActiveMempool { (i128::from(fee_sat).saturating_add(i128::from(prep.fee_delta))).max(0) as u64; let conflict_set = if !scan.direct_conflicts.is_empty() { - // Libre HasNoNewUnconfirmed: parents of the direct conflicts only, - // not their ancestor closure. One direct conflict may be CPFPed. - self.reject_new_unconfirmed(tx, &scan.direct_conflicts)?; - let direct: Vec = scan.direct_conflicts.into_iter().collect(); + let direct: Vec = scan.direct_conflicts.iter().copied().collect(); let set = self.graph.conflict_set(&direct); let (old_fee, old_weight) = self.graph.set_fee_weight(&set); let (direct_fee, direct_weight) = self @@ -1395,6 +1387,11 @@ impl ActiveMempool { weight: combined_w, }); } + // Parents of the direct conflicts only, not their ancestor closure. + // After the cap, so an over-size cluster is `too-large-cluster`. + if !scan.direct_conflicts.is_empty() { + self.reject_new_unconfirmed(tx, &scan.direct_conflicts)?; + } // Core `bad-txns-spends-conflicting-tx`. A parent in the conflict set // is evicted by this accept, and its outputs with it. if parent_txids.iter().any(|p| conflict_set.contains(p)) { diff --git a/crates/rbitcoin-mempool/src/accept_life_journey.rs b/crates/rbitcoin-mempool/src/accept_life_journey.rs index 7b012f81eb..c95bcbcfee 100644 --- a/crates/rbitcoin-mempool/src/accept_life_journey.rs +++ b/crates/rbitcoin-mempool/src/accept_life_journey.rs @@ -248,8 +248,8 @@ fn full_rbf_and_staged_commit(life: &mut Life) -> (OutPoint, Txid) { } /// Pure RBFR: fee 1,500 loses BIP125's absolute fee to the 2,000 parent and -/// child, but pays 1.25x the direct conflict and unpins both. A merger that -/// spends two unconfirmed parents is rejected: more than one direct conflict. +/// child, but pays 1.25x the direct conflict and unpins both. One conflict +/// may be replaced by spending that conflict's parent. fn replacements_unpin_and_leave_the_cluster_count(life: &mut Life) { let f = life.chain.fund(1_000_000); let pinned = spend_tx(f, 999_000); @@ -269,19 +269,6 @@ fn replacements_unpin_and_leave_the_cluster_count(life: &mut Life) { life.accept(&k).unwrap(); pairs.push((coin, p, k)); } - let parent_outs: Vec = pairs - .iter() - .map(|(_, p, _)| out(p.compute_txid(), 0)) - .collect(); - let merger = spend_all(&parent_outs, 10_000); - let err = life.accept(&merger).unwrap_err(); - assert!( - matches!( - err, - AcceptError::Policy("unconfirmed input has multiple conflicts") - ), - "{err}" - ); for (coin, p, k) in &pairs { assert!(life.live(k)); let bump = spend_tx(out(p.compute_txid(), 0), 10_000); @@ -786,9 +773,9 @@ fn orphan_chain_promotes_to_the_cluster_cap() { let _ = std::fs::remove_dir_all(&dir); } -/// Libre `HasNoNewUnconfirmed`: an unconfirmed input must already be a parent -/// of the direct conflicts, and there is only one direct conflict. A -/// single-conflict CPFP of that parent stays allowed. +/// An unconfirmed input must already be a parent of the direct conflicts. +/// Several of those conflicts may be replaced together. A single-conflict +/// CPFP of that parent stays allowed. #[test] fn replacement_unconfirmed_inputs_follow_libre_rule() { let dir = tmp_dir(); @@ -849,15 +836,38 @@ fn replacement_unconfirmed_inputs_follow_libre_rule() { &[out(wide.compute_txid(), 0), out(wide.compute_txid(), 1)], 50_000, ); - let err = life.accept(&merger).unwrap_err(); + life.accept(&merger) + .expect("several conflicts may be replaced by spending their parent outputs"); + assert!(!life.live(&k0) && !life.live(&k1)); + assert!(life.live(&wide)); + assert!(life.live(&merger)); + assert_eq!(life.cluster_len(&merger), 2); + + // A full chain plus a separate conflict. Spending the chain tip and the + // conflict's confirmed input would join the full cluster. Core reports + // the cap. The conflict stays. + let mut value = 100_000 + (MAX_CLUSTER_COUNT as u64) * 1_000; + let mut tip = spend_tx(life.chain.fund(value + 1_000), value); + life.accept(&tip).unwrap(); + for _ in 1..MAX_CLUSTER_COUNT { + value -= 1_000; + let next = spend_tx(out(tip.compute_txid(), 0), value); + life.accept(&next).unwrap(); + tip = next; + } + assert_eq!(life.cluster_len(&tip), MAX_CLUSTER_COUNT); + let capped = life.chain.fund(100_000); + let capped_victim = spend_tx(capped, 90_000); + life.accept(&capped_victim).unwrap(); + let over = spend_all(&[out(tip.compute_txid(), 0), capped], 1_000); + let err = life.accept(&over).unwrap_err(); assert!( - matches!( - err, - AcceptError::Policy("unconfirmed input has multiple conflicts") - ), + matches!(err, AcceptError::ClusterTooLarge { .. }), "{err}" ); - assert!(life.live(&k0) && life.live(&k1)); + assert!(life.live(&capped_victim)); + assert!(life.live(&tip)); + assert!(!life.live(&over)); let _ = std::fs::remove_dir_all(&dir); } diff --git a/docs/external_findings/051-rbfr-direct-set.md b/docs/external_findings/051-rbfr-direct-set.md index e6358ccc38..e3ef32a6dc 100644 --- a/docs/external_findings/051-rbfr-direct-set.md +++ b/docs/external_findings/051-rbfr-direct-set.md @@ -10,9 +10,9 @@ package. `pure_rbfr_pays` takes that direct fee and weight. `rbf_allows_replacement` still also allows a BIP125-style payment against the full conflict set. A high-feerate replacement with a lower absolute fee than a fat descendant package is admitted by the direct-set rule. -An unconfirmed input must be a parent of the direct conflicts, and only -one direct conflict is allowed. That input rule does not change -direct-set pricing. +An unconfirmed input must be a parent of the direct conflicts. Several +of those conflicts may be replaced together. That input rule does not +change direct-set pricing. **Regression:** `rbitcoin-mempool` `accept::tests::mempool_accept_life`, diff --git a/docs/external_findings/096-unconfirmed-rbf-input.md b/docs/external_findings/096-unconfirmed-rbf-input.md index 21c4af423f..801826f8d7 100644 --- a/docs/external_findings/096-unconfirmed-rbf-input.md +++ b/docs/external_findings/096-unconfirmed-rbf-input.md @@ -4,6 +4,6 @@ **Status:** fixed **Found by:** bobberb and muse, 2026-10-18 -A replacement could spend any mempool output that was not itself in the conflict set. Libre Relay keeps BIP125 rule 2 and adds one limit: an unconfirmed input must be a parent of the direct conflicts, and there may be only one direct conflict. A single-conflict CPFP of that parent still replaces, and the direct-set feerate rule is unchanged. +A replacement could spend any mempool output that was not itself in the conflict set. An unconfirmed input must be a parent of the direct conflicts. Several of those conflicts may be replaced together, which is what Core's cluster replacement does. Spending a conflict's own output is still refused. A single-conflict CPFP of that parent still replaces, and the direct-set feerate rule is unchanged. **Regression:** `rbitcoin-mempool` `replacement_unconfirmed_inputs_follow_libre_rule`. diff --git a/docs/operator/operations.md b/docs/operator/operations.md index 2db2335e09..be07b0ac44 100644 --- a/docs/operator/operations.md +++ b/docs/operator/operations.md @@ -552,7 +552,7 @@ scrape_configs: | Min relay | **0.1 sat/vB** (100 sat/kvB) | | Dust | **not enforced** | | Script templates | allow if consensus-valid (within weight/CPU) | -| RBF | **full RBF** (no BIP125 signaling required). An unconfirmed input is allowed only when it is a parent of the one direct conflict | +| RBF | **full RBF** (no BIP125 signaling required). An unconfirmed input is allowed only when it is a parent of a transaction this one replaces. Several of those may be replaced together | | Annex | empty OK; non-empty only if first data byte after `0x50` is `0x00` | | Cluster caps | 64 txs / 101 kvB (404 kWU) | | Eviction | worst linearization **chunk** when over weight budget |