From 9636c7311362d1f9236ee46db62e2afd43176d41 Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Thu, 7 Mar 2024 11:18:50 +0200 Subject: [PATCH] refactor(execution): bouncer update function --- crates/blockifier/src/bouncer.rs | 137 ++++++++++++-- crates/blockifier/src/bouncer_test.rs | 196 +++++++++++++++++++- crates/blockifier/src/transaction/errors.rs | 4 + 3 files changed, 318 insertions(+), 19 deletions(-) diff --git a/crates/blockifier/src/bouncer.rs b/crates/blockifier/src/bouncer.rs index 8a90af3e43..9f3f7d6a8b 100644 --- a/crates/blockifier/src/bouncer.rs +++ b/crates/blockifier/src/bouncer.rs @@ -15,7 +15,8 @@ use crate::fee::gas_usage::{ }; use crate::state::cached_state::{StateChangesKeys, StorageEntry, TransactionalState}; use crate::state::state_api::StateReader; -use crate::transaction::objects::ResourcesMapping; +use crate::transaction::errors::TransactionExecutionError; +use crate::transaction::objects::{ResourcesMapping, TransactionExecutionInfo}; use crate::utils::usize_from_u128; #[cfg(test)] @@ -38,6 +39,11 @@ macro_rules! impl_checked_sub { pub type HashMapWrapper = HashMap; +pub struct BouncerConfig { + pub block_max_capacity: BouncerWeights, + pub block_max_capacity_with_keccak: BouncerWeights, +} + #[derive( Clone, Copy, Debug, Default, derive_more::Add, derive_more::Sub, Deserialize, PartialEq, )] @@ -66,7 +72,17 @@ impl From for BouncerWeights { fn from(data: ExecutionResources) -> Self { BouncerWeights { n_steps: data.n_steps + data.n_memory_holes, - builtin_count: data.builtin_instance_counter.into(), + builtin_count: BuiltinCount { + pedersen: *data + .builtin_instance_counter + .get(BuiltinName::pedersen.name()) + .unwrap_or(&0), + poseidon: *data + .builtin_instance_counter + .get(BuiltinName::poseidon.name()) + .unwrap_or(&0), + ..Default::default() + }, ..Default::default() } } @@ -111,22 +127,24 @@ impl From for BuiltinCount { } } -#[derive(Clone)] +#[derive(Clone, Default)] pub struct Bouncer { pub executed_class_hashes: HashSet, pub visited_storage_entries: HashSet, pub state_changes_keys: StateChangesKeys, + pub block_contains_keccak: bool, // The capacity is calculated based of the values of the other Bouncer fields. - capacity: BouncerWeights, + available_capacity: BouncerWeights, } impl Bouncer { - pub fn new(capacity: BouncerWeights) -> Self { + pub fn new(available_capacity: BouncerWeights, block_contains_keccak: bool) -> Self { Bouncer { executed_class_hashes: HashSet::new(), state_changes_keys: StateChangesKeys::default(), visited_storage_entries: HashSet::new(), - capacity, + available_capacity, + block_contains_keccak, } } @@ -138,25 +156,112 @@ impl Bouncer { self.executed_class_hashes.extend(other.executed_class_hashes); self.state_changes_keys.extend(&other.state_changes_keys); self.visited_storage_entries.extend(other.visited_storage_entries); - self.capacity = other.capacity; + self.block_contains_keccak = other.block_contains_keccak; + self.available_capacity = other.available_capacity; } } #[derive(Clone)] pub struct TransactionalBouncer { - // The bouncer can be modified only through the merge method. + // The block bouncer can be modified only through the merge method. bouncer: Bouncer, - // The transactional bouncer can be modified only through the update method. + // The transaction bouncer can be modified only through the update method. transactional: Bouncer, } impl TransactionalBouncer { - pub fn new(parent: Bouncer) -> TransactionalBouncer { - let capacity = parent.capacity; - TransactionalBouncer { bouncer: parent, transactional: Bouncer::new(capacity) } + pub fn new(block_bouncer: Bouncer) -> TransactionalBouncer { + let transactional = + Bouncer::new(block_bouncer.available_capacity, block_bouncer.block_contains_keccak); + TransactionalBouncer { bouncer: block_bouncer, transactional } } - // TODO update function (in the next PRs) + /// Updates the bouncer with a new transaction info and weights after execution. + pub fn update( + &mut self, + bouncer_config: &BouncerConfig, + state: &mut TransactionalState<'_, S>, + tx_execution_info: &TransactionExecutionInfo, + l1_handler_payload_size: Option, + ) -> TransactionExecutorResult<()> { + let tx_execution_summary = tx_execution_info.summarize(); + self.update_auxiliary_info(state, &tx_execution_summary)?; + self.update_available_capacity( + bouncer_config, + state, + &tx_execution_summary, + &tx_execution_info.bouncer_resources, + l1_handler_payload_size, + )?; + + Ok(()) + } + + /// This function is called by the update function to update the bouncer capacity. + fn update_available_capacity( + &mut self, + bouncer_config: &BouncerConfig, + state: &mut TransactionalState<'_, S>, + tx_execution_summary: &ExecutionSummary, + bouncer_resources: &ResourcesMapping, + l1_handler_payload_size: Option, + ) -> TransactionExecutorResult<()> { + let tx_weights = self.get_tx_weights( + state, + tx_execution_summary, + bouncer_resources, + l1_handler_payload_size, + )?; + + if !self.transactional.block_contains_keccak && tx_weights.builtin_count.keccak > 0 { + // This is the first transaction to contain keccak in this block. Available capacity + // should be updated according to the keccak max capacity. + self.update_available_capacity_with_keccak(bouncer_config)?; + } + + // Check if the transaction is too large to fit any block. + let mut max_capacity = bouncer_config.block_max_capacity; + if self.transactional.block_contains_keccak { + max_capacity = bouncer_config.block_max_capacity_with_keccak; + } + max_capacity.checked_sub(tx_weights).ok_or(TransactionExecutionError::TxTooLarge)?; + + // Check if the transaction can fit the current block available capacity. + self.transactional.available_capacity = self + .transactional + .available_capacity + .checked_sub(tx_weights) + .ok_or(TransactionExecutionError::BlockFull)?; + Ok(()) + } + + /// Assumption: the block does not contain keccak yet. + fn update_available_capacity_with_keccak( + &mut self, + bouncer_config: &BouncerConfig, + ) -> TransactionExecutorResult<()> { + // First zero the keccak capacity to be able to subtract the max_capacity_with_keccak from + // max_capacity (that is without keccak). + let mut max_capacity_with_keccak_tmp = bouncer_config.block_max_capacity_with_keccak; + max_capacity_with_keccak_tmp.builtin_count.keccak = 0; + // compute the diff between the max_capacity and the max_capacity_with_keccak. + let max_capacity_with_keccak_diff = bouncer_config + .block_max_capacity + .checked_sub(max_capacity_with_keccak_tmp) + .expect("max_capacity_with_keccak should be smaller than max_capacity"); + // Subtract the diff from the available capacity. + self.transactional.available_capacity = self + .transactional + .available_capacity + .checked_sub(max_capacity_with_keccak_diff) + .ok_or(TransactionExecutionError::BlockFull)?; + // Add back the keccack capacity that was reset at the beggining. + self.transactional.available_capacity.builtin_count.keccak = + bouncer_config.block_max_capacity_with_keccak.builtin_count.keccak; + // Mark this block as contains keccak. + self.transactional.block_contains_keccak = true; + Ok(()) + } pub fn get_tx_weights( &mut self, @@ -216,7 +321,7 @@ impl TransactionalBouncer { .expect("n_steps must be present in the execution info") + execution_info_resources .remove(constants::N_MEMORY_HOLES) - .expect("n_memory_hols must be present in the execution info"), + .expect("n_memory_holes must be present in the execution info"), builtin_count: BuiltinCount::from(execution_info_resources), state_diff_size: 0, }; @@ -226,9 +331,11 @@ impl TransactionalBouncer { pub fn update_auxiliary_info( &mut self, - tx_execution_summary: &ExecutionSummary, state: &mut TransactionalState<'_, S>, + tx_execution_summary: &ExecutionSummary, ) -> TransactionExecutorResult<()> { + // TODO(Yael): consider removing the auxiliary_info from the bouncer and using the + // ExecutionSummary directly. self.transactional .executed_class_hashes .extend(&tx_execution_summary.executed_class_hashes); diff --git a/crates/blockifier/src/bouncer_test.rs b/crates/blockifier/src/bouncer_test.rs index 6cfa40e626..22468d3e30 100644 --- a/crates/blockifier/src/bouncer_test.rs +++ b/crates/blockifier/src/bouncer_test.rs @@ -1,6 +1,19 @@ +use std::collections::HashMap; use std::ops::Sub; +use assert_matches::assert_matches; +use cairo_vm::serde::deserialize_program::BuiltinName; + +use super::BouncerConfig; +use crate::abi::constants; +use crate::blockifier::transaction_executor::TransactionExecutorError; use crate::bouncer::{Bouncer, BouncerWeights, BuiltinCount}; +use crate::context::BlockContext; +use crate::execution::call_info::ExecutionSummary; +use crate::state::cached_state::CachedState; +use crate::test_utils::initial_test_state::test_state; +use crate::transaction::errors::TransactionExecutionError; +use crate::transaction::objects::ResourcesMapping; #[test] fn test_block_weights_sub_checked() { @@ -99,15 +112,190 @@ fn test_transactional_bouncer() { state_diff_size: 2, }; - let bouncer = Bouncer::new(initial_bouncer_weights); + let bouncer = Bouncer::new(initial_bouncer_weights, true); let mut transactional_bouncer = bouncer.create_transactional(); - transactional_bouncer.transactional.capacity = weights_to_commit; + transactional_bouncer.transactional.available_capacity = weights_to_commit; // Test transactional bouncer abort. let final_weights = transactional_bouncer.clone().abort(); - assert!(final_weights.capacity == initial_bouncer_weights); + assert!(final_weights.available_capacity == initial_bouncer_weights); // Test transactional bouncer commit. let final_weights = transactional_bouncer.commit(); - assert!(final_weights.capacity == weights_to_commit); + assert!(final_weights.available_capacity == weights_to_commit); +} + +#[test] +fn test_update_capcity() { + let state = &mut test_state(&BlockContext::create_for_account_testing().chain_info, 0, &[]); + let mut transactional_state = CachedState::create_transactional(state); + + let execution_summary = ExecutionSummary { ..Default::default() }; + let bouncer = Bouncer { + available_capacity: BouncerWeights { + builtin_count: BuiltinCount { + bitwise: 10, + ecdsa: 10, + ec_op: 10, + keccak: 0, + pedersen: 10, + poseidon: 10, + range_check: 10, + }, + gas: 10, + message_segment_length: 10, + n_steps: 10, + n_events: 10, + state_diff_size: 10, + }, + block_contains_keccak: false, + ..Default::default() + }; + let mut transactional_bouncer = bouncer.create_transactional(); + + let block_max_capacity = BouncerWeights { + builtin_count: BuiltinCount { + bitwise: 20, + ecdsa: 20, + ec_op: 20, + keccak: 0, + pedersen: 20, + poseidon: 20, + range_check: 20, + }, + gas: 20, + message_segment_length: 20, + n_steps: 20, + n_events: 20, + state_diff_size: 20, + }; + let mut block_max_capacity_with_keccak = block_max_capacity; + block_max_capacity_with_keccak.builtin_count.keccak = 1; + + let mut bouncer_config = BouncerConfig { block_max_capacity, block_max_capacity_with_keccak }; + + let mut bouncer_resources: HashMap = HashMap::new(); + bouncer_resources.insert(BuiltinName::bitwise.name().to_string(), 1); + bouncer_resources.insert(BuiltinName::ecdsa.name().to_string(), 1); + bouncer_resources.insert(BuiltinName::ec_op.name().to_string(), 1); + // Keccak is 0 since it has special handling which will be tested separately. + bouncer_resources.insert(BuiltinName::keccak.name().to_string(), 0); + bouncer_resources.insert(BuiltinName::pedersen.name().to_string(), 1); + bouncer_resources.insert(BuiltinName::poseidon.name().to_string(), 1); + bouncer_resources.insert(BuiltinName::range_check.name().to_string(), 1); + bouncer_resources.insert(constants::BLOB_GAS_USAGE.to_string(), 1); + bouncer_resources.insert(constants::L1_GAS_USAGE.to_string(), 1); + bouncer_resources.insert(constants::N_STEPS_RESOURCE.to_string(), 1); + bouncer_resources.insert(constants::N_MEMORY_HOLES.to_string(), 1); + let mut bouncer_resources = ResourcesMapping(bouncer_resources); + + // Test for success. + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!(result, Ok(())); + + // Test for BlockFull error. + bouncer_resources.0.insert(BuiltinName::bitwise.name().to_string(), 10); + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!( + result, + Err(TransactionExecutorError::TransactionExecutionError( + TransactionExecutionError::BlockFull + )) + ); + + // Test for TxTooLarge error. + bouncer_resources.0.insert(BuiltinName::bitwise.name().to_string(), 21); + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!( + result, + Err(TransactionExecutorError::TransactionExecutionError( + TransactionExecutionError::TxTooLarge + )) + ); + bouncer_resources.0.insert(BuiltinName::bitwise.name().to_string(), 1); + + // Test a transaction with keccak. + bouncer_resources.0.insert(BuiltinName::keccak.name().to_string(), 1); + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!(result, Ok(())); + + // Test for BlockFull error with keccak, transaction weights exceeds limit. + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!( + result, + Err(TransactionExecutorError::TransactionExecutionError( + TransactionExecutionError::BlockFull + )) + ); + + // Test for BlockFull error with keccak, keccak_max_capacity is too low. + let bouncer = Bouncer { + available_capacity: BouncerWeights { + builtin_count: BuiltinCount { + bitwise: 5, + ecdsa: 10, + ec_op: 10, + keccak: 0, + pedersen: 10, + poseidon: 10, + range_check: 10, + }, + gas: 10, + message_segment_length: 10, + n_steps: 10, + n_events: 10, + state_diff_size: 10, + }, + block_contains_keccak: true, + ..Default::default() + }; + let mut transactional_bouncer = bouncer.create_transactional(); + // Reduce the max capacity of bitwise for keccak, so the already used bitwise will not fit in + // the block once the capacity is updated with keccak. + bouncer_config.block_max_capacity_with_keccak.builtin_count.bitwise = 10; + // There is one keccak usage so this resource should fit into a keccak block. + bouncer_resources.0.insert(BuiltinName::keccak.name().to_string(), 1); + let result = transactional_bouncer.update_available_capacity( + &bouncer_config, + &mut transactional_state, + &execution_summary, + &bouncer_resources, + None, + ); + assert_matches!( + result, + Err(TransactionExecutorError::TransactionExecutionError( + TransactionExecutionError::BlockFull + )) + ); } diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index 83ee24aa13..c84005f579 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -51,6 +51,8 @@ pub enum TransactionFeeError { #[derive(Debug, Error)] pub enum TransactionExecutionError { + #[error("Transaction cannot be added to the current block, block capacity reached.")] + BlockFull, #[error( "Declare transaction version {declare_version:?} must have a contract class of Cairo \ version {cairo_version:?}." @@ -85,6 +87,8 @@ pub enum TransactionExecutionError { TransactionPreValidationError(#[from] TransactionPreValidationError), #[error(transparent)] TryFromIntError(#[from] std::num::TryFromIntError), + #[error("Transaction size exceeds the maximum block capacity.")] + TxTooLarge, // TODO(Zuphit): add `gen_transaction_execution_error_trace` if needed. #[error("Transaction validation has failed: {error}")] ValidateTransactionError { error: EntryPointExecutionError, storage_address: ContractAddress },