Use Amount in calculate_change_output_value

Author: jkczyz

lightning/src/ln/channel.rs

@@ -6703,12 +6703,12 @@ impl FundingNegotiationContext {
 					},
 				}
 			};
-			let mut change_output =
-				TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script };
+			let mut change_output = TxOut { value: change_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(self.funding_feerate_sat_per_1000_weight, change_output_weight);
-			let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee);
+			let change_value_decreased_with_fee =
+				change_value.to_sat().saturating_sub(change_output_fee);
 			// 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);

lightning/src/ln/interactivetxs.rs

@@ -2337,22 +2337,23 @@ impl InteractiveTxConstructor {
 pub(super) fn calculate_change_output_value(
 	context: &FundingNegotiationContext, is_splice: bool, shared_output_funding_script: &ScriptBuf,
 	change_output_dust_limit: u64,
-) -> Result<Option<u64>, AbortReason> {
+) -> Result<Option<Amount>, AbortReason> {
 	assert!(context.our_funding_contribution > SignedAmount::ZERO);
-	let our_funding_contribution_satoshis = context.our_funding_contribution.to_sat() as u64;
+	let our_funding_contribution = context.our_funding_contribution.to_unsigned().unwrap();
 
-	let mut total_input_satoshis = 0u64;
+	let mut total_input_value = Amount::ZERO;
 	let mut our_funding_inputs_weight = 0u64;
 	for FundingTxInput { utxo, .. } in context.our_funding_inputs.iter() {
-		total_input_satoshis = total_input_satoshis.saturating_add(utxo.output.value.to_sat());
+		total_input_value = total_input_value.checked_add(utxo.output.value).unwrap_or(Amount::MAX);
 
 		let weight = BASE_INPUT_WEIGHT + utxo.satisfaction_weight;
 		our_funding_inputs_weight = our_funding_inputs_weight.saturating_add(weight);
 	}
 
 	let funding_outputs = &context.our_funding_outputs;
-	let total_output_satoshis =
-		funding_outputs.iter().fold(0u64, |total, out| total.saturating_add(out.value.to_sat()));
+	let total_output_value = funding_outputs
+		.iter()
+		.fold(Amount::ZERO, |total, out| total.checked_add(out.value).unwrap_or(Amount::MAX));
 	let our_funding_outputs_weight = funding_outputs.iter().fold(0u64, |weight, out| {
 		weight.saturating_add(get_output_weight(&out.script_pubkey).to_wu())
 	});
@@ -2376,15 +2377,22 @@ pub(super) fn calculate_change_output_value(
 		}
 	}
 
-	let fees_sats = fee_for_weight(context.funding_feerate_sat_per_1000_weight, weight);
-	let net_total_less_fees =
-		total_input_satoshis.saturating_sub(total_output_satoshis).saturating_sub(fees_sats);
-	if net_total_less_fees < our_funding_contribution_satoshis {
+	let contributed_fees =
+		Amount::from_sat(fee_for_weight(context.funding_feerate_sat_per_1000_weight, weight));
+	let net_total_less_fees = total_input_value
+		.checked_sub(total_output_value)
+		.unwrap_or(Amount::ZERO)
+		.checked_sub(contributed_fees)
+		.unwrap_or(Amount::ZERO);
+	if net_total_less_fees < our_funding_contribution {
 		// Not enough to cover contribution plus fees
 		return Err(AbortReason::InsufficientFees);
 	}
-	let remaining_value = net_total_less_fees.saturating_sub(our_funding_contribution_satoshis);
-	if remaining_value < change_output_dust_limit {
+
+	let remaining_value = net_total_less_fees
+		.checked_sub(our_funding_contribution)
+		.expect("remaining_value should not be negative");
+	if remaining_value.to_sat() < change_output_dust_limit {
 		// Enough to cover contribution plus fees, but leftover is below dust limit; no change
 		Ok(None)
 	} else {
@@ -3440,14 +3448,14 @@ mod tests {
 			total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
 		assert_eq!(
 			calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
-			Ok(Some((gross_change - fees - common_fees).to_sat())),
+			Ok(Some(gross_change - fees - common_fees)),
 		);
 
 		// There is leftover for change, without common fees
 		let context = FundingNegotiationContext { is_initiator: false, ..context };
 		assert_eq!(
 			calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
-			Ok(Some((gross_change - fees).to_sat())),
+			Ok(Some(gross_change - fees)),
 		);
 
 		// Insufficient inputs, no leftover
@@ -3482,7 +3490,7 @@ mod tests {
 			total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
 		assert_eq!(
 			calculate_change_output_value(&context, false, &ScriptBuf::new(), 100),
-			Ok(Some((gross_change - fees).to_sat())),
+			Ok(Some(gross_change - fees)),
 		);
 
 		// Larger fee, smaller change
@@ -3496,7 +3504,7 @@ mod tests {
 			total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
 		assert_eq!(
 			calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
-			Ok(Some((gross_change - fees * 3 - common_fees * 3).to_sat())),
+			Ok(Some(gross_change - fees * 3 - common_fees * 3)),
 		);
 	}