Fixups for #2419

[dunxen]

Fixes #2955.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.82540% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 90.22%. Comparing base (ac9a2c8) to head (937f8bf).
Report is 39 commits behind head on main.

Files	Patch %	Lines
lightning/src/ln/interactivetxs.rs	97.34%	8 Missing and 2 partials ⚠️
lightning/src/sign/mod.rs	0.00%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2981      +/-   ##
==========================================
+ Coverage   89.42%   90.22%   +0.79%     
==========================================
  Files         117      118       +1     
  Lines       96290   104849    +8559     
  Branches    96290   104849    +8559     
==========================================
+ Hits        86109    94600    +8491     
- Misses       7962     8103     +141     
+ Partials     2219     2146      -73     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dunxen]

The most recent commit is a rough exploration of how we might handle splice in and out. Definitely needs polish.

[optout21]

LGTM

[TheBlueMatt]

Just some questions about the last commit, sorry for the delay here.

[optout21]

LGTM

[TheBlueMatt]

LGTM.

[dunxen]

Updated last commit. Sorry, there was rebase on main that I did earlier that made it through but these are the changes for this last push:

diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index e514ae63..5ba67ff2 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -182,3 +182,23 @@ impl ConstructedTransaction {

-	pub fn build_unsigned_tx(&self) -> Result<Transaction, AbortReason> {
+	pub fn weight(&self) -> u64 {
+		let inputs_weight = self.remote_inputs.iter().chain(self.local_inputs.iter()).fold(
+			0u64,
+			|weight, InteractiveTxInput { prev_output, .. }| {
+				weight.saturating_add(estimate_input_weight(prev_output))
+			},
+		);
+		let outputs_weight = self.remote_outputs.iter().chain(self.local_outputs.iter()).fold(
+			0u64,
+			|weight, InteractiveTxOutput { tx_out, .. }| {
+				weight.saturating_add(get_output_weight(&tx_out.script_pubkey))
+			},
+		);
+
+		let tx =
+			Transaction { version: 2, lock_time: self.lock_time, input: vec![], output: vec![] };
+
+		tx.weight().to_wu().saturating_add(inputs_weight).saturating_add(outputs_weight)
+	}
+
+	pub fn into_unsigned_tx(self) -> Result<Transaction, AbortReason> {
 		// Inputs and outputs must be sorted by serial_id
@@ -194,5 +214,5 @@ impl ConstructedTransaction {
 		let input: Vec<TxIn> =
-			inputs.clone().into_iter().map(|InteractiveTxInput { input, .. }| input).collect();
+			inputs.into_iter().map(|InteractiveTxInput { input, .. }| input).collect();
 		let output: Vec<TxOut> =
-			outputs.clone().into_iter().map(|InteractiveTxOutput { tx_out, .. }| tx_out).collect();
+			outputs.into_iter().map(|InteractiveTxOutput { tx_out, .. }| tx_out).collect();

@@ -219,2 +239,14 @@ struct NegotiationContext {

+pub(crate) fn estimate_input_weight(prev_output: &TxOut) -> u64 {
+	if prev_output.script_pubkey.is_v0_p2wpkh() {
+		P2WPKH_INPUT_WEIGHT_LOWER_BOUND
+	} else if prev_output.script_pubkey.is_v0_p2wsh() {
+		P2WSH_INPUT_WEIGHT_LOWER_BOUND
+	} else if prev_output.script_pubkey.is_v1_p2tr() {
+		P2TR_INPUT_WEIGHT_LOWER_BOUND
+	} else {
+		UNKNOWN_SEGWIT_VERSION_INPUT_WEIGHT_LOWER_BOUND
+	}
+}
+
 pub(crate) fn get_output_weight(script_pubkey: &ScriptBuf) -> u64 {
@@ -482,11 +514,3 @@ impl NegotiationContext {
 			total_inputs_weight =
-				total_inputs_weight.saturating_add(if prev_output.script_pubkey.is_v0_p2wpkh() {
-					P2WPKH_INPUT_WEIGHT_LOWER_BOUND
-				} else if prev_output.script_pubkey.is_v0_p2wsh() {
-					P2WSH_INPUT_WEIGHT_LOWER_BOUND
-				} else if prev_output.script_pubkey.is_v1_p2tr() {
-					P2TR_INPUT_WEIGHT_LOWER_BOUND
-				} else {
-					UNKNOWN_SEGWIT_VERSION_INPUT_WEIGHT_LOWER_BOUND
-				});
+				total_inputs_weight.saturating_add(estimate_input_weight(prev_output));
 		}
@@ -552,4 +576,3 @@ impl NegotiationContext {

-		let unsigned_tx = constructed_tx.build_unsigned_tx()?;
-		if unsigned_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 {
+		if constructed_tx.weight() > MAX_STANDARD_TX_WEIGHT as u64 {
 			return Err(AbortReason::TransactionTooLarge);
@@ -1233,4 +1256,4 @@ mod tests {
 		assert_eq!(
-			final_tx_a.unwrap().build_unsigned_tx().unwrap(),
-			final_tx_b.unwrap().build_unsigned_tx().unwrap()
+			final_tx_a.unwrap().into_unsigned_tx().unwrap(),
+			final_tx_b.unwrap().into_unsigned_tx().unwrap()
 		);

[dunxen]

Changes: git diff-tree -U1 00edceff6 3ced4e329

git diff-tree -U1 00edceff6 3ced4e329
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 5ba67ff2..9f8ca4d1 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -126,6 +126,2 @@ pub(crate) struct ConstructedTransaction {

-	remote_fees: u64,
-	local_fees: u64,
-	total_fees: u64,
-
 	lock_time: AbsoluteLockTime,
@@ -157,6 +153,2 @@ impl ConstructedTransaction {

-		let local_fees = local_inputs_value_satoshis.saturating_sub(local_outputs_value_satoshis);
-		let remote_fees =
-			remote_inputs_value_satoshis.saturating_sub(remote_outputs_value_satoshis);
-
 		Self {
@@ -174,6 +166,2 @@ impl ConstructedTransaction {

-			local_fees,
-			remote_fees,
-			total_fees: local_fees.saturating_add(remote_fees),
-
 			lock_time,
@@ -202,3 +190,3 @@ impl ConstructedTransaction {

-	pub fn into_unsigned_tx(self) -> Result<Transaction, AbortReason> {
+	pub fn into_unsigned_tx(self) -> Transaction {
 		// Inputs and outputs must be sorted by serial_id
@@ -207,5 +195,5 @@ impl ConstructedTransaction {
 		let mut inputs: Vec<InteractiveTxInput> =
-			inputs.iter().chain(self.remote_inputs.iter()).map(|input| input.clone()).collect();
+			inputs.into_iter().chain(self.remote_inputs.into_iter()).collect();
 		let mut outputs: Vec<InteractiveTxOutput> =
-			outputs.iter().chain(self.remote_outputs.iter()).map(|input| input.clone()).collect();
+			outputs.into_iter().chain(self.remote_outputs.into_iter()).collect();
 		inputs.sort_unstable_by_key(|InteractiveTxInput { serial_id, .. }| *serial_id);
@@ -218,5 +206,3 @@ impl ConstructedTransaction {

-		let unsigned_tx = Transaction { version: 2, lock_time: self.lock_time, input, output };
-
-		Ok(unsigned_tx)
+		Transaction { version: 2, lock_time: self.lock_time, input, output }
 	}
@@ -229,7 +215,5 @@ struct NegotiationContext {
 	received_tx_add_output_count: u16,
-	local_inputs: HashMap<SerialId, InteractiveTxInput>,
-	remote_inputs: HashMap<SerialId, InteractiveTxInput>,
+	inputs: HashMap<SerialId, InteractiveTxInput>,
 	prevtx_outpoints: HashSet<OutPoint>,
-	local_outputs: HashMap<SerialId, InteractiveTxOutput>,
-	remote_outputs: HashMap<SerialId, InteractiveTxOutput>,
+	outputs: HashMap<SerialId, InteractiveTxOutput>,
 	tx_locktime: AbsoluteLockTime,
@@ -256,18 +240,50 @@ pub(crate) fn get_output_weight(script_pubkey: &ScriptBuf) -> u64 {

+fn is_serial_id_valid_for_counterparty(holder_is_initiator: bool, serial_id: &SerialId) -> bool {
+	// A received `SerialId`'s parity must match the role of the counterparty.
+	holder_is_initiator == serial_id.is_for_non_initiator()
+}
+
 impl NegotiationContext {
 	fn is_serial_id_valid_for_counterparty(&self, serial_id: &SerialId) -> bool {
-		// A received `SerialId`'s parity must match the role of the counterparty.
-		self.holder_is_initiator == serial_id.is_for_non_initiator()
+		is_serial_id_valid_for_counterparty(self.holder_is_initiator, serial_id)
 	}

-	fn total_input_count(&self) -> usize {
-		self.local_inputs.len().saturating_add(self.remote_inputs.len())
+	fn remote_inputs_value(&self) -> u64 {
+		self.inputs
+			.iter()
+			.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+			.fold(0u64, |acc, (_, InteractiveTxInput { prev_output, .. })| {
+				acc.saturating_add(prev_output.value)
+			})
 	}

-	fn total_output_count(&self) -> usize {
-		self.local_outputs.len().saturating_add(self.remote_outputs.len())
+	fn remote_outputs_value(&self) -> u64 {
+		self.outputs
+			.iter()
+			.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+			.fold(0u64, |acc, (_, InteractiveTxOutput { tx_out, .. })| {
+				acc.saturating_add(tx_out.value)
+			})
+	}
+
+	fn remote_inputs_weight(&self) -> u64 {
+		self.inputs
+			.iter()
+			.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+			.fold(0u64, |weight, (_, InteractiveTxInput { prev_output, .. })| {
+				weight.saturating_add(estimate_input_weight(prev_output))
+			})
 	}

-	fn total_input_and_output_count(&self) -> usize {
-		self.total_input_count().saturating_add(self.total_output_count())
+	fn remote_outputs_weight(&self) -> u64 {
+		self.outputs
+			.iter()
+			.filter_map(|(serial_id, output)| {
+				if self.is_serial_id_valid_for_counterparty(serial_id) {
+					Some(get_output_weight(&output.tx_out.script_pubkey))
+				} else {
+					None
+				}
+			})
+			.sum()
 	}
@@ -332,4 +348,3 @@ impl NegotiationContext {
 		};
-		let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out };
-		match self.remote_inputs.entry(msg.serial_id) {
+		match self.inputs.entry(msg.serial_id) {
 			hash_map::Entry::Occupied(_) => {
@@ -338,5 +353,6 @@ impl NegotiationContext {
 				//    - the `serial_id` is already included in the transaction
-				return Err(AbortReason::DuplicateSerialId);
+				Err(AbortReason::DuplicateSerialId)
 			},
 			hash_map::Entry::Vacant(entry) => {
+				let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out };
 				entry.insert(InteractiveTxInput {
@@ -350,6 +366,6 @@ impl NegotiationContext {
 				});
+				self.prevtx_outpoints.insert(prev_outpoint);
+				Ok(())
 			},
 		}
-		self.prevtx_outpoints.insert(prev_outpoint);
-		Ok(())
 	}
@@ -361,3 +377,3 @@ impl NegotiationContext {

-		self.remote_inputs
+		self.inputs
 			.remove(&msg.serial_id)
@@ -397,3 +413,3 @@ impl NegotiationContext {
 		let mut outputs_value: u64 = 0;
-		for output in self.remote_outputs.iter() {
+		for output in self.outputs.iter() {
 			outputs_value = outputs_value.saturating_add(output.1.tx_out.value);
@@ -427,7 +443,3 @@ impl NegotiationContext {

-		let output = InteractiveTxOutput {
-			serial_id: msg.serial_id,
-			tx_out: TxOut { value: msg.sats, script_pubkey: msg.script.clone() },
-		};
-		match self.remote_outputs.entry(msg.serial_id) {
+		match self.outputs.entry(msg.serial_id) {
 			hash_map::Entry::Occupied(_) => {
@@ -436,9 +448,12 @@ impl NegotiationContext {
 				//    - the `serial_id` is already included in the transaction
-				return Err(AbortReason::DuplicateSerialId);
+				Err(AbortReason::DuplicateSerialId)
 			},
 			hash_map::Entry::Vacant(entry) => {
-				entry.insert(output);
+				entry.insert(InteractiveTxOutput {
+					serial_id: msg.serial_id,
+					tx_out: TxOut { value: msg.sats, script_pubkey: msg.script.clone() },
+				});
+				Ok(())
 			},
-		};
-		Ok(())
+		}
 	}
@@ -449,3 +464,3 @@ impl NegotiationContext {
 		}
-		if let Some(_) = self.remote_outputs.remove(&msg.serial_id) {
+		if let Some(_) = self.outputs.remove(&msg.serial_id) {
 			Ok(())
@@ -473,3 +488,3 @@ impl NegotiationContext {
 		}
-		self.local_inputs.insert(
+		self.inputs.insert(
 			msg.serial_id,
@@ -481,3 +496,3 @@ impl NegotiationContext {
 	fn sent_tx_add_output(&mut self, msg: &msgs::TxAddOutput) -> Result<(), AbortReason> {
-		self.local_outputs.insert(
+		self.outputs.insert(
 			msg.serial_id,
@@ -492,3 +507,3 @@ impl NegotiationContext {
 	fn sent_tx_remove_input(&mut self, msg: &msgs::TxRemoveInput) -> Result<(), AbortReason> {
-		self.local_inputs.remove(&msg.serial_id);
+		self.inputs.remove(&msg.serial_id);
 		Ok(())
@@ -497,3 +512,3 @@ impl NegotiationContext {
 	fn sent_tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput) -> Result<(), AbortReason> {
-		self.local_outputs.remove(&msg.serial_id);
+		self.outputs.remove(&msg.serial_id);
 		Ok(())
@@ -504,16 +519,4 @@ impl NegotiationContext {
 	) -> Result<(), AbortReason> {
-		let mut counterparty_weight_contributed: u64 = self
-			.remote_outputs
-			.values()
-			.map(|output| get_output_weight(&output.tx_out.script_pubkey))
-			.sum();
-		// We don't know the counterparty's witnesses ahead of time obviously, so we use the lower bounds
-		// specified in BOLT 3.
-		let mut total_inputs_weight: u64 = 0;
-		for InteractiveTxInput { prev_output, .. } in self.remote_inputs.values() {
-			total_inputs_weight =
-				total_inputs_weight.saturating_add(estimate_input_weight(prev_output));
-		}
-		counterparty_weight_contributed =
-			counterparty_weight_contributed.saturating_add(total_inputs_weight);
+		let counterparty_weight_contributed =
+			self.remote_inputs_weight().saturating_add(self.remote_outputs_weight());
 		let mut required_counterparty_contribution_fee =
@@ -539,13 +542,4 @@ impl NegotiationContext {
 		// - the peer's total input satoshis is less than their outputs
-		let remote_inputs_value = self
-			.remote_inputs
-			.values()
-			.fold(0u64, |acc, InteractiveTxInput { prev_output, .. }| {
-				acc.saturating_add(prev_output.value)
-			});
-		let remote_outputs_value = self
-			.remote_outputs
-			.values()
-			.fold(0u64, |acc, InteractiveTxOutput { tx_out, .. }| acc.saturating_add(tx_out.value));
-
+		let remote_inputs_value = self.remote_inputs_value();
+		let remote_outputs_value = self.remote_outputs_value();
 		// ...actually the counterparty might be splicing out, so that their balance also contributes
@@ -559,4 +553,4 @@ impl NegotiationContext {
 		// - there are more than 252 outputs
-		if self.total_input_count() > MAX_INPUTS_OUTPUTS_COUNT
-			|| self.total_output_count() > MAX_INPUTS_OUTPUTS_COUNT
+		if self.inputs.len() > MAX_INPUTS_OUTPUTS_COUNT
+			|| self.outputs.len() > MAX_INPUTS_OUTPUTS_COUNT
 		{
@@ -568,7 +562,17 @@ impl NegotiationContext {

+		let holder_is_initiator = self.holder_is_initiator;
+		let (remote_inputs, local_inputs): (Vec<_>, Vec<_>) =
+			self.inputs.into_values().partition(|InteractiveTxInput { serial_id, .. }| {
+				is_serial_id_valid_for_counterparty(holder_is_initiator, serial_id)
+			});
+		let (remote_outputs, local_outputs): (Vec<_>, Vec<_>) =
+			self.outputs.into_values().partition(|InteractiveTxOutput { serial_id, .. }| {
+				is_serial_id_valid_for_counterparty(holder_is_initiator, serial_id)
+			});
+
 		let constructed_tx = ConstructedTransaction::new(
-			self.local_inputs.into_values().collect(),
-			self.local_outputs.into_values().collect(),
-			self.remote_inputs.into_values().collect(),
-			self.remote_outputs.into_values().collect(),
+			local_inputs,
+			local_outputs,
+			remote_inputs,
+			remote_outputs,
 			self.tx_locktime,
@@ -788,7 +792,5 @@ impl StateMachine {
 			received_tx_add_output_count: 0,
-			local_inputs: new_hash_map(),
-			remote_inputs: new_hash_map(),
+			inputs: new_hash_map(),
 			prevtx_outpoints: new_hash_set(),
-			local_outputs: new_hash_map(),
-			remote_outputs: new_hash_map(),
+			outputs: new_hash_map(),
 			feerate_sat_per_kw,
@@ -1125,3 +1127,3 @@ mod tests {
 	struct TestSession {
-		description: String,
+		description: &'static str,
 		inputs_a: Vec<(TxIn, TransactionU16LenLimited)>,
@@ -1255,6 +1257,3 @@ mod tests {
 		assert!(message_send_b.is_none());
-		assert_eq!(
-			final_tx_a.unwrap().into_unsigned_tx().unwrap(),
-			final_tx_b.unwrap().into_unsigned_tx().unwrap()
-		);
+		assert_eq!(final_tx_a.unwrap().into_unsigned_tx(), final_tx_b.unwrap().into_unsigned_tx());
 		assert!(session.expect_error.is_none(), "Test: {}", session.description);
@@ -1396,5 +1395,4 @@ mod tests {
 	fn test_interactive_tx_constructor() {
-		// No contributions.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "No contributions".to_string(),
+			description: "No contributions",
 			inputs_a: vec![],
@@ -1405,5 +1403,4 @@ mod tests {
 		});
-		// Single contribution, no initiator inputs.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, no initiator inputs".to_string(),
+			description: "Single contribution, no initiator inputs",
 			inputs_a: vec![],
@@ -1414,5 +1411,4 @@ mod tests {
 		});
-		// Single contribution, no initiator outputs.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, no initiator outputs".to_string(),
+			description: "Single contribution, no initiator outputs",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]),
@@ -1423,5 +1419,4 @@ mod tests {
 		});
-		// Single contribution, no fees.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, no fees".to_string(),
+			description: "Single contribution, no fees",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]),
@@ -1432,3 +1427,2 @@ mod tests {
 		});
-		// Single contribution with P2WPKH input, insufficient fees.
 		let p2wpkh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WPKH_INPUT_WEIGHT_LOWER_BOUND);
@@ -1441,3 +1435,3 @@ mod tests {
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, with P2WPKH input, insufficient fees".to_string(),
+			description: "Single contribution, with P2WPKH input, insufficient fees",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]),
@@ -1450,5 +1444,4 @@ mod tests {
 		});
-		// Single contribution with P2WPKH, sufficient fees
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution with P2WPKH input, sufficient fees".to_string(),
+			description: "Single contribution with P2WPKH input, sufficient fees",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]),
@@ -1461,6 +1454,5 @@ mod tests {
 		});
-		// Single contribution with P2WSH input, insufficient fees.
 		let p2wsh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WSH_INPUT_WEIGHT_LOWER_BOUND);
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, with P2WSH input, insufficient fees".to_string(),
+			description: "Single contribution, with P2WSH input, insufficient fees",
 			inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]),
@@ -1473,5 +1465,4 @@ mod tests {
 		});
-		// Single contribution with P2WSH, sufficient fees
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution with P2WSH input, sufficient fees".to_string(),
+			description: "Single contribution with P2WSH input, sufficient fees",
 			inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]),
@@ -1484,7 +1475,6 @@ mod tests {
 		});
-		// Single contribution with P2TR input, insufficient fees.
 		let p2tr_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2TR_INPUT_WEIGHT_LOWER_BOUND);
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution, with P2TR input, insufficient fees".to_string(),
-			inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]),
+			description: "Single contribution, with P2TR input, insufficient fees",
+			inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]),
 			outputs_a: generate_outputs(&[TestOutput::P2WPKH(
@@ -1496,5 +1486,4 @@ mod tests {
 		});
-		// Single contribution with P2TR input, sufficient fees
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Single contribution with P2TR input, sufficient fees".to_string(),
+			description: "Single contribution with P2TR input, sufficient fees",
 			inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]),
@@ -1507,6 +1496,4 @@ mod tests {
 		});
-		// Initiator contributes sufficient fees, but non-initiator does not.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator contributes sufficient fees, but non-initiator does not"
-				.to_string(),
+			description: "Initiator contributes sufficient fees, but non-initiator does not",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]),
@@ -1517,5 +1504,4 @@ mod tests {
 		});
-		// Multi-input-output contributions from both sides.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Multi-input-output contributions from both sides".to_string(),
+			description: "Multi-input-output contributions from both sides",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000); 2]),
@@ -1536,5 +1522,4 @@ mod tests {

-		// Prevout from initiator is not a witness program
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Prevout from initiator is not a witness program".to_string(),
+			description: "Prevout from initiator is not a witness program",
 			inputs_a: generate_inputs(&[TestOutput::P2PKH(1_000_000)]),
@@ -1546,3 +1531,2 @@ mod tests {

-		// Invalid input sequence from initiator.
 		let tx =
@@ -1554,3 +1538,3 @@ mod tests {
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Invalid input sequence from initiator".to_string(),
+			description: "Invalid input sequence from initiator",
 			inputs_a: vec![(invalid_sequence_input, tx.clone())],
@@ -1561,3 +1545,2 @@ mod tests {
 		});
-		// Duplicate prevout from initiator.
 		let duplicate_input = TxIn {
@@ -1568,3 +1551,3 @@ mod tests {
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Duplicate prevout from initiator".to_string(),
+			description: "Duplicate prevout from initiator",
 			inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())],
@@ -1575,3 +1558,2 @@ mod tests {
 		});
-		// Non-initiator uses same prevout as initiator.
 		let duplicate_input = TxIn {
@@ -1582,3 +1564,3 @@ mod tests {
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Non-initiator uses same prevout as initiator".to_string(),
+			description: "Non-initiator uses same prevout as initiator",
 			inputs_a: vec![(duplicate_input.clone(), tx.clone())],
@@ -1589,5 +1571,4 @@ mod tests {
 		});
-		// Initiator sends too many TxAddInputs
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator sends too many TxAddInputs".to_string(),
+			description: "Initiator sends too many TxAddInputs",
 			inputs_a: generate_fixed_number_of_inputs(MAX_RECEIVED_TX_ADD_INPUT_COUNT + 1),
@@ -1598,7 +1579,6 @@ mod tests {
 		});
-		// Attempt to queue up two inputs with duplicate serial ids. We use a deliberately bad
-		// entropy source, `DuplicateEntropySource` to simulate this.
 		do_test_interactive_tx_constructor_with_entropy_source(
 			TestSession {
-				description: "Attempt to queue up two inputs with duplicate serial ids".to_string(),
+				// We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this.
+				description: "Attempt to queue up two inputs with duplicate serial ids",
 				inputs_a: generate_fixed_number_of_inputs(2),
@@ -1611,5 +1591,4 @@ mod tests {
 		);
-		// Initiator sends too many TxAddOutputs.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator sends too many TxAddOutputs".to_string(),
+			description: "Initiator sends too many TxAddOutputs",
 			inputs_a: vec![],
@@ -1620,5 +1599,4 @@ mod tests {
 		});
-		// Initiator sends an output below dust value.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator sends an output below dust value".to_string(),
+			description: "Initiator sends an output below dust value",
 			inputs_a: vec![],
@@ -1631,5 +1609,4 @@ mod tests {
 		});
-		// Initiator sends an output above maximum sats allowed.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator sends an output above maximum sats allowed".to_string(),
+			description: "Initiator sends an output above maximum sats allowed",
 			inputs_a: vec![],
@@ -1640,5 +1617,4 @@ mod tests {
 		});
-		// Initiator sends an output without a witness program.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Initiator sends an output without a witness program".to_string(),
+			description: "Initiator sends an output without a witness program",
 			inputs_a: vec![],
@@ -1649,8 +1625,6 @@ mod tests {
 		});
-		// Attempt to queue up two outputs with duplicate serial ids. We use a deliberately bad
-		// entropy source, `DuplicateEntropySource` to simulate this.
 		do_test_interactive_tx_constructor_with_entropy_source(
 			TestSession {
-				description: "Attempt to queue up two outputs with duplicate serial ids"
-					.to_string(),
+				// We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this.
+				description: "Attempt to queue up two outputs with duplicate serial ids",
 				inputs_a: vec![],
@@ -1664,5 +1638,4 @@ mod tests {

-		// Peer contributed more output value than inputs
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Peer contributed more output value than inputs".to_string(),
+			description: "Peer contributed more output value than inputs",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000)]),
@@ -1674,5 +1647,4 @@ mod tests {

-		// Peer contributed more than allowed number of inputs.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Peer contributed more than allowed number of inputs".to_string(),
+			description: "Peer contributed more than allowed number of inputs",
 			inputs_a: generate_fixed_number_of_inputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1),
@@ -1686,5 +1658,4 @@ mod tests {
 		});
-		// Peer contributed more than allowed number of outputs.
 		do_test_interactive_tx_constructor(TestSession {
-			description: "Peer contributed more than allowed number of outputs".to_string(),
+			description: "Peer contributed more than allowed number of outputs",
 			inputs_a: generate_inputs(&[TestOutput::P2WPKH(TOTAL_BITCOIN_SUPPLY_SATOSHIS)]),

[dunxen]

Changes: git diff-tree -U1 3ced4e329 570ebfa07

diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 9f8ca4d1..88f1aa19 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -114,7 +114,6 @@ pub(crate) struct InteractiveTxOutput {
 pub(crate) struct ConstructedTransaction {
-	local_inputs: Vec<InteractiveTxInput>,
-	local_outputs: Vec<InteractiveTxOutput>,
+	holder_is_initiator: bool,

-	remote_inputs: Vec<InteractiveTxInput>,
-	remote_outputs: Vec<InteractiveTxOutput>,
+	inputs: Vec<InteractiveTxInput>,
+	outputs: Vec<InteractiveTxOutput>,

@@ -131,30 +130,34 @@ impl ConstructedTransaction {
 	pub fn new(
-		local_inputs: Vec<InteractiveTxInput>, local_outputs: Vec<InteractiveTxOutput>,
-		remote_inputs: Vec<InteractiveTxInput>, remote_outputs: Vec<InteractiveTxOutput>,
-		lock_time: AbsoluteLockTime,
+		holder_is_initiator: bool, inputs: Vec<InteractiveTxInput>,
+		outputs: Vec<InteractiveTxOutput>, lock_time: AbsoluteLockTime,
 	) -> Self {
-		let local_inputs_value_satoshis =
-			local_inputs.iter().fold(0u64, |acc, InteractiveTxInput { ref prev_output, .. }| {
-				acc.saturating_add(prev_output.value)
-			});
-		let local_outputs_value_satoshis =
-			local_outputs.iter().fold(0u64, |acc, InteractiveTxOutput { ref tx_out, .. }| {
-				acc.saturating_add(tx_out.value)
-			});
+		let mut local_inputs_value_satoshis = 0u64;
+		let mut remote_inputs_value_satoshis = 0u64;
+		for input in &inputs {
+			if is_serial_id_valid_for_counterparty(holder_is_initiator, &input.serial_id) {
+				remote_inputs_value_satoshis =
+					remote_inputs_value_satoshis.saturating_add(input.prev_output.value);
+			} else {
+				local_inputs_value_satoshis =
+					local_inputs_value_satoshis.saturating_add(input.prev_output.value);
+			}
+		}

-		let remote_inputs_value_satoshis =
-			remote_inputs.iter().fold(0u64, |acc, InteractiveTxInput { ref prev_output, .. }| {
-				acc.saturating_add(prev_output.value)
-			});
-		let remote_outputs_value_satoshis =
-			remote_outputs.iter().fold(0u64, |acc, InteractiveTxOutput { ref tx_out, .. }| {
-				acc.saturating_add(tx_out.value)
-			});
+		let mut local_outputs_value_satoshis = 0u64;
+		let mut remote_outputs_value_satoshis = 0u64;
+		for output in &outputs {
+			if is_serial_id_valid_for_counterparty(holder_is_initiator, &output.serial_id) {
+				remote_outputs_value_satoshis =
+					remote_outputs_value_satoshis.saturating_add(output.tx_out.value);
+			} else {
+				local_outputs_value_satoshis =
+					local_outputs_value_satoshis.saturating_add(output.tx_out.value);
+			}
+		}

 		Self {
-			local_inputs,
-			local_outputs,
+			holder_is_initiator,

-			remote_inputs,
-			remote_outputs,
+			inputs,
+			outputs,

@@ -171,14 +174,10 @@ impl ConstructedTransaction {
 	pub fn weight(&self) -> u64 {
-		let inputs_weight = self.remote_inputs.iter().chain(self.local_inputs.iter()).fold(
-			0u64,
-			|weight, InteractiveTxInput { prev_output, .. }| {
+		let inputs_weight =
+			self.inputs.iter().fold(0u64, |weight, InteractiveTxInput { prev_output, .. }| {
 				weight.saturating_add(estimate_input_weight(prev_output))
-			},
-		);
-		let outputs_weight = self.remote_outputs.iter().chain(self.local_outputs.iter()).fold(
-			0u64,
-			|weight, InteractiveTxOutput { tx_out, .. }| {
+			});
+		let outputs_weight =
+			self.outputs.iter().fold(0u64, |weight, InteractiveTxOutput { tx_out, .. }| {
 				weight.saturating_add(get_output_weight(&tx_out.script_pubkey))
-			},
-		);
+			});

@@ -192,8 +191,4 @@ impl ConstructedTransaction {
 		// Inputs and outputs must be sorted by serial_id
-		let ConstructedTransaction { local_inputs: inputs, local_outputs: outputs, .. } = self;
+		let ConstructedTransaction { mut inputs, mut outputs, .. } = self;

-		let mut inputs: Vec<InteractiveTxInput> =
-			inputs.into_iter().chain(self.remote_inputs.into_iter()).collect();
-		let mut outputs: Vec<InteractiveTxOutput> =
-			outputs.into_iter().chain(self.remote_outputs.into_iter()).collect();
 		inputs.sort_unstable_by_key(|InteractiveTxInput { serial_id, .. }| *serial_id);
@@ -562,17 +557,6 @@ impl NegotiationContext {

-		let holder_is_initiator = self.holder_is_initiator;
-		let (remote_inputs, local_inputs): (Vec<_>, Vec<_>) =
-			self.inputs.into_values().partition(|InteractiveTxInput { serial_id, .. }| {
-				is_serial_id_valid_for_counterparty(holder_is_initiator, serial_id)
-			});
-		let (remote_outputs, local_outputs): (Vec<_>, Vec<_>) =
-			self.outputs.into_values().partition(|InteractiveTxOutput { serial_id, .. }| {
-				is_serial_id_valid_for_counterparty(holder_is_initiator, serial_id)
-			});
-
 		let constructed_tx = ConstructedTransaction::new(
-			local_inputs,
-			local_outputs,
-			remote_inputs,
-			remote_outputs,
+			self.holder_is_initiator,
+			self.inputs.into_values().collect(),
+			self.outputs.into_values().collect(),
 			self.tx_locktime,

[dunxen]

Update the last commit message as we no longer split inputs and outputs.

[dunxen]

Changes: git diff-tree -U1 570ebfa07 937f8bf39

diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 88f1aa19..a667df19 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -17,3 +17,4 @@ use bitcoin::policy::MAX_STANDARD_TX_WEIGHT;
 use bitcoin::{
-	absolute::LockTime as AbsoluteLockTime, OutPoint, ScriptBuf, Sequence, Transaction, TxIn, TxOut,
+	absolute::LockTime as AbsoluteLockTime, OutPoint, ScriptBuf, Sequence, Transaction, TxIn,
+	TxOut, Weight,
 };
@@ -129,35 +130,21 @@ pub(crate) struct ConstructedTransaction {
 impl ConstructedTransaction {
-	pub fn new(
-		holder_is_initiator: bool, inputs: Vec<InteractiveTxInput>,
-		outputs: Vec<InteractiveTxOutput>, lock_time: AbsoluteLockTime,
-	) -> Self {
-		let mut local_inputs_value_satoshis = 0u64;
-		let mut remote_inputs_value_satoshis = 0u64;
-		for input in &inputs {
-			if is_serial_id_valid_for_counterparty(holder_is_initiator, &input.serial_id) {
-				remote_inputs_value_satoshis =
-					remote_inputs_value_satoshis.saturating_add(input.prev_output.value);
-			} else {
-				local_inputs_value_satoshis =
-					local_inputs_value_satoshis.saturating_add(input.prev_output.value);
-			}
-		}
+	fn new(context: NegotiationContext) -> Self {
+		let local_inputs_value_satoshis = context
+			.inputs
+			.iter()
+			.filter(|(serial_id, _)| {
+				!is_serial_id_valid_for_counterparty(context.holder_is_initiator, serial_id)
+			})
+			.fold(0u64, |value, (_, input)| value.saturating_add(input.prev_output.value));

-		let mut local_outputs_value_satoshis = 0u64;
-		let mut remote_outputs_value_satoshis = 0u64;
-		for output in &outputs {
-			if is_serial_id_valid_for_counterparty(holder_is_initiator, &output.serial_id) {
-				remote_outputs_value_satoshis =
-					remote_outputs_value_satoshis.saturating_add(output.tx_out.value);
-			} else {
-				local_outputs_value_satoshis =
-					local_outputs_value_satoshis.saturating_add(output.tx_out.value);
-			}
-		}
+		let local_outputs_value_satoshis = context
+			.outputs
+			.iter()
+			.filter(|(serial_id, _)| {
+				!is_serial_id_valid_for_counterparty(context.holder_is_initiator, serial_id)
+			})
+			.fold(0u64, |value, (_, output)| value.saturating_add(output.tx_out.value));

 		Self {
-			holder_is_initiator,
-
-			inputs,
-			outputs,
+			holder_is_initiator: context.holder_is_initiator,

@@ -166,6 +153,9 @@ impl ConstructedTransaction {

-			remote_inputs_value_satoshis,
-			remote_outputs_value_satoshis,
+			remote_inputs_value_satoshis: context.remote_inputs_value(),
+			remote_outputs_value_satoshis: context.remote_outputs_value(),
+
+			inputs: context.inputs.into_values().collect(),
+			outputs: context.outputs.into_values().collect(),

-			lock_time,
+			lock_time: context.tx_locktime,
 		}
@@ -173,6 +163,6 @@ impl ConstructedTransaction {

-	pub fn weight(&self) -> u64 {
+	pub fn weight(&self) -> Weight {
 		let inputs_weight =
 			self.inputs.iter().fold(0u64, |weight, InteractiveTxInput { prev_output, .. }| {
-				weight.saturating_add(estimate_input_weight(prev_output))
+				weight.saturating_add(estimate_input_weight(prev_output).to_wu())
 			});
@@ -180,3 +170,3 @@ impl ConstructedTransaction {
 			self.outputs.iter().fold(0u64, |weight, InteractiveTxOutput { tx_out, .. }| {
-				weight.saturating_add(get_output_weight(&tx_out.script_pubkey))
+				weight.saturating_add(get_output_weight(&tx_out.script_pubkey).to_wu())
 			});
@@ -186,3 +176,5 @@ impl ConstructedTransaction {

-		tx.weight().to_wu().saturating_add(inputs_weight).saturating_add(outputs_weight)
+		Weight::from_wu(
+			tx.weight().to_wu().saturating_add(inputs_weight).saturating_add(outputs_weight),
+		)
 	}
@@ -218,4 +210,4 @@ struct NegotiationContext {

-pub(crate) fn estimate_input_weight(prev_output: &TxOut) -> u64 {
-	if prev_output.script_pubkey.is_v0_p2wpkh() {
+pub(crate) fn estimate_input_weight(prev_output: &TxOut) -> Weight {
+	Weight::from_wu(if prev_output.script_pubkey.is_v0_p2wpkh() {
 		P2WPKH_INPUT_WEIGHT_LOWER_BOUND
@@ -227,8 +219,10 @@ pub(crate) fn estimate_input_weight(prev_output: &TxOut) -> u64 {
 		UNKNOWN_SEGWIT_VERSION_INPUT_WEIGHT_LOWER_BOUND
-	}
+	})
 }

-pub(crate) fn get_output_weight(script_pubkey: &ScriptBuf) -> u64 {
-	(8 /* value */ + script_pubkey.consensus_encode(&mut sink()).unwrap() as u64)
-		* WITNESS_SCALE_FACTOR as u64
+pub(crate) fn get_output_weight(script_pubkey: &ScriptBuf) -> Weight {
+	Weight::from_wu(
+		(8 /* value */ + script_pubkey.consensus_encode(&mut sink()).unwrap() as u64)
+			* WITNESS_SCALE_FACTOR as u64,
+	)
 }
@@ -263,22 +257,22 @@ impl NegotiationContext {

-	fn remote_inputs_weight(&self) -> u64 {
-		self.inputs
-			.iter()
-			.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
-			.fold(0u64, |weight, (_, InteractiveTxInput { prev_output, .. })| {
-				weight.saturating_add(estimate_input_weight(prev_output))
-			})
+	fn remote_inputs_weight(&self) -> Weight {
+		Weight::from_wu(
+			self.inputs
+				.iter()
+				.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+				.fold(0u64, |weight, (_, InteractiveTxInput { prev_output, .. })| {
+					weight.saturating_add(estimate_input_weight(prev_output).to_wu())
+				}),
+		)
 	}

-	fn remote_outputs_weight(&self) -> u64 {
-		self.outputs
-			.iter()
-			.filter_map(|(serial_id, output)| {
-				if self.is_serial_id_valid_for_counterparty(serial_id) {
-					Some(get_output_weight(&output.tx_out.script_pubkey))
-				} else {
-					None
-				}
-			})
-			.sum()
+	fn remote_outputs_weight(&self) -> Weight {
+		Weight::from_wu(
+			self.outputs
+				.iter()
+				.filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+				.fold(0u64, |weight, (_, InteractiveTxOutput { tx_out, .. })| {
+					weight.saturating_add(get_output_weight(&tx_out.script_pubkey).to_wu())
+				}),
+		)
 	}
@@ -514,4 +508,6 @@ impl NegotiationContext {
 	) -> Result<(), AbortReason> {
-		let counterparty_weight_contributed =
-			self.remote_inputs_weight().saturating_add(self.remote_outputs_weight());
+		let counterparty_weight_contributed = self
+			.remote_inputs_weight()
+			.to_wu()
+			.saturating_add(self.remote_outputs_weight().to_wu());
 		let mut required_counterparty_contribution_fee =
@@ -557,10 +553,5 @@ impl NegotiationContext {

-		let constructed_tx = ConstructedTransaction::new(
-			self.holder_is_initiator,
-			self.inputs.into_values().collect(),
-			self.outputs.into_values().collect(),
-			self.tx_locktime,
-		);
+		let constructed_tx = ConstructedTransaction::new(self);

-		if constructed_tx.weight() > MAX_STANDARD_TX_WEIGHT as u64 {
+		if constructed_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 {
 			return Err(AbortReason::TransactionTooLarge);
@@ -1414,3 +1405,3 @@ mod tests {
 			TEST_FEERATE_SATS_PER_KW,
-			get_output_weight(&generate_p2wpkh_script_pubkey()),
+			get_output_weight(&generate_p2wpkh_script_pubkey()).to_wu(),
 		);

[TheBlueMatt]

Otherwise lgtm.

[TheBlueMatt]

Wont this always fail for a splice-out cause the param will be 0 (we ignore self.to_remote_value_satoshis) and check_counterparty_fees will always fail cause the required fee is non-0?

[dunxen]

We decided to fix the splicing-specific issues in #2989 which is WIP as we wanted to unblock dual-funding first.

I'm going to go ahead and remove any mention of to_remote_value_satoshis in this PR and have that all handled in #2989, though :)

[dunxen]

Changes: git diff-tree -U1 937f8bf39 59a8bd5d6

diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index a667df19..5f01bcd1 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -164,17 +164,18 @@ impl ConstructedTransaction {
 	pub fn weight(&self) -> Weight {
-		let inputs_weight =
-			self.inputs.iter().fold(0u64, |weight, InteractiveTxInput { prev_output, .. }| {
-				weight.saturating_add(estimate_input_weight(prev_output).to_wu())
-			});
-		let outputs_weight =
-			self.outputs.iter().fold(0u64, |weight, InteractiveTxOutput { tx_out, .. }| {
-				weight.saturating_add(get_output_weight(&tx_out.script_pubkey).to_wu())
-			});
-
-		let tx =
-			Transaction { version: 2, lock_time: self.lock_time, input: vec![], output: vec![] };
-
-		Weight::from_wu(
-			tx.weight().to_wu().saturating_add(inputs_weight).saturating_add(outputs_weight),
-		)
+		let inputs_weight = self.inputs.iter().fold(
+			Weight::from_wu(0),
+			|weight, InteractiveTxInput { prev_output, .. }| {
+				weight.checked_add(estimate_input_weight(prev_output)).unwrap_or(Weight::MAX)
+			},
+		);
+		let outputs_weight = self.outputs.iter().fold(
+			Weight::from_wu(0),
+			|weight, InteractiveTxOutput { tx_out, .. }| {
+				weight.checked_add(get_output_weight(&tx_out.script_pubkey)).unwrap_or(Weight::MAX)
+			},
+		);
+		Weight::from_wu(TX_COMMON_FIELDS_WEIGHT)
+			.checked_add(inputs_weight)
+			.and_then(|weight| weight.checked_add(outputs_weight))
+			.unwrap_or(Weight::MAX)
 	}
@@ -207,3 +208,2 @@ struct NegotiationContext {
 	feerate_sat_per_kw: u32,
-	to_remote_value_satoshis: u64,
 }
@@ -349,3 +349,3 @@ impl NegotiationContext {
 					input: TxIn {
-						previous_output: prev_outpoint.clone(),
+						previous_output: prev_outpoint,
 						sequence: Sequence(msg.sequence),
@@ -453,3 +453,3 @@ impl NegotiationContext {
 		}
-		if let Some(_) = self.outputs.remove(&msg.serial_id) {
+		if self.outputs.remove(&msg.serial_id).is_some() {
 			Ok(())
@@ -473,3 +473,3 @@ impl NegotiationContext {
 			tx.output.get(msg.prevtx_out as usize).ok_or(AbortReason::PrevTxOutInvalid)?.clone();
-		if !self.prevtx_outpoints.insert(input.previous_output.clone()) {
+		if !self.prevtx_outpoints.insert(input.previous_output) {
 			// We have added an input that already exists
@@ -535,6 +535,3 @@ impl NegotiationContext {
 		let remote_outputs_value = self.remote_outputs_value();
-		// ...actually the counterparty might be splicing out, so that their balance also contributes
-		// to the total input value.
-		if remote_inputs_value.saturating_add(self.to_remote_value_satoshis) < remote_outputs_value
-		{
+		if remote_inputs_value < remote_outputs_value {
 			return Err(AbortReason::OutputsValueExceedsInputsValue);
@@ -758,6 +755,3 @@ macro_rules! define_state_machine_transitions {
 impl StateMachine {
-	fn new(
-		feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime,
-		to_remote_value_satoshis: u64,
-	) -> Self {
+	fn new(feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime) -> Self {
 		let context = NegotiationContext {
@@ -771,3 +765,2 @@ impl StateMachine {
 			feerate_sat_per_kw,
-			to_remote_value_satoshis,
 		};
@@ -879,5 +872,2 @@ impl InteractiveTxConstructor {
 	///
-	/// If this is for a dual_funded channel then the `to_remote_value_satoshis` parameter should be set
-	/// to zero.
-	///
 	/// A tuple is returned containing the newly instantiate `InteractiveTxConstructor` and optionally
@@ -888,3 +878,3 @@ impl InteractiveTxConstructor {
 		inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>,
-		outputs_to_contribute: Vec<TxOut>, to_remote_value_satoshis: u64,
+		outputs_to_contribute: Vec<TxOut>,
 	) -> (Self, Option<InteractiveTxMessageSend>)
@@ -893,8 +883,4 @@ impl InteractiveTxConstructor {
 	{
-		let state_machine = StateMachine::new(
-			feerate_sat_per_kw,
-			is_initiator,
-			funding_tx_locktime,
-			to_remote_value_satoshis,
-		);
+		let state_machine =
+			StateMachine::new(feerate_sat_per_kw, is_initiator, funding_tx_locktime);
 		let mut inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)> =
@@ -1003,3 +989,3 @@ impl InteractiveTxConstructor {
 				let msg_send = self.maybe_send_message()?;
-				return match &self.state_machine {
+				match &self.state_machine {
 					StateMachine::NegotiationComplete(s) => {
@@ -1012,5 +998,5 @@ impl InteractiveTxConstructor {
 						debug_assert!(false, "We cannot transition to any other states after receiving `tx_complete` and responding");
-						return Err(AbortReason::InvalidStateTransition);
+						Err(AbortReason::InvalidStateTransition)
 					},
-				};
+				}
 			},
@@ -1139,3 +1125,2 @@ mod tests {
 			session.outputs_a,
-			0,
 		);
@@ -1149,3 +1134,2 @@ mod tests {
 			session.outputs_b,
-			0,
 		);

[TheBlueMatt]

Your turn @jkczyz

[TheBlueMatt]

Gonna go ahead and merge since this only really touches interactivetxs.rs which is unused in non-flags LDK.