Unify splice and RBF APIs by jkczyz · Pull Request #4486 · lightningdevkit/rust-lightning

jkczyz · 2026-03-16T15:52:08Z

Users previously had to choose between splice_channel and rbf_channel upfront. This PR merges them into a single entry point and makes the supporting changes needed to enable that.

Merge rbf_channel into splice_channel. The returned FundingTemplate now carries PriorContribution (Adjusted/Unadjusted) so users can reuse their existing contribution for an RBF without new coin selection.
To support this, move feerate parameters from splice_channel/rbf_channel to FundingTemplate's splice methods, giving users control at coin-selection time and exposing the minimum RBF feerate via min_rbf_feerate().
Additionally, funding_contributed now automatically adjusts the contribution feerate upward to meet the 25/24 RBF requirement when a pending splice appears between splice_channel and funding_contributed calls, falling back to waiting for the pending splice to lock when adjustment isn't possible.

ldk-reviews-bot · 2026-03-16T15:52:12Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-03-16T15:58:13Z

lightning/src/ln/channel.rs

+	/// Attempts to adjust the contribution's feerate to the minimum RBF feerate so the splice can
+	/// proceed as an RBF immediately rather than waiting for the pending splice to lock.
+	/// Returns the adjusted contribution on success, or the original on failure.
+	fn maybe_adjust_for_rbf<L: Logger>(
+		&self, contribution: FundingContribution, min_rbf_feerate: FeeRate, logger: &L,
+	) -> FundingContribution {
+		if contribution.feerate() >= min_rbf_feerate {
+			return contribution;
+		}
+
+		let holder_balance = match self
+			.get_holder_counterparty_balances_floor_incl_fee(&self.funding)
+			.map(|(holder, _)| holder)
+		{
+			Ok(balance) => balance,
+			Err(_) => return contribution,
+		};
+
+		if let Err(e) =
+			contribution.net_value_for_initiator_at_feerate(min_rbf_feerate, holder_balance)
+		{
+			log_info!(
+				logger,
+				"Cannot adjust to minimum RBF feerate {}: {}; will proceed as fresh splice after lock",
+				min_rbf_feerate,
+				e,
+			);
+			return contribution;
+		}
+
+		log_info!(
+			logger,
+			"Adjusting contribution feerate from {} to minimum RBF feerate {}",
+			contribution.feerate(),
+			min_rbf_feerate,
+		);
+		contribution
+			.for_initiator_at_feerate(min_rbf_feerate, holder_balance)
+			.expect("feerate compatibility already checked")
 	}


Bug: When maybe_adjust_for_rbf cannot adjust the feerate (the Err path at line ~12082), it returns the original contribution unchanged and logs "will proceed as fresh splice after lock". However, nothing actually implements the "wait for lock" behavior. The contribution is queued as-is into QuiescentAction::Splice, and later in stfu() (line ~13734), the decision to send tx_init_rbf vs splice_init is based solely on self.pending_splice.is_some() — there's no check whether the queued contribution's feerate satisfies the 25/24 RBF minimum.

So the flow is:

funding_contributed → maybe_adjust_for_rbf fails → original low-feerate contribution queued

Quiescence completes → stfu() sees pending_splice.is_some() → sends tx_init_rbf with the too-low feerate

Counterparty rejects with InsufficientRbfFeerate → tx_abort → splice fails entirely

The splice doesn't "proceed as fresh splice after lock" — it fails immediately. Either stfu() should check can_initiate_rbf() + feerate compatibility before choosing the RBF path, or funding_contributed should avoid queuing contributions that can't meet the RBF minimum when a pending splice exists.

Bug: When maybe_adjust_for_rbf cannot adjust the feerate (the Err path at line ~12082), it returns the original contribution unchanged and logs "will proceed as fresh splice after lock". However, nothing actually implements the "wait for lock" behavior. The contribution is queued as-is into QuiescentAction::Splice, and later in stfu() (line ~13734), the decision to send tx_init_rbf vs splice_init is based solely on self.pending_splice.is_some() — there's no check whether the queued contribution's feerate satisfies the 25/24 RBF minimum.

That's inaccurate. The fee rate check is try_send_stfu, so we'll never send stfu when there's a pending splice that we can't RBF.

ldk-claude-review-bot · 2026-03-16T15:58:22Z

lightning/src/ln/channel.rs

+					for output in pending_splice.prior_contributed_outputs() {
+						contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey);
+					}


Filtering outputs by script_pubkey alone could incorrectly remove the wrong output if two outputs in different rounds happen to share the same script_pubkey but have different values (e.g., different splice-out amounts to the same address). The same pattern is used in quiescent_action_into_error. Consider comparing the full TxOut (script_pubkey + value) instead:

Suggested change

for output in pending_splice.prior_contributed_outputs() {

contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey);

}

for output in pending_splice.prior_contributed_outputs() {

contributed_outputs.retain(|o| o != output);

}

This is intentional. We don't want to reclaim an address if still in use (e.g., if the change output was adjusted).

ldk-claude-review-bot · 2026-03-16T15:58:29Z

lightning/src/ln/channel.rs

+				if let Some(ref pending_splice) = self.pending_splice {
+					for input in pending_splice.contributed_inputs() {
+						inputs.retain(|i| *i != input);
+					}
+					for output in pending_splice.contributed_outputs() {
+						outputs.retain(|o| o.script_pubkey != output.script_pubkey);
+					}


Same issue here — filtering contributed outputs by script_pubkey only. Should compare full TxOut to avoid false matches when the same script_pubkey is reused with different amounts across rounds.

lightning/src/ln/channel.rs

ldk-claude-review-bot · 2026-03-16T15:58:47Z

lightning/src/ln/channel.rs

+					if self.pending_splice.is_some() {
+						let tx_init_rbf = self.send_tx_init_rbf(context);
+						self.pending_splice.as_mut().unwrap()
+							.contributions.push(prior_contribution);
+						return Ok(Some(StfuResponse::TxInitRbf(tx_init_rbf)));


This is the other half of the maybe_adjust_for_rbf bug: the RBF vs. fresh-splice decision here is purely self.pending_splice.is_some(), with no check on whether the contribution's feerate actually satisfies the 25/24 RBF minimum. When maybe_adjust_for_rbf couldn't adjust the feerate, we still land here and send tx_init_rbf with a feerate that will be rejected.

Consider checking self.can_initiate_rbf() and comparing against contribution.feerate() before choosing this path, falling back to send_splice_init when the feerate is insufficient (and the pending splice would need to lock first).

This is the other half of the maybe_adjust_for_rbf bug: the RBF vs. fresh-splice decision here is purely self.pending_splice.is_some(), with no check on whether the contribution's feerate actually satisfies the 25/24 RBF minimum. When maybe_adjust_for_rbf couldn't adjust the feerate, we still land here and send tx_init_rbf with a feerate that will be rejected.

Wrong for the same reason. We'll never send stfu in this case.

Consider checking self.can_initiate_rbf() and comparing against contribution.feerate() before choosing this path, falling back to send_splice_init when the feerate is insufficient (and the pending splice would need to lock first).

Like what we currently do by not sending stfu.

ldk-claude-review-bot · 2026-03-16T15:58:55Z

lightning/src/ln/channel.rs

 }

 impl_writeable_tlv_based_enum_upgradable!(FundingNegotiation,
 	(0, AwaitingSignatures) => {


Nit: When restoring from old data, funding_feerate_sat_per_1000_weight defaults to 0. This propagates to last_funding_feerate_sat_per_1000_weight = Some(0) when signing completes, making the 25/24 RBF minimum check (new * 24) < (0 * 25) always pass. So the first RBF after restoring old state won't enforce the feerate bump rule. Consider whether this is acceptable or if a sentinel/None should be used instead.

IIRC, this an acceptable tradeoff. Our counterparty should reject, and we won't be able to RBF until they do first. For accepting, we check against fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::UrgentOnChainSweep) if not set.

ldk-claude-review-bot · 2026-03-16T15:59:10Z

Based on my thorough re-review of the entire PR diff, I have examined every changed file and hunk. Here are my findings:

Re-Review Summary

No new issues found beyond the prior review pass.

Prior Comments Now Resolved

Several prior comments no longer apply because the code was refactored between review passes (PriorContribution changed from an Adjusted/Unadjusted enum to a simple struct):

channel.rs:12030 — debug_assert wrong function name: Fixed (now correctly says "build_prior_contribution requires pending_splice")
channelmanager.rs:6532 — doc reference to "this method": Fixed (now correctly references ChannelManager::splice_channel)
funding.rs:295 — opaque Err(()) errors: Fixed (now returns FundingContributionError with diagnostic info)
funding.rs:451,479 — max_feerate in Unadjusted path: Not applicable (Adjusted/Unadjusted variants removed)
funding.rs:456,493 — debug_assert fires after counterparty RBF abort: Not applicable (debug_asserts removed from rbf/rbf_sync)
funding.rs:468 — dead code branch: Not applicable (code restructured)
channel.rs:12003 — holder_balance Err drops prior contribution: Fixed (now returns PriorContribution with holder_balance: None; rbf methods handle None by falling through to coin selection)

Prior Comments That Were Incorrect

channel.rs:13793 — "DiscardFunding includes shared inputs on re-validation failure": Incorrect. The code at line 13816 calls self.splice_funding_failed_for(contribution) which properly filters against contributed_inputs()/contributed_outputs().
channel.rs:12273 — "validation failure path unfiltered": Incorrect. The code at line 12246 also uses self.splice_funding_failed_for(contribution) which handles the filtering.

Prior Comments Still Applicable (not repeated)

channel.rs:6824 — output filtering by script_pubkey only (not full TxOut)
channel.rs:2930-2931 — even TLV tags (8, 10) break downgrade compatibility
channel.rs:2964 area — default feerate 0 from old serialized data
channel.rs:13787 area — quiescent state left set on re-validation failure; no message to counterparty
channel.rs:11974,12025 area — stale min_rbf_feerate from in-progress negotiation
channel.rs:12273 area — validate_splice_contributions runs before maybe_adjust_for_rbf (minor)
channel.rs:6992 — pop heuristic relies on feerate round-tripping through u32
channelmanager.rs:4715 — FundingContribution doc link missing

TheBlueMatt · 2026-03-17T00:45:55Z

Can be rebased (and presumably undrafted) now 🎉

…plate The user doesn't choose the feerate at splice_channel/rbf_channel time — they choose it when performing coin selection. Moving feerate to the FundingTemplate::splice_* methods gives users more control and lets rbf_channel expose the minimum RBF feerate (25/24 of previous) on the template so users can choose an appropriate feerate. splice_channel and rbf_channel no longer take min_feerate/max_feerate. Instead, FundingTemplate gains a min_rbf_feerate() accessor that returns the RBF floor when applicable (from negotiated candidates or in-progress funding negotiations). The feerate parameters move to the splice_in_sync, splice_out_sync, and splice_in_and_out_sync methods (and their async variants), which validate that min_feerate >= min_rbf_feerate before coin selection. Fee estimation documentation moves from splice_channel/rbf_channel to funding_contributed, where the contribution (and its feerate range) is actually provided and the splice process begins. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

lightning/src/ln/funding.rs

lightning/src/ln/channel.rs

TheBlueMatt · 2026-03-17T18:07:28Z

lightning/src/ln/funding.rs

+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum PriorContribution {
+	/// The prior contribution's feerate meets or exceeds the minimum RBF feerate.
+	Adjusted(FundingContribution),


I'm not convinced we need to expose this in a public API? Why shouldn't we do the adjustment when building the funding contribution instead?

At the time, I was thinking that we should attempt it before returning the FundingTemplate since we need the channel balance to call net_value_for_initiator_at_feerate. But it seems we should just include the balance in the template instead. We can't really control that shifting whether we do the computation upfront or wait for the user to do it later.

TheBlueMatt · 2026-03-17T18:12:29Z

lightning/src/ln/funding.rs

+	///
+	/// `max_feerate` is the maximum feerate the caller is willing to accept as acceptor. It is
+	/// used as the returned contribution's `max_feerate` and also constrains coin selection when
+	/// re-running it for unadjusted prior contributions or fee-bump-only contributions.


We should explicitly call out that you can RBF your counterparty's transaction, and in doing so will take over responsibility for all the fees, so to be sure to check if that's what you want.

Updated, though they would still pay for their own inputs/outputs.

TheBlueMatt · 2026-03-17T18:47:15Z

lightning/src/ln/channel.rs

+				.as_ref()
+				.and_then(|pending_splice| pending_splice.funding_negotiation.as_ref())
+			{
+				// A splice is currently being negotiated.


There's some checks in can_initiate_rbf that might also apply here. eg if the channel is 0-conf we can't ever rbf.

Those will be checked when try_send_stfu is called. For the second case (i.e., negotiation fails), we'd have a min_rbf_feerate set when not needed, though. Refactored the RBF-compatibility check to be used first to avoid this.

lightning/src/ln/channelmanager.rs

TheBlueMatt · 2026-03-17T19:21:32Z

lightning/src/ln/funding.rs

 	/// coin selection.
 	pub fn splice_in_sync<W: CoinSelectionSourceSync>(
-		self, value_added: Amount, wallet: W,
+		self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W,


What the existing methods do in the context of an RBF is pretty unclear. It currently appears to just entirely replace the existing splice-in with a higher feerate. ISTM we could instead be being asked to add additional funds in addition to what was already there? For splice-out that's almost certainly what we're being asked to do.

Hmm... it would be confusing if they were to call the say splice_in when the prior contribution was created with splice_out, effectively making it a splice_in_and_out. Curious what @wpaulino thinks given he's working on the the mixed-mode use cases. Updated the docs for now but open to considering the prior contribution in some way.

lightning/src/ln/channel.rs

lightning/src/ln/funding.rs

ldk-claude-review-bot · 2026-03-18T18:42:58Z

lightning/src/ln/funding.rs

+			Some(PriorContribution { contribution, holder_balance }) => {
+				// The prior contribution's feerate is the negotiated feerate from the
+				// previous splice, which is always below the RBF minimum (negotiated + 25).
+				debug_assert!(contribution.feerate < rbf_feerate);


Bug (debug builds): This debug_assert is incorrect and will fire after a counterparty-initiated RBF is aborted.

Scenario:

Our splice completes at feerate X, contribution stored in contributions

Counterparty initiates RBF at feerate Y ≥ X×25/24

In tx_init_rbf handler, our prior contribution is pop()'d, adjusted to feerate Y via for_acceptor_at_feerate, and push()'d back

RBF is aborted (tx_abort) — our contributions.last() now has feerate = Y

User calls splice_channel() → can_initiate_rbf() returns min_rbf = X×25/24

build_prior_contribution() clones the contribution with feerate = Y

User calls rbf_sync() → rbf_feerate = X×25/24 ≤ Y → debug_assert!(Y < X×25/24) fires

In release builds the fallback path handles this correctly (the net_value_for_initiator_at_feerate call returns FeeRateTooLow and coin selection re-runs). But in debug/test builds this panics.

The comment "The prior contribution's feerate is the negotiated feerate from the previous splice, which is always below the RBF minimum" is not always true — it may be a feerate-adjusted version from a failed counterparty RBF.

Fixed and added a test.

ldk-claude-review-bot · 2026-03-18T18:43:21Z

lightning/src/ln/funding.rs

+			Some(PriorContribution { contribution, holder_balance }) => {
+				// The prior contribution's feerate is the negotiated feerate from the
+				// previous splice, which is always below the RBF minimum (negotiated + 25).
+				debug_assert!(contribution.feerate < rbf_feerate);


Same debug_assert issue as in rbf — will fire in debug builds after a counterparty-initiated RBF abort leaves a higher-feerate contribution in contributions.last(). See comment on the rbf method above for the full scenario.

ldk-claude-review-bot · 2026-03-18T18:43:35Z

lightning/src/ln/channel.rs

+		// If a pending splice exists with negotiated candidates, attempt to adjust the
+		// contribution's feerate to the minimum RBF feerate so it can proceed as an RBF immediately
+		// rather than waiting for the splice to lock.
+		let contribution = if let Ok(min_rbf_feerate) = self.can_initiate_rbf() {
+			self.maybe_adjust_for_rbf(contribution, min_rbf_feerate, logger)
+		} else {
+			contribution
+		};


Nit: The try_send_stfu guard at line ~13806 does correctly implement the "wait for lock" behavior by refusing to send stfu when contribution.feerate() < min_rbf_feerate. So the prior review concern about the unadjusted contribution immediately becoming a tx_init_rbf is mitigated — the splice will indeed wait. However, consider adding a brief comment here (or in the log message below at line 12098) noting that try_send_stfu gates this, since the "will proceed as fresh splice after lock" log message in maybe_adjust_for_rbf otherwise reads as if waiting is automatic when it actually depends on a downstream guard.

Added a comment at line 12098.

lightning/src/ln/channelmanager.rs

ldk-claude-review-bot · 2026-03-18T19:19:01Z

lightning/src/ln/channel.rs

+		// If a pending splice exists with negotiated candidates, attempt to adjust the
+		// contribution's feerate to the minimum RBF feerate so it can proceed as an RBF immediately
+		// rather than waiting for the splice to lock.
+		let contribution = if let Ok(min_rbf_feerate) = self.can_initiate_rbf() {
+			self.maybe_adjust_for_rbf(contribution, min_rbf_feerate, logger)
+		} else {
+			contribution
+		};


Minor: validate_splice_contributions at line 12177 runs before this adjustment. For splice-out contributions (no inputs, fees from channel balance), the adjusted contribution has a more negative net_value() due to higher fees — but this adjusted value is never re-validated against channel reserves.

compute_feerate_adjustment checks holder_balance >= target_fee + value_removed, but doesn't check the v2 channel reserve constraint that validate_splice_contributions enforces (post-splice balance >= 1% of post-splice channel value). A marginal splice-out could pass validation at the original feerate but violate channel reserves after the ~4% RBF bump.

In practice this window is very small (the 25/24 fee increase is tiny relative to reserves), and the commitment transaction construction would catch any real violation, so this wouldn't cause fund loss — just a late negotiation failure with a less clear error.

The window is very small and commitment transaction construction catches any real violation. compute_feerate_adjustment already checks holder balance covers withdrawal plus fees at the adjusted feerate.

ldk-claude-review-bot · 2026-03-18T19:19:11Z

lightning/src/ln/funding.rs

+		if feerate > max_feerate {
+			return Err(());
+		}
+
+		if let Some(min_rbf_feerate) = min_rbf_feerate {
+			if feerate < min_rbf_feerate {
+				return Err(());
+			}


The feerate validation failures here return opaque Err(()) with no diagnostic info. Previously, the feerate checks in splice_channel/rbf_channel returned descriptive APIError::APIMisuseError messages (e.g., "min_feerate exceeds max_feerate"). Now those checks moved here, the user gets Err(()) from the splice methods with no clue whether the issue is feerate > max_feerate, feerate < min_rbf_feerate, or coin selection failure.

Consider at minimum documenting the error conditions on each splice method, or providing a richer error type. For example, a user who forgets to check min_rbf_feerate() and passes a low feerate will get a silent Err(()) with no guidance.

Added FundingContributionError.

ldk-claude-review-bot · 2026-03-18T19:19:23Z

lightning/src/ln/channel.rs

+						Ok(min_rbf_feerate) if contribution.feerate() < min_rbf_feerate => {
+							log_given_level!(
+								logger,
+								logger_level,
+								"Waiting for splice to lock: feerate {} below minimum RBF feerate {}",
+								contribution.feerate(),
+								min_rbf_feerate,
+							);
+							return None;


Note: This gate resolves the concern from the prior review comment at line 12110 about maybe_adjust_for_rbf returning the original low-feerate contribution. When adjustment fails, the contribution is queued with feerate < min_rbf_feerate. This gate catches it: try_send_stfu returns None, preventing stfu from being sent. The splice waits until the pending splice locks (clearing pending_splice), at which point self.pending_splice.is_some() is false, the gate is skipped, and the splice proceeds as fresh via splice_init. The "wait for lock" behavior is correctly implemented here.

Similarly, the concern at line 13749 (decision purely based on pending_splice.is_some()) is safe because this gate ensures we never reach the stfu() handler at line 13745 with an insufficient feerate.

Comment added.

ldk-claude-review-bot · 2026-03-23T19:52:27Z

lightning/src/ln/funding.rs

+						return Ok(adjusted);
+					}
+				}
+				build_funding_contribution!(contribution.value_added, contribution.outputs, shared_input, min_rbf_feerate, rbf_feerate, max_feerate, true, wallet, await)


When re-running coin selection for a splice-out prior contribution (no inputs, fees from channel balance) that couldn't be adjusted, force_coin_selection = true causes inputs to be selected from the wallet. This semantically changes the contribution from "splice-out (fees from channel balance)" to "splice-out with wallet inputs (fees from inputs)".

The net_value() computation differs:

Original: net_value = -output_values - estimated_fee (fee deducted from balance)

After re-run: net_value = -output_values (fee paid by new inputs, not deducted)

This means the channel retains more balance than the user's original splice-out intended. While this is the correct fallback when the channel balance can't cover the higher RBF fee, it's a non-obvious semantic change. Consider documenting this behavior in the rbf/rbf_sync docs — that a splice-out prior contribution may gain wallet inputs when adjustment fails.

Re-wrote the docs for clarity and to address this point.

ldk-claude-review-bot · 2026-03-23T19:53:41Z

lightning/src/ln/channel.rs

-							"Waiting on sending stfu for splice RBF: {msg}"
-						);
-						return None;
+					match self.can_initiate_rbf() {


Note for the overall flow: when maybe_adjust_for_rbf (line 12234) adjusts a contribution's feerate/change/value_added before queuing it, and later stfu() clones that adjusted contribution into contributions, the stored prior has the adjusted values — not the user's original. On the next splice_channel() → build_prior_contribution() → prior_contribution(), the user sees modified value_added() (potentially higher due to surplus from change removal during adjustment). Consider noting this in the prior_contribution() docs so API consumers aren't surprised.

Updated docs.

ldk-claude-review-bot · 2026-03-23T19:53:53Z

lightning/src/ln/funding.rs

+				if let Some(holder_balance) = holder_balance {
+					if contribution
+						.net_value_for_initiator_at_feerate(rbf_feerate, holder_balance)
+						.is_ok()


When net_value_for_initiator_at_feerate returns Ok but compute_feerate_adjustment returns FeeRateTooLow (which happens when rbf_feerate < contribution.feerate — e.g., after a counterparty-initiated RBF left a higher-feerate contribution in contributions.last()), this is_ok() check fails and we fall through to build_funding_contribution!. This re-runs coin selection at rbf_feerate, which is lower than the prior contribution's feerate. The resulting contribution has a lower feerate than the prior round.

This is correct behavior (the user explicitly asked for the minimum RBF feerate and the prior's higher feerate came from a counterparty RBF), but it's non-obvious. A comment noting this case would help readability.

Added comment.

ldk-claude-review-bot · 2026-03-23T19:54:22Z

lightning/src/ln/channel.rs

 			let prior_contribution = self
 				.pending_splice
-				.as_mut()
+				.as_ref()
 				.expect("pending_splice is Some")
 				.contributions
-				.pop()
-				.expect("prior_net_value was Some");
+				.last()
+				.expect("prior_net_value was Some")
+				.clone();


Good change from pop() to last().clone(). The old pop() would remove the prior contribution and only push the adjusted one, so contributions would lose the original. With last().clone(), both the original and adjusted entries are preserved. On abort, reset_pending_splice_state pops the adjusted entry (feerate mismatch with last_funding_feerate), correctly restoring the original.

Note: this means contributions grows by one per successful counterparty-initiated RBF round (both the original and adjusted entries remain). For very long-lived channels with many RBF rounds, this could accumulate entries. Not a practical concern (each entry is small and rounds are rare), but worth being aware of for correctness reasoning about the pop heuristic.

Once the splice confirms, these are discarded, just like the negotiated FudningScope candidates. I think the only real concern is whether we want to limit RBF attempts. The spec suggests rejecting after 10 RBF attempts if the fee rate is not high enough for confirmation. @wpaulino @TheBlueMatt Any preference?

I don't see why we wouldn't just stick with the spec recommendation. I thought we had up to 20 though (see BATCH_SIZE_LIMIT in peer_handler.rs) unless that changed recently.

Yea, no strong opinion, might as well just allow up to the message limit IMO

Ah, yeah there is the practical limit of how many commitment_signed's can be batched. But I think the key point is "and the feerate is not high enough to ensure quick confirmation". So we'd possibly want to check our fee estimator, and use the lower limit if applicable. Otherwise, we wouldn't be able to initiate an RBF ourselves at a suitable feetate if the limit is exhausted by our counterparty.

Related, we don't limit how quickly the counterparty may RBF:

- If another RBF attempt has been created recently: - SHOULD send `tx_abort` to reject this RBF attempt and wait for the previous RBF attempt to confirm.

I noticed this during interop as Eclair has a setting for this.

Ah, yeah there is the practical limit of how many commitment_signed's can be batched. But I think the key point is "and the feerate is not high enough to ensure quick confirmation". So we'd possibly want to check our fee estimator, and use the lower limit if applicable.

SGTM.

Related, we don't limit how quickly the counterparty may RBF:

We could use timer_tick_occurred to only allow a single RBF attempt for every N ticks.

We could use timer_tick_occurred to only allow a single RBF attempt for every N ticks.

How many ticks were you thinking? FWIW, Eclair measures in terms of blocks, defaulting to six. This will likely affect our tests, so may hold off for a follow-up.

Hmm, not really sure its worth overthinking this. If our peer wants to brick our channel they have many ways to do that. It seems reasonable to have a "if we've already RBF'd a few times and they want to RBF to something that still definitely wont confirm" we could reject it, but having a lot of logic to avoid hitting a limit here seems unnecessary.

Done now in the last commit.

ldk-claude-review-bot · 2026-03-23T19:54:31Z

lightning/src/ln/channelmanager.rs

+	/// # Return Value
 	///
-	/// [`FundingContribution`]: crate::ln::funding::FundingContribution
+	/// Returns a [`FundingTemplate`] which should be used to obtain a [`FundingContribution`]
+	/// to pass to [`ChannelManager::funding_contributed`]. If a splice has been negotiated but


The docs mention FundingContribution and FundingTemplate with links, but FundingContribution is not re-exported or linked explicitly. Consider adding a [FundingContribution] doc link definition (like the one for FundingTemplate at line 4715) so the link resolves for rustdoc:

/// [`FundingContribution`]: crate::ln::funding::FundingContribution

Both are imported in this file, so not sure what you are talking about. We don't need to re-export for docs, AFAICT.

codecov · 2026-03-23T21:35:51Z

Codecov Report

❌ Patch coverage is 87.32171% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.23%. Comparing base (123c757) to head (145c7b8).
⚠️ Report is 92 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/funding.rs	80.90%	64 Missing and 8 partials ⚠️
lightning/src/ln/channel.rs	97.84%	3 Missing and 1 partial ⚠️
lightning/src/ln/channelmanager.rs	94.11%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4486      +/-   ##
==========================================
+ Coverage   86.00%   86.23%   +0.23%     
==========================================
  Files         159      160       +1     
  Lines      105430   107909    +2479     
  Branches   105430   107909    +2479     
==========================================
+ Hits        90670    93057    +2387     
+ Misses      12246    12212      -34     
- Partials     2514     2640     +126

Flag	Coverage Δ
tests	`86.23% <87.32%> (+0.23%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ldk-claude-review-bot · 2026-03-23T23:38:09Z

lightning/src/ln/channel.rs

+						let (contributed_inputs, contributed_outputs) =
+							contribution.into_contributed_inputs_and_outputs();
+						self.context.channel_state.clear_quiescent();
+						return Ok(Some(StfuResponse::SpliceFailed(SpliceFundingFailed {
+							funding_txo: None,
+							channel_type: None,
+							contributed_inputs,
+							contributed_outputs,
+						})));


Bug: When the contribution fails re-validation at quiescence time, the DiscardFunding event includes all inputs/outputs from the contribution — even those shared with the prior locked round's splice tx.

Scenario: User calls rbf_sync() which adjusts the prior contribution (reusing the same wallet inputs). The RBF contribution fails re-validation here. into_contributed_inputs_and_outputs() returns the prior round's inputs. The DiscardFunding event tells the user those inputs can be re-spent, but they're still committed to the prior pending splice tx.

Compare with maybe_create_splice_funding_failed! (line 6753-6759) which filters prior contributions:

for input in pending_splice.prior_contributed_inputs() { contributed_inputs.retain(|i| *i != input); }

Since the failing contribution was never pushed to contributions, ALL entries in contributions are prior rounds. The fix should filter against contributed_inputs() (all contributions), not prior_contributed_inputs():

Suggested change

let (contributed_inputs, contributed_outputs) =

contribution.into_contributed_inputs_and_outputs();

self.context.channel_state.clear_quiescent();

return Ok(Some(StfuResponse::SpliceFailed(SpliceFundingFailed {

funding_txo: None,

channel_type: None,

contributed_inputs,

contributed_outputs,

})));

let (mut contributed_inputs, mut contributed_outputs) =

contribution.into_contributed_inputs_and_outputs();

if let Some(pending_splice) = self.pending_splice.as_ref() {

for input in pending_splice.contributed_inputs() {

contributed_inputs.retain(|i| *i != input);

}

for output in pending_splice.contributed_outputs() {

contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey);

}

}

self.context.channel_state.clear_quiescent();

return Ok(Some(StfuResponse::SpliceFailed(SpliceFundingFailed {

funding_txo: None,

channel_type: None,

contributed_inputs,

contributed_outputs,

})));

Yup, noticed this too and addressed now using a helper for filtering.

ldk-claude-review-bot · 2026-03-23T23:38:14Z

lightning/src/ln/channel.rs

+						);
+						let (contributed_inputs, contributed_outputs) =
+							contribution.into_contributed_inputs_and_outputs();
+						self.context.channel_state.clear_quiescent();


Bug: clear_quiescent() exits the quiescent session locally, but no message is sent to the counterparty. At this point, both sides have exchanged stfu — the counterparty is quiescent and waiting for us (the initiator) to send splice_init or tx_init_rbf. By clearing quiescent without sending anything:

The counterparty remains in quiescent mode indefinitely, blocking all channel operations on their side.

If our side later tries to send an update (HTLC, commitment_signed), the counterparty will reject it as a quiescence violation, causing a disconnect.

This contrasts with reset_pending_splice_state (line 6982) which also calls clear_quiescent() — but there, a tx_abort message has already been exchanged, so the counterparty has exited quiescence too.

Consider either:

Proceeding with splice_init anyway (letting the negotiation fail gracefully via tx_abort), or

Sending a warning/error message to the counterparty to signal that the quiescent session is being abandoned

@wpaulino Should we just let quiescence timeout instead of clearing here?

We should presumably disconnect, at a minimum, but if there's no way to cancel a quiescence before we've initiated a quiescent action there really should be in the spec. For now just disconnect tho?

Disconnect immediately via ChannelError::WarnAndDisconnect rather than waiting for the timeout

Needed to also return QuiescentError in order to emit events.

TheBlueMatt

The fixups look good to me. IMO feel free to squash and we can land when @wpaulino is happy.

lightning/src/ln/channel.rs

wpaulino · 2026-03-24T23:16:50Z

lightning/src/ln/channel.rs

-
-		self.can_initiate_rbf(min_feerate).map_err(|err| APIError::APIMisuseError { err })?;
+		let min_rbf_feerate =
+			self.can_initiate_rbf().map_err(|err| APIError::APIMisuseError { err })?;


Shouldn't we be using a less strict version of this? I'm not sure we want to reject the RBF just because there's one currently being negotiated. It can just be queued as a QuiescentAction.

This is changed in a later commit to use is_rbf_compatible when combining the API.

wpaulino · 2026-03-24T23:24:38Z

lightning/src/ln/funding.rs

+	#[test]
+	fn test_rbf_sync_no_prior_fee_bump_only_runs_coin_selection() {
+		// When there is no prior contribution (e.g., acceptor), rbf_sync should run coin
+		// selection to add inputs for a fee-bump-only contribution.


I'm confused, why would we RBF something that we've never contributed to? We're not adding any value to the channel either.

We can fail FundingTemplate::rbf if you prefer. It's just what we do if you try to using FundingTemplate::rbf when only your counterparty has contributed.

lightning/src/ln/channel.rs

wpaulino · 2026-03-24T23:34:36Z

lightning/src/ln/channel.rs

+		// This prevents the counterparty from exhausting the RBF budget at low feerates
+		// that won't lead to timely confirmation.
+		const MAX_LOW_FEERATE_RBF_ATTEMPTS: usize = 3;
+		if pending_splice.negotiated_candidates.len() > MAX_LOW_FEERATE_RBF_ATTEMPTS {


Shouldn't we enforce this for our initiated RBFs as well?

Ah, yeah it is a spec requirement, too.

wpaulino · 2026-03-24T23:38:07Z

lightning/src/ln/channel.rs

+		const MAX_LOW_FEERATE_RBF_ATTEMPTS: usize = 3;
+		if pending_splice.negotiated_candidates.len() > MAX_LOW_FEERATE_RBF_ATTEMPTS {
+			let min_feerate =
+				fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);


Not a fan of this heuristic, the counterparty may have a much better fee estimator than us, preventing them from getting things bumped. We can revisit in the future if it's an issue though.

We could proactively make MAX_LOW_FEERATE_RBF_ATTEMPTS set to 10 like the spec dictates?

Updated the constant to 10 as discussed offline.

…uted When splice_channel is called before a counterparty's splice exists, the user builds a contribution at their chosen feerate without a minimum RBF feerate. If the counterparty completes a splice before funding_contributed is called, the contribution's feerate may be below the 25/24 RBF requirement. Rather than always waiting for the pending splice to lock (which would proceed as a fresh splice), funding_contributed now attempts to adjust the contribution's feerate upward to the minimum RBF feerate when the budget allows, enabling an immediate RBF. When the adjustment isn't possible (max_feerate too low or insufficient fee buffer), the contribution is left unchanged and try_send_stfu delays until the pending splice locks, at which point the splice proceeds at the original feerate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Users previously had to choose between splice_channel (fresh splice) and rbf_channel (fee bump) upfront. Since splice_channel already detects pending splices and computes the minimum RBF feerate, rbf_channel was redundant. Merging into a single API lets the user call one method and discover from the returned FundingTemplate whether an RBF is possible. The FundingTemplate now carries the user's prior contribution from the previous splice negotiation when one is available. This lets users reuse their existing contribution for an RBF without performing new coin selection. A PriorContribution enum distinguishes whether the contribution has been adjusted to the minimum RBF feerate (Adjusted) or could not be adjusted due to insufficient fee buffer or max_feerate constraints (Unadjusted). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

When the counterparty initiates an RBF, the prior contribution was popped and replaced with the feerate-adjusted version. If the RBF aborted, the adjusted version persisted, leaving a stale higher feerate in contributions. Change contributions to be an append-only log where each negotiation round pushes a new entry. On abort, pop the last entry if its feerate doesn't match the locked feerate. This naturally preserves the original contribution as an earlier entry in the vec. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Replace opaque Err(()) returns from FundingTemplate methods with a descriptive FundingContributionError enum. This gives callers diagnostic information about what went wrong: feerate bounds violations, invalid splice values, coin selection failures, or non-RBF scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Outbound HTLCs can be sent between funding_contributed and quiescence, reducing the holder's balance. Re-validate the contribution when quiescence is achieved and balances are stable. On failure, emit SpliceFailed + DiscardFunding events and disconnect the peer so both sides cleanly exit quiescence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

After a few RBF attempts, both our own and the counterparty's RBF should target a feerate that will actually confirm. Reject attempts with feerates below the fee estimator's NonAnchorChannelFee target to prevent exhausting the RBF budget at low feerates. The spec requires: "MUST set a high enough feerate to ensure quick confirmation." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ldk-claude-review-bot reviewed Mar 16, 2026

View reviewed changes

lightning/src/ln/channel.rs Show resolved Hide resolved

ldk-claude-review-bot reviewed Mar 16, 2026

View reviewed changes

jkczyz force-pushed the 2026-03-splicing-rbf-merge branch from 1986278 to 7fb35ec Compare March 17, 2026 02:57

jkczyz marked this pull request as ready for review March 17, 2026 02:57

jkczyz requested review from TheBlueMatt and wpaulino March 17, 2026 02:57

ldk-claude-review-bot reviewed Mar 17, 2026

View reviewed changes

lightning/src/ln/funding.rs Outdated Show resolved Hide resolved

ldk-claude-review-bot reviewed Mar 17, 2026

View reviewed changes

lightning/src/ln/channel.rs Outdated Show resolved Hide resolved

TheBlueMatt reviewed Mar 17, 2026

View reviewed changes

jkczyz force-pushed the 2026-03-splicing-rbf-merge branch from 7fb35ec to 1189c8b Compare March 18, 2026 16:42