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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce FundingTransactionReadyForSignatures event #3889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 53 additions & 0 deletions lightning/src/events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1582,6 +1582,48 @@ pub enum Event {
/// onion messages.
peer_node_id: PublicKey,
},
/// Indicates that a channel funding transaction constructed interactively is ready to be
/// signed. This event will only be triggered if at least one input was contributed.
///
/// The transaction contains all inputs provided by both parties along with the channel's funding
/// output and a change output if applicable.
///
/// No part of the transaction should be changed before signing as the content of the transaction
/// has already been negotiated with the counterparty.
///
/// Each signature MUST use the `SIGHASH_ALL` flag to avoid invalidation of the initial commitment and
/// hence possible loss of funds.
///
/// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) signed
/// funding transaction.
///
/// Generated in [`ChannelManager`] message handling.
///
/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
/// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed
FundingTransactionReadyForSigning {
/// The channel_id of the channel which you'll need to pass back into
/// [`ChannelManager::funding_transaction_signed`].
///
/// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed
channel_id: ChannelId,
/// The counterparty's node_id, which you'll need to pass back into
/// [`ChannelManager::funding_transaction_signed`].
///
/// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed
counterparty_node_id: PublicKey,
/// The `user_channel_id` value passed in for outbound channels, or for inbound channels if
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
/// `user_channel_id` will be randomized for inbound channels.
///
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
user_channel_id: u128,
/// The unsigned transaction to be signed and passed back to
/// [`ChannelManager::funding_transaction_signed`].
///
/// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed
unsigned_transaction: Transaction,
},
}

impl Writeable for Event {
Expand Down Expand Up @@ -2012,6 +2054,13 @@ impl Writeable for Event {
(8, former_temporary_channel_id, required),
});
},
&Event::FundingTransactionReadyForSigning { .. } => {
45u8.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
Copy link
Contributor

Choose a reason for hiding this comment

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

exchange*

// `ChannelManager::funding_transaction_signed` and ALWAYS before we send a `tx_signatures`
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems wrong, we immediately send the initial commitment_signed as soon as funding_tx_constructed. We may want to wait to emit FundingTransactionReadyForSigning until we exchange commitment_signed though. Doing so may help unify the logic for when we need to regenerate the event upon channel_reestablish.

Copy link
Contributor Author

@dunxen dunxen Jul 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, yeah this comment is outdated. And agreed, I'll move the event emit to after commitment_signed exchange.

},
// 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
// data via `write_tlv_fields`.
Expand Down Expand Up @@ -2583,6 +2632,10 @@ impl MaybeReadable for Event {
former_temporary_channel_id: former_temporary_channel_id.0.unwrap(),
}))
},
45u8 => {
// Value 45 is used for `Event::FundingTransactionReadyForSigning`.
Ok(None)
},
// Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
// Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
// reads.
Expand Down
74 changes: 46 additions & 28 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use bitcoin::constants::ChainHash;
use bitcoin::script::{Builder, Script, ScriptBuf, WScriptHash};
use bitcoin::sighash::EcdsaSighashType;
use bitcoin::transaction::{Transaction, TxIn, TxOut};
use bitcoin::Weight;
use bitcoin::{Weight, Witness};

use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::hashes::sha256::Hash as Sha256;
Expand All @@ -35,7 +35,7 @@ use crate::chain::channelmonitor::{
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::chain::BestBlock;
use crate::events::bump_transaction::BASE_INPUT_WEIGHT;
use crate::events::{ClosureReason, Event};
use crate::events::ClosureReason;
use crate::ln::chan_utils;
#[cfg(splicing)]
use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT;
Expand Down Expand Up @@ -1763,7 +1763,7 @@ where

pub fn funding_tx_constructed<L: Deref>(
&mut self, signing_session: InteractiveTxSigningSession, logger: &L,
) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
) -> Result<(msgs::CommitmentSigned, Option<Transaction>), ChannelError>
where
L::Target: Logger,
{
Expand Down Expand Up @@ -2925,7 +2925,7 @@ where
#[rustfmt::skip]
pub fn funding_tx_constructed<L: Deref>(
&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
) -> Result<(msgs::CommitmentSigned, Option<Transaction>), ChannelError>
where
L::Target: Logger
{
Expand Down Expand Up @@ -2967,7 +2967,7 @@ where
},
};

let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 {
let funding_tx_opt = 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 @@ -2981,28 +2981,7 @@ where
}
None
} else {
// TODO(dual_funding): Send event for signing if we've contributed funds.
// Inform the user that SIGHASH_ALL must be used for all signatures when contributing
// inputs/signatures.
// Also warn the user that we don't do anything to prevent the counterparty from
// providing non-standard witnesses which will prevent the funding transaction from
// confirming. This warning must appear in doc comments wherever the user is contributing
// funds, whether they are initiator or acceptor.
//
// The following warning can be used when the APIs allowing contributing inputs become available:
// <div class="warning">
// WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which
// will prevent the funding transaction from being relayed on the bitcoin network and hence being
// confirmed.
// </div>
debug_assert!(
false,
"We don't support users providing inputs but somehow we had more than zero inputs",
);
return Err(ChannelError::Close((
"V2 channel rejected due to sender error".into(),
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
)));
Some(signing_session.unsigned_tx().build_unsigned_tx())
};

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

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

Expand Down Expand Up @@ -7640,6 +7619,45 @@ where
}
}

fn verify_interactive_tx_signatures(&mut self, _witnesses: &Vec<Witness>) {
if let Some(ref mut _signing_session) = self.interactive_tx_signing_session {
// Check that sighash_all was used:
// TODO(dual_funding): Check sig for sighash
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason this isn't done yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah we're actually unable to do this reliably, especially if inputs were spend P2WSH. The public comments warn the client to ensure it is SIGHASH_ALL (or SIGHASH_DEFAULT/SIGHASH_ALL) if spending a P2TR UTXO.

These TODOs need to be removed.

}
}

pub fn funding_transaction_signed<L: Deref>(
&mut self, witnesses: Vec<Witness>, logger: &L,
) -> Result<Option<msgs::TxSignatures>, APIError>
where
L::Target: Logger,
{
self.verify_interactive_tx_signatures(&witnesses);
if let Some(ref mut signing_session) = self.interactive_tx_signing_session {
let logger = WithChannelContext::from(logger, &self.context, None);
if let Some(holder_tx_signatures) = signing_session
.provide_holder_witnesses(self.context.channel_id, witnesses)
.map_err(|err| APIError::APIMisuseError { err })?
{
if self.is_awaiting_initial_mon_persist() {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the splicing case, we'd want to check is_monitor_update_in_progress only

log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
self.context.monitor_pending_tx_signatures = Some(holder_tx_signatures);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not store the signatures here as well and just keep them in the session instead

return Ok(None);
}
return Ok(Some(holder_tx_signatures));
} else {
return Ok(None);
}
} else {
return Err(APIError::APIMisuseError {
err: format!(
"Channel with id {} not expecting funding signatures",
self.context.channel_id
),
});
}
}

#[rustfmt::skip]
pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<Transaction>, Option<msgs::TxSignatures>), ChannelError>
where L::Target: Logger
Expand Down
92 changes: 89 additions & 3 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5911,6 +5911,86 @@ where
result
}

/// Handles a signed funding transaction generated by interactive transaction construction and
/// provided by the client.
///
/// Do NOT broadcast the funding transaction yourself. When we have safely received our
/// counterparty's signature(s) the funding transaction will automatically be broadcast via the
/// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
///
/// `SIGHASH_ALL` MUST be used for all signatures when providing signatures.
///
/// <div class="warning">
/// WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which
/// will prevent the funding transaction from being relayed on the bitcoin network and hence being
/// confirmed.
/// </div>
///
/// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect
/// `counterparty_node_id` is provided.
///
/// Returns [`APIMisuseError`] when a channel is not in a state where it is expecting funding
/// signatures.
///
/// [`ChannelUnavailable`]: APIError::ChannelUnavailable
/// [`APIMisuseError`]: APIError::APIMisuseError
pub fn funding_transaction_signed(
&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, transaction: Transaction,
) -> Result<(), APIError> {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let witnesses: Vec<_> = transaction
.input
.into_iter()
.filter_map(|input| if input.witness.is_empty() { None } else { Some(input.witness) })
.collect();
Comment on lines +5941 to +5945
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't have a strong opinion here, but seems we can avoid this by passing the Transaction through and only collecting witnesses when we are ready to construct TxSignatures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, yeah will look at changing this after fixups.


let per_peer_state = self.per_peer_state.read().unwrap();
let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
APIError::ChannelUnavailable {
err: format!(
"Can't find a peer matching the passed counterparty node_id {}",
counterparty_node_id
),
}
})?;

let mut peer_state_lock = peer_state_mutex.lock().unwrap();
let peer_state = &mut *peer_state_lock;

match peer_state.channel_by_id.get_mut(channel_id) {
Some(channel) => match channel.as_funded_mut() {
Some(chan) => {
if let Some(tx_signatures) =
chan.funding_transaction_signed(witnesses, &self.logger)?
{
peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures {
node_id: *counterparty_node_id,
msg: tx_signatures,
});
}
},
None => {
return Err(APIError::APIMisuseError {
err: format!(
"Channel with id {} not expecting funding signatures",
channel_id
),
})
},
},
None => {
return Err(APIError::ChannelUnavailable {
err: format!(
"Channel with id {} not found for the passed counterparty node_id {}",
channel_id, counterparty_node_id
),
})
},
}

Ok(())
}

/// Atomically applies partial updates to the [`ChannelConfig`] of the given channels.
///
/// Once the updates are applied, each eligible channel (advertised with a known short channel
Expand Down Expand Up @@ -9037,13 +9117,19 @@ 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_ready_for_sig_event_opt) = chan_entry
let (commitment_signed, funding_tx_opt) = 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(funding_ready_for_sig_event) = funding_ready_for_sig_event_opt {
if let Some(unsigned_transaction) = funding_tx_opt {
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push_back((funding_ready_for_sig_event, None));
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,
Expand Down
17 changes: 13 additions & 4 deletions lightning/src/ln/interactivetxs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,14 @@ impl InteractiveTxSigningSession {
/// unsigned transaction.
pub fn provide_holder_witnesses(
&mut self, channel_id: ChannelId, witnesses: Vec<Witness>,
) -> Result<(), ()> {
if self.local_inputs_count() != witnesses.len() {
return Err(());
) -> Result<Option<TxSignatures>, String> {
let local_inputs_count = self.local_inputs_count();
if local_inputs_count != witnesses.len() {
return Err(format!(
"Provided witness count of {} does not match required count for {} inputs",
witnesses.len(),
local_inputs_count
));
}

self.unsigned_tx.add_local_witnesses(witnesses.clone());
Expand All @@ -409,7 +414,11 @@ impl InteractiveTxSigningSession {
shared_input_signature: None,
});

Ok(())
if self.holder_sends_tx_signatures_first && self.has_received_commitment_signed {
Ok(self.holder_tx_signatures.clone())
} else {
Ok(None)
}
}

pub fn remote_inputs_count(&self) -> usize {
Expand Down
Loading