From 585493a3e1654e57d8be79b7d405d1290abe7ea1 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 4 Mar 2022 21:24:39 +0000 Subject: [PATCH 01/10] Handle cases where a channel is in use w/o an SCID in ChannelManager In the next few commits we add support for 0conf channels, allowing us to have an active channel with HTLC and other updates flying prior to having an SCID available. This would break several assumptions made in `ChannelManager`, which we address here by looking at SCID aliases in addition to SCIDs. --- lightning/src/ln/channel.rs | 7 ++++--- lightning/src/ln/channelmanager.rs | 9 +++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 43032c51a3c..a0b19920fca 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3565,9 +3565,10 @@ impl Channel { // We will never broadcast the funding transaction when we're in MonitorUpdateFailed (and // we assume the user never directly broadcasts the funding transaction and waits for us to - // do it). Thus, we can only ever hit monitor_pending_funding_locked when we're an inbound - // channel which failed to persist the monitor on funding_created, and we got the funding - // transaction confirmed before the monitor was persisted. + // do it). Thus, we can only ever hit monitor_pending_funding_locked when we're + // * an inbound channel that failed to persist the monitor on funding_created and we got + // the funding transaction confirmed before the monitor was persisted, or + // * a 0-conf channel and intended to send the funding_locked before any broadcast at all. let funding_locked = if self.monitor_pending_funding_locked { assert!(!self.is_outbound(), "Funding transaction broadcast by the local client before it should have - LDK didn't do it!"); self.monitor_pending_funding_locked = false; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8725059c360..239b45ebf5a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -93,6 +93,8 @@ use util::crypto::sign; pub(super) enum PendingHTLCRouting { Forward { onion_packet: msgs::OnionPacket, + /// The SCID from the onion that we should forward to. This could be a "real" SCID, an + /// outbound SCID alias, or a phantom node SCID. short_channel_id: u64, // This should be NonZero eventually when we bump MSRV }, Receive { @@ -136,6 +138,8 @@ pub(super) enum HTLCForwardInfo { // `process_pending_htlc_forwards()` for constructing the // `HTLCSource::PreviousHopData` for failed and forwarded // HTLCs. + // + // Note that this may be an outbound SCID alias for the associated channel. prev_short_channel_id: u64, prev_htlc_id: u64, prev_funding_outpoint: OutPoint, @@ -149,6 +153,7 @@ pub(super) enum HTLCForwardInfo { /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, Hash, PartialEq, Eq)] pub(crate) struct HTLCPreviousHopData { + // Note that this may be an outbound SCID alias for the associated channel. short_channel_id: u64, htlc_id: u64, incoming_packet_shared_secret: [u8; 32], @@ -1398,7 +1403,7 @@ macro_rules! handle_chan_restoration_locked { let res = loop { let forwards: Vec<(PendingHTLCInfo, u64)> = $pending_forwards; // Force type-checking to resolve if !forwards.is_empty() { - htlc_forwards = Some(($channel_entry.get().get_short_channel_id().expect("We can't have pending forwards before funding confirmation"), + htlc_forwards = Some(($channel_entry.get().get_short_channel_id().unwrap_or($channel_entry.get().outbound_scid_alias()), $channel_entry.get().get_funding_txo().unwrap(), forwards)); } @@ -4676,7 +4681,7 @@ impl ChannelMana break Ok((raa_updates.accepted_htlcs, raa_updates.failed_htlcs, raa_updates.finalized_claimed_htlcs, chan.get().get_short_channel_id() - .expect("RAA should only work on a short-id-available channel"), + .unwrap_or(chan.get().outbound_scid_alias()), chan.get().get_funding_txo().unwrap())) }, hash_map::Entry::Vacant(_) => break Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id)) From 168b3a51ae024e3abf73112c2acf9f60523c5674 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 13 May 2022 18:30:12 +0000 Subject: [PATCH 02/10] Define a crate-public constant for max fake SCID in blocks offset --- lightning/src/util/scid_utils.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lightning/src/util/scid_utils.rs b/lightning/src/util/scid_utils.rs index 8552358c35a..676c303bfa8 100644 --- a/lightning/src/util/scid_utils.rs +++ b/lightning/src/util/scid_utils.rs @@ -79,6 +79,10 @@ pub(crate) mod fake_scid { const MAX_NAMESPACES: u8 = 8; // We allocate 3 bits for the namespace identifier. const NAMESPACE_ID_BITMASK: u8 = 0b111; + const BLOCKS_PER_MONTH: u32 = 144 /* blocks per day */ * 30 /* days per month */; + pub(crate) const MAX_SCID_BLOCKS_FROM_NOW: u32 = BLOCKS_PER_MONTH; + + /// Fake scids are divided into namespaces, with each namespace having its own identifier between /// [0..7]. This allows us to identify what namespace a fake scid corresponds to upon HTLC /// receipt, and handle the HTLC accordingly. The namespace identifier is encrypted when encoded @@ -100,7 +104,6 @@ pub(crate) mod fake_scid { // Ensure we haven't created a namespace that doesn't fit into the 3 bits we've allocated for // namespaces. assert!((*self as u8) < MAX_NAMESPACES); - const BLOCKS_PER_MONTH: u32 = 144 /* blocks per day */ * 30 /* days per month */; let rand_bytes = keys_manager.get_secure_random_bytes(); let segwit_activation_height = segwit_activation_height(genesis_hash); @@ -109,7 +112,7 @@ pub(crate) mod fake_scid { // We want to ensure that this fake channel won't conflict with any transactions we haven't // seen yet, in case `highest_seen_blockheight` is updated before we get full information // about transactions confirmed in the given block. - blocks_since_segwit_activation = blocks_since_segwit_activation.saturating_sub(BLOCKS_PER_MONTH); + blocks_since_segwit_activation = blocks_since_segwit_activation.saturating_sub(MAX_SCID_BLOCKS_FROM_NOW); let rand_for_height = u32::from_be_bytes(rand_bytes[..4].try_into().unwrap()); let fake_scid_height = segwit_activation_height + rand_for_height % (blocks_since_segwit_activation + 1); From 35cd39da15f9bf8d55cb3249c6d32cd7756cd21f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 1 Feb 2022 21:57:01 +0000 Subject: [PATCH 03/10] Lock outbound channels at 0conf if the peer indicates support for it If our peer sets a minimum depth of 0, and we're set to trusting ourselves to not double-spend our own funding transactions, send a funding_locked message immediately after funding signed. Note that some special care has to be taken around the `channel_state` values - `ChannelFunded` no longer implies the funding transaction is confirmed on-chain. Thus, for example, the should-we-re-broadcast logic has to now accept `channel_state` values greater than `ChannelFunded` as indicating we may still need to re-broadcast our funding tranasction, unless `minimum_depth` is greater than 0. Further note that this starts writing `Channel` objects with a `MIN_SERIALIZATION_VERSION` of 2. Thus, LDK versions prior to 0.0.99 (July 2021) will now refuse to read serialized Channels/ChannelManagers. --- lightning/src/ln/channel.rs | 68 ++++++++++++++--------- lightning/src/ln/channelmanager.rs | 43 ++++++++++---- lightning/src/ln/priv_short_conf_tests.rs | 34 ++++++++++++ lightning/src/util/config.rs | 19 +++++++ 4 files changed, 127 insertions(+), 37 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a0b19920fca..f221ee6227c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -710,6 +710,11 @@ pub(super) struct Channel { // Our counterparty can offer us SCID aliases which they will map to this channel when routing // outbound payments. These can be used in invoice route hints to avoid explicitly revealing // the channel's funding UTXO. + // + // We also use this when sending our peer a channel_update that isn't to be broadcasted + // publicly - allowing them to re-use their map of SCID -> channel for channel_update -> + // associated channel mapping. + // // We only bother storing the most recent SCID alias at any time, though our counterparty has // to store all of them. latest_inbound_scid_alias: Option, @@ -1987,12 +1992,6 @@ impl Channel { if msg.minimum_depth > peer_limits.max_minimum_depth { return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", peer_limits.max_minimum_depth, msg.minimum_depth))); } - if msg.minimum_depth == 0 { - // Note that if this changes we should update the serialization minimum version to - // indicate to older clients that they don't understand some features of the current - // channel. - return Err(ChannelError::Close("Minimum confirmation depth must be at least 1".to_owned())); - } if let Some(ty) = &msg.channel_type { if *ty != self.channel_type { @@ -2029,7 +2028,12 @@ impl Channel { self.counterparty_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis); self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat; self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs; - self.minimum_depth = Some(msg.minimum_depth); + + if peer_limits.trust_own_funding_0conf { + self.minimum_depth = Some(msg.minimum_depth); + } else { + self.minimum_depth = Some(cmp::max(1, msg.minimum_depth)); + } let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.funding_pubkey, @@ -2169,7 +2173,7 @@ impl Channel { /// Handles a funding_signed message from the remote end. /// If this call is successful, broadcast the funding transaction (and not before!) - pub fn funding_signed(&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, logger: &L) -> Result<(ChannelMonitor, Transaction), ChannelError> where L::Target: Logger { + pub fn funding_signed(&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, logger: &L) -> Result<(ChannelMonitor, Transaction, Option), ChannelError> where L::Target: Logger { if !self.is_outbound() { return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())); } @@ -2238,7 +2242,7 @@ impl Channel { log_info!(logger, "Received funding_signed from peer for channel {}", log_bytes!(self.channel_id())); - Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap())) + Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap(), self.check_get_funding_locked(0))) } /// Handles a funding_locked message from our peer. If we've already sent our funding_locked @@ -3540,12 +3544,13 @@ impl Channel { /// monitor update failure must *not* have been sent to the remote end, and must instead /// have been dropped. They will be regenerated when monitor_updating_restored is called. pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, - mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, + resend_funding_locked: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, mut pending_finalized_claimed_htlcs: Vec ) { self.monitor_pending_revoke_and_ack |= resend_raa; self.monitor_pending_commitment_signed |= resend_commitment; + self.monitor_pending_funding_locked |= resend_funding_locked; self.monitor_pending_forwards.append(&mut pending_forwards); self.monitor_pending_failures.append(&mut pending_fails); self.monitor_pending_finalized_fulfills.append(&mut pending_finalized_claimed_htlcs); @@ -3559,9 +3564,18 @@ impl Channel { assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32); self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32); - let funding_broadcastable = if self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.is_outbound() { - self.funding_transaction.take() - } else { None }; + // If we're past (or at) the FundingSent stage on an outbound channel, try to + // (re-)broadcast the funding transaction as we may have declined to broadcast it when we + // first received the funding_signed. + let mut funding_broadcastable = + if self.is_outbound() && self.channel_state & !MULTI_STATE_FLAGS >= ChannelState::FundingSent as u32 { + self.funding_transaction.take() + } else { None }; + // That said, if the funding transaction is already confirmed (ie we're active with a + // minimum_depth over 0) don't bother re-broadcasting the confirmed funding tx. + if self.channel_state & !MULTI_STATE_FLAGS >= ChannelState::ChannelFunded as u32 && self.minimum_depth != Some(0) { + funding_broadcastable = None; + } // We will never broadcast the funding transaction when we're in MonitorUpdateFailed (and // we assume the user never directly broadcasts the funding transaction and waits for us to @@ -3570,7 +3584,8 @@ impl Channel { // the funding transaction confirmed before the monitor was persisted, or // * a 0-conf channel and intended to send the funding_locked before any broadcast at all. let funding_locked = if self.monitor_pending_funding_locked { - assert!(!self.is_outbound(), "Funding transaction broadcast by the local client before it should have - LDK didn't do it!"); + assert!(!self.is_outbound() || self.minimum_depth == Some(0), + "Funding transaction broadcast by the local client before it should have - LDK didn't do it!"); self.monitor_pending_funding_locked = false; let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); Some(msgs::FundingLocked { @@ -4552,6 +4567,11 @@ impl Channel { self.channel_state >= ChannelState::FundingSent as u32 } + /// Returns true if our funding_locked has been sent + pub fn is_our_funding_locked(&self) -> bool { + (self.channel_state & ChannelState::OurFundingLocked as u32) != 0 || self.channel_state >= ChannelState::ChannelFunded as u32 + } + /// Returns true if our peer has either initiated or agreed to shut down the channel. pub fn received_shutdown(&self) -> bool { (self.channel_state & ChannelState::RemoteShutdownSent as u32) != 0 @@ -4582,7 +4602,7 @@ impl Channel { } fn check_get_funding_locked(&mut self, height: u32) -> Option { - if self.funding_tx_confirmation_height == 0 { + if self.funding_tx_confirmation_height == 0 && self.minimum_depth != Some(0) { return None; } @@ -4759,9 +4779,9 @@ impl Channel { // close the channel and hope we can get the latest state on chain (because presumably // the funding transaction is at least still in the mempool of most nodes). // - // Note that ideally we wouldn't force-close if we see *any* reorg on a 1-conf channel, - // but not doing so may lead to the `ChannelManager::short_to_id` map being - // inconsistent, so we currently have to. + // Note that ideally we wouldn't force-close if we see *any* reorg on a 1-conf or + // 0-conf channel, but not doing so may lead to the `ChannelManager::short_to_id` map + // being inconsistent, so we currently have to. if funding_tx_confirmations == 0 && self.funding_tx_confirmed_in.is_some() { let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations); @@ -5620,7 +5640,7 @@ impl Channel { } const SERIALIZATION_VERSION: u8 = 2; -const MIN_SERIALIZATION_VERSION: u8 = 1; +const MIN_SERIALIZATION_VERSION: u8 = 2; impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,; (0, FailRelay), @@ -5685,12 +5705,10 @@ impl Writeable for Channel { self.user_id.write(writer)?; - // Write out the old serialization for the config object. This is read by version-1 - // deserializers, but we will read the version in the TLV at the end instead. - self.config.forwarding_fee_proportional_millionths.write(writer)?; - self.config.cltv_expiry_delta.write(writer)?; - self.config.announced_channel.write(writer)?; - self.config.commit_upfront_shutdown_pubkey.write(writer)?; + // Version 1 deserializers expected to read parts of the config object here. Version 2 + // deserializers (0.0.99) now read config through TLVs, and as we now require them for + // `minimum_depth` we simply write dummy values here. + writer.write_all(&[0; 8])?; self.channel_id.write(writer)?; (self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 239b45ebf5a..5f65a66ce91 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1286,7 +1286,7 @@ macro_rules! remove_channel { } macro_rules! handle_monitor_err { - ($self: ident, $err: expr, $short_to_id: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr, $chan_id: expr) => { + ($self: ident, $err: expr, $short_to_id: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $resend_funding_locked: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr, $chan_id: expr) => { match $err { ChannelMonitorUpdateErr::PermanentFailure => { log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateErr::PermanentFailure", log_bytes!($chan_id[..])); @@ -1324,13 +1324,13 @@ macro_rules! handle_monitor_err { if !$resend_raa { debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst || !$resend_commitment); } - $chan.monitor_update_failed($resend_raa, $resend_commitment, $failed_forwards, $failed_fails, $failed_finalized_fulfills); + $chan.monitor_update_failed($resend_raa, $resend_commitment, $resend_funding_locked, $failed_forwards, $failed_fails, $failed_finalized_fulfills); (Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore("Failed to update ChannelMonitor".to_owned()), *$chan_id)), false) }, } }; - ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr) => { { - let (res, drop) = handle_monitor_err!($self, $err, $channel_state.short_to_id, $entry.get_mut(), $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, $failed_finalized_fulfills, $entry.key()); + ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $resend_funding_locked: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr) => { { + let (res, drop) = handle_monitor_err!($self, $err, $channel_state.short_to_id, $entry.get_mut(), $action_type, $resend_raa, $resend_commitment, $resend_funding_locked, $failed_forwards, $failed_fails, $failed_finalized_fulfills, $entry.key()); if drop { $entry.remove_entry(); } @@ -1338,16 +1338,19 @@ macro_rules! handle_monitor_err { } }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $chan_id: expr, COMMITMENT_UPDATE_ONLY) => { { debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst); - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, true, Vec::new(), Vec::new(), Vec::new(), $chan_id) + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, true, false, Vec::new(), Vec::new(), Vec::new(), $chan_id) } }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $chan_id: expr, NO_UPDATE) => { - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, false, Vec::new(), Vec::new(), Vec::new(), $chan_id) + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, false, false, Vec::new(), Vec::new(), Vec::new(), $chan_id) + }; + ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_funding_locked: expr, OPTIONALLY_RESEND_FUNDING_LOCKED) => { + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, false, $resend_funding_locked, Vec::new(), Vec::new(), Vec::new()) }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => { - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, Vec::new(), Vec::new(), Vec::new()) + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, false, Vec::new(), Vec::new(), Vec::new()) }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => { - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new()) + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, false, $failed_forwards, $failed_fails, Vec::new()) }; } @@ -4268,7 +4271,7 @@ impl ChannelMana // hasn't persisted to disk yet - we can't lose money on a transaction that we haven't // accepted payment from yet. We do, however, need to wait to send our funding_locked // until we have persisted our monitor. - chan.monitor_update_failed(false, false, Vec::new(), Vec::new(), Vec::new()); + chan.monitor_update_failed(false, false, false, Vec::new(), Vec::new(), Vec::new()); }, } } @@ -4299,12 +4302,12 @@ impl ChannelMana if chan.get().get_counterparty_node_id() != *counterparty_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id)); } - let (monitor, funding_tx) = match chan.get_mut().funding_signed(&msg, best_block, &self.logger) { + let (monitor, funding_tx, funding_locked) = match chan.get_mut().funding_signed(&msg, best_block, &self.logger) { Ok(update) => update, Err(e) => try_chan_entry!(self, Err(e), channel_state, chan), }; if let Err(e) = self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor) { - let mut res = handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, false, false); + let mut res = handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, funding_locked.is_some(), OPTIONALLY_RESEND_FUNDING_LOCKED); if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res { // We weren't able to watch the channel to begin with, so no updates should be made on // it. Previously, full_stack_target found an (unreachable) panic when the @@ -4315,6 +4318,9 @@ impl ChannelMana } return res } + if let Some(msg) = funding_locked { + send_funding_locked!(channel_state.short_to_id, channel_state.pending_msg_events, chan.get(), msg); + } funding_tx }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id)) @@ -4665,7 +4671,7 @@ impl ChannelMana } else { if let Err(e) = handle_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, - raa_updates.commitment_update.is_some(), + raa_updates.commitment_update.is_some(), false, raa_updates.accepted_htlcs, raa_updates.failed_htlcs, raa_updates.finalized_claimed_htlcs) { break Err(e); @@ -5520,6 +5526,19 @@ where } } } + if channel.is_our_funding_locked() { + if let Some(real_scid) = channel.get_short_channel_id() { + // If we sent a 0conf funding_locked, and now have an SCID, we add it + // to the short_to_id map here. Note that we check whether we can relay + // using the real SCID at relay-time (i.e. enforce option_scid_alias + // then), and if the funding tx is ever un-confirmed we force-close the + // channel, ensuring short_to_id is always consistent. + let scid_insert = short_to_id.insert(real_scid, channel.channel_id()); + assert!(scid_insert.is_none() || scid_insert.unwrap() == channel.channel_id(), + "SCIDs should never collide - ensure you weren't behind by a full {} blocks when creating channels", + fake_scid::MAX_SCID_BLOCKS_FROM_NOW); + } + } } else if let Err(reason) = res { update_maps_on_chan_removal!(self, short_to_id, channel); // It looks like our counterparty went on-chain or funding transaction was diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 47e2fb33e3c..48dab6a22fd 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -564,3 +564,37 @@ fn test_scid_alias_returned() { PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap()) .blamed_chan_closed(false).expected_htlc_error_data(0x1000|12, &err_data)); } + +#[test] +fn test_simple_0conf_channel() { + // If our peer tells us they will accept our channel with 0 confs, and we funded the channel, + // we should trust the funding won't be double-spent (assuming `trust_own_funding_0conf` is + // set)! + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); + let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); + let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + accept_channel.minimum_depth = 0; + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel); + + let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); + nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); + let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); + check_added_monitors!(nodes[1], 1); + let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed); + check_added_monitors!(nodes[0], 1); + + let as_funding_locked = get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked); +} diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 3868d29aab4..4d1c7c99aad 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -159,6 +159,24 @@ pub struct ChannelHandshakeLimits { /// /// Default value: 144, or roughly one day and only applies to outbound channels. pub max_minimum_depth: u32, + /// Whether we implicitly trust funding transactions generated by us for our own outbound + /// channels to not be double-spent. + /// + /// If this is set, we assume that our own funding transactions are *never* double-spent, and + /// thus we can trust them without any confirmations. This is generally a reasonable + /// assumption, given we're the only ones who could ever double-spend it (assuming we have sole + /// control of the signing keys). + /// + /// You may wish to un-set this if you allow the user to (or do in an automated fashion) + /// double-spend the funding transaction to RBF with an alternative channel open. + /// + /// This only applies if our counterparty set their confirmations-required value to 0, and we + /// always trust our own funding transaction at 1 confirmation irrespective of this value. + /// Thus, this effectively acts as a `min_minimum_depth`, with the only possible values being + /// `true` (0) and `false` (1). + /// + /// Default value: true + pub trust_own_funding_0conf: bool, /// Set to force an incoming channel to match our announced channel preference in /// [`ChannelConfig::announced_channel`]. /// @@ -187,6 +205,7 @@ impl Default for ChannelHandshakeLimits { min_max_htlc_value_in_flight_msat: 0, max_channel_reserve_satoshis: ::max_value(), min_max_accepted_htlcs: 0, + trust_own_funding_0conf: true, max_minimum_depth: 144, force_announced_channel_preference: true, their_to_self_delay: MAX_LOCAL_BREAKDOWN_TIMEOUT, From 9569bfe820a4c2053b3842ddce0080c9c9dd9a44 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 2 Feb 2022 23:01:05 +0000 Subject: [PATCH 04/10] Add API and signaling to accept incoming channels at 0conf --- lightning/src/ln/channel.rs | 8 +++++- lightning/src/ln/channelmanager.rs | 34 ++++++++++++++++++++--- lightning/src/ln/priv_short_conf_tests.rs | 16 +++++++++-- lightning/src/util/config.rs | 8 ++++++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f221ee6227c..a49444f09d0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1312,7 +1312,7 @@ impl Channel { counterparty_htlc_minimum_msat: msg.htlc_minimum_msat, holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: msg.max_accepted_htlcs, - minimum_depth: Some(config.own_channel_config.minimum_depth), + minimum_depth: Some(cmp::max(config.own_channel_config.minimum_depth, 1)), counterparty_forwarding_info: None, @@ -4878,6 +4878,12 @@ impl Channel { self.inbound_awaiting_accept } + /// Sets this channel to accepting 0conf, must be done before `get_accept_channel` + pub fn set_0conf(&mut self) { + assert!(self.inbound_awaiting_accept); + self.minimum_depth = Some(0); + } + /// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which /// should be sent back to the counterparty node. /// diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5f65a66ce91..acc1310fa71 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4119,20 +4119,45 @@ impl ChannelMana } } - /// Called to accept a request to open a channel after [`Event::OpenChannelRequest`] has been - /// triggered. + /// Accepts a request to open a channel after a [`Event::OpenChannelRequest`]. /// /// The `temporary_channel_id` parameter indicates which inbound channel should be accepted, /// and the `counterparty_node_id` parameter is the id of the peer which has requested to open /// the channel. /// - /// For inbound channels, the `user_channel_id` parameter will be provided back in + /// The `user_channel_id` parameter will be provided back in /// [`Event::ChannelClosed::user_channel_id`] to allow tracking of which events correspond - /// with which `accept_inbound_channel` call. + /// with which `accept_inbound_channel`/`accept_inbound_channel_from_trusted_peer_0conf` call. /// /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u64) -> Result<(), APIError> { + self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id) + } + + /// Accepts a request to open a channel after a [`events::Event::OpenChannelRequest`], treating + /// it as confirmed immediately. + /// + /// The `user_channel_id` parameter will be provided back in + /// [`Event::ChannelClosed::user_channel_id`] to allow tracking of which events correspond + /// with which `accept_inbound_channel`/`accept_inbound_channel_from_trusted_peer_0conf` call. + /// + /// Unlike [`ChannelManager::accept_inbound_channel`], this method accepts the incoming channel + /// and (if the counterparty agrees), enables forwarding of payments immediately. + /// + /// This fully trusts that the counterparty has honestly and correctly constructed the funding + /// transaction and blindly assumes that it will eventually confirm. + /// + /// If it does not confirm before we decide to close the channel, or if the funding transaction + /// does not pay to the correct script the correct amount, *you will lose funds*. + /// + /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest + /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id + pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u64) -> Result<(), APIError> { + self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id) + } + + fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u64) -> Result<(), APIError> { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); let mut channel_state_lock = self.channel_state.lock().unwrap(); @@ -4145,6 +4170,7 @@ impl ChannelMana if *counterparty_node_id != channel.get().get_counterparty_node_id() { return Err(APIError::APIMisuseError { err: "The passed counterparty_node_id doesn't match the channel's counterparty node_id".to_owned() }); } + if accept_0conf { channel.get_mut().set_0conf(); } channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { node_id: channel.get().get_counterparty_node_id(), msg: channel.get_mut().accept_inbound_channel(user_channel_id), diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 48dab6a22fd..a7bc3e5656d 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -573,15 +573,27 @@ fn test_simple_0conf_channel() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + }, + _ => panic!("Unexpected event"), + }; + let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); - accept_channel.minimum_depth = 0; + assert_eq!(accept_channel.minimum_depth, 0); nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel); let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 4d1c7c99aad..bdd222e31c5 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -22,7 +22,15 @@ pub struct ChannelHandshakeConfig { /// Applied only for inbound channels (see ChannelHandshakeLimits::max_minimum_depth for the /// equivalent limit applied to outbound channels). /// + /// A lower-bound of 1 is applied, requiring all channels to have a confirmed commitment + /// transaction before operation. If you wish to accept channels with zero confirmations, see + /// [`UserConfig::manually_accept_inbound_channels`] and + /// [`ChannelManager::accept_inbound_channel_from_trusted_peer_0conf`]. + /// /// Default value: 6. + /// + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`ChannelManager::accept_inbound_channel_from_trusted_peer_0conf`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel_from_trusted_peer_0conf pub minimum_depth: u32, /// Set to the number of blocks we require our counterparty to wait to claim their money (ie /// the number of blocks we have to punish our counterparty if they broadcast a revoked From ff9e4572b605968efbc8e0a035459a1f6f6d265d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 10 Feb 2022 00:09:42 +0000 Subject: [PATCH 05/10] Send funding_locked immediately for inbound channels with 0conf --- lightning/src/ln/channel.rs | 6 ++-- lightning/src/ln/channelmanager.rs | 13 ++++++-- lightning/src/ln/priv_short_conf_tests.rs | 40 ++++++++++++++++++++--- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a49444f09d0..70a13f9ebe1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2093,7 +2093,7 @@ impl Channel { &self.get_counterparty_pubkeys().funding_pubkey } - pub fn funding_created(&mut self, msg: &msgs::FundingCreated, best_block: BestBlock, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor), ChannelError> where L::Target: Logger { + pub fn funding_created(&mut self, msg: &msgs::FundingCreated, best_block: BestBlock, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor, Option), ChannelError> where L::Target: Logger { if self.is_outbound() { return Err(ChannelError::Close("Received funding_created for an outbound channel?".to_owned())); } @@ -2168,7 +2168,7 @@ impl Channel { Ok((msgs::FundingSigned { channel_id: self.channel_id, signature - }, channel_monitor)) + }, channel_monitor, self.check_get_funding_locked(0))) } /// Handles a funding_signed message from the remote end. @@ -6692,7 +6692,7 @@ mod tests { }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; let funding_created_msg = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap(); - let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&logger).unwrap(); + let (funding_signed_msg, _, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&logger).unwrap(); // Node B --> Node A: funding signed let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&logger); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index acc1310fa71..8193ba443c2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2293,6 +2293,9 @@ impl ChannelMana action: msgs::ErrorAction::IgnoreError }); } + if chan.get_short_channel_id().is_none() { + return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}); + } log_trace!(self.logger, "Attempting to generate broadcast channel update for channel {}", log_bytes!(chan.channel_id())); self.get_channel_update_for_unicast(chan) } @@ -2304,7 +2307,7 @@ impl ChannelMana /// May be called with channel_state already locked! fn get_channel_update_for_unicast(&self, chan: &Channel) -> Result { log_trace!(self.logger, "Attempting to generate channel update for channel {}", log_bytes!(chan.channel_id())); - let short_channel_id = match chan.get_short_channel_id() { + let short_channel_id = match chan.get_short_channel_id().or(chan.latest_inbound_scid_alias()) { None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}), Some(id) => id, }; @@ -4262,7 +4265,7 @@ impl ChannelMana } fn internal_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> { - let ((funding_msg, monitor), mut chan) = { + let ((funding_msg, monitor, mut funding_locked), mut chan) = { let best_block = *self.best_block.read().unwrap(); let mut channel_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_lock; @@ -4297,7 +4300,8 @@ impl ChannelMana // hasn't persisted to disk yet - we can't lose money on a transaction that we haven't // accepted payment from yet. We do, however, need to wait to send our funding_locked // until we have persisted our monitor. - chan.monitor_update_failed(false, false, false, Vec::new(), Vec::new(), Vec::new()); + chan.monitor_update_failed(false, false, funding_locked.is_some(), Vec::new(), Vec::new(), Vec::new()); + funding_locked = None; // Don't send the funding_locked now }, } } @@ -4312,6 +4316,9 @@ impl ChannelMana node_id: counterparty_node_id.clone(), msg: funding_msg, }); + if let Some(msg) = funding_locked { + send_funding_locked!(channel_state.short_to_id, channel_state.pending_msg_events, chan, msg); + } e.insert(chan); } } diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index a7bc3e5656d..ed11b993fc7 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -570,6 +570,9 @@ fn test_simple_0conf_channel() { // If our peer tells us they will accept our channel with 0 confs, and we funded the channel, // we should trust the funding won't be double-spent (assuming `trust_own_funding_0conf` is // set)! + // Further, if we `accept_inbound_channel_from_trusted_peer_0conf`, funding locked messages + // should fly immediately and the channel should be available for use as soon as they are + // received. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -602,11 +605,38 @@ fn test_simple_0conf_channel() { nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); check_added_monitors!(nodes[1], 1); - let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); - - nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed); - check_added_monitors!(nodes[0], 1); + let bs_signed_locked = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(bs_signed_locked.len(), 2); + let as_funding_locked; + match &bs_signed_locked[0] { + MessageSendEvent::SendFundingSigned { node_id, msg } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg); + check_added_monitors!(nodes[0], 1); + + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0], tx); + + as_funding_locked = get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()); + } + _ => panic!("Unexpected event"), + } + match &bs_signed_locked[1] { + MessageSendEvent::SendFundingLocked { node_id, msg } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &msg); + } + _ => panic!("Unexpected event"), + } - let as_funding_locked = get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()); nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked); + + let as_channel_update = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id()); + let bs_channel_update = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_channel_update); + nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_channel_update); + + assert_eq!(nodes[0].node.list_usable_channels().len(), 1); + assert_eq!(nodes[1].node.list_usable_channels().len(), 1); } From 26288e301440565b433958bf9bac89f019ed35f2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 1 Apr 2022 01:36:38 +0000 Subject: [PATCH 06/10] Expose outbound SCID alias in `ChannelDetails` and use in routing This supports routing outbound over 0-conf channels by utilizing the outbound SCID alias that we assign to all channels to refer to the selected channel when routing. --- fuzz/src/router.rs | 1 + lightning/src/ln/channelmanager.rs | 28 +++++++++++++++++++++++ lightning/src/ln/priv_short_conf_tests.rs | 2 ++ lightning/src/routing/router.rs | 8 ++++--- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 786bfa3e589..80ea1f1bc73 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -222,6 +222,7 @@ pub fn do_test(data: &[u8], out: Out) { channel_type: None, short_channel_id: Some(scid), inbound_scid_alias: None, + outbound_scid_alias: None, channel_value_satoshis: capacity, user_channel_id: 0, inbound_capacity_msat: 0, unspendable_punishment_reserve: None, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8193ba443c2..8053f9c32cc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -968,9 +968,25 @@ pub struct ChannelDetails { /// Note that if [`inbound_scid_alias`] is set, it must be used for invoices and inbound /// payments instead of this. See [`get_inbound_payment_scid`]. /// + /// For channels with [`confirmations_required`] set to `Some(0)`, [`outbound_scid_alias`] may + /// be used in place of this in outbound routes. See [`get_outbound_payment_scid`]. + /// /// [`inbound_scid_alias`]: Self::inbound_scid_alias + /// [`outbound_scid_alias`]: Self::outbound_scid_alias /// [`get_inbound_payment_scid`]: Self::get_inbound_payment_scid + /// [`get_outbound_payment_scid`]: Self::get_outbound_payment_scid + /// [`confirmations_required`]: Self::confirmations_required pub short_channel_id: Option, + /// An optional [`short_channel_id`] alias for this channel, randomly generated by us and + /// usable in place of [`short_channel_id`] to reference the channel in outbound routes when + /// the channel has not yet been confirmed (as long as [`confirmations_required`] is + /// `Some(0)`). + /// + /// This will be `None` as long as the channel is not available for routing outbound payments. + /// + /// [`short_channel_id`]: Self::short_channel_id + /// [`confirmations_required`]: Self::confirmations_required + pub outbound_scid_alias: Option, /// An optional [`short_channel_id`] alias for this channel, randomly generated by our /// counterparty and usable in place of [`short_channel_id`] in invoice route hints. Our /// counterparty will recognize the alias provided here in place of the [`short_channel_id`] @@ -1088,6 +1104,16 @@ impl ChannelDetails { pub fn get_inbound_payment_scid(&self) -> Option { self.inbound_scid_alias.or(self.short_channel_id) } + + /// Gets the current SCID which should be used to identify this channel for outbound payments. + /// This should be used in [`Route`]s to describe the first hop or in other contexts where + /// we're sending or forwarding a payment outbound over this channel. + /// + /// This is either the [`ChannelDetails::short_channel_id`], if set, or the + /// [`ChannelDetails::outbound_scid_alias`]. See those for more information. + pub fn get_outbound_payment_scid(&self) -> Option { + self.short_channel_id.or(self.outbound_scid_alias) + } } /// If a payment fails to send, it can be in one of several states. This enum is returned as the @@ -1714,6 +1740,7 @@ impl ChannelMana // `have_received_message` indicates that type negotiation has completed. channel_type: if channel.have_received_message() { Some(channel.get_channel_type().clone()) } else { None }, short_channel_id: channel.get_short_channel_id(), + outbound_scid_alias: if channel.is_usable() { Some(channel.outbound_scid_alias()) } else { None }, inbound_scid_alias: channel.latest_inbound_scid_alias(), channel_value_satoshis: channel.get_value_satoshis(), unspendable_punishment_reserve: to_self_reserve_satoshis, @@ -5984,6 +6011,7 @@ impl_writeable_tlv_based!(ChannelDetails, { (2, channel_id, required), (3, channel_type, option), (4, counterparty, required), + (5, outbound_scid_alias, option), (6, funding_txo, option), (8, short_channel_id, option), (10, channel_value_satoshis, required), diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index ed11b993fc7..24a0e478761 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -639,4 +639,6 @@ fn test_simple_0conf_channel() { assert_eq!(nodes[0].node.list_usable_channels().len(), 1); assert_eq!(nodes[1].node.list_usable_channels().len(), 1); + + send_payment(&nodes[0], &[&nodes[1]], 100_000); } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index dc094b9b6e0..c02da4d705f 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -368,7 +368,7 @@ enum CandidateRouteHop<'a> { impl<'a> CandidateRouteHop<'a> { fn short_channel_id(&self) -> u64 { match self { - CandidateRouteHop::FirstHop { details } => details.short_channel_id.unwrap(), + CandidateRouteHop::FirstHop { details } => details.get_outbound_payment_scid().unwrap(), CandidateRouteHop::PublicHop { short_channel_id, .. } => *short_channel_id, CandidateRouteHop::PrivateHop { hint } => hint.short_channel_id, } @@ -777,7 +777,7 @@ where L::Target: Logger { HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 }); if let Some(hops) = first_hops { for chan in hops { - if chan.short_channel_id.is_none() { + if chan.get_outbound_payment_scid().is_none() { panic!("first_hops should be filled in with usable channels, not pending ones"); } if chan.counterparty.node_id == *our_node_pubkey { @@ -1331,7 +1331,7 @@ where L::Target: Logger { let mut features_set = false; if let Some(first_channels) = first_hop_targets.get(&ordered_hops.last().unwrap().0.node_id) { for details in first_channels { - if details.short_channel_id.unwrap() == ordered_hops.last().unwrap().0.candidate.short_channel_id() { + if details.get_outbound_payment_scid().unwrap() == ordered_hops.last().unwrap().0.candidate.short_channel_id() { ordered_hops.last_mut().unwrap().1 = details.counterparty.features.to_context(); features_set = true; break; @@ -1742,6 +1742,7 @@ mod tests { funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }), channel_type: None, short_channel_id, + outbound_scid_alias: None, inbound_scid_alias: None, channel_value_satoshis: 0, user_channel_id: 0, @@ -5474,6 +5475,7 @@ mod benches { channel_type: None, short_channel_id: Some(1), inbound_scid_alias: None, + outbound_scid_alias: None, channel_value_satoshis: 10_000_000, user_channel_id: 0, balance_msat: 10_000_000, From 8be97f0389d7a399d091431cc6a480199c099722 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 1 Apr 2022 01:34:38 +0000 Subject: [PATCH 07/10] Add a new test for 0conf-with-monitor-update-failures This tests a few cases of monitor failure updates that were broken in earlier versions of the 0conf patchset. --- lightning/src/ln/priv_short_conf_tests.rs | 150 +++++++++++++++++++++- lightning/src/util/test_utils.rs | 5 + 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 24a0e478761..c6525681749 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -11,7 +11,7 @@ //! other behavior that exists only on private channels or with a semi-trusted counterparty (eg //! LSP). -use chain::Watch; +use chain::{ChannelMonitorUpdateErr, Watch}; use chain::channelmonitor::ChannelMonitor; use chain::keysinterface::{Recipient, KeysInterface}; use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, MIN_CLTV_EXPIRY_DELTA}; @@ -642,3 +642,151 @@ fn test_simple_0conf_channel() { send_payment(&nodes[0], &[&nodes[1]], 100_000); } + +#[test] +fn test_0conf_channel_with_async_monitor() { + // Test that we properly send out funding_locked in (both inbound- and outbound-) zero-conf + // channels if ChannelMonitor updates return a `TemporaryFailure` during the initial channel + // negotiation. + + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(chan_config), None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0, InitFeatures::known(), InitFeatures::known()); + + chan_config.channel_options.announced_channel = false; + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(chan_config)).unwrap(); + let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + }, + _ => panic!("Unexpected event"), + }; + + let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + assert_eq!(accept_channel.minimum_depth, 0); + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel); + + let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); + nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); + let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + + chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure)); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); + check_added_monitors!(nodes[1], 1); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + + let channel_id = funding_output.to_channel_id(); + nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); + + let bs_signed_locked = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(bs_signed_locked.len(), 2); + chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure)); + + match &bs_signed_locked[0] { + MessageSendEvent::SendFundingSigned { node_id, msg } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg); + check_added_monitors!(nodes[0], 1); + } + _ => panic!("Unexpected event"), + } + match &bs_signed_locked[1] { + MessageSendEvent::SendFundingLocked { node_id, msg } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &msg); + } + _ => panic!("Unexpected event"), + } + + assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty()); + + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + nodes[0].chain_monitor.complete_sole_pending_chan_update(&channel_id); + let as_locked_update = nodes[0].node.get_and_clear_pending_msg_events(); + + // Note that the funding transaction is actually released when + // get_and_clear_pending_msg_events, above, checks for monitor events. + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0], tx); + + match &as_locked_update[0] { + MessageSendEvent::SendFundingLocked { node_id, msg } => { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &msg); + } + _ => panic!("Unexpected event"), + } + let bs_channel_update = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id()); + + let as_channel_update = match &as_locked_update[1] { + MessageSendEvent::SendChannelUpdate { node_id, msg } => { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + msg.clone() + } + _ => panic!("Unexpected event"), + }; + + chanmon_cfgs[0].persister.set_update_ret(Ok(())); + chanmon_cfgs[1].persister.set_update_ret(Ok(())); + + nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_channel_update); + nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_channel_update); + + assert_eq!(nodes[0].node.list_usable_channels().len(), 1); + assert_eq!(nodes[1].node.list_usable_channels().len(), 2); + + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + // Now that we have useful channels, try sending a payment where the we hit a temporary monitor + // failure before we've ever confirmed the funding transaction. This previously caused a panic. + let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 1_000_000); + + nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap(); + check_added_monitors!(nodes[0], 1); + + let as_send = SendEvent::from_node(&nodes[0]); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_send.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_send.commitment_msg); + check_added_monitors!(nodes[1], 1); + + let (bs_raa, bs_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed); + check_added_monitors!(nodes[0], 1); + + chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure)); + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id())); + check_added_monitors!(nodes[1], 1); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + chanmon_cfgs[1].persister.set_update_ret(Ok(())); + let (outpoint, _, latest_update) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&bs_raa.channel_id).unwrap().clone(); + nodes[1].chain_monitor.chain_monitor.channel_monitor_updated(outpoint, latest_update).unwrap(); + check_added_monitors!(nodes[1], 0); + expect_pending_htlcs_forwardable!(nodes[1]); + check_added_monitors!(nodes[1], 1); + + let bs_send = SendEvent::from_node(&nodes[1]); + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &bs_send.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[1], bs_send.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[2]); + expect_payment_received!(nodes[2], payment_hash, payment_secret, 1_000_000); + claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); + + confirm_transaction(&nodes[0], &tx); + confirm_transaction(&nodes[1], &tx); + + send_payment(&nodes[0], &[&nodes[1]], 100_000); +} diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 3682a0e8f0a..1d4eccd81e6 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -116,6 +116,11 @@ impl<'a> TestChainMonitor<'a> { expect_channel_force_closed: Mutex::new(None), } } + + pub fn complete_sole_pending_chan_update(&self, channel_id: &[u8; 32]) { + let (outpoint, _, latest_update) = self.latest_monitor_update_id.lock().unwrap().get(channel_id).unwrap().clone(); + self.chain_monitor.channel_monitor_updated(outpoint, latest_update).unwrap(); + } } impl<'a> chain::Watch for TestChainMonitor<'a> { fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor) -> Result<(), chain::ChannelMonitorUpdateErr> { From 7ed7a7d22e6b7bb047db5a7fbbd1f19036b7e06f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 1 Apr 2022 21:29:59 +0000 Subject: [PATCH 08/10] Correctly handle sending announcement sigs on public 0conf channels --- lightning/src/ln/channel.rs | 7 +- lightning/src/ln/functional_test_utils.rs | 15 +- lightning/src/ln/functional_tests.rs | 8 +- lightning/src/ln/priv_short_conf_tests.rs | 173 ++++++++++++++++------ 4 files changed, 149 insertions(+), 54 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 70a13f9ebe1..8362f6d73b7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4657,12 +4657,11 @@ impl Channel { pub fn transactions_confirmed(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L) -> Result<(Option, Option), ClosureReason> where L::Target: Logger { - let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS); if let Some(funding_txo) = self.get_funding_txo() { for &(index_in_block, tx) in txdata.iter() { - // If we haven't yet sent a funding_locked, but are in FundingSent (ignoring - // whether they've sent a funding_locked or not), check if we should send one. - if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 { + // Check if the transaction is the expected funding transaction, and if it is, + // check that it pays the right amount to the right script. + if self.funding_tx_confirmation_height == 0 { if tx.txid() == funding_txo.txid { let txo_idx = funding_txo.index as usize; if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() || diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c0e33e5b93a..e54874b9e2e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -21,6 +21,7 @@ use ln::features::{InitFeatures, InvoiceFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler}; use util::enforcing_trait_impls::EnforcingSigner; +use util::scid_utils; use util::test_utils; use util::test_utils::{panicking, TestChainMonitor}; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; @@ -48,9 +49,13 @@ pub const CHAN_CONFIRM_DEPTH: u32 = 10; /// Mine the given transaction in the next block and then mine CHAN_CONFIRM_DEPTH - 1 blocks on /// top, giving the given transaction CHAN_CONFIRM_DEPTH confirmations. -pub fn confirm_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) { - confirm_transaction_at(node, tx, node.best_block_info().1 + 1); +/// +/// Returns the SCID a channel confirmed in the given transaction will have, assuming the funding +/// output is the 1st output in the transaction. +pub fn confirm_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) -> u64 { + let scid = confirm_transaction_at(node, tx, node.best_block_info().1 + 1); connect_blocks(node, CHAN_CONFIRM_DEPTH - 1); + scid } /// Mine a signle block containing the given transaction pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) { @@ -59,7 +64,10 @@ pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transac } /// Mine the given transaction at the given height, mining blocks as required to build to that /// height -pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction, conf_height: u32) { +/// +/// Returns the SCID a channel confirmed in the given transaction will have, assuming the funding +/// output is the 1st output in the transaction. +pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction, conf_height: u32) -> u64 { let first_connect_height = node.best_block_info().1 + 1; assert!(first_connect_height <= conf_height); if conf_height > first_connect_height { @@ -74,6 +82,7 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T } block.txdata.push(tx.clone()); connect_block(node, &block); + scid_utils::scid_from_parts(conf_height as u64, block.txdata.len() as u64 - 1, 0).unwrap() } /// The possible ways we may notify a ChannelManager of a new block diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 48b4b07c7d7..53cadecb063 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9222,7 +9222,11 @@ fn test_duplicate_chan_id() { let funding_created = { let mut a_channel_lock = nodes[0].node.channel_state.lock().unwrap(); - let mut as_chan = a_channel_lock.by_id.get_mut(&open_chan_2_msg.temporary_channel_id).unwrap(); + // Once we call `get_outbound_funding_created` the channel has a duplicate channel_id as + // another channel in the ChannelManager - an invalid state. Thus, we'd panic later when we + // try to create another channel. Instead, we drop the channel entirely here (leaving the + // channelmanager in a possibly nonsense state instead). + let mut as_chan = a_channel_lock.by_id.remove(&open_chan_2_msg.temporary_channel_id).unwrap(); let logger = test_utils::TestLogger::new(); as_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap() }; @@ -9260,7 +9264,7 @@ fn test_duplicate_chan_id() { let events_4 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_4.len(), 0); assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); - assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].txid(), funding_output.txid); + assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0], tx); let (funding_locked, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx); let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked); diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index c6525681749..3a8b9a4fbe3 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -565,80 +565,87 @@ fn test_scid_alias_returned() { .blamed_chan_closed(false).expected_htlc_error_data(0x1000|12, &err_data)); } -#[test] -fn test_simple_0conf_channel() { - // If our peer tells us they will accept our channel with 0 confs, and we funded the channel, - // we should trust the funding won't be double-spent (assuming `trust_own_funding_0conf` is - // set)! - // Further, if we `accept_inbound_channel_from_trusted_peer_0conf`, funding locked messages - // should fly immediately and the channel should be available for use as soon as they are - // received. - - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let mut chan_config = test_default_channel_config(); - chan_config.manually_accept_inbound_channels = true; +// Receiver must have been initialized with manually_accept_inbound_channels set to true. +fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>, initiator_config: Option) -> bitcoin::Transaction { + initiator.node.create_channel(receiver.node.get_our_node_id(), 100_000, 10_001, 42, initiator_config).unwrap(); + let open_channel = get_event_msg!(initiator, MessageSendEvent::SendOpenChannel, receiver.node.get_our_node_id()); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); - let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - - nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); - let events = nodes[1].node.get_and_clear_pending_events(); + receiver.node.handle_open_channel(&initiator.node.get_our_node_id(), InitFeatures::known(), &open_channel); + let events = receiver.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { Event::OpenChannelRequest { temporary_channel_id, .. } => { - nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0).unwrap(); + receiver.node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &initiator.node.get_our_node_id(), 0).unwrap(); }, _ => panic!("Unexpected event"), }; - let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + let mut accept_channel = get_event_msg!(receiver, MessageSendEvent::SendAcceptChannel, initiator.node.get_our_node_id()); assert_eq!(accept_channel.minimum_depth, 0); - nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel); + initiator.node.handle_accept_channel(&receiver.node.get_our_node_id(), InitFeatures::known(), &accept_channel); - let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); - nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); - let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver.node.get_our_node_id(), 100_000, 42); + initiator.node.funding_transaction_generated(&temporary_channel_id, &receiver.node.get_our_node_id(), tx.clone()).unwrap(); + let funding_created = get_event_msg!(initiator, MessageSendEvent::SendFundingCreated, receiver.node.get_our_node_id()); - nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); - check_added_monitors!(nodes[1], 1); - let bs_signed_locked = nodes[1].node.get_and_clear_pending_msg_events(); + receiver.node.handle_funding_created(&initiator.node.get_our_node_id(), &funding_created); + check_added_monitors!(receiver, 1); + let bs_signed_locked = receiver.node.get_and_clear_pending_msg_events(); assert_eq!(bs_signed_locked.len(), 2); let as_funding_locked; match &bs_signed_locked[0] { MessageSendEvent::SendFundingSigned { node_id, msg } => { - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg); - check_added_monitors!(nodes[0], 1); + assert_eq!(*node_id, initiator.node.get_our_node_id()); + initiator.node.handle_funding_signed(&receiver.node.get_our_node_id(), &msg); + check_added_monitors!(initiator, 1); - assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); - assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0], tx); + assert_eq!(initiator.tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1); + assert_eq!(initiator.tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0], tx); - as_funding_locked = get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()); + as_funding_locked = get_event_msg!(initiator, MessageSendEvent::SendFundingLocked, receiver.node.get_our_node_id()); } _ => panic!("Unexpected event"), } match &bs_signed_locked[1] { MessageSendEvent::SendFundingLocked { node_id, msg } => { - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &msg); + assert_eq!(*node_id, initiator.node.get_our_node_id()); + initiator.node.handle_funding_locked(&receiver.node.get_our_node_id(), &msg); } _ => panic!("Unexpected event"), } - nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked); + receiver.node.handle_funding_locked(&initiator.node.get_our_node_id(), &as_funding_locked); - let as_channel_update = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id()); - let bs_channel_update = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id()); + let as_channel_update = get_event_msg!(initiator, MessageSendEvent::SendChannelUpdate, receiver.node.get_our_node_id()); + let bs_channel_update = get_event_msg!(receiver, MessageSendEvent::SendChannelUpdate, initiator.node.get_our_node_id()); - nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_channel_update); - nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_channel_update); + initiator.node.handle_channel_update(&receiver.node.get_our_node_id(), &bs_channel_update); + receiver.node.handle_channel_update(&initiator.node.get_our_node_id(), &as_channel_update); - assert_eq!(nodes[0].node.list_usable_channels().len(), 1); - assert_eq!(nodes[1].node.list_usable_channels().len(), 1); + assert_eq!(initiator.node.list_usable_channels().len(), 1); + assert_eq!(receiver.node.list_usable_channels().len(), 1); + + tx +} + +#[test] +fn test_simple_0conf_channel() { + // If our peer tells us they will accept our channel with 0 confs, and we funded the channel, + // we should trust the funding won't be double-spent (assuming `trust_own_funding_0conf` is + // set)! + // Further, if we `accept_inbound_channel_from_trusted_peer_0conf`, funding locked messages + // should fly immediately and the channel should be available for use as soon as they are + // received. + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + open_zero_conf_channel(&nodes[0], &nodes[1], None); send_payment(&nodes[0], &[&nodes[1]], 100_000); } @@ -790,3 +797,79 @@ fn test_0conf_channel_with_async_monitor() { send_payment(&nodes[0], &[&nodes[1]], 100_000); } + +#[test] +fn test_0conf_close_no_early_chan_update() { + // Tests that even with a public channel 0conf channel, we don't generate a channel_update on + // closing. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // This is the default but we force it on anyway + chan_config.channel_options.announced_channel = true; + open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); + + // We can use the channel immediately, but won't generate a channel_update until we get confs + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + nodes[0].node.force_close_all_channels(); + check_added_monitors!(nodes[0], 1); + check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed); + let _ = get_err_msg!(nodes[0], nodes[1].node.get_our_node_id()); +} + +#[test] +fn test_public_0conf_channel() { + // Tests that we will announce a public channel (after confirmation) even if its 0conf. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // This is the default but we force it on anyway + chan_config.channel_options.announced_channel = true; + let tx = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); + + // We can use the channel immediately, but we can't announce it until we get 6+ confirmations + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + let scid = confirm_transaction(&nodes[0], &tx); + let as_announcement_sigs = get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id()); + assert_eq!(confirm_transaction(&nodes[1], &tx), scid); + let bs_announcement_sigs = get_event_msg!(nodes[1], MessageSendEvent::SendAnnouncementSignatures, nodes[0].node.get_our_node_id()); + + nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &as_announcement_sigs); + nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs); + + let bs_announcement = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(bs_announcement.len(), 1); + let announcement; + let bs_update; + match bs_announcement[0] { + MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => { + announcement = msg.clone(); + bs_update = update_msg.clone(); + }, + _ => panic!("Unexpected event"), + }; + + let as_announcement = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(as_announcement.len(), 1); + match as_announcement[0] { + MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => { + assert!(announcement == *msg); + assert_eq!(update_msg.contents.short_channel_id, scid); + assert_eq!(update_msg.contents.short_channel_id, announcement.contents.short_channel_id); + assert_eq!(update_msg.contents.short_channel_id, bs_update.contents.short_channel_id); + }, + _ => panic!("Unexpected event"), + }; +} From 86ad2527bdb73bfc3f2d78b1ee56ba3bc0b6c64b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 2 May 2022 15:23:52 +0000 Subject: [PATCH 09/10] Add test of 0conf channels getting the funding transaction reorg'd In a previous version of the 0-conf code we did not correctly handle 0-conf channels getting the funding transaction reorg'd out (and the real SCID possibly changing on us). --- lightning/src/ln/priv_short_conf_tests.rs | 51 ++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 3a8b9a4fbe3..f4f52a8baf4 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -22,7 +22,7 @@ use ln::msgs; use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ChannelUpdate}; use ln::wire::Encode; use util::enforcing_trait_impls::EnforcingSigner; -use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use util::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider}; use util::config::UserConfig; use util::ser::{Writeable, ReadableArgs}; use util::test_utils; @@ -873,3 +873,52 @@ fn test_public_0conf_channel() { _ => panic!("Unexpected event"), }; } + +#[test] +fn test_0conf_channel_reorg() { + // If we accept a 0conf channel, which is then confirmed, but then changes SCID in a reorg, we + // have to make sure we handle this correctly (or, currently, just force-close the channel). + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // This is the default but we force it on anyway + chan_config.channel_options.announced_channel = true; + let tx = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); + + // We can use the channel immediately, but we can't announce it until we get 6+ confirmations + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + mine_transaction(&nodes[0], &tx); + mine_transaction(&nodes[1], &tx); + + // Send a payment using the channel's real SCID, which will be public in a few blocks once we + // can generate a channel_announcement. + let real_scid = nodes[0].node.list_usable_channels()[0].short_channel_id.unwrap(); + assert_eq!(nodes[1].node.list_usable_channels()[0].short_channel_id.unwrap(), real_scid); + + let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 10_000); + assert_eq!(route.paths[0][0].short_channel_id, real_scid); + send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1]]], 10_000, payment_hash, payment_secret); + claim_payment(&nodes[0], &[&nodes[1]], payment_preimage); + + disconnect_blocks(&nodes[0], 1); + disconnect_blocks(&nodes[1], 1); + + // At this point the channel no longer has an SCID again. In the future we should likely + // support simply un-setting the SCID and waiting until the channel gets re-confirmed, but for + // now we force-close the channel here. + check_closed_event!(&nodes[0], 1, ClosureReason::ProcessingError { + err: "Funding transaction was un-confirmed. Locked at 0 confs, now have 0 confs.".to_owned() + }); + check_closed_broadcast!(nodes[0], true); + check_closed_event!(&nodes[1], 1, ClosureReason::ProcessingError { + err: "Funding transaction was un-confirmed. Locked at 0 confs, now have 0 confs.".to_owned() + }); + check_closed_broadcast!(nodes[1], true); +} From ffaf9bc145608b20cc34be845c5d2e6600a74d34 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 4 May 2022 02:34:10 +0000 Subject: [PATCH 10/10] Add note about SCID collisions in `PaymentPathFailed` This isn't specific to 0-conf but is useful for users to note as the network moves towards more SCID aliases. --- lightning/src/util/events.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index ef0c619b02e..8d5fe00facf 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -331,6 +331,10 @@ pub enum Event { path: Vec, /// The channel responsible for the failed payment path. /// + /// Note that for route hints or for the first hop in a path this may be an SCID alias and + /// may not refer to a channel in the public network graph. These aliases may also collide + /// with channels in the public network graph. + /// /// If this is `Some`, then the corresponding channel should be avoided when the payment is /// retried. May be `None` for older [`Event`] serializations. short_channel_id: Option,