Merge pull request #3423 from dunxen/2024-11-PR3137-followups

Follow-ups to PR 3137

Cargo.toml

@@ -66,5 +66,4 @@ check-cfg = [
     "cfg(require_route_graph_test)",
     "cfg(splicing)",
     "cfg(async_payments)",
-    "cfg(dual_funding)",
 ]

lightning/src/ln/channel.rs

@@ -1182,7 +1182,6 @@ enum ChannelPhase<SP: Deref> where SP::Target: SignerProvider {
 	Undefined,
 	UnfundedOutboundV1(OutboundV1Channel<SP>),
 	UnfundedInboundV1(InboundV1Channel<SP>),
-	#[allow(dead_code)] // TODO(dual_funding): Remove once creating V2 channels is enabled.
 	UnfundedV2(PendingV2Channel<SP>),
 	Funded(FundedChannel<SP>),
 }
@@ -1399,7 +1398,6 @@ impl<SP: Deref> Channel<SP> where
 				debug_assert!(false);
 				ReconnectionMsg::None
 			},
-			#[cfg(dual_funding)]
 			ChannelPhase::UnfundedV2(chan) => {
 				if chan.context.is_outbound() {
 					ReconnectionMsg::Open(OpenChannelMessage::V2(
@@ -1413,8 +1411,6 @@ impl<SP: Deref> Channel<SP> where
 					ReconnectionMsg::None
 				}
 			},
-			#[cfg(not(dual_funding))]
-			ChannelPhase::UnfundedV2(_) => ReconnectionMsg::None,
 		}
 	}
 
@@ -1434,7 +1430,6 @@ impl<SP: Deref> Channel<SP> where
 					.map(|msg| Some(OpenChannelMessage::V1(msg)))
 			},
 			ChannelPhase::UnfundedInboundV1(_) => Ok(None),
-			#[cfg(dual_funding)]
 			ChannelPhase::UnfundedV2(chan) => {
 				if chan.context.is_outbound() {
 					chan.maybe_handle_error_without_close(chain_hash, fee_estimator)
@@ -1443,11 +1438,6 @@ impl<SP: Deref> Channel<SP> where
 					Ok(None)
 				}
 			},
-			#[cfg(not(dual_funding))]
-			ChannelPhase::UnfundedV2(_) => {
-				debug_assert!(false);
-				Ok(None)
-			},
 		}
 	}
 
@@ -1488,32 +1478,67 @@ impl<SP: Deref> Channel<SP> where
 	where
 		L::Target: Logger
 	{
-		let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
+		if let ChannelPhase::UnfundedV2(chan) = &mut self.phase {
-		let result = if let ChannelPhase::UnfundedV2(chan) = phase {
 			let logger = WithChannelContext::from(logger, &chan.context, None);
-			match chan.funding_tx_constructed(signing_session, &&logger) {
+			chan.funding_tx_constructed(signing_session, &&logger)
-				Ok((chan, commitment_signed, event)) => {
-					self.phase = ChannelPhase::Funded(chan);
-					Ok((commitment_signed, event))
-				},
-				Err((chan, e)) => {
-					self.phase = ChannelPhase::UnfundedV2(chan);
-					Err(e)
-				},
-			}
 		} else {
-			self.phase = phase;
 			Err(ChannelError::Warn("Got a tx_complete message with no interactive transaction construction expected or in-progress".to_owned()))
-		};
+		}
-
-		debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
-		result
 	}
 
 	pub fn force_shutdown(&mut self, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult {
 		let (funding, context) = self.funding_and_context_mut();
 		context.force_shutdown(funding, should_broadcast, closure_reason)
 	}
+
+	pub fn commitment_signed<L: Deref>(
+		&mut self, msg: &msgs::CommitmentSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
+	) -> Result<(Option<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>>, Option<ChannelMonitorUpdate>), ChannelError>
+	where
+		L::Target: Logger
+	{
+		let phase = core::mem::replace(&mut self.phase, ChannelPhase::Undefined);
+		match phase {
+			ChannelPhase::UnfundedV2(chan) => {
+				let holder_commitment_point = match chan.unfunded_context.holder_commitment_point {
+					Some(point) => point,
+					None => {
+						let channel_id = chan.context.channel_id();
+						// TODO(dual_funding): Add async signing support.
+						return Err( ChannelError::close(
+							format!("Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
+								channel_id)));
+					}
+				};
+				let mut funded_channel = FundedChannel {
+					funding: chan.funding,
+					context: chan.context,
+					interactive_tx_signing_session: chan.interactive_tx_signing_session,
+					holder_commitment_point,
+					is_v2_established: true,
+				};
+				let res = funded_channel.commitment_signed_initial_v2(msg, best_block, signer_provider, logger)
+					.map(|monitor| (Some(monitor), None))
+					// TODO: Change to `inspect_err` when MSRV is high enough.
+					.map_err(|err| {
+						// We always expect a `ChannelError` close.
+						debug_assert!(matches!(err, ChannelError::Close(_)));
+						err
+					});
+				self.phase = ChannelPhase::Funded(funded_channel);
+				res
+			},
+			ChannelPhase::Funded(mut funded_channel) => {
+				let res = funded_channel.commitment_signed(msg, logger).map(|monitor_update_opt| (None, monitor_update_opt));
+				self.phase = ChannelPhase::Funded(funded_channel);
+				res
+			},
+			_ => {
+				debug_assert!(!matches!(self.phase, ChannelPhase::Undefined));
+				Err(ChannelError::close("Got a commitment_signed message for an unfunded V1 channel!".into()))
+			}
+		}
+	}
 }
 
 impl<SP: Deref> From<OutboundV1Channel<SP>> for Channel<SP>
@@ -2204,8 +2229,8 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	}
 
 	pub fn funding_tx_constructed<L: Deref>(
-		mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
+		&mut self, mut signing_session: InteractiveTxSigningSession, logger: &L
-	) -> Result<(FundedChannel<SP>, msgs::CommitmentSigned, Option<Event>), (PendingV2Channel<SP>, ChannelError)>
+	) -> Result<(msgs::CommitmentSigned, Option<Event>), ChannelError>
 	where
 		L::Target: Logger
 	{
@@ -2221,7 +2246,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 						(
 							"Multiple outputs matched the expected script and value".to_owned(),
 							ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-						))).map_err(|e| (self, e));
+						)));
 				}
 				output_index = Some(idx as u16);
 			}
@@ -2233,7 +2258,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 				(
 					"No output matched the funding script_pubkey".to_owned(),
 					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-				))).map_err(|e| (self, e));
+				)));
 		};
 		self.context.channel_transaction_parameters.funding_outpoint = Some(outpoint);
 		self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
@@ -2247,20 +2272,23 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			},
 			Err(err) => {
 				self.context.channel_transaction_parameters.funding_outpoint = None;
-				return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })))
+				return Err(ChannelError::Close((err.to_string(), ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) })));
-					.map_err(|e| (self, e));
 			},
 		};
 
-		let funding_ready_for_sig_event = None;
+		let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 {
-		if signing_session.local_inputs_count() == 0 {
 			debug_assert_eq!(our_funding_satoshis, 0);
 			if signing_session.provide_holder_witnesses(self.context.channel_id, Vec::new()).is_err() {
 				debug_assert!(
 					false,
 					"Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found",
 				);
+				return Err(ChannelError::Close((
+					"V2 channel rejected due to sender error".into(),
+					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
+				)));
 			}
+			None
 		} else {
 			// TODO(dual_funding): Send event for signing if we've contributed funds.
 			// Inform the user that SIGHASH_ALL must be used for all signatures when contributing
@@ -2276,32 +2304,23 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			// will prevent the funding transaction from being relayed on the bitcoin network and hence being
 			// confirmed.
 			// </div>
-		}
+			debug_assert!(
+				false,
+				"We don't support users providing inputs but somehow we had more than zero inputs",
+			);
+			return Err(ChannelError::Close((
+				"V2 channel rejected due to sender error".into(),
+				ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
+			)));
+		};
 
 		self.context.channel_state = ChannelState::FundingNegotiated;
 
 		// Clear the interactive transaction constructor
 		self.interactive_tx_constructor.take();
+		self.interactive_tx_signing_session = Some(signing_session);
 
-		match self.unfunded_context.holder_commitment_point {
+		Ok((commitment_signed, funding_ready_for_sig_event))
-			Some(holder_commitment_point) => {
-				let funded_chan = FundedChannel {
-					funding: self.funding,
-					context: self.context,
-					interactive_tx_signing_session: Some(signing_session),
-					holder_commitment_point,
-				};
-				Ok((funded_chan, commitment_signed, funding_ready_for_sig_event))
-			},
-			None => {
-				Err(ChannelError::close(
-					format!(
-						"Expected to have holder commitment points available upon finishing interactive tx construction for channel {}",
-						self.context.channel_id(),
-					)))
-					.map_err(|e| (self, e))
-			},
-		}
 	}
 }
 
@@ -4501,8 +4520,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	{
 		if !matches!(
 			self.channel_state, ChannelState::NegotiatingFunding(flags)
-			if flags == (NegotiatingFundingFlags::OUR_INIT_SENT | NegotiatingFundingFlags::THEIR_INIT_SENT)) {
+			if flags == (NegotiatingFundingFlags::OUR_INIT_SENT | NegotiatingFundingFlags::THEIR_INIT_SENT)
-			panic!("Tried to get an initial commitment_signed messsage at a time other than immediately after initial handshake completion (or tried to get funding_created twice)");
+		) {
+			debug_assert!(false);
+			return Err(ChannelError::Close(("Tried to get an initial commitment_signed messsage at a time other than \
+				immediately after initial handshake completion (or tried to get funding_created twice)".to_string(),
+				ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true) }
+			)));
 		}
 
 		let signature = match self.get_initial_counterparty_commitment_signature(funding, logger) {
@@ -4525,7 +4549,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		})
 	}
 
-	#[cfg(all(test, dual_funding))]
+	#[cfg(all(test))]
 	pub fn get_initial_counterparty_commitment_signature_for_test<L: Deref>(
 		&mut self, funding: &FundingScope, logger: &L, channel_transaction_parameters: ChannelTransactionParameters,
 		counterparty_cur_commitment_point_override: PublicKey,
@@ -4650,6 +4674,9 @@ pub(super) struct FundedChannel<SP: Deref> where SP::Target: SignerProvider {
 	pub context: ChannelContext<SP>,
 	pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
 	holder_commitment_point: HolderCommitmentPoint,
+	/// Indicates whether this funded channel had been established with V2 channel
+	/// establishment.
+	is_v2_established: bool,
 }
 
 #[cfg(any(test, fuzzing))]
@@ -6130,14 +6157,22 @@ impl<SP: Deref> FundedChannel<SP> where
 		}
 	}
 
-	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError>
+	pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<Option<msgs::TxSignatures>, ChannelError>
 		where L::Target: Logger
 	{
-		if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
+		if !matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) {
 			return Err(ChannelError::close("Received tx_signatures in strange state!".to_owned()));
 		}
 
 		if let Some(ref mut signing_session) = self.interactive_tx_signing_session {
+			if msg.tx_hash != signing_session.unsigned_tx.compute_txid() {
+				return Err(ChannelError::Close(
+					(
+						"The txid for the transaction does not match".to_string(),
+						ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
+					)));
+			}
+
 			if msg.witnesses.len() != signing_session.remote_inputs_count() {
 				return Err(ChannelError::Warn(
 					"Witness count did not match contributed input count".to_string()
@@ -6159,33 +6194,25 @@ impl<SP: Deref> FundedChannel<SP> where
 				// for spending. Doesn't seem to be anything in rust-bitcoin.
 			}
 
-			if msg.tx_hash != signing_session.unsigned_tx.compute_txid() {
+			let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
-				return Err(ChannelError::Close(
-					(
-						"The txid for the transaction does not match".to_string(),
-						ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-					)));
-			}
-
-			let (tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
 				.map_err(|_| ChannelError::Warn("Witness count did not match contributed input count".to_string()))?;
+
+
 			if funding_tx_opt.is_some() {
-				self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+				// We have a finalized funding transaction, so we can set the funding transaction and reset the
+				// signing session fields.
+				self.context.funding_transaction = funding_tx_opt;
+				self.context.next_funding_txid = None;
+				self.interactive_tx_signing_session = None;
 			}
-			self.context.funding_transaction = funding_tx_opt.clone();
 
-			self.context.next_funding_txid = None;
+			if holder_tx_signatures_opt.is_some() && self.is_awaiting_initial_mon_persist() {
-
-			// Clear out the signing session
-			self.interactive_tx_signing_session = None;
-
-			if tx_signatures_opt.is_some() && self.context.channel_state.is_monitor_update_in_progress() {
 				log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
-				self.context.monitor_pending_tx_signatures = tx_signatures_opt;
+				self.context.monitor_pending_tx_signatures = holder_tx_signatures_opt;
-				return Ok((None, None));
+				return Ok(None);
 			}
 
-			Ok((tx_signatures_opt, funding_tx_opt))
+			Ok(holder_tx_signatures_opt)
 		} else {
 			Err(ChannelError::Close((
 				"Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(),
@@ -6402,12 +6429,12 @@ impl<SP: Deref> FundedChannel<SP> where
 		assert!(self.context.channel_state.is_monitor_update_in_progress());
 		self.context.channel_state.clear_monitor_update_in_progress();
 
-		// If we're past (or at) the AwaitingChannelReady stage on an outbound channel, try to
+		// If we're past (or at) the AwaitingChannelReady stage on an outbound (or V2-established) channel,
-		// (re-)broadcast the funding transaction as we may have declined to broadcast it when we
+		// 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 = None;
 		if let Some(funding_transaction) = &self.context.funding_transaction {
-			if self.context.is_outbound() &&
+			if (self.context.is_outbound() || self.is_v2_established()) &&
 				(matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) ||
 				matches!(self.context.channel_state, ChannelState::ChannelReady(_)))
 			{
@@ -8923,6 +8950,10 @@ impl<SP: Deref> FundedChannel<SP> where
 			false
 		}
 	}
+
+	pub fn is_v2_established(&self) -> bool {
+		self.is_v2_established
+	}
 }
 
 /// A not-yet-funded outbound (from holder) channel using V1 channel establishment.
@@ -9190,6 +9221,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 			funding: self.funding,
 			context: self.context,
 			interactive_tx_signing_session: None,
+			is_v2_established: false,
 			holder_commitment_point,
 		};
 
@@ -9457,6 +9489,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 			funding: self.funding,
 			context: self.context,
 			interactive_tx_signing_session: None,
+			is_v2_established: false,
 			holder_commitment_point,
 		};
 		let need_channel_ready = channel.check_get_channel_ready(0, logger).is_some()
@@ -9494,6 +9527,8 @@ pub(super) struct PendingV2Channel<SP: Deref> where SP::Target: SignerProvider {
 	pub dual_funding_context: DualFundingChannelContext,
 	/// The current interactive transaction construction session under negotiation.
 	pub interactive_tx_constructor: Option<InteractiveTxConstructor>,
+	/// The signing session created after `tx_complete` handling
+	pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
 }
 
 impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
@@ -9559,6 +9594,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 				our_funding_inputs: funding_inputs,
 			},
 			interactive_tx_constructor: None,
+			interactive_tx_signing_session: None,
 		};
 		Ok(chan)
 	}
@@ -9566,7 +9602,6 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	/// If we receive an error message, it may only be a rejection of the channel type we tried,
 	/// not of our ability to open any channel at all. Thus, on error, we should first call this
 	/// and see if we get a new `OpenChannelV2` message, otherwise the channel is failed.
-	#[cfg(dual_funding)]
 	pub(crate) fn maybe_handle_error_without_close<F: Deref>(
 		&mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator<F>
 	) -> Result<msgs::OpenChannelV2, ()>
@@ -9577,7 +9612,6 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 		Ok(self.get_open_channel_v2(chain_hash))
 	}
 
-	#[cfg(dual_funding)]
 	pub fn get_open_channel_v2(&self, chain_hash: ChainHash) -> msgs::OpenChannelV2 {
 		if !self.context.is_outbound() {
 			debug_assert!(false, "Tried to send open_channel2 for an inbound channel?");
@@ -9732,6 +9766,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			context,
 			dual_funding_context,
 			interactive_tx_constructor,
+			interactive_tx_signing_session: None,
 			unfunded_context,
 		})
 	}
@@ -10270,7 +10305,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 			let mut _val: u64 = Readable::read(reader)?;
 		}
 
-		let channel_id = Readable::read(reader)?;
+		let channel_id: ChannelId = Readable::read(reader)?;
 		let channel_state = ChannelState::from_u32(Readable::read(reader)?).map_err(|_| DecodeError::InvalidValue)?;
 		let channel_value_satoshis = Readable::read(reader)?;
 
@@ -10706,6 +10741,10 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				}
 			},
 		};
+		let is_v2_established = channel_id.is_v2_channel_id(
+			&channel_parameters.holder_pubkeys.revocation_basepoint,
+			&channel_parameters.counterparty_parameters.as_ref()
+				.expect("Persisted channel must have counterparty parameters").pubkeys.revocation_basepoint);
 
 		Ok(FundedChannel {
 			funding: FundingScope {
@@ -10848,6 +10887,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				is_holder_quiescence_initiator: None,
 			},
 			interactive_tx_signing_session: None,
+			is_v2_established,
 			holder_commitment_point,
 		})
 	}

lightning/src/ln/channelmanager.rs

@@ -49,7 +49,6 @@ use crate::ln::inbound_payment;
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channel::{self, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, ShutdownResult, UpdateFulfillCommitFetch, OutboundV1Channel, ReconnectionMsg, InboundV1Channel, WithChannelContext};
-#[cfg(any(dual_funding, splicing))]
 use crate::ln::channel::PendingV2Channel;
 use crate::ln::channel_state::ChannelDetails;
 use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
@@ -1450,13 +1449,11 @@ impl <SP: Deref> PeerState<SP> where SP::Target: SignerProvider {
 #[derive(Clone)]
 pub(super) enum OpenChannelMessage {
 	V1(msgs::OpenChannel),
-	#[cfg(dual_funding)]
 	V2(msgs::OpenChannelV2),
 }
 
 pub(super) enum OpenChannelMessageRef<'a> {
 	V1(&'a msgs::OpenChannel),
-	#[cfg(dual_funding)]
 	V2(&'a msgs::OpenChannelV2),
 }
 
@@ -7846,7 +7843,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 							(*temporary_channel_id, Channel::from(channel), message_send_event)
 						})
 					},
-					#[cfg(dual_funding)]
 					OpenChannelMessage::V2(open_channel_msg) => {
 						PendingV2Channel::new_inbound(
 							&self.fee_estimator, &self.entropy_source, &self.signer_provider,
@@ -8009,7 +8005,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	fn internal_open_channel(&self, counterparty_node_id: &PublicKey, msg: OpenChannelMessageRef<'_>) -> Result<(), MsgHandleErrInternal> {
 		let common_fields = match msg {
 			OpenChannelMessageRef::V1(msg) => &msg.common_fields,
-			#[cfg(dual_funding)]
 			OpenChannelMessageRef::V2(msg) => &msg.common_fields,
 		};
 
@@ -8087,7 +8082,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				funding_satoshis: common_fields.funding_satoshis,
 				channel_negotiation_type: match msg {
 					OpenChannelMessageRef::V1(msg) => InboundChannelFunds::PushMsat(msg.push_msat),
-					#[cfg(dual_funding)]
 					OpenChannelMessageRef::V2(_) => InboundChannelFunds::DualFunded,
 				},
 				channel_type,
@@ -8097,7 +8091,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			peer_state.inbound_channel_request_by_id.insert(channel_id, InboundChannelRequest {
 				open_channel_msg: match msg {
 					OpenChannelMessageRef::V1(msg) => OpenChannelMessage::V1(msg.clone()),
-					#[cfg(dual_funding)]
 					OpenChannelMessageRef::V2(msg) => OpenChannelMessage::V2(msg.clone()),
 				},
 				ticks_remaining: UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS,
@@ -8133,7 +8126,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				});
 				(Channel::from(channel), message_send_event)
 			},
-			#[cfg(dual_funding)]
 			OpenChannelMessageRef::V2(msg) => {
 				let channel = PendingV2Channel::new_inbound(
 					&self.fee_estimator, &self.entropy_source, &self.signer_provider,
@@ -8532,14 +8524,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				match chan_entry.get_mut().as_funded_mut() {
 					Some(chan) => {
 						let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-						let (tx_signatures_opt, funding_tx_opt) = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry);
+						let tx_signatures_opt = try_channel_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_entry);
 						if let Some(tx_signatures) = tx_signatures_opt {
 							peer_state.pending_msg_events.push(events::MessageSendEvent::SendTxSignatures {
 								node_id: *counterparty_node_id,
 								msg: tx_signatures,
 							});
 						}
-						if let Some(ref funding_tx) = funding_tx_opt {
+						if let Some(ref funding_tx) = chan.context.unbroadcasted_funding() {
 							self.tx_broadcaster.broadcast_transactions(&[funding_tx]);
 							{
 								let mut pending_events = self.pending_events.lock().unwrap();
@@ -8958,14 +8950,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		let peer_state = &mut *peer_state_lock;
 		match peer_state.channel_by_id.entry(msg.channel_id) {
 			hash_map::Entry::Occupied(mut chan_entry) => {
-				if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
+				let chan = chan_entry.get_mut();
-					let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+				let logger = WithChannelContext::from(&self.logger, &chan.context(), None);
-					let funding_txo = chan.context.get_funding_txo();
+				let funding_txo = chan.context().get_funding_txo();
+				let (monitor_opt, monitor_update_opt) = try_channel_entry!(
+					self, peer_state, chan.commitment_signed(msg, best_block, &self.signer_provider, &&logger),
+					chan_entry);
 
-					if chan.interactive_tx_signing_session.is_some() {
+				if let Some(chan) = chan.as_funded_mut() {
-						let monitor = try_channel_entry!(
+					if let Some(monitor) = monitor_opt {
-							self, peer_state, chan.commitment_signed_initial_v2(msg, best_block, &self.signer_provider, &&logger),
-							chan_entry);
 						let monitor_res = self.chain_monitor.watch_channel(monitor.channel_id(), monitor);
 						if let Ok(persist_state) = monitor_res {
 							handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
@@ -8980,19 +8973,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 								)
 							)), chan_entry)
 						}
-					} else {
+					} else if let Some(monitor_update) = monitor_update_opt {
-						let monitor_update_opt = try_channel_entry!(
+						handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
-							self, peer_state, chan.commitment_signed(msg, &&logger), chan_entry);
+							peer_state, per_peer_state, chan);
-						if let Some(monitor_update) = monitor_update_opt {
-							handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
-								peer_state, per_peer_state, chan);
-						}
 					}
-					Ok(())
-				} else {
-					return try_channel_entry!(self, peer_state, Err(ChannelError::close(
-						"Got a commitment_signed message for an unfunded channel!".into())), chan_entry);
 				}
+				Ok(())
 			},
 			hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
 		}
@@ -11659,7 +11645,6 @@ where
 		// Note that we never need to persist the updated ChannelManager for an inbound
 		// open_channel message - pre-funded channels are never written so there should be no
 		// change to the contents.
-		#[cfg(dual_funding)]
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_open_channel(&counterparty_node_id, OpenChannelMessageRef::V2(msg));
 			let persist = match &res {
@@ -11672,10 +11657,6 @@ where
 			let _ = handle_error!(self, res, counterparty_node_id);
 			persist
 		});
-		#[cfg(not(dual_funding))]
-		let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close(
-			"Dual-funded channels not supported".to_owned(),
-			msg.common_fields.temporary_channel_id.clone())), counterparty_node_id);
 	}
 
 	fn handle_accept_channel(&self, counterparty_node_id: PublicKey, msg: &msgs::AcceptChannel) {
@@ -12074,7 +12055,6 @@ where
 								node_id: chan.context().get_counterparty_node_id(),
 								msg,
 							}),
-						#[cfg(dual_funding)]
 						ReconnectionMsg::Open(OpenChannelMessage::V2(msg)) =>
 							pending_msg_events.push(events::MessageSendEvent::SendOpenChannelV2 {
 								node_id: chan.context().get_counterparty_node_id(),
@@ -12181,7 +12161,6 @@ where
 							});
 							return;
 						},
-						#[cfg(dual_funding)]
 						Ok(Some(OpenChannelMessage::V2(msg))) => {
 							peer_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannelV2 {
 								node_id: counterparty_node_id,
@@ -12751,7 +12730,6 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures {
 	if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx {
 		features.set_anchors_zero_fee_htlc_tx_optional();
 	}
-	#[cfg(dual_funding)]
 	features.set_dual_fund_optional();
 	// Only signal quiescence support in tests for now, as we don't yet support any
 	// quiescent-dependent protocols (e.g., splicing).

lightning/src/ln/dual_funding_tests.rs

@@ -9,7 +9,6 @@
 
 //! Tests that test the creation of dual-funded channels in ChannelManager.
 
-#[cfg(dual_funding)]
 use {
 	crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator},
 	crate::events::{Event, MessageSendEvent, MessageSendEventsProvider},
@@ -21,27 +20,24 @@ use {
 	crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint},
 	crate::ln::functional_test_utils::*,
 	crate::ln::msgs::ChannelMessageHandler,
-	crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete},
+	crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete, TxSignatures},
 	crate::ln::types::ChannelId,
 	crate::prelude::*,
 	crate::sign::ChannelSigner as _,
 	crate::util::ser::TransactionU16LenLimited,
 	crate::util::test_utils,
+	bitcoin::Witness,
 };
 
-#[cfg(dual_funding)]
 // Dual-funding: V2 Channel Establishment Tests
 struct V2ChannelEstablishmentTestSession {
 	funding_input_sats: u64,
 	initiator_input_value_satoshis: u64,
 }
 
-#[cfg(dual_funding)]
 // TODO(dual_funding): Use real node and API for creating V2 channels as initiator when available,
 // instead of manually constructing messages.
-fn do_test_v2_channel_establishment(
+fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) {
-	session: V2ChannelEstablishmentTestSession, test_async_persist: bool,
-) {
 	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]);
@@ -199,11 +195,7 @@ fn do_test_v2_channel_establishment(
 		partial_signature_with_nonce: None,
 	};
 
-	if test_async_persist {
+	chanmon_cfgs[1].persister.set_update_ret(crate::chain::ChannelMonitorUpdateStatus::InProgress);
-		chanmon_cfgs[1]
-			.persister
-			.set_update_ret(crate::chain::ChannelMonitorUpdateStatus::InProgress);
-	}
 
 	// Handle the initial commitment_signed exchange. Order is not important here.
 	nodes[1]
@@ -211,25 +203,15 @@ fn do_test_v2_channel_establishment(
 		.handle_commitment_signed(nodes[0].node.get_our_node_id(), &msg_commitment_signed_from_0);
 	check_added_monitors(&nodes[1], 1);
 
-	if test_async_persist {
+	// The funding transaction should not have been broadcast before persisting initial monitor has
-		let events = nodes[1].node.get_and_clear_pending_events();
+	// been completed.
-		assert!(events.is_empty());
+	assert_eq!(nodes[1].tx_broadcaster.txn_broadcast().len(), 0);
+	assert_eq!(nodes[1].node.get_and_clear_pending_events().len(), 0);
 
-		chanmon_cfgs[1]
+	// Complete the persistence of the monitor.
-			.persister
+	let events = nodes[1].node.get_and_clear_pending_events();
-			.set_update_ret(crate::chain::ChannelMonitorUpdateStatus::Completed);
+	assert!(events.is_empty());
-		let (latest_update, _) = *nodes[1]
+	nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
-			.chain_monitor
-			.latest_monitor_update_id
-			.lock()
-			.unwrap()
-			.get(&channel_id)
-			.unwrap();
-		nodes[1]
-			.chain_monitor
-			.chain_monitor
-			.force_channel_monitor_updated(channel_id, latest_update);
-	}
 
 	let events = nodes[1].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
@@ -245,25 +227,30 @@ fn do_test_v2_channel_establishment(
 	);
 
 	assert_eq!(tx_signatures_msg.channel_id, channel_id);
+
+	let mut witness = Witness::new();
+	witness.push([0x0]);
+	// Receive tx_signatures from channel initiator.
+	nodes[1].node.handle_tx_signatures(
+		nodes[0].node.get_our_node_id(),
+		&TxSignatures {
+			channel_id,
+			tx_hash: funding_outpoint.unwrap().txid,
+			witnesses: vec![witness],
+			shared_input_signature: None,
+		},
+	);
+
+	// For an inbound channel V2 channel the transaction should be broadcast once receiving a
+	// tx_signature and applying local tx_signatures:
+	let broadcasted_txs = nodes[1].tx_broadcaster.txn_broadcast();
+	assert_eq!(broadcasted_txs.len(), 1);
 }
 
 #[test]
-#[cfg(dual_funding)]
 fn test_v2_channel_establishment() {
-	// Only initiator contributes, no persist pending
+	do_test_v2_channel_establishment(V2ChannelEstablishmentTestSession {
-	do_test_v2_channel_establishment(
+		funding_input_sats: 100_00,
-		V2ChannelEstablishmentTestSession {
+		initiator_input_value_satoshis: 150_000,
-			funding_input_sats: 100_000,
+	});
-			initiator_input_value_satoshis: 150_000,
-		},
-		false,
-	);
-	// Only initiator contributes, persist pending
-	do_test_v2_channel_establishment(
-		V2ChannelEstablishmentTestSession {
-			funding_input_sats: 100_000,
-			initiator_input_value_satoshis: 150_000,
-		},
-		true,
-	);
 }

lightning/src/ln/interactivetxs.rs

@@ -316,14 +316,18 @@ impl InteractiveTxSigningSession {
 
 	/// Handles a `tx_signatures` message received from the counterparty.
 	///
+	/// If the holder is required to send their `tx_signatures` message and these signatures have
+	/// already been provided to the signing session, then this return value will be `Some`, otherwise
+	/// None.
+	///
+	/// If the holder has already provided their `tx_signatures` to the signing session, a funding
+	/// transaction will be finalized and returned as Some, otherwise None.
+	///
 	/// Returns an error if the witness count does not equal the counterparty's input count in the
 	/// unsigned transaction.
 	pub fn received_tx_signatures(
 		&mut self, tx_signatures: TxSignatures,
 	) -> Result<(Option<TxSignatures>, Option<Transaction>), ()> {
-		if self.counterparty_sent_tx_signatures {
-			return Ok((None, None));
-		};
 		if self.remote_inputs_count() != tx_signatures.witnesses.len() {
 			return Err(());
 		}
@@ -336,13 +340,16 @@ impl InteractiveTxSigningSession {
 			None
 		};
 
-		let funding_tx = if self.holder_tx_signatures.is_some() {
+		// Check if the holder has provided its signatures and if so,
+		// return the finalized funding transaction.
+		let funding_tx_opt = if self.holder_tx_signatures.is_some() {
 			Some(self.finalize_funding_tx())
 		} else {
+			// This means we're still waiting for the holder to provide their signatures.
 			None
 		};
 
-		Ok((holder_tx_signatures, funding_tx))
+		Ok((holder_tx_signatures, funding_tx_opt))
 	}
 
 	/// Provides the holder witnesses for the unsigned transaction.
@@ -351,7 +358,7 @@ impl InteractiveTxSigningSession {
 	/// unsigned transaction.
 	pub fn provide_holder_witnesses(
 		&mut self, channel_id: ChannelId, witnesses: Vec<Witness>,
-	) -> Result<Option<TxSignatures>, ()> {
+	) -> Result<(), ()> {
 		if self.local_inputs_count() != witnesses.len() {
 			return Err(());
 		}
@@ -363,13 +370,8 @@ impl InteractiveTxSigningSession {
 			witnesses: witnesses.into_iter().collect(),
 			shared_input_signature: None,
 		});
-		if self.received_commitment_signed
+
-			&& (self.holder_sends_tx_signatures_first || self.counterparty_sent_tx_signatures)
+		Ok(())
-		{
-			Ok(self.holder_tx_signatures.clone())
-		} else {
-			Ok(None)
-		}
 	}
 
 	pub fn remote_inputs_count(&self) -> usize {

lightning/src/ln/peer_handler.rs

@@ -346,7 +346,6 @@ impl ChannelMessageHandler for ErroringMessageHandler {
 		features.set_basic_mpp_optional();
 		features.set_wumbo_optional();
 		features.set_shutdown_any_segwit_optional();
-		#[cfg(dual_funding)]
 		features.set_dual_fund_optional();
 		features.set_channel_type_optional();
 		features.set_scid_privacy_optional();

lightning/src/ln/types.rs

@@ -99,6 +99,13 @@ impl ChannelId {
 		let our_revocation_point_bytes = our_revocation_basepoint.0.serialize();
 		Self(Sha256::hash(&[[0u8; 33], our_revocation_point_bytes].concat()).to_byte_array())
 	}
+
+	/// Indicates whether this is a V2 channel ID for the given local and remote revocation basepoints.
+	pub fn is_v2_channel_id(
+		&self, ours: &RevocationBasepoint, theirs: &RevocationBasepoint,
+	) -> bool {
+		*self == Self::v2_from_revocation_basepoints(ours, theirs)
+	}
 }
 
 impl Writeable for ChannelId {
@@ -213,4 +220,18 @@ mod tests {
 
 		assert_eq!(ChannelId::v2_from_revocation_basepoints(&ours, &theirs), expected_id);
 	}
+
+	#[test]
+	fn test_is_v2_channel_id() {
+		let our_pk = "0324653eac434488002cc06bbfb7f10fe18991e35f9fe4302dbea6d2353dc0ab1c";
+		let ours = RevocationBasepoint(PublicKey::from_str(&our_pk).unwrap());
+		let their_pk = "02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619";
+		let theirs = RevocationBasepoint(PublicKey::from_str(&their_pk).unwrap());
+
+		let channel_id = ChannelId::v2_from_revocation_basepoints(&ours, &theirs);
+		assert!(channel_id.is_v2_channel_id(&ours, &theirs));
+
+		let channel_id = ChannelId::v1_from_funding_txid(&[2; 32], 1);
+		assert!(!channel_id.is_v2_channel_id(&ours, &theirs))
+	}
 }