Skip to content

Commit

Permalink
Fix/invoke view only (#1697)
Browse files Browse the repository at this point in the history
* Use a default account if the tx is only being simulated

* Add a comment to get_matches_from for --help

* Update invoke_view_with_non_existent_source_account test
  • Loading branch information
elizabethengelman authored Nov 13, 2024
1 parent 9e15e95 commit 02c9c8e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 20 deletions.
4 changes: 1 addition & 3 deletions cmd/crates/soroban-test/tests/it/integration/hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ async fn invoke_view_with_non_existent_source_account() {
let id = deploy_hello(sandbox).await;
let world = "world";
let mut cmd = hello_world_cmd(&id, world);
cmd.config.source_account = Address::default();
cmd.is_view = true;
let res = sandbox.run_cmd_with(cmd, "test").await.unwrap();
let res = sandbox.run_cmd_with(cmd, "").await.unwrap();
assert_eq!(res, TxnResult::Res(format!(r#"["Hello",{world:?}]"#)));
}

Expand Down
3 changes: 3 additions & 0 deletions cmd/soroban-cli/src/commands/contract/arg_parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ pub fn build_host_function_parameters(
}
cmd.build();
let long_help = cmd.render_long_help();

// get_matches_from exits the process if `help`, `--help` or `-h`are passed in the slop
// see clap documentation for more info: https://github.com/clap-rs/clap/blob/v4.1.8/src/builder/command.rs#L631
let mut matches_ = cmd.get_matches_from(slop);
let Some((function, matches_)) = &matches_.remove_subcommand() else {
println!("{long_help}");
Expand Down
62 changes: 45 additions & 17 deletions cmd/soroban-cli/src/commands/contract/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{fmt::Debug, fs, io};

use clap::{arg, command, Parser, ValueEnum};

use soroban_rpc::{SimulateHostFunctionResult, SimulateTransactionResponse};
use soroban_rpc::{Client, SimulateHostFunctionResult, SimulateTransactionResponse};
use soroban_spec::read::FromWasmError;

use super::super::events;
Expand Down Expand Up @@ -163,7 +163,7 @@ impl Cmd {
.transpose()
}

fn send(&self, sim_res: &SimulateTransactionResponse) -> Result<ShouldSend, Error> {
fn should_send_tx(&self, sim_res: &SimulateTransactionResponse) -> Result<ShouldSend, Error> {
Ok(match self.send {
Send::Default => {
if self.is_view {
Expand All @@ -179,6 +179,28 @@ impl Cmd {
Send::Yes => ShouldSend::Yes,
})
}

// uses a default account to check if the tx should be sent after the simulation
async fn should_send_after_sim(
&self,
host_function_params: InvokeContractArgs,
rpc_client: Client,
) -> Result<ShouldSend, Error> {
let account_details = default_account_entry();
let sequence: i64 = account_details.seq_num.into();
let AccountId(PublicKey::PublicKeyTypeEd25519(account_id)) = account_details.account_id;

let tx = build_invoke_contract_tx(
host_function_params.clone(),
sequence + 1,
self.fee.fee,
account_id,
)?;
let txn = simulate_and_assemble_transaction(&rpc_client, &tx).await?;
let txn = self.fee.apply_to_assembled_txn(txn); // do we need this part?
let sim_res = txn.sim_response();
self.should_send_tx(sim_res)
}
}

#[async_trait::async_trait]
Expand All @@ -205,19 +227,6 @@ impl NetworkRunnable for Cmd {
let _ = build_host_function_parameters(&contract_id, &self.slop, spec_entries, config)?;
}
let client = network.rpc_client()?;
let account_details = if self.is_view {
default_account_entry()
} else {
client
.verify_network_passphrase(Some(&network.network_passphrase))
.await?;

client
.get_account(&config.source_account()?.to_string())
.await?
};
let sequence: i64 = account_details.seq_num.into();
let AccountId(PublicKey::PublicKeyTypeEd25519(account_id)) = account_details.account_id;

let spec_entries = get_remote_contract_spec(
&contract_id.0,
Expand All @@ -229,9 +238,27 @@ impl NetworkRunnable for Cmd {
.await
.map_err(Error::from)?;

// Get the ledger footprint
let (function, spec, host_function_params, signers) =
build_host_function_parameters(&contract_id, &self.slop, &spec_entries, config)?;

let should_send_tx = self
.should_send_after_sim(host_function_params.clone(), client.clone())
.await?;

let account_details = if should_send_tx == ShouldSend::Yes {
client
.verify_network_passphrase(Some(&network.network_passphrase))
.await?;

client
.get_account(&config.source_account()?.to_string())
.await?
} else {
default_account_entry()
};
let sequence: i64 = account_details.seq_num.into();
let AccountId(PublicKey::PublicKeyTypeEd25519(account_id)) = account_details.account_id;

let tx = build_invoke_contract_tx(
host_function_params.clone(),
sequence + 1,
Expand All @@ -250,7 +277,7 @@ impl NetworkRunnable for Cmd {
if global_args.map_or(true, |a| !a.no_cache) {
data::write(sim_res.clone().into(), &network.rpc_uri()?)?;
}
let should_send = self.send(sim_res)?;
let should_send = self.should_send_tx(sim_res)?;
let (return_value, events) = match should_send {
ShouldSend::Yes => {
let global::Args { no_cache, .. } = global_args.cloned().unwrap_or_default();
Expand Down Expand Up @@ -365,6 +392,7 @@ pub enum Send {
Yes,
}

#[derive(Debug, PartialEq)]
enum ShouldSend {
DefaultNo,
No,
Expand Down

0 comments on commit 02c9c8e

Please sign in to comment.