RBF splice funding transactions#4427
Draft
jkczyz wants to merge 20 commits intolightningdevkit:mainfrom
Draft
Conversation
Wallet-related types were tightly coupled to bump_transaction, making them less accessible for other use cases like channel funding and splicing. Extract these utilities to a dedicated module for improved code organization and reusability across the codebase. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Synchronous wallet utilities were coupled to bump_transaction::sync, limiting their reusability for other features like channel funding and splicing which need synchronous wallet operations. Consolidate all wallet utilities in a single module for consistency and improved code organization. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
FundingTxInput was originally designed for channel funding but is now used more broadly for coin selection and splicing. The name ConfirmedUtxo better reflects its general-purpose nature as a confirmed UTXO with previous transaction data. Make ConfirmedUtxo the real struct in wallet_utils and alias FundingTxInput to it for backward compatibility. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Instead of using Deref in APIs using CoinSelectionSource, implement CoinSelectionSource for any Deref with a Target that implements CoinSelectionSource.
FundingTemplate is only ever created by the splice initiator, so is_initiator is always true. The channel already knows who initiated the splice from who won quiescence (is_holder_quiescence_initiator), making the field on FundingTemplate and FundingContribution redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Now that ChannelManager::splice_channel only reads, there is no need to notify listeners about event handling nor persistence.
Split FundingContribution::net_value() (which returned Result<SignedAmount, String>) into a separate validate() method and an infallible net_value() that returns SignedAmount directly. The validate() method checks prevtx sizes and input sufficiency, while net_value() computes the net contribution amount. To make net_value() safe to call without error handling, add MAX_MONEY bounds checks in the build_funding_contribution! macro before coin selection. This ensures FundingContribution is valid by construction: value_added and the sum of outputs are each bounded by MAX_MONEY (~2.1e15 sat), so the worst-case net_value() computation (-2 * MAX_MONEY ~= -4.2e15) is well within i64 range (~-9.2e18). Update callers in channel.rs to use the new separate methods, simplifying error handling at call sites where net_value() previously required unwrapping a Result. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a helper function to assert that SpliceFailed events contain the expected channel_id and contributed inputs/outputs. This ensures that tests verify the contributions match what was originally provided. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When a splice fails, users need to reclaim UTXOs they contributed to the funding transaction. Previously, the contributed inputs and outputs were included in the SpliceFailed event. This commit splits them into a separate DiscardFunding event with a new FundingInfo::Contribution variant, providing a consistent interface for UTXO cleanup across all funding failure scenarios. Changes: - Add FundingInfo::Contribution variant to hold inputs/outputs for DiscardFunding events - Remove contributed_inputs/outputs fields from SpliceFailed event - Add QuiescentError enum for better error handling in funding_contributed - Emit DiscardFunding on all funding_contributed error paths - Filter duplicate inputs/outputs when contribution overlaps existing pending contribution - Return Err(APIError) from funding_contributed on all error cases - Add comprehensive test coverage for funding_contributed error paths Co-Authored-By: Claude Opus 4.5 <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 will contribute to the splice as the acceptor, resulting in a single splice transaction and eliminating the need for a second splice. Unfortunately, they will over pay fees since the FundingContribution was produced as the initiator. However, CoinSelectionSource doesn't support specifying whether common fields need to be paid for. Adjusting the change could work, but there may not be a change output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 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>
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. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the acceptor has a pending contribution from the quiescence tie-breaker scenario, the initiator's proposed feerate may differ from the feerate used during coin selection. Add FundingContribution::adjust_for_feerate to redistribute the fee budget and change output so the acceptor pays their fair share at the target feerate. If the adjustment fails (feerate too high), preserve the QuiescentAction for a future splice and proceed without the acceptor's contribution. The acceptor peeks at the QuiescentAction to check feerate compatibility before validation runs, consuming it only after validation passes. This ensures the QuiescentAction is preserved if either feerate adjustment or splice/RBF validation fails. For splice-out, the estimated fee is updated to the fair fee at the target feerate, returning surplus to the channel balance instead of burning it. Extend the test_splice_rbf_tiebreak_feerate_too_high integration test to demonstrate the full cycle: after the RBF splice locks, node 1's preserved QuiescentAction leads to a new splice being initiated and completed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
👋 Thanks for assigning @wpaulino as a reviewer! |
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
2 participants
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 infunding_contributedQuiescentActionreuse any contributions from previous attemptssplice_channelcontribution if splice not yet lockedDiscardFundingis generated in edge casesBased on #4416