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.

Sign up for GitHub

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
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd1b44e
Put PendingV2Channel implementation behind a trait
optout21 Apr 2, 2025
94f6051
Add RefundingChannel, that can act as both funded and pendingV2
optout21 Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 195 additions & 53 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2292,7 +2292,20 @@ impl<SP: Deref> InitialRemoteCommitmentReceiver<SP> for FundedChannel<SP> where
}
}

impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
// TODO Naming
pub(super) trait PendingV2ChannelTrait<SP: Deref> where SP::Target: SignerProvider {
Comment on lines +2295 to +2296
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?

fn context(&self) -> &ChannelContext<SP>;
fn context_mut(&mut self) -> &mut ChannelContext<SP>;
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?

fn swap_out_dual_funding_context_inputs(&mut self, funding_inputs: &mut Vec<(TxIn, TransactionU16LenLimited)>);
fn unfunded_context(&self) -> &UnfundedChannelContext;
fn interactive_tx_constructor(&self) -> Option<&InteractiveTxConstructor>;
fn interactive_tx_constructor_mut(&mut self) -> &mut Option<InteractiveTxConstructor>;
fn interactive_tx_signing_session_mut(&mut self) -> &mut Option<InteractiveTxSigningSession>;

/// Prepare and start interactive transaction negotiation.
/// `change_destination_opt` - Optional destination for optional change; if None,
/// default destination address is used.
Expand All @@ -2304,11 +2317,11 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
) -> Result<Option<InteractiveTxMessageSend>, AbortReason>
where ES::Target: EntropySource
{
debug_assert!(matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_)));
debug_assert!(self.interactive_tx_constructor.is_none());
debug_assert!(matches!(self.context().channel_state, ChannelState::NegotiatingFunding(_)));
debug_assert!(self.interactive_tx_constructor().is_none());

let mut funding_inputs = Vec::new();
mem::swap(&mut self.dual_funding_context.our_funding_inputs, &mut funding_inputs);
self.swap_out_dual_funding_context_inputs(&mut funding_inputs);

// TODO(splicing): Add prev funding tx as input, must be provided as a parameter

Expand All @@ -2319,14 +2332,14 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
let mut expected_remote_shared_funding_output = None;

let shared_funding_output = TxOut {
value: Amount::from_sat(self.funding.get_value_satoshis()),
script_pubkey: self.funding.get_funding_redeemscript().to_p2wsh(),
value: Amount::from_sat(self.funding().get_value_satoshis()),
script_pubkey: self.funding().get_funding_redeemscript().to_p2wsh(),
};

if self.funding.is_outbound() {
if self.funding().is_outbound() {
funding_outputs.push(
OutputOwned::Shared(SharedOwnedOutput::new(
shared_funding_output, self.dual_funding_context.our_funding_satoshis,
shared_funding_output, self.dual_funding_context().our_funding_satoshis,
))
);
} else {
Expand All @@ -2338,13 +2351,13 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
let change_script = if let Some(script) = change_destination_opt {
script
} else {
signer_provider.get_destination_script(self.context.channel_keys_id)
signer_provider.get_destination_script(self.context().channel_keys_id)
.map_err(|_err| AbortReason::InternalError("Error getting destination script"))?
};
let change_value_opt = calculate_change_output_value(
self.funding.is_outbound(), self.dual_funding_context.our_funding_satoshis,
self.funding().is_outbound(), self.dual_funding_context().our_funding_satoshis,
&funding_inputs, &funding_outputs,
self.dual_funding_context.funding_feerate_sat_per_1000_weight,
self.dual_funding_context().funding_feerate_sat_per_1000_weight,
change_script.minimal_non_dust().to_sat(),
)?;
if let Some(change_value) = change_value_opt {
Expand All @@ -2353,10 +2366,10 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
script_pubkey: change_script,
};
let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu();
let change_output_fee = fee_for_weight(self.dual_funding_context.funding_feerate_sat_per_1000_weight, change_output_weight);
let change_output_fee = fee_for_weight(self.dual_funding_context().funding_feerate_sat_per_1000_weight, change_output_weight);
let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee);
// Check dust limit again
if change_value_decreased_with_fee > self.context.holder_dust_limit_satoshis {
if change_value_decreased_with_fee > self.context().holder_dust_limit_satoshis {
change_output.value = Amount::from_sat(change_value_decreased_with_fee);
funding_outputs.push(OutputOwned::Single(change_output));
}
Expand All @@ -2365,70 +2378,71 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
let constructor_args = InteractiveTxConstructorArgs {
entropy_source,
holder_node_id,
counterparty_node_id: self.context.counterparty_node_id,
channel_id: self.context.channel_id(),
feerate_sat_per_kw: self.dual_funding_context.funding_feerate_sat_per_1000_weight,
is_initiator: self.funding.is_outbound(),
funding_tx_locktime: self.dual_funding_context.funding_tx_locktime,
counterparty_node_id: self.context().counterparty_node_id,
channel_id: self.context().channel_id(),
feerate_sat_per_kw: self.dual_funding_context().funding_feerate_sat_per_1000_weight,
is_initiator: self.funding().is_outbound(),
funding_tx_locktime: self.dual_funding_context().funding_tx_locktime,
inputs_to_contribute: funding_inputs,
outputs_to_contribute: funding_outputs,
expected_remote_shared_funding_output,
};
let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?;
let msg = tx_constructor.take_initiator_first_message();

self.interactive_tx_constructor = Some(tx_constructor);
*self.interactive_tx_constructor_mut() = Some(tx_constructor);

Ok(msg)
}

pub fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor {
fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() {
Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_input(msg).map_err(
|reason| reason.into_tx_abort_msg(self.context.channel_id())),
|reason| reason.into_tx_abort_msg(self.context().channel_id())),
None => Err(msgs::TxAbort {
channel_id: self.context.channel_id(),
channel_id: self.context().channel_id(),
data: b"No interactive transaction negotiation in progress".to_vec()
}),
})
}

pub fn tx_add_output(&mut self, msg: &msgs::TxAddOutput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor {
fn tx_add_output(&mut self, msg: &msgs::TxAddOutput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() {
Some(ref mut tx_constructor) => tx_constructor.handle_tx_add_output(msg).map_err(
|reason| reason.into_tx_abort_msg(self.context.channel_id())),
|reason| reason.into_tx_abort_msg(self.context().channel_id())),
None => Err(msgs::TxAbort {
channel_id: self.context.channel_id(),
channel_id: self.context().channel_id(),
data: b"No interactive transaction negotiation in progress".to_vec()
}),
})
}

pub fn tx_remove_input(&mut self, msg: &msgs::TxRemoveInput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor {
fn tx_remove_input(&mut self, msg: &msgs::TxRemoveInput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() {
Some(ref mut tx_constructor) => tx_constructor.handle_tx_remove_input(msg).map_err(
|reason| reason.into_tx_abort_msg(self.context.channel_id())),
|reason| reason.into_tx_abort_msg(self.context().channel_id())),
None => Err(msgs::TxAbort {
channel_id: self.context.channel_id(),
channel_id: self.context().channel_id(),
data: b"No interactive transaction negotiation in progress".to_vec()
}),
})
}

pub fn tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor {
fn tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput)-> InteractiveTxMessageSendResult {
InteractiveTxMessageSendResult(match self.interactive_tx_constructor_mut() {
Some(ref mut tx_constructor) => tx_constructor.handle_tx_remove_output(msg).map_err(
|reason| reason.into_tx_abort_msg(self.context.channel_id())),
|reason| reason.into_tx_abort_msg(self.context().channel_id())),
None => Err(msgs::TxAbort {
channel_id: self.context.channel_id(),
channel_id: self.context().channel_id(),
data: b"No interactive transaction negotiation in progress".to_vec()
}),
})
}

pub fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult {
let tx_constructor = match &mut self.interactive_tx_constructor {
Some(ref mut tx_constructor) => tx_constructor,
fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult {
let interactive_tx_constructor = self.interactive_tx_constructor_mut();
let tx_constructor = match interactive_tx_constructor {
Some(tx_constructor) => tx_constructor,
None => {
let tx_abort = msgs::TxAbort {
channel_id: msg.channel_id,
Expand All @@ -2446,25 +2460,25 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
};

if let HandleTxCompleteValue::SendTxComplete(_, ref signing_session) = tx_complete {
self.context.next_funding_txid = Some(signing_session.unsigned_tx.compute_txid());
self.context_mut().next_funding_txid = Some(signing_session.unsigned_tx.compute_txid());
};

HandleTxCompleteResult(Ok(tx_complete))
}

pub fn funding_tx_constructed<L: Deref>(
fn funding_tx_constructed<L: Deref>(
&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
where
L::Target: Logger
{
let our_funding_satoshis = self.dual_funding_context.our_funding_satoshis;
let transaction_number = self.unfunded_context.transaction_number();
let our_funding_satoshis = self.dual_funding_context().our_funding_satoshis;
let transaction_number = self.unfunded_context().transaction_number();

let mut output_index = None;
let expected_spk = self.funding.get_funding_redeemscript().to_p2wsh();
let expected_spk = self.funding().get_funding_redeemscript().to_p2wsh();
for (idx, outp) in signing_session.unsigned_tx.outputs().enumerate() {
if outp.script_pubkey() == &expected_spk && outp.value() == self.funding.get_value_satoshis() {
if outp.script_pubkey() == &expected_spk && outp.value() == self.funding().get_value_satoshis() {
if output_index.is_some() {
return Err(ChannelError::Close(
(
Expand All @@ -2484,24 +2498,25 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
)));
};
self.funding.channel_transaction_parameters.funding_outpoint = Some(outpoint);
self.funding_mut().channel_transaction_parameters.funding_outpoint = Some(outpoint);

self.context.assert_no_commitment_advancement(transaction_number, "initial commitment_signed");
let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger);
self.context().assert_no_commitment_advancement(transaction_number, "initial commitment_signed");
let (funding_mut, context_mut) = self.funding_and_context_mut();
let commitment_signed = context_mut.get_initial_commitment_signed(&funding_mut, logger);
let commitment_signed = match commitment_signed {
Ok(commitment_signed) => {
self.funding.funding_transaction = Some(signing_session.unsigned_tx.build_unsigned_tx());
self.funding_mut().funding_transaction = Some(signing_session.unsigned_tx.build_unsigned_tx());
commitment_signed
},
Err(err) => {
self.funding.channel_transaction_parameters.funding_outpoint = None;
self.funding_mut().channel_transaction_parameters.funding_outpoint = None;
return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })));
},
};

let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 {
debug_assert_eq!(our_funding_satoshis, 0);
if signing_session.provide_holder_witnesses(self.context.channel_id, Vec::new()).is_err() {
if signing_session.provide_holder_witnesses(self.context().channel_id, Vec::new()).is_err() {
debug_assert!(
false,
"Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found",
Expand Down Expand Up @@ -2537,16 +2552,143 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
)));
};

self.context.channel_state = ChannelState::FundingNegotiated;
self.context_mut().channel_state = ChannelState::FundingNegotiated;

// Clear the interactive transaction constructor
self.interactive_tx_constructor.take();
self.interactive_tx_signing_session = Some(signing_session);
*self.interactive_tx_constructor_mut() = None;
*self.interactive_tx_signing_session_mut() = Some(signing_session);

Ok((commitment_signed, funding_ready_for_sig_event))
}
}

impl<SP: Deref> PendingV2ChannelTrait<SP> for PendingV2Channel<SP> where SP::Target: SignerProvider {
#[inline]
fn context(&self) -> &ChannelContext<SP> {
&self.context
}

#[inline]
fn context_mut(&mut self) -> &mut ChannelContext<SP> {
&mut self.context
}

#[inline]
fn funding(&self) -> &FundingScope {
&self.funding
}

#[inline]
fn funding_mut(&mut self) -> &mut FundingScope {
&mut self.funding
}

#[inline]
fn funding_and_context_mut(&mut self) -> (&mut FundingScope, &mut ChannelContext<SP>) {
(&mut self.funding, &mut self.context)
}

#[inline]
fn dual_funding_context(&self) -> &DualFundingChannelContext {
&self.dual_funding_context
}

fn swap_out_dual_funding_context_inputs(&mut self, funding_inputs: &mut Vec<(TxIn, TransactionU16LenLimited)>) {
mem::swap(&mut self.dual_funding_context.our_funding_inputs, funding_inputs);
}

#[inline]
fn unfunded_context(&self) -> &UnfundedChannelContext {
&self.unfunded_context
}

#[inline]
fn interactive_tx_constructor(&self) -> Option<&InteractiveTxConstructor> {
self.interactive_tx_constructor.as_ref()
}

#[inline]
fn interactive_tx_constructor_mut(&mut self) -> &mut Option<InteractiveTxConstructor> {
&mut self.interactive_tx_constructor
}

#[inline]
fn interactive_tx_signing_session_mut(&mut self) -> &mut Option<InteractiveTxSigningSession> {
&mut self.interactive_tx_signing_session
}
}

#[cfg(splicing)]
struct RefundingChannel<SP: Deref> where SP::Target: SignerProvider {
funded_channel: FundedChannel<SP>,

// Fields belonging for PendingV2Channel, except duplicate context
pending_funding: FundingScope,
// Note: there is a single context
pending_unfunded_context: UnfundedChannelContext,
pending_dual_funding_context: DualFundingChannelContext,
/// The current interactive transaction construction session under negotiation.
pending_interactive_tx_constructor: Option<InteractiveTxConstructor>,
pending_interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
}

#[cfg(splicing)]
impl<SP: Deref> PendingV2ChannelTrait<SP> for RefundingChannel<SP> where SP::Target: SignerProvider {
#[inline]
fn context(&self) -> &ChannelContext<SP> {
&self.funded_channel.context
}

#[inline]
fn context_mut(&mut self) -> &mut ChannelContext<SP> {
&mut self.funded_channel.context
}

#[inline]
fn funding(&self) -> &FundingScope {
&self.pending_funding
}

#[inline]
fn funding_mut(&mut self) -> &mut FundingScope {
&mut self.pending_funding
}

#[inline]
fn funding_and_context_mut(&mut self) -> (&mut FundingScope, &mut ChannelContext<SP>) {
(&mut self.pending_funding, &mut self.funded_channel.context)
}

#[inline]
fn dual_funding_context(&self) -> &DualFundingChannelContext {
&self.pending_dual_funding_context
}

fn swap_out_dual_funding_context_inputs(&mut self, funding_inputs: &mut Vec<(TxIn, TransactionU16LenLimited)>) {
mem::swap(&mut self.pending_dual_funding_context.our_funding_inputs, funding_inputs);
}

#[inline]
fn unfunded_context(&self) -> &UnfundedChannelContext {
&self.pending_unfunded_context
}

#[inline]
fn interactive_tx_constructor(&self) -> Option<&InteractiveTxConstructor> {
self.pending_interactive_tx_constructor.as_ref()
}

#[inline]
fn interactive_tx_constructor_mut(&mut self) -> &mut Option<InteractiveTxConstructor> {
&mut self.pending_interactive_tx_constructor
}

#[inline]
fn interactive_tx_signing_session_mut(&mut self) -> &mut Option<InteractiveTxSigningSession> {
&mut self.pending_interactive_tx_signing_session
}
}

impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
fn new_for_inbound_channel<'a, ES: Deref, F: Deref, L: Deref>(
fee_estimator: &'a LowerBoundedFeeEstimator<F>,
Expand Down
Loading
Loading