Skip to content

Commit

Permalink
refactor(blockifier): state_maps is field in StateChanges (#2172)
Browse files Browse the repository at this point in the history
  • Loading branch information
yoavGrs authored Nov 19, 2024
1 parent cd88a38 commit 66e1652
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 39 deletions.
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
36 changes: 16 additions & 20 deletions crates/blockifier/src/state/cached_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<S: StateReader> CachedState<S> {
// 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<StateChanges> {
Ok(self.to_state_diff()?.into())
Ok(StateChanges { state_maps: self.to_state_diff()? })
}

pub fn update_cache(
Expand Down Expand Up @@ -657,15 +657,17 @@ 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
/// is important. The state changes are merged in the order they appear in the given vector.
pub fn merge(state_changes: Vec<Self>) -> 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
Expand All @@ -674,11 +676,11 @@ impl StateChanges {
pub fn get_modified_contracts(&self) -> HashSet<ContractAddress> {
// Storage updates.
let mut modified_contracts: HashSet<ContractAddress> =
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
}
Expand All @@ -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;
}
}
Expand All @@ -709,29 +711,23 @@ 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(),
}
}

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<StateMaps> 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)]
Expand Down
32 changes: 17 additions & 15 deletions crates/blockifier/src/state/cached_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,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.
Expand Down Expand Up @@ -1433,7 +1433,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.
Expand Down Expand Up @@ -1482,7 +1482,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);
}

Expand Down

0 comments on commit 66e1652

Please sign in to comment.