From 1406d616ba5161e6cf4974117f3fbaf2410175cc Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 1 Aug 2024 16:09:49 -0400 Subject: [PATCH 1/6] feat: add `--send` to invoke and default to send if there are writes --- FULL_HELP_DOCS.md | 14 ++++- .../src/commands/contract/invoke.rs | 54 +++++++++++++------ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 6c694bce8..214b9488b 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -480,7 +480,7 @@ stellar contract invoke ... -- --help ###### **Options:** * `--id ` — Contract ID to invoke -* `--is-view` — View the result simulating and do not sign and submit transaction +* `--is-view` — View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` * `--rpc-url ` — RPC server endpoint * `--network-passphrase ` — Network passphrase to sign the transaction sent to the rpc server * `--network ` — Name of network to use from config @@ -495,6 +495,18 @@ stellar contract invoke ... -- --help * `--instructions ` — Number of instructions to simulate * `--build-only` — Build the transaction and only write the base64 xdr to stdout * `--sim-only` — Simulate the transaction and only write the base64 xdr to stdout +* `--send ` — Whether or not to send a transaction + + Default value: `if-write` + + Possible values: + - `if-write`: + Only send transaction if there are ledger writes, otherwise return simulation result + - `no`: + Do not send transaction, return simulation result + - `yes`: + Always send transaction + diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 041613ba2..948b3154e 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -6,7 +6,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::{fmt::Debug, fs, io}; -use clap::{arg, command, value_parser, Parser}; +use clap::{arg, command, value_parser, Parser, ValueEnum}; use ed25519_dalek::SigningKey; use heck::ToKebabCase; @@ -21,6 +21,7 @@ use soroban_env_host::{ HostError, }; +use soroban_rpc::SimulateTransactionResponse; use soroban_spec::read::FromWasmError; use super::super::events; @@ -44,7 +45,7 @@ pub struct Cmd { // For testing only #[arg(skip)] pub wasm: Option, - /// View the result simulating and do not sign and submit transaction + /// View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` #[arg(long, env = "STELLAR_INVOKE_VIEW")] pub is_view: bool, /// Function name as subcommand, then arguments for that function as `--arg-name value` @@ -54,6 +55,9 @@ pub struct Cmd { pub config: config::Args, #[command(flatten)] pub fee: crate::fee::Args, + /// Whether or not to send a transaction + #[arg(long, value_enum, default_value("if-write"))] + pub send: Send, } impl FromStr for Cmd { @@ -153,14 +157,9 @@ impl From for Error { } impl Cmd { - fn is_view(&self) -> bool { - self.is_view || - // TODO: Remove at next major release. Was added to retain backwards - // compatibility when this env var used to be used for the --is-view - // option. - std::env::var("SYSTEM_TEST_VERBOSE_OUTPUT").as_deref() == Ok("true") + fn send(&self, sim_res: &SimulateTransactionResponse) -> Result { + Ok(self.send.should_send(sim_res)? && !self.is_view) } - fn build_host_function_parameters( &self, contract_id: [u8; 32], @@ -366,20 +365,13 @@ impl NetworkRunnable for Cmd { if global_args.map_or(true, |a| !a.no_cache) { data::write(sim_res.clone().into(), &network.rpc_uri()?)?; } - let (return_value, events) = if self.is_view() { - // log_auth_cost_and_footprint(Some(&sim_res.transaction_data()?.resources)); - (sim_res.results()?[0].xdr.clone(), sim_res.events()?) - } else { + let (return_value, events) = if self.send(sim_res)? { let global::Args { no_cache, .. } = global_args.cloned().unwrap_or_default(); // Need to sign all auth entries let mut txn = txn.transaction().clone(); - // let auth = auth_entries(&txn); - // crate::log::auth(&[auth]); - if let Some(tx) = config.sign_soroban_authorizations(&txn, &signers).await? { txn = tx; } - // log_auth_cost_and_footprint(resources(&txn)); let res = client .send_transaction_polling(&config.sign_with_local_key(txn).await?) .await?; @@ -387,6 +379,8 @@ impl NetworkRunnable for Cmd { data::write(res.clone().try_into()?, &network.rpc_uri()?)?; } (res.return_value()?, res.contract_events()?) + } else { + (sim_res.results()?[0].xdr.clone(), sim_res.events()?) }; crate::log::diagnostic_events(&events, tracing::Level::INFO); @@ -561,3 +555,29 @@ Each arg has a corresponding ---file-path which is a path to a file co Note: The only types which aren't JSON are Bytes and BytesN, which are raw bytes"# ) } + +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, ValueEnum, Default)] +pub enum Send { + /// Only send transaction if there are ledger writes, otherwise return simulation result + #[default] + IfWrite, + /// Do not send transaction, return simulation result + No, + /// Always send transaction + Yes, +} + +impl Send { + pub fn should_send(self, sim_res: &SimulateTransactionResponse) -> Result { + Ok(match self { + Send::IfWrite => !sim_res + .transaction_data()? + .resources + .footprint + .read_write + .is_empty(), + Send::No => false, + Send::Yes => true, + }) + } +} From f9e7e376cd6596ae193c7118c48af0dbb5b7e3b5 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Thu, 1 Aug 2024 19:40:10 -0400 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> --- FULL_HELP_DOCS.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 214b9488b..30aa9f9cb 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -480,7 +480,7 @@ stellar contract invoke ... -- --help ###### **Options:** * `--id ` — Contract ID to invoke -* `--is-view` — View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` +* `--is-view` — View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` * `--rpc-url ` — RPC server endpoint * `--network-passphrase ` — Network passphrase to sign the transaction sent to the rpc server * `--network ` — Name of network to use from config @@ -501,11 +501,11 @@ stellar contract invoke ... -- --help Possible values: - `if-write`: - Only send transaction if there are ledger writes, otherwise return simulation result + Send the transaction to the network if there are ledger writes or events published, otherwise output the simulation result and do not send to the network - `no`: - Do not send transaction, return simulation result + Do not send the transaction to the network, and output the simulation result - `yes`: - Always send transaction + Send the transaction to the network, and output the transaction result From af0f2442dfb10e8939a46db1a2aeaa7d5fb733ad Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Fri, 2 Aug 2024 09:15:23 -0400 Subject: [PATCH 3/6] Update cmd/soroban-cli/src/commands/contract/invoke.rs Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> --- cmd/soroban-cli/src/commands/contract/invoke.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 948b3154e..ccb8bf315 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -45,7 +45,7 @@ pub struct Cmd { // For testing only #[arg(skip)] pub wasm: Option, - /// View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` + /// View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` #[arg(long, env = "STELLAR_INVOKE_VIEW")] pub is_view: bool, /// Function name as subcommand, then arguments for that function as `--arg-name value` From 4afe330048945c55a265d3b20d9467c706f70ab4 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Fri, 2 Aug 2024 09:29:01 -0400 Subject: [PATCH 4/6] feat: add published events to if-write --- FULL_HELP_DOCS.md | 8 ++-- .../src/commands/contract/invoke.rs | 39 ++++++++++++------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 30aa9f9cb..8eda3d4b7 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -480,7 +480,7 @@ stellar contract invoke ... -- --help ###### **Options:** * `--id ` — Contract ID to invoke -* `--is-view` — View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` +* `--is-view` — View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` * `--rpc-url ` — RPC server endpoint * `--network-passphrase ` — Network passphrase to sign the transaction sent to the rpc server * `--network ` — Name of network to use from config @@ -501,11 +501,11 @@ stellar contract invoke ... -- --help Possible values: - `if-write`: - Send the transaction to the network if there are ledger writes or events published, otherwise output the simulation result and do not send to the network + Only send transaction if there are ledger writes or published events, otherwise return simulation result - `no`: - Do not send the transaction to the network, and output the simulation result + Do not send transaction, return simulation result - `yes`: - Send the transaction to the network, and output the transaction result + Always send transaction diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index ccb8bf315..79f0bfcba 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -12,11 +12,12 @@ use heck::ToKebabCase; use soroban_env_host::{ xdr::{ - self, AccountEntry, AccountEntryExt, AccountId, Hash, HostFunction, InvokeContractArgs, - InvokeHostFunctionOp, LedgerEntryData, Limits, Memo, MuxedAccount, Operation, - OperationBody, Preconditions, PublicKey, ScAddress, ScSpecEntry, ScSpecFunctionV0, - ScSpecTypeDef, ScVal, ScVec, SequenceNumber, String32, StringM, Thresholds, Transaction, - TransactionExt, Uint256, VecM, WriteXdr, + self, AccountEntry, AccountEntryExt, AccountId, ContractEvent, ContractEventType, + DiagnosticEvent, Hash, HostFunction, InvokeContractArgs, InvokeHostFunctionOp, + LedgerEntryData, Limits, Memo, MuxedAccount, Operation, OperationBody, Preconditions, + PublicKey, ScAddress, ScSpecEntry, ScSpecFunctionV0, ScSpecTypeDef, ScVal, ScVec, + SequenceNumber, String32, StringM, Thresholds, Transaction, TransactionExt, Uint256, VecM, + WriteXdr, }, HostError, }; @@ -45,7 +46,7 @@ pub struct Cmd { // For testing only #[arg(skip)] pub wasm: Option, - /// View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` + /// View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` #[arg(long, env = "STELLAR_INVOKE_VIEW")] pub is_view: bool, /// Function name as subcommand, then arguments for that function as `--arg-name value` @@ -558,7 +559,7 @@ Note: The only types which aren't JSON are Bytes and BytesN, which are raw bytes #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, ValueEnum, Default)] pub enum Send { - /// Only send transaction if there are ledger writes, otherwise return simulation result + /// Only send transaction if there are ledger writes or published events, otherwise return simulation result #[default] IfWrite, /// Do not send transaction, return simulation result @@ -570,14 +571,26 @@ pub enum Send { impl Send { pub fn should_send(self, sim_res: &SimulateTransactionResponse) -> Result { Ok(match self { - Send::IfWrite => !sim_res - .transaction_data()? - .resources - .footprint - .read_write - .is_empty(), + Send::IfWrite => has_write(sim_res)? || has_published_event(sim_res)?, Send::No => false, Send::Yes => true, }) } } + +fn has_write(sim_res: &SimulateTransactionResponse) -> Result { + Ok(!sim_res + .transaction_data()? + .resources + .footprint + .read_write + .is_empty()) +} +fn has_published_event(sim_res: &SimulateTransactionResponse) -> Result { + Ok(!sim_res.events()?.iter().any( + |DiagnosticEvent { + event: ContractEvent { type_, .. }, + .. + }| matches!(type_, ContractEventType::Contract), + )) +} From 6e58f50b68e4ca5df00c74992384573a6fad574a Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 5 Aug 2024 10:05:49 -0400 Subject: [PATCH 5/6] fix: apply suggestion --- FULL_HELP_DOCS.md | 2 +- cmd/soroban-cli/src/commands/contract/invoke.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 8eda3d4b7..5a9f58bab 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -480,7 +480,7 @@ stellar contract invoke ... -- --help ###### **Options:** * `--id ` — Contract ID to invoke -* `--is-view` — View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` +* `--is-view` — View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` * `--rpc-url ` — RPC server endpoint * `--network-passphrase ` — Network passphrase to sign the transaction sent to the rpc server * `--network ` — Name of network to use from config diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 79f0bfcba..3012e4daf 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -46,7 +46,7 @@ pub struct Cmd { // For testing only #[arg(skip)] pub wasm: Option, - /// View the result simulating and do not sign and submit transaction. Ieprecated use `--send=no` + /// View the result simulating and do not sign and submit transaction. Deprecated use `--send=no` #[arg(long, env = "STELLAR_INVOKE_VIEW")] pub is_view: bool, /// Function name as subcommand, then arguments for that function as `--arg-name value` From 1e3975602a3ffbadfb064b53b9b390ff854152a9 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 7 Aug 2024 01:23:23 -0400 Subject: [PATCH 6/6] Update invoke.rs Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> --- cmd/soroban-cli/src/commands/contract/invoke.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 85080b625..fb8fabab7 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -57,7 +57,7 @@ pub struct Cmd { #[command(flatten)] pub fee: crate::fee::Args, /// Whether or not to send a transaction - #[arg(long, value_enum, default_value("if-write"))] + #[arg(long, value_enum, default_value("if-write"), env = "STELLAR_SEND")] pub send: Send, }