f use all input value (minus fees) when contributing

dunxen · dunxen · commit 10114e6825b0 · 2024-06-13T13:43:44.000+02:00
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -1461,29 +1461,10 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider
 	fn is_initiator(&self) -> bool;
 
 	fn begin_interactive_funding_tx_construction<ES: Deref>(
-		&mut self, signer_provider: &SP, entropy_source: &ES,
+		&mut self, entropy_source: &ES,
 	) -> Result<Option<InteractiveTxMessageSend>, APIError>
 	where ES::Target: EntropySource
 	{
-		let mut funding_inputs_prev_outputs: Vec<TxOut> = Vec::with_capacity(self.dual_funding_context_mut().our_funding_inputs.len());
-		// Check that vouts exist for each TxIn in provided transactions.
-		for (idx, input) in self.dual_funding_context_mut().our_funding_inputs.iter().enumerate() {
-			if let Some(output) = input.1.as_transaction().output.get(input.0.previous_output.vout as usize) {
-				funding_inputs_prev_outputs.push(output.clone());
-			} else {
-				return Err(APIError::APIMisuseError {
-					err: format!("Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]",
-						input.1.as_transaction().txid(), input.0.previous_output.vout, idx) });
-			}
-		}
-		let total_input_satoshis: u64 = self.dual_funding_context_mut().our_funding_inputs.iter().map(
-			|input| input.1.as_transaction().output[input.0.previous_output.vout as usize].value.to_sat()).sum();
-		if total_input_satoshis < self.dual_funding_context_mut().our_funding_satoshis {
-			return Err(APIError::APIMisuseError {
-				err: format!("Total value of funding inputs must be at least funding amount. It was {} sats",
-					total_input_satoshis) });
-		}
-
 		let mut funding_outputs = Vec::new();
 		if self.is_initiator() {
 			funding_outputs.push(TxOut {
@@ -1492,11 +1473,6 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider
 			});
 		}
 
-		maybe_add_funding_change_output(signer_provider, self.is_initiator(), self.dual_funding_context_mut().our_funding_satoshis,
-			&funding_inputs_prev_outputs, &mut funding_outputs, self.dual_funding_context_mut().funding_feerate_sat_per_1000_weight,
-			total_input_satoshis, self.context().holder_dust_limit_satoshis, self.context().channel_keys_id).map_err(
-				|_| APIError::APIMisuseError { err: "Could not create change output".to_string() })?;
-
 		let (tx_constructor, msg) = InteractiveTxConstructor::new(
 			entropy_source, self.context().channel_id(),
 			self.dual_funding_context_mut().funding_feerate_sat_per_1000_weight, self.is_initiator(),
@@ -3670,53 +3646,39 @@ pub(crate) fn per_outbound_htlc_counterparty_commit_tx_fee_msat(feerate_per_kw:
 }
 
 #[cfg(any(dual_funding, splicing))]
-pub(super) fn maybe_add_funding_change_output<SP: Deref>(
-	signer_provider: &SP, is_initiator: bool, our_funding_satoshis: u64,
-	funding_inputs_prev_outputs: &Vec<TxOut>, funding_outputs: &mut Vec<TxOut>,
-	funding_feerate_sat_per_1000_weight: u32, total_input_satoshis: u64,
-	holder_dust_limit_satoshis: u64, channel_keys_id: [u8; 32],
-) -> Result<Option<TxOut>, ChannelError> where
-	SP::Target: SignerProvider,
-{
-	// Add the total estimated weight of our contributed inputs...
-	let mut our_contributed_weight = funding_inputs_prev_outputs.iter().fold(0u64, |weight, prev_output| {
-		weight.saturating_add(estimate_input_weight(prev_output).to_wu())
-	}).saturating_add(
-		// ... with the total weight of our contributed outputs.
-		funding_outputs.iter().fold(0u64, |weight, txout| {
-			weight.saturating_add(get_output_weight(&txout.script_pubkey).to_wu())
-		})
-	);
+pub(super) fn calculate_our_funding_satoshis(
+	is_initiator: bool, funding_inputs: &[(TxIn, TransactionU16LenLimited)],
+	funding_outputs: &[TxOut], funding_feerate_sat_per_1000_weight: u32,
+	holder_dust_limit_satoshis: u64,
+) -> Result<u64, APIError> {
+	let mut total_input_satoshis = 0u64;
+	let mut our_contributed_weight = 0u64;
+
+	for (idx, input) in funding_inputs.iter().enumerate() {
+		if let Some(output) = input.1.as_transaction().output.get(input.0.previous_output.vout as usize) {
+			total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat());
+			our_contributed_weight = our_contributed_weight.saturating_add(estimate_input_weight(output).to_wu());
+		} else {
+			return Err(APIError::APIMisuseError {
+				err: format!("Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]",
+					input.1.as_transaction().txid(), input.0.previous_output.vout, idx) });
+		}
+	}
+	our_contributed_weight = our_contributed_weight.saturating_add(funding_outputs.iter().fold(0u64, |weight, txout| {
+		weight.saturating_add(get_output_weight(&txout.script_pubkey).to_wu())
+	}));
 
 	// If we are the initiator, we must pay for weight of all common fields in the funding transaction.
 	if is_initiator {
-		our_contributed_weight += TX_COMMON_FIELDS_WEIGHT;
+		our_contributed_weight = our_contributed_weight.saturating_add(TX_COMMON_FIELDS_WEIGHT);
 	}
 
-	let remaining_value = total_input_satoshis
-		.saturating_sub(our_funding_satoshis)
+	let funding_satoshis = total_input_satoshis
 		.saturating_sub(fee_for_weight(funding_feerate_sat_per_1000_weight, our_contributed_weight));
-
-	if remaining_value < holder_dust_limit_satoshis {
-		Ok(None)
+	if funding_satoshis < holder_dust_limit_satoshis {
+		Ok(0)
 	} else {
-		let change_script = signer_provider.get_destination_script(channel_keys_id).map_err(
-			|_| ChannelError::Close(
-				(
-					"Failed to get change script as new destination script".to_owned(),
-					ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
-				))
-		)?;
-		let mut change_output = TxOut {
-			value: Amount::from_sat(remaining_value),
-			script_pubkey: change_script,
-		};
-		let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu();
-
-		let change_output_fee = fee_for_weight(funding_feerate_sat_per_1000_weight, change_output_weight);
-		change_output.value = Amount::from_sat(remaining_value.saturating_sub(change_output_fee));
-		funding_outputs.push(change_output.clone());
-		Ok(Some(change_output))
+		Ok(funding_satoshis)
 	}
 }
 
@@ -8423,14 +8385,22 @@ impl<SP: Deref> InboundV2Channel<SP> where SP::Target: SignerProvider {
 	pub fn new<ES: Deref, F: Deref, L: Deref>(
 		fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP,
 		counterparty_node_id: PublicKey, our_supported_features: &ChannelTypeFeatures,
-		their_features: &InitFeatures, msg: &msgs::OpenChannelV2, funding_satoshis: u64,
+		their_features: &InitFeatures, msg: &msgs::OpenChannelV2,
 		funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, user_id: u128, config: &UserConfig,
 		current_chain_height: u32, logger: &L,
 	) -> Result<InboundV2Channel<SP>, ChannelError>
 		where ES::Target: EntropySource,
 			  F::Target: FeeEstimator,
 			  L::Target: Logger,
 	{
+		let funding_satoshis = calculate_our_funding_satoshis(
+			false, &funding_inputs, &[], msg.funding_feerate_sat_per_1000_weight,
+			msg.common_fields.dust_limit_satoshis
+		).map_err(|_| ChannelError::Close(
+			(
+				"Failed to accept channel".to_string(),
+				ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
+			)))?;
 		let channel_value_satoshis = funding_satoshis.saturating_add(msg.common_fields.funding_satoshis);
 		let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
 			channel_value_satoshis, msg.common_fields.dust_limit_satoshis);
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -6697,7 +6697,7 @@ where
 	/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
 	/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
 	pub fn accept_inbound_channel(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
-		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id, 0, vec![])
+		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id, vec![])
 	}
 
 	/// Accepts a request to open a channel after a [`events::Event::OpenChannelRequest`], treating
@@ -6719,7 +6719,7 @@ where
 	/// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
 	/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
 	pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
-		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id, 0, vec![])
+		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id, vec![])
 	}
 
 	/// Accepts a request to open a dual-funded channel with a contribution provided by us after an
@@ -6733,8 +6733,10 @@ where
 	/// [`Event::ChannelClosed::user_channel_id`] to allow tracking of which events correspond
 	/// with which `accept_inbound_channel_*` call.
 	///
-	/// `funding_satoshis` is the amount we are contributing to the channel.
-	/// Raises [`APIError::APIMisuseError`] when `funding_satoshis` > 2**24.
+	/// The `funding_inputs` parameter provides the `txin`s along with their previous transactions that
+	/// will be used to contribute towards our portion of the channel value. Our contribution will be
+	/// calculated as the total value of these inputs minus the fees we need to cover for the
+	/// interactive funding transaction.
 	///
 	/// Note that this method will return an error and reject the channel, if it requires support
 	/// for zero confirmations.
@@ -6747,11 +6749,11 @@ where
 	/// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
 	#[cfg(any(dual_funding, splicing))]
 	pub fn accept_inbound_channel_with_contribution(&self, temporary_channel_id: &ChannelId,
-		counterparty_node_id: &PublicKey, user_channel_id: u128, funding_satoshis: u64,
-		funding_inputs: Vec<(TxIn, Transaction)>) -> Result<(), APIError> {
+		counterparty_node_id: &PublicKey, user_channel_id: u128, funding_inputs: Vec<(TxIn, Transaction)>,
+	) -> Result<(), APIError> {
 		let funding_inputs = Self::length_limit_holder_input_prev_txs(funding_inputs)?;
 		self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id,
-			funding_satoshis, funding_inputs)
+			funding_inputs)
 	}
 
 	#[cfg(any(dual_funding, splicing))]
@@ -6768,7 +6770,7 @@ where
 	// TODO(dual_funding): Remove param _-prefix once #[cfg(dual_funding)] is dropped.
 	fn do_accept_inbound_channel(
 		&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, accept_0conf: bool,
-		user_channel_id: u128, _funding_satoshis: u64, _funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>,
+		user_channel_id: u128, _funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>,
 	) -> Result<(), APIError> {
 		let logger = WithContext::from(&self.logger, Some(*counterparty_node_id), Some(*temporary_channel_id), None);
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
@@ -6808,12 +6810,11 @@ where
 					OpenChannelMessage::V2(open_channel_msg) => {
 						let channel_res = InboundV2Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
 							counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features,
-							&open_channel_msg, _funding_satoshis, _funding_inputs, user_channel_id, &self.default_configuration, best_block_height,
+							&open_channel_msg, _funding_inputs, user_channel_id, &self.default_configuration, best_block_height,
 							&self.logger);
 						match channel_res {
 							Ok(mut channel) => {
-								let tx_msg_opt_res = channel.begin_interactive_funding_tx_construction(&self.signer_provider,
-									&self.entropy_source);
+								let tx_msg_opt_res = channel.begin_interactive_funding_tx_construction(&self.entropy_source);
 								match tx_msg_opt_res {
 									Ok(tx_msg_opt) => {
 										if let Some(tx_msg) = tx_msg_opt {
@@ -7177,7 +7178,7 @@ where
 				let user_channel_id = u128::from_be_bytes(random_bytes);
 				let mut channel = match InboundV2Channel::new(&self.fee_estimator, &self.entropy_source,
 					&self.signer_provider, counterparty_node_id.clone(), &self.channel_type_features(),
-					&peer_state.latest_features, &msg, 0, vec![], user_channel_id, &self.default_configuration,
+					&peer_state.latest_features, &msg, vec![], user_channel_id, &self.default_configuration,
 					best_block_height, &self.logger)
 				{
 					Err(e) => {
@@ -7197,7 +7198,7 @@ where
 				let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 				channel.context.set_outbound_scid_alias(outbound_scid_alias);
 
-				channel.begin_interactive_funding_tx_construction(&self.signer_provider, &self.entropy_source)
+				channel.begin_interactive_funding_tx_construction(&self.entropy_source)
 					.map_err(|_| MsgHandleErrInternal::send_err_msg_no_close(
 						"Failed to start interactive transaction construction".to_owned(), msg.common_fields.temporary_channel_id))?;
 
