RBF splice funding transactions#4427
Conversation
|
👋 Thanks for assigning @wpaulino as a reviewer! |
1552b89 to
08c05f6
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4427 +/- ##
==========================================
+ Coverage 85.94% 86.05% +0.11%
==========================================
Files 159 159
Lines 104607 105279 +672
Branches 104607 105279 +672
==========================================
+ Hits 89901 90596 +695
+ Misses 12194 12163 -31
- Partials 2512 2520 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
08c05f6 to
97858df
Compare
|
🔔 1st Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
1 similar comment
|
🔔 1st Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
84058e4 to
4aaa312
Compare
Now that the Splice variant (containing non-serializable FundingContribution) is the only variant produced, and the previous commit consumes the acceptor's quiescent_action in splice_init(), there is no longer a need to persist it. This allows removing LegacySplice, SpliceInstructions, ChangeStrategy, and related code paths including calculate_change_output, calculate_change_output_value, and the legacy send_splice_init method. With ChangeStrategy removed, the only remaining path in calculate_change_output was FromCoinSelection which always returned Ok(None), making it dead code. The into_interactive_tx_constructor method is simplified accordingly, and the signer_provider parameter is removed from it and from splice_init/splice_ack since it was only needed for the removed change output calculation. On deserialization, quiescent_action (TLV 65) is still read for backwards compatibility but discarded, and the awaiting_quiescence channel state flag is cleared since it cannot be acted upon without a quiescent_action. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the single public InteractiveTxConstructor::new() with separate new_for_outbound() and new_for_inbound() constructors. This moves the initiator's first message preparation out of the core constructor, making it infallible and removing is_initiator from the args struct. Callers no longer need to handle constructor errors, which avoids having to generate SpliceFailed/DiscardFunding events after the QuiescentAction has already been consumed during splice_init/splice_ack handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a `change_output: Option<&TxOut>` parameter to `estimate_transaction_fee` so the initial fee estimate accounts for the change output's weight. Previously, the change output weight was omitted from `estimated_fee` in `FundingContribution`, causing the estimate to be slightly too low when a change output was present. This also eliminates an unnecessary `Vec<TxOut>` allocation in `compute_feerate_adjustment`, which previously cloned outputs into a temporary Vec just to include the change output for the fee estimate. A mock `TightBudgetWallet` is added to `splicing_tests` to demonstrate that `validate()` correctly rejects contributions where the input value is sufficient without the change output weight but insufficient with it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4aaa312 to
d40306c
Compare
When constructing a FundingContribution, it's always assumed the estimated_fee is for when used as the initiator, who pays for the common fields and shared inputs / outputs. However, when the contribution is used as the acceptor, we'd be overpaying fees. Additionally, the initiator's chosen fee rate may not be compatible with the acceptors contributions. The selected UTXOs may not be enough to pay for a higher feerate (i.e., the change output is not enough to pay or there is no change output). This change provides a method on FundingContribution for adjusting the fee rate with the above concerns in mind. It also updates it to include a max_feerate specified by the user when initiating a splice. This ensures the acceptor isn't forced to pay an overly high fee rate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When both nodes want to splice simultaneously, the quiescence tie-breaker designates one as the initiator. Previously, the losing node responded with zero contribution, requiring a second full splice session after the first splice locked. This is wasteful, especially for often-offline nodes that may connect and immediately want to splice. Instead, the losing node contributes to the winner's splice as the acceptor, merging both contributions into a single splice transaction. Since the FundingContribution was originally built with initiator fees (which include common fields and shared input/output weight), the fee is adjusted to the acceptor rate before contributing, with the surplus returned to the change output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use a single get_and_clear_pending_msg_events() + match pattern for the initiator's turn, matching the existing acceptor code path. Also add assertions that all expected initiator inputs and outputs were sent. Co-Authored-By: Claude Opus 4.6 <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 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. - BudgetInsufficient: our fee budget can't cover the acceptor's fair fee at this feerate. Proceed without contribution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply the same explicit FeeRateAdjustmentError handling to the tx_init_rbf acceptor path as was done for splice_init: - TooLow: proceed without queued contribution, preserve QuiescentAction for an RBF retry at our preferred feerate. - TooHigh: reject with WarnAndDisconnect. - BudgetInsufficient: proceed without queued contribution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d40306c to
f76abb6
Compare
After a splice has been negotiated but hasn't been locked, its fee rate may be bumped using replace-by-fee (RBF). This requires the channel to re-enter quiescence upon which
tx_init_rbf/tx_ack_rbfare exchanged and the interactive-tx constructor protocol commences as usual.This PR adds support for initiating and accepting RBF attempts, as well as contributing to the attempt as an acceptor when losing the quiescence tie breaker, assuming the initiator's fee rate allows for it without needing to re-run coin selection. Instead, the difference in fee rate may be fine if not paying for common fields and shared input / outputs as the acceptor allows for it or if the change output (if any) can be adjusted accordingly.
TODO for follow-up PRs:
feerateinFundingTemplateto bemin_feerateand have user supply it inFundingContributionbuildershave RBF acceptor w/oQuiescentActionreuse any contributions from previous attemptssplice_channelcontribution if splice not yet lockedDiscardFundingis generated in edge casesBased on #4416