From: Matt Corallo <git@bluematt.me>
Date: Mon, 22 Jul 2024 20:03:41 +0000 (+0000)
Subject: Enforce segwit inputs for all "safe" funding transactions
X-Git-Tag: v0.0.124-beta~36^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=refs%2Fheads%2F2024-07-funding-check;p=rust-lightning

Enforce segwit inputs for all "safe" funding transactions

8403755a2a524beb9f6c8951f51dd60f7c54c912 introduced a separate path
for funding a channel without a full funding transaction, relying
on users to manually broadcast the funding tx. One of the major
things that makes this path less safe is that for other paths we're
supposed to validate that all inputs have witnesses, making the
funding transaction (likely) txid-non-malleable.

However, in one of several rewrites of that commit the funding tx
tests ended up getting elided in some call paths, which is fixed
here.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9bceb5816..893595ce1 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4605,23 +4605,22 @@ where
 	/// If there is an error, all channels in the batch are to be considered closed.
 	pub fn batch_funding_transaction_generated(&self, temporary_channels: &[(&ChannelId, &PublicKey)], funding_transaction: Transaction) -> Result<(), APIError> {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
-		let mut result = Ok(());
-
-		if !funding_transaction.is_coinbase() {
-			for inp in funding_transaction.input.iter() {
-				if inp.witness.is_empty() {
-					result = result.and(Err(APIError::APIMisuseError {
-						err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
-					}));
-				}
-			}
-		}
-		result.and(self.batch_funding_transaction_generated_intern(temporary_channels, FundingType::Checked(funding_transaction)))
+		self.batch_funding_transaction_generated_intern(temporary_channels, FundingType::Checked(funding_transaction))
 	}
 
 	fn batch_funding_transaction_generated_intern(&self, temporary_channels: &[(&ChannelId, &PublicKey)], funding: FundingType) -> Result<(), APIError> {
 		let mut result = Ok(());
 		if let FundingType::Checked(funding_transaction) = &funding {
+			if !funding_transaction.is_coinbase() {
+				for inp in funding_transaction.input.iter() {
+					if inp.witness.is_empty() {
+						result = result.and(Err(APIError::APIMisuseError {
+							err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
+						}));
+					}
+				}
+			}
+
 			if funding_transaction.output.len() > u16::max_value() as usize {
 				result = result.and(Err(APIError::APIMisuseError {
 					err: "Transaction had more than 2^16 outputs, which is not supported".to_owned()