From 2e3b04498c1d32d27eafecd3c5fd2a1ea1fcf516 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Wed, 3 Jun 2026 09:33:11 -0400 Subject: [PATCH] De-panic and validate contract CLI input handling Bad input to the contract CLI could abort the process: - a non-numeric --make-outputs value reached output_amounts(), which unwrap()s the float parse; - `contract new` with neither --send nor --receive hit a panic in get_net_change; - `contract sign` panicked ("No address to encrypt for") when given no --encrypt-for and an incoming slate with no embedded sender. Validate --make-outputs and require exactly one of --send/--receive at parse time (ParseError, before reaching the panicking paths), and return a typed error instead of panicking when there is no address to encrypt for. Verified at runtime: each now yields a clean "Invalid Arguments" error. --- controller/src/command.rs | 6 ++++-- src/cmd/wallet_args.rs | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/controller/src/command.rs b/controller/src/command.rs index 1c5b026..31e4c4e 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -1887,8 +1887,10 @@ where if args.counterparty_addr.is_some() { args.counterparty_addr.unwrap() } else { - if !slatepack.sender.is_some() { - panic!("No address to encrypt for. Contracts only support encrypted slates right now."); + if slatepack.sender.is_none() { + return Err(grin_wallet_libwallet::Error::GenericError( + "No address to encrypt for: pass --encrypt-for. Contracts only support encrypted slates.".into(), + )); } String::try_from(&slatepack.sender.unwrap())? }; diff --git a/src/cmd/wallet_args.rs b/src/cmd/wallet_args.rs index 60c96a8..0d6afd4 100644 --- a/src/cmd/wallet_args.rs +++ b/src/cmd/wallet_args.rs @@ -1000,6 +1000,11 @@ pub fn parse_contract_new_args( "You can only specify receive or send, not both.", ))); }; + if receive.is_none() && send.is_none() { + return Err(ParseError::ArgumentError(String::from( + "You must specify either --receive or --send.", + ))); + }; // TODO: verify this is correct e.g. which values are passed here by default etc. let src_acct_name = Some(String::from(account)); let add_outputs = args.is_present("add-outputs"); @@ -1024,7 +1029,14 @@ pub fn parse_contract_new_args( } }; let make_outputs = match args.value_of("make-outputs") { - Some(v) => Some(String::from(v)), + Some(v) => { + // Validate each comma-separated amount here so a bad value returns a + // ParseError instead of panicking later in output_amounts(). + for amt in v.split(',') { + parse_grin_amount(amt, "make-outputs")?; + } + Some(String::from(v)) + } None => None, }; @@ -1096,7 +1108,14 @@ pub fn parse_contract_setup_args( } }; let make_outputs = match args.value_of("make-outputs") { - Some(v) => Some(String::from(v)), + Some(v) => { + // Validate each comma-separated amount here so a bad value returns a + // ParseError instead of panicking later in output_amounts(). + for amt in v.split(',') { + parse_grin_amount(amt, "make-outputs")?; + } + Some(String::from(v)) + } None => None, }; // TODO: should we catch if the person calls "--receive=5" when it should be "--send=5"?