From f04f346e43ad1127c6beaac7df9279157969fb15 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Wed, 9 Oct 2024 15:59:33 +0200 Subject: [PATCH 01/12] cleanup Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 126 ++++++++++++++++++++--- crates/iota/src/client_ptb/ptb.rs | 1 + crates/iota/tests/cli_tests.rs | 158 ++++++++++++++++++++++++++++- 3 files changed, 270 insertions(+), 15 deletions(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index b297d6ddc3f..2e19bf3ae38 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -649,6 +649,13 @@ pub struct Opts { /// --signed-tx-bytes `. #[arg(long, required = false)] pub serialize_signed_transaction: bool, + + /// Select which fields of the response to display. + /// If not provided, all fields are displayed. + /// The fields are: effects, input, events, object_changes, + /// balance_changes. + #[clap(long, required = false, value_delimiter = ',', num_args = 0.., value_parser = parse_emit_opts)] + pub emit: Vec, } /// Global options with gas @@ -665,23 +672,37 @@ pub struct OptsWithGas { impl Opts { /// Uses the passed gas_budget for the gas budget variable and sets all - /// other flags to false. + /// other flags to false, and emit to an empty vector(defaulting to all emit options). pub fn for_testing(gas_budget: u64) -> Self { Self { gas_budget: Some(gas_budget), dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + emit: vec![], } } /// Uses the passed gas_budget for the gas budget variable, sets dry run to - /// true, and sets all other flags to false. + /// true, and sets all other flags to false, and emit to an empty vector(defaulting to all emit options). pub fn for_testing_dry_run(gas_budget: u64) -> Self { Self { gas_budget: Some(gas_budget), dry_run: true, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + emit: vec![], + } + } + + /// Uses the passed gas_budget for the gas budget variable, sets dry run to + /// false, and sets all other flags to false, and emit to the passed emit vector. + pub fn for_testing_emit_options(gas_budget: u64, emit: Vec) -> Self { + Self { + gas_budget: Some(gas_budget), + dry_run: false, + serialize_unsigned_transaction: false, + serialize_signed_transaction: false, + emit, } } } @@ -703,6 +724,50 @@ impl OptsWithGas { rest: Opts::for_testing_dry_run(gas_budget), } } + + /// Sets the gas object to gas, and uses the passed gas_budget for the gas + /// budget variable. Dry run is set to false, and emit to the passed emit vector. + /// All other flags are set to false. + pub fn for_testing_emit_options( + gas: Option, + gas_budget: u64, + emit: Vec, + ) -> Self { + Self { + gas, + rest: Opts::for_testing_emit_options(gas_budget, emit), + } + } +} + +#[derive(Clone, Debug)] +pub enum EmitOption { + Effects, + Input, + Events, + ObjectChanges, + BalanceChanges, +} + +/// Converts a string into the corresponding `EmitOption` enum. +/// +/// # Arguments +/// * `s` - A string representing an option. +/// +/// # Returns +/// * `Ok(EmitOption)` if the string is valid. +/// * `Err(String)` if the string is invalid. +/// +/// Valid options: "effects", "input", "events", "object_changes", "balance_changes". +fn parse_emit_opts(s: &str) -> Result { + match s { + "effects" => Ok(EmitOption::Effects), + "input" => Ok(EmitOption::Input), + "events" => Ok(EmitOption::Events), + "object_changes" => Ok(EmitOption::ObjectChanges), + "balance_changes" => Ok(EmitOption::BalanceChanges), + _ => Err(format!("Invalid option: {}", s)), + } } #[derive(serde::Deserialize)] @@ -2930,18 +2995,28 @@ pub(crate) async fn dry_run_or_execute_or_serialize( )) } else { let transaction = Transaction::new(sender_signed_data); - let mut response = context.execute_transaction_may_fail(transaction).await?; - if let Some(effects) = response.effects.as_mut() { - prerender_clever_errors(effects, client.read_api()).await; - } - let effects = response.effects.as_ref().ok_or_else(|| { - anyhow!("Effects from IotaTransactionBlockResult should not be empty") - })?; - if let IotaExecutionStatus::Failure { error } = effects.status() { - return Err(anyhow!( - "Error executing transaction '{}': {error}", - response.digest - )); + let response: iota_json_rpc_types::IotaTransactionBlockResponse = + client + .quorum_driver_api() + .execute_transaction_block( + transaction, + if opts.emit.is_empty() { + IotaTransactionBlockResponseOptions::new() + .with_effects() + .with_input() + .with_events() + .with_object_changes() + .with_balance_changes() + } else { + opts_from_cli(opts.emit) + }, + Some(iota_types::quorum_driver_types::ExecuteTransactionRequestType::WaitForLocalExecution), + ) + .await?; + + let errors: &Vec = response.errors.as_ref(); + if !errors.is_empty() { + return Err(anyhow!("Error executing transaction: {:#?}", errors)); } Ok(IotaClientCommandResult::TransactionBlock(response)) } @@ -2959,3 +3034,26 @@ pub(crate) async fn prerender_clever_errors( } } } + +fn opts_from_cli(opts: Vec) -> IotaTransactionBlockResponseOptions { + if opts.is_empty() { + return IotaTransactionBlockResponseOptions::new() + .with_effects() + .with_input() + .with_events() + .with_object_changes() + .with_balance_changes(); + } + + let mut options = IotaTransactionBlockResponseOptions::new(); + for opt in opts { + match opt { + EmitOption::Input => options.show_input = true, + EmitOption::Events => options.show_events = true, + EmitOption::ObjectChanges => options.show_object_changes = true, + EmitOption::BalanceChanges => options.show_balance_changes = true, + EmitOption::Effects => options.show_effects = true, + } + } + options +} diff --git a/crates/iota/src/client_ptb/ptb.rs b/crates/iota/src/client_ptb/ptb.rs index f82fbe70636..3276bfddc5b 100644 --- a/crates/iota/src/client_ptb/ptb.rs +++ b/crates/iota/src/client_ptb/ptb.rs @@ -154,6 +154,7 @@ impl PTB { gas_budget: program_metadata.gas_budget.map(|x| x.value), serialize_unsigned_transaction: program_metadata.serialize_unsigned_set, serialize_signed_transaction: program_metadata.serialize_signed_set, + emit: Vec::new(), }, }; diff --git a/crates/iota/tests/cli_tests.rs b/crates/iota/tests/cli_tests.rs index c981e06059f..0637167a182 100644 --- a/crates/iota/tests/cli_tests.rs +++ b/crates/iota/tests/cli_tests.rs @@ -19,7 +19,7 @@ use iota::iota_commands::IndexerFeatureArgs; use iota::{ client_commands::{ IotaClientCommandResult, IotaClientCommands, Opts, OptsWithGas, SwitchResponse, - estimate_gas_budget, + estimate_gas_budget, EmitOption }, client_ptb::ptb::PTB, iota_commands::{IotaCommand, parse_host_port}, @@ -2113,6 +2113,10 @@ async fn test_package_management_on_upgrade_command_conflict() -> Result<(), any // Purposely add a conflicting `published-at` address to the Move manifest. lines.insert(idx + 1, "published-at = \"0xbad\"".to_string()); let new = lines.join("\n"); + + #[cfg(target_os = "windows")] + move_toml.seek_write(new.as_bytes(), 0).unwrap(); + #[cfg(not(target_os = "windows"))] move_toml.write_at(new.as_bytes(), 0).unwrap(); // Create a new build config for the upgrade. Initialize its lock file to the @@ -2965,6 +2969,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: true, serialize_signed_transaction: false, + emit: Vec::new(), }, } .execute(context) @@ -2979,6 +2984,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: true, + emit: Vec::new(), }, } .execute(context) @@ -2994,6 +3000,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: true, + emit: Vec::new(), }, } .execute(context) @@ -3845,6 +3852,7 @@ async fn test_gas_estimation() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + emit: Vec::new(), }, } .execute(context) @@ -4089,3 +4097,151 @@ async fn test_parse_host_port() { let input = "127.9.0.1:asb"; assert!(parse_host_port(input.to_string(), 9123).is_err()); } + +#[sim_test] +async fn test_call_command_emit_args() -> Result<(), anyhow::Error> { + // Publish the package + move_package::package_hooks::register_package_hooks(Box::new(IotaPackageHooks)); + let mut test_cluster = TestClusterBuilder::new().build().await; + let rgp = test_cluster.get_reference_gas_price().await; + let address = test_cluster.get_address_0(); + let context = &mut test_cluster.wallet; + let client = context.get_client().await?; + let object_refs = client + .read_api() + .get_owned_objects( + address, + Some(IotaObjectResponseQuery::new_with_options( + IotaObjectDataOptions::new() + .with_type() + .with_owner() + .with_previous_transaction(), + )), + None, + None, + ) + .await? + .data; + + let gas_obj_id = object_refs.first().unwrap().object().unwrap().object_id; + + // Provide path to well formed package sources + let mut package_path = PathBuf::from(TEST_DATA_DIR); + package_path.push("dummy_modules_upgrade"); + let build_config = BuildConfig::new_for_testing().config; + let resp = IotaClientCommands::Publish { + package_path: package_path.clone(), + build_config, + skip_dependency_verification: false, + with_unpublished_dependencies: false, + opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH), + } + .execute(context) + .await?; + + let effects = match resp { + IotaClientCommandResult::TransactionBlock(response) => response.effects.unwrap(), + _ => panic!("Expected TransactionBlock response"), + }; + + let package = effects + .created() + .iter() + .find(|refe| matches!(refe.owner, Owner::Immutable)) + .unwrap(); + + let start_call_result = IotaClientCommands::Call { + package: package.reference.object_id, + module: "trusted_coin".to_string(), + function: "f".to_string(), + type_args: vec![], + gas_price: None, + args: vec![], + opts: OptsWithGas::for_testing_emit_options( + None, + rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, + vec![EmitOption::BalanceChanges], + ), + } + .execute(context) + .await?; + + if let Some(tx_block_response) = start_call_result.tx_block_response() { + // Assert Balance Changes are present in the response + assert!(tx_block_response.balance_changes.is_some()); + + // Assert every other field is not present in the response + assert!(tx_block_response.effects.is_none()); + assert!(tx_block_response.object_changes.is_none()); + assert!(tx_block_response.events.is_none()); + assert!(tx_block_response.transaction.is_none()); + } else { + panic!("Transaction block response is None"); + } + + // Make another call, this time with multiple emit args + let start_call_result = IotaClientCommands::Call { + package: package.reference.object_id, + module: "trusted_coin".to_string(), + function: "f".to_string(), + type_args: vec![], + gas_price: None, + args: vec![], + opts: OptsWithGas::for_testing_emit_options( + None, + rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, + vec![ + EmitOption::BalanceChanges, + EmitOption::Effects, + EmitOption::ObjectChanges, + ], + ), + } + .execute(context) + .await?; + + start_call_result.print(true); + + // Assert Balance Changes, effects and object changes are present in the + // response + if let Some(tx_block_response) = start_call_result.tx_block_response() { + assert!(tx_block_response.balance_changes.is_some()); + assert!(tx_block_response.effects.is_some()); + assert!(tx_block_response.object_changes.is_some()); + assert!(tx_block_response.events.is_none()); + assert!(tx_block_response.transaction.is_none()); + } else { + panic!("Transaction block response is None"); + } + + // Make another call, this time with no emit args. This should return the full + // response + let start_call_result = IotaClientCommands::Call { + package: package.reference.object_id, + module: "trusted_coin".to_string(), + function: "f".to_string(), + type_args: vec![], + gas_price: None, + args: vec![], + opts: OptsWithGas::for_testing_emit_options( + None, + rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, + vec![], + ), + } + .execute(context) + .await?; + + // Assert all fields are present in the response + if let Some(tx_block_response) = start_call_result.tx_block_response() { + assert!(tx_block_response.balance_changes.is_some()); + assert!(tx_block_response.effects.is_some()); + assert!(tx_block_response.object_changes.is_some()); + assert!(tx_block_response.events.is_some()); + assert!(tx_block_response.transaction.is_some()); + } else { + panic!("Transaction block response is None"); + } + + Ok(()) +} From 576412b45f26287ed6a909556ba177cba3a0a8aa Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Thu, 10 Oct 2024 12:51:47 +0200 Subject: [PATCH 02/12] use strum to remove custom value parser Signed-off-by: salaheldinsoliman --- Cargo.lock | 2 ++ crates/iota/Cargo.toml | 2 ++ crates/iota/src/client_commands.rs | 38 ++++-------------------------- 3 files changed, 9 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c4dd77cb70..1af62198503 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5708,6 +5708,8 @@ dependencies = [ "shell-words", "shlex", "signature 1.6.4", + "strum 0.26.3", + "strum_macros 0.26.4", "tabled", "tap", "telemetry-subscribers", diff --git a/crates/iota/Cargo.toml b/crates/iota/Cargo.toml index 8668390c270..d9495fc03ce 100644 --- a/crates/iota/Cargo.toml +++ b/crates/iota/Cargo.toml @@ -49,6 +49,8 @@ tower-http.workspace = true tracing.workspace = true url.workspace = true uuid.workspace = true +strum_macros.workspace = true +strum.workspace = true iota-bridge.workspace = true iota-config.workspace = true diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index 2e19bf3ae38..a8401700077 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -83,6 +83,7 @@ use tabled::{ }, }; use tracing::info; +use strum_macros::EnumString; use crate::{ clever_error_rendering::render_clever_error_opt, @@ -654,7 +655,7 @@ pub struct Opts { /// If not provided, all fields are displayed. /// The fields are: effects, input, events, object_changes, /// balance_changes. - #[clap(long, required = false, value_delimiter = ',', num_args = 0.., value_parser = parse_emit_opts)] + #[clap(long, required = false, value_delimiter = ',', num_args = 0..)] pub emit: Vec, } @@ -740,7 +741,8 @@ impl OptsWithGas { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, EnumString)] +#[strum(serialize_all = "snake_case")] pub enum EmitOption { Effects, Input, @@ -749,27 +751,6 @@ pub enum EmitOption { BalanceChanges, } -/// Converts a string into the corresponding `EmitOption` enum. -/// -/// # Arguments -/// * `s` - A string representing an option. -/// -/// # Returns -/// * `Ok(EmitOption)` if the string is valid. -/// * `Err(String)` if the string is invalid. -/// -/// Valid options: "effects", "input", "events", "object_changes", "balance_changes". -fn parse_emit_opts(s: &str) -> Result { - match s { - "effects" => Ok(EmitOption::Effects), - "input" => Ok(EmitOption::Input), - "events" => Ok(EmitOption::Events), - "object_changes" => Ok(EmitOption::ObjectChanges), - "balance_changes" => Ok(EmitOption::BalanceChanges), - _ => Err(format!("Invalid option: {}", s)), - } -} - #[derive(serde::Deserialize)] struct FaucetResponse { error: Option, @@ -3000,16 +2981,7 @@ pub(crate) async fn dry_run_or_execute_or_serialize( .quorum_driver_api() .execute_transaction_block( transaction, - if opts.emit.is_empty() { - IotaTransactionBlockResponseOptions::new() - .with_effects() - .with_input() - .with_events() - .with_object_changes() - .with_balance_changes() - } else { - opts_from_cli(opts.emit) - }, + opts_from_cli(opts.emit), Some(iota_types::quorum_driver_types::ExecuteTransactionRequestType::WaitForLocalExecution), ) .await?; From 2a8e8d2643ea6d3aaae376effbd879523b53b6ab Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Thu, 10 Oct 2024 12:57:17 +0200 Subject: [PATCH 03/12] appease dprint Signed-off-by: salaheldinsoliman --- crates/iota/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/iota/Cargo.toml b/crates/iota/Cargo.toml index d9495fc03ce..42e810f2b56 100644 --- a/crates/iota/Cargo.toml +++ b/crates/iota/Cargo.toml @@ -40,6 +40,8 @@ serde.workspace = true serde_json.workspace = true serde_yaml.workspace = true signature.workspace = true +strum_macros.workspace = true +strum.workspace = true tabled.workspace = true tap.workspace = true thiserror.workspace = true @@ -49,8 +51,6 @@ tower-http.workspace = true tracing.workspace = true url.workspace = true uuid.workspace = true -strum_macros.workspace = true -strum.workspace = true iota-bridge.workspace = true iota-config.workspace = true From d7b119e99a7a3599e917d52ce4937ae3540a99d2 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Thu, 10 Oct 2024 13:35:59 +0200 Subject: [PATCH 04/12] remove unneeded type annotation Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index a8401700077..7344c787f3c 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -61,6 +61,7 @@ use iota_types::{ transaction::{ SenderSignedData, Transaction, TransactionData, TransactionDataAPI, TransactionKind, }, + quorum_driver_types::ExecuteTransactionRequestType }; use json_to_table::json_to_table; use move_binary_format::CompiledModule; @@ -2976,13 +2977,13 @@ pub(crate) async fn dry_run_or_execute_or_serialize( )) } else { let transaction = Transaction::new(sender_signed_data); - let response: iota_json_rpc_types::IotaTransactionBlockResponse = + let response = client .quorum_driver_api() .execute_transaction_block( transaction, opts_from_cli(opts.emit), - Some(iota_types::quorum_driver_types::ExecuteTransactionRequestType::WaitForLocalExecution), + Some(ExecuteTransactionRequestType::WaitForLocalExecution), ) .await?; From 3d1fd705e821bac592af6d48351590354a26a715 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Thu, 10 Oct 2024 13:38:37 +0200 Subject: [PATCH 05/12] appease dprint Signed-off-by: salaheldinsoliman --- crates/iota/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iota/Cargo.toml b/crates/iota/Cargo.toml index 42e810f2b56..3836f8c05d0 100644 --- a/crates/iota/Cargo.toml +++ b/crates/iota/Cargo.toml @@ -40,8 +40,8 @@ serde.workspace = true serde_json.workspace = true serde_yaml.workspace = true signature.workspace = true -strum_macros.workspace = true strum.workspace = true +strum_macros.workspace = true tabled.workspace = true tap.workspace = true thiserror.workspace = true From 0225d7308b1c50fd61d8008435f75cc1486c0b6e Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Tue, 22 Oct 2024 13:21:08 +0200 Subject: [PATCH 06/12] remove strum_macros from Cargo.toml Signed-off-by: salaheldinsoliman --- Cargo.lock | 1 - crates/iota/Cargo.toml | 1 - crates/iota/src/client_commands.rs | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1af62198503..86c45a4df06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5709,7 +5709,6 @@ dependencies = [ "shlex", "signature 1.6.4", "strum 0.26.3", - "strum_macros 0.26.4", "tabled", "tap", "telemetry-subscribers", diff --git a/crates/iota/Cargo.toml b/crates/iota/Cargo.toml index 3836f8c05d0..be58619b49e 100644 --- a/crates/iota/Cargo.toml +++ b/crates/iota/Cargo.toml @@ -41,7 +41,6 @@ serde_json.workspace = true serde_yaml.workspace = true signature.workspace = true strum.workspace = true -strum_macros.workspace = true tabled.workspace = true tap.workspace = true thiserror.workspace = true diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index 7344c787f3c..f7b5bbb6efa 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -84,7 +84,7 @@ use tabled::{ }, }; use tracing::info; -use strum_macros::EnumString; +use strum::EnumString; use crate::{ clever_error_rendering::render_clever_error_opt, From 948e74c142fc4bcefa78bda696bc094095a3af73 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Wed, 23 Oct 2024 13:40:00 +0200 Subject: [PATCH 07/12] use HashSet instead of Vec Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 23 ++++++++++++++--------- crates/iota/src/client_ptb/ptb.rs | 4 +++- crates/iota/tests/cli_tests.rs | 18 +++++++++--------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index f7b5bbb6efa..d0b242aed3c 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -3,12 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - collections::{BTreeMap, btree_map::Entry}, + collections::{BTreeMap, btree_map::Entry, HashSet}, fmt::{Debug, Display, Formatter, Write}, fs, path::{Path, PathBuf}, str::FromStr, sync::Arc, + cmp::Eq }; use anyhow::{Context, anyhow, bail, ensure}; @@ -656,8 +657,8 @@ pub struct Opts { /// If not provided, all fields are displayed. /// The fields are: effects, input, events, object_changes, /// balance_changes. - #[clap(long, required = false, value_delimiter = ',', num_args = 0..)] - pub emit: Vec, + #[clap(long, required = false, value_delimiter = ',', num_args = 0.., value_parser = parse_emit_option)] + pub emit: HashSet, } /// Global options with gas @@ -681,7 +682,7 @@ impl Opts { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, - emit: vec![], + emit: HashSet::new(), } } /// Uses the passed gas_budget for the gas budget variable, sets dry run to @@ -692,13 +693,13 @@ impl Opts { dry_run: true, serialize_unsigned_transaction: false, serialize_signed_transaction: false, - emit: vec![], + emit: HashSet::new(), } } /// Uses the passed gas_budget for the gas budget variable, sets dry run to /// false, and sets all other flags to false, and emit to the passed emit vector. - pub fn for_testing_emit_options(gas_budget: u64, emit: Vec) -> Self { + pub fn for_testing_emit_options(gas_budget: u64, emit: HashSet) -> Self { Self { gas_budget: Some(gas_budget), dry_run: false, @@ -733,7 +734,7 @@ impl OptsWithGas { pub fn for_testing_emit_options( gas: Option, gas_budget: u64, - emit: Vec, + emit: HashSet, ) -> Self { Self { gas, @@ -742,7 +743,7 @@ impl OptsWithGas { } } -#[derive(Clone, Debug, EnumString)] +#[derive(Clone, Debug, EnumString, Hash, Eq, PartialEq)] #[strum(serialize_all = "snake_case")] pub enum EmitOption { Effects, @@ -3008,7 +3009,7 @@ pub(crate) async fn prerender_clever_errors( } } -fn opts_from_cli(opts: Vec) -> IotaTransactionBlockResponseOptions { +fn opts_from_cli(opts: HashSet) -> IotaTransactionBlockResponseOptions { if opts.is_empty() { return IotaTransactionBlockResponseOptions::new() .with_effects() @@ -3030,3 +3031,7 @@ fn opts_from_cli(opts: Vec) -> IotaTransactionBlockResponseOptions { } options } + +fn parse_emit_option(s: &str) -> Result { + EmitOption::from_str(s).map_err(|_| format!("Invalid emit option: {}", s)) +} diff --git a/crates/iota/src/client_ptb/ptb.rs b/crates/iota/src/client_ptb/ptb.rs index 3276bfddc5b..b8c87ba9796 100644 --- a/crates/iota/src/client_ptb/ptb.rs +++ b/crates/iota/src/client_ptb/ptb.rs @@ -2,6 +2,8 @@ // Modifications Copyright (c) 2024 IOTA Stiftung // SPDX-License-Identifier: Apache-2.0 +use std::collections::HashSet; + use anyhow::{Error, anyhow, ensure}; use clap::{Args, ValueHint, arg}; use iota_json_rpc_types::{IotaExecutionStatus, IotaTransactionBlockEffectsAPI}; @@ -154,7 +156,7 @@ impl PTB { gas_budget: program_metadata.gas_budget.map(|x| x.value), serialize_unsigned_transaction: program_metadata.serialize_unsigned_set, serialize_signed_transaction: program_metadata.serialize_signed_set, - emit: Vec::new(), + emit: HashSet::new(), }, }; diff --git a/crates/iota/tests/cli_tests.rs b/crates/iota/tests/cli_tests.rs index 47c399ebe63..da033ce1e69 100644 --- a/crates/iota/tests/cli_tests.rs +++ b/crates/iota/tests/cli_tests.rs @@ -9,7 +9,7 @@ use std::os::windows::fs::FileExt; #[cfg(not(msim))] use std::str::FromStr; use std::{ - collections::BTreeSet, env, fmt::Write, fs::read_dir, io::Read, net::SocketAddr, path::PathBuf, + collections::{BTreeSet, HashSet}, env, fmt::Write, fs::read_dir, io::Read, net::SocketAddr, path::PathBuf, str, thread, time::Duration, }; @@ -2970,7 +2970,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: true, serialize_signed_transaction: false, - emit: Vec::new(), + emit: HashSet::new(), }, } .execute(context) @@ -2985,7 +2985,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: true, - emit: Vec::new(), + emit: HashSet::new(), }, } .execute(context) @@ -3001,7 +3001,7 @@ async fn test_serialize_tx() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: true, - emit: Vec::new(), + emit: HashSet::new(), }, } .execute(context) @@ -3853,7 +3853,7 @@ async fn test_gas_estimation() -> Result<(), anyhow::Error> { dry_run: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, - emit: Vec::new(), + emit: HashSet::new(), }, } .execute(context) @@ -4241,7 +4241,7 @@ async fn test_call_command_emit_args() -> Result<(), anyhow::Error> { opts: OptsWithGas::for_testing_emit_options( None, rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, - vec![EmitOption::BalanceChanges], + HashSet::from([EmitOption::BalanceChanges]), ), } .execute(context) @@ -4271,11 +4271,11 @@ async fn test_call_command_emit_args() -> Result<(), anyhow::Error> { opts: OptsWithGas::for_testing_emit_options( None, rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, - vec![ + HashSet::from([ EmitOption::BalanceChanges, EmitOption::Effects, EmitOption::ObjectChanges, - ], + ]), ), } .execute(context) @@ -4307,7 +4307,7 @@ async fn test_call_command_emit_args() -> Result<(), anyhow::Error> { opts: OptsWithGas::for_testing_emit_options( None, rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH, - vec![], + HashSet::new(), ), } .execute(context) From 217d1c79f683ecd8722843f47d6205edb963b533 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Tue, 29 Oct 2024 11:47:43 +0100 Subject: [PATCH 08/12] refactor opts_from_cli Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index d0b242aed3c..5d8ab7f4e1c 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -3017,19 +3017,17 @@ fn opts_from_cli(opts: HashSet) -> IotaTransactionBlockResponseOptio .with_events() .with_object_changes() .with_balance_changes(); + } else { + IotaTransactionBlockResponseOptions { + show_input: opts.contains(&EmitOption::Input), + show_events: opts.contains(&EmitOption::Events), + show_object_changes: opts.contains(&EmitOption::ObjectChanges), + show_balance_changes: opts.contains(&EmitOption::BalanceChanges), + show_effects: opts.contains(&EmitOption::Effects), + show_raw_effects: false, + show_raw_input: false, + } } - - let mut options = IotaTransactionBlockResponseOptions::new(); - for opt in opts { - match opt { - EmitOption::Input => options.show_input = true, - EmitOption::Events => options.show_events = true, - EmitOption::ObjectChanges => options.show_object_changes = true, - EmitOption::BalanceChanges => options.show_balance_changes = true, - EmitOption::Effects => options.show_effects = true, - } - } - options } fn parse_emit_option(s: &str) -> Result { From 84719d65e13dc3b0adb7b2d75a4b4c6a36643cbb Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Mon, 4 Nov 2024 15:54:58 +0100 Subject: [PATCH 09/12] apply fmt Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 40 ++++++++++++++++-------------- crates/iota/tests/cli_tests.rs | 15 ++++++++--- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index 5d8ab7f4e1c..a80fb49c46a 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -3,13 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - collections::{BTreeMap, btree_map::Entry, HashSet}, + cmp::Eq, + collections::{BTreeMap, HashSet, btree_map::Entry}, fmt::{Debug, Display, Formatter, Write}, fs, path::{Path, PathBuf}, str::FromStr, sync::Arc, - cmp::Eq }; use anyhow::{Context, anyhow, bail, ensure}; @@ -58,11 +58,11 @@ use iota_types::{ move_package::UpgradeCap, object::Owner, parse_iota_type_tag, + quorum_driver_types::ExecuteTransactionRequestType, signature::GenericSignature, transaction::{ SenderSignedData, Transaction, TransactionData, TransactionDataAPI, TransactionKind, }, - quorum_driver_types::ExecuteTransactionRequestType }; use json_to_table::json_to_table; use move_binary_format::CompiledModule; @@ -74,6 +74,7 @@ use reqwest::StatusCode; use serde::Serialize; use serde_json::{Value, json}; use shared_crypto::intent::Intent; +use strum::EnumString; use tabled::{ builder::Builder as TableBuilder, settings::{ @@ -85,7 +86,6 @@ use tabled::{ }, }; use tracing::info; -use strum::EnumString; use crate::{ clever_error_rendering::render_clever_error_opt, @@ -675,7 +675,8 @@ pub struct OptsWithGas { impl Opts { /// Uses the passed gas_budget for the gas budget variable and sets all - /// other flags to false, and emit to an empty vector(defaulting to all emit options). + /// other flags to false, and emit to an empty vector(defaulting to all emit + /// options). pub fn for_testing(gas_budget: u64) -> Self { Self { gas_budget: Some(gas_budget), @@ -686,7 +687,8 @@ impl Opts { } } /// Uses the passed gas_budget for the gas budget variable, sets dry run to - /// true, and sets all other flags to false, and emit to an empty vector(defaulting to all emit options). + /// true, and sets all other flags to false, and emit to an empty + /// vector(defaulting to all emit options). pub fn for_testing_dry_run(gas_budget: u64) -> Self { Self { gas_budget: Some(gas_budget), @@ -698,7 +700,8 @@ impl Opts { } /// Uses the passed gas_budget for the gas budget variable, sets dry run to - /// false, and sets all other flags to false, and emit to the passed emit vector. + /// false, and sets all other flags to false, and emit to the passed emit + /// vector. pub fn for_testing_emit_options(gas_budget: u64, emit: HashSet) -> Self { Self { gas_budget: Some(gas_budget), @@ -729,8 +732,8 @@ impl OptsWithGas { } /// Sets the gas object to gas, and uses the passed gas_budget for the gas - /// budget variable. Dry run is set to false, and emit to the passed emit vector. - /// All other flags are set to false. + /// budget variable. Dry run is set to false, and emit to the passed emit + /// vector. All other flags are set to false. pub fn for_testing_emit_options( gas: Option, gas_budget: u64, @@ -2978,15 +2981,14 @@ pub(crate) async fn dry_run_or_execute_or_serialize( )) } else { let transaction = Transaction::new(sender_signed_data); - let response = - client - .quorum_driver_api() - .execute_transaction_block( - transaction, - opts_from_cli(opts.emit), - Some(ExecuteTransactionRequestType::WaitForLocalExecution), - ) - .await?; + let response = client + .quorum_driver_api() + .execute_transaction_block( + transaction, + opts_from_cli(opts.emit), + Some(ExecuteTransactionRequestType::WaitForLocalExecution), + ) + .await?; let errors: &Vec = response.errors.as_ref(); if !errors.is_empty() { @@ -3026,7 +3028,7 @@ fn opts_from_cli(opts: HashSet) -> IotaTransactionBlockResponseOptio show_effects: opts.contains(&EmitOption::Effects), show_raw_effects: false, show_raw_input: false, - } + } } } diff --git a/crates/iota/tests/cli_tests.rs b/crates/iota/tests/cli_tests.rs index 65e2f574df4..4b288987270 100644 --- a/crates/iota/tests/cli_tests.rs +++ b/crates/iota/tests/cli_tests.rs @@ -9,8 +9,15 @@ use std::os::windows::fs::FileExt; #[cfg(not(msim))] use std::str::FromStr; use std::{ - collections::{BTreeSet, HashSet}, env, fmt::Write, fs::read_dir, io::Read, net::SocketAddr, path::PathBuf, - str, thread, time::Duration, + collections::{BTreeSet, HashSet}, + env, + fmt::Write, + fs::read_dir, + io::Read, + net::SocketAddr, + path::PathBuf, + str, thread, + time::Duration, }; use expect_test::expect; @@ -18,8 +25,8 @@ use expect_test::expect; use iota::iota_commands::IndexerFeatureArgs; use iota::{ client_commands::{ - IotaClientCommandResult, IotaClientCommands, Opts, OptsWithGas, SwitchResponse, - estimate_gas_budget, EmitOption + EmitOption, IotaClientCommandResult, IotaClientCommands, Opts, OptsWithGas, SwitchResponse, + estimate_gas_budget, }, client_ptb::ptb::PTB, iota_commands::{IotaCommand, parse_host_port}, From 9fa36f4f1a4402f2a917bc2a9df118311009d247 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Mon, 4 Nov 2024 16:23:43 +0100 Subject: [PATCH 10/12] appease clippy Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index a80fb49c46a..ec48459994a 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -3013,7 +3013,7 @@ pub(crate) async fn prerender_clever_errors( fn opts_from_cli(opts: HashSet) -> IotaTransactionBlockResponseOptions { if opts.is_empty() { - return IotaTransactionBlockResponseOptions::new() + IotaTransactionBlockResponseOptions::new() .with_effects() .with_input() .with_events() From dccb59173b4509eeaa2b4cdcea2ee9a1fabfee3d Mon Sep 17 00:00:00 2001 From: salaheldinsoliman Date: Tue, 5 Nov 2024 13:03:21 +0100 Subject: [PATCH 11/12] appease clippy Signed-off-by: salaheldinsoliman --- crates/iota/src/client_commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index ec48459994a..d1d77deccfd 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -3018,7 +3018,7 @@ fn opts_from_cli(opts: HashSet) -> IotaTransactionBlockResponseOptio .with_input() .with_events() .with_object_changes() - .with_balance_changes(); + .with_balance_changes() } else { IotaTransactionBlockResponseOptions { show_input: opts.contains(&EmitOption::Input), From a3a0f5216c37d79f43b5a3240dd96429ed036878 Mon Sep 17 00:00:00 2001 From: salaheldinsoliman <49910731+salaheldinsoliman@users.noreply.github.com> Date: Fri, 8 Nov 2024 13:25:21 +0100 Subject: [PATCH 12/12] Apply suggestions from code review Co-authored-by: Thoralf-M <46689931+Thoralf-M@users.noreply.github.com> --- crates/iota/src/client_commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iota/src/client_commands.rs b/crates/iota/src/client_commands.rs index d1d77deccfd..f0dfd52b394 100644 --- a/crates/iota/src/client_commands.rs +++ b/crates/iota/src/client_commands.rs @@ -2992,7 +2992,7 @@ pub(crate) async fn dry_run_or_execute_or_serialize( let errors: &Vec = response.errors.as_ref(); if !errors.is_empty() { - return Err(anyhow!("Error executing transaction: {:#?}", errors)); + return Err(anyhow!("Error executing transaction: {errors:#?}")); } Ok(IotaClientCommandResult::TransactionBlock(response)) }