From 40dc32c11e6e226887013ffeebbd185b155c4cb2 Mon Sep 17 00:00:00 2001 From: danielx <66756900+danielxiangzl@users.noreply.github.com> Date: Wed, 31 May 2023 17:34:14 -0700 Subject: [PATCH] [Executor] Move block gas limit to state computer (#8441) [Executor] moving block gas limit to state computer, refactoring and fixing tests --- api/test-context/src/test_context.rs | 2 +- aptos-move/aptos-debugger/src/lib.rs | 2 +- .../aptos-transaction-benchmarks/src/main.rs | 10 +- .../src/transactions.rs | 36 ++--- .../src/aptos_test_harness.rs | 2 +- .../src/bins/run_aptos_p2p.rs | 2 +- aptos-move/aptos-vm/src/aptos_vm.rs | 39 +---- aptos-move/aptos-vm/src/block_executor/mod.rs | 4 +- aptos-move/aptos-vm/src/lib.rs | 10 +- .../sharded_block_executor/executor_shard.rs | 6 +- .../src/sharded_block_executor/mod.rs | 13 +- aptos-move/block-executor/src/executor.rs | 14 +- .../src/proptest_types/tests.rs | 104 +++++++------ .../src/proptest_types/types.rs | 4 +- aptos-move/e2e-tests/src/executor.rs | 2 +- consensus/src/state_computer.rs | 25 ++-- .../executor-benchmark/src/native_executor.rs | 14 +- .../src/transaction_executor.rs | 2 +- .../src/integration_test_impl.rs | 20 ++- execution/executor-types/src/lib.rs | 5 +- execution/executor/src/block_executor.rs | 69 ++------- execution/executor/src/chunk_executor.rs | 6 +- .../executor/src/components/chunk_output.rs | 69 +++------ execution/executor/src/db_bootstrapper.rs | 9 +- execution/executor/src/fuzzing.rs | 26 +--- .../executor/src/mock_vm/mock_vm_test.rs | 6 +- execution/executor/src/mock_vm/mod.rs | 23 +-- .../src/tests/chunk_executor_tests.rs | 11 +- execution/executor/src/tests/mod.rs | 139 +++++++++--------- .../executor/tests/db_bootstrapper_test.rs | 5 +- .../tests/storage_integration_test.rs | 7 +- .../test_helpers/transaction_test_helpers.rs | 11 +- 32 files changed, 289 insertions(+), 408 deletions(-) diff --git a/api/test-context/src/test_context.rs b/api/test-context/src/test_context.rs index 49accba9b661b..e1f14dd1796d6 100644 --- a/api/test-context/src/test_context.rs +++ b/api/test-context/src/test_context.rs @@ -604,7 +604,7 @@ impl TestContext { let parent_id = self.executor.committed_block_id(); let result = self .executor - .execute_block((metadata.id(), txns.clone()), parent_id) + .execute_block((metadata.id(), txns.clone()), parent_id, None) .unwrap(); let mut compute_status = result.compute_status().clone(); assert_eq!(compute_status.len(), txns.len(), "{:?}", result); diff --git a/aptos-move/aptos-debugger/src/lib.rs b/aptos-move/aptos-debugger/src/lib.rs index eed8ee3b9b064..bf34e3043917d 100644 --- a/aptos-move/aptos-debugger/src/lib.rs +++ b/aptos-move/aptos-debugger/src/lib.rs @@ -58,7 +58,7 @@ impl AptosDebugger { txns: Vec, ) -> Result> { let state_view = DebuggerStateView::new(self.debugger.clone(), version); - AptosVM::execute_block(txns, &state_view) + AptosVM::execute_block(txns, &state_view, None) .map_err(|err| format_err!("Unexpected VM Error: {:?}", err)) } diff --git a/aptos-move/aptos-transaction-benchmarks/src/main.rs b/aptos-move/aptos-transaction-benchmarks/src/main.rs index e710dd0c604d5..022493cac8535 100755 --- a/aptos-move/aptos-transaction-benchmarks/src/main.rs +++ b/aptos-move/aptos-transaction-benchmarks/src/main.rs @@ -49,7 +49,7 @@ struct ParamSweepOpt { pub num_runs: usize, #[clap(long)] - pub maybe_gas_limit: Option, + pub maybe_block_gas_limit: Option, } #[derive(Debug, Parser)] @@ -76,7 +76,7 @@ struct ExecuteOpt { pub no_conflict_txns: bool, #[clap(long)] - pub maybe_gas_limit: Option, + pub maybe_block_gas_limit: Option, } fn param_sweep(opt: ParamSweepOpt) { @@ -91,7 +91,7 @@ fn param_sweep(opt: ParamSweepOpt) { let run_parallel = !opt.skip_parallel; let run_sequential = !opt.skip_sequential; - let maybe_gas_limit = opt.maybe_gas_limit; + let maybe_block_gas_limit = opt.maybe_block_gas_limit; assert!( run_sequential || run_parallel, @@ -110,7 +110,7 @@ fn param_sweep(opt: ParamSweepOpt) { 1, concurrency_level, false, - maybe_gas_limit, + maybe_block_gas_limit, ); par_tps.sort(); seq_tps.sort(); @@ -171,7 +171,7 @@ fn execute(opt: ExecuteOpt) { opt.num_executor_shards, opt.concurrency_level_per_shard, opt.no_conflict_txns, - opt.maybe_gas_limit, + opt.maybe_block_gas_limit, ); let sum: usize = par_tps.iter().sum(); diff --git a/aptos-move/aptos-transaction-benchmarks/src/transactions.rs b/aptos-move/aptos-transaction-benchmarks/src/transactions.rs index fb6d65617cf72..a1ce63d35f2ef 100644 --- a/aptos-move/aptos-transaction-benchmarks/src/transactions.rs +++ b/aptos-move/aptos-transaction-benchmarks/src/transactions.rs @@ -73,7 +73,6 @@ where self.num_transactions, 1, AccountPickStyle::Unlimited, - None, ) }, |state| state.execute_sequential(), @@ -92,7 +91,6 @@ where self.num_transactions, 1, AccountPickStyle::Unlimited, - None, ) }, |state| state.execute_parallel(), @@ -113,7 +111,7 @@ where num_executor_shards: usize, concurrency_level_per_shard: usize, no_conflict_txn: bool, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> (Vec, Vec) { let mut par_tps = Vec::new(); let mut seq_tps = Vec::new(); @@ -138,7 +136,6 @@ where num_txn, num_executor_shards, account_pick_style, - maybe_gas_limit, ); for i in 0..total_runs { @@ -149,6 +146,7 @@ where run_seq, no_conflict_txn, concurrency_level_per_shard, + maybe_block_gas_limit, ); } else { let tps = state.execute_blockstm_benchmark( @@ -156,6 +154,7 @@ where run_seq, no_conflict_txn, concurrency_level_per_shard, + maybe_block_gas_limit, ); par_tps.push(tps.0); seq_tps.push(tps.1); @@ -188,14 +187,12 @@ where num_transactions: usize, num_executor_shards: usize, account_pick_style: AccountPickStyle, - maybe_gas_limit: Option, ) -> Self { Self::with_universe( strategy, universe_strategy(num_accounts, num_transactions, account_pick_style), num_transactions, num_executor_shards, - maybe_gas_limit, ) } @@ -206,7 +203,6 @@ where universe_strategy: impl Strategy, num_transactions: usize, num_executor_shards: usize, - maybe_gas_limit: Option, ) -> Self { let mut runner = TestRunner::default(); let universe_gen = universe_strategy @@ -221,13 +217,9 @@ where let universe = universe_gen.setup_gas_cost_stability(&mut executor); let state_view = Arc::new(executor.get_state_view().clone()); - let parallel_block_executor = Arc::new(ShardedBlockExecutor::new( - num_executor_shards, - None, - maybe_gas_limit, - )); - let sequential_block_executor = - Arc::new(ShardedBlockExecutor::new(1, Some(1), maybe_gas_limit)); + let parallel_block_executor = + Arc::new(ShardedBlockExecutor::new(num_executor_shards, None)); + let sequential_block_executor = Arc::new(ShardedBlockExecutor::new(1, Some(1))); let validator_set = ValidatorSet::fetch_config( &FakeExecutor::from_head_genesis() @@ -292,7 +284,7 @@ where let txns = self.gen_transaction(false); let executor = self.sequential_block_executor; executor - .execute_block(self.state_view.clone(), txns, 1) + .execute_block(self.state_view.clone(), txns, 1, None) .expect("VM should not fail to start"); } @@ -303,7 +295,7 @@ where let txns = self.gen_transaction(false); let executor = self.parallel_block_executor.clone(); executor - .execute_block(self.state_view.clone(), txns, num_cpus::get()) + .execute_block(self.state_view.clone(), txns, num_cpus::get(), None) .expect("VM should not fail to start"); } @@ -312,6 +304,7 @@ where transactions: Vec, block_executor: Arc>, concurrency_level_per_shard: usize, + maybe_block_gas_limit: Option, ) -> usize { let block_size = transactions.len(); let timer = Instant::now(); @@ -320,6 +313,7 @@ where self.state_view.clone(), transactions, concurrency_level_per_shard, + maybe_block_gas_limit, ) .expect("VM should not fail to start"); let exec_time = timer.elapsed().as_millis(); @@ -333,6 +327,7 @@ where run_seq: bool, no_conflict_txns: bool, conurrency_level_per_shard: usize, + maybe_block_gas_limit: Option, ) -> (usize, usize) { let transactions = self.gen_transaction(no_conflict_txns); let par_tps = if run_par { @@ -341,6 +336,7 @@ where transactions.clone(), self.parallel_block_executor.clone(), conurrency_level_per_shard, + maybe_block_gas_limit, ); println!("Parallel execution finishes, TPS = {}", tps); tps @@ -349,8 +345,12 @@ where }; let seq_tps = if run_seq { println!("Sequential execution starts..."); - let tps = - self.execute_benchmark(transactions, self.sequential_block_executor.clone(), 1); + let tps = self.execute_benchmark( + transactions, + self.sequential_block_executor.clone(), + 1, + maybe_block_gas_limit, + ); println!("Sequential execution finishes, TPS = {}", tps); tps } else { diff --git a/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs b/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs index fb671b281013a..96f880b559fca 100644 --- a/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs +++ b/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs @@ -468,7 +468,7 @@ impl<'a> AptosTestAdapter<'a> { /// Should error if the transaction ends up being discarded, or having a status other than /// EXECUTED. fn run_transaction(&mut self, txn: Transaction) -> Result { - let mut outputs = AptosVM::execute_block(vec![txn], &self.storage.clone())?; + let mut outputs = AptosVM::execute_block(vec![txn], &self.storage.clone(), None)?; assert_eq!(outputs.len(), 1); diff --git a/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs b/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs index e1ee32f805fcb..ec1ecd8a226af 100644 --- a/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs +++ b/aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs @@ -44,7 +44,7 @@ fn main() -> Result<()> { }) .collect(); - let res = AptosVM::execute_block(txns, &state_store)?; + let res = AptosVM::execute_block(txns, &state_store, None)?; for i in 0..NUM_TXNS { assert!(res[i as usize].status().status().unwrap().is_success()); } diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 80163c8661e57..a3a1a1298255a 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -1472,6 +1472,7 @@ impl VMExecutor for AptosVM { fn execute_block( transactions: Vec, state_view: &(impl StateView + Sync), + maybe_block_gas_limit: Option, ) -> Result, VMStatus> { fail_point!("move_adapter::execute_block", |_| { Err(VMStatus::Error( @@ -1492,41 +1493,7 @@ impl VMExecutor for AptosVM { transactions, state_view, Self::get_concurrency_level(), - None, - ); - if ret.is_ok() { - // Record the histogram count for transactions per block. - BLOCK_TRANSACTION_COUNT.observe(count as f64); - } - ret - } - - fn execute_block_with_gas_limit( - transactions: Vec, - state_view: &(impl StateView + Sync), - maybe_gas_limit: Option, - ) -> std::result::Result, VMStatus> { - fail_point!("move_adapter::execute_block", |_| { - Err(VMStatus::Error( - StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, - None, - )) - }); - - let log_context = AdapterLogSchema::new(state_view.id(), 0); - info!( - log_context, - "Executing block, transaction count: {}", - transactions.len() - ); - - let count = transactions.len(); - let ret = BlockAptosVM::execute_block( - Arc::clone(&RAYON_EXEC_POOL), - transactions, - state_view, - Self::get_concurrency_level(), - maybe_gas_limit, + maybe_block_gas_limit, ); if ret.is_ok() { // Record the histogram count for transactions per block. @@ -1539,6 +1506,7 @@ impl VMExecutor for AptosVM { sharded_block_executor: &ShardedBlockExecutor, transactions: Vec, state_view: Arc, + maybe_block_gas_limit: Option, ) -> Result, VMStatus> { let log_context = AdapterLogSchema::new(state_view.id(), 0); info!( @@ -1552,6 +1520,7 @@ impl VMExecutor for AptosVM { state_view, transactions, AptosVM::get_concurrency_level(), + maybe_block_gas_limit, ); if ret.is_ok() { // Record the histogram count for transactions per block. diff --git a/aptos-move/aptos-vm/src/block_executor/mod.rs b/aptos-move/aptos-vm/src/block_executor/mod.rs index 302f64db151c4..2d9b3d9d93514 100644 --- a/aptos-move/aptos-vm/src/block_executor/mod.rs +++ b/aptos-move/aptos-vm/src/block_executor/mod.rs @@ -136,7 +136,7 @@ impl BlockAptosVM { transactions: Vec, state_view: &S, concurrency_level: usize, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> Result, VMStatus> { let _timer = BLOCK_EXECUTOR_EXECUTE_BLOCK_SECONDS.start_timer(); // Verify the signatures of all the transactions in parallel. @@ -162,7 +162,7 @@ impl BlockAptosVM { let executor = BlockExecutor::, S>::new( concurrency_level, executor_thread_pool, - maybe_gas_limit, + maybe_block_gas_limit, ); let ret = executor.execute_block(state_view, signature_verified_block, state_view); diff --git a/aptos-move/aptos-vm/src/lib.rs b/aptos-move/aptos-vm/src/lib.rs index 0152b7df53543..54125625acedb 100644 --- a/aptos-move/aptos-vm/src/lib.rs +++ b/aptos-move/aptos-vm/src/lib.rs @@ -152,14 +152,7 @@ pub trait VMExecutor: Send + Sync { fn execute_block( transactions: Vec, state_view: &(impl StateView + Sync), - ) -> Result, VMStatus>; - - /// Executes a block of transactions with per_block_gas_limit - /// and returns output for each one of them. - fn execute_block_with_gas_limit( - transactions: Vec, - state_view: &(impl StateView + Sync), - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> Result, VMStatus>; /// Executes a block of transactions using a sharded block executor and returns the results. @@ -167,6 +160,7 @@ pub trait VMExecutor: Send + Sync { sharded_block_executor: &ShardedBlockExecutor, transactions: Vec, state_view: Arc, + maybe_block_gas_limit: Option, ) -> Result, VMStatus>; } diff --git a/aptos-move/aptos-vm/src/sharded_block_executor/executor_shard.rs b/aptos-move/aptos-vm/src/sharded_block_executor/executor_shard.rs index 2206e00c62ae6..2f3d5b427a6b0 100644 --- a/aptos-move/aptos-vm/src/sharded_block_executor/executor_shard.rs +++ b/aptos-move/aptos-vm/src/sharded_block_executor/executor_shard.rs @@ -20,7 +20,6 @@ pub struct ExecutorShard { executor_thread_pool: Arc, command_rx: Receiver>, result_tx: Sender, VMStatus>>, - maybe_gas_limit: Option, } impl ExecutorShard { @@ -30,7 +29,6 @@ impl ExecutorShard { num_executor_threads: usize, command_rx: Receiver>, result_tx: Sender, VMStatus>>, - maybe_gas_limit: Option, ) -> Self { let executor_thread_pool = Arc::new( rayon::ThreadPoolBuilder::new() @@ -49,7 +47,6 @@ impl ExecutorShard { executor_thread_pool, command_rx, result_tx, - maybe_gas_limit, } } @@ -61,6 +58,7 @@ impl ExecutorShard { state_view, transactions, concurrency_level_per_shard, + maybe_block_gas_limit, ) => { trace!( "Shard {} received ExecuteBlock command of block size {} ", @@ -72,7 +70,7 @@ impl ExecutorShard { transactions, state_view.as_ref(), concurrency_level_per_shard, - self.maybe_gas_limit, + maybe_block_gas_limit, ); drop(state_view); self.result_tx.send(ret).unwrap(); diff --git a/aptos-move/aptos-vm/src/sharded_block_executor/mod.rs b/aptos-move/aptos-vm/src/sharded_block_executor/mod.rs index 87e913c230f55..3537e08dbba5e 100644 --- a/aptos-move/aptos-vm/src/sharded_block_executor/mod.rs +++ b/aptos-move/aptos-vm/src/sharded_block_executor/mod.rs @@ -33,16 +33,12 @@ pub struct ShardedBlockExecutor { } pub enum ExecutorShardCommand { - ExecuteBlock(Arc, Vec, usize), + ExecuteBlock(Arc, Vec, usize, Option), Stop, } impl ShardedBlockExecutor { - pub fn new( - num_executor_shards: usize, - executor_threads_per_shard: Option, - maybe_gas_limit: Option, - ) -> Self { + pub fn new(num_executor_shards: usize, executor_threads_per_shard: Option) -> Self { assert!(num_executor_shards > 0, "num_executor_shards must be > 0"); let executor_threads_per_shard = executor_threads_per_shard.unwrap_or_else(|| { (num_cpus::get() as f64 / num_executor_shards as f64).ceil() as usize @@ -61,7 +57,6 @@ impl ShardedBlockExecutor { executor_threads_per_shard, transactions_rx, result_tx, - maybe_gas_limit, )); } info!( @@ -85,6 +80,7 @@ impl ShardedBlockExecutor { state_view: Arc, block: Vec, concurrency_level_per_shard: usize, + maybe_block_gas_limit: Option, ) -> Result, VMStatus> { let block_partitions = self.partitioner.partition(block, self.num_executor_shards); // Number of partitions might be smaller than the number of executor shards in case of @@ -96,6 +92,7 @@ impl ShardedBlockExecutor { state_view.clone(), transactions, concurrency_level_per_shard, + maybe_block_gas_limit, )) .unwrap(); } @@ -135,7 +132,6 @@ fn spawn_executor_shard( concurrency_level: usize, command_rx: Receiver>, result_tx: Sender, VMStatus>>, - maybe_gas_limit: Option, ) -> thread::JoinHandle<()> { // create and start a new executor shard in a separate thread thread::Builder::new() @@ -147,7 +143,6 @@ fn spawn_executor_shard( concurrency_level, command_rx, result_tx, - maybe_gas_limit, ); executor_shard.start(); }) diff --git a/aptos-move/block-executor/src/executor.rs b/aptos-move/block-executor/src/executor.rs index b0013246a7db5..3bd172dd5539d 100644 --- a/aptos-move/block-executor/src/executor.rs +++ b/aptos-move/block-executor/src/executor.rs @@ -49,7 +49,7 @@ pub struct BlockExecutor { // threads that may be concurrently participating in parallel execution. concurrency_level: usize, executor_thread_pool: Arc, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, phantom: PhantomData<(T, E, S)>, } @@ -64,7 +64,7 @@ where pub fn new( concurrency_level: usize, executor_thread_pool: Arc, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> Self { assert!( concurrency_level > 0 && concurrency_level <= num_cpus::get(), @@ -74,7 +74,7 @@ where Self { concurrency_level, executor_thread_pool, - maybe_gas_limit, + maybe_block_gas_limit, phantom: PhantomData, } } @@ -221,7 +221,7 @@ where fn coordinator_commit_hook( &self, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, scheduler: &Scheduler, post_commit_txs: &Vec>, worker_idx: &mut usize, @@ -266,7 +266,7 @@ where }, }; - if let Some(per_block_gas_limit) = maybe_gas_limit { + if let Some(per_block_gas_limit) = maybe_block_gas_limit { // When the accumulated gas of the committed txns exceeds PER_BLOCK_GAS_LIMIT, early halt BlockSTM. if *accumulated_gas >= per_block_gas_limit { // Set the execution output status to be SkipRest, to skip the rest of the txns. @@ -360,7 +360,7 @@ where match &role { CommitRole::Coordinator(post_commit_txs) => { self.coordinator_commit_hook( - self.maybe_gas_limit, + self.maybe_block_gas_limit, scheduler, post_commit_txs, &mut worker_idx, @@ -575,7 +575,7 @@ where break; } - if let Some(per_block_gas_limit) = self.maybe_gas_limit { + if let Some(per_block_gas_limit) = self.maybe_block_gas_limit { // When the accumulated gas of the committed txns // exceeds per_block_gas_limit, halt sequential execution. if accumulated_gas >= per_block_gas_limit { diff --git a/aptos-move/block-executor/src/proptest_types/tests.rs b/aptos-move/block-executor/src/proptest_types/tests.rs index 93d6cda5fc9af..d897a68f8604e 100644 --- a/aptos-move/block-executor/src/proptest_types/tests.rs +++ b/aptos-move/block-executor/src/proptest_types/tests.rs @@ -29,7 +29,7 @@ fn run_transactions( skip_rest_transactions: Vec, num_repeat: usize, module_access: (bool, bool), - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) where K: Hash + Clone + Debug + Eq + Send + Sync + PartialOrd + Ord + 'static, V: Clone + Eq + Send + Sync + Arbitrary + 'static, @@ -67,7 +67,7 @@ fn run_transactions( >::new( num_cpus::get(), executor_thread_pool.clone(), - maybe_gas_limit, + maybe_block_gas_limit, ) .execute_transactions_parallel((), &transactions, &data_view); @@ -76,7 +76,8 @@ fn run_transactions( continue; } - let baseline = ExpectedOutput::generate_baseline(&transactions, None, maybe_gas_limit); + let baseline = + ExpectedOutput::generate_baseline(&transactions, None, maybe_block_gas_limit); baseline.assert_output(&output); } } @@ -134,7 +135,7 @@ proptest! { } } -fn dynamic_read_writes_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn dynamic_read_writes_with_block_gas_limit(num_txns: usize, maybe_block_gas_limit: Option) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 100) @@ -156,11 +157,11 @@ fn dynamic_read_writes_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn deltas_writes_mixed_with_block_gas_limit(num_txns: usize, maybe_block_gas_limit: Option) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 50) @@ -200,16 +201,17 @@ fn deltas_writes_mixed_with_gas_limit(num_txns: usize, maybe_gas_limit: Option::new( num_cpus::get(), executor_thread_pool.clone(), - maybe_gas_limit, + maybe_block_gas_limit, ) .execute_transactions_parallel((), &transactions, &data_view); - let baseline = ExpectedOutput::generate_baseline(&transactions, None, maybe_gas_limit); + let baseline = + ExpectedOutput::generate_baseline(&transactions, None, maybe_block_gas_limit); baseline.assert_output(&output); } } -fn deltas_resolver_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn deltas_resolver_with_block_gas_limit(num_txns: usize, maybe_block_gas_limit: Option) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 50) @@ -249,7 +251,7 @@ fn deltas_resolver_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) >::new( num_cpus::get(), executor_thread_pool.clone(), - maybe_gas_limit, + maybe_block_gas_limit, ) .execute_transactions_parallel((), &transactions, &data_view); @@ -260,13 +262,19 @@ fn deltas_resolver_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) .map(|out| out.delta_writes()) .collect(); - let baseline = - ExpectedOutput::generate_baseline(&transactions, Some(delta_writes), maybe_gas_limit); + let baseline = ExpectedOutput::generate_baseline( + &transactions, + Some(delta_writes), + maybe_block_gas_limit, + ); baseline.assert_output(&output); } } -fn dynamic_read_writes_contended_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn dynamic_read_writes_contended_with_block_gas_limit( + num_txns: usize, + maybe_block_gas_limit: Option, +) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 10) @@ -289,11 +297,14 @@ fn dynamic_read_writes_contended_with_gas_limit(num_txns: usize, maybe_gas_limit vec![], 100, (false, false), - maybe_gas_limit, + maybe_block_gas_limit, ); } -fn module_publishing_fallback_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn module_publishing_fallback_with_block_gas_limit( + num_txns: usize, + maybe_block_gas_limit: Option, +) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 100) @@ -315,7 +326,7 @@ fn module_publishing_fallback_with_gas_limit(num_txns: usize, maybe_gas_limit: O vec![], 2, (false, true), - maybe_gas_limit, + maybe_block_gas_limit, ); run_transactions( &universe, @@ -324,7 +335,7 @@ fn module_publishing_fallback_with_gas_limit(num_txns: usize, maybe_gas_limit: O vec![], 2, (false, true), - maybe_gas_limit, + maybe_block_gas_limit, ); run_transactions( &universe, @@ -333,11 +344,14 @@ fn module_publishing_fallback_with_gas_limit(num_txns: usize, maybe_gas_limit: O vec![], 2, (true, true), - maybe_gas_limit, + maybe_block_gas_limit, ); } -fn publishing_fixed_params_with_gas_limit(num_txns: usize, maybe_gas_limit: Option) { +fn publishing_fixed_params_with_block_gas_limit( + num_txns: usize, + maybe_block_gas_limit: Option, +) { let mut runner = TestRunner::default(); let universe = vec(any::<[u8; 32]>(), 50) @@ -407,7 +421,7 @@ fn publishing_fixed_params_with_gas_limit(num_txns: usize, maybe_gas_limit: Opti Transaction, ValueType<[u8; 32]>>, Task, ValueType<[u8; 32]>>, DeltaDataView, ValueType<[u8; 32]>>, - >::new(num_cpus::get(), executor_thread_pool, maybe_gas_limit) + >::new(num_cpus::get(), executor_thread_pool, maybe_block_gas_limit) .execute_transactions_parallel((), &transactions, &data_view); assert_ok!(output); @@ -463,27 +477,27 @@ fn publishing_fixed_params_with_gas_limit(num_txns: usize, maybe_gas_limit: Opti #[test] fn dynamic_read_writes() { - dynamic_read_writes_with_gas_limit(3000, None); + dynamic_read_writes_with_block_gas_limit(3000, None); } #[test] fn deltas_writes_mixed() { - deltas_writes_mixed_with_gas_limit(1000, None); + deltas_writes_mixed_with_block_gas_limit(1000, None); } #[test] fn deltas_resolver() { - deltas_resolver_with_gas_limit(1000, None); + deltas_resolver_with_block_gas_limit(1000, None); } #[test] fn dynamic_read_writes_contended() { - dynamic_read_writes_contended_with_gas_limit(1000, None); + dynamic_read_writes_contended_with_block_gas_limit(1000, None); } #[test] fn module_publishing_fallback() { - module_publishing_fallback_with_gas_limit(3000, None); + module_publishing_fallback_with_block_gas_limit(3000, None); } #[test] @@ -491,7 +505,7 @@ fn module_publishing_fallback() { // not overlapping module r/w keys. fn module_publishing_races() { for _ in 0..5 { - publishing_fixed_params_with_gas_limit(300, None); + publishing_fixed_params_with_block_gas_limit(300, None); } } @@ -550,35 +564,41 @@ proptest! { } #[test] -fn dynamic_read_writes_with_block_gas_limit() { - dynamic_read_writes_with_gas_limit(3000, Some(rand::thread_rng().gen_range(0, 3000) as u64)); - dynamic_read_writes_with_gas_limit(3000, Some(0)); +fn dynamic_read_writes_with_block_gas_limit_test() { + dynamic_read_writes_with_block_gas_limit( + 3000, + Some(rand::thread_rng().gen_range(0, 3000) as u64), + ); + dynamic_read_writes_with_block_gas_limit(3000, Some(0)); } #[test] -fn deltas_writes_mixed_with_block_gas_limit() { - deltas_writes_mixed_with_gas_limit(1000, Some(rand::thread_rng().gen_range(0, 1000) as u64)); - deltas_writes_mixed_with_gas_limit(1000, Some(0)); +fn deltas_writes_mixed_with_block_gas_limit_test() { + deltas_writes_mixed_with_block_gas_limit( + 1000, + Some(rand::thread_rng().gen_range(0, 1000) as u64), + ); + deltas_writes_mixed_with_block_gas_limit(1000, Some(0)); } #[test] -fn deltas_resolver_with_block_gas_limit() { - deltas_resolver_with_gas_limit(1000, Some(rand::thread_rng().gen_range(0, 1000) as u64)); - deltas_resolver_with_gas_limit(1000, Some(0)); +fn deltas_resolver_with_block_gas_limit_test() { + deltas_resolver_with_block_gas_limit(1000, Some(rand::thread_rng().gen_range(0, 1000) as u64)); + deltas_resolver_with_block_gas_limit(1000, Some(0)); } #[test] -fn dynamic_read_writes_contended_with_block_gas_limit() { - dynamic_read_writes_contended_with_gas_limit( +fn dynamic_read_writes_contended_with_block_gas_limit_test() { + dynamic_read_writes_contended_with_block_gas_limit( 1000, Some(rand::thread_rng().gen_range(0, 1000) as u64), ); - dynamic_read_writes_contended_with_gas_limit(1000, Some(0)); + dynamic_read_writes_contended_with_block_gas_limit(1000, Some(0)); } #[test] -fn module_publishing_fallback_with_block_gas_limit() { - module_publishing_fallback_with_gas_limit( +fn module_publishing_fallback_with_block_gas_limit_test() { + module_publishing_fallback_with_block_gas_limit( 3000, // Need to execute at least 2 txns to trigger module publishing fallback Some(rand::thread_rng().gen_range(1, 3000) as u64), @@ -588,9 +608,9 @@ fn module_publishing_fallback_with_block_gas_limit() { #[test] // Test a single transaction intersection interleaves with a lot of dependencies and // not overlapping module r/w keys. -fn module_publishing_races_with_block_gas_limit() { +fn module_publishing_races_with_block_gas_limit_test() { for _ in 0..5 { - publishing_fixed_params_with_gas_limit( + publishing_fixed_params_with_block_gas_limit( 300, Some(rand::thread_rng().gen_range(0, 300) as u64), ); diff --git a/aptos-move/block-executor/src/proptest_types/types.rs b/aptos-move/block-executor/src/proptest_types/types.rs index c0c8aca22ac88..d1064637c2813 100644 --- a/aptos-move/block-executor/src/proptest_types/types.rs +++ b/aptos-move/block-executor/src/proptest_types/types.rs @@ -536,7 +536,7 @@ impl ExpectedOutput { pub fn generate_baseline( txns: &[Transaction], resolved_deltas: Option>>, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> Self { let mut current_world = HashMap::new(); // Delta world stores the latest u128 value of delta aggregator. When empty, the @@ -640,7 +640,7 @@ impl ExpectedOutput { // In unit tests, the gas_used of any txn is set to be 1. accumulated_gas += 1; - if let Some(block_gas_limit) = maybe_gas_limit { + if let Some(block_gas_limit) = maybe_block_gas_limit { if accumulated_gas >= block_gas_limit { return Self::ExceedBlockGasLimit(idx, result_vec); } diff --git a/aptos-move/e2e-tests/src/executor.rs b/aptos-move/e2e-tests/src/executor.rs index c1edbf84c862c..20e6be81d52cc 100644 --- a/aptos-move/e2e-tests/src/executor.rs +++ b/aptos-move/e2e-tests/src/executor.rs @@ -436,7 +436,7 @@ impl FakeExecutor { } } - let output = AptosVM::execute_block(txn_block.clone(), &self.data_store); + let output = AptosVM::execute_block(txn_block.clone(), &self.data_store, None); if !self.no_parallel_exec { let parallel_output = self.execute_transaction_block_parallel(txn_block); assert_eq!(output, parallel_output); diff --git a/consensus/src/state_computer.rs b/consensus/src/state_computer.rs index 5a17885d5138f..e87161cc9ed6c 100644 --- a/consensus/src/state_computer.rs +++ b/consensus/src/state_computer.rs @@ -58,6 +58,7 @@ pub struct ExecutionProxy { write_mutex: AsyncMutex, payload_manager: Mutex>>, transaction_shuffler: Mutex>>, + maybe_block_gas_limit: Mutex>, transaction_deduper: Mutex>>, } @@ -92,6 +93,7 @@ impl ExecutionProxy { write_mutex: AsyncMutex::new(LogicalTime::new(0, 0)), payload_manager: Mutex::new(None), transaction_shuffler: Mutex::new(None), + maybe_block_gas_limit: Mutex::new(None), transaction_deduper: Mutex::new(None), } } @@ -127,7 +129,7 @@ impl StateComputer for ExecutionProxy { let deduped_txns = txn_deduper.dedup(txns); let shuffled_txns = txn_shuffler.shuffle(deduped_txns); - let block_gas_limit = self.executor.get_block_gas_limit(); + let block_gas_limit = *self.maybe_block_gas_limit.lock(); // TODO: figure out error handling for the prologue txn let executor = self.executor.clone(); @@ -141,7 +143,11 @@ impl StateComputer for ExecutionProxy { let compute_result = monitor!( "execute_block", tokio::task::spawn_blocking(move || { - executor.execute_block((block_id, transactions_to_execute), parent_block_id) + executor.execute_block( + (block_id, transactions_to_execute), + parent_block_id, + block_gas_limit, + ) }) .await ) @@ -185,7 +191,7 @@ impl StateComputer for ExecutionProxy { let txn_deduper = self.transaction_deduper.lock().as_ref().unwrap().clone(); let txn_shuffler = self.transaction_shuffler.lock().as_ref().unwrap().clone(); - let block_gas_limit = self.executor.get_block_gas_limit(); + let block_gas_limit = *self.maybe_block_gas_limit.lock(); for block in blocks { block_ids.push(block.id()); @@ -295,7 +301,7 @@ impl StateComputer for ExecutionProxy { epoch_state: &EpochState, payload_manager: Arc, transaction_shuffler: Arc, - _block_gas_limit: Option, + block_gas_limit: Option, transaction_deduper: Arc, ) { *self.validators.lock() = epoch_state @@ -306,9 +312,7 @@ impl StateComputer for ExecutionProxy { self.transaction_shuffler .lock() .replace(transaction_shuffler); - // TODO: Temporarily disable initializing block gas limit and leave it as default None, - // until there is a better way to handle the possible panic when executor is initialized. - // self.executor.update_block_gas_limit(block_gas_limit); + *self.maybe_block_gas_limit.lock() = block_gas_limit; self.transaction_deduper.lock().replace(transaction_deduper); } @@ -352,6 +356,7 @@ async fn test_commit_sync_race() { &self, _block: (HashValue, Vec), _parent_block_id: HashValue, + _maybe_block_gas_limit: Option, ) -> Result { Ok(StateComputeResult::new_dummy()) } @@ -370,12 +375,6 @@ async fn test_commit_sync_race() { } fn finish(&self) {} - - fn get_block_gas_limit(&self) -> Option { - None - } - - fn update_block_gas_limit(&self, _block_gas_limit: Option) {} } #[async_trait::async_trait] diff --git a/execution/executor-benchmark/src/native_executor.rs b/execution/executor-benchmark/src/native_executor.rs index 118fa4eb8b510..7294123769ce1 100644 --- a/execution/executor-benchmark/src/native_executor.rs +++ b/execution/executor-benchmark/src/native_executor.rs @@ -338,6 +338,7 @@ impl TransactionBlockExecutor for NativeExecutor { fn execute_transaction_block( transactions: Vec, state_view: CachedStateView, + _maybe_block_gas_limit: Option, ) -> Result { let transaction_outputs = NATIVE_EXECUTOR_POOL.install(|| { transactions @@ -411,17 +412,4 @@ impl TransactionBlockExecutor for NativeExecutor { state_cache: state_view.into_state_cache(), }) } - - // Dummy function that is not supposed to be used - fn execute_transaction_block_with_gas_limit( - _transactions: Vec, - state_view: CachedStateView, - _maybe_gas_limit: Option, - ) -> Result { - Ok(ChunkOutput { - transactions: vec![], - transaction_outputs: vec![], - state_cache: state_view.into_state_cache(), - }) - } } diff --git a/execution/executor-benchmark/src/transaction_executor.rs b/execution/executor-benchmark/src/transaction_executor.rs index f47b7c20c25cc..a156f13d8dc42 100644 --- a/execution/executor-benchmark/src/transaction_executor.rs +++ b/execution/executor-benchmark/src/transaction_executor.rs @@ -61,7 +61,7 @@ where let block_id = HashValue::random(); let output = self .executor - .execute_block((block_id, transactions), self.parent_block_id) + .execute_block((block_id, transactions), self.parent_block_id, None) .unwrap(); assert_eq!(output.compute_status().len(), num_txns); diff --git a/execution/executor-test-helpers/src/integration_test_impl.rs b/execution/executor-test-helpers/src/integration_test_impl.rs index fad11604db79a..83d3b77bffa31 100644 --- a/execution/executor-test-helpers/src/integration_test_impl.rs +++ b/execution/executor-test-helpers/src/integration_test_impl.rs @@ -21,7 +21,7 @@ use aptos_types::{ block_metadata::BlockMetadata, chain_id::ChainId, event::EventKey, - test_helpers::transaction_test_helpers::block, + test_helpers::transaction_test_helpers::{block, BLOCK_GAS_LIMIT}, transaction::{ Transaction, Transaction::UserTransaction, TransactionListWithProof, TransactionWithProof, WriteSetPayload, @@ -160,10 +160,14 @@ pub fn test_execution_with_storage_impl() -> Arc { txn_factory.transfer(account3.address(), 10 * B), ))); } - let block3 = block(block3, executor.get_block_gas_limit()); // append state checkpoint txn + let block3 = block(block3, BLOCK_GAS_LIMIT); // append state checkpoint txn let output1 = executor - .execute_block((block1_id, block1.clone()), parent_block_id) + .execute_block( + (block1_id, block1.clone()), + parent_block_id, + BLOCK_GAS_LIMIT, + ) .unwrap(); let li1 = gen_ledger_info_with_sigs(1, &output1, block1_id, &[signer.clone()]); let epoch2_genesis_id = Block::make_genesis_block_from_ledger_info(li1.ledger_info()).id(); @@ -371,7 +375,7 @@ pub fn test_execution_with_storage_impl() -> Arc { // Execute block 2, 3, 4 let output2 = executor - .execute_block((block2_id, block2), epoch2_genesis_id) + .execute_block((block2_id, block2), epoch2_genesis_id, BLOCK_GAS_LIMIT) .unwrap(); let li2 = gen_ledger_info_with_sigs(2, &output2, block2_id, &[signer.clone()]); let epoch3_genesis_id = Block::make_genesis_block_from_ledger_info(li2.ledger_info()).id(); @@ -386,7 +390,11 @@ pub fn test_execution_with_storage_impl() -> Arc { assert_eq!(current_version, 13); let output3 = executor - .execute_block((block3_id, block3.clone()), epoch3_genesis_id) + .execute_block( + (block3_id, block3.clone()), + epoch3_genesis_id, + BLOCK_GAS_LIMIT, + ) .unwrap(); let li3 = gen_ledger_info_with_sigs(3, &output3, block3_id, &[signer]); executor.commit_blocks(vec![block3_id], li3).unwrap(); @@ -430,7 +438,7 @@ pub fn test_execution_with_storage_impl() -> Arc { .unwrap(); // With block gas limit, StateCheckpoint txn is inserted to block after execution. - let diff = executor.get_block_gas_limit().map(|_| 0).unwrap_or(1); + let diff = BLOCK_GAS_LIMIT.map(|_| 0).unwrap_or(1); let transaction_list_with_proof = db .reader diff --git a/execution/executor-types/src/lib.rs b/execution/executor-types/src/lib.rs index 69ca00e860fec..eeafe613814f2 100644 --- a/execution/executor-types/src/lib.rs +++ b/execution/executor-types/src/lib.rs @@ -92,6 +92,7 @@ pub trait BlockExecutorTrait: Send + Sync { &self, block: (HashValue, Vec), parent_block_id: HashValue, + maybe_block_gas_limit: Option, ) -> Result; /// Saves eligible blocks to persistent storage. @@ -124,10 +125,6 @@ pub trait BlockExecutorTrait: Send + Sync { /// Finishes the block executor by releasing memory held by inner data structures(SMT). fn finish(&self); - - fn get_block_gas_limit(&self) -> Option; - - fn update_block_gas_limit(&self, block_gas_limit: Option); } #[derive(Clone)] diff --git a/execution/executor/src/block_executor.rs b/execution/executor/src/block_executor.rs index b1568a582821c..9097df8babcd2 100644 --- a/execution/executor/src/block_executor.rs +++ b/execution/executor/src/block_executor.rs @@ -16,7 +16,7 @@ use crate::{ use anyhow::Result; use aptos_crypto::HashValue; use aptos_executor_types::{BlockExecutorTrait, Error, StateComputeResult}; -use aptos_infallible::{Mutex, RwLock}; +use aptos_infallible::RwLock; use aptos_logger::prelude::*; use aptos_scratchpad::SparseMerkleTree; use aptos_state_view::StateViewId; @@ -35,12 +35,7 @@ pub trait TransactionBlockExecutor: Send + Sync { fn execute_transaction_block( transactions: Vec, state_view: CachedStateView, - ) -> Result; - - fn execute_transaction_block_with_gas_limit( - transactions: Vec, - state_view: CachedStateView, - maybe_gas_limit: Option, + maybe_block_gas_limit: Option, ) -> Result; } @@ -48,19 +43,12 @@ impl TransactionBlockExecutor for AptosVM { fn execute_transaction_block( transactions: Vec, state_view: CachedStateView, + maybe_block_gas_limit: Option, ) -> Result { - ChunkOutput::by_transaction_execution::(transactions, state_view) - } - - fn execute_transaction_block_with_gas_limit( - transactions: Vec, - state_view: CachedStateView, - maybe_gas_limit: Option, - ) -> Result { - ChunkOutput::by_transaction_execution_with_gas_limit::( + ChunkOutput::by_transaction_execution::( transactions, state_view, - maybe_gas_limit, + maybe_block_gas_limit, ) } } @@ -101,24 +89,6 @@ impl BlockExecutorTrait for BlockExecutor where V: TransactionBlockExecutor, { - fn get_block_gas_limit(&self) -> Option { - self.maybe_initialize().expect("Failed to initialize."); - self.inner - .read() - .as_ref() - .expect("BlockExecutor is not reset") - .get_block_gas_limit() - } - - fn update_block_gas_limit(&self, block_gas_limit: Option) { - self.maybe_initialize().expect("Failed to initialize."); - self.inner - .write() - .as_ref() - .expect("BlockExecutor is not reset") - .update_block_gas_limit(block_gas_limit); - } - fn committed_block_id(&self) -> HashValue { self.maybe_initialize().expect("Failed to initialize."); self.inner @@ -137,13 +107,14 @@ where &self, block: (HashValue, Vec), parent_block_id: HashValue, + maybe_block_gas_limit: Option, ) -> Result { self.maybe_initialize()?; self.inner .read() .as_ref() .expect("BlockExecutor is not reset") - .execute_block(block, parent_block_id) + .execute_block(block, parent_block_id, maybe_block_gas_limit) } fn commit_blocks_ext( @@ -168,7 +139,6 @@ struct BlockExecutorInner { db: DbReaderWriter, block_tree: BlockTree, phantom: PhantomData, - block_gas_limit: Mutex>, } impl BlockExecutorInner @@ -181,7 +151,6 @@ where db, block_tree, phantom: PhantomData, - block_gas_limit: Mutex::new(None), }) } @@ -200,15 +169,6 @@ impl BlockExecutorInner where V: TransactionBlockExecutor, { - fn get_block_gas_limit(&self) -> Option { - self.block_gas_limit.lock().as_ref().copied() - } - - fn update_block_gas_limit(&self, block_gas_limit: Option) { - let mut gas_limit = self.block_gas_limit.lock(); - *gas_limit = block_gas_limit; - } - fn committed_block_id(&self) -> HashValue { self.block_tree.root_block().id } @@ -217,6 +177,7 @@ where &self, block: (HashValue, Vec), parent_block_id: HashValue, + maybe_block_gas_limit: Option, ) -> Result { let _timer = APTOS_EXECUTOR_EXECUTE_BLOCK_SECONDS.start_timer(); let (block_id, transactions) = block; @@ -261,8 +222,6 @@ where )? }; - let maybe_gas_limit = self.get_block_gas_limit(); - let chunk_output = { let _timer = APTOS_EXECUTOR_VM_EXECUTE_BLOCK_SECONDS.start_timer(); fail_point!("executor::vm_execute_block", |_| { @@ -270,15 +229,7 @@ where "Injected error in vm_execute_block" ))) }); - if maybe_gas_limit.is_some() { - V::execute_transaction_block_with_gas_limit( - transactions, - state_view, - maybe_gas_limit, - )? - } else { - V::execute_transaction_block(transactions, state_view)? - } + V::execute_transaction_block(transactions, state_view, maybe_block_gas_limit)? }; chunk_output.trace_log_transaction_status(); @@ -287,7 +238,7 @@ where .start_timer(); let (output, _, _) = chunk_output - .apply_to_ledger_for_block(parent_view, maybe_gas_limit.map(|_| block_id))?; + .apply_to_ledger_for_block(parent_view, maybe_block_gas_limit.map(|_| block_id))?; output }; diff --git a/execution/executor/src/chunk_executor.rs b/execution/executor/src/chunk_executor.rs index 3e3724a40b217..e2c3b9f27459f 100644 --- a/execution/executor/src/chunk_executor.rs +++ b/execution/executor/src/chunk_executor.rs @@ -203,7 +203,8 @@ impl ChunkExecutorInner { let state_view = self.state_view(&latest_view)?; let chunk_output = { let _timer = APTOS_EXECUTOR_VM_EXECUTE_CHUNK_SECONDS.start_timer(); - ChunkOutput::by_transaction_execution::(transactions, state_view)? + // State sync executor shouldn't have block gas limit. + ChunkOutput::by_transaction_execution::(transactions, state_view, None)? }; let executed_chunk = Self::apply_chunk_output_for_state_sync( verified_target_li, @@ -529,7 +530,8 @@ impl ChunkExecutorInner { .cloned() .collect(); - let chunk_output = ChunkOutput::by_transaction_execution::(txns, state_view)?; + // State sync executor shouldn't have block gas limit. + let chunk_output = ChunkOutput::by_transaction_execution::(txns, state_view, None)?; // not `zip_eq`, deliberately for (version, txn_out, txn_info, write_set, events) in multizip(( begin_version..end_version, diff --git a/execution/executor/src/components/chunk_output.rs b/execution/executor/src/components/chunk_output.rs index b4fd421ec6262..6561082edf249 100644 --- a/execution/executor/src/components/chunk_output.rs +++ b/execution/executor/src/components/chunk_output.rs @@ -29,7 +29,6 @@ pub static SHARDED_BLOCK_EXECUTOR: Lazy( transactions: Vec, state_view: CachedStateView, + maybe_block_gas_limit: Option, ) -> Result { - let transaction_outputs = Self::execute_block::(transactions.clone(), &state_view)?; - - // to print txn output for debugging, uncomment: - // println!("{:?}", transaction_outputs.iter().map(|t| t.status() ).collect::>()); - - update_counters_for_processed_chunk(&transactions, &transaction_outputs, "executed"); - - Ok(Self { - transactions, - transaction_outputs, - state_cache: state_view.into_state_cache(), - }) - } - - pub fn by_transaction_execution_with_gas_limit( - transactions: Vec, - state_view: CachedStateView, - maybe_gas_limit: Option, - ) -> Result { - let transaction_outputs = Self::execute_block_with_gas_limit::( - transactions.clone(), - &state_view, - maybe_gas_limit, - )?; + let transaction_outputs = + Self::execute_block::(transactions.clone(), &state_view, maybe_block_gas_limit)?; // to print txn output for debugging, uncomment: // println!("{:?}", transaction_outputs.iter().map(|t| t.status() ).collect::>()); @@ -89,10 +67,14 @@ impl ChunkOutput { pub fn by_transaction_execution_sharded( transactions: Vec, state_view: CachedStateView, + maybe_block_gas_limit: Option, ) -> Result { let state_view_arc = Arc::new(state_view); - let transaction_outputs = - Self::execute_block_sharded::(transactions.clone(), state_view_arc.clone())?; + let transaction_outputs = Self::execute_block_sharded::( + transactions.clone(), + state_view_arc.clone(), + maybe_block_gas_limit, + )?; update_counters_for_processed_chunk(&transactions, &transaction_outputs, "executed"); @@ -172,11 +154,13 @@ impl ChunkOutput { fn execute_block_sharded( transactions: Vec, state_view: Arc, + maybe_block_gas_limit: Option, ) -> Result> { Ok(V::execute_block_sharded( SHARDED_BLOCK_EXECUTOR.lock().deref(), transactions, state_view, + maybe_block_gas_limit, )?) } @@ -186,22 +170,12 @@ impl ChunkOutput { fn execute_block( transactions: Vec, state_view: &CachedStateView, + maybe_block_gas_limit: Option, ) -> Result> { - Ok(V::execute_block(transactions, &state_view)?) - } - - /// Executes the block of [Transaction]s using the [VMExecutor] and returns - /// a vector of [TransactionOutput]s. - #[cfg(not(feature = "consensus-only-perf-test"))] - fn execute_block_with_gas_limit( - transactions: Vec, - state_view: &CachedStateView, - maybe_gas_limit: Option, - ) -> Result> { - Ok(V::execute_block_with_gas_limit( + Ok(V::execute_block( transactions, &state_view, - maybe_gas_limit, + maybe_block_gas_limit, )?) } @@ -213,13 +187,16 @@ impl ChunkOutput { fn execute_block( transactions: Vec, state_view: &CachedStateView, + maybe_block_gas_limit: Option, ) -> Result> { use aptos_state_view::{StateViewId, TStateView}; use aptos_types::write_set::WriteSet; let transaction_outputs = match state_view.id() { // this state view ID implies a genesis block in non-test cases. - StateViewId::Miscellaneous => V::execute_block(transactions, &state_view)?, + StateViewId::Miscellaneous => { + V::execute_block(transactions, &state_view, maybe_block_gas_limit)? + }, _ => transactions .iter() .map(|_| { @@ -234,16 +211,6 @@ impl ChunkOutput { }; Ok(transaction_outputs) } - - /// In consensus-only mode, we do not care about gas limits. - #[cfg(feature = "consensus-only-perf-test")] - fn execute_block_with_gas_limit( - transactions: Vec, - state_view: &CachedStateView, - _maybe_gas_limit: Option, - ) -> Result> { - Self::execute_block::(transactions, state_view) - } } pub fn update_counters_for_processed_chunk( diff --git a/execution/executor/src/db_bootstrapper.rs b/execution/executor/src/db_bootstrapper.rs index 2f8b7ef9123ec..c898a6269b215 100644 --- a/execution/executor/src/db_bootstrapper.rs +++ b/execution/executor/src/db_bootstrapper.rs @@ -136,9 +136,12 @@ pub fn calculate_genesis( get_state_epoch(&base_state_view)? }; - let (mut output, _, _) = - ChunkOutput::by_transaction_execution::(vec![genesis_txn.clone()], base_state_view)? - .apply_to_ledger(&executed_trees, None)?; + let (mut output, _, _) = ChunkOutput::by_transaction_execution::( + vec![genesis_txn.clone()], + base_state_view, + None, + )? + .apply_to_ledger(&executed_trees, None)?; ensure!( !output.to_commit.is_empty(), "Genesis txn execution failed." diff --git a/execution/executor/src/fuzzing.rs b/execution/executor/src/fuzzing.rs index c24db7ec31894..45dd95abf8860 100644 --- a/execution/executor/src/fuzzing.rs +++ b/execution/executor/src/fuzzing.rs @@ -15,6 +15,7 @@ use aptos_storage_interface::{ }; use aptos_types::{ ledger_info::LedgerInfoWithSignatures, + test_helpers::transaction_test_helpers::BLOCK_GAS_LIMIT, transaction::{Transaction, TransactionOutput, TransactionToCommit, Version}, vm_status::VMStatus, }; @@ -38,7 +39,7 @@ pub fn fuzz_execute_and_commit_blocks( let mut block_ids = vec![]; for block in blocks { let block_id = block.0; - let _execution_results = executor.execute_block(block, parent_block_id); + let _execution_results = executor.execute_block(block, parent_block_id, BLOCK_GAS_LIMIT); parent_block_id = block_id; block_ids.push(block_id); } @@ -52,19 +53,12 @@ impl TransactionBlockExecutor for FakeVM { fn execute_transaction_block( transactions: Vec, state_view: CachedStateView, + maybe_block_gas_limit: Option, ) -> Result { - ChunkOutput::by_transaction_execution::(transactions, state_view) - } - - fn execute_transaction_block_with_gas_limit( - transactions: Vec, - state_view: CachedStateView, - maybe_gas_limit: Option, - ) -> Result { - ChunkOutput::by_transaction_execution_with_gas_limit::( + ChunkOutput::by_transaction_execution::( transactions, state_view, - maybe_gas_limit, + maybe_block_gas_limit, ) } } @@ -74,6 +68,7 @@ impl VMExecutor for FakeVM { _sharded_block_executor: &ShardedBlockExecutor, _transactions: Vec, _state_view: Arc, + _maybe_block_gas_limit: Option, ) -> Result, VMStatus> { Ok(Vec::new()) } @@ -81,14 +76,7 @@ impl VMExecutor for FakeVM { fn execute_block( _transactions: Vec, _state_view: &impl StateView, - ) -> Result, VMStatus> { - Ok(Vec::new()) - } - - fn execute_block_with_gas_limit( - _transactions: Vec, - _state_view: &impl StateView, - _maybe_gas_limit: Option, + _maybe_block_gas_limit: Option, ) -> Result, VMStatus> { Ok(Vec::new()) } diff --git a/execution/executor/src/mock_vm/mock_vm_test.rs b/execution/executor/src/mock_vm/mock_vm_test.rs index 73c24af2e0164..05cbd65739387 100644 --- a/execution/executor/src/mock_vm/mock_vm_test.rs +++ b/execution/executor/src/mock_vm/mock_vm_test.rs @@ -45,7 +45,7 @@ fn test_mock_vm_different_senders() { txns.push(encode_mint_transaction(gen_address(i), amount)); } - let outputs = MockVM::execute_block(txns.clone(), &MockStateView) + let outputs = MockVM::execute_block(txns.clone(), &MockStateView, None) .expect("MockVM should not fail to start"); for (output, txn) in itertools::zip_eq(outputs.iter(), txns.iter()) { @@ -82,7 +82,7 @@ fn test_mock_vm_same_sender() { } let outputs = - MockVM::execute_block(txns, &MockStateView).expect("MockVM should not fail to start"); + MockVM::execute_block(txns, &MockStateView, None).expect("MockVM should not fail to start"); for (i, output) in outputs.iter().enumerate() { assert_eq!( @@ -116,7 +116,7 @@ fn test_mock_vm_payment() { ]; let output = - MockVM::execute_block(txns, &MockStateView).expect("MockVM should not fail to start"); + MockVM::execute_block(txns, &MockStateView, None).expect("MockVM should not fail to start"); let mut output_iter = output.iter(); output_iter.next(); diff --git a/execution/executor/src/mock_vm/mod.rs b/execution/executor/src/mock_vm/mod.rs index f69543306d32f..6df256d61093c 100644 --- a/execution/executor/src/mock_vm/mod.rs +++ b/execution/executor/src/mock_vm/mod.rs @@ -61,19 +61,12 @@ impl TransactionBlockExecutor for MockVM { fn execute_transaction_block( transactions: Vec, state_view: CachedStateView, + maybe_block_gas_limit: Option, ) -> Result { - ChunkOutput::by_transaction_execution::(transactions, state_view) - } - - fn execute_transaction_block_with_gas_limit( - transactions: Vec, - state_view: CachedStateView, - maybe_gas_limit: Option, - ) -> Result { - ChunkOutput::by_transaction_execution_with_gas_limit::( + ChunkOutput::by_transaction_execution::( transactions, state_view, - maybe_gas_limit, + maybe_block_gas_limit, ) } } @@ -82,6 +75,7 @@ impl VMExecutor for MockVM { fn execute_block( transactions: Vec, state_view: &impl StateView, + _maybe_block_gas_limit: Option, ) -> Result, VMStatus> { if state_view.is_genesis() { assert_eq!( @@ -212,18 +206,11 @@ impl VMExecutor for MockVM { Ok(outputs) } - fn execute_block_with_gas_limit( - transactions: Vec, - state_view: &(impl StateView + Sync), - _maybe_gas_limit: Option, - ) -> Result, VMStatus> { - MockVM::execute_block(transactions, state_view) - } - fn execute_block_sharded( _sharded_block_executor: &ShardedBlockExecutor, _transactions: Vec, _state_view: Arc, + _maybe_block_gas_limit: Option, ) -> std::result::Result, VMStatus> { todo!() } diff --git a/execution/executor/src/tests/chunk_executor_tests.rs b/execution/executor/src/tests/chunk_executor_tests.rs index be6a4b19dd14c..d7c057bddc8fe 100644 --- a/execution/executor/src/tests/chunk_executor_tests.rs +++ b/execution/executor/src/tests/chunk_executor_tests.rs @@ -17,7 +17,7 @@ use aptos_executor_types::{BlockExecutorTrait, ChunkExecutorTrait}; use aptos_storage_interface::DbReaderWriter; use aptos_types::{ ledger_info::LedgerInfoWithSignatures, - test_helpers::transaction_test_helpers::block, + test_helpers::transaction_test_helpers::{block, BLOCK_GAS_LIMIT}, transaction::{TransactionListWithProof, TransactionOutputListWithProof}, }; use rand::Rng; @@ -274,15 +274,12 @@ fn test_executor_execute_and_commit_chunk_local_result_mismatch() { .collect::>(); let output = executor .execute_block( - (block_id, block(txns, executor.get_block_gas_limit())), + (block_id, block(txns, BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); - // With no block gas limit, StateCheckpoint txn is inserted to block before execution. - // So the ledger_info version needs to + 1 with no block gas limit. - let maybe_gas_limit = executor.get_block_gas_limit(); - let diff = maybe_gas_limit.map(|_| 0).unwrap_or(1); - let ledger_info = tests::gen_ledger_info(5 + diff, output.root_hash(), block_id, 1); + let ledger_info = tests::gen_ledger_info(5 + 1, output.root_hash(), block_id, 1); executor.commit_blocks(vec![block_id], ledger_info).unwrap(); } diff --git a/execution/executor/src/tests/mod.rs b/execution/executor/src/tests/mod.rs index 22ea1df12519e..e9e4338642b06 100644 --- a/execution/executor/src/tests/mod.rs +++ b/execution/executor/src/tests/mod.rs @@ -30,7 +30,7 @@ use aptos_types::{ ledger_info::{LedgerInfo, LedgerInfoWithSignatures}, proof::definition::LeafCount, state_store::{state_key::StateKey, state_value::StateValue}, - test_helpers::transaction_test_helpers::block, + test_helpers::transaction_test_helpers::{block, BLOCK_GAS_LIMIT}, transaction::{ ExecutionStatus, RawTransaction, Script, SignedTransaction, Transaction, TransactionListWithProof, TransactionOutput, TransactionPayload, TransactionStatus, @@ -53,8 +53,9 @@ fn execute_and_commit_block( let output = executor .execute_block( - (id, block(vec![txn], executor.get_block_gas_limit())), + (id, block(vec![txn], BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); let version = 2 * (txn_index + 1); @@ -80,7 +81,6 @@ impl TestExecutor { let waypoint = generate_waypoint::(&db, &genesis).unwrap(); maybe_bootstrap::(&db, &genesis, waypoint).unwrap(); let executor = BlockExecutor::new(db.clone()); - executor.update_block_gas_limit(Some(1000)); // Can comment out this line to test without gas limit TestExecutor { _path: path, @@ -152,11 +152,9 @@ fn test_executor_status() { let output = executor .execute_block( - ( - block_id, - block(vec![txn0, txn1, txn2], executor.get_block_gas_limit()), - ), + (block_id, block(vec![txn0, txn1, txn2], BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); @@ -184,10 +182,7 @@ fn test_executor_status_consensus_only() { let output = executor .execute_block( - ( - block_id, - block(vec![txn0, txn1, txn2], executor.get_block_gas_limit()), - ), + (block_id, block(vec![txn0, txn1, txn2], BLOCK_GAS_LIMIT)), parent_block_id, ) .unwrap(); @@ -216,8 +211,9 @@ fn test_executor_one_block() { .collect::>(); let output = executor .execute_block( - (block_id, block(txns, executor.get_block_gas_limit())), + (block_id, block(txns, BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); let version = num_user_txns + 1; @@ -261,20 +257,16 @@ fn test_executor_two_blocks_with_failed_txns() { .collect::>(); let _output1 = executor .execute_block( - ( - block1_id, - block(block1_txns, executor.get_block_gas_limit()), - ), + (block1_id, block(block1_txns, BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); let output2 = executor .execute_block( - ( - block2_id, - block(block2_txns, executor.get_block_gas_limit()), - ), + (block2_id, block(block2_txns, BLOCK_GAS_LIMIT)), block1_id, + BLOCK_GAS_LIMIT, ) .unwrap(); @@ -294,11 +286,9 @@ fn test_executor_commit_twice() { let block1_id = gen_block_id(1); let output1 = executor .execute_block( - ( - block1_id, - block(block1_txns, executor.get_block_gas_limit()), - ), + (block1_id, block(block1_txns, BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); let ledger_info = gen_ledger_info(6, output1.root_hash(), block1_id, 1); @@ -326,11 +316,9 @@ fn test_executor_execute_same_block_multiple_times() { for _i in 0..100 { let output = executor .execute_block( - ( - block_id, - block(txns.clone(), executor.get_block_gas_limit()), - ), + (block_id, block(txns.clone(), BLOCK_GAS_LIMIT)), parent_block_id, + BLOCK_GAS_LIMIT, ) .unwrap(); responses.push(output); @@ -339,10 +327,10 @@ fn test_executor_execute_same_block_multiple_times() { assert_eq!(responses.len(), 1); } -fn ledger_version_from_block_size(block_size: usize, maybe_gas_limit: Option) -> usize { +fn ledger_version_from_block_size(block_size: usize, maybe_block_gas_limit: Option) -> usize { // With block gas limit, StateCheckpoint txn is inserted to block after execution. // So the ledger_info version needs to block_size + 1 with block gas limit. - block_size + maybe_gas_limit.map(|_| 1).unwrap_or(0) + block_size + maybe_block_gas_limit.map(|_| 1).unwrap_or(0) } /// Generates a list of `TransactionListWithProof`s according to the given ranges. @@ -368,17 +356,20 @@ fn create_transaction_chunks( let txn = encode_mint_transaction(gen_address(i), 100); txns.push(txn); } - if executor.get_block_gas_limit().is_none() { + if BLOCK_GAS_LIMIT.is_none() { txns.push(Transaction::StateCheckpoint(HashValue::random())); } let id = gen_block_id(1); let output = executor - .execute_block((id, txns.clone()), executor.committed_block_id()) + .execute_block( + (id, txns.clone()), + executor.committed_block_id(), + BLOCK_GAS_LIMIT, + ) .unwrap(); - let ledger_version = - ledger_version_from_block_size(txns.len(), executor.get_block_gas_limit()) as u64; + let ledger_version = ledger_version_from_block_size(txns.len(), BLOCK_GAS_LIMIT) as u64; let ledger_info = gen_ledger_info(ledger_version, output.root_hash(), id, 1); executor .commit_blocks(vec![id], ledger_info.clone()) @@ -411,12 +402,20 @@ fn test_noop_block_after_reconfiguration() { let first_txn = encode_reconfiguration_transaction(); let first_block_id = gen_block_id(1); let output1 = executor - .execute_block((first_block_id, vec![first_txn]), parent_block_id) + .execute_block( + (first_block_id, vec![first_txn]), + parent_block_id, + BLOCK_GAS_LIMIT, + ) .unwrap(); parent_block_id = first_block_id; - let second_block = TestBlock::new(10, 10, gen_block_id(2), executor.get_block_gas_limit()); + let second_block = TestBlock::new(10, 10, gen_block_id(2), BLOCK_GAS_LIMIT); let output2 = executor - .execute_block((second_block.id, second_block.txns), parent_block_id) + .execute_block( + (second_block.id, second_block.txns), + parent_block_id, + BLOCK_GAS_LIMIT, + ) .unwrap(); assert_eq!(output1.root_hash(), output2.root_hash()); } @@ -589,24 +588,24 @@ fn test_reconfig_suffix_empty_blocks() { db: _, executor, } = TestExecutor::new(); - // add gas limit to be consistent with block executor that will add state checkpoint txn - let block_a = TestBlock::new(10000, 1, gen_block_id(1), Some(0)); + let block_a = TestBlock::new(10000, 1, gen_block_id(1), BLOCK_GAS_LIMIT); + // add block gas limit to be consistent with block executor that will add state checkpoint txn let mut block_b = TestBlock::new(10000, 1, gen_block_id(2), Some(0)); - let block_c = TestBlock::new(1, 1, gen_block_id(3), Some(0)); - let block_d = TestBlock::new(1, 1, gen_block_id(4), Some(0)); + let block_c = TestBlock::new(1, 1, gen_block_id(3), BLOCK_GAS_LIMIT); + let block_d = TestBlock::new(1, 1, gen_block_id(4), BLOCK_GAS_LIMIT); block_b.txns.push(encode_reconfiguration_transaction()); let parent_block_id = executor.committed_block_id(); executor - .execute_block((block_a.id, block_a.txns), parent_block_id) + .execute_block((block_a.id, block_a.txns), parent_block_id, BLOCK_GAS_LIMIT) .unwrap(); let output = executor - .execute_block((block_b.id, block_b.txns), block_a.id) + .execute_block((block_b.id, block_b.txns), block_a.id, BLOCK_GAS_LIMIT) .unwrap(); executor - .execute_block((block_c.id, block_c.txns), block_b.id) + .execute_block((block_c.id, block_c.txns), block_b.id, BLOCK_GAS_LIMIT) .unwrap(); executor - .execute_block((block_d.id, block_d.txns), block_c.id) + .execute_block((block_d.id, block_d.txns), block_c.id, BLOCK_GAS_LIMIT) .unwrap(); let ledger_info = gen_ledger_info(20002, output.root_hash(), block_d.id, 1); @@ -624,7 +623,12 @@ struct TestBlock { } impl TestBlock { - fn new(num_user_txns: u64, amount: u32, id: HashValue, maybe_gas_limit: Option) -> Self { + fn new( + num_user_txns: u64, + amount: u32, + id: HashValue, + maybe_block_gas_limit: Option, + ) -> Self { let txns = if num_user_txns == 0 { Vec::new() } else { @@ -632,7 +636,7 @@ impl TestBlock { (0..num_user_txns) .map(|index| encode_mint_transaction(gen_address(index), u64::from(amount))) .collect(), - maybe_gas_limit, + maybe_block_gas_limit, ) }; TestBlock { txns, id } @@ -641,7 +645,10 @@ impl TestBlock { // Executes a list of transactions by executing and immediately committing one at a time. Returns // the root hash after all transactions are committed. -fn run_transactions_naive(transactions: Vec) -> HashValue { +fn run_transactions_naive( + transactions: Vec, + maybe_block_gas_limit: Option, +) -> HashValue { let executor = TestExecutor::new(); let db = &executor.db; let mut ledger_view: ExecutedTrees = db.reader.get_latest_executed_trees().unwrap(); @@ -656,6 +663,7 @@ fn run_transactions_naive(transactions: Vec) -> HashValue { Arc::new(AsyncProofFetcher::new(db.reader.clone())), ) .unwrap(), + maybe_block_gas_limit, ) .unwrap(); let (executed, _, _) = out.apply_to_ledger(&ledger_view, None).unwrap(); @@ -689,13 +697,13 @@ proptest! { let executor = TestExecutor::new(); let block_id = gen_block_id(1); - let mut block = TestBlock::new(num_user_txns, 10, block_id, executor.get_block_gas_limit()); + let mut block = TestBlock::new(num_user_txns, 10, block_id, BLOCK_GAS_LIMIT); let num_txns = block.txns.len() as LeafCount; block.txns[reconfig_txn_index as usize] = encode_reconfiguration_transaction(); let parent_block_id = executor.committed_block_id(); let output = executor.execute_block( - (block_id, block.txns.clone()), parent_block_id + (block_id, block.txns.clone()), parent_block_id, BLOCK_GAS_LIMIT ).unwrap(); // assert: txns after the reconfiguration are with status "Retry" @@ -714,11 +722,11 @@ proptest! { // retry txns after reconfiguration let retry_block_id = gen_block_id(2); let retry_output = executor.execute_block( - (retry_block_id, block.txns.iter().skip(reconfig_txn_index as usize + 1).cloned().collect()), parent_block_id + (retry_block_id, block.txns.iter().skip(reconfig_txn_index as usize + 1).cloned().collect()), parent_block_id, BLOCK_GAS_LIMIT ).unwrap(); prop_assert!(retry_output.compute_status().iter().all(|s| matches!(*s, TransactionStatus::Keep(_)))); - let ledger_version = ledger_version_from_block_size(num_txns as usize, executor.get_block_gas_limit()) as u64; + let ledger_version = ledger_version_from_block_size(num_txns as usize, BLOCK_GAS_LIMIT) as u64; // commit let ledger_info = gen_ledger_info(ledger_version, retry_output.root_hash(), retry_block_id, 12345 /* timestamp */); @@ -750,22 +758,20 @@ proptest! { fn test_executor_restart(a_size in 1..30u64, b_size in 1..30u64, amount in any::()) { let TestExecutor { _path, db, executor } = TestExecutor::new(); - let block_a = TestBlock::new(a_size, amount, gen_block_id(1), executor.get_block_gas_limit()); - let block_b = TestBlock::new(b_size, amount, gen_block_id(2), executor.get_block_gas_limit()); + let block_a = TestBlock::new(a_size, amount, gen_block_id(1), BLOCK_GAS_LIMIT); + let block_b = TestBlock::new(b_size, amount, gen_block_id(2), BLOCK_GAS_LIMIT); let mut parent_block_id; let mut root_hash; - let maybe_gas_limit = executor.get_block_gas_limit(); - // First execute and commit one block, then destroy executor. { parent_block_id = executor.committed_block_id(); let output_a = executor.execute_block( - (block_a.id, block_a.txns.clone()), parent_block_id + (block_a.id, block_a.txns.clone()), parent_block_id, BLOCK_GAS_LIMIT ).unwrap(); root_hash = output_a.root_hash(); - let ledger_info = gen_ledger_info(ledger_version_from_block_size(block_a.txns.len(), maybe_gas_limit) as u64, root_hash, block_a.id, 1); + let ledger_info = gen_ledger_info(ledger_version_from_block_size(block_a.txns.len(), BLOCK_GAS_LIMIT) as u64, root_hash, block_a.id, 1); executor.commit_blocks(vec![block_a.id], ledger_info).unwrap(); parent_block_id = block_a.id; } @@ -773,11 +779,10 @@ proptest! { // Now we construct a new executor and run one more block. { let executor = BlockExecutor::::new(db); - executor.update_block_gas_limit(maybe_gas_limit); - let output_b = executor.execute_block((block_b.id, block_b.txns.clone()), parent_block_id).unwrap(); + let output_b = executor.execute_block((block_b.id, block_b.txns.clone()), parent_block_id, BLOCK_GAS_LIMIT).unwrap(); root_hash = output_b.root_hash(); let ledger_info = gen_ledger_info( - (ledger_version_from_block_size(block_a.txns.len(), maybe_gas_limit) + ledger_version_from_block_size(block_b.txns.len(), maybe_gas_limit)) as u64, + (ledger_version_from_block_size(block_a.txns.len(), BLOCK_GAS_LIMIT) + ledger_version_from_block_size(block_b.txns.len(), BLOCK_GAS_LIMIT)) as u64, root_hash, block_b.id, 2, @@ -788,15 +793,15 @@ proptest! { let expected_root_hash = run_transactions_naive({ let mut txns = vec![]; txns.extend(block_a.txns.iter().cloned()); - if executor.get_block_gas_limit().is_some() { + if BLOCK_GAS_LIMIT.is_some() { txns.push(Transaction::StateCheckpoint(block_a.id)); } txns.extend(block_b.txns.iter().cloned()); - if executor.get_block_gas_limit().is_some() { + if BLOCK_GAS_LIMIT.is_some() { txns.push(Transaction::StateCheckpoint(block_b.id)); } txns - }); + }, BLOCK_GAS_LIMIT); prop_assert_eq!(root_hash, expected_root_hash); } @@ -832,13 +837,13 @@ proptest! { let first_block_id = gen_block_id(1); let _output1 = executor.execute_block( (first_block_id, first_block_txns), - parent_block_id + parent_block_id, BLOCK_GAS_LIMIT ).unwrap(); let second_block_id = gen_block_id(2); let output2 = executor.execute_block( - (second_block_id, block(second_block_txns, executor.get_block_gas_limit())), - first_block_id, + (second_block_id, block(second_block_txns, BLOCK_GAS_LIMIT)), + first_block_id, BLOCK_GAS_LIMIT ).unwrap(); let version = chunk_size + overlap_size + num_new_txns + 1; diff --git a/execution/executor/tests/db_bootstrapper_test.rs b/execution/executor/tests/db_bootstrapper_test.rs index 0ccbf3e8a237e..f6b7f0c946e51 100644 --- a/execution/executor/tests/db_bootstrapper_test.rs +++ b/execution/executor/tests/db_bootstrapper_test.rs @@ -30,7 +30,7 @@ use aptos_types::{ event::EventHandle, on_chain_config::{access_path_for_config, ConfigurationResource, OnChainConfig, ValidatorSet}, state_store::state_key::StateKey, - test_helpers::transaction_test_helpers::block, + test_helpers::transaction_test_helpers::{block, BLOCK_GAS_LIMIT}, transaction::{authenticator::AuthenticationKey, ChangeSet, Transaction, WriteSetPayload}, trusted_state::TrustedState, validator_signer::ValidatorSigner, @@ -87,8 +87,9 @@ fn execute_and_commit(txns: Vec, db: &DbReaderWriter, signer: &Vali let executor = BlockExecutor::::new(db.clone()); let output = executor .execute_block( - (block_id, block(txns, executor.get_block_gas_limit())), + (block_id, block(txns, BLOCK_GAS_LIMIT)), executor.committed_block_id(), + BLOCK_GAS_LIMIT, ) .unwrap(); assert_eq!(output.num_leaves(), target_version + 1); diff --git a/execution/executor/tests/storage_integration_test.rs b/execution/executor/tests/storage_integration_test.rs index e986325192e8b..158ccf2f391ef 100644 --- a/execution/executor/tests/storage_integration_test.rs +++ b/execution/executor/tests/storage_integration_test.rs @@ -19,6 +19,7 @@ use aptos_types::{ account_view::AccountView, block_metadata::BlockMetadata, state_store::state_key::StateKey, + test_helpers::transaction_test_helpers::BLOCK_GAS_LIMIT, transaction::{Transaction, WriteSetPayload}, trusted_state::TrustedState, validator_signer::ValidatorSigner, @@ -139,7 +140,11 @@ fn test_reconfiguration() { let txn_block = vec![txn1, txn2, txn3]; let block_id = gen_block_id(1); let vm_output = executor - .execute_block((block_id, txn_block.clone()), parent_block_id) + .execute_block( + (block_id, txn_block.clone()), + parent_block_id, + BLOCK_GAS_LIMIT, + ) .unwrap(); // Make sure the execution result sees the reconfiguration diff --git a/types/src/test_helpers/transaction_test_helpers.rs b/types/src/test_helpers/transaction_test_helpers.rs index 9d4d1de16bc56..eac86008f93a2 100644 --- a/types/src/test_helpers/transaction_test_helpers.rs +++ b/types/src/test_helpers/transaction_test_helpers.rs @@ -15,6 +15,10 @@ use aptos_crypto::{ed25519::*, traits::*, HashValue}; const MAX_GAS_AMOUNT: u64 = 1_000_000; const TEST_GAS_PRICE: u64 = 100; +// The block gas limit parameter for executor tests +pub const BLOCK_GAS_LIMIT: Option = Some(1000); +// pub const BLOCK_GAS_LIMIT: Option = None; + static EMPTY_SCRIPT: &[u8] = include_bytes!("empty_script.mv"); // Create an expiration time 'seconds' after now @@ -239,8 +243,11 @@ pub fn get_test_txn_with_chain_id( SignedTransaction::new(raw_txn, public_key, signature) } -pub fn block(mut user_txns: Vec, maybe_gas_limit: Option) -> Vec { - if maybe_gas_limit.is_none() { +pub fn block( + mut user_txns: Vec, + maybe_block_gas_limit: Option, +) -> Vec { + if maybe_block_gas_limit.is_none() { user_txns.push(Transaction::StateCheckpoint(HashValue::random())); } user_txns