From d1c6a9db92e045377fbdbc773342d9bfd802c594 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Wed, 7 Aug 2024 01:36:20 -0400 Subject: [PATCH] feat: add `--send` to `contract invoke` and default to only sending transaction if there are writes in the ledger footprint (#1518) --- FULL_HELP_DOCS.md | 14 +++- .../src/commands/contract/invoke.rs | 77 +++++++++++++------ 2 files changed, 68 insertions(+), 23 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index b7e0011e3..a84f43467 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -481,7 +481,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. 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 @@ -496,6 +496,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 or published events, 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 6d0a0799b..fb8fabab7 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -6,21 +6,23 @@ 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; 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, }; +use soroban_rpc::SimulateTransactionResponse; use soroban_spec::read::FromWasmError; use super::super::events; @@ -44,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 + /// 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` @@ -54,6 +56,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"), env = "STELLAR_SEND")] + pub send: Send, } impl FromStr for Cmd { @@ -153,14 +158,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 +366,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?; @@ -392,6 +385,8 @@ impl NetworkRunnable for Cmd { .map(crate::log::extract_events) .unwrap_or_default(); (res.return_value()?, events) + } else { + (sim_res.results()?[0].xdr.clone(), sim_res.events()?) }; crate::log::events(&events); output_to_string(&spec, &return_value, &function) @@ -565,3 +560,41 @@ 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 or published events, 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 => 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), + )) +}