Conversation
The maybe_create_splice_funding_failed! macro only emitted SpliceFailed and DiscardFunding events for the splice initiator. When an acceptor contributed inputs/outputs and the negotiation failed (e.g., disconnect), their contributions were silently discarded with no event notification, preventing the acceptor from reclaiming its UTXOs. Replace the is_initiator() filter with a post-hoc check on whether there are contributions to discard. The initiator always gets events, the acceptor gets events when it has contributions, and acceptors without contributions get no events (nothing to discard). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a splice funding transaction has been negotiated but not yet confirmed, either party may initiate RBF to bump the feerate. This enables the acceptor to handle such requests, allowing continued progress toward on-chain confirmation of splices in rising fee environments. Only the acceptor side is implemented; the acceptor does not contribute funds beyond the shared funding input. The initiator side (sending tx_init_rbf and handling tx_ack_rbf) is left for a follow-up. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The channel monitor previously rejected any new pending funding when one already existed. This prevented adding RBF candidates for a pending splice since each candidate needs its own pending funding entry. Relax the check to only reject new pending funding when its splice parent differs from existing entries, allowing multiple RBF candidates that compete to confirm the same splice. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Expose ChannelManager::rbf_channel as the entry point for bumping the feerate of a pending splice funding transaction. Like splice_channel, it returns a FundingTemplate to be completed and passed to funding_contributed. Validates that a pending splice exists with at least one negotiated candidate, no active funding negotiation, and that the new feerate satisfies the 25/24 increase rule required by the spec. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the quiescence initiator has a pending splice and enters the stfu handler with a QuiescentAction::Splice, send tx_init_rbf to bump the existing splice's feerate rather than starting a new splice_init. This reuses the same QuiescentAction::Splice variant for both initial splices and RBF attempts -- the stfu handler distinguishes them by checking whether pending_splice already exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After sending tx_init_rbf, the initiator receives tx_ack_rbf from the acceptor. Implement the handler to validate the response and begin interactive transaction construction for the RBF funding transaction. Only clear the interactive signing session in `reset_pending_splice_state` when the current funding negotiation is in `AwaitingSignatures`. When an earlier round completed signing and a later RBF round is in `AwaitingAck` or `ConstructingTransaction`, the session belongs to the prior round and must be preserved. Otherwise, disconnecting mid-RBF would destroy the completed prior round's signing session and fire a false debug assertion. Update test_splice_rbf_acceptor_basic to exercise the full initiator flow: rbf_channel → funding_contributed → STFU exchange → tx_init_rbf → tx_ack_rbf → interactive TX → signing → mining → splice_locked. This replaces the previous test that manually constructed tx_init_rbf. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, the tx_init_rbf acceptor always contributed zero to the RBF transaction. This is incorrect when both parties try to RBF simultaneously and one loses the quiescence tie-breaker — the loser becomes the acceptor but still has a pending QuiescentAction::Splice with inputs/outputs that should be included in the RBF transaction. Consume the acceptor's QuiescentAction in the tx_init_rbf handler, just as is already done in the splice_init handler, and report the contribution in the TxAckRbf response.
When the counterparty initiates an RBF and we have no new contribution queued via QuiescentAction, we must re-use our prior contribution so that our splice is not lost. Track contributions in a new field on PendingFunding so the last entry can be re-used in this scenario. Each entry stores the feerate-adjusted version because that reflects what was actually negotiated and allows correct feerate re-adjustment on subsequent RBFs. Only explicitly provided contributions (from a QuiescentAction) append to the vec. Re-used contributions are replaced in-place with the version adjusted for the new feerate so they remain accurate for further RBF rounds, without growing the vec. Add test_splice_rbf_acceptor_recontributes to verify that when the counterparty initiates an RBF and we have no new QuiescentAction queued, our prior contribution is automatically re-used so the splice is preserved. Add test_splice_rbf_recontributes_feerate_too_high to verify that when the counterparty RBFs at a feerate too high for our prior contribution to cover, the RBF is rejected rather than proceeding without our contribution. Add test for sequential RBF splice attempts Add test_splice_rbf_sequential that exercises three consecutive RBF rounds on the same splice (initial → RBF lightningdevkit#1 → RBF lightningdevkit#2) to verify: - Each round requires the 25/24 feerate increase (253 → 264 → 275) - DiscardFunding events reference the correct funding txid from each replaced candidate - The final RBF splice can be mined and splice_locked successfully Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When funding_contributed is called while a splice negotiation is already in progress, unique contributions are computed to determine what to return via FailSplice or DiscardFunding. Without considering negotiated candidates stored in PendingFunding::contributions, UTXOs locked in earlier candidates could be incorrectly returned as reclaimable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SpliceFundingFailed events return contributed inputs and outputs to the user so they can unlock the associated UTXOs. When an RBF attempt is in progress, inputs/outputs already consumed by prior contributions must be excluded to avoid the user prematurely unlocking UTXOs that are still needed by the active funding negotiation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the generic error handling in splice_init and tx_init_rbf with explicit matching on FeeRateAdjustmentError variants: - FeeRateTooLow: initiator's feerate is below our minimum. Proceed without contribution and preserve QuiescentAction for an RBF retry at our preferred feerate. - FeeRateTooHigh: initiator's feerate exceeds our maximum and would consume too much of our change output. Reject the splice with WarnAndDisconnect. - FeeBufferInsufficient: our fee buffer can't cover the acceptor's estimated fee at this feerate. Proceed without contribution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…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>
…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>
|
👋 Hi! I see this is a draft PR. |
| /// 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") | ||
| } |
There was a problem hiding this comment.
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_rbffails → original low-feerate contribution queued- Quiescence completes →
stfu()seespending_splice.is_some()→ sendstx_init_rbfwith 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.
There was a problem hiding this comment.
Bug: When
maybe_adjust_for_rbfcannot adjust the feerate (theErrpath 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 intoQuiescentAction::Splice, and later instfu()(line ~13734), the decision to sendtx_init_rbfvssplice_initis based solely onself.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.
| for output in pending_splice.prior_contributed_outputs() { | ||
| contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey); | ||
| } |
There was a problem hiding this comment.
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:
| 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); | |
| } |
There was a problem hiding this comment.
This is intentional. We don't want to reclaim an address if still in use (e.g., if the change output was adjusted).
| 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); | ||
| } |
There was a problem hiding this comment.
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.
| (8, last_funding_feerate_sat_per_1000_weight, option), | ||
| (10, contributions, optional_vec), |
There was a problem hiding this comment.
New fields use even TLV tags (8 and 10). In LDK's TLV convention, even tags are "must understand" — an older reader encountering an unknown even tag will fail to deserialize. This means a node that upgrades, performs an RBF (making contributions non-empty), and then downgrades would be unable to read its own channel state.
If forward compatibility is desired, these should use odd tags (e.g., 9 and 11) so older readers can skip them gracefully. If breaking downgrade is intentional for the splice feature, disregard.
| 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))); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
This is the other half of the
maybe_adjust_for_rbfbug: the RBF vs. fresh-splice decision here is purelyself.pending_splice.is_some(), with no check on whether the contribution's feerate actually satisfies the 25/24 RBF minimum. Whenmaybe_adjust_for_rbfcouldn't adjust the feerate, we still land here and sendtx_init_rbfwith 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 againstcontribution.feerate()before choosing this path, falling back tosend_splice_initwhen the feerate is insufficient (and the pending splice would need to lock first).
Like what we currently do by not sending stfu.
| @@ -2936,6 +2955,7 @@ impl_writeable_tlv_based_enum_upgradable!(FundingNegotiation, | |||
| (0, AwaitingSignatures) => { | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Automated ReviewThe main issue is a logic bug in the RBF fallback path: when |
Add dedicated methods that handle the prior contribution logic internally — returning the adjusted prior when possible, re-running coin selection when needed (insufficient fee buffer or low max_feerate), or creating a fee-bump-only contribution when no prior exists. Also: - Add force_coin_selection parameter to build_funding_contribution! so rbf/rbf_sync can force coin selection for splice-out RBF (where the channel balance alone is insufficient) and fee-bump-only cases, while splice_out_sync continues to skip it (fees from channel balance) - Remove take_prior_contribution; replace with prior_contribution() returning Option<&PriorContribution> for inspection - Add value_added()/outputs() accessors to FundingContribution and PriorContribution - Guard against max_feerate < adjusted prior's feerate - Update and add tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Can be rebased (and presumably undrafted) now 🎉 |
4 participants
Users previously had to choose between
splice_channelandrbf_channelupfront. This PR merges them into a single entry point and makes the supporting changes needed to enable that.rbf_channelintosplice_channel. The returnedFundingTemplatenow carriesPriorContribution(Adjusted/Unadjusted) so users can reuse their existing contribution for an RBF without new coin selection.splice_channel/rbf_channeltoFundingTemplate's splice methods, giving users control at coin-selection time and exposing the minimum RBF feerate viamin_rbf_feerate().funding_contributednow automatically adjusts the contribution feerate upward to meet the 25/24 RBF requirement when a pending splice appears betweensplice_channelandfunding_contributedcalls, falling back to waiting for the pending splice to lock when adjustment isn't possible.