From 63cb2a858e4bd40fe4844d53fee3ef358d408530 Mon Sep 17 00:00:00 2001 From: Yoni <78365039+Yoni-Starkware@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:48:38 +0300 Subject: [PATCH] refactor: add BouncerConfig to BlockContext (#1956) --- crates/blockifier/bench/blockifier_bench.rs | 4 +- crates/blockifier/src/blockifier/block.rs | 10 +++- .../blockifier/src/blockifier/block_test.rs | 4 ++ .../src/blockifier/stateful_validator.rs | 10 +--- .../src/blockifier/stateful_validator_test.rs | 8 +-- .../src/blockifier/transaction_executor.rs | 4 +- .../blockifier/transaction_executor_test.rs | 56 ++++++------------- crates/blockifier/src/context.rs | 3 + .../blockifier/src/test_utils/struct_impls.rs | 54 ++++++------------ .../src/py_block_executor.rs | 11 ++-- crates/native_blockifier/src/py_validator.rs | 9 +-- 11 files changed, 61 insertions(+), 112 deletions(-) diff --git a/crates/blockifier/bench/blockifier_bench.rs b/crates/blockifier/bench/blockifier_bench.rs index d93827849f..eea3030073 100644 --- a/crates/blockifier/bench/blockifier_bench.rs +++ b/crates/blockifier/bench/blockifier_bench.rs @@ -10,7 +10,6 @@ use blockifier::abi::abi_utils::selector_from_name; use blockifier::blockifier::config::TransactionExecutorConfig; use blockifier::blockifier::transaction_executor::TransactionExecutor; -use blockifier::bouncer::BouncerConfig; use blockifier::context::{BlockContext, ChainInfo}; use blockifier::invoke_tx_args; use blockifier::test_utils::contracts::FeatureContract; @@ -61,8 +60,7 @@ impl TransfersGenerator { let state = test_state(&chain_info, BALANCE * 1000, &[(account_contract, N_ACCOUNTS)]); // TODO(Avi, 20/05/2024): Enable concurrency. let executor_config = TransactionExecutorConfig::default(); - let executor = - TransactionExecutor::new(state, block_context, BouncerConfig::max(), executor_config); + let executor = TransactionExecutor::new(state, block_context, executor_config); let account_addresses = (0..N_ACCOUNTS) .map(|instance_id| account_contract.get_instance_address(instance_id)) .collect::>(); diff --git a/crates/blockifier/src/blockifier/block.rs b/crates/blockifier/src/blockifier/block.rs index 98ab8bf05b..2d91d4f858 100644 --- a/crates/blockifier/src/blockifier/block.rs +++ b/crates/blockifier/src/blockifier/block.rs @@ -6,6 +6,7 @@ use starknet_api::hash::StarkFelt; use starknet_api::state::StorageKey; use crate::abi::constants; +use crate::bouncer::BouncerConfig; use crate::context::{BlockContext, ChainInfo}; use crate::state::errors::StateError; use crate::state::state_api::{State, StateResult}; @@ -62,6 +63,7 @@ pub fn pre_process_block( block_info: BlockInfo, chain_info: ChainInfo, versioned_constants: VersionedConstants, + bouncer_config: BouncerConfig, concurrency_mode: bool, ) -> StateResult { let should_block_hash_be_provided = @@ -81,7 +83,13 @@ pub fn pre_process_block( return Err(StateError::OldBlockHashNotProvided); } - Ok(BlockContext { block_info, chain_info, versioned_constants, concurrency_mode }) + Ok(BlockContext { + block_info, + chain_info, + versioned_constants, + bouncer_config, + concurrency_mode, + }) } pub struct BlockNumberHashPair { diff --git a/crates/blockifier/src/blockifier/block_test.rs b/crates/blockifier/src/blockifier/block_test.rs index bf93491f22..9dc41dd531 100644 --- a/crates/blockifier/src/blockifier/block_test.rs +++ b/crates/blockifier/src/blockifier/block_test.rs @@ -5,6 +5,7 @@ use starknet_api::state::StorageKey; use crate::abi::constants; use crate::blockifier::block::{pre_process_block, BlockInfo, BlockNumberHashPair}; +use crate::bouncer::BouncerConfig; use crate::context::ChainInfo; use crate::state::state_api::StateReader; use crate::test_utils::contracts::FeatureContract; @@ -29,6 +30,7 @@ fn test_pre_process_block() { block_info, ChainInfo::default(), VersionedConstants::default(), + BouncerConfig::max(), false, ) .unwrap(); @@ -50,6 +52,7 @@ fn test_pre_process_block() { block_info, ChainInfo::default(), VersionedConstants::default(), + BouncerConfig::max(), false, ) .is_ok() @@ -63,6 +66,7 @@ fn test_pre_process_block() { block_info, ChainInfo::default(), VersionedConstants::default(), + BouncerConfig::max(), false, ); assert_eq!( diff --git a/crates/blockifier/src/blockifier/stateful_validator.rs b/crates/blockifier/src/blockifier/stateful_validator.rs index d551ed29d0..a7d36de1c1 100644 --- a/crates/blockifier/src/blockifier/stateful_validator.rs +++ b/crates/blockifier/src/blockifier/stateful_validator.rs @@ -5,7 +5,6 @@ use thiserror::Error; use crate::blockifier::config::TransactionExecutorConfig; use crate::blockifier::transaction_executor::{TransactionExecutor, TransactionExecutorError}; -use crate::bouncer::BouncerConfig; use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::CallInfo; use crate::fee::actual_cost::TransactionReceipt; @@ -47,14 +46,9 @@ impl StatefulValidator { state: CachedState, block_context: BlockContext, max_nonce_for_validation_skip: Nonce, - bouncer_config: BouncerConfig, ) -> Self { - let tx_executor = TransactionExecutor::new( - state, - block_context, - bouncer_config, - TransactionExecutorConfig::default(), - ); + let tx_executor = + TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default()); Self { tx_executor, max_nonce_for_validation_skip } } diff --git a/crates/blockifier/src/blockifier/stateful_validator_test.rs b/crates/blockifier/src/blockifier/stateful_validator_test.rs index 49d172758a..2c04daaefc 100644 --- a/crates/blockifier/src/blockifier/stateful_validator_test.rs +++ b/crates/blockifier/src/blockifier/stateful_validator_test.rs @@ -2,7 +2,6 @@ use rstest::rstest; use starknet_api::transaction::{Fee, TransactionVersion}; use crate::blockifier::stateful_validator::StatefulValidator; -use crate::bouncer::BouncerConfig; use crate::context::BlockContext; use crate::nonce; use crate::test_utils::contracts::FeatureContract; @@ -65,12 +64,7 @@ fn test_transaction_validator( } // Test the stateful validator. - let mut stateful_validator = StatefulValidator::create( - state, - block_context, - nonce!(0_u32), - BouncerConfig::create_for_testing(), - ); + let mut stateful_validator = StatefulValidator::create(state, block_context, nonce!(0_u32)); let reuslt = stateful_validator.perform_validations(tx, None); assert!(reuslt.is_ok(), "Validation failed: {:?}", reuslt.unwrap_err()); diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 3c7dbe9a93..219519ecd4 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -7,7 +7,7 @@ use starknet_api::core::ClassHash; use thiserror::Error; use crate::blockifier::config::TransactionExecutorConfig; -use crate::bouncer::{Bouncer, BouncerConfig, BouncerWeights}; +use crate::bouncer::{Bouncer, BouncerWeights}; use crate::context::BlockContext; use crate::execution::call_info::CallInfo; use crate::fee::actual_cost::TransactionReceipt; @@ -52,10 +52,10 @@ impl TransactionExecutor { pub fn new( state: CachedState, block_context: BlockContext, - bouncer_config: BouncerConfig, config: TransactionExecutorConfig, ) -> Self { log::debug!("Initializing Transaction Executor..."); + let bouncer_config = block_context.bouncer_config.clone(); // Note: the state might not be empty even at this point; it is the creator's // responsibility to tune the bouncer according to pre and post block process. let tx_executor = diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index 0b4fc987c5..4c75d367fa 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -7,7 +7,7 @@ use starknet_api::transaction::{Fee, TransactionVersion}; use crate::blockifier::config::TransactionExecutorConfig; use crate::blockifier::transaction_executor::{TransactionExecutor, TransactionExecutorError}; -use crate::bouncer::{Bouncer, BouncerConfig, BouncerWeights}; +use crate::bouncer::{Bouncer, BouncerWeights}; use crate::context::BlockContext; use crate::state::cached_state::CachedState; use crate::state::state_api::StateReader; @@ -35,12 +35,8 @@ fn tx_executor_test_body( charge_fee: bool, expected_bouncer_weights: BouncerWeights, ) { - let mut tx_executor = TransactionExecutor::new( - state, - block_context, - BouncerConfig::create_for_testing(), - TransactionExecutorConfig::default(), - ); + let mut tx_executor = + TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default()); // TODO(Arni, 30/03/2024): Consider adding a test for the transaction execution info. If A test // should not be added, rename the test to `test_bouncer_info`. // TODO(Arni, 30/03/2024): Test all bouncer weights. @@ -245,27 +241,16 @@ fn test_l1_handler(block_context: BlockContext, #[values(true, false)] charge_fe exceeds the maximum block capacity.")] #[case::transaction_too_large(BouncerWeights::default(), 11)] -fn test_bouncing( - block_context: BlockContext, - #[case] initial_bouncer_weights: BouncerWeights, - #[case] n_events: usize, -) { +fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_events: usize) { let max_n_events_in_block = 10; + let block_context = BlockContext::create_for_bouncer_testing(max_n_events_in_block); + let TestInitData { state, account_address, contract_address, mut nonce_manager } = create_test_init_data(&block_context.chain_info, CairoVersion::Cairo1); - let mut tx_executor = TransactionExecutor::new( - state, - block_context, - BouncerConfig { - block_max_capacity: BouncerWeights { - n_events: max_n_events_in_block, - ..BouncerWeights::max(false) - }, - ..BouncerConfig::default() - }, - TransactionExecutorConfig::default(), - ); + let mut tx_executor = + TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default()); + tx_executor.bouncer.set_accumulated_weights(initial_bouncer_weights); tx_executor @@ -283,24 +268,15 @@ fn test_bouncing( } #[rstest] -fn test_execute_txs_bouncing(block_context: BlockContext) { +fn test_execute_txs_bouncing() { + let max_n_events_in_block = 10; + let block_context = BlockContext::create_for_bouncer_testing(max_n_events_in_block); + let TestInitData { state, account_address, contract_address, .. } = create_test_init_data(&block_context.chain_info, CairoVersion::Cairo1); - let max_n_events_in_block = 10; - let bouncer_config = BouncerConfig { - block_max_capacity: BouncerWeights { - n_events: max_n_events_in_block, - ..BouncerWeights::max(false) - }, - ..BouncerConfig::default() - }; - let mut tx_executor = TransactionExecutor::new( - state, - block_context, - bouncer_config.clone(), - TransactionExecutorConfig::default(), - ); + let mut tx_executor = + TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default()); let txs: Vec = [ emit_n_events_tx(1, account_address, contract_address, nonce!(0_u32)), @@ -346,7 +322,7 @@ fn test_execute_txs_bouncing(block_context: BlockContext) { assert_eq!(remaining_tx_results.len(), 0); // Reset the bouncer and add the remaining transactions. - tx_executor.bouncer = Bouncer::new(bouncer_config); + tx_executor.bouncer = Bouncer::new(tx_executor.block_context.bouncer_config.clone()); let remaining_tx_results = tx_executor.execute_txs(remaining_txs, true); assert_eq!(remaining_tx_results.len(), 2); diff --git a/crates/blockifier/src/context.rs b/crates/blockifier/src/context.rs index 9b30b44a76..994ab2120c 100644 --- a/crates/blockifier/src/context.rs +++ b/crates/blockifier/src/context.rs @@ -1,6 +1,7 @@ use starknet_api::core::{ChainId, ContractAddress}; use crate::blockifier::block::BlockInfo; +use crate::bouncer::BouncerConfig; use crate::transaction::objects::{ FeeType, HasRelatedFeeType, TransactionInfo, TransactionInfoCreator, }; @@ -24,6 +25,7 @@ pub struct BlockContext { pub(crate) block_info: BlockInfo, pub(crate) chain_info: ChainInfo, pub(crate) versioned_constants: VersionedConstants, + pub(crate) bouncer_config: BouncerConfig, pub(crate) concurrency_mode: bool, } @@ -40,6 +42,7 @@ impl BlockContext { block_info: block_info.clone(), chain_info: chain_info.clone(), versioned_constants: versioned_constants.clone(), + bouncer_config: BouncerConfig::max(), concurrency_mode: false, } } diff --git a/crates/blockifier/src/test_utils/struct_impls.rs b/crates/blockifier/src/test_utils/struct_impls.rs index df36454e40..f543bb86e3 100644 --- a/crates/blockifier/src/test_utils/struct_impls.rs +++ b/crates/blockifier/src/test_utils/struct_impls.rs @@ -9,7 +9,7 @@ use starknet_api::{contract_address, patricia_key}; use super::update_json_value; use crate::blockifier::block::{BlockInfo, GasPrices}; -use crate::bouncer::{BouncerConfig, BouncerWeights, BuiltinCount}; +use crate::bouncer::{BouncerConfig, BouncerWeights}; use crate::context::{BlockContext, ChainInfo, FeeTokenAddresses, TransactionContext}; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::{ContractClassV0, ContractClassV1}; @@ -142,6 +142,7 @@ impl BlockContext { block_info: BlockInfo::create_for_testing(), chain_info: ChainInfo::create_for_testing(), versioned_constants: VersionedConstants::create_for_testing(), + bouncer_config: BouncerConfig::max(), concurrency_mode: false, } } @@ -151,10 +152,24 @@ impl BlockContext { block_info: BlockInfo::create_for_testing(), chain_info: ChainInfo::create_for_testing(), versioned_constants: VersionedConstants::create_for_account_testing(), + bouncer_config: BouncerConfig::max(), concurrency_mode: false, } } + pub fn create_for_bouncer_testing(max_n_events_in_block: usize) -> Self { + Self { + bouncer_config: BouncerConfig { + block_max_capacity: BouncerWeights { + n_events: max_n_events_in_block, + ..BouncerWeights::max(false) + }, + ..BouncerConfig::default() + }, + ..Self::create_for_testing() + } + } + pub fn create_for_account_testing_with_kzg(use_kzg_da: bool) -> Self { Self { block_info: BlockInfo::create_for_testing_with_kzg(use_kzg_da), @@ -188,40 +203,3 @@ impl ContractClassV1 { Self::try_from_json_string(&raw_contract_class).unwrap() } } - -impl BouncerConfig { - pub fn create_for_testing() -> Self { - Self { - block_max_capacity_with_keccak: BouncerWeights::create_for_testing(true), - block_max_capacity: BouncerWeights::create_for_testing(false), - } - } -} - -impl BouncerWeights { - pub fn create_for_testing(with_keccak: bool) -> Self { - Self { - gas: 2500000, - n_steps: 2500000, - message_segment_length: 3750, - state_diff_size: 20000, - n_events: 10000, - builtin_count: BuiltinCount::create_for_testing(with_keccak), - } - } -} - -impl BuiltinCount { - pub fn create_for_testing(with_keccak: bool) -> Self { - let keccak = if with_keccak { 1220 } else { 0 }; - Self { - bitwise: 39062, - ecdsa: 1220, - ec_op: 2441, - keccak, - pedersen: 78125, - poseidon: 78125, - range_check: 156250, - } - } -} diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 228fa437dd..23526dbb56 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -173,15 +173,12 @@ impl PyBlockExecutor { &self.general_config, &next_block_info, &self.versioned_constants, + self.bouncer_config.clone(), self.tx_executor_config.concurrency_config.enabled, )?; - let tx_executor = TransactionExecutor::new( - state, - block_context, - self.bouncer_config.clone(), - self.tx_executor_config.clone(), - ); + let tx_executor = + TransactionExecutor::new(state, block_context, self.tx_executor_config.clone()); self.tx_executor = Some(tx_executor); Ok(()) @@ -540,6 +537,7 @@ fn pre_process_block( general_config: &PyGeneralConfig, block_info: &PyBlockInfo, versioned_constants: &VersionedConstants, + bouncer_config: BouncerConfig, concurrency_mode: bool, ) -> NativeBlockifierResult { let old_block_number_and_hash = old_block_number_and_hash @@ -563,6 +561,7 @@ fn pre_process_block( block_info, chain_info, versioned_constants.clone(), + bouncer_config, concurrency_mode, )?; diff --git a/crates/native_blockifier/src/py_validator.rs b/crates/native_blockifier/src/py_validator.rs index 635b0bd52e..e602532ee1 100644 --- a/crates/native_blockifier/src/py_validator.rs +++ b/crates/native_blockifier/src/py_validator.rs @@ -1,5 +1,4 @@ use blockifier::blockifier::stateful_validator::StatefulValidator; -use blockifier::bouncer::BouncerConfig; use blockifier::context::BlockContext; use blockifier::state::cached_state::CachedState; use blockifier::versioned_constants::VersionedConstants; @@ -46,12 +45,8 @@ impl PyValidator { // Create the stateful validator. let max_nonce_for_validation_skip = Nonce(max_nonce_for_validation_skip.0); - let stateful_validator = StatefulValidator::create( - state, - block_context, - max_nonce_for_validation_skip, - BouncerConfig::max(), - ); + let stateful_validator = + StatefulValidator::create(state, block_context, max_nonce_for_validation_skip); Ok(Self { stateful_validator }) }