Skip to content

Follow-ups to PR 3137 #3423

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

Merged
merged 7 commits into from
Feb 27, 2025
Merged

Follow-ups to PR 3137 #3423

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7fc2cf4
Use debug_assert & error on unexpected initial commitment_signed
dunxen Nov 21, 2024
ac060f9
Remove dual_funding cfg-flags
dunxen Jan 9, 2025
eb18913
Move txid check to start of `tx_signatures` handling
dunxen Nov 21, 2024
f0f6a35
Remove unnecessary funding tx clone & remote tx_sigs received check
dunxen Nov 27, 2024
a86a745
s/`Option<TxSignatures>`/`()` for return from `provide_holder_witnesses`
dunxen Nov 27, 2024
d71c31d
Clean up conditional assignment of `funding_ready_for_sig_event`
dunxen Nov 27, 2024
9f9a7a9
Promote V2 channels to `FundedChannel` on initial `commitment_signed`…
dunxen Nov 27, 2024
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
114 changes: 67 additions & 47 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,32 +1478,67 @@ impl<SP: Deref> Channel<SP> where
where
L::Target: Logger
{
let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
let result = if let ChannelPhase::UnfundedV2(chan) = phase {
if let ChannelPhase::UnfundedV2(chan) = &mut self.phase {
let logger = WithChannelContext::from(logger, &chan.context, None);
match chan.funding_tx_constructed(signing_session, &&logger) {
Ok((chan, commitment_signed, event)) => {
self.phase = ChannelPhase::Funded(chan);
Ok((commitment_signed, event))
},
Err((chan, e)) => {
self.phase = ChannelPhase::UnfundedV2(chan);
Err(e)
},
}
chan.funding_tx_constructed(signing_session, &&logger)
} else {
self.phase = phase;
Err(ChannelError::Warn("Got a tx_complete message with no interactive transaction construction expected or in-progress".to_owned()))
};

debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
result
}
}

pub fn force_shutdown(&mut self, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult {
let (funding, context) = self.funding_and_context_mut();
context.force_shutdown(funding, should_broadcast, closure_reason)
}

pub fn commitment_signed<L: Deref>(
&mut self, msg: &msgs::CommitmentSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
) -> Result<(Option<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>>, Option<ChannelMonitorUpdate>), ChannelError>
where
L::Target: Logger
{
let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
match phase {
ChannelPhase::UnfundedV2(chan) => {
let holder_commitment_point = match chan.unfunded_context.holder_commitment_point {
Some(point) => point,
None => {
let channel_id = chan.context.channel_id();
// TODO(dual_funding): Add async signing support.
return Err( ChannelError::close(
format!("Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
channel_id)));
}
};
let mut funded_channel = FundedChannel {
funding: chan.funding,
context: chan.context,
interactive_tx_signing_session: chan.interactive_tx_signing_session,
holder_commitment_point,
is_v2_established: true,
};
let res = funded_channel.commitment_signed_initial_v2(msg, best_block, signer_provider, logger)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably no need to hold up this PR, but is there a reason why commitment_signed_initial_v2 is a method on FundedChannel rather than PendingV2Channel? Seems that would be consistent with v1 channels calling initial_commitment_signed from an unfunded phase, only transitioning to FundedChannel after it returns.

.map(|monitor| (Some(monitor), None))
// TODO: Change to `inspect_err` when MSRV is high enough.
.map_err(|err| {
// We always expect a `ChannelError` close.
debug_assert!(matches!(err, ChannelError::Close(_)));
err
});
self.phase = ChannelPhase::Funded(funded_channel);
res
},
ChannelPhase::Funded(mut funded_channel) => {
let res = funded_channel.commitment_signed(msg, logger).map(|monitor_update_opt| (None, monitor_update_opt));
self.phase = ChannelPhase::Funded(funded_channel);
res
},
_ => {
debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
Err(ChannelError::close("Got a commitment_signed message for an unfunded V1 channel!".into()))
}
}
}
}

impl<SP: Deref> From<OutboundV1Channel<SP>> for Channel<SP>
Expand Down Expand Up @@ -2194,8 +2229,8 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
}

pub fn funding_tx_constructed<L: Deref>(
mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
) -> Result<(FundedChannel<SP>, msgs::CommitmentSigned, Option<Event>), (PendingV2Channel<SP>, ChannelError)>
&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
where
L::Target: Logger
{
Expand All @@ -2211,7 +2246,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
(
"Multiple outputs matched the expected script and value".to_owned(),
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
))).map_err(|e| (self, e));
)));
}
output_index = Some(idx as u16);
}
Expand All @@ -2223,7 +2258,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
(
"No output matched the funding script_pubkey".to_owned(),
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
))).map_err(|e| (self, e));
)));
};
self.context.channel_transaction_parameters.funding_outpoint = Some(outpoint);
self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
Expand All @@ -2237,8 +2272,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
},
Err(err) => {
self.context.channel_transaction_parameters.funding_outpoint = None;
return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })))
.map_err(|e| (self, e));
return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })));
},
};

Expand All @@ -2249,10 +2283,10 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
false,
"Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found",
);
return Err((self, ChannelError::Close((
return Err(ChannelError::Close((
"V2 channel rejected due to sender error".into(),
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
))));
)));
}
None
} else {
Expand All @@ -2274,37 +2308,19 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
false,
"We don't support users providing inputs but somehow we had more than zero inputs",
);
return Err((self, ChannelError::Close((
return Err(ChannelError::Close((
"V2 channel rejected due to sender error".into(),
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
))));
)));
};

self.context.channel_state = ChannelState::FundingNegotiated;

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

match self.unfunded_context.holder_commitment_point {
Some(holder_commitment_point) => {
let funded_chan = FundedChannel {
funding: self.funding,
context: self.context,
interactive_tx_signing_session: Some(signing_session),
holder_commitment_point,
is_v2_established: true,
};
Ok((funded_chan, commitment_signed, funding_ready_for_sig_event))
},
None => {
Err(ChannelError::close(
format!(
"Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
self.context.channel_id(),
)))
.map_err(|e| (self, e))
},
}
Ok((commitment_signed, funding_ready_for_sig_event))
}
}

Expand Down Expand Up @@ -9511,6 +9527,8 @@ pub(super) struct PendingV2Channel<SP: Deref> where SP::Target: SignerProvider {
pub dual_funding_context: DualFundingChannelContext,
/// The current interactive transaction construction session under negotiation.
pub interactive_tx_constructor: Option<InteractiveTxConstructor>,
/// The signing session created after `tx_complete` handling
pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
}

impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
Expand Down Expand Up @@ -9576,6 +9594,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
our_funding_inputs: funding_inputs,
},
interactive_tx_constructor: None,
interactive_tx_signing_session: None,
};
Ok(chan)
}
Expand Down Expand Up @@ -9747,6 +9766,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
context,
dual_funding_context,
interactive_tx_constructor,
interactive_tx_signing_session: None,
unfunded_context,
})
}
Expand Down
32 changes: 13 additions & 19 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8950,14 +8950,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
let peer_state = &mut *peer_state_lock;
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
let funding_txo = chan.context.get_funding_txo();

if chan.interactive_tx_signing_session.is_some() {
let monitor = try_channel_entry!(
self, peer_state, chan.commitment_signed_initial_v2(msg, best_block, &self.signer_provider, &&logger),
chan_entry);
let chan = chan_entry.get_mut();
let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
let funding_txo = chan.context().get_funding_txo();
let (monitor_opt, monitor_update_opt) = try_channel_entry!(
self, peer_state, chan.commitment_signed(msg, best_block, &self.signer_provider, &&logger),
chan_entry);

if let Some(chan) = chan.as_funded_mut() {
if let Some(monitor) = monitor_opt {
let monitor_res = self.chain_monitor.watch_channel(monitor.channel_id(), monitor);
if let Ok(persist_state) = monitor_res {
handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
Expand All @@ -8972,19 +8973,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
)
)), chan_entry)
}
} else {
let monitor_update_opt = try_channel_entry!(
self, peer_state, chan.commitment_signed(msg, &&logger), chan_entry);
if let Some(monitor_update) = monitor_update_opt {
handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
peer_state, per_peer_state, chan);
}
} else if let Some(monitor_update) = monitor_update_opt {
handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
peer_state, per_peer_state, chan);
}
Ok(())
} else {
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
"Got a commitment_signed message for an unfunded channel!".into())), chan_entry);
}
Ok(())
},
hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
}
Expand Down
Loading