From a9d82a5039346fb7dcab588412379d1d46ecf5bf Mon Sep 17 00:00:00 2001 From: wiesche Date: Tue, 1 Sep 2026 15:37:11 +0200 Subject: [PATCH] fix contract slatepack cli output --- config/src/config.rs | 2 +- controller/src/command.rs | 112 ++++++++++++++--------- libwallet/src/contract/actions/README.md | 1 - src/bin/grin-wallet.rs | 10 +- src/cmd/wallet.rs | 26 ++++-- src/cmd/wallet_args.rs | 12 +++ 6 files changed, 107 insertions(+), 56 deletions(-) diff --git a/config/src/config.rs b/config/src/config.rs index c32a5bb..e4f3778 100644 --- a/config/src/config.rs +++ b/config/src/config.rs @@ -336,7 +336,7 @@ pub fn initial_setup_wallet( Some(p) => p, None => get_wallet_path(chain_type, create_path)?, }; - println!("Wallet path: {}", wallet_path.display()); + eprintln!("Wallet path: {}", wallet_path.display()); // Get path to the node directory, let node_path = get_node_path(Some(wallet_path.clone()), chain_type)?; diff --git a/controller/src/command.rs b/controller/src/command.rs index c3c0d13..b1f2032 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -649,24 +649,38 @@ impl SlatepackOut { serde_json::to_string_pretty(&self).unwrap() } - pub fn print(&self, as_json: bool) -> () { - if !self.is_finalized { - if as_json { - println!("{}", self.as_json()); - } else { - println!("{}", self); - } + fn render(&self, as_json: bool) -> String { + if as_json { + self.as_json() } else { - println!("Transaction was broadcasted."); // TODO: as_json makes no sense here, fix later. + self.to_string() } } + + pub fn print(&self, as_json: bool) { + println!("{}", self.render(as_json)); + } +} + +fn slatepack_recipient(dest: Option<&str>) -> Result, Error> { + dest.map(SlatepackAddress::try_from) + .transpose() + .map_err(Error::from) +} + +fn print_contract_status(message: &str, as_json: bool) { + if as_json { + eprintln!("{}", message); + } else { + println!("{}", message); + } } pub fn print_slatepack( api: &mut Owner, keychain_mask: Option<&SecretKey>, slate: &Slate, - counterparty_addr: &str, + counterparty_addr: Option, out_file: Option, as_json: bool, ) -> Result<(), libwallet::Error> @@ -678,7 +692,7 @@ where // For now, we don't compact slates with sl.compact(). We first make them work without compaction. // Writing the file, serializing and encrypting can all fail for ordinary reasons, so // report them through the normal CLI error path rather than unwrapping. - let slate_out = prepare_slatepack(api, keychain_mask, &slate, &counterparty_addr, out_file)?; + let slate_out = prepare_slatepack(api, keychain_mask, &slate, counterparty_addr, out_file)?; slate_out.print(as_json); Ok(()) } @@ -687,7 +701,7 @@ pub fn prepare_slatepack( owner_api: &mut Owner, keychain_mask: Option<&SecretKey>, slate: &Slate, - dest: &str, + dest: Option, out_file_override: Option, ) -> Result where @@ -699,17 +713,12 @@ where // Output the slatepack file to stdout and to a file let mut message = String::from(""); - let mut address = None; let mut tld = String::from(""); let wallet_inst = owner_api.wallet_inst.clone(); let config_path = owner_api.config_path(); controller::owner_single_use(wallet_inst, keychain_mask, config_path, |api, m| { - address = match SlatepackAddress::try_from(dest) { - Ok(a) => Some(a), - Err(_) => None, - }; // encrypt for recipient by default - let recipients = match address.clone() { + let recipients = match dest.clone() { Some(a) => vec![a], None => vec![], }; @@ -739,7 +748,7 @@ where let is_finalized = can_finalize(slate); let slate_out = SlatepackOut { - is_encrypted: address.is_some(), + is_encrypted: dest.is_some(), is_finalized: is_finalized, out_file: out_file_name, message: message, @@ -1749,6 +1758,7 @@ where K: keychain::Keychain + 'static, { let contract_new_args = args.to_api_args()?; + let recipient = slatepack_recipient(args.counterparty_addr.as_deref())?; let wallet_inst = owner_api.wallet_inst.clone(); let config_path = owner_api.config_path(); controller::owner_single_use(wallet_inst, keychain_mask, config_path, |api, m| { @@ -1758,7 +1768,7 @@ where api, keychain_mask, &slate, - args.counterparty_addr.as_deref().unwrap_or(""), + recipient.clone(), args.outfile, args.as_json, )?; @@ -1846,11 +1856,12 @@ where { // Args for signing are just setup args let contract_sign_args = args.to_api_args()?; + let recipient = slatepack_recipient(args.counterparty_addr.as_deref())?; let wallet_inst = owner_api.wallet_inst.clone(); let config_path = owner_api.config_path(); controller::owner_single_use(wallet_inst, keychain_mask, config_path, |api, m| { // Read the slatepack from stdin - println!("Paste slatepack:"); + print_contract_status("Paste slatepack:", args.as_json); let mut slatepack_msg = String::new(); io::stdin().read_line(&mut slatepack_msg).map_err(|e| { libwallet::Error::GenericError(format!("Failed to read from stdin: {}", e)) @@ -1866,15 +1877,8 @@ where )?; // Encrypt the reply for --encrypt-for if given, else for the incoming slatepack's - // sender. If neither is known the incoming slate was plaintext, so reply plaintext - // (empty dest -> unencrypted slatepack). - let counterparty_addr = if let Some(addr) = args.counterparty_addr { - addr - } else if let Some(sender) = slatepack.sender { - String::try_from(&sender)? - } else { - String::new() - }; + // sender. If neither is known the incoming slate was plaintext, so reply plaintext. + let recipient = recipient.clone().or(slatepack.sender); let mut slate = owner::slate_from_slatepack_message( api.wallet_inst.clone(), keychain_mask, @@ -1884,20 +1888,19 @@ where slate = api.contract_sign(m, &slate, &contract_sign_args)?; - print_slatepack( - api, - keychain_mask, - &slate, - &counterparty_addr, - args.outfile, - args.as_json, - )?; + let slate_out = prepare_slatepack(api, keychain_mask, &slate, recipient, args.outfile)?; - if broadcast_tx { - let is_finalized = can_finalize(&slate); - if is_finalized { - api.post_tx(keychain_mask, &slate, true)?; + if broadcast_tx && slate_out.is_finalized { + if let Err(e) = api.post_tx(keychain_mask, &slate, true) { + slate_out.print(args.as_json); + return Err(e); } + if args.as_json { + slate_out.print(true); + } + print_contract_status("Transaction was broadcasted.", args.as_json); + } else { + slate_out.print(args.as_json); } Ok(()) @@ -1970,7 +1973,7 @@ where )?; if let Some(slate) = slate_opt { // A revoke has no counterparty, so write the replacement as plaintext. - let slate_out = prepare_slatepack(api, keychain_mask, &slate, "", None)?; + let slate_out = prepare_slatepack(api, keychain_mask, &slate, None, None)?; println!("{}", slate_out); } else { println!("Contract revoked. No replacement transaction was created."); @@ -2007,3 +2010,28 @@ mod send_tests { assert_eq!(estimate_strategies(false), &["smallest", "all"]); } } + +#[cfg(test)] +mod contract_tests { + use super::*; + + #[test] + fn invalid_recipient() { + assert!(slatepack_recipient(Some("invalid")).is_err()); + assert!(slatepack_recipient(None).unwrap().is_none()); + } + + #[test] + fn finalized_json() { + let output = SlatepackOut { + is_encrypted: false, + is_finalized: true, + out_file: "slatepack/test.S3.slatepack".to_string(), + message: "BEGINSLATEPACK. test. ENDSLATEPACK.".to_string(), + }; + + let value: serde_json::Value = serde_json::from_str(&output.render(true)).unwrap(); + assert_eq!(value["is_finalized"], true); + assert_eq!(value["message"], output.message); + } +} diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index f398984..5c29dd2 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -65,7 +65,6 @@ Separating side effects until the 'save_step' part would make these functions mu - Preserve kernel features and their arguments across V4 and V5; see #793. - Preserve missing proof sender addresses and timestamps instead of replacing them with zero values. - - Fix Slatepack output for invalid recipients, failed broadcasts and JSON output. - Choose the sender derivation path explicitly and keep encrypted outgoing files identifiable locally. - Decode the sender and slate from an incoming Slatepack in one pass. diff --git a/src/bin/grin-wallet.rs b/src/bin/grin-wallet.rs index 6e90995..709e704 100644 --- a/src/bin/grin-wallet.rs +++ b/src/bin/grin-wallet.rs @@ -135,11 +135,11 @@ fn real_main() -> i32 { // Load logging config let mut l = config.members.logging.clone().unwrap(); - // no logging to stdout if we're running cli - match args.subcommand() { - ("cli", _) => l.log_to_stdout = true, - _ => {} - }; + if cmd::wallet_args::contract_json_output(&args) { + l.log_to_stdout = false; + } else if let ("cli", _) = args.subcommand() { + l.log_to_stdout = true; + } init_logger(Some(l), None); info!( "Using wallet configuration file at {}", diff --git a/src/cmd/wallet.rs b/src/cmd/wallet.rs index e538ef6..ac801ec 100644 --- a/src/cmd/wallet.rs +++ b/src/cmd/wallet.rs @@ -22,6 +22,14 @@ use std::time::Duration; const MIN_COMPAT_NODE_VERSION: &str = "4.0.0-alpha.1"; +fn print_status(message: &str, stderr: bool) { + if stderr { + eprintln!("{}", message); + } else { + println!("{}", message); + } +} + pub fn wallet_command( wallet_args: &ArgMatches<'_>, config: GlobalWalletConfig, @@ -30,6 +38,7 @@ pub fn wallet_command( where C: NodeClient + 'static, { + let json_output = wallet_args::contract_json_output(wallet_args); // Check the node version info, and exit with report if we're not compatible let global_wallet_args = wallet_args::parse_global_args(&config.members.wallet, &wallet_args) .expect("Can't read configuration file"); @@ -41,15 +50,15 @@ where let min_version = Version::parse(MIN_COMPAT_NODE_VERSION).ok(); if let Ok(v) = node_version { if Some(v.clone()) < min_version { - println!("The Grin Node in use (version {}) is outdated and incompatible with this wallet version.", v); - println!( + eprintln!("The Grin Node in use (version {}) is outdated and incompatible with this wallet version.", v); + eprintln!( "Please update the node to version {} or later and try again.", MIN_COMPAT_NODE_VERSION ); return 1; } } else { - println!( + eprintln!( "Can not parse node version {}. Minimal compatible node version: {}", v.node_version, MIN_COMPAT_NODE_VERSION ); @@ -64,12 +73,15 @@ where thread::sleep(Duration::from_millis(100)); if let Err(e) = res { - println!("Wallet command failed: {}", e); + print_status(&format!("Wallet command failed: {}", e), json_output); 1 } else { - println!( - "Command '{}' completed successfully", - wallet_args.subcommand().0 + print_status( + &format!( + "Command '{}' completed successfully", + wallet_args.subcommand().0 + ), + json_output, ); 0 } diff --git a/src/cmd/wallet_args.rs b/src/cmd/wallet_args.rs index 1a31532..d8fd001 100644 --- a/src/cmd/wallet_args.rs +++ b/src/cmd/wallet_args.rs @@ -1194,6 +1194,18 @@ pub fn parse_contract_revoke_args( Ok(command::ContractRevokeArgs { tx_id: tx_id }) } +pub fn contract_json_output(args: &ArgMatches<'_>) -> bool { + match args.subcommand() { + ("contract", Some(contract_args)) => match contract_args.subcommand() { + ("new", Some(command_args)) | ("sign", Some(command_args)) => { + command_args.is_present("as-json") + } + _ => false, + }, + _ => false, + } +} + pub fn wallet_command( wallet_args: &ArgMatches, config: GlobalWalletConfig,