Use NegotiationError to include contributed inputs and outputs

When an interactive tx sessions is aborted, the user will need to be
notified with an event indicating which inputs and outputs were
contributed. This allows them to re-use inputs that are no longer in
use. This commit introduces a NegotiationError type to achieve this,
which includes the inputs and outputs along with an AbortReason.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -60,7 +60,8 @@ use crate::ln::funding::{FundingTxInput, SpliceContribution};
 use crate::ln::interactivetxs::{
 	calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue,
 	InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend,
-	InteractiveTxSigningSession, SharedOwnedInput, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT,
+	InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, SharedOwnedOutput,
+	TX_COMMON_FIELDS_WEIGHT,
 };
 use crate::ln::msgs;
 use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
@@ -1678,12 +1679,13 @@ where
 	}
 
 	fn fail_interactive_tx_negotiation<L: Deref>(
-		&mut self, reason: AbortReason, logger: &L,
+		&mut self, error: NegotiationError, logger: &L,
 	) -> msgs::TxAbort
 	where
 		L::Target: Logger,
 	{
 		let logger = WithChannelContext::from(logger, &self.context(), None);
+		let NegotiationError { reason, .. } = error;
 		log_info!(logger, "Failed interactive transaction negotiation: {reason}");
 
 		let _interactive_tx_constructor = match &mut self.phase {
@@ -1720,11 +1722,15 @@ where
 	{
 		match self.interactive_tx_constructor_mut() {
 			Some(interactive_tx_constructor) => interactive_tx_constructor.handle_tx_add_input(msg),
-			None => Err(AbortReason::InternalError(
-				"Received unexpected interactive transaction negotiation message",
-			)),
+			None => Err(NegotiationError {
+				reason: AbortReason::InternalError(
+					"Received unexpected interactive transaction negotiation message",
+				),
+				contributed_inputs: Vec::new(),
+				contributed_outputs: Vec::new(),
+			}),
 		}
-		.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+		.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))
 	}
 
 	pub fn tx_add_output<L: Deref>(
@@ -1737,11 +1743,15 @@ where
 			Some(interactive_tx_constructor) => {
 				interactive_tx_constructor.handle_tx_add_output(msg)
 			},
-			None => Err(AbortReason::InternalError(
-				"Received unexpected interactive transaction negotiation message",
-			)),
+			None => Err(NegotiationError {
+				reason: AbortReason::InternalError(
+					"Received unexpected interactive transaction negotiation message",
+				),
+				contributed_inputs: Vec::new(),
+				contributed_outputs: Vec::new(),
+			}),
 		}
-		.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+		.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))
 	}
 
 	pub fn tx_remove_input<L: Deref>(
@@ -1754,11 +1764,15 @@ where
 			Some(interactive_tx_constructor) => {
 				interactive_tx_constructor.handle_tx_remove_input(msg)
 			},
-			None => Err(AbortReason::InternalError(
-				"Received unexpected interactive transaction negotiation message",
-			)),
+			None => Err(NegotiationError {
+				reason: AbortReason::InternalError(
+					"Received unexpected interactive transaction negotiation message",
+				),
+				contributed_inputs: Vec::new(),
+				contributed_outputs: Vec::new(),
+			}),
 		}
-		.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+		.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))
 	}
 
 	pub fn tx_remove_output<L: Deref>(
@@ -1771,11 +1785,15 @@ where
 			Some(interactive_tx_constructor) => {
 				interactive_tx_constructor.handle_tx_remove_output(msg)
 			},
-			None => Err(AbortReason::InternalError(
-				"Received unexpected interactive transaction negotiation message",
-			)),
+			None => Err(NegotiationError {
+				reason: AbortReason::InternalError(
+					"Received unexpected interactive transaction negotiation message",
+				),
+				contributed_inputs: Vec::new(),
+				contributed_outputs: Vec::new(),
+			}),
 		}
-		.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))
+		.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))
 	}
 
 	pub fn tx_complete<L: Deref>(
@@ -1786,11 +1804,15 @@ where
 	{
 		let tx_complete_action = match self.interactive_tx_constructor_mut() {
 			Some(interactive_tx_constructor) => interactive_tx_constructor.handle_tx_complete(msg),
-			None => Err(AbortReason::InternalError(
-				"Received unexpected interactive transaction negotiation message",
-			)),
+			None => Err(NegotiationError {
+				reason: AbortReason::InternalError(
+					"Received unexpected interactive transaction negotiation message",
+				),
+				contributed_inputs: Vec::new(),
+				contributed_outputs: Vec::new(),
+			}),
 		}
-		.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?;
+		.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))?;
 
 		let (interactive_tx_msg_send, negotiation_complete) = match tx_complete_action {
 			HandleTxCompleteValue::SendTxMessage(interactive_tx_msg_send) => {
@@ -1808,7 +1830,7 @@ where
 
 		let commitment_signed = self
 			.funding_tx_constructed(logger)
-			.map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?;
+			.map_err(|e| self.fail_interactive_tx_negotiation(e, logger))?;
 		Ok((interactive_tx_msg_send, Some(commitment_signed)))
 	}
 
@@ -1891,7 +1913,7 @@ where
 
 	fn funding_tx_constructed<L: Deref>(
 		&mut self, logger: &L,
-	) -> Result<msgs::CommitmentSigned, AbortReason>
+	) -> Result<msgs::CommitmentSigned, NegotiationError>
 	where
 		L::Target: Logger,
 	{
@@ -1949,15 +1971,23 @@ where
 					}
 				}
 
-				return Err(AbortReason::InternalError(
-					"Got a tx_complete message in an invalid state",
-				));
+				return Err(NegotiationError {
+					reason: AbortReason::InternalError(
+						"Got a tx_complete message in an invalid state",
+					),
+					contributed_inputs: Vec::new(),
+					contributed_outputs: Vec::new(),
+				});
 			},
 			_ => {
 				debug_assert!(false);
-				return Err(AbortReason::InternalError(
-					"Got a tx_complete message in an invalid phase",
-				));
+				return Err(NegotiationError {
+					reason: AbortReason::InternalError(
+						"Got a tx_complete message in an invalid phase",
+					),
+					contributed_inputs: Vec::new(),
+					contributed_outputs: Vec::new(),
+				});
 			},
 		}
 	}
@@ -6053,7 +6083,7 @@ where
 	fn funding_tx_constructed<L: Deref>(
 		&mut self, funding: &mut FundingScope, signing_session: InteractiveTxSigningSession,
 		is_splice: bool, holder_commitment_transaction_number: u64, logger: &L
-	) -> Result<msgs::CommitmentSigned, AbortReason>
+	) -> Result<msgs::CommitmentSigned, NegotiationError>
 	where
 		L::Target: Logger
 	{
@@ -6062,15 +6092,17 @@ where
 		for (idx, outp) in signing_session.unsigned_tx().tx().output.iter().enumerate() {
 			if outp.script_pubkey == expected_spk && outp.value.to_sat() == funding.get_value_satoshis() {
 				if output_index.is_some() {
-					return Err(AbortReason::DuplicateFundingOutput);
+					let reason = AbortReason::DuplicateFundingOutput;
+					return Err(signing_session.into_negotiation_error(reason));
 				}
 				output_index = Some(idx as u16);
 			}
 		}
 		let outpoint = if let Some(output_index) = output_index {
 			OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index }
 		} else {
-			return Err(AbortReason::MissingFundingOutput);
+			let reason = AbortReason::MissingFundingOutput;
+			return Err(signing_session.into_negotiation_error(reason));
 		};
 		funding
 			.channel_transaction_parameters.funding_outpoint = Some(outpoint);
@@ -6092,7 +6124,9 @@ where
 			// TODO(splicing): Support async signing
 			None => {
 				funding.channel_transaction_parameters.funding_outpoint = None;
-				return Err(AbortReason::InternalError("Failed to compute commitment_signed signatures"));
+				let signing_session = self.interactive_tx_signing_session.take().unwrap();
+				let reason = AbortReason::InternalError("Failed to compute commitment_signed signatures");
+				return Err(signing_session.into_negotiation_error(reason));
 			},
 		};
 
@@ -6508,9 +6542,9 @@ impl FundingNegotiationContext {
 	/// Prepare and start interactive transaction negotiation.
 	/// If error occurs, it is caused by our side, not the counterparty.
 	fn into_interactive_tx_constructor<SP: Deref, ES: Deref>(
-		self, context: &ChannelContext<SP>, funding: &FundingScope, signer_provider: &SP,
+		mut self, context: &ChannelContext<SP>, funding: &FundingScope, signer_provider: &SP,
 		entropy_source: &ES, holder_node_id: PublicKey,
-	) -> Result<InteractiveTxConstructor, AbortReason>
+	) -> Result<InteractiveTxConstructor, NegotiationError>
 	where
 		SP::Target: SignerProvider,
 		ES::Target: EntropySource,
@@ -6536,25 +6570,32 @@ impl FundingNegotiationContext {
 
 		// Optionally add change output
 		let change_value_opt = if self.our_funding_contribution > SignedAmount::ZERO {
-			calculate_change_output_value(
+			match calculate_change_output_value(
 				&self,
 				self.shared_funding_input.is_some(),
 				&shared_funding_output.script_pubkey,
 				context.holder_dust_limit_satoshis,
-			)?
+			) {
+				Ok(change_value_opt) => change_value_opt,
+				Err(reason) => {
+					return Err(self.into_negotiation_error(reason));
+				},
+			}
 		} else {
 			None
 		};
 
-		let mut funding_outputs = self.our_funding_outputs;
-
 		if let Some(change_value) = change_value_opt {
 			let change_script = if let Some(script) = self.change_script {
 				script
 			} else {
-				signer_provider
-					.get_destination_script(context.channel_keys_id)
-					.map_err(|_err| AbortReason::InternalError("Error getting change script"))?
+				match signer_provider.get_destination_script(context.channel_keys_id) {
+					Ok(script) => script,
+					Err(_) => {
+						let reason = AbortReason::InternalError("Error getting change script");
+						return Err(self.into_negotiation_error(reason));
+					},
+				}
 			};
 			let mut change_output =
 				TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script };
@@ -6565,7 +6606,7 @@ impl FundingNegotiationContext {
 			// Check dust limit again
 			if change_value_decreased_with_fee > context.holder_dust_limit_satoshis {
 				change_output.value = Amount::from_sat(change_value_decreased_with_fee);
-				funding_outputs.push(change_output);
+				self.our_funding_outputs.push(change_output);
 			}
 		}
 
@@ -6583,10 +6624,19 @@ impl FundingNegotiationContext {
 				shared_funding_output,
 				funding.value_to_self_msat / 1000,
 			),
-			outputs_to_contribute: funding_outputs,
+			outputs_to_contribute: self.our_funding_outputs,
 		};
 		InteractiveTxConstructor::new(constructor_args)
 	}
+
+	fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError {
+		let contributed_inputs =
+			self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect();
+
+		let contributed_outputs = self.our_funding_outputs;
+
+		NegotiationError { reason, contributed_inputs, contributed_outputs }
+	}
 }
 
 // Holder designates channel data owned for the benefit of the user client.
@@ -13660,8 +13710,8 @@ where
 				outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(),
 			}
 		).map_err(|err| {
-			let reason = ClosureReason::ProcessingError { err: err.to_string() };
-			ChannelError::Close((err.to_string(), reason))
+			let reason = ClosureReason::ProcessingError { err: err.reason.to_string() };
+			ChannelError::Close((err.reason.to_string(), reason))
 		})?);
 
 		let unfunded_context = UnfundedChannelContext {