Implement logic for handling last tx_complete, signature for shared input

Author: optout21

lightning/src/ln/channel.rs

@@ -30,7 +30,7 @@ use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentPreimage, PaymentHash};
 use crate::types::features::{ChannelTypeFeatures, InitFeatures};
 use crate::ln::interactivetxs::{
-	calculate_change_output_value, get_output_weight, HandleTxCompleteValue, HandleTxCompleteResult, InteractiveTxConstructor,
+	calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue, HandleTxCompleteResult, InteractiveTxConstructor,
 	InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, InteractiveTxMessageSendResult,
 	OutputOwned, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT,
 };
@@ -2223,13 +2223,15 @@ impl<SP: Deref> InitialRemoteCommitmentReceiver<SP> for FundedChannel<SP> where
 
 impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	/// Prepare and start interactive transaction negotiation.
-	/// `change_destination_opt` - Optional destination for optional change; if None, default destination address is used.
+	/// `change_destination_opt` - Optional destination for optional change; if None,
+	///   default destination address is used.
+	/// If error occurs, it is caused by our side, not the counterparty.
 	#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled
 	fn begin_interactive_funding_tx_construction<ES: Deref>(
 		&mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey,
 		change_destination_opt: Option<ScriptBuf>,
 		prev_funding_input: Option<(TxIn, TransactionU16LenLimited)>,
-	) -> Result<Option<InteractiveTxMessageSend>, ChannelError>
+	) -> Result<Option<InteractiveTxMessageSend>, AbortReason>
 	where ES::Target: EntropySource
 	{
 		debug_assert!(self.interactive_tx_constructor.is_none());
@@ -2273,19 +2275,14 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			script
 		} else {
 			signer_provider.get_destination_script(self.context.channel_keys_id)
-				.map_err(|err| ChannelError::Warn(format!(
-					"Failed to get change script as new destination script, {:?}", err,
-				)))?
+				.map_err(|_err| AbortReason::InternalErrorGettingDestinationScript)?
 		};
 		let change_value_opt = calculate_change_output_value(
 			self.funding.is_outbound(), self.dual_funding_context.our_funding_satoshis,
 			&funding_inputs_prev_outputs, &funding_outputs,
 			self.dual_funding_context.funding_feerate_sat_per_1000_weight,
 			change_script.minimal_non_dust().to_sat(),
-		).map_err(|err| ChannelError::Warn(format!(
-			"Insufficient inputs, cannot cover intended contribution of {} and fees; {}",
-			self.dual_funding_context.our_funding_satoshis, err
-		)))?;
+		)?;
 		if let Some(change_value) = change_value_opt {
 			let mut change_output = TxOut {
 				value: Amount::from_sat(change_value),
@@ -2313,8 +2310,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 			outputs_to_contribute: funding_outputs,
 			expected_remote_shared_funding_output,
 		};
-		let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)
-			.map_err(|_| ChannelError::Warn("Incorrect shared output provided".into()))?;
+		let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?;
 		let msg = tx_constructor.take_initiator_first_message();
 
 		self.interactive_tx_constructor = Some(tx_constructor);
@@ -4999,23 +4995,18 @@ impl DualFundingChannelContext {
 	/// Obtain prev outputs for each supplied input and matching transaction.
 	/// Will error when a prev tx does not have an output for the specified vout.
 	/// Also checks for matching of transaction IDs.
-	fn txouts_from_input_prev_txs(inputs: &Vec<(TxIn, TransactionU16LenLimited)>) -> Result<Vec<&TxOut>, ChannelError> {
+	fn txouts_from_input_prev_txs(inputs: &Vec<(TxIn, TransactionU16LenLimited)>) -> Result<Vec<&TxOut>, AbortReason> {
 		let mut prev_outputs: Vec<&TxOut> = Vec::with_capacity(inputs.len());
 		// Check that vouts exist for each TxIn in provided transactions.
 		for (idx, (txin, tx)) in inputs.iter().enumerate() {
 			let txid = tx.as_transaction().compute_txid();
 			if txin.previous_output.txid != txid {
-				return Err(ChannelError::Warn(
-					format!("Transaction input txid mismatch, {} vs. {}, at index {}", txin.previous_output.txid, txid, idx)
-				));
+				return Err(AbortReason::ProvidedInputsAndPrevtxsTxIdMismatch(idx as u32));
 			}
 			if let Some(output) = tx.as_transaction().output.get(txin.previous_output.vout as usize) {
 				prev_outputs.push(output);
 			} else {
-				return Err(ChannelError::Warn(
-					format!("Transaction with txid {} does not have an output with vout of {} corresponding to TxIn, at index {}",
-						txid, txin.previous_output.vout, idx)
-				));
+				return Err(AbortReason::ProvidedInputsAndPrevtxsVoutNotFound(idx as u32));
 			}
 		}
 		Ok(prev_outputs)

lightning/src/ln/interactivetxs.rs

@@ -108,6 +108,12 @@ pub(crate) enum AbortReason {
 	/// if funding output is provided by the peer this is an interop error,
 	/// if provided by the same node than internal input consistency error.
 	InvalidLowFundingOutputValue,
+	/// TxId mismatch in the provided inputs and previous transactions, input index in data
+	ProvidedInputsAndPrevtxsTxIdMismatch(u32),
+	/// A vout provided in an input is not found in the matching previous transaction, input index in data
+	ProvidedInputsAndPrevtxsVoutNotFound(u32),
+	/// Internal error, error while getting destination script
+	InternalErrorGettingDestinationScript,
 }
 
 impl AbortReason {
@@ -118,34 +124,44 @@ impl AbortReason {
 
 impl Display for AbortReason {
 	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-		f.write_str(match self {
-			AbortReason::InvalidStateTransition => "State transition was invalid",
-			AbortReason::UnexpectedCounterpartyMessage => "Unexpected message",
-			AbortReason::ReceivedTooManyTxAddInputs => "Too many `tx_add_input`s received",
-			AbortReason::ReceivedTooManyTxAddOutputs => "Too many `tx_add_output`s received",
+		f.write_str(&match self {
+			AbortReason::InvalidStateTransition => "State transition was invalid".into(),
+			AbortReason::UnexpectedCounterpartyMessage => "Unexpected message".into(),
+			AbortReason::ReceivedTooManyTxAddInputs => "Too many `tx_add_input`s received".into(),
+			AbortReason::ReceivedTooManyTxAddOutputs => "Too many `tx_add_output`s received".into(),
 			AbortReason::IncorrectInputSequenceValue => {
-				"Input has a sequence value greater than 0xFFFFFFFD"
+				"Input has a sequence value greater than 0xFFFFFFFD".into()
 			},
-			AbortReason::IncorrectSerialIdParity => "Parity for `serial_id` was incorrect",
-			AbortReason::SerialIdUnknown => "The `serial_id` is unknown",
-			AbortReason::DuplicateSerialId => "The `serial_id` already exists",
-			AbortReason::PrevTxOutInvalid => "Invalid previous transaction output",
+			AbortReason::IncorrectSerialIdParity => "Parity for `serial_id` was incorrect".into(),
+			AbortReason::SerialIdUnknown => "The `serial_id` is unknown".into(),
+			AbortReason::DuplicateSerialId => "The `serial_id` already exists".into(),
+			AbortReason::PrevTxOutInvalid => "Invalid previous transaction output".into(),
 			AbortReason::ExceededMaximumSatsAllowed => {
-				"Output amount exceeded total bitcoin supply"
+				"Output amount exceeded total bitcoin supply".into()
 			},
-			AbortReason::ExceededNumberOfInputsOrOutputs => "Too many inputs or outputs",
-			AbortReason::TransactionTooLarge => "Transaction weight is too large",
-			AbortReason::BelowDustLimit => "Output amount is below the dust limit",
-			AbortReason::InvalidOutputScript => "The output script is non-standard",
-			AbortReason::InsufficientFees => "Insufficient fees paid",
+			AbortReason::ExceededNumberOfInputsOrOutputs => "Too many inputs or outputs".into(),
+			AbortReason::TransactionTooLarge => "Transaction weight is too large".into(),
+			AbortReason::BelowDustLimit => "Output amount is below the dust limit".into(),
+			AbortReason::InvalidOutputScript => "The output script is non-standard".into(),
+			AbortReason::InsufficientFees => "Insufficient fees paid".into(),
 			AbortReason::OutputsValueExceedsInputsValue => {
-				"Total value of outputs exceeds total value of inputs"
+				"Total value of outputs exceeds total value of inputs".into()
 			},
-			AbortReason::InvalidTx => "The transaction is invalid",
-			AbortReason::MissingFundingOutput => "No shared funding output found",
-			AbortReason::DuplicateFundingOutput => "More than one funding output found",
+			AbortReason::InvalidTx => "The transaction is invalid".into(),
+			AbortReason::MissingFundingOutput => "No shared funding output found".into(),
+			AbortReason::DuplicateFundingOutput => "More than one funding output found".into(),
 			AbortReason::InvalidLowFundingOutputValue => {
-				"Local part of funding output value is greater than the funding output value"
+				"Local part of funding output value is greater than the funding output value".into()
+			},
+			AbortReason::ProvidedInputsAndPrevtxsTxIdMismatch(idx) => format!(
+				"TxId mismatch in the provided inputs and previous transactions, input index {}",
+				idx
+			),
+			AbortReason::ProvidedInputsAndPrevtxsVoutNotFound(idx) => format!(
+				"Vout provided in an input is not found in the previous transaction, input index {}",
+				idx),
+			AbortReason::InternalErrorGettingDestinationScript => {
+				"Internal error getting destination script".into()
 			},
 		})
 	}