From a0cc1c53e1be3f2df7be43d5c3ccfcb5c94c09a8 Mon Sep 17 00:00:00 2001 From: Lu Zhang <8418040+longbowlu@users.noreply.github.com> Date: Tue, 16 Aug 2022 17:07:33 -0700 Subject: [PATCH] [Onboard quorum driver to fullnode] 3/n add cluster test case (#3867) * add execute transaction end points * add execute txn endpoint * add cluster test * rebase * get gas objs * fmt --- Cargo.lock | 4 +- crates/sui-cluster-test/src/lib.rs | 9 ++ crates/sui-cluster-test/src/test_case.rs | 1 + .../fullnode_execute_transaction_test.rs | 128 ++++++++++++++++++ crates/sui-faucet/src/faucet/simple_faucet.rs | 3 +- crates/sui-sdk/src/lib.rs | 42 +++++- crates/sui/src/client_commands.rs | 8 +- crates/test-utils/src/messages.rs | 24 +++- 8 files changed, 204 insertions(+), 15 deletions(-) create mode 100644 crates/sui-cluster-test/src/test_case/fullnode_execute_transaction_test.rs diff --git a/Cargo.lock b/Cargo.lock index 89d012342aad0..7ab5d9549c02c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -54,9 +54,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.59" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c91f1f46651137be86f3a2b9a8359f9ab421d04d941c62b5982e1ca21113adf9" +checksum = "bb07d2053ccdbe10e2af2995a2f116c1330396493dc1269f6a91d0ae82e19704" dependencies = [ "backtrace", ] diff --git a/crates/sui-cluster-test/src/lib.rs b/crates/sui-cluster-test/src/lib.rs index e270777a86ab5..92c42a15e4352 100644 --- a/crates/sui-cluster-test/src/lib.rs +++ b/crates/sui-cluster-test/src/lib.rs @@ -8,6 +8,7 @@ use config::ClusterTestOpt; use std::sync::Arc; use sui::client_commands::WalletContext; use sui_json_rpc_types::SuiTransactionResponse; +use test_utils::messages::make_transactions_with_wallet_context; use sui_sdk::SuiClient; use sui_types::gas_coin::GasCoin; @@ -17,6 +18,7 @@ use sui_types::{ }; use test_case::{ call_contract_test::CallContractTest, coin_merge_split_test::CoinMergeSplitTest, + fullnode_execute_transaction_test::FullNodeExecuteTransactionTest, native_transfer_test::NativeTransferTest, shared_object_test::SharedCounterTest, }; use tokio::time::{sleep, Duration}; @@ -73,6 +75,12 @@ impl TestContext { self.client.get_wallet_address() } + /// See `make_transactions_with_wallet_context` for potential caveats + /// of this helper function. + pub async fn make_transactions(&mut self, max_txn_num: usize) -> Vec { + make_transactions_with_wallet_context(self.get_wallet_mut(), max_txn_num).await + } + async fn sign_and_execute( &self, txn_data: TransactionData, @@ -156,6 +164,7 @@ impl ClusterTest { TestCase::new(CoinMergeSplitTest {}), TestCase::new(CallContractTest {}), TestCase::new(SharedCounterTest {}), + TestCase::new(FullNodeExecuteTransactionTest {}), ]; // TODO: improve the runner parallelism for efficiency diff --git a/crates/sui-cluster-test/src/test_case.rs b/crates/sui-cluster-test/src/test_case.rs index 4bbbc20a18364..f79a1d6b6525c 100644 --- a/crates/sui-cluster-test/src/test_case.rs +++ b/crates/sui-cluster-test/src/test_case.rs @@ -3,5 +3,6 @@ pub mod call_contract_test; pub mod coin_merge_split_test; +pub mod fullnode_execute_transaction_test; pub mod native_transfer_test; pub mod shared_object_test; diff --git a/crates/sui-cluster-test/src/test_case/fullnode_execute_transaction_test.rs b/crates/sui-cluster-test/src/test_case/fullnode_execute_transaction_test.rs new file mode 100644 index 0000000000000..900d614fdc98a --- /dev/null +++ b/crates/sui-cluster-test/src/test_case/fullnode_execute_transaction_test.rs @@ -0,0 +1,128 @@ +// Copyright (c) 2022, Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use crate::{TestCaseImpl, TestContext}; +use async_trait::async_trait; +use sui_json_rpc_types::{SuiExecuteTransactionResponse, SuiExecutionStatus}; +use sui_types::messages::ExecuteTransactionRequestType; +use tracing::info; + +pub struct FullNodeExecuteTransactionTest; + +#[async_trait] +impl TestCaseImpl for FullNodeExecuteTransactionTest { + fn name(&self) -> &'static str { + "FullNodeExecuteTransaction" + } + + fn description(&self) -> &'static str { + "Test executing transaction via Fullnode Quorum Driver" + } + + async fn run(&self, ctx: &mut TestContext) -> Result<(), anyhow::Error> { + ctx.get_sui_from_faucet(Some(3)).await; + let mut txns = ctx.make_transactions(3).await; + assert!( + txns.len() >= 3, + "Expect at least 3 txns, but only got {}. Do we get enough gas objects from faucet?", + txns.len(), + ); + + let fullnode = ctx.get_fullnode(); + + // Test WaitForEffectsCert + let txn = txns.swap_remove(0); + let txn_digest = *txn.digest(); + + info!("Test execution with ImmediateReturn"); + let response = fullnode + .execute_transaction_by_fullnode( + txn.clone(), + ExecuteTransactionRequestType::ImmediateReturn, + ) + .await?; + if let SuiExecuteTransactionResponse::ImmediateReturn { tx_digest } = response { + assert_eq!(txn_digest, tx_digest); + + // Verify fullnode observes the txn + ctx.let_fullnode_sync().await; + + fullnode + .get_transaction(tx_digest) + .await + .unwrap_or_else(|e| { + panic!( + "Failed get transaction {:?} from fullnode: {:?}", + txn_digest, e + ) + }); + } else { + panic!("Expect ImmediateReturn but got {:?}", response); + } + + info!("Test execution with WaitForTxCert"); + let txn = txns.swap_remove(0); + let txn_digest = *txn.digest(); + let response = fullnode + .execute_transaction_by_fullnode( + txn.clone(), + ExecuteTransactionRequestType::WaitForTxCert, + ) + .await?; + if let SuiExecuteTransactionResponse::TxCert { certificate } = response { + assert_eq!(txn_digest, certificate.transaction_digest); + + // Verify fullnode observes the txn + ctx.let_fullnode_sync().await; + + fullnode + .get_transaction(txn_digest) + .await + .unwrap_or_else(|e| { + panic!( + "Failed get transaction {:?} from fullnode: {:?}", + txn_digest, e + ) + }); + } else { + panic!("Expect TxCert but got {:?}", response); + } + + info!("Test execution with WaitForEffectsCert"); + let txn = txns.swap_remove(0); + let txn_digest = *txn.digest(); + + let response = fullnode + .execute_transaction_by_fullnode(txn, ExecuteTransactionRequestType::WaitForEffectsCert) + .await?; + if let SuiExecuteTransactionResponse::EffectsCert { + certificate, + effects, + } = response + { + assert_eq!(txn_digest, certificate.transaction_digest); + if !matches!(effects.effects.status, SuiExecutionStatus::Success { .. }) { + panic!( + "Failed to execute transfer tranasction {:?}: {:?}", + txn_digest, effects.effects.status + ) + } + // Verify fullnode observes the txn + ctx.let_fullnode_sync().await; + + fullnode + .get_transaction(txn_digest) + .await + .unwrap_or_else(|e| { + panic!( + "Failed get transaction {:?} from fullnode: {:?}", + txn_digest, e + ) + }); + } else { + panic!("Expect EffectsCert but got {:?}", response); + } + + Ok(()) + } +} diff --git a/crates/sui-faucet/src/faucet/simple_faucet.rs b/crates/sui-faucet/src/faucet/simple_faucet.rs index bb16ea603256c..fa70ce3780218 100644 --- a/crates/sui-faucet/src/faucet/simple_faucet.rs +++ b/crates/sui-faucet/src/faucet/simple_faucet.rs @@ -66,7 +66,6 @@ impl SimpleFaucet { // Ok to unwrap() since `get_gas_objects` guarantees gas .map(|q| GasCoin::try_from(&q.1).unwrap()) .collect::>(); - info!("Coins held: {:?}", coins); let (producer, consumer) = mpsc::channel(coins.len()); for coin in &coins { @@ -75,7 +74,7 @@ impl SimpleFaucet { } } - info!("Using coins: {:?}", coins); + debug!("Using coins: {:?}", coins); let metrics = FaucetMetrics::new(prometheus_registry); diff --git a/crates/sui-sdk/src/lib.rs b/crates/sui-sdk/src/lib.rs index 223832949ec38..a10b672baf0f2 100644 --- a/crates/sui-sdk/src/lib.rs +++ b/crates/sui-sdk/src/lib.rs @@ -7,6 +7,7 @@ use futures_core::Stream; use jsonrpsee::core::client::Subscription; use jsonrpsee::http_client::{HttpClient, HttpClientBuilder}; use jsonrpsee::ws_client::{WsClient, WsClientBuilder}; +use rpc_types::SuiExecuteTransactionResponse; use serde::Deserialize; use serde::Serialize; use std::fmt::Write; @@ -15,6 +16,7 @@ use sui_config::gateway::GatewayConfig; use sui_core::gateway_state::{GatewayClient, GatewayState}; use sui_json::SuiJsonValue; use sui_json_rpc::api::EventStreamingApiClient; +use sui_json_rpc::api::QuorumDriverApiClient; use sui_json_rpc::api::RpcBcsApiClient; use sui_json_rpc::api::RpcFullNodeReadApiClient; use sui_json_rpc::api::RpcGatewayApiClient; @@ -30,6 +32,7 @@ use sui_types::base_types::{ObjectID, SuiAddress, TransactionDigest}; use sui_types::crypto::SignableBytes; use sui_types::messages::{Transaction, TransactionData}; use sui_types::sui_serde::Base64; +use types::messages::ExecuteTransactionRequestType; pub mod crypto; @@ -224,18 +227,53 @@ impl SuiClient { Ok(match &self { Self::Http(c) => { let (tx_bytes, flag, signature, pub_key) = tx.to_network_data_for_execution(); - c.execute_transaction(tx_bytes, flag, signature, pub_key) + RpcGatewayApiClient::execute_transaction(c, tx_bytes, flag, signature, pub_key) .await? } Self::Ws(c) => { let (tx_bytes, flag, signature, pub_key) = tx.to_network_data_for_execution(); - c.execute_transaction(tx_bytes, flag, signature, pub_key) + RpcGatewayApiClient::execute_transaction(c, tx_bytes, flag, signature, pub_key) .await? } Self::Embedded(c) => c.execute_transaction(tx).await?, }) } + pub async fn execute_transaction_by_fullnode( + &self, + tx: Transaction, + request_type: ExecuteTransactionRequestType, + ) -> anyhow::Result { + Ok(match &self { + Self::Http(c) => { + let (tx_bytes, flag, signature, pub_key) = tx.to_network_data_for_execution(); + QuorumDriverApiClient::execute_transaction( + c, + tx_bytes, + flag, + signature, + pub_key, + request_type, + ) + .await? + } + Self::Ws(c) => { + let (tx_bytes, flag, signature, pub_key) = tx.to_network_data_for_execution(); + QuorumDriverApiClient::execute_transaction( + c, + tx_bytes, + flag, + signature, + pub_key, + request_type, + ) + .await? + } + // TODO do we want to support an embedded quorum driver? + Self::Embedded(_c) => unimplemented!(), + }) + } + pub async fn transfer_object( &self, signer: SuiAddress, diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index b057e04ffb6a5..83a3b8a165d44 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -407,7 +407,7 @@ impl SuiClientCommands { .await? .iter() // Ok to unwrap() since `get_gas_objects` guarantees gas - .map(|(_, object)| GasCoin::try_from(object).unwrap()) + .map(|(_val, object, _object_ref)| GasCoin::try_from(object).unwrap()) .collect(); SuiClientCommandResult::Gas(coins) } @@ -561,7 +561,7 @@ impl WalletContext { pub async fn gas_objects( &self, address: SuiAddress, - ) -> Result, anyhow::Error> { + ) -> Result, anyhow::Error> { let object_refs = self.gateway.get_objects_owned_by_address(address).await?; // TODO: We should ideally fetch the objects from local cache @@ -572,7 +572,7 @@ impl WalletContext { if matches!( o.data.type_(), Some(v) if *v == GasCoin::type_().to_string()) { // Okay to unwrap() since we already checked type let gas_coin = GasCoin::try_from(&o)?; - values_objects.push((gas_coin.value(), o)); + values_objects.push((gas_coin.value(), o, oref)); } } _ => continue, @@ -607,7 +607,7 @@ impl WalletContext { ) -> Result<(u64, SuiParsedObject), anyhow::Error> { for o in self.gas_objects(address).await.unwrap() { if o.0 >= budget && !forbidden_gas_objects.contains(&o.1.id()) { - return Ok(o); + return Ok((o.0, o.1)); } } Err(anyhow!( diff --git a/crates/test-utils/src/messages.rs b/crates/test-utils/src/messages.rs index dfb74f4cf4a18..8c2ad6dec20cf 100644 --- a/crates/test-utils/src/messages.rs +++ b/crates/test-utils/src/messages.rs @@ -51,9 +51,22 @@ pub async fn get_account_and_gas_coins( Ok(res) } -/// A helper function to get all accounts and their owned objects +pub async fn get_gas_objects_with_wallet_context( + context: &WalletContext, + address: SuiAddress, +) -> Vec { + context + .gas_objects(address) + .await + .unwrap() + .into_iter() + .map(|(_val, _object, object_ref)| object_ref) + .collect() +} + +/// A helper function to get all accounts and their owned gas objects /// with a WalletContext. -pub async fn get_account_and_objects( +pub async fn get_account_and_gas_objects( context: &WalletContext, ) -> Vec<(SuiAddress, Vec)> { let owned_gas_objects = futures::future::join_all( @@ -61,7 +74,7 @@ pub async fn get_account_and_objects( .keystore .addresses() .iter() - .map(|account| context.gateway.get_objects_owned_by_address(*account)), + .map(|account| get_gas_objects_with_wallet_context(context, *account)), ) .await; context @@ -69,11 +82,12 @@ pub async fn get_account_and_objects( .addresses() .iter() .zip(owned_gas_objects.into_iter()) - .map(|(address, objects)| (*address, objects.unwrap())) + .map(|(address, objects)| (*address, objects)) .collect::>() } /// A helper function to make Transactions with controlled accounts in WalletContext. +/// Particularly, the wallet needs to own gas objects for transactions. /// However, if this function is called multiple times without any "sync" actions /// on gas object management, txns may fail and objects may be locked. /// @@ -86,7 +100,7 @@ pub async fn make_transactions_with_wallet_context( max_txn_num: usize, ) -> Vec { let recipient = get_key_pair::().0; - let accounts_and_objs = get_account_and_objects(context).await; + let accounts_and_objs = get_account_and_gas_objects(context).await; let mut res = Vec::with_capacity(max_txn_num); for (address, objs) in &accounts_and_objs { for obj in objs {