From dd83080edcc8fab676ba30b7d542e759b4cf0436 Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 24 Apr 2024 17:44:33 +0530 Subject: [PATCH 1/4] Add a new field to OutboundV1Channel 1. This field saves info about the previously received AcceptChannel and previously created FundingTransaction. --- lightning/src/ln/channel.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c02659cd0b0..dce5e12b12d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1247,6 +1247,29 @@ impl UnfundedChannelContext { } } +/// Stores values related to retrying the handshake process for outbound channels +/// after disconnection and reconnection. +pub(super) struct OutboundContext { + received_accept_channel_msg: Option, + created_funding_transaction: Option +} + +impl OutboundContext { + /// Create a new [`OutboundContext`] with blank values. + pub fn new() -> Self { + OutboundContext { + received_accept_channel_msg: None, + created_funding_transaction: None, + } + } +} + +struct FundingTransaction { + funding_transaction: Transaction, + outpoint: OutPoint, + is_batch_funding: bool, +} + /// Contains everything about the channel including state, and various flags. pub(super) struct ChannelContext where SP::Target: SignerProvider { config: LegacyChannelConfig, @@ -7285,6 +7308,7 @@ impl Channel where pub(super) struct OutboundV1Channel where SP::Target: SignerProvider { pub context: ChannelContext, pub unfunded_context: UnfundedChannelContext, + pub outbound_context: OutboundContext, } impl OutboundV1Channel where SP::Target: SignerProvider { @@ -7327,7 +7351,8 @@ impl OutboundV1Channel where SP::Target: SignerProvider { holder_signer, pubkeys, )?, - unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 } + unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 }, + outbound_context: OutboundContext::new(), }; Ok(chan) } From 080c90b63ea1ff9b8feb5ae091ecb2c33e66fd9b Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 24 Apr 2024 17:55:45 +0530 Subject: [PATCH 2/4] Update get_open_channel flow. - If our channel_state has moved forward, we check if it is at the negotiation stage, and that we have access to previously received accept_channel_msg. We panic only when both conditions fail. --- lightning/src/ln/channel.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index dce5e12b12d..e46188053ef 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1262,6 +1262,18 @@ impl OutboundContext { created_funding_transaction: None, } } + + /// Determines whether an `AcceptChannel` message has been received during a previous + /// channel handshake. + pub fn received_accept_channel(&self) -> bool { + self.received_accept_channel_msg.is_some() + } + + /// Determines whether a Funding Transaction had been created during a previous + /// channel handshake. + pub fn created_funding_transaction(&self) -> bool { + self.created_funding_transaction.is_some() + } } struct FundingTransaction { @@ -7465,8 +7477,8 @@ impl OutboundV1Channel where SP::Target: SignerProvider { if !self.context.is_outbound() { panic!("Tried to open a channel for an inbound channel?"); } - if self.context.have_received_message() { - panic!("Cannot generate an open_channel after we've moved forward"); + if self.context.have_received_message() && !self.deja_vu() { + panic!("Cannot generate an open_channel after we've moved forward earlier, but doesn't know the previous accept_channel_msg"); } if self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { @@ -7738,6 +7750,19 @@ impl OutboundV1Channel where SP::Target: SignerProvider { Ok((channel, channel_monitor)) } + /// Determines if the outbound channel handshake process is being retried. + fn deja_vu(&self) -> bool { + let negotiating = !matches!(self.context.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT); + let negotiated = self.context.channel_state == ChannelState::FundingNegotiated; + + if (negotiated && self.outbound_context.created_funding_transaction()) || + (negotiating && self.outbound_context.received_accept_channel()) { + true + } else { + false + } + } + /// Indicates that the signer may have some signatures for us, so we should retry if we're /// blocked. #[cfg(async_signing)] From 4fb03520b01d030d2ee807e52cbf5dbfe8c30db9 Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 24 Apr 2024 18:06:15 +0530 Subject: [PATCH 3/4] Update accept_channel flow. 1. If we had an acceptchannel earlier, we check if it matches the one received just now. If not we error, because the channel parameters have changed and that warrants renegotiating parameters again. 2. If they do match we simply skip the rest of the function because we had updated those values already. --- lightning/src/ln/channel.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e46188053ef..b794cdf239e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7525,9 +7525,16 @@ impl OutboundV1Channel where SP::Target: SignerProvider { if !self.context.is_outbound() { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned())); } - if !matches!(self.context.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT) { + if !matches!(self.context.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT) && !self.deja_vu() { return Err(ChannelError::Close("Got an accept_channel message at a strange time".to_owned())); } + if self.outbound_context.received_accept_channel() { + if self.outbound_context.received_accept_channel_msg.as_ref().unwrap() == msg { + return Ok(()); + } else { + return Err(ChannelError::Close("The new accep_channel_msg doesn't match the old one. Should renegotiating channel parameters".to_owned())); + } + } if msg.common_fields.dust_limit_satoshis > 21000000 * 100000000 { return Err(ChannelError::Close(format!("Peer never wants payout outputs? dust_limit_satoshis was {}", msg.common_fields.dust_limit_satoshis))); } @@ -7614,6 +7621,10 @@ impl OutboundV1Channel where SP::Target: SignerProvider { } } else { None }; + + // Save the accept channel msg after all the sanity checks to the outbound_context parameters. + self.outbound_context.received_accept_channel_msg = Some(msg.clone()); + self.context.counterparty_dust_limit_satoshis = msg.common_fields.dust_limit_satoshis; self.context.counterparty_max_htlc_value_in_flight_msat = cmp::min(msg.common_fields.max_htlc_value_in_flight_msat, self.context.channel_value_satoshis * 1000); self.context.counterparty_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis); From 3dfc3795605e00ef69c1cf5ec716464fd5eea100 Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 24 Apr 2024 18:58:57 +0530 Subject: [PATCH 4/4] Update get_funding_created flow. - Introduce a new enum that tracks if the Funding Transaction is a previously received transaction or a newly created one. - Update the flow and sanity checks accordingly to allow using previously used transaction if present. --- lightning/src/ln/channel.rs | 78 +++++++++++++++++++++++++--- lightning/src/ln/channelmanager.rs | 7 ++- lightning/src/ln/functional_tests.rs | 5 +- 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b794cdf239e..3c117b172c4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1276,7 +1276,9 @@ impl OutboundContext { } } -struct FundingTransaction { +/// Represents a funding transaction used in the establishment [`Channel`] +#[derive(Clone)] +pub struct FundingTransaction { funding_transaction: Transaction, outpoint: OutPoint, is_batch_funding: bool, @@ -7316,6 +7318,27 @@ impl Channel where } } +/// Represents whether the associated [`FundingTransaction`] instance is newly created +/// or was generated during a previous channel handshake. +#[allow(unused)] +pub enum TransactionEnum { + New(FundingTransaction), + Old(FundingTransaction), +} + +impl TransactionEnum { + /// Creates a new `TransactionEnum::New` variant representing a newly created funding transaction. + pub fn new_funding_transaction(transaction: Transaction, txo: OutPoint, is_batch_funding: bool) -> Self { + TransactionEnum::New( + FundingTransaction { + funding_transaction: transaction, + outpoint: txo, + is_batch_funding + } + ) + } +} + /// A not-yet-funded outbound (from holder) channel using V1 channel establishment. pub(super) struct OutboundV1Channel where SP::Target: SignerProvider { pub context: ChannelContext, @@ -7408,7 +7431,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { /// Note that channel_id changes during this call! /// Do NOT broadcast the funding transaction until after a successful funding_signed call! /// If an Err is returned, it is a ChannelError::Close. - pub fn get_funding_created(&mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L) + pub fn get_funding_created(&mut self, transaction: TransactionEnum, logger: &L) -> Result, (Self, ChannelError)> where L::Target: Logger { if !self.context.is_outbound() { panic!("Tried to create outbound funding_created message on an inbound channel!"); @@ -7425,6 +7448,39 @@ impl OutboundV1Channel where SP::Target: SignerProvider { panic!("Should not have advanced channel commitment tx numbers prior to funding_created"); } + let (funding_transaction, funding_txo, is_batch_funding) = match transaction { + TransactionEnum::New(tx) => { + self.outbound_context.created_funding_transaction = Some(tx.clone()); + (tx.funding_transaction, tx.outpoint, tx.is_batch_funding) + }, + TransactionEnum::Old(tx) => { + // Sanity checks + if self.context.channel_id != ChannelId::v1_from_funding_outpoint(tx.outpoint) { + panic!("Previously saved funding_transaction in channel parameter does not match funding_txo saved in outbound channel parameters.") + } + if self.context.funding_transaction != Some(tx.funding_transaction) { + panic!("Previously saved funding_transaction in channel parameter does not match funding_transaction saved in outbound channel parameters.") + } + if self.context.is_batch_funding != Some(()).filter(|_| tx.is_batch_funding) { + panic!("Previously saved funding_transaction in channel parameter does not match is_batch_funding saved in outbound channel parameters.") + } + + let funding_created = self.get_funding_created_msg(logger); + if funding_created.is_none() { + #[cfg(not(async_signing))] { + panic!("Failed to get signature for new funding creation"); + } + #[cfg(async_signing)] { + if !self.context.signer_pending_funding { + log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding"); + self.context.signer_pending_funding = true; + } + } + } + return Ok(funding_created); + } + }; + self.context.channel_transaction_parameters.funding_outpoint = Some(funding_txo); self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters); @@ -9382,7 +9438,7 @@ mod tests { use crate::ln::{PaymentHash, PaymentPreimage}; use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint}; use crate::ln::channelmanager::{self, HTLCSource, PaymentId}; - use crate::ln::channel::InitFeatures; + use crate::ln::channel::{InitFeatures, TransactionEnum}; use crate::ln::channel::{AwaitingChannelReadyFlags, Channel, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, HTLCUpdateAwaitingACK, commit_tx_fee_msat}; use crate::ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS, MIN_THEIR_CHAN_RESERVE_SATOSHIS}; use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, NodeFeatures}; @@ -9568,7 +9624,9 @@ mod tests { value: 10000000, script_pubkey: output_script.clone(), }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; - let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap(); + + let funding_transaction = TransactionEnum::new_funding_transaction(tx, funding_outpoint, false); + let funding_created_msg = node_a_chan.get_funding_created(funding_transaction, &&logger).map_err(|_| ()).unwrap(); let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap(); // Node B --> Node A: funding signed @@ -9697,7 +9755,8 @@ mod tests { value: 10000000, script_pubkey: output_script.clone(), }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; - let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap(); + let transaction = TransactionEnum::new_funding_transaction(tx, funding_outpoint, false); + let funding_created_msg = node_a_chan.get_funding_created(transaction, &&logger).map_err(|_| ()).unwrap(); let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap(); // Node B --> Node A: funding signed @@ -9886,7 +9945,8 @@ mod tests { value: 10000000, script_pubkey: output_script.clone(), }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; - let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap(); + let transaction = TransactionEnum::new_funding_transaction(tx.clone(), funding_outpoint, false); + let funding_created_msg = node_a_chan.get_funding_created(transaction, &&logger).map_err(|_| ()).unwrap(); let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap(); // Node B --> Node A: funding signed @@ -9953,7 +10013,8 @@ mod tests { value: 10000000, script_pubkey: outbound_chan.context.get_funding_redeemscript(), }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; - let funding_created = outbound_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap().unwrap(); + let transaction = TransactionEnum::new_funding_transaction(tx.clone(), funding_outpoint, false); + let funding_created = outbound_chan.get_funding_created(transaction, &&logger).map_err(|_| ()).unwrap().unwrap(); let mut chan = match inbound_chan.funding_created(&funding_created, best_block, &&keys_provider, &&logger) { Ok((chan, _, _)) => chan, Err((_, e)) => panic!("{}", e), @@ -11085,8 +11146,9 @@ mod tests { }, ]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; + let transaction = TransactionEnum::new_funding_transaction(tx.clone(), funding_outpoint, true); let funding_created_msg = node_a_chan.get_funding_created( - tx.clone(), funding_outpoint, true, &&logger, + transaction, &&logger, ).map_err(|_| ()).unwrap(); let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created( &funding_created_msg.unwrap(), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 50366faad1c..a7c0cf911da 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -75,6 +75,8 @@ use crate::util::string::UntrustedString; use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter}; use crate::util::logger::{Level, Logger, WithContext}; use crate::util::errors::APIError; +use super::channel::TransactionEnum; + #[cfg(not(c_bindings))] use { crate::offers::offer::DerivedMetadata, @@ -4500,8 +4502,9 @@ where Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => { funding_txo = find_funding_output(&chan, &funding_transaction)?; - let logger = WithChannelContext::from(&self.logger, &chan.context); - let funding_res = chan.get_funding_created(funding_transaction, funding_txo, is_batch_funding, &&logger) + let logger: WithChannelContext<'_, L> = WithChannelContext::from(&self.logger, &chan.context); + let transaction = TransactionEnum::new_funding_transaction(funding_transaction, funding_txo, is_batch_funding); + let funding_res = chan.get_funding_created(transaction, &&logger) .map_err(|(mut chan, e)| if let ChannelError::Close(msg) = e { let channel_id = chan.context.channel_id(); let reason = ClosureReason::ProcessingError { err: msg.clone() }; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 465d6288d9d..4e8a6c46db4 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -20,7 +20,7 @@ use crate::chain::transaction::OutPoint; use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider}; use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; use crate::ln::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash}; -use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase}; +use crate::ln::channel::{commitment_tx_base_weight, get_holder_selected_channel_reserve_satoshis, ChannelPhase, InboundV1Channel, OutboundV1Channel, TransactionEnum, COINBASE_MATURITY, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError}; use crate::ln::{chan_utils, onion_utils}; @@ -9235,7 +9235,8 @@ fn test_duplicate_chan_id() { match a_peer_state.channel_by_id.remove(&open_chan_2_msg.common_fields.temporary_channel_id).unwrap() { ChannelPhase::UnfundedOutboundV1(mut chan) => { let logger = test_utils::TestLogger::new(); - chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap() + let transaction = TransactionEnum::new_funding_transaction(tx.clone(), funding_outpoint, false); + chan.get_funding_created(transaction, &&logger).map_err(|_| ()).unwrap() }, _ => panic!("Unexpected ChannelPhase variant"), }.unwrap()