From 15cbbc96e84a87a21377d3f0b0345b4ea58bd7c1 Mon Sep 17 00:00:00 2001
From: jwinterm <johnwmurphy@gmail.com>
Date: Thu, 4 Jun 2026 19:34:21 -0400
Subject: [PATCH] Contracts: validate net_change instead of unwrapping

contract `new` and the foreign `contract_new` API unwrapped the optional
net_change field, so a request omitting it panicked the wallet. Return an
error instead.

- new::compute and foreign::contract_new: net_change.ok_or_else(..).
- test: contract_new with no net change errors rather than panicking.

Addresses PR #754 review (new.rs, foreign.rs).
---
 controller/tests/contract_accounts.rs | 35 +++++++++++++++++++++++++++
 libwallet/src/api_impl/foreign.rs     |  4 ++-
 libwallet/src/contract/actions/new.rs |  4 ++-
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/controller/tests/contract_accounts.rs b/controller/tests/contract_accounts.rs
index 5d32654..c5ec317 100644
--- a/controller/tests/contract_accounts.rs
+++ b/controller/tests/contract_accounts.rs
@@ -295,3 +295,38 @@ fn wallet_contract_unknown_account() -> Result<(), libwallet::Error> {
 	clean_output_dir(test_dir);
 	Ok(())
 }
+
+/// A contract with no net change (neither --send nor --receive) must error
+/// rather than panic on an unwrap of the optional field.
+fn contract_missing_net_change_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
+	let (wallets, _chain, stopper, _bh) =
+		create_wallets(vec![vec![("default", 1)]], test_dir).unwrap();
+	let wallet1 = wallets[0].0.clone();
+	let mask1 = wallets[0].1.as_ref();
+
+	let res = wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| {
+		let args = &ContractNewArgsAPI {
+			setup_args: ContractSetupArgsAPI {
+				net_change: None,
+				..Default::default()
+			},
+			..Default::default()
+		};
+		api.contract_new(m, args)?;
+		Ok(())
+	});
+	assert!(res.is_err(), "expected an error when net_change is missing");
+
+	stopper.store(false, Ordering::Relaxed);
+	thread::sleep(Duration::from_millis(200));
+	Ok(())
+}
+
+#[test]
+fn wallet_contract_missing_net_change() -> Result<(), libwallet::Error> {
+	let test_dir = "test_output/contract_missing_net_change";
+	setup(test_dir);
+	contract_missing_net_change_impl(test_dir)?;
+	clean_output_dir(test_dir);
+	Ok(())
+}
diff --git a/libwallet/src/api_impl/foreign.rs b/libwallet/src/api_impl/foreign.rs
index 67079c0..bf9308c 100644
--- a/libwallet/src/api_impl/foreign.rs
+++ b/libwallet/src/api_impl/foreign.rs
@@ -251,7 +251,9 @@ where
 	C: NodeClient + 'a,
 	K: Keychain + 'a,
 {
-	let net_change = args.setup_args.net_change.unwrap();
+	let net_change = args.setup_args.net_change.ok_or_else(|| {
+		Error::GenericError("Contract requires a net change (--send or --receive)".to_string())
+	})?;
 	if net_change <= 0 {
 		return Err(Error::GenericError(
 			"Can't create a non-receiving contract from a foreign API.".to_string(),
diff --git a/libwallet/src/contract/actions/new.rs b/libwallet/src/contract/actions/new.rs
index ce1e5a5..ecf8c27 100644
--- a/libwallet/src/contract/actions/new.rs
+++ b/libwallet/src/contract/actions/new.rs
@@ -61,7 +61,9 @@ where
 	C: NodeClient + 'a,
 	K: Keychain + 'a,
 {
-	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())
+	})?;
 	debug!("contract::new => net_change passed: {}", net_change);
 
 	// Initialize a new contract (if net_change is positive, I'm the receiver meaning this is invoice flow)