From f3427169837dfa6ac552786e218cfe1a23898a06 Mon Sep 17 00:00:00 2001 From: Xun Li Date: Mon, 18 Mar 2024 21:09:28 -0700 Subject: [PATCH] [bench] Properly sign transactions in single node benchmark (#16519) ## Description When forming a cert, properly sign the transaction on the validator. This will make it possible to measure performance changes with transaction lock cache. ## Test Plan CI --- If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --- .../src/benchmark_context.rs | 27 ++++++- .../sui-single-node-benchmark/src/command.rs | 6 ++ crates/sui-single-node-benchmark/src/lib.rs | 3 +- crates/sui-single-node-benchmark/src/main.rs | 1 + .../tests/smoke_tests.rs | 73 ++++++++++--------- 5 files changed, 72 insertions(+), 38 deletions(-) diff --git a/crates/sui-single-node-benchmark/src/benchmark_context.rs b/crates/sui-single-node-benchmark/src/benchmark_context.rs index 45bd96bc8354f..ad5fe4f3a992f 100644 --- a/crates/sui-single-node-benchmark/src/benchmark_context.rs +++ b/crates/sui-single-node-benchmark/src/benchmark_context.rs @@ -18,7 +18,9 @@ use sui_types::base_types::{ObjectID, ObjectRef, SuiAddress}; use sui_types::effects::{TransactionEffects, TransactionEffectsAPI}; use sui_types::messages_grpc::HandleTransactionResponse; use sui_types::mock_checkpoint_builder::ValidatorKeypairProvider; -use sui_types::transaction::{CertifiedTransaction, SignedTransaction, Transaction}; +use sui_types::transaction::{ + CertifiedTransaction, SignedTransaction, Transaction, VerifiedTransaction, +}; use tracing::info; pub struct BenchmarkContext { @@ -155,6 +157,7 @@ impl BenchmarkContext { pub(crate) async fn certify_transactions( &self, transactions: Vec, + skip_signing: bool, ) -> Vec { info!("Creating transaction certificates"); let tasks: FuturesUnordered<_> = transactions @@ -163,8 +166,23 @@ impl BenchmarkContext { let validator = self.validator(); tokio::spawn(async move { let committee = validator.get_committee(); - let validator = validator.get_validator(); - let sig = SignedTransaction::sign(0, &tx, &*validator.secret, validator.name); + let validator_state = validator.get_validator(); + let sig = if skip_signing { + SignedTransaction::sign( + 0, + &tx, + &*validator_state.secret, + validator_state.name, + ) + } else { + let verified_tx = VerifiedTransaction::new_unchecked(tx.clone()); + validator_state + .handle_transaction(validator.get_epoch_store(), verified_tx) + .await + .unwrap() + .status + .into_signed_for_testing() + }; CertifiedTransaction::new(tx.into_data(), vec![sig], committee).unwrap() }) }) @@ -177,8 +195,9 @@ impl BenchmarkContext { &self, transactions: Vec, print_sample_tx: bool, + skip_signing: bool, ) { - let mut transactions = self.certify_transactions(transactions).await; + let mut transactions = self.certify_transactions(transactions, skip_signing).await; if print_sample_tx { self.execute_sample_transaction(transactions.pop().unwrap().into_unsigned()) .await; diff --git a/crates/sui-single-node-benchmark/src/command.rs b/crates/sui-single-node-benchmark/src/command.rs index f3258b456faa2..bc06786638b72 100644 --- a/crates/sui-single-node-benchmark/src/command.rs +++ b/crates/sui-single-node-benchmark/src/command.rs @@ -31,6 +31,12 @@ pub struct Command { help = "Whether to print out a sample transaction and effects that is going to be benchmarked on" )] pub print_sample_tx: bool, + #[arg( + long, + default_value_t = false, + help = "If true, skip signing on the validators, instead, creating certificates directly using validator secrets" + )] + pub skip_signing: bool, #[arg( long, default_value = "baseline", diff --git a/crates/sui-single-node-benchmark/src/lib.rs b/crates/sui-single-node-benchmark/src/lib.rs index ba85b174cea2b..30b5a69dab688 100644 --- a/crates/sui-single-node-benchmark/src/lib.rs +++ b/crates/sui-single-node-benchmark/src/lib.rs @@ -23,6 +23,7 @@ pub async fn run_benchmark( component: Component, checkpoint_size: usize, print_sample_tx: bool, + skip_signing: bool, ) { let mut ctx = BenchmarkContext::new( workload.clone(), @@ -47,7 +48,7 @@ pub async fn run_benchmark( .await; } _ => { - ctx.benchmark_transaction_execution(transactions, print_sample_tx) + ctx.benchmark_transaction_execution(transactions, print_sample_tx, skip_signing) .await; } } diff --git a/crates/sui-single-node-benchmark/src/main.rs b/crates/sui-single-node-benchmark/src/main.rs index 8060d121e8c84..600c0fe0975c6 100644 --- a/crates/sui-single-node-benchmark/src/main.rs +++ b/crates/sui-single-node-benchmark/src/main.rs @@ -19,6 +19,7 @@ async fn main() { args.component, args.checkpoint_size, args.print_sample_tx, + args.skip_signing, ) .await; diff --git a/crates/sui-single-node-benchmark/tests/smoke_tests.rs b/crates/sui-single-node-benchmark/tests/smoke_tests.rs index 57de6fe114194..a5d512ae7364e 100644 --- a/crates/sui-single-node-benchmark/tests/smoke_tests.rs +++ b/crates/sui-single-node-benchmark/tests/smoke_tests.rs @@ -10,44 +10,49 @@ use sui_single_node_benchmark::workload::Workload; #[sim_test] async fn benchmark_non_move_transactions_smoke_test() { - for component in Component::iter() { - run_benchmark( - Workload::new( - 10, - WorkloadKind::PTB { - num_transfers: 2, - use_native_transfer: true, - num_dynamic_fields: 0, - computation: 0, - }, - ), - component, - 1000, - false, - ) - .await; + for skip_signing in [true, false] { + for component in Component::iter() { + run_benchmark( + Workload::new( + 10, + WorkloadKind::PTB { + num_transfers: 2, + use_native_transfer: true, + num_dynamic_fields: 0, + computation: 0, + }, + ), + component, + 1000, + false, + skip_signing, + ) + .await; + } } } #[sim_test] async fn benchmark_move_transactions_smoke_test() { - // This test makes sure that the benchmark runs. - for component in Component::iter() { - run_benchmark( - Workload::new( - 10, - WorkloadKind::PTB { - num_transfers: 2, - use_native_transfer: false, - num_dynamic_fields: 1, - computation: 1, - }, - ), - component, - 1000, - false, - ) - .await; + for skip_signing in [true, false] { + for component in Component::iter() { + run_benchmark( + Workload::new( + 10, + WorkloadKind::PTB { + num_transfers: 2, + use_native_transfer: false, + num_dynamic_fields: 1, + computation: 1, + }, + ), + component, + 1000, + false, + skip_signing, + ) + .await; + } } } @@ -72,6 +77,7 @@ async fn benchmark_publish_from_source() { component, 1000, false, + false, ) .await; } @@ -98,6 +104,7 @@ async fn benchmark_publish_from_bytecode() { component, 1000, false, + false, ) .await; }