Revert "[cli] Switch to WaitForEffectsCert for CLI (#19375)" (#19423)

## Description This reverts PR #19375. Unclear how to handle `waitForLocalExecution` in the CLI for now. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
MystenLabs · Sep 17, 2024 · 94d9e01 · 94d9e01
1 parent 5563665
commit 94d9e01
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 101 deletions.
diff --git a/crates/sui-sdk/src/lib.rs b/crates/sui-sdk/src/lib.rs
@@ -91,16 +91,14 @@ use sui_json_rpc_api::{
 pub use sui_json_rpc_types as rpc_types;
 use sui_json_rpc_types::{
     ObjectsPage, SuiObjectDataFilter, SuiObjectDataOptions, SuiObjectResponse,
-    SuiObjectResponseQuery, SuiTransactionBlockResponse,
+    SuiObjectResponseQuery,
 };
 use sui_transaction_builder::{DataReader, TransactionBuilder};
 pub use sui_types as types;
 use sui_types::base_types::{ObjectID, ObjectInfo, SuiAddress};
-use types::transaction::Transaction;
 
 use crate::apis::{CoinReadApi, EventApi, GovernanceApi, QuorumDriverApi, ReadApi};
 use crate::error::{Error, SuiRpcResult};
-use crate::rpc_types::SuiTransactionBlockResponseOptions;
 
 pub mod apis;
 pub mod error;
@@ -115,10 +113,6 @@ pub const SUI_LOCAL_NETWORK_GAS_URL: &str = "http://127.0.0.1:5003/gas";
 pub const SUI_DEVNET_URL: &str = "https://fullnode.devnet.sui.io:443";
 pub const SUI_TESTNET_URL: &str = "https://fullnode.testnet.sui.io:443";
 
-const WAIT_FOR_LOCAL_EXECUTION_TIMEOUT: Duration = Duration::from_secs(90);
-const WAIT_FOR_LOCAL_EXECUTION_DELAY: Duration = Duration::from_millis(200);
-const WAIT_FOR_LOCAL_EXECUTION_INTERVAL: Duration = Duration::from_secs(2);
-
 /// A Sui client builder for connecting to the Sui network
 ///
 /// By default the `maximum concurrent requests` is set to 256 and
@@ -538,58 +532,6 @@ impl SuiClient {
     pub fn ws(&self) -> Option<&WsClient> {
         self.api.ws.as_ref()
     }
-
-    /// Execute a transaction and wait for the effects cert.
-    /// The transaction execution is not guaranteed to succeed and may fail. This is usually only
-    /// needed in non-test environment or the caller is explicitly testing some failure behavior.
-    ///
-    // Used mostly in the CLI as it needs to wait for the effects cert instead of local execution.
-    // Use the `execute_transaction_may_fail` from WalletContext
-    // for most cases where read-after write consistency is needed.
-    pub async fn execute_tx(
-        &self,
-        tx: Transaction,
-    ) -> Result<SuiTransactionBlockResponse, anyhow::Error> {
-        let tx_clone = tx.clone();
-        let resp = self
-        .quorum_driver_api()
-        .execute_transaction_block(
-            tx,
-            SuiTransactionBlockResponseOptions::new()
-                .with_effects()
-                .with_input()
-                .with_events()
-                .with_object_changes()
-                .with_balance_changes(),
-            Some(sui_types::quorum_driver_types::ExecuteTransactionRequestType::WaitForEffectsCert),
-        )
-        .await?;
-
-        // poll for tx to simulate wait for local execution
-        tokio::time::timeout(WAIT_FOR_LOCAL_EXECUTION_TIMEOUT, async {
-            // Apply a short delay to give the full node a chance to catch up.
-            tokio::time::sleep(WAIT_FOR_LOCAL_EXECUTION_DELAY).await;
-
-            let mut interval = tokio::time::interval(WAIT_FOR_LOCAL_EXECUTION_INTERVAL);
-            loop {
-                interval.tick().await;
-
-                if let Ok(poll_response) = self
-                    .read_api()
-                    .get_transaction_with_options(
-                        *tx_clone.digest(),
-                        SuiTransactionBlockResponseOptions::default(),
-                    )
-                    .await
-                {
-                    break poll_response;
-                }
-            }
-        })
-        .await?;
-
-        Ok(resp)
-    }
 }
 
 #[async_trait]

diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs
@@ -1554,8 +1554,7 @@ impl SuiClientCommands {
                 }
                 let transaction = Transaction::from_generic_sig_data(data, sigs);
 
-                let client = context.get_client().await?;
-                let response = client.execute_tx(transaction).await?;
+                let response = context.execute_transaction_may_fail(transaction).await?;
                 SuiClientCommandResult::TransactionBlock(response)
             }
             SuiClientCommands::ExecuteCombinedSignedTx { signed_tx_bytes } => {
@@ -1566,8 +1565,7 @@ impl SuiClientCommands {
                         .map_err(|_| anyhow!("Invalid Base64 encoding"))?
                 ).map_err(|_| anyhow!("Failed to parse SenderSignedData bytes, check if it matches the output of sui client commands with --serialize-signed-transaction"))?;
                 let transaction = Envelope::<SenderSignedData, EmptySignInfo>::new(data);
-                let client = context.get_client().await?;
-                let response = client.execute_tx(transaction).await?;
+                let response = context.execute_transaction_may_fail(transaction).await?;
                 SuiClientCommandResult::TransactionBlock(response)
             }
             SuiClientCommands::NewEnv {
@@ -2870,9 +2868,11 @@ pub(crate) async fn dry_run_or_execute_or_serialize(
             ))
         } else {
             let transaction = Transaction::new(sender_signed_data);
-            debug!("Executing transaction: {:?}", &transaction);
-            let mut response = client.execute_tx(transaction.clone()).await?;
-            debug!("Transaction executed: {:?}", &transaction);
+            debug!("Executing transaction: {:?}", transaction);
+            let mut response = context
+                .execute_transaction_may_fail(transaction.clone())
+                .await?;
+            debug!("Transaction executed: {:?}", transaction);
             if let Some(effects) = response.effects.as_mut() {
                 prerender_clever_errors(effects, client.read_api()).await;
             }

diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs
@@ -252,11 +252,11 @@ async fn test_ptb_publish_and_complex_arg_resolution() -> Result<(), anyhow::Err
     // Print it out to CLI/logs
     resp.print(true);
 
-    let SuiClientCommandResult::TransactionBlock(ref response) = resp else {
+    let SuiClientCommandResult::TransactionBlock(response) = resp else {
         unreachable!("Invalid response");
     };
 
-    let SuiTransactionBlockEffects::V1(effects) = response.effects.as_ref().unwrap();
+    let SuiTransactionBlockEffects::V1(effects) = response.effects.unwrap();
 
     assert!(effects.status.is_ok());
     assert_eq!(effects.gas_object().object_id(), gas_obj_id);
@@ -1072,12 +1072,12 @@ async fn test_receive_argument() -> Result<(), anyhow::Error> {
     .execute(context)
     .await?;
 
-    let owned_obj_ids = if let SuiClientCommandResult::TransactionBlock(ref response) = resp {
+    let owned_obj_ids = if let SuiClientCommandResult::TransactionBlock(response) = resp {
         assert_eq!(
             response.effects.as_ref().unwrap().gas_object().object_id(),
             gas_obj_id
         );
-        let x = response.effects.as_ref().unwrap();
+        let x = response.effects.unwrap();
         x.created().to_vec()
     } else {
         unreachable!("Invalid response");
@@ -1232,8 +1232,8 @@ async fn test_receive_argument_by_immut_ref() -> Result<(), anyhow::Error> {
     .await?;
 
     let (parent, child) =
-        if let SuiClientCommandResult::TransactionBlock(ref response) = start_call_result {
-            let created = response.effects.as_ref().unwrap().created().to_vec();
+        if let SuiClientCommandResult::TransactionBlock(response) = start_call_result {
+            let created = response.effects.unwrap().created().to_vec();
             let owners: BTreeSet<ObjectID> = created
                 .iter()
                 .flat_map(|refe| {
@@ -1320,12 +1320,12 @@ async fn test_receive_argument_by_mut_ref() -> Result<(), anyhow::Error> {
     .execute(context)
     .await?;
 
-    let owned_obj_ids = if let SuiClientCommandResult::TransactionBlock(ref response) = resp {
+    let owned_obj_ids = if let SuiClientCommandResult::TransactionBlock(response) = resp {
         assert_eq!(
             response.effects.as_ref().unwrap().gas_object().object_id(),
             gas_obj_id
         );
-        let x = response.effects.as_ref().unwrap();
+        let x = response.effects.unwrap();
         x.created().to_vec()
     } else {
         unreachable!("Invalid response");
@@ -2543,7 +2543,6 @@ async fn test_merge_coin() -> Result<(), anyhow::Error> {
     }
     .execute(context)
     .await?;
-
     let g = if let SuiClientCommandResult::TransactionBlock(r) = resp {
         assert!(r.status_ok().unwrap(), "Command failed: {:?}", r);
         assert_eq!(r.effects.as_ref().unwrap().gas_object().object_id(), gas);
@@ -3633,7 +3632,6 @@ async fn test_transfer() -> Result<(), anyhow::Error> {
     }
     .execute(context)
     .await?;
-
     // transfer command will transfer the object_id1 to address2, and use object_id2 as gas
     // we check if object1 is owned by address 2 and if the gas object used is object_id2
     if let SuiClientCommandResult::TransactionBlock(response) = transfer {
@@ -3670,8 +3668,8 @@ async fn test_transfer_sui() -> Result<(), anyhow::Error> {
     let (mut test_cluster, client, rgp, objects, recipients, addresses) =
         test_cluster_helper().await;
     let object_id1 = objects[0];
-    let address2 = addresses[0];
     let recipient1 = &recipients[0];
+    let address2 = addresses[0];
     let context = &mut test_cluster.wallet;
     let amount = 1000;
     let transfer_sui = SuiClientCommands::TransferSui {
@@ -3714,7 +3712,6 @@ async fn test_transfer_sui() -> Result<(), anyhow::Error> {
     } else {
         panic!("TransferSui test failed");
     }
-
     // transfer the whole object by not passing an amount
     let transfer_sui = SuiClientCommands::TransferSui {
         to: recipient1.clone(),
@@ -3724,7 +3721,6 @@ async fn test_transfer_sui() -> Result<(), anyhow::Error> {
     }
     .execute(context)
     .await?;
-
     if let SuiClientCommandResult::TransactionBlock(response) = transfer_sui {
         assert!(response.status_ok().unwrap());
         assert_eq!(
@@ -3785,7 +3781,6 @@ async fn test_gas_estimation() -> Result<(), anyhow::Error> {
     .execute(context)
     .await
     .unwrap();
-
     if let SuiClientCommandResult::TransactionBlock(response) = transfer_sui_cmd {
         assert!(response.status_ok().unwrap());
         let gas_used = response.effects.as_ref().unwrap().gas_object().object_id();

diff --git a/crates/sui/tests/snapshots/cli_tests__clever_errors.snap b/crates/sui/tests/snapshots/cli_tests__clever_errors.snap