Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: stop storing tx number #45

Merged
merged 1 commit into from
Jun 20, 2024
Merged

chore: stop storing tx number #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions src/instruction_handlers/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ fn sstore(
let key = Register1::get(args, &mut vm.state);
let value = Register2::get(args, &mut vm.state);

let refund = vm.world_diff.write_storage(
world,
vm.state.current_frame.address,
key,
value,
vm.state.transaction_number,
);
let refund = vm
.world_diff
.write_storage(world, vm.state.current_frame.address, key, value);

assert!(refund <= SSTORE_COST);
vm.state.current_frame.gas += refund;
Expand Down
31 changes: 12 additions & 19 deletions src/world_diff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use zkevm_opcode_defs::system_params::{
#[derive(Default)]
pub struct WorldDiff {
// These are rolled back on revert or panic (and when the whole VM is rolled back).
storage_changes: RollbackableMap<(H160, U256), (u16, U256)>,
storage_changes: RollbackableMap<(H160, U256), U256>,
paid_changes: RollbackableMap<(H160, U256), u32>,
transient_storage_changes: RollbackableMap<(H160, U256), U256>,
events: RollbackableLog<Event>,
Expand Down Expand Up @@ -72,7 +72,6 @@ impl WorldDiff {
.as_ref()
.get(&(contract, key))
.cloned()
.map(|v| v.1)
.unwrap_or_else(|| world.read_storage(contract, key).unwrap_or_default());

let refund = if world.is_free_storage_slot(&contract, &key)
Expand All @@ -94,10 +93,8 @@ impl WorldDiff {
contract: H160,
key: U256,
value: U256,
tx_number: u16,
) -> u32 {
self.storage_changes
.insert((contract, key), (tx_number, value));
self.storage_changes.insert((contract, key), value);

let initial_value = self
.storage_initial_values
Expand Down Expand Up @@ -136,21 +133,21 @@ impl WorldDiff {
refund
}

pub fn get_storage_state(&self) -> &BTreeMap<(H160, U256), (u16, U256)> {
pub fn get_storage_state(&self) -> &BTreeMap<(H160, U256), U256> {
self.storage_changes.as_ref()
}

pub fn get_storage_changes(
&self,
) -> impl Iterator<Item = ((H160, U256), (u16, Option<U256>, U256))> + '_ {
) -> impl Iterator<Item = ((H160, U256), (Option<U256>, U256))> + '_ {
self.storage_changes
.as_ref()
.iter()
.filter_map(|(key, &(tx_number, value))| {
.filter_map(|(key, &value)| {
if self.storage_initial_values[key].unwrap_or_default() == value {
None
} else {
Some((*key, (tx_number, self.storage_initial_values[key], value)))
Some((*key, (self.storage_initial_values[key], value)))
}
})
}
Expand All @@ -162,14 +159,13 @@ impl WorldDiff {
self.storage_changes
.changes_after(snapshot.storage_changes)
.into_iter()
.map(|(key, (before, (tx_id, after)))| {
.map(|(key, (before, after))| {
let initial = self.storage_initial_values[&key];
(
key,
StorageChange {
before: before.map(|x| x.1).or(initial),
before: before.or(initial),
after,
tx_number: tx_id,
is_initial: initial.is_none(),
},
)
Expand Down Expand Up @@ -313,7 +309,6 @@ pub struct Snapshot {
pub struct StorageChange {
pub before: Option<U256>,
pub after: U256,
pub tx_number: u16,
/// `true` if the slot is not set in the World.
/// A write may be initial even if it isn't the first write to a slot!
pub is_initial: bool,
Expand Down Expand Up @@ -343,7 +338,7 @@ mod tests {

let checkpoint1 = world_diff.snapshot();
for (key, value) in &first_changes {
world_diff.write_storage(&mut NoWorld, key.0, key.1, *value, 0);
world_diff.write_storage(&mut NoWorld, key.0, key.1, *value);
}
assert_eq!(
world_diff
Expand All @@ -357,15 +352,14 @@ mod tests {
before: initial_values.get(key).copied(),
after: *value,
is_initial: initial_values.get(key).is_none(),
tx_number: 0,
}
))
.collect()
);

let checkpoint2 = world_diff.snapshot();
for (key, value) in &second_changes {
world_diff.write_storage(&mut NoWorld, key.0, key.1, *value, 1);
world_diff.write_storage(&mut NoWorld, key.0, key.1, *value);
}
assert_eq!(
world_diff
Expand All @@ -379,7 +373,6 @@ mod tests {
before: first_changes.get(key).or(initial_values.get(key)).copied(),
after: *value,
is_initial: initial_values.get(key).is_none(),
tx_number: 1,
}
))
.collect()
Expand All @@ -389,13 +382,13 @@ mod tests {
.into_iter()
.filter_map(|(key, value)| {
let initial = initial_values.get(&key).copied();
(initial.unwrap_or_default() != value).then_some((key, (0, initial, value)))
(initial.unwrap_or_default() != value).then_some((key, (initial, value)))
})
.collect::<BTreeMap<_, _>>();
for (key, value) in second_changes {
let initial = initial_values.get(&key).copied();
if initial.unwrap_or_default() != value {
combined.insert(key, (1, initial, value));
combined.insert(key, (initial, value));
} else {
combined.remove(&key);
}
Expand Down
Loading