Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --send to contract invoke and default to only sending transaction if there are writes in the ledger footprint #1518

Merged
merged 9 commits into from
Aug 7, 2024
14 changes: 13 additions & 1 deletion FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ stellar contract invoke ... -- --help
###### **Options:**

* `--id <CONTRACT_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`
willemneal marked this conversation as resolved.
Show resolved Hide resolved
willemneal marked this conversation as resolved.
Show resolved Hide resolved
* `--rpc-url <RPC_URL>` — RPC server endpoint
* `--network-passphrase <NETWORK_PASSPHRASE>` — Network passphrase to sign the transaction sent to the rpc server
* `--network <NETWORK>` — Name of network to use from config
Expand All @@ -495,6 +495,18 @@ stellar contract invoke ... -- --help
* `--instructions <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 <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




Expand Down
77 changes: 55 additions & 22 deletions cmd/soroban-cli/src/commands/contract/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,7 +46,7 @@ pub struct Cmd {
// For testing only
#[arg(skip)]
pub wasm: Option<std::path::PathBuf>,
/// 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`
willemneal marked this conversation as resolved.
Show resolved Hide resolved
#[arg(long, env = "STELLAR_INVOKE_VIEW")]
pub is_view: bool,
/// Function name as subcommand, then arguments for that function as `--arg-name value`
Expand All @@ -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"))]
willemneal marked this conversation as resolved.
Show resolved Hide resolved
pub send: Send,
}

impl FromStr for Cmd {
Expand Down Expand Up @@ -153,14 +158,9 @@ impl From<Infallible> 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<bool, Error> {
Ok(self.send.should_send(sim_res)? && !self.is_view)
}

fn build_host_function_parameters(
&self,
contract_id: [u8; 32],
Expand Down Expand Up @@ -366,27 +366,22 @@ 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?;
if !no_cache {
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);
Expand Down Expand Up @@ -561,3 +556,41 @@ Each arg has a corresponding --<arg_name>-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<bool, Error> {
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<bool, Error> {
Ok(!sim_res
.transaction_data()?
.resources
.footprint
.read_write
.is_empty())
}
fn has_published_event(sim_res: &SimulateTransactionResponse) -> Result<bool, Error> {
Ok(!sim_res.events()?.iter().any(
|DiagnosticEvent {
event: ContractEvent { type_, .. },
..
}| matches!(type_, ContractEventType::Contract),
))
}
Loading