Skip to content

Introduce FundingTransactionReadyForSignatures event #3889

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 1 addition & 4 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2161,10 +2161,7 @@ impl Writeable for Event {
},
&Event::FundingTransactionReadyForSigning { .. } => {
49u8.write(writer)?;
// We never write out FundingTransactionReadyForSigning events as, upon disconnection, peers
// drop any V2-established/spliced channels which have not yet exchanged the initial `commitment_signed`.
// We only exhange the initial `commitment_signed` after the client calls
// `ChannelManager::funding_transaction_signed` and ALWAYS before we send a `tx_signatures`
// We never write out FundingTransactionReadyForSigning events as they will be regenerated necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

when necessary*

},
// Note that, going forward, all new events must only write data inside of
// `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
Expand Down
31 changes: 16 additions & 15 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ where

pub fn funding_tx_constructed<L: Deref>(
&mut self, signing_session: InteractiveTxSigningSession, logger: &L,
) -> Result<(msgs::CommitmentSigned, Option<Transaction>), ChannelError>
) -> Result<msgs::CommitmentSigned, ChannelError>
where
L::Target: Logger,
{
Expand All @@ -1786,7 +1786,7 @@ where
#[rustfmt::skip]
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>
) -> Result<(Option<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>>, Option<ChannelMonitorUpdate>, Option<Transaction>), ChannelError>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's time we make this an enum?

where
L::Target: Logger
{
Expand All @@ -1813,7 +1813,7 @@ where
pending_splice: None,
};
let res = funded_channel.commitment_signed_initial_v2(msg, best_block, signer_provider, logger)
.map(|monitor| (Some(monitor), None))
.map(|(monitor, funding_tx_opt)| (Some(monitor), None, funding_tx_opt))
// TODO: Change to `inspect_err` when MSRV is high enough.
.map_err(|err| {
// We always expect a `ChannelError` close.
Expand All @@ -1840,15 +1840,15 @@ where
let res = if has_negotiated_pending_splice && !session_received_commitment_signed {
funded_channel
.splice_initial_commitment_signed(msg, logger)
.map(|monitor_update_opt| (None, monitor_update_opt))
.map(|monitor_update_opt| (None, monitor_update_opt, None))
Copy link
Contributor

Choose a reason for hiding this comment

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

We will also need to return the unsigned transaction here for splices.

} else {
funded_channel.commitment_signed(msg, logger)
.map(|monitor_update_opt| (None, monitor_update_opt))
.map(|monitor_update_opt| (None, monitor_update_opt, None))
};

#[cfg(not(splicing))]
let res = funded_channel.commitment_signed(msg, logger)
.map(|monitor_update_opt| (None, monitor_update_opt));
.map(|monitor_update_opt| (None, monitor_update_opt, None));

self.phase = ChannelPhase::Funded(funded_channel);
res
Expand Down Expand Up @@ -2916,7 +2916,7 @@ where
#[rustfmt::skip]
pub fn funding_tx_constructed<L: Deref>(
&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
) -> Result<(msgs::CommitmentSigned, Option<Transaction>), ChannelError>
) -> Result<msgs::CommitmentSigned, ChannelError>
where
L::Target: Logger
{
Expand Down Expand Up @@ -2954,7 +2954,8 @@ where
},
};

let funding_tx_opt = if signing_session.local_inputs_count() == 0 {
// Check that we have the expected number of local inputs
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() {
debug_assert!(
Expand All @@ -2965,10 +2966,7 @@ where
let reason = ClosureReason::ProcessingError { err: msg.to_owned() };
return Err(ChannelError::Close((msg.to_owned(), reason)));
}
None
} else {
Some(signing_session.unsigned_tx().build_unsigned_tx())
};
}

let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
channel_state.set_interactive_signing();
Expand All @@ -2978,7 +2976,7 @@ where
self.interactive_tx_constructor.take();
self.interactive_tx_signing_session = Some(signing_session);

Ok((commitment_signed, funding_tx_opt))
Ok(commitment_signed)
}
}

Expand Down Expand Up @@ -6628,7 +6626,7 @@ where
#[rustfmt::skip]
pub fn commitment_signed_initial_v2<L: Deref>(
&mut self, msg: &msgs::CommitmentSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
) -> Result<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, ChannelError>
) -> Result<(ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, Option<Transaction>), ChannelError>
where L::Target: Logger
{
if !self.context.channel_state.is_interactive_signing()
Expand Down Expand Up @@ -6657,8 +6655,11 @@ where
// so they'll be sent as soon as that's done.
self.context.monitor_pending_tx_signatures = Some(tx_signatures);
}
// Only build the unsigned transaction for signing if there are any holder inputs to actually sign
let funding_tx_opt = self.interactive_tx_signing_session.as_ref().and_then(|session|
session.local_inputs_count().gt(&0).then_some(session.unsigned_tx().build_unsigned_tx()));

Ok(channel_monitor)
Ok((channel_monitor, funding_tx_opt))
}

/// Handles an incoming `commitment_signed` message for the first commitment transaction of the
Expand Down
24 changes: 12 additions & 12 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9088,20 +9088,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
peer_state.pending_msg_events.push(msg_send_event);
};
if let Some(signing_session) = signing_session_opt {
let (commitment_signed, funding_tx_opt) = chan_entry
let commitment_signed = chan_entry
.get_mut()
.funding_tx_constructed(signing_session, &self.logger)
.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
if let Some(unsigned_transaction) = funding_tx_opt {
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push_back((
Event::FundingTransactionReadyForSigning {
unsigned_transaction,
counterparty_node_id,
channel_id: msg.channel_id,
user_channel_id: chan_entry.get().context().get_user_id(),
}, None));
}
peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs {
node_id: counterparty_node_id,
channel_id: msg.channel_id,
Expand Down Expand Up @@ -9631,11 +9621,21 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
let chan = chan_entry.get_mut();
let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
let funding_txo = chan.funding().get_funding_txo();
let (monitor_opt, monitor_update_opt) = try_channel_entry!(
let (monitor_opt, monitor_update_opt, funding_tx_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(unsigned_transaction) = funding_tx_opt {
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push_back((
Event::FundingTransactionReadyForSigning {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll also need to generate this on reestablish when we have already received commitment_signed but not tx_signatures.

unsigned_transaction,
counterparty_node_id: *counterparty_node_id,
channel_id: msg.channel_id,
user_channel_id: chan.context.get_user_id(),
}, None));
}
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 {
Expand Down