Replace funding input tuple with struct

The funding inputs used for splicing and v2 channel establishment are
passed as a tuple of txin, prevtx, and witness weight. Add a struct so
that the items included can be better documented.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -13,7 +13,7 @@ use bitcoin::consensus::encode;
 use bitcoin::constants::ChainHash;
 use bitcoin::script::{Builder, Script, ScriptBuf, WScriptHash};
 use bitcoin::sighash::EcdsaSighashType;
-use bitcoin::transaction::{Transaction, TxIn, TxOut};
+use bitcoin::transaction::{Transaction, TxOut};
 use bitcoin::{Weight, Witness};
 
 use bitcoin::hash_types::{BlockHash, Txid};
@@ -26,7 +26,7 @@ use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1};
 use bitcoin::secp256k1::{PublicKey, SecretKey};
 #[cfg(splicing)]
 use bitcoin::Sequence;
-use bitcoin::{secp256k1, sighash};
+use bitcoin::{secp256k1, sighash, TxIn};
 
 use crate::chain::chaininterface::{
 	fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator,
@@ -53,7 +53,7 @@ use crate::ln::channel_state::{
 	OutboundHTLCDetails, OutboundHTLCStateDetails,
 };
 use crate::ln::channelmanager::{
-	self, FundingConfirmedMessage, HTLCFailureMsg, HTLCSource, OpenChannelMessage,
+	self, FundingConfirmedMessage, FundingTxInput, HTLCFailureMsg, HTLCSource, OpenChannelMessage,
 	PaymentClaimDetails, PendingHTLCInfo, PendingHTLCStatus, RAACommitmentOrder, SentHTLCId,
 	BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA,
 };
@@ -5915,10 +5915,12 @@ fn estimate_v2_funding_transaction_fee(
 #[cfg(splicing)]
 #[rustfmt::skip]
 fn check_v2_funding_inputs_sufficient(
-	contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool,
+	contribution_amount: i64, funding_inputs: &[FundingTxInput], is_initiator: bool,
 	is_splice: bool, funding_feerate_sat_per_1000_weight: u32,
 ) -> Result<u64, ChannelError> {
-	let mut total_input_witness_weight = Weight::from_wu(funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum());
+	let mut total_input_witness_weight = Weight::from_wu(
+		funding_inputs.iter().map(|input| input.utxo.satisfaction_weight).sum(),
+	);
 	let mut funding_inputs_len = funding_inputs.len();
 	if is_initiator && is_splice {
 		// consider the weight of the input and witness needed for spending the old funding transaction
@@ -5928,15 +5930,8 @@ fn check_v2_funding_inputs_sufficient(
 	let estimated_fee = estimate_v2_funding_transaction_fee(is_initiator, funding_inputs_len, total_input_witness_weight, funding_feerate_sat_per_1000_weight);
 
 	let mut total_input_sats = 0u64;
-	for (idx, input) in funding_inputs.iter().enumerate() {
-		if let Some(output) = input.1.output.get(input.0.previous_output.vout as usize) {
-			total_input_sats = total_input_sats.saturating_add(output.value.to_sat());
-		} else {
-			return Err(ChannelError::Warn(format!(
-				"Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]",
-				input.1.compute_txid(), input.0.previous_output.vout, idx
-			)));
-		}
+	for FundingTxInput { utxo, .. } in funding_inputs.iter() {
+		total_input_sats = total_input_sats.saturating_add(utxo.output.value.to_sat());
 	}
 
 	// If the inputs are enough to cover intended contribution amount, with fees even when
@@ -5978,7 +5973,7 @@ pub(super) struct FundingNegotiationContext {
 	pub shared_funding_input: Option<SharedOwnedInput>,
 	/// The funding inputs we will be contributing to the channel.
 	#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
-	pub our_funding_inputs: Vec<(TxIn, Transaction, Weight)>,
+	pub our_funding_inputs: Vec<FundingTxInput>,
 	/// The change output script. This will be used if needed or -- if not set -- generated using
 	/// `SignerProvider::get_destination_script`.
 	#[allow(dead_code)] // TODO(splicing): Remove once splicing is enabled.
@@ -6050,8 +6045,13 @@ impl FundingNegotiationContext {
 			}
 		}
 
-		let funding_inputs =
-			self.our_funding_inputs.into_iter().map(|(txin, tx, _)| (txin, tx)).collect();
+		let funding_inputs = self
+			.our_funding_inputs
+			.into_iter()
+			.map(|FundingTxInput { utxo, sequence, prevtx }| {
+				(TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() }, prevtx)
+			})
+			.collect();
 
 		let constructor_args = InteractiveTxConstructorArgs {
 			entropy_source,
@@ -10604,9 +10604,8 @@ where
 	///   generated by `SignerProvider::get_destination_script`.
 	#[cfg(splicing)]
 	pub fn splice_channel(
-		&mut self, our_funding_contribution_satoshis: i64,
-		our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option<ScriptBuf>,
-		funding_feerate_per_kw: u32, locktime: u32,
+		&mut self, our_funding_contribution_satoshis: i64, our_funding_inputs: Vec<FundingTxInput>,
+		change_script: Option<ScriptBuf>, funding_feerate_per_kw: u32, locktime: u32,
 	) -> Result<msgs::SpliceInit, APIError> {
 		// Check if a splice has been initiated already.
 		// Note: only a single outstanding splice is supported (per spec)
@@ -10672,7 +10671,7 @@ where
 			),
 		})?;
 
-		for (txin, tx, _) in our_funding_inputs.iter() {
+		for FundingTxInput { utxo, prevtx, .. } in our_funding_inputs.iter() {
 			const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
 				channel_id: ChannelId([0; 32]),
 				serial_id: 0,
@@ -10681,12 +10680,12 @@ where
 				sequence: 0,
 				shared_input_txid: None,
 			};
-			let message_len = MESSAGE_TEMPLATE.serialized_length() + tx.serialized_length();
+			let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length();
 			if message_len > LN_MAX_MSG_LEN {
 				return Err(APIError::APIMisuseError {
 					err: format!(
 						"Funding input references a prevtx that is too large for tx_add_input: {}",
-						txin.previous_output,
+						utxo.outpoint,
 					),
 				});
 			}
@@ -12468,7 +12467,7 @@ where
 	pub fn new_outbound<ES: Deref, F: Deref, L: Deref>(
 		fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP,
 		counterparty_node_id: PublicKey, their_features: &InitFeatures, funding_satoshis: u64,
-		funding_inputs: Vec<(TxIn, Transaction, Weight)>, user_id: u128, config: &UserConfig,
+		funding_inputs: Vec<FundingTxInput>, user_id: u128, config: &UserConfig,
 		current_chain_height: u32, outbound_scid_alias: u64, funding_confirmation_target: ConfirmationTarget,
 		logger: L,
 	) -> Result<Self, APIError>
@@ -12682,8 +12681,12 @@ where
 			value: Amount::from_sat(funding.get_value_satoshis()),
 			script_pubkey: funding.get_funding_redeemscript().to_p2wsh(),
 		};
-		let inputs_to_contribute =
-			our_funding_inputs.into_iter().map(|(txin, tx, _)| (txin, tx)).collect();
+		let inputs_to_contribute = our_funding_inputs
+			.into_iter()
+			.map(|FundingTxInput { utxo, sequence, prevtx }| {
+				(TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() }, prevtx)
+			})
+			.collect();
 
 		let interactive_tx_constructor = Some(InteractiveTxConstructor::new(
 			InteractiveTxConstructorArgs {
@@ -14119,6 +14122,8 @@ mod tests {
 		TOTAL_BITCOIN_SUPPLY_SATOSHIS,
 	};
 	use crate::ln::channel_keys::{RevocationBasepoint, RevocationKey};
+	#[cfg(splicing)]
+	use crate::ln::channelmanager::FundingTxInput;
 	use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
 	use crate::ln::msgs;
 	use crate::ln::msgs::{ChannelUpdate, UnsignedChannelUpdate, MAX_VALUE_MSAT};
@@ -14151,11 +14156,9 @@ mod tests {
 	use bitcoin::secp256k1::ffi::Signature as FFISignature;
 	use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1};
 	use bitcoin::secp256k1::{PublicKey, SecretKey};
-	#[cfg(splicing)]
-	use bitcoin::transaction::TxIn;
 	use bitcoin::transaction::{Transaction, TxOut, Version};
 	#[cfg(splicing)]
-	use bitcoin::Weight;
+	use bitcoin::{ScriptBuf, Sequence, WPubkeyHash};
 	use bitcoin::{WitnessProgram, WitnessVersion};
 	use std::cmp;
 
@@ -15897,19 +15900,17 @@ mod tests {
 
 	#[cfg(splicing)]
 	#[rustfmt::skip]
-	fn funding_input_sats(input_value_sats: u64) -> (TxIn, Transaction, Weight) {
-		use crate::sign::P2WPKH_WITNESS_WEIGHT;
-
-		let input_1_prev_out = TxOut { value: Amount::from_sat(input_value_sats), script_pubkey: bitcoin::ScriptBuf::default() };
-		let input_1_prev_tx = Transaction {
-			input: vec![], output: vec![input_1_prev_out],
-			version: Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO,
+	fn funding_input_sats(input_value_sats: u64) -> FundingTxInput {
+		let prevout = TxOut {
+			value: Amount::from_sat(input_value_sats),
+			script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()),
 		};
-		let input_1_txin = TxIn {
-			previous_output: bitcoin::OutPoint { txid: input_1_prev_tx.compute_txid(), vout: 0 },
-			..Default::default()
+		let prevtx = Transaction {
+			input: vec![], output: vec![prevout],
+			version: Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO,
 		};
-		(input_1_txin, input_1_prev_tx, Weight::from_wu(P2WPKH_WITNESS_WEIGHT))
+
+		FundingTxInput::new_p2wpkh(prevtx, 0, Sequence::ZERO).unwrap()
 	}
 
 	#[cfg(splicing)]
@@ -15930,7 +15931,7 @@ mod tests {
 				true,
 				2000,
 			).unwrap(),
-			2268,
+			2284,
 		);
 
 		// negative case, inputs clearly insufficient
@@ -15946,13 +15947,13 @@ mod tests {
 			);
 			assert_eq!(
 				format!("{:?}", res.err().unwrap()),
-				"Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.",
+				"Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1738. Need more inputs.",
 			);
 		}
 
 		// barely covers
 		{
-			let expected_fee: u64 = 2268;
+			let expected_fee: u64 = 2284;
 			assert_eq!(
 				check_v2_funding_inputs_sufficient(
 					(300_000 - expected_fee - 20) as i64,
@@ -15982,13 +15983,13 @@ mod tests {
 			);
 			assert_eq!(
 				format!("{:?}", res.err().unwrap()),
-				"Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.",
+				"Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2513. Need more inputs.",
 			);
 		}
 
 		// barely covers, less fees (no extra weight, no init)
 		{
-			let expected_fee: u64 = 1076;
+			let expected_fee: u64 = 1092;
 			assert_eq!(
 				check_v2_funding_inputs_sufficient(
 					(300_000 - expected_fee - 20) as i64,

lightning/src/ln/channelmanager.rs

@@ -30,9 +30,9 @@ use bitcoin::hashes::{Hash, HashEngine, HmacEngine};
 
 use bitcoin::secp256k1::Secp256k1;
 use bitcoin::secp256k1::{PublicKey, SecretKey};
-use bitcoin::{secp256k1, Sequence, SignedAmount};
 #[cfg(splicing)]
-use bitcoin::{ScriptBuf, TxIn, Weight};
+use bitcoin::ScriptBuf;
+use bitcoin::{secp256k1, Sequence, SignedAmount, Weight};
 
 use crate::blinded_path::message::MessageForwardNode;
 use crate::blinded_path::message::{AsyncPaymentsContext, OffersContext};
@@ -51,6 +51,7 @@ use crate::chain::channelmonitor::{
 };
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Watch};
+use crate::events::bump_transaction::{Utxo, EMPTY_SCRIPT_SIG_WEIGHT};
 use crate::events::{
 	self, ClosureReason, Event, EventHandler, EventsProvider, HTLCHandlingFailureType,
 	InboundChannelFunds, PaymentFailureReason, ReplayEvent,
@@ -116,7 +117,10 @@ use crate::routing::router::{
 	RouteParameters, RouteParametersConfig, Router,
 };
 use crate::sign::ecdsa::EcdsaChannelSigner;
-use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
+use crate::sign::{
+	EntropySource, NodeSigner, Recipient, SignerProvider, P2TR_KEY_PATH_WITNESS_WEIGHT,
+	P2WPKH_WITNESS_WEIGHT,
+};
 #[cfg(any(feature = "_test_utils", test))]
 use crate::types::features::Bolt11InvoiceFeatures;
 use crate::types::features::{
@@ -200,6 +204,101 @@ pub use crate::ln::outbound_payment::{
 };
 use crate::ln::script::ShutdownScript;
 
+/// An input to contribute to a channel's funding transaction either when using the v2 channel
+/// establishment protocol or when splicing.
+#[derive(Clone)]
+pub struct FundingTxInput {
+	/// The unspent [`TxOut`] that the input spends.
+	///
+	/// [`TxOut`]: bitcoin::TxOut
+	pub(super) utxo: Utxo,
+
+	/// The sequence number to use in the [`TxIn`].
+	///
+	/// [`TxIn`]: bitcoin::TxIn
+	pub(super) sequence: Sequence,
+
+	/// The transaction containing the unspent [`TxOut`] referenced by [`utxo`].
+	///
+	/// [`TxOut`]: bitcoin::TxOut
+	/// [`utxo`]: Self::utxo
+	pub(super) prevtx: Transaction,
+}
+
+impl FundingTxInput {
+	fn new<F: FnOnce(&bitcoin::Script) -> bool>(
+		prevtx: Transaction, vout: u32, sequence: Sequence, witness_weight: Weight,
+		script_filter: F,
+	) -> Result<Self, ()> {
+		Ok(FundingTxInput {
+			utxo: Utxo {
+				outpoint: bitcoin::OutPoint { txid: prevtx.compute_txid(), vout },
+				output: prevtx
+					.output
+					.get(vout as usize)
+					.filter(|output| script_filter(&output.script_pubkey))
+					.ok_or(())?
+					.clone(),
+				satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + witness_weight.to_wu(),
+			},
+			sequence,
+			prevtx,
+		})
+	}
+
+	/// Creates an input spending a P2WPKH output from the given `prevtx` at index `vout` using the
+	/// provided `sequence` number.
+	///
+	/// Returns `Err` if no such output exists in `prevtx` at index `vout`.
+	pub fn new_p2wpkh(prevtx: Transaction, vout: u32, sequence: Sequence) -> Result<Self, ()> {
+		let witness_weight = Weight::from_wu(P2WPKH_WITNESS_WEIGHT);
+		FundingTxInput::new(prevtx, vout, sequence, witness_weight, bitcoin::Script::is_p2wpkh)
+	}
+
+	/// Creates an input spending a P2WSH output from the given `prevtx` at index `vout` using the
+	/// provided `sequence` number.
+	///
+	/// Requires passing the weight of witness needed to satisfy the output's script.
+	///
+	/// Returns `Err` if no such output exists in `prevtx` at index `vout`.
+	pub fn new_p2wsh(
+		prevtx: Transaction, vout: u32, sequence: Sequence, witness_weight: Weight,
+	) -> Result<Self, ()> {
+		FundingTxInput::new(prevtx, vout, sequence, witness_weight, bitcoin::Script::is_p2wsh)
+	}
+
+	/// Creates an input spending a P2TR output from the given `prevtx` at index `vout` using the
+	/// provided `sequence` number.
+	///
+	/// This is meant for inputs spending a taproot output using the key path. See
+	/// [`new_p2tr_script_spend`] for when spending using a script path.
+	///
+	/// Returns `Err` if no such output exists in `prevtx` at index `vout`.
+	///
+	/// [`new_p2tr_script_spend`]: Self::new_p2tr_script_spend
+	pub fn new_p2tr_key_spend(
+		prevtx: Transaction, vout: u32, sequence: Sequence,
+	) -> Result<Self, ()> {
+		let witness_weight = Weight::from_wu(P2TR_KEY_PATH_WITNESS_WEIGHT);
+		FundingTxInput::new(prevtx, vout, sequence, witness_weight, bitcoin::Script::is_p2tr)
+	}
+
+	/// Creates an input spending a P2TR output from the given `prevtx` at index `vout` using the
+	/// provided `sequence` number.
+	///
+	/// Requires passing the weight of witness needed to satisfy a script path of the taproot
+	/// output. See [`new_p2tr_key_spend`] for when spending using the key path.
+	///
+	/// Returns `Err` if no such output exists in `prevtx` at index `vout`.
+	///
+	/// [`new_p2tr_key_spend`]: Self::new_p2tr_key_spend
+	pub fn new_p2tr_script_spend(
+		prevtx: Transaction, vout: u32, sequence: Sequence, witness_weight: Weight,
+	) -> Result<Self, ()> {
+		FundingTxInput::new(prevtx, vout, sequence, witness_weight, bitcoin::Script::is_p2tr)
+	}
+}
+
 // We hold various information about HTLC relay in the HTLC objects in Channel itself:
 //
 // Upon receipt of an HTLC from a peer, we'll give it a PendingHTLCStatus indicating if it should
@@ -4459,7 +4558,7 @@ where
 	#[rustfmt::skip]
 	pub fn splice_channel(
 		&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64,
-		our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option<ScriptBuf>,
+		our_funding_inputs: Vec<FundingTxInput>, change_script: Option<ScriptBuf>,
 		funding_feerate_per_kw: u32, locktime: Option<u32>,
 	) -> Result<(), APIError> {
 		let mut res = Ok(());
@@ -4480,9 +4579,8 @@ where
 	#[cfg(splicing)]
 	fn internal_splice_channel(
 		&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey,
-		our_funding_contribution_satoshis: i64,
-		our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option<ScriptBuf>,
-		funding_feerate_per_kw: u32, locktime: Option<u32>,
+		our_funding_contribution_satoshis: i64, our_funding_inputs: Vec<FundingTxInput>,
+		change_script: Option<ScriptBuf>, funding_feerate_per_kw: u32, locktime: Option<u32>,
 	) -> Result<(), APIError> {
 		let per_peer_state = self.per_peer_state.read().unwrap();