Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 480
Introduce FundingContributionBuilder API#4516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
b4b3bfb41a2baa11610af58f226eb1c3e299f9fe58File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -12324,7 +12324,25 @@ where | ||
| ); | ||
| let min_rbf_feerate = prev_feerate.map(min_rbf_feerate); | ||
| let prior = if pending_splice.last_funding_feerate_sat_per_1000_weight.is_some() { | ||
| self.build_prior_contribution() | ||
| if let Some(prior) = self | ||
| .pending_splice | ||
| .as_ref() | ||
| .and_then(|pending_splice| pending_splice.contributions.last()) | ||
| { | ||
| let holder_balance = self | ||
| .get_holder_counterparty_balances_floor_incl_fee(&self.funding) | ||
| .map(|(h, _)| h) | ||
| .map_err(|e| APIError::ChannelUnavailable { | ||
| err: format!( | ||
| "Channel {} cannot be spliced at this time: {}", | ||
| self.context.channel_id(), | ||
| e | ||
| ), | ||
| })?; | ||
wpaulino marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| Some(PriorContribution::new(prior.clone(), holder_balance)) | ||
| } else { | ||
| None | ||
| } | ||
| } else { | ||
| None | ||
| }; | ||
| @@ -12346,21 +12364,6 @@ where | ||
| Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate, prior_contribution)) | ||
| } | ||
| /// Clones the prior contribution and fetches the holder balance for deferred feerate | ||
| /// adjustment. | ||
| fn build_prior_contribution(&self) -> Option<PriorContribution> { | ||
| debug_assert!( | ||
| self.pending_splice.is_some(), | ||
| "build_prior_contribution requires pending_splice" | ||
| ); | ||
| let prior = self.pending_splice.as_ref()?.contributions.last()?; | ||
| let holder_balance = self | ||
| .get_holder_counterparty_balances_floor_incl_fee(&self.funding) | ||
| .map(|(h, _)| h) | ||
| .ok(); | ||
| Some(PriorContribution::new(prior.clone(), holder_balance)) | ||
| } | ||
| /// Returns whether this channel can ever RBF, independent of splice state. | ||
| fn is_rbf_compatible(&self) -> Result<(), String> { | ||
| if self.context.minimum_depth(&self.funding) == Some(0) { | ||
| @@ -12532,14 +12535,12 @@ where | ||
| }; | ||
| } | ||
| if let Err(e) = contribution.validate().and_then(|()| { | ||
| // For splice-out, our_funding_contribution is adjusted to cover fees if there | ||
| // aren't any inputs. | ||
| let our_funding_contribution = contribution.net_value(); | ||
| let our_funding_contribution = contribution.net_value(); | ||
| if let Err(e) = | ||
| self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO) | ||
| }) { | ||
| { | ||
| log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); | ||
| return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution))); | ||
| } | ||
| @@ -14101,13 +14102,11 @@ where | ||
| // funding_contributed and quiescence, reducing the holder's | ||
| // balance. If invalid, disconnect and return the contribution so | ||
| // the user can reclaim their inputs. | ||
| if let Err(e) = contribution.validate().and_then(|()| { | ||
| let our_funding_contribution = contribution.net_value(); | ||
| self.validate_splice_contributions( | ||
| our_funding_contribution, | ||
| SignedAmount::ZERO, | ||
| ) | ||
| }) { | ||
| let our_funding_contribution = contribution.net_value(); | ||
| if let Err(e) = self.validate_splice_contributions( | ||
| our_funding_contribution, | ||
| SignedAmount::ZERO, | ||
| ) { | ||
| let failed = self.splice_funding_failed_for(contribution); | ||
| return Err(( | ||
| ChannelError::WarnAndDisconnect(format!( | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6638,20 +6638,32 @@ impl< | ||
| /// The splice initiator is responsible for paying fees for common fields, shared inputs, and | ||
| /// shared outputs along with any contributed inputs and outputs. When building a | ||
| /// [`FundingContribution`], fees are estimated at `min_feerate` assuming initiator | ||
| /// responsibility and must be covered by the supplied inputs for splice-in or the channel | ||
| /// balance for splice-out. If the counterparty also initiates a splice and wins the | ||
| /// tie-break, they become the initiator and choose the feerate. The fee is then | ||
| /// re-estimated at the counterparty's feerate for only our contributed inputs and outputs, | ||
| /// which may be higher or lower than the original estimate. The contribution is dropped and | ||
| /// the splice proceeds without it when: | ||
| /// responsibility. Contributions fall into two cases: | ||
| /// - **input-backed contributions**: when wallet inputs are selected, those inputs pay for both | ||
| /// the requested value added to the channel and any explicit withdrawal outputs. For | ||
| /// example, a 60,000 sat input might add 50,000 sat to the channel, pay a 2,000 sat fee, | ||
| /// and return 8,000 sat as change. A later RBF first tries to preserve that 50,000 sat | ||
| /// value added and cover any higher fee or newly requested withdrawal from the original | ||
| /// 10,000 sat fee buffer (2,000 sat fee + 8,000 sat change). If that buffer is not enough, | ||
| /// the prior contribution cannot be reused without selecting new wallet inputs. | ||
| /// - **input-less contributions**: when no wallet inputs are selected, fees and explicit | ||
| /// withdrawal outputs are paid from the channel balance. For example, a pure splice-out that | ||
| /// withdraws 20,000 sat from a 100,000 sat holder balance leaves up to 80,000 sat available | ||
| /// for fees. A later RBF keeps the 20,000 sat withdrawal only while that remaining balance | ||
| /// can still cover the re-estimated fee. | ||
| /// | ||
| /// If the counterparty also initiates a splice and wins the tie-break, they become the | ||
| /// initiator and choose the feerate. The fee is then re-estimated at the counterparty's | ||
| /// feerate for only our contributed inputs and outputs, which may be higher or lower than the | ||
| /// original estimate. The contribution is dropped and the splice proceeds without it when: | ||
| /// - the counterparty's feerate is below `min_feerate` | ||
| /// - the counterparty's feerate is above `max_feerate` and the re-estimated fee exceeds the | ||
| /// original fee estimate | ||
| /// - the re-estimated fee exceeds the *fee buffer* regardless of `max_feerate` | ||
wpaulino marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| /// | ||
| /// The fee buffer is the maximum fee that can be accommodated: | ||
| /// - **splice-in**: the selected inputs' value minus the contributed amount | ||
| /// - **splice-out**: the channel balance minus the withdrawal outputs | ||
| /// - **input-backed contributions**: the original fee plus any change output value | ||
| /// - **input-less contributions**: the channel balance minus the withdrawal outputs | ||
| /// | ||
| /// # Events | ||
| /// | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug:
splice_outnow has additive output semantics when a prior contribution exists (viawith_prior_contribution). On RBF calls, the builder is seeded with the prior's outputs, thenadd_outputs(outputs)appends the same withdrawal again — doubling the withdrawal amount.Trace for the RBF case:
outputs = [TxOut { value: 546, ... }], no inputs (splice-out)outputs = [withdrawal]splice_outcallsadd_outputs([withdrawal])→outputs = [withdrawal, withdrawal]build()→amend_without_coin_selectionconstructs a splice-out with doubled outputscompute_feerate_adjustmentchecksfee + 1092 <= holder_balance— which passes (capacity > 20K sat)The old
splice_out_syncdestructured the template with..which discarded the prior contribution, so outputs were always treated as absolute. The newsplice_outis additive.Fix: use
without_prior_contributionin the closure when a prior exists, or userbf_prior_contribution_syncfor the RBF case. For example:(The
splice_inclosure has the analogous issue — already flagged in a prior review pass.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpaulino Should we address this given the earlier comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline. We'll wait for #4550 to land.