fix Minor changes, error enum, comments

Author: optout21

lightning/src/ln/interactivetxs.rs

@@ -108,6 +108,10 @@ pub(crate) enum AbortReason {
 	InvalidTx,
 	/// No funding (shared) input found.
 	MissingFundingInput,
+	/// A funding (shared) input was seen, but we don't expect one
+	UnexpectedFundingInput,
+	/// In tx_add_input, the prev_tx field must be filled in case of non-shared input
+	MissingPrevTx,
 	/// No funding (shared) output found.
 	MissingFundingOutput,
 	/// More than one funding (shared) output found.
@@ -165,6 +169,12 @@ impl Display for AbortReason {
 			},
 			AbortReason::InvalidTx => f.write_str("The transaction is invalid"),
 			AbortReason::MissingFundingInput => f.write_str("No shared funding input found"),
+			AbortReason::UnexpectedFundingInput => {
+				f.write_str("A funding (shared) input was seen, but we don't expect one")
+			},
+			AbortReason::MissingPrevTx => f.write_str(
+				"In tx_add_input, the prev_tx field must be filled in case of non-shared input",
+			),
 			AbortReason::MissingFundingOutput => f.write_str("No shared funding output found"),
 			AbortReason::DuplicateFundingOutput => {
 				f.write_str("More than one funding output found")
@@ -482,10 +492,10 @@ struct NegotiationContext {
 	/// - For the initiator:
 	/// The intended previous funding input. This will be added alongside to the
 	/// provided inputs.
-	/// The values are the output value and the the holder's part of the shared input.
+	/// The values are the output value and the holder's part of the shared input.
 	/// - For the acceptor:
 	/// The expected previous funding input. It should be added by the initiator node.
-	/// The values are the output value and the the holder's part of the shared input.
+	/// The values are the output value and the holder's part of the shared input.
 	shared_funding_input: Option<(OutPoint, u64, u64)>,
 	/// The intended/extended funding output, potentially co-owned by both peers (shared).
 	/// - For the initiator:
@@ -620,19 +630,16 @@ impl NegotiationContext {
 
 		// Extract info from msg, check if shared
 		let (input, prev_outpoint) = if let Some(shared_txid) = &msg.shared_input_txid {
-			// This is a shared input
 			if self.holder_is_initiator {
 				return Err(AbortReason::DuplicateFundingInput);
 			}
 			if let Some(shared_funding_input) = &self.shared_funding_input {
-				// There can only be one shared output.
 				if self.inputs.values().any(|input| matches!(input.input, InputOwned::Shared(_))) {
 					return Err(AbortReason::DuplicateFundingInput);
 				}
-				// Check if receied shared input matches the expected
+				// Check if received shared input matches the expected
 				if shared_funding_input.0.txid != *shared_txid {
-					// Shared input TXID differs from expected
-					return Err(AbortReason::MissingFundingInput);
+					return Err(AbortReason::UnexpectedFundingInput);
 				} else {
 					let previous_output = OutPoint { txid: *shared_txid, vout: msg.prevtx_out };
 					let txin = TxIn {
@@ -649,11 +656,9 @@ impl NegotiationContext {
 					(InputOwned::Shared(shared_input), previous_output)
 				}
 			} else {
-				// Unexpected shared input received
-				return Err(AbortReason::MissingFundingInput);
+				return Err(AbortReason::UnexpectedFundingInput);
 			}
 		} else {
-			// Non-shared input
 			if let Some(prevtx) = &msg.prevtx {
 				let transaction = prevtx.as_transaction();
 				let txid = transaction.compute_txid();
@@ -687,7 +692,7 @@ impl NegotiationContext {
 					return Err(AbortReason::PrevTxOutInvalid);
 				}
 			} else {
-				return Err(AbortReason::MissingFundingInput);
+				return Err(AbortReason::MissingPrevTx);
 			}
 		};
 
@@ -711,7 +716,6 @@ impl NegotiationContext {
 					//       (and not removed) input's
 					return Err(AbortReason::PrevTxOutInvalid);
 				}
-				self.prevtx_outpoints.insert(prev_outpoint);
 
 				Ok(())
 			},
@@ -791,11 +795,9 @@ impl NegotiationContext {
 
 		let txout = TxOut { value: Amount::from_sat(msg.sats), script_pubkey: msg.script.clone() };
 		let output = if txout == self.shared_funding_output.tx_out {
-			// This is a shared output
 			if self.holder_is_initiator {
 				return Err(AbortReason::DuplicateFundingOutput);
 			}
-			// There can only be one shared output.
 			if self.outputs.values().any(|output| matches!(output.output, OutputOwned::Shared(_))) {
 				return Err(AbortReason::DuplicateFundingOutput);
 			}
@@ -837,7 +839,6 @@ impl NegotiationContext {
 	fn sent_tx_add_input(&mut self, msg: &msgs::TxAddInput) -> Result<(), AbortReason> {
 		let vout = msg.prevtx_out as usize;
 		let (prev_outpoint, input) = if let Some(shared_input_txid) = msg.shared_input_txid {
-			// This is the shared input
 			let prev_outpoint = OutPoint { txid: shared_input_txid, vout: msg.prevtx_out };
 			let txin = TxIn {
 				previous_output: prev_outpoint,
@@ -860,10 +861,9 @@ impl NegotiationContext {
 					InputOwned::Shared(SharedOwnedInput::new(txin, prev_output, local_owned)),
 				)
 			} else {
-				return Err(AbortReason::MissingFundingInput);
+				return Err(AbortReason::UnexpectedFundingInput);
 			}
 		} else {
-			// Non-shared input
 			if let Some(prevtx) = &msg.prevtx {
 				let prev_txid = prevtx.as_transaction().compute_txid();
 				let prev_outpoint = OutPoint { txid: prev_txid, vout: msg.prevtx_out };
@@ -898,7 +898,6 @@ impl NegotiationContext {
 	fn sent_tx_add_output(&mut self, msg: &msgs::TxAddOutput) -> Result<(), AbortReason> {
 		let txout = TxOut { value: Amount::from_sat(msg.sats), script_pubkey: msg.script.clone() };
 		let output = if txout == self.shared_funding_output.tx_out {
-			// this is the shared output
 			OutputOwned::Shared(self.shared_funding_output.clone())
 		} else {
 			OutputOwned::Single(txout)
@@ -2956,7 +2955,7 @@ mod tests {
 			b_shared_input: None,
 			shared_output_b: generate_funding_txout(108_000, 0),
 			outputs_b: vec![],
-			expect_error: Some((AbortReason::MissingFundingInput, ErrorCulprit::NodeA)),
+			expect_error: Some((AbortReason::UnexpectedFundingInput, ErrorCulprit::NodeA)),
 		});
 	}