Update get_funding_created flow.

shaavan · shaavan · commit 3dfc3795605e · 2024-04-24T19:35:44.000+05:30
- 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.
diff --git 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<SP: Deref> Channel<SP> 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<SP: Deref> where SP::Target: SignerProvider {
 	pub context: ChannelContext<SP>,
@@ -7408,7 +7431,7 @@ impl<SP: Deref> OutboundV1Channel<SP> 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<L: Deref>(&mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L)
+	pub fn get_funding_created<L: Deref>(&mut self, transaction: TransactionEnum, logger: &L)
 	-> Result<Option<msgs::FundingCreated>, (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<SP: Deref> OutboundV1Channel<SP> 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
@@ -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
@@ -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()
