Contracts: return errors for the remaining reachable panics

- Context::get_net_change unwrapped setup_args, which is None for the
  standard flows that share Context, so a non-contract context reaching
  a contract path terminated the wallet. It returns a Result now.
- A recomputed fee that no longer matches the stored one was an
  assert_eq, which aborts; report both values instead.
- Selection used abs(), which panics on i64::MIN, and unchecked addition
  for the amount equation.
- build_output_amount_list subtracted the custom output sum from the
  input sum as u64, before the checked conversion that was meant to
  guard it. A receiver without a payjoin contributes no inputs while
  --make-outputs is still allowed, so that underflows. The balance is
  evaluated in checked i64 now, with a test for the no-payjoin case.

libwallet/src/contract/actions/view.rs

@@ -72,7 +72,7 @@ where
 
 	// If we have a local context for this slate we've agreed on a net change; surface it.
 	let agreed_net_change = match w.get_private_context(keychain_mask, slate.id.as_bytes()) {
-		Ok(ctx) => Some(ctx.get_net_change()),
+		Ok(ctx) => Some(ctx.get_net_change()?),
 		Err(Error::NotFoundErr(_)) => None,
 		Err(e) => return Err(e),
 	};

libwallet/src/contract/context.rs

@@ -94,7 +94,9 @@ where
 	updater::refresh_outputs(w, keychain_mask, parent_key_id, false)?;
 
 	// Fee contribution estimation
-	let net_change = setup_args.net_change.unwrap();
+	let net_change = setup_args.net_change.ok_or_else(|| {
+		Error::GenericError("Contract requires a net change (--send or --receive)".to_string())
+	})?;
 	// select inputs to estimate fee cost
 	let (inputs, _, my_fee) =
 		prepare_outputs(w, &parent_key_id, current_height, &setup_args, None)?;
@@ -129,7 +131,7 @@ where
 	context.setup_args = Some(setup_args.clone());
 	debug!(
 		"Setting Context.net_change as: {}",
-		context.get_net_change()
+		context.get_net_change()?
 	);
 
 	Ok(context)
@@ -152,7 +154,10 @@ where
 		debug!("contract::utils::add_outputs => outputs have already been added, returning.");
 		return Ok(());
 	}
-	let setup_args = context.setup_args.as_ref().unwrap();
+	let setup_args = context
+		.setup_args
+		.as_ref()
+		.ok_or_else(|| Error::GenericError("Context carries no contract setup args".to_string()))?;
 	debug!("contract::utils::add_outputs => adding outputs");
 	let current_height = w.w2n_client().get_chain_tip()?.0;
 	let parent_key_id = &context.parent_key_id;
@@ -165,7 +170,19 @@ where
 		&setup_args,
 		context.fee,
 	)?;
-	assert_eq!(my_fee.fee(), context.fee.unwrap().fee(), "my_fee!=ctx.fee");
+	// An old or damaged context, or a future change to the fee schedule, can reach this
+	// with a stored fee that no longer matches what we just recomputed. Report both
+	// values instead of terminating the wallet.
+	let ctx_fee = context
+		.fee
+		.ok_or_else(|| Error::GenericError("Context carries no fee".to_string()))?;
+	if my_fee.fee() != ctx_fee.fee() {
+		return Err(Error::GenericError(format!(
+			"Recomputed fee {} does not match the fee stored in the context {}",
+			my_fee.fee(),
+			ctx_fee.fee()
+		)));
+	}
 	// Add selected/created inputs/outputs to the context
 	add_inputs_to_ctx(context, &inputs)?;
 	add_outputs_to_ctx(w, keychain_mask, context, my_output_amounts)?;

libwallet/src/contract/selection.rs

@@ -90,15 +90,25 @@ where
 	// For either party, the following MUST hold for the inputs the function returns:
 	//  Σmy_inputs >= Σmy_outputs + my_fee_cost
 	// Each party later balances the equation by adding an additional output (change output or receiver output)
-	let net_change = setup_args.net_change.unwrap();
+	let net_change = setup_args.net_change.ok_or_else(|| {
+		Error::GenericError("Contract requires a net change (--send or --receive)".to_string())
+	})?;
 	let custom_outputs_amount_sum = setup_args.selection_args.sum_output_amounts()?;
+	// unsigned_abs, as abs() panics on i64::MIN
 	let pay_amount = if net_change < 0 {
-		net_change.abs() as u64
+		net_change.unsigned_abs()
 	} else {
 		0
 	};
 	// Add the amount we pay and the custom outputs to rhs of the equation
-	let rhs = pay_amount + custom_outputs_amount_sum;
+	let rhs = pay_amount
+		.checked_add(custom_outputs_amount_sum)
+		.ok_or_else(|| {
+			Error::GenericError(format!(
+				"Amount overflow: pay amount {} plus custom outputs {}",
+				pay_amount, custom_outputs_amount_sum
+			))
+		})?;
 	let required_inputs = setup_args.selection_args.required_inputs();
 	let is_payjoin = setup_args.selection_args.is_payjoin();
 	let is_self_spend = setup_args.num_participants == 1;
@@ -258,26 +268,41 @@ fn build_output_amount_list(
 	my_fee_cost: u64,
 	setup_args: &ContractSetupArgsAPI,
 ) -> Result<Vec<u64>, Error> {
-	let expected_net_change = setup_args.net_change.unwrap();
+	let expected_net_change = setup_args.net_change.ok_or_else(|| {
+		Error::GenericError("Contract requires a net change (--send or --receive)".to_string())
+	})?;
 	let mut my_output_amounts = setup_args.selection_args.output_amounts()?;
-	let custom_outputs_sum = my_output_amounts.iter().sum::<u64>();
-	// We know that `Σmy_inputs >= Σmy_outputs + my_fee_cost` holds so we balance the equation by adding
-	// an additional output holding the missing amount (change output or receiver output)
-	// Checked conversions so a very large amount can't wrap into a wrong (possibly negative) change.
-	let input_minus_outputs = i64::try_from(my_input_sum - custom_outputs_sum).map_err(|_| {
-		Error::GenericError(format!(
-			"input sum {} exceeds i64",
-			my_input_sum - custom_outputs_sum
-		))
+	let custom_outputs_sum = my_output_amounts
+		.iter()
+		.try_fold(0u64, |acc, v| acc.checked_add(*v))
+		.ok_or_else(|| Error::GenericError("Custom output amounts overflow".to_string()))?;
+	// We balance `Σmy_inputs >= Σmy_outputs + my_fee_cost` by adding an output holding the
+	// missing amount (change output or receiver output). A receiver that is not doing a
+	// payjoin contributes no inputs while --make-outputs is still allowed, so my_input_sum
+	// can be below custom_outputs_sum: the whole equation is evaluated in checked i64 so
+	// that case is reported rather than wrapping.
+	let input_sum = i64::try_from(my_input_sum)
+		.map_err(|_| Error::GenericError(format!("input sum {} exceeds i64", my_input_sum)))?;
+	let outputs_sum = i64::try_from(custom_outputs_sum).map_err(|_| {
+		Error::GenericError(format!("output sum {} exceeds i64", custom_outputs_sum))
 	})?;
 	let fee_cost = i64::try_from(my_fee_cost)
 		.map_err(|_| Error::GenericError(format!("fee cost {} exceeds i64", my_fee_cost)))?;
-	let my_change_output_amount = input_minus_outputs + expected_net_change - fee_cost;
+	let my_change_output_amount = input_sum
+		.checked_sub(outputs_sum)
+		.and_then(|v| v.checked_add(expected_net_change))
+		.and_then(|v| v.checked_sub(fee_cost))
+		.ok_or_else(|| {
+			Error::GenericError(format!(
+				"contract::selection::build_output_amount_list => amount overflow. Values: my_input_sum: {}, custom_outputs_sum: {}, expected_net_change: {}, my_fee_cost: {}",
+				input_sum, outputs_sum, expected_net_change, fee_cost
+			))
+		})?;
 	// A negative change output would mean the selection math is off; reject it explicitly.
 	if my_change_output_amount < 0 {
 		return Err(Error::GenericError(format!(
-			"contract::selection::build_output_amount_list => negative change output. Values: my_input_sum: {}, expected_net_change: {}, my_fee_cost: {}",
-			my_input_sum as i64, expected_net_change, my_fee_cost as i64
+			"contract::selection::build_output_amount_list => negative change output. Values: my_input_sum: {}, custom_outputs_sum: {}, expected_net_change: {}, my_fee_cost: {}",
+			input_sum, outputs_sum, expected_net_change, fee_cost
 		)));
 	}
 	// Add our change/receiver output (which can be a zero-value output) to the list of outputs
@@ -338,6 +363,39 @@ mod tests {
 		rv
 	}
 
+	// A receiver that is not doing a payjoin contributes no inputs, while --make-outputs
+	// is still allowed, so the output balance is computed from a zero input sum against a
+	// non-zero custom output sum. That has to be reported, not wrapped around.
+	#[test]
+	fn receiver_no_payjoin_with_custom_outputs() {
+		let setup_args = ContractSetupArgsAPI {
+			net_change: Some(5_000_000_000),
+			selection_args: OutputSelectionArgs {
+				// no use_inputs, so is_payjoin() is false and no inputs are selected
+				make_outputs: Some(vec![1_000_000_000]),
+				..Default::default()
+			},
+			..Default::default()
+		};
+		// Receiving more than the custom outputs and the fee leaves a valid balance
+		let my_fee = my_fee_contribution(0, 2, 1, 2).unwrap();
+		let amounts = build_output_amount_list(0, my_fee.fee(), &setup_args).unwrap();
+		assert_eq!(amounts.len(), 2);
+		assert_eq!(amounts[0], 1_000_000_000);
+		assert_eq!(amounts[1], 5_000_000_000 - 1_000_000_000 - my_fee.fee());
+
+		// Custom outputs beyond what we receive are rejected rather than underflowing
+		let setup_args = ContractSetupArgsAPI {
+			net_change: Some(1_000_000_000),
+			selection_args: OutputSelectionArgs {
+				make_outputs: Some(vec![5_000_000_000]),
+				..Default::default()
+			},
+			..Default::default()
+		};
+		assert!(build_output_amount_list(0, my_fee.fee(), &setup_args).is_err());
+	}
+
 	#[test]
 	fn sender_no_inputs() {
 		// net_change=-1, no inputs, no fee committed => NotEnoughFunds

libwallet/src/contract/utils.rs

@@ -150,18 +150,19 @@ where
 			debug!("contract::sign => context found");
 			// We have a context so we must have agreed on a certain net_change value in Context.net_change.
 			// If we have both Context.net_change and setup_args.net_change, then they must be equal.
+			let ctx_net_change = ctx.get_net_change()?;
 			match expected_net_change {
 				Some(args_net_change) => {
-					if ctx.get_net_change() != args_net_change {
+					if ctx_net_change != args_net_change {
 						return Err(Error::GenericError(format!(
 							"Expected net change mismatch! Context.net_change: {}, setup_args.net_change: {}",
-							ctx.get_net_change(), args_net_change
+							ctx_net_change, args_net_change
 						)));
 					}
 				}
 				None => (),
 			}
-			expected_net_change = Some(ctx.get_net_change());
+			expected_net_change = Some(ctx_net_change);
 		}
 		Err(Error::NotFoundErr(_)) => {
 			debug!("contract::utils::get_net_change => context not found")
@@ -212,7 +213,7 @@ where
 	let mut tx_log_entry = {
 		if !context.log_id.is_some() {
 			// We create a new entry with log_id=0 and but replace it with the real id before committing
-			create_tx_log_entry(slate, context.get_net_change(), parent_key_id.clone(), 0)?
+			create_tx_log_entry(slate, context.get_net_change()?, parent_key_id.clone(), 0)?
 		} else {
 			w.get_tx_log_entry(parent_key_id.clone(), context.log_id.unwrap())?
 				.unwrap()

libwallet/src/types.rs

@@ -455,9 +455,16 @@ impl Context {
 }
 
 impl Context {
-	/// Returns net_change for the contract
-	pub fn get_net_change(&self) -> i64 {
-		self.setup_args.as_ref().unwrap().net_change.unwrap()
+	/// Returns net_change for the contract. Context is shared with the standard
+	/// transaction flows, where setup_args is None, so a context that does not belong to
+	/// a contract is reported rather than unwrapped.
+	pub fn get_net_change(&self) -> Result<i64, Error> {
+		self.setup_args
+			.as_ref()
+			.and_then(|args| args.net_change)
+			.ok_or_else(|| {
+				Error::GenericError("Context carries no contract net change".to_string())
+			})
 	}
 
 	/// Tracks an output contributing to my excess value (if it needs to