RBF splice funding transactions#4427
Draft
jkczyz wants to merge 26 commits intolightningdevkit:mainfrom
Draft
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:
|
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>
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. Provide a method on FundingContribution that adjusts the fees and the change output, if possible.
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>
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. Validation is split into a separate validate_tx_ack_rbf method (taking &self) to prevent state modifications during validation, following the same pattern as validate_splice_ack. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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. Also update test_splice_rbf_insufficient_feerate to verify the 25/24 feerate rule is enforced on both sides: the rbf_channel API rejects insufficient feerates before any messages are sent, and the acceptor rejects them when handling tx_init_rbf from a misbehaving peer. 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. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
Add test_splice_rbf_disconnect_filters_prior_contributions covering the reset_pending_splice_state macro path: when disconnecting during an RBF round that reuses the same UTXOs as a prior round, the DiscardFunding event should filter out inputs still committed to the prior round while keeping change outputs that differ due to the higher feerate. Extend do_abandon_splice_quiescent_action_on_shutdown with a pending_splice parameter covering the abandon_quiescent_action path: when shutdown occurs while a splice is queued and a prior splice is pending, the DiscardFunding event should similarly filter overlapping inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
08c05f6 to
97858df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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