Skip to content

Contribute funding inputs on accepting dual-funded channel #3735

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft
60 changes: 60 additions & 0 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,55 @@ pub enum Event {
/// onion messages.
peer_node_id: PublicKey,
},
/// Indicates that a funding transaction constructed via interactive transaction construction for a
/// channel is ready to be signed by the client. This event will only be triggered
/// if at least one input was contributed by the holder and needs to be signed.
///
/// 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,
// TODO(dual_funding): Enable links when methods are implemented
/// The `user_channel_id` value passed in to `ChannelManager::create_dual_funded_channel` for outbound
/// channels, or to [`ChannelManager::accept_inbound_channel`] or `ChannelManager::accept_inbound_channel_with_contribution`
/// for inbound channels if [`UserConfig::manually_accept_inbound_channels`] config flag is set to true.
/// Otherwise `user_channel_id` will be randomized for an inbound channel.
/// This may be zero for objects serialized with LDK versions prior to 0.0.113.
///
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
// [`ChannelManager::create_dual_funded_channel`]: crate::ln::channelmanager::ChannelManager::create_dual_funded_channel
// [`ChannelManager::accept_inbound_channel_with_contribution`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel_with_contribution
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 @@ -1996,6 +2045,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
// `ChannelManager::funding_transaction_signed` and ALWAYS before we send a `tx_signatures`
},
// 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 @@ -2560,6 +2616,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
73 changes: 48 additions & 25 deletions lightning/src/ln/channel.rs
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 Down Expand Up @@ -2800,7 +2800,7 @@ where
},
};

let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 {
let funding_ready_for_sig_event_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 @@ -2814,28 +2814,12 @@ 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(Event::FundingTransactionReadyForSigning {
channel_id: self.context.channel_id,
counterparty_node_id: self.context.counterparty_node_id,
user_channel_id: self.context.user_id,
unsigned_transaction: signing_session.unsigned_tx().build_unsigned_tx(),
})
};

let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
Expand All @@ -2846,7 +2830,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_ready_for_sig_event_opt))
}
}

Expand Down Expand Up @@ -7104,6 +7088,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
}
}

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() {
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.

Thoughts on just tracking this as an Option<()> and we get the message from the signing 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
70 changes: 70 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5863,6 +5863,76 @@ 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>
pub fn funding_transaction_signed(
&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, transaction: Transaction,
) -> Result<(), APIError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to acquire the PersistenceNotifierGuard throughout this method

let witnesses: Vec<_> = transaction
.input
.into_iter()
.filter_map(|input| if input.witness.is_empty() { None } else { Some(input.witness) })
.collect();

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
17 changes: 13 additions & 4 deletions lightning/src/ln/interactivetxs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,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 @@ -420,7 +425,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