From d0a53fea40aea1700e603bbf3ff2b3b0526e13dd Mon Sep 17 00:00:00 2001 From: Yoav Gross Date: Tue, 19 Nov 2024 11:10:15 +0200 Subject: [PATCH] refactor(blockifier): state_maps is field in StateChanges --- .../src/concurrency/worker_logic.rs | 2 +- crates/blockifier/src/state/cached_state.rs | 36 +++++++++---------- .../blockifier/src/state/cached_state_test.rs | 32 +++++++++-------- .../transaction/account_transactions_test.rs | 6 ++-- 4 files changed, 37 insertions(+), 39 deletions(-) diff --git a/crates/blockifier/src/concurrency/worker_logic.rs b/crates/blockifier/src/concurrency/worker_logic.rs index eb26f2a26e..b50a011c4d 100644 --- a/crates/blockifier/src/concurrency/worker_logic.rs +++ b/crates/blockifier/src/concurrency/worker_logic.rs @@ -233,7 +233,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> { let mut execution_output = lock_mutex_in_array(&self.execution_outputs, tx_index); let writes = &execution_output.as_ref().expect(EXECUTION_OUTPUTS_UNWRAP_ERROR).writes; // TODO(Yoni): get rid of this clone. - let mut tx_state_changes_keys = StateChanges::from(writes.clone()).into_keys(); + let mut tx_state_changes_keys = StateChanges { state_maps: writes.clone() }.into_keys(); let tx_result = &mut execution_output.as_mut().expect(EXECUTION_OUTPUTS_UNWRAP_ERROR).result; diff --git a/crates/blockifier/src/state/cached_state.rs b/crates/blockifier/src/state/cached_state.rs index f78f1b5307..2272268112 100644 --- a/crates/blockifier/src/state/cached_state.rs +++ b/crates/blockifier/src/state/cached_state.rs @@ -55,7 +55,7 @@ impl CachedState { // TODO(Yoni, 1/8/2024): remove this function. /// Returns the state changes made on this state. pub fn get_actual_state_changes(&mut self) -> StateResult { - Ok(self.to_state_diff()?.into()) + Ok(StateChanges { state_maps: self.to_state_diff()? }) } pub fn update_cache( @@ -657,7 +657,9 @@ impl StateChangesKeys { /// Holds the state changes. #[cfg_attr(any(feature = "testing", test), derive(Clone))] #[derive(Debug, Default, Eq, PartialEq)] -pub struct StateChanges(pub StateMaps); +pub struct StateChanges { + pub state_maps: StateMaps, +} impl StateChanges { /// Merges the given state changes into a single one. Note that the order of the state changes @@ -665,7 +667,7 @@ impl StateChanges { pub fn merge(state_changes: Vec) -> Self { let mut merged_state_changes = Self::default(); for state_change in state_changes { - merged_state_changes.0.extend(&state_change.0); + merged_state_changes.state_maps.extend(&state_change.state_maps); } merged_state_changes @@ -674,11 +676,11 @@ impl StateChanges { pub fn get_modified_contracts(&self) -> HashSet { // Storage updates. let mut modified_contracts: HashSet = - self.0.storage.keys().map(|address_key_pair| address_key_pair.0).collect(); + self.state_maps.storage.keys().map(|address_key_pair| address_key_pair.0).collect(); // Nonce updates. - modified_contracts.extend(self.0.nonces.keys()); + modified_contracts.extend(self.state_maps.nonces.keys()); // Class hash updates (deployed contracts + replace_class syscall). - modified_contracts.extend(self.0.class_hashes.keys()); + modified_contracts.extend(self.state_maps.class_hashes.keys()); modified_contracts } @@ -695,10 +697,10 @@ impl StateChanges { // fee transfer. The fee transfer is going to update the balance of the sequencer // and the balance of the sender contract, but we don't charge the sender for the // sequencer balance change as it is amortized across the block. - let mut n_storage_updates = self.0.storage.len(); + let mut n_storage_updates = self.state_maps.storage.len(); if let Some(sender_address) = sender_address { let sender_balance_key = get_fee_token_var_address(sender_address); - if !self.0.storage.contains_key(&(fee_token_address, sender_balance_key)) { + if !self.state_maps.storage.contains_key(&(fee_token_address, sender_balance_key)) { n_storage_updates += 1; } } @@ -709,8 +711,8 @@ impl StateChanges { StateChangesCount { n_storage_updates, - n_class_hash_updates: self.0.class_hashes.len(), - n_compiled_class_hash_updates: self.0.compiled_class_hashes.len(), + n_class_hash_updates: self.state_maps.class_hashes.len(), + n_compiled_class_hash_updates: self.state_maps.compiled_class_hashes.len(), n_modified_contracts: modified_contracts.len(), } } @@ -718,20 +720,14 @@ impl StateChanges { pub fn into_keys(self) -> StateChangesKeys { StateChangesKeys { modified_contracts: self.get_modified_contracts(), - nonce_keys: self.0.nonces.into_keys().collect(), - class_hash_keys: self.0.class_hashes.into_keys().collect(), - storage_keys: self.0.storage.into_keys().collect(), - compiled_class_hash_keys: self.0.compiled_class_hashes.into_keys().collect(), + nonce_keys: self.state_maps.nonces.into_keys().collect(), + class_hash_keys: self.state_maps.class_hashes.into_keys().collect(), + storage_keys: self.state_maps.storage.into_keys().collect(), + compiled_class_hash_keys: self.state_maps.compiled_class_hashes.into_keys().collect(), } } } -impl From for StateChanges { - fn from(state_maps: StateMaps) -> Self { - Self(state_maps) - } -} - /// Holds the number of state changes. #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] diff --git a/crates/blockifier/src/state/cached_state_test.rs b/crates/blockifier/src/state/cached_state_test.rs index 21f856ec05..620b969c60 100644 --- a/crates/blockifier/src/state/cached_state_test.rs +++ b/crates/blockifier/src/state/cached_state_test.rs @@ -378,7 +378,7 @@ fn test_state_changes_merge( ); // Get the storage updates addresses and keys from the state_changes1, to overwrite. - let mut storage_updates_keys = state_changes1.0.storage.keys(); + let mut storage_updates_keys = state_changes1.state_maps.storage.keys(); let &(contract_address, storage_key) = storage_updates_keys .find(|(contract_address, _)| contract_address == &contract_address!(CONTRACT_ADDRESS)) .unwrap(); @@ -448,20 +448,22 @@ fn test_cache_get_write_keys() { let class_hash0 = class_hash!("0x300"); - let state_changes = StateChanges(StateMaps { - nonces: HashMap::from([(contract_address0, Nonce(some_felt))]), - class_hashes: HashMap::from([ - (contract_address1, some_class_hash), - (contract_address2, some_class_hash), - ]), - storage: HashMap::from([ - ((contract_address1, storage_key!(0x300_u16)), some_felt), - ((contract_address1, storage_key!(0x600_u16)), some_felt), - ((contract_address3, storage_key!(0x600_u16)), some_felt), - ]), - compiled_class_hashes: HashMap::from([(class_hash0, compiled_class_hash!(0x3_u16))]), - declared_contracts: HashMap::default(), - }); + let state_changes = StateChanges { + state_maps: StateMaps { + nonces: HashMap::from([(contract_address0, Nonce(some_felt))]), + class_hashes: HashMap::from([ + (contract_address1, some_class_hash), + (contract_address2, some_class_hash), + ]), + storage: HashMap::from([ + ((contract_address1, storage_key!(0x300_u16)), some_felt), + ((contract_address1, storage_key!(0x600_u16)), some_felt), + ((contract_address3, storage_key!(0x600_u16)), some_felt), + ]), + compiled_class_hashes: HashMap::from([(class_hash0, compiled_class_hash!(0x3_u16))]), + declared_contracts: HashMap::default(), + }, + }; let expected_keys = StateChangesKeys { nonce_keys: HashSet::from([contract_address0]), diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 8548801fa8..a5a1d8da17 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -1387,7 +1387,7 @@ fn test_count_actual_storage_changes( }; assert_eq!(expected_modified_contracts, state_changes_1.get_modified_contracts()); - assert_eq!(expected_storage_updates_1, state_changes_1.0.storage); + assert_eq!(expected_storage_updates_1, state_changes_1.state_maps.storage); assert_eq!(state_changes_count_1, expected_state_changes_count_1); // Second transaction: storage cell starts and ends with value 1. @@ -1424,7 +1424,7 @@ fn test_count_actual_storage_changes( }; assert_eq!(expected_modified_contracts_2, state_changes_2.get_modified_contracts()); - assert_eq!(expected_storage_updates_2, state_changes_2.0.storage); + assert_eq!(expected_storage_updates_2, state_changes_2.state_maps.storage); assert_eq!(state_changes_count_2, expected_state_changes_count_2); // Transfer transaction: transfer 1 ETH to recepient. @@ -1473,7 +1473,7 @@ fn test_count_actual_storage_changes( expected_modified_contracts_transfer, state_changes_transfer.get_modified_contracts() ); - assert_eq!(expected_storage_update_transfer, state_changes_transfer.0.storage); + assert_eq!(expected_storage_update_transfer, state_changes_transfer.state_maps.storage); assert_eq!(state_changes_count_3, expected_state_changes_count_3); }