soroban-cli: Fix --cost flag (stellar#911)

* WIP -- log SorobanResources instead * Log resources as cost if `--verbose` set or `--cost` * Simpler solution, log budget for sandbox, and resources for real * Only log the final events&auth when invoking on a real network, not every time you simulate * make fmt * clippy * Update test for assemble to be a passthrough * make fmt * feedback * fix formatting
urvisavla · Sep 1, 2023 · f1f9fa8 · f1f9fa8
1 parent 5f78b67
commit f1f9fa8
Show file tree

Hide file tree

Showing 13 changed files with 154 additions and 95 deletions.
diff --git a/cmd/crates/soroban-test/src/lib.rs b/cmd/crates/soroban-test/src/lib.rs
@@ -29,9 +29,8 @@ use assert_cmd::{assert::Assert, Command};
 use assert_fs::{fixture::FixtureError, prelude::PathChild, TempDir};
 use fs_extra::dir::CopyOptions;
 
-pub use soroban_cli::commands::contract::invoke;
 use soroban_cli::{
-    commands::{config, contract},
+    commands::{config, contract, contract::invoke, global},
     CommandParser, Pwd,
 };
 
@@ -134,7 +133,17 @@ impl TestEnv {
     /// Invoke an already parsed invoke command
     pub fn invoke_cmd(&self, mut cmd: invoke::Cmd) -> Result<String, invoke::Error> {
         cmd.set_pwd(self.dir());
-        cmd.run_in_sandbox()
+        cmd.run_in_sandbox(&global::Args {
+            locator: config::locator::Args {
+                global: false,
+                config_dir: None,
+            },
+            filter_logs: Vec::default(),
+            quiet: false,
+            verbose: false,
+            very_verbose: false,
+            list: false,
+        })
     }
 
     /// Reference to current directory of the `TestEnv`.

diff --git a/cmd/soroban-cli/src/commands/contract/bump.rs b/cmd/soroban-cli/src/commands/contract/bump.rs
@@ -170,7 +170,7 @@ impl Cmd {
         };
 
         let (result, meta, events) = client
-            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None)
+            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None, None)
             .await?;
 
         tracing::trace!(?result);

diff --git a/cmd/soroban-cli/src/commands/contract/deploy.rs b/cmd/soroban-cli/src/commands/contract/deploy.rs
@@ -185,7 +185,7 @@ impl Cmd {
             &key,
         )?;
         client
-            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None)
+            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None, None)
             .await?;
         Ok(stellar_strkey::Contract(contract_id.0).to_string())
     }

diff --git a/cmd/soroban-cli/src/commands/contract/install.rs b/cmd/soroban-cli/src/commands/contract/install.rs
@@ -106,6 +106,7 @@ impl Cmd {
                 &[],
                 &network.network_passphrase,
                 None,
+                None,
             )
             .await?
         {

diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs
@@ -19,7 +19,8 @@ use soroban_env_host::{
         InvokeHostFunctionOp, LedgerEntryData, LedgerFootprint, LedgerKey, LedgerKeyAccount, Memo,
         MuxedAccount, Operation, OperationBody, Preconditions, PublicKey, ScAddress, ScSpecEntry,
         ScSpecFunctionV0, ScSpecTypeDef, ScVal, ScVec, SequenceNumber, SorobanAddressCredentials,
-        SorobanAuthorizationEntry, SorobanCredentials, Transaction, TransactionExt, Uint256, VecM,
+        SorobanAuthorizationEntry, SorobanCredentials, SorobanResources, Transaction,
+        TransactionExt, Uint256, VecM,
     },
     DiagnosticLevel, Host, HostError,
 };
@@ -32,7 +33,7 @@ use super::super::{
     events,
 };
 use crate::{
-    commands::HEADING_SANDBOX,
+    commands::{global, HEADING_SANDBOX},
     rpc::{self, Client},
     utils::{self, contract_spec, create_ledger_footprint, default_account_ledger_entry},
     Pwd,
@@ -262,21 +263,24 @@ impl Cmd {
         Ok((function.clone(), spec, invoke_args, signers))
     }
 
-    pub async fn run(&self) -> Result<(), Error> {
-        let res = self.invoke().await?;
+    pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> {
+        let res = self.invoke(global_args).await?;
         println!("{res}");
         Ok(())
     }
 
-    pub async fn invoke(&self) -> Result<String, Error> {
+    pub async fn invoke(&self, global_args: &global::Args) -> Result<String, Error> {
         if self.config.is_no_network() {
-            self.run_in_sandbox()
+            self.run_in_sandbox(global_args)
         } else {
-            self.run_against_rpc_server().await
+            self.run_against_rpc_server(global_args).await
         }
     }
 
-    pub async fn run_against_rpc_server(&self) -> Result<String, Error> {
+    pub async fn run_against_rpc_server(
+        &self,
+        global_args: &global::Args,
+    ) -> Result<String, Error> {
         let network = self.config.get_network()?;
         tracing::trace!(?network);
         let contract_id = self.contract_id()?;
@@ -315,6 +319,8 @@ impl Cmd {
                 &signers,
                 &network.network_passphrase,
                 Some(log_events),
+                (global_args.verbose || global_args.very_verbose || self.cost)
+                    .then_some(log_resources),
             )
             .await?;
 
@@ -331,7 +337,7 @@ impl Cmd {
         output_to_string(&spec, &return_value, &function)
     }
 
-    pub fn run_in_sandbox(&self) -> Result<String, Error> {
+    pub fn run_in_sandbox(&self, global_args: &global::Args) -> Result<String, Error> {
         let contract_id = self.contract_id()?;
         // Initialize storage and host
         // TODO: allow option to separate input and output file
@@ -417,8 +423,12 @@ impl Cmd {
         let budget = h.budget_cloned();
         let (storage, events) = h.try_finish()?;
         let footprint = &create_ledger_footprint(&storage.footprint);
+
         crate::log::host_events(&events.0);
-        log_events(footprint, &[contract_auth.try_into()?], &[], Some(&budget));
+        log_events(footprint, &[contract_auth.try_into()?], &[]);
+        if global_args.verbose || global_args.very_verbose || self.cost {
+            log_budget(&budget);
+        }
 
         let ledger_changes = get_ledger_changes(&budget, &storage, &state)?;
         let mut expiration_ledger_bumps: HashMap<LedgerKey, u32> = HashMap::new();
@@ -489,14 +499,18 @@ fn log_events(
     footprint: &LedgerFootprint,
     auth: &[VecM<SorobanAuthorizationEntry>],
     events: &[xdr::DiagnosticEvent],
-    budget: Option<&Budget>,
 ) {
     crate::log::auth(auth);
     crate::log::diagnostic_events(events, tracing::Level::TRACE);
     crate::log::footprint(footprint);
-    if let Some(budget) = budget {
-        crate::log::budget(budget);
-    }
+}
+
+fn log_budget(budget: &Budget) {
+    crate::log::budget(budget);
+}
+
+fn log_resources(resources: &SorobanResources) {
+    crate::log::cost(resources);
 }
 
 pub fn output_to_string(spec: &Spec, res: &ScVal, function: &str) -> Result<String, Error> {

diff --git a/cmd/soroban-cli/src/commands/contract/mod.rs b/cmd/soroban-cli/src/commands/contract/mod.rs
@@ -10,6 +10,8 @@ pub mod optimize;
 pub mod read;
 pub mod restore;
 
+use crate::commands::global;
+
 #[derive(Debug, clap::Subcommand)]
 pub enum Cmd {
     /// Generate code client bindings for a contract
@@ -94,15 +96,15 @@ pub enum Error {
 }
 
 impl Cmd {
-    pub async fn run(&self) -> Result<(), Error> {
+    pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> {
         match &self {
             Cmd::Bindings(bindings) => bindings.run().await?,
             Cmd::Build(build) => build.run()?,
             Cmd::Bump(bump) => bump.run().await?,
             Cmd::Deploy(deploy) => deploy.run().await?,
             Cmd::Inspect(inspect) => inspect.run()?,
             Cmd::Install(install) => install.run().await?,
-            Cmd::Invoke(invoke) => invoke.run().await?,
+            Cmd::Invoke(invoke) => invoke.run(global_args).await?,
             Cmd::Optimize(optimize) => optimize.run()?,
             Cmd::Fetch(fetch) => fetch.run().await?,
             Cmd::Read(read) => read.run().await?,

diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs
@@ -177,7 +177,7 @@ impl Cmd {
         };
 
         let (result, meta, events) = client
-            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None)
+            .prepare_and_send_transaction(&tx, &key, &[], &network.network_passphrase, None, None)
             .await?;
 
         tracing::trace!(?result);

diff --git a/cmd/soroban-cli/src/commands/lab/token/wrap.rs b/cmd/soroban-cli/src/commands/lab/token/wrap.rs
@@ -126,7 +126,7 @@ impl Cmd {
         )?;
 
         client
-            .prepare_and_send_transaction(&tx, &key, &[], network_passphrase, None)
+            .prepare_and_send_transaction(&tx, &key, &[], network_passphrase, None, None)
             .await?;
 
         Ok(stellar_strkey::Contract(contract_id.0).to_string())

diff --git a/cmd/soroban-cli/src/commands/mod.rs b/cmd/soroban-cli/src/commands/mod.rs
@@ -91,7 +91,7 @@ impl Root {
         match &mut self.cmd {
             Cmd::Completion(completion) => completion.run(),
             Cmd::Config(config) => config.run().await?,
-            Cmd::Contract(contract) => contract.run().await?,
+            Cmd::Contract(contract) => contract.run(&self.global_args).await?,
             Cmd::Events(events) => events.run().await?,
             Cmd::Lab(lab) => lab.run().await?,
             Cmd::Version(version) => version.run(),

diff --git a/cmd/soroban-cli/src/log.rs b/cmd/soroban-cli/src/log.rs
@@ -1,11 +1,13 @@
 pub mod auth;
 pub mod budget;
+pub mod cost;
 pub mod diagnostic_event;
 pub mod footprint;
 pub mod host_event;
 
 pub use auth::*;
 pub use budget::*;
+pub use cost::*;
 pub use diagnostic_event::*;
 pub use footprint::*;
 pub use host_event::*;
diff --git a/cmd/soroban-cli/src/log/cost.rs b/cmd/soroban-cli/src/log/cost.rs
@@ -0,0 +1,27 @@
+use soroban_env_host::xdr::SorobanResources;
+use std::fmt::{Debug, Display};
+
+struct Cost<'a>(&'a SorobanResources);
+
+impl Debug for Cost<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        // TODO: Should we output the footprint here?
+        writeln!(f, "==================== Cost ====================")?;
+        writeln!(f, "CPU used: {}", self.0.instructions,)?;
+        writeln!(f, "Bytes read: {}", self.0.read_bytes,)?;
+        writeln!(f, "Bytes written: {}", self.0.write_bytes,)?;
+        writeln!(f, "==============================================")?;
+        Ok(())
+    }
+}
+
+impl Display for Cost<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        Debug::fmt(self, f)
+    }
+}
+
+pub fn cost(resources: &SorobanResources) {
+    let cost = Cost(resources);
+    tracing::debug!(?cost);
+}
diff --git a/cmd/soroban-cli/src/rpc/mod.rs b/cmd/soroban-cli/src/rpc/mod.rs
@@ -5,14 +5,11 @@ use jsonrpsee_core::{self, client::ClientT, rpc_params};
 use jsonrpsee_http_client::{HeaderMap, HttpClient, HttpClientBuilder};
 use serde_aux::prelude::{deserialize_default_from_null, deserialize_number_from_string};
 use soroban_env_host::xdr::DepthLimitedRead;
-use soroban_env_host::{
-    budget::Budget,
-    xdr::{
-        self, AccountEntry, AccountId, ContractDataEntry, DiagnosticEvent, Error as XdrError,
-        LedgerEntryData, LedgerFootprint, LedgerKey, LedgerKeyAccount, PublicKey, ReadXdr,
-        SorobanAuthorizationEntry, Transaction, TransactionEnvelope, TransactionMeta,
-        TransactionMetaV3, TransactionResult, TransactionV1Envelope, Uint256, VecM, WriteXdr,
-    },
+use soroban_env_host::xdr::{
+    self, AccountEntry, AccountId, ContractDataEntry, DiagnosticEvent, Error as XdrError,
+    LedgerEntryData, LedgerFootprint, LedgerKey, LedgerKeyAccount, PublicKey, ReadXdr,
+    SorobanAuthorizationEntry, SorobanResources, Transaction, TransactionEnvelope, TransactionMeta,
+    TransactionMetaV3, TransactionResult, TransactionV1Envelope, Uint256, VecM, WriteXdr,
 };
 use soroban_sdk::token;
 use std::{
@@ -35,9 +32,10 @@ pub type LogEvents = fn(
     footprint: &LedgerFootprint,
     auth: &[VecM<SorobanAuthorizationEntry>],
     events: &[DiagnosticEvent],
-    budget: Option<&Budget>,
 ) -> ();
 
+pub type LogResources = fn(resources: &SorobanResources) -> ();
+
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
     #[error("invalid address: {0}")]
@@ -603,16 +601,22 @@ soroban config identity fund {address} --helper-url <url>"#
     pub async fn prepare_transaction(
         &self,
         tx: &Transaction,
-        log_events: Option<LogEvents>,
-    ) -> Result<Transaction, Error> {
+    ) -> Result<(Transaction, Vec<DiagnosticEvent>), Error> {
         tracing::trace!(?tx);
         let sim_response = self
             .simulate_transaction(&TransactionEnvelope::Tx(TransactionV1Envelope {
                 tx: tx.clone(),
                 signatures: VecM::default(),
             }))
             .await?;
-        assemble(tx, &sim_response, log_events)
+
+        let events = sim_response
+            .events
+            .iter()
+            .map(DiagnosticEvent::from_xdr_base64)
+            .collect::<Result<Vec<_>, _>>()?;
+
+        Ok((assemble(tx, &sim_response)?, events))
     }
 
     pub async fn prepare_and_send_transaction(
@@ -622,25 +626,47 @@ soroban config identity fund {address} --helper-url <url>"#
         signers: &[ed25519_dalek::Keypair],
         network_passphrase: &str,
         log_events: Option<LogEvents>,
+        log_resources: Option<LogResources>,
     ) -> Result<(TransactionResult, TransactionMeta, Vec<DiagnosticEvent>), Error> {
         let GetLatestLedgerResponse { sequence, .. } = self.get_latest_ledger().await?;
-        let unsigned_tx = self
-            .prepare_transaction(tx_without_preflight, log_events)
-            .await?;
+        let (unsigned_tx, events) = self.prepare_transaction(tx_without_preflight).await?;
         let (part_signed_tx, signed_auth_entries) = sign_soroban_authorizations(
             &unsigned_tx,
             source_key,
             signers,
             sequence + 60, // ~5 minutes of ledgers
             network_passphrase,
         )?;
-        let fee_ready_txn = if signed_auth_entries.is_empty() {
-            part_signed_tx
+        let (fee_ready_txn, events) = if signed_auth_entries.is_empty() {
+            (part_signed_tx, events)
         } else {
             // re-simulate to calculate the new fees
-            self.prepare_transaction(&part_signed_tx, log_events)
-                .await?
+            self.prepare_transaction(&part_signed_tx).await?
         };
+
+        // Try logging stuff if requested
+        if let Transaction {
+            ext: xdr::TransactionExt::V1(xdr::SorobanTransactionData { resources, .. }),
+            ..
+        } = fee_ready_txn.clone()
+        {
+            if let Some(log) = log_events {
+                if let xdr::Operation {
+                    body:
+                        xdr::OperationBody::InvokeHostFunction(xdr::InvokeHostFunctionOp {
+                            auth, ..
+                        }),
+                    ..
+                } = &fee_ready_txn.operations[0]
+                {
+                    log(&resources.footprint, &[auth.clone()], &events);
+                }
+            }
+            if let Some(log) = log_resources {
+                log(&resources);
+            }
+        }
+
         let tx = utils::sign_transaction(source_key, &fee_ready_txn, network_passphrase)?;
         self.send_transaction(&tx).await
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -106,6 +106,7 @@ impl Cmd { @@
                     &[],
                     &network.network_passphrase,
                     None,
+                    None,
                 )
                 .await?
             {
@@ Expand Down @@