Skip to content

[Draft] RefundingChannel: combine FundedChannel and PendingV2Channel using a trait for Pending #3720

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

Closed
wants to merge 2 commits into from

Conversation

optout21
Copy link
Contributor

@optout21 optout21 commented Apr 8, 2025

This is a simpler version of #3702.

The goal: create a RefundingChannel phase, for splice negotiation, that can act both as a funded channel (pre-splice) and pending channel (transaction negotiation).

How?

  • a new trait PendingV2ChannelTrait (name TBD) created, and PendingV2Channel implements it. Most implememtations are moved from the struct to the trait
  • new RefundingChannel that has a FundedChannel for the funded, and several fields from pending for the pending.
  • The funded channel can be accessed through as_funded() or directly
  • The pending channel can be accessed through the pending trait

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Apr 8, 2025

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignemnt, please click here.

fn funding(&self) -> &FundingScope;
fn funding_mut(&mut self) -> &mut FundingScope;
fn funding_and_context_mut(&mut self) -> (&mut FundingScope, &mut ChannelContext<SP>);
fn dual_funding_context(&self) -> &DualFundingChannelContext;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only need to use dual_funding_context in begin_interactive_funding_tx_construction? If so, seems like it should be passed into that and consumed rather than needing a method. Then you wouldn't need swap_out_dual_funding_context_inputs either.

In other words, can DualFundingChannelContext be constructed immediately before calling begin_interactive_funding_tx_construction rather than being a field on each channel type?

Comment on lines +2295 to +2296
// TODO Naming
pub(super) trait PendingV2ChannelTrait<SP: Deref> where SP::Target: SignerProvider {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about FundingTxConstruction?

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@jkczyz jkczyz requested a review from wpaulino April 8, 2025 22:33
@optout21
Copy link
Contributor Author

optout21 commented Apr 9, 2025

One additional idea to consider, as discussed today: don't introduce new phase/channelstruct, but keep using FundedChannel throughout spicing, with additional fields needed placed in an optional Splicing struct.

@wpaulino wpaulino removed their request for review April 10, 2025 00:42
@optout21
Copy link
Contributor Author

Continued in #3736, closing this one

@optout21 optout21 closed this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants