Contribute to splice as acceptor#4416
Draft
jkczyz wants to merge 12 commits intolightningdevkit:mainfrom
Draft
Conversation
|
👋 Hi! I see this is a draft PR. |
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>
fc3e1da to
758ab0a
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.
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
FundingContributionwas produced as the initiator. However,CoinSelectionSourcedoesn't support specifying whether common fields need to be paid for. Adjusting the change could work, but there may not be a change output.Based on #4388.