Contribute to splice as acceptor#4416
Draft
jkczyz wants to merge 10 commits intolightningdevkit:mainfrom
Draft
Conversation
|
👋 Hi! I see this is a draft PR. |
fc3e1da to
758ab0a
Compare
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>
The following types have methods for returning contributed inputs and outputs: - FundingNegotiationContext - InteractiveTxConstructor - InteractiveTxSigningSession - ConstructedTransaction Having iterators for these can avoid allocations, which is useful for filtering contributed input and outputs when producing DiscardFunding events. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b6cec12 to
5a26025
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4416 +/- ##
==========================================
+ Coverage 85.87% 85.99% +0.11%
==========================================
Files 157 159 +2
Lines 103769 104622 +853
Branches 103769 104622 +853
==========================================
+ Hits 89115 89970 +855
+ Misses 12158 12148 -10
- Partials 2496 2504 +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:
|
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.6 <noreply@anthropic.com>
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 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>
5a26025 to
658f332
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.