From 6f09863c5189da80065bf6112e29a30b7f765858 Mon Sep 17 00:00:00 2001 From: Yoni <78365039+Yoni-Starkware@users.noreply.github.com> Date: Thu, 26 Dec 2024 14:47:32 +0200 Subject: [PATCH] chore(blockifier, native_blockifier, starknet_batcher): remove visited PCS (#2974) --- .../src/blockifier/transaction_executor.rs | 38 ++------------ .../blockifier/src/concurrency/fee_utils.rs | 2 +- .../blockifier/src/concurrency/flow_test.rs | 14 ++---- .../src/concurrency/versioned_state.rs | 17 ++----- .../src/concurrency/versioned_state_test.rs | 21 +++----- .../src/concurrency/worker_logic.rs | 24 +++------ .../src/concurrency/worker_logic_test.rs | 3 -- .../src/execution/entry_point_execution.rs | 50 +------------------ crates/blockifier/src/state/cached_state.rs | 28 +---------- crates/blockifier/src/state/state_api.rs | 14 +----- .../src/state_reader/utils.rs | 3 +- .../src/py_block_executor.rs | 18 ++----- crates/starknet_batcher/src/block_builder.rs | 6 +-- .../src/block_builder_test.rs | 8 +-- crates/starknet_batcher/src/test_utils.rs | 2 - .../src/transaction_executor.rs | 10 +--- 16 files changed, 37 insertions(+), 221 deletions(-) diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 79bba304be..edb4bc89d7 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -1,11 +1,9 @@ -use std::collections::{HashMap, HashSet}; use std::panic::{self, catch_unwind, AssertUnwindSafe}; use std::sync::{Arc, Mutex}; use itertools::FoldWhile::{Continue, Done}; use itertools::Itertools; use starknet_api::block::BlockHashAndNumber; -use starknet_api::core::ClassHash; use thiserror::Error; use crate::blockifier::block::pre_process_block; @@ -39,7 +37,6 @@ pub enum TransactionExecutorError { } pub type TransactionExecutorResult = Result; -pub type VisitedSegmentsMapping = Vec<(ClassHash, Vec)>; /// A transaction executor, used for building a single block. pub struct TransactionExecutor { @@ -141,32 +138,9 @@ impl TransactionExecutor { results } - /// Returns the state diff, a list of contract class hash with the corresponding list of - /// visited segment values and the block weights. + /// Returns the state diff and the block weights. // TODO(Yoav): Consume "self". - pub fn finalize( - &mut self, - ) -> TransactionExecutorResult<(CommitmentStateDiff, VisitedSegmentsMapping, BouncerWeights)> - { - // Get the visited segments of each contract class. - // This is done by taking all the visited PCs of each contract, and compress them to one - // representative for each visited segment. - let visited_segments = self - .block_state - .as_ref() - .expect(BLOCK_STATE_ACCESS_ERR) - .visited_pcs - .iter() - .map(|(class_hash, class_visited_pcs)| -> TransactionExecutorResult<_> { - let contract_class = self - .block_state - .as_ref() - .expect(BLOCK_STATE_ACCESS_ERR) - .get_compiled_class(*class_hash)?; - Ok((*class_hash, contract_class.get_visited_segments(class_visited_pcs)?)) - }) - .collect::>()?; - + pub fn finalize(&mut self) -> TransactionExecutorResult<(CommitmentStateDiff, BouncerWeights)> { log::debug!("Final block weights: {:?}.", self.bouncer.get_accumulated_weights()); let mut block_state = self.block_state.take().expect(BLOCK_STATE_ACCESS_ERR); let state_diff = if self.block_context.versioned_constants.enable_stateful_compression { @@ -181,7 +155,7 @@ impl TransactionExecutor { } else { block_state.to_state_diff()?.state_maps }; - Ok((state_diff.into(), visited_segments, *self.bouncer.get_accumulated_weights())) + Ok((state_diff.into(), *self.bouncer.get_accumulated_weights())) } } @@ -276,7 +250,6 @@ impl TransactionExecutor { let n_committed_txs = worker_executor.scheduler.get_n_committed_txs(); let mut tx_execution_results = Vec::new(); - let mut visited_pcs: HashMap> = HashMap::new(); for execution_output in worker_executor.execution_outputs.iter() { if tx_execution_results.len() >= n_committed_txs { break; @@ -288,9 +261,6 @@ impl TransactionExecutor { .expect("Output must be ready."); tx_execution_results .push(locked_execution_output.result.map_err(TransactionExecutorError::from)); - for (class_hash, class_visited_pcs) in locked_execution_output.visited_pcs { - visited_pcs.entry(class_hash).or_default().extend(class_visited_pcs); - } } let block_state_after_commit = Arc::try_unwrap(worker_executor) @@ -301,7 +271,7 @@ impl TransactionExecutor { it." ) }) - .commit_chunk_and_recover_block_state(n_committed_txs, visited_pcs); + .commit_chunk_and_recover_block_state(n_committed_txs); self.block_state.replace(block_state_after_commit); tx_execution_results diff --git a/crates/blockifier/src/concurrency/fee_utils.rs b/crates/blockifier/src/concurrency/fee_utils.rs index 8ea34ea065..5188185277 100644 --- a/crates/blockifier/src/concurrency/fee_utils.rs +++ b/crates/blockifier/src/concurrency/fee_utils.rs @@ -118,5 +118,5 @@ pub fn add_fee_to_sequencer_balance( ]), ..StateMaps::default() }; - state.apply_writes(&writes, &ContractClassMapping::default(), &HashMap::default()); + state.apply_writes(&writes, &ContractClassMapping::default()); } diff --git a/crates/blockifier/src/concurrency/flow_test.rs b/crates/blockifier/src/concurrency/flow_test.rs index 5b828c3260..15f0af4f50 100644 --- a/crates/blockifier/src/concurrency/flow_test.rs +++ b/crates/blockifier/src/concurrency/flow_test.rs @@ -48,11 +48,7 @@ fn scheduler_flow_test( Task::ExecutionTask(tx_index), &versioned_state, ); - state_proxy.apply_writes( - &new_writes, - &ContractClassMapping::default(), - &HashMap::default(), - ); + state_proxy.apply_writes(&new_writes, &ContractClassMapping::default()); scheduler.finish_execution_during_commit(tx_index); } } @@ -61,11 +57,9 @@ fn scheduler_flow_test( Task::ExecutionTask(tx_index) => { let (_, writes) = get_reads_writes_for(Task::ExecutionTask(tx_index), &versioned_state); - versioned_state.pin_version(tx_index).apply_writes( - &writes, - &ContractClassMapping::default(), - &HashMap::default(), - ); + versioned_state + .pin_version(tx_index) + .apply_writes(&writes, &ContractClassMapping::default()); scheduler.finish_execution(tx_index); Task::AskForTask } diff --git a/crates/blockifier/src/concurrency/versioned_state.rs b/crates/blockifier/src/concurrency/versioned_state.rs index 1d3c9a9270..65ea81cd7c 100644 --- a/crates/blockifier/src/concurrency/versioned_state.rs +++ b/crates/blockifier/src/concurrency/versioned_state.rs @@ -1,4 +1,3 @@ -use std::collections::{HashMap, HashSet}; use std::sync::{Arc, Mutex, MutexGuard}; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; @@ -199,11 +198,7 @@ impl VersionedState { } impl VersionedState { - pub fn commit_chunk_and_recover_block_state( - mut self, - n_committed_txs: usize, - visited_pcs: HashMap>, - ) -> U { + pub fn commit_chunk_and_recover_block_state(mut self, n_committed_txs: usize) -> U { if n_committed_txs == 0 { return self.into_initial_state(); } @@ -212,7 +207,7 @@ impl VersionedState { let class_hash_to_class = self.compiled_contract_classes.get_writes_up_to_index(commit_index); let mut state = self.into_initial_state(); - state.apply_writes(&writes, &class_hash_to_class, &visited_pcs); + state.apply_writes(&writes, &class_hash_to_class); state } } @@ -271,14 +266,8 @@ impl VersionedStateProxy { } } -// TODO(Noa, 15/5/24): Consider using visited_pcs. impl UpdatableState for VersionedStateProxy { - fn apply_writes( - &mut self, - writes: &StateMaps, - class_hash_to_class: &ContractClassMapping, - _visited_pcs: &HashMap>, - ) { + fn apply_writes(&mut self, writes: &StateMaps, class_hash_to_class: &ContractClassMapping) { self.state().apply_writes(self.tx_index, writes, class_hash_to_class) } } diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index 4c09b2cacd..a76beafa85 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/crates/blockifier/src/concurrency/versioned_state_test.rs @@ -441,7 +441,6 @@ fn test_apply_writes( safe_versioned_state.pin_version(0).apply_writes( &transactional_states[0].cache.borrow().writes, &transactional_states[0].class_hash_to_class.borrow().clone(), - &HashMap::default(), ); assert!(transactional_states[1].get_class_hash_at(contract_address).unwrap() == class_hash_0); assert!(transactional_states[1].get_compiled_class(class_hash).unwrap() == contract_class_0); @@ -470,7 +469,6 @@ fn test_apply_writes_reexecute_scenario( safe_versioned_state.pin_version(0).apply_writes( &transactional_states[0].cache.borrow().writes, &transactional_states[0].class_hash_to_class.borrow().clone(), - &HashMap::default(), ); // Although transaction 0 wrote to the shared state, version 1 needs to be re-executed to see // the new value (its read value has already been cached). @@ -516,11 +514,9 @@ fn test_delete_writes( feature_contract.get_runnable_class(), ) .unwrap(); - safe_versioned_state.pin_version(i).apply_writes( - &tx_state.cache.borrow().writes, - &tx_state.class_hash_to_class.borrow(), - &HashMap::default(), - ); + safe_versioned_state + .pin_version(i) + .apply_writes(&tx_state.cache.borrow().writes, &tx_state.class_hash_to_class.borrow()); } safe_versioned_state.pin_version(tx_index_to_delete_writes).delete_writes( @@ -579,11 +575,7 @@ fn test_delete_writes_completeness( let tx_index = 0; let mut versioned_state_proxy = safe_versioned_state.pin_version(tx_index); - versioned_state_proxy.apply_writes( - &state_maps_writes, - &class_hash_to_class_writes, - &HashMap::default(), - ); + versioned_state_proxy.apply_writes(&state_maps_writes, &class_hash_to_class_writes); assert_eq!( safe_versioned_state.0.lock().unwrap().get_writes_of_index(tx_index), state_maps_writes @@ -658,9 +650,8 @@ fn test_versioned_proxy_state_flow( for proxy in versioned_proxy_states { drop(proxy); } - let modified_block_state = safe_versioned_state - .into_inner_state() - .commit_chunk_and_recover_block_state(4, HashMap::new()); + let modified_block_state = + safe_versioned_state.into_inner_state().commit_chunk_and_recover_block_state(4); assert!(modified_block_state.get_class_hash_at(contract_address).unwrap() == class_hash_3); assert!(modified_block_state.get_compiled_class(class_hash).unwrap() == contract_class_2); diff --git a/crates/blockifier/src/concurrency/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index 851a462f31..76dc7313a5 100644 --- a/crates/blockifier/src/concurrency/worker_logic.rs +++ b/crates/blockifier/src/concurrency/worker_logic.rs @@ -1,11 +1,9 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::fmt::Debug; use std::sync::Mutex; use std::thread; use std::time::Duration; -use starknet_api::core::ClassHash; - use super::versioned_state::VersionedState; use crate::blockifier::transaction_executor::TransactionExecutorError; use crate::bouncer::Bouncer; @@ -33,7 +31,6 @@ pub struct ExecutionTaskOutput { // TODO(Yoni): rename to state_diff. pub writes: StateMaps, pub contract_classes: ContractClassMapping, - pub visited_pcs: HashMap>, pub result: TransactionExecutionResult, } @@ -123,6 +120,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { fn execute_tx(&self, tx_index: TxIndex) { let mut tx_versioned_state = self.state.pin_version(tx_index); let tx = &self.chunk[tx_index]; + // TODO(Yoni): is it necessary to use a transactional state here? let mut transactional_state = TransactionalState::create_transactional(&mut tx_versioned_state); let concurrency_mode = true; @@ -135,23 +133,19 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { let tx_reads_writes = transactional_state.cache.take(); let writes = tx_reads_writes.to_state_diff().state_maps; let contract_classes = transactional_state.class_hash_to_class.take(); - let visited_pcs = transactional_state.visited_pcs; - // The versioned state does not carry the visited PCs. - tx_versioned_state.apply_writes(&writes, &contract_classes, &HashMap::default()); + tx_versioned_state.apply_writes(&writes, &contract_classes); ExecutionTaskOutput { reads: tx_reads_writes.initial_reads, writes, contract_classes, - visited_pcs, result: execution_result, } } Err(_) => ExecutionTaskOutput { reads: transactional_state.cache.take().initial_reads, - // Failed transaction - ignore the writes and visited PCs. + // Failed transaction - ignore the writes. writes: StateMaps::default(), contract_classes: HashMap::default(), - visited_pcs: HashMap::default(), result: execution_result, }, }; @@ -261,13 +255,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { } impl WorkerExecutor<'_, U> { - pub fn commit_chunk_and_recover_block_state( - self, - n_committed_txs: usize, - visited_pcs: HashMap>, - ) -> U { - self.state - .into_inner_state() - .commit_chunk_and_recover_block_state(n_committed_txs, visited_pcs) + pub fn commit_chunk_and_recover_block_state(self, n_committed_txs: usize) -> U { + self.state.into_inner_state().commit_chunk_and_recover_block_state(n_committed_txs) } } diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index e75fdd3a57..873f28b627 100644 --- a/crates/blockifier/src/concurrency/worker_logic_test.rs +++ b/crates/blockifier/src/concurrency/worker_logic_test.rs @@ -385,7 +385,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) { assert_eq!(execution_output.writes, writes.diff(&reads)); assert_eq!(execution_output.reads, reads); - assert_ne!(execution_output.visited_pcs, HashMap::default()); // Failed execution. let tx_index = 1; @@ -404,7 +403,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) { }; assert_eq!(execution_output.reads, reads); assert_eq!(execution_output.writes, StateMaps::default()); - assert_eq!(execution_output.visited_pcs, HashMap::default()); // Reverted execution. let tx_index = 2; @@ -418,7 +416,6 @@ fn test_worker_execute(default_all_resource_bounds: ValidResourceBounds) { let execution_output = execution_output.as_ref().unwrap(); assert!(execution_output.result.as_ref().unwrap().is_reverted()); assert_ne!(execution_output.writes, StateMaps::default()); - assert_ne!(execution_output.visited_pcs, HashMap::default()); // Validate status change. for tx_index in 0..3 { diff --git a/crates/blockifier/src/execution/entry_point_execution.rs b/crates/blockifier/src/execution/entry_point_execution.rs index 0667962145..0297de89ee 100644 --- a/crates/blockifier/src/execution/entry_point_execution.rs +++ b/crates/blockifier/src/execution/entry_point_execution.rs @@ -1,5 +1,3 @@ -use std::collections::HashSet; - use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::types::layout_name::LayoutName; use cairo_vm::types::relocatable::{MaybeRelocatable, Relocatable}; @@ -61,11 +59,6 @@ pub fn execute_entry_point_call( state: &mut dyn State, context: &mut EntryPointExecutionContext, ) -> EntryPointExecutionResult { - // Fetch the class hash from `call`. - let class_hash = call.class_hash.ok_or(EntryPointExecutionError::InternalError( - "Class hash must not be None when executing an entry point.".into(), - ))?; - let tracked_resource = *context.tracked_resource_stack.last().expect("Unexpected empty tracked resource."); let VmExecutionContext { @@ -90,15 +83,6 @@ pub fn execute_entry_point_call( let program_segment_size = bytecode_length + program_extra_data_length; run_entry_point(&mut runner, &mut syscall_handler, entry_point, args, program_segment_size)?; - // Collect the set PC values that were visited during the entry point execution. - register_visited_pcs( - &mut runner, - syscall_handler.base.state, - class_hash, - program_segment_size, - bytecode_length, - )?; - Ok(finalize_execution( runner, syscall_handler, @@ -108,38 +92,6 @@ pub fn execute_entry_point_call( )?) } -// Collects the set PC values that were visited during the entry point execution. -fn register_visited_pcs( - runner: &mut CairoRunner, - state: &mut dyn State, - class_hash: starknet_api::core::ClassHash, - program_segment_size: usize, - bytecode_length: usize, -) -> EntryPointExecutionResult<()> { - let mut class_visited_pcs = HashSet::new(); - // Relocate the trace, putting the program segment at address 1 and the execution segment right - // after it. - // TODO(lior): Avoid unnecessary relocation once the VM has a non-relocated `get_trace()` - // function. - runner.relocate_trace(&[1, 1 + program_segment_size])?; - for trace_entry in runner.relocated_trace.as_ref().expect("Relocated trace not found") { - let pc = trace_entry.pc; - if pc < 1 { - return Err(EntryPointExecutionError::InternalError(format!( - "Invalid PC value {pc} in trace." - ))); - } - let real_pc = pc - 1; - // Jumping to a PC that is not inside the bytecode is possible. For example, to obtain - // the builtin costs. Filter out these values. - if real_pc < bytecode_length { - class_visited_pcs.insert(real_pc); - } - } - state.add_visited_pcs(class_hash, &class_visited_pcs); - Ok(()) -} - pub fn initialize_execution_context<'a>( call: CallEntryPoint, compiled_class: &'a CompiledClassV1, @@ -150,7 +102,7 @@ pub fn initialize_execution_context<'a>( // Instantiate Cairo runner. let proof_mode = false; - let trace_enabled = true; + let trace_enabled = false; let mut runner = CairoRunner::new( &compiled_class.0.program, LayoutName::starknet, diff --git a/crates/blockifier/src/state/cached_state.rs b/crates/blockifier/src/state/cached_state.rs index 92fed01eeb..5fc582edc4 100644 --- a/crates/blockifier/src/state/cached_state.rs +++ b/crates/blockifier/src/state/cached_state.rs @@ -32,8 +32,6 @@ pub struct CachedState { // Using interior mutability to update caches during `State`'s immutable getters. pub(crate) cache: RefCell, pub(crate) class_hash_to_class: RefCell, - /// A map from class hash to the set of PC values that were visited in the class. - pub visited_pcs: HashMap>, } impl CachedState { @@ -42,7 +40,6 @@ impl CachedState { state, cache: RefCell::new(StateCache::default()), class_hash_to_class: RefCell::new(HashMap::default()), - visited_pcs: HashMap::default(), } } @@ -78,12 +75,6 @@ impl CachedState { self.class_hash_to_class.get_mut().extend(local_contract_cache_updates); } - pub fn update_visited_pcs_cache(&mut self, visited_pcs: &HashMap>) { - for (class_hash, class_visited_pcs) in visited_pcs { - self.add_visited_pcs(*class_hash, class_visited_pcs); - } - } - /// Updates cache with initial cell values for write-only access. /// If written values match the original, the cell is unchanged and not counted as a /// storage-change for fee calculation. @@ -112,15 +103,9 @@ impl CachedState { } impl UpdatableState for CachedState { - fn apply_writes( - &mut self, - writes: &StateMaps, - class_hash_to_class: &ContractClassMapping, - visited_pcs: &HashMap>, - ) { + fn apply_writes(&mut self, writes: &StateMaps, class_hash_to_class: &ContractClassMapping) { // TODO(Noa,15/5/24): Reconsider the clone. self.update_cache(writes, class_hash_to_class.clone()); - self.update_visited_pcs_cache(visited_pcs); } } @@ -278,10 +263,6 @@ impl State for CachedState { self.cache.get_mut().set_compiled_class_hash_write(class_hash, compiled_class_hash); Ok(()) } - - fn add_visited_pcs(&mut self, class_hash: ClassHash, pcs: &HashSet) { - self.visited_pcs.entry(class_hash).or_default().extend(pcs); - } } #[cfg(any(feature = "testing", test))] @@ -291,7 +272,6 @@ impl Default for CachedState TransactionalState<'_, U> { pub fn commit(self) { let state = self.state.0; let child_cache = self.cache.into_inner(); - state.apply_writes( - &child_cache.writes, - &self.class_hash_to_class.into_inner(), - &self.visited_pcs, - ) + state.apply_writes(&child_cache.writes, &self.class_hash_to_class.into_inner()) } } diff --git a/crates/blockifier/src/state/state_api.rs b/crates/blockifier/src/state/state_api.rs index 96e24c1e1a..074d7835c5 100644 --- a/crates/blockifier/src/state/state_api.rs +++ b/crates/blockifier/src/state/state_api.rs @@ -1,5 +1,3 @@ -use std::collections::{HashMap, HashSet}; - use starknet_api::abi::abi_utils::get_fee_token_var_address; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; use starknet_api::state::StorageKey; @@ -102,19 +100,9 @@ pub trait State: StateReader { class_hash: ClassHash, compiled_class_hash: CompiledClassHash, ) -> StateResult<()>; - - /// Marks the given set of PC values as visited for the given class hash. - // TODO(lior): Once we have a BlockResources object, move this logic there. Make sure reverted - // entry points do not affect the final set of PCs. - fn add_visited_pcs(&mut self, _class_hash: ClassHash, _pcs: &HashSet) {} } /// A class defining the API for updating a state with transactions writes. pub trait UpdatableState: StateReader { - fn apply_writes( - &mut self, - writes: &StateMaps, - class_hash_to_class: &ContractClassMapping, - visited_pcs: &HashMap>, - ); + fn apply_writes(&mut self, writes: &StateMaps, class_hash_to_class: &ContractClassMapping); } diff --git a/crates/blockifier_reexecution/src/state_reader/utils.rs b/crates/blockifier_reexecution/src/state_reader/utils.rs index d456581fab..46faf39dc7 100644 --- a/crates/blockifier_reexecution/src/state_reader/utils.rs +++ b/crates/blockifier_reexecution/src/state_reader/utils.rs @@ -237,8 +237,7 @@ pub fn reexecute_and_verify_correctness< let block_state = transaction_executor.block_state.clone(); // Finalize block and read actual statediff. - let (actual_state_diff, _, _) = - transaction_executor.finalize().expect("Couldn't finalize block"); + let (actual_state_diff, _) = transaction_executor.finalize().expect("Couldn't finalize block"); assert_eq_state_diff!(expected_state_diff, actual_state_diff); diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 58bcc6f9e6..6bf6694aff 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -43,7 +43,6 @@ use crate::storage::{ }; pub(crate) type RawTransactionExecutionResult = Vec; -pub(crate) type PyVisitedSegmentsMapping = Vec<(PyFelt, Vec)>; #[cfg(test)] #[path = "py_block_executor_test.rs"] @@ -254,19 +253,10 @@ impl PyBlockExecutor { }) } - /// Returns the state diff, a list of contract class hash with the corresponding list of - /// visited segment values and the block weights. - pub fn finalize( - &mut self, - ) -> NativeBlockifierResult<(PyStateDiff, PyVisitedSegmentsMapping, Py)> { + /// Returns the state diff and the block weights. + pub fn finalize(&mut self) -> NativeBlockifierResult<(PyStateDiff, Py)> { log::debug!("Finalizing execution..."); - let (commitment_state_diff, visited_pcs, block_weights) = self.tx_executor().finalize()?; - let visited_pcs = visited_pcs - .into_iter() - .map(|(class_hash, class_visited_pcs_vec)| { - (PyFelt::from(class_hash), class_visited_pcs_vec) - }) - .collect(); + let (commitment_state_diff, block_weights) = self.tx_executor().finalize()?; let py_state_diff = PyStateDiff::from(commitment_state_diff); let serialized_block_weights = @@ -276,7 +266,7 @@ impl PyBlockExecutor { log::debug!("Finalized execution."); - Ok((py_state_diff, visited_pcs, raw_block_weights)) + Ok((py_state_diff, raw_block_weights)) } // Storage Alignment API. diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index 7fc3595c08..c0ccf46e66 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -6,7 +6,6 @@ use blockifier::blockifier::transaction_executor::{ TransactionExecutor, TransactionExecutorError as BlockifierTransactionExecutorError, TransactionExecutorResult, - VisitedSegmentsMapping, }; use blockifier::bouncer::{BouncerConfig, BouncerWeights}; use blockifier::context::{BlockContext, ChainInfo}; @@ -67,7 +66,6 @@ pub enum FailOnErrorCause { pub struct BlockExecutionArtifacts { pub execution_infos: IndexMap, pub commitment_state_diff: CommitmentStateDiff, - pub visited_segments_mapping: VisitedSegmentsMapping, pub bouncer_weights: BouncerWeights, pub l2_gas_used: GasAmount, } @@ -167,12 +165,10 @@ impl BlockBuilderTrait for BlockBuilder { ) .await?; } - let (commitment_state_diff, visited_segments_mapping, bouncer_weights) = - self.executor.close_block()?; + let (commitment_state_diff, bouncer_weights) = self.executor.close_block()?; Ok(BlockExecutionArtifacts { execution_infos, commitment_state_diff, - visited_segments_mapping, bouncer_weights, l2_gas_used, }) diff --git a/crates/starknet_batcher/src/block_builder_test.rs b/crates/starknet_batcher/src/block_builder_test.rs index 3c36cff083..bf1d8f38ae 100644 --- a/crates/starknet_batcher/src/block_builder_test.rs +++ b/crates/starknet_batcher/src/block_builder_test.rs @@ -53,7 +53,6 @@ fn block_execution_artifacts( BlockExecutionArtifacts { execution_infos, commitment_state_diff: Default::default(), - visited_segments_mapping: Default::default(), bouncer_weights: BouncerWeights { l1_gas: 100, ..BouncerWeights::empty() }, // Each mock transaction uses 1 L2 gas so the total amount should be the number of txs. l2_gas_used, @@ -264,7 +263,6 @@ fn transaction_failed_test_expectations() -> TestExpectations { mock_transaction_executor.expect_close_block().times(1).return_once(move || { Ok(( expected_block_artifacts_copy.commitment_state_diff, - expected_block_artifacts_copy.visited_segments_mapping, expected_block_artifacts_copy.bouncer_weights, )) }); @@ -295,11 +293,7 @@ fn set_close_block_expectations( let output_block_artifacts = block_builder_expected_output(block_size); let output_block_artifacts_copy = output_block_artifacts.clone(); mock_transaction_executor.expect_close_block().times(1).return_once(move || { - Ok(( - output_block_artifacts.commitment_state_diff, - output_block_artifacts.visited_segments_mapping, - output_block_artifacts.bouncer_weights, - )) + Ok((output_block_artifacts.commitment_state_diff, output_block_artifacts.bouncer_weights)) }); output_block_artifacts_copy } diff --git a/crates/starknet_batcher/src/test_utils.rs b/crates/starknet_batcher/src/test_utils.rs index d6329b0084..e7f4f35cd8 100644 --- a/crates/starknet_batcher/src/test_utils.rs +++ b/crates/starknet_batcher/src/test_utils.rs @@ -1,6 +1,5 @@ use std::ops::Range; -use blockifier::blockifier::transaction_executor::VisitedSegmentsMapping; use blockifier::bouncer::BouncerWeights; use blockifier::state::cached_state::CommitmentStateDiff; use indexmap::IndexMap; @@ -36,7 +35,6 @@ impl BlockExecutionArtifacts { class_hash_to_compiled_class_hash: IndexMap::new(), address_to_nonce: IndexMap::from_iter([(contract_address!("0x7"), nonce!(1_u64))]), }, - visited_segments_mapping: VisitedSegmentsMapping::default(), bouncer_weights: BouncerWeights::empty(), l2_gas_used: GasAmount::default(), } diff --git a/crates/starknet_batcher/src/transaction_executor.rs b/crates/starknet_batcher/src/transaction_executor.rs index a781f81bd1..106689579d 100644 --- a/crates/starknet_batcher/src/transaction_executor.rs +++ b/crates/starknet_batcher/src/transaction_executor.rs @@ -1,7 +1,6 @@ use blockifier::blockifier::transaction_executor::{ TransactionExecutor, TransactionExecutorResult, - VisitedSegmentsMapping, }; use blockifier::bouncer::BouncerWeights; use blockifier::state::cached_state::CommitmentStateDiff; @@ -17,9 +16,7 @@ pub trait TransactionExecutorTrait: Send { &mut self, txs: &[BlockifierTransaction], ) -> Vec>; - fn close_block( - &mut self, - ) -> TransactionExecutorResult<(CommitmentStateDiff, VisitedSegmentsMapping, BouncerWeights)>; + fn close_block(&mut self) -> TransactionExecutorResult<(CommitmentStateDiff, BouncerWeights)>; } impl TransactionExecutorTrait for TransactionExecutor { @@ -32,10 +29,7 @@ impl TransactionExecutorTrait for TransactionExecu } /// Finalizes the block creation and returns the commitment state diff, visited /// segments mapping and bouncer. - fn close_block( - &mut self, - ) -> TransactionExecutorResult<(CommitmentStateDiff, VisitedSegmentsMapping, BouncerWeights)> - { + fn close_block(&mut self) -> TransactionExecutorResult<(CommitmentStateDiff, BouncerWeights)> { self.finalize() } }