From 5de8afb1347c8842e4f87f334e7728ece83f6100 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 13 Aug 2024 15:45:10 -0300 Subject: [PATCH 01/57] Derive partial-eq for VmState --- src/call_frame.rs | 4 ++-- src/heaps.rs | 2 +- src/state.rs | 9 +++++---- src/store.rs | 2 +- src/value.rs | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/call_frame.rs b/src/call_frame.rs index 7d228d10..862a3153 100644 --- a/src/call_frame.rs +++ b/src/call_frame.rs @@ -4,7 +4,7 @@ use zkevm_opcode_defs::ethereum_types::Address; use crate::{state::Stack, store::SnapShot, utils::is_kernel}; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct CallFrame { pub pc: u64, pub transient_storage_snapshot: SnapShot, @@ -14,7 +14,7 @@ pub struct CallFrame { pub storage_snapshot: SnapShot, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Context { pub frame: CallFrame, pub near_call_frames: Vec, diff --git a/src/heaps.rs b/src/heaps.rs index caaafb6f..8f7611b6 100644 --- a/src/heaps.rs +++ b/src/heaps.rs @@ -2,7 +2,7 @@ use zkevm_opcode_defs::system_params::NEW_FRAME_MEMORY_STIPEND; use crate::{eravm_error::HeapError, state::Heap}; -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct Heaps { heaps: Vec, } diff --git a/src/state.rs b/src/state.rs index ddb91b6c..df55d247 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,4 +1,5 @@ use std::num::Saturating; +use std::u32; use crate::call_frame::{CallFrame, Context}; use crate::heaps::Heaps; @@ -18,16 +19,16 @@ pub const CALLDATA_HEAP: u32 = 1; pub const FIRST_HEAP: u32 = 2; pub const FIRST_AUX_HEAP: u32 = 3; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Stack { pub stack: Vec, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Heap { heap: Vec, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct VMState { // The first register, r0, is actually always zero and not really used. // Writing to it does nothing. @@ -478,7 +479,7 @@ impl Heap { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Event { pub key: U256, pub value: U256, diff --git a/src/store.rs b/src/store.rs index 7eeb5821..e7fb9ff1 100644 --- a/src/store.rs +++ b/src/store.rs @@ -125,7 +125,7 @@ impl StateStorage { } } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct SnapShot { pub storage_changes: HashMap, } diff --git a/src/value.rs b/src/value.rs index 65383814..431b70ec 100644 --- a/src/value.rs +++ b/src/value.rs @@ -2,7 +2,7 @@ use u256::U256; /// In the zkEVM, all data in the stack and on registers is tagged to determine /// whether they are a pointer or not. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub struct TaggedValue { pub value: U256, pub is_pointer: bool, From 59470a72d0d82a55b762f7d740b1bfeae65390b0 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 13 Aug 2024 15:45:39 -0300 Subject: [PATCH 02/57] Add initial gas param for VmState new --- src/state.rs | 6 ++---- src/tracers/last_state_saver_tracer.rs | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/state.rs b/src/state.rs index df55d247..7c0f934a 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,5 +1,4 @@ use std::num::Saturating; -use std::u32; use crate::call_frame::{CallFrame, Context}; use crate::heaps::Heaps; @@ -51,8 +50,6 @@ pub struct VMState { pub use_hooks: bool, } -// Totally arbitrary, probably we will have to change it later. -pub const DEFAULT_INITIAL_GAS: u32 = 1 << 16; impl VMState { #[allow(clippy::too_many_arguments)] pub fn new( @@ -65,6 +62,7 @@ impl VMState { evm_interpreter_code_hash: [u8; 32], hook_address: u32, use_hooks: bool, + initial_gas: u32, ) -> Self { let mut registers = [TaggedValue::default(); 15]; let calldata_ptr = FatPointer { @@ -78,7 +76,7 @@ impl VMState { let context = Context::new( program_code.clone(), - u32::MAX - 0x80000000, + initial_gas, contract_address, contract_address, caller, diff --git a/src/tracers/last_state_saver_tracer.rs b/src/tracers/last_state_saver_tracer.rs index 533e0c25..7e2bdcb6 100644 --- a/src/tracers/last_state_saver_tracer.rs +++ b/src/tracers/last_state_saver_tracer.rs @@ -25,6 +25,7 @@ impl LastStateSaverTracer { Default::default(), 0, false, + 0, ), } } From 26c2b18ae054e47eef621efab53ecd694a32f321 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 13 Aug 2024 16:11:34 -0300 Subject: [PATCH 03/57] Update era-compiler-tester branch --- .gitmodules | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitmodules b/.gitmodules index 203428a9..8f39238b 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,4 @@ [submodule "era-compiler-tester"] path = era-compiler-tester url = https://github.com/lambdaclass/era-compiler-tester.git + branch = zksync-era-tests From e5cb98ba4e938a1f00792eea606d21a7fb2e82be Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Wed, 14 Aug 2024 10:39:30 -0300 Subject: [PATCH 04/57] fix test bytecode_publishing --- src/store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/store.rs b/src/store.rs index e7fb9ff1..720a276e 100644 --- a/src/store.rs +++ b/src/store.rs @@ -56,7 +56,7 @@ impl ContractStorage for ContractStorageMemory { pub struct StateStorage { pub storage_changes: HashMap, pub initial_storage: Rc>, - l2_to_l1_logs: Vec, + pub l2_to_l1_logs: Vec, } impl Default for StateStorage { From f0dce4107263dec4ea6458d2584e63a25eff6882 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 14 Aug 2024 11:07:56 -0300 Subject: [PATCH 05/57] Store changes for get_used_contracts tests --- src/store.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/store.rs b/src/store.rs index 720a276e..7f27e99c 100644 --- a/src/store.rs +++ b/src/store.rs @@ -40,6 +40,7 @@ impl InitialStorage for InitialStorageMemory { pub trait ContractStorage: Debug { fn decommit(&self, hash: U256) -> Result>, StorageError>; + fn hash_map(&self) -> Result>, StorageError>; } #[derive(Debug)] pub struct ContractStorageMemory { @@ -50,6 +51,10 @@ impl ContractStorage for ContractStorageMemory { fn decommit(&self, hash: U256) -> Result>, StorageError> { Ok(self.contract_storage.get(&hash).cloned()) } + + fn hash_map(&self) -> Result>, StorageError> { + Ok(self.contract_storage.clone()) + } } #[derive(Debug)] From c862649e8295cd56e51aca86944be56dd7c49af5 Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Fri, 16 Aug 2024 16:28:46 -0300 Subject: [PATCH 06/57] adapt to zksync-era tests --- era-compiler-tester | 2 +- src/call_frame.rs | 6 ++-- src/execution.rs | 12 ++++---- src/heaps.rs | 2 +- src/lib.rs | 2 +- src/op_handlers/far_call.rs | 20 +++++++++++-- src/rollbacks.rs | 40 +++++++++++++++++++++----- src/state.rs | 23 +++++++++++---- src/store.rs | 8 +++++- src/tracers/last_state_saver_tracer.rs | 1 + src/value.rs | 2 +- src/vm.rs | 2 +- 12 files changed, 89 insertions(+), 31 deletions(-) diff --git a/era-compiler-tester b/era-compiler-tester index 85f5cb88..eab44db2 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit 85f5cb88bad91c32e11e4d326abcc7a868f5ca97 +Subproject commit eab44db21bd7b7e2ff907530133283477626ea02 diff --git a/src/call_frame.rs b/src/call_frame.rs index 5eff6f84..6ff089bb 100644 --- a/src/call_frame.rs +++ b/src/call_frame.rs @@ -4,7 +4,7 @@ use zkevm_opcode_defs::ethereum_types::Address; use crate::{execution::Stack, state::StateSnapshot, utils::is_kernel}; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct CallFrame { pub pc: u64, pub gas_left: Saturating, @@ -13,7 +13,7 @@ pub struct CallFrame { pub snapshot: StateSnapshot, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct CodePage(Vec); impl CodePage { @@ -25,7 +25,7 @@ impl CodePage { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Context { pub frame: CallFrame, pub near_call_frames: Vec, diff --git a/src/execution.rs b/src/execution.rs index 88f8ee3e..34af025b 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -18,16 +18,17 @@ pub const CALLDATA_HEAP: u32 = 1; pub const FIRST_HEAP: u32 = 2; pub const FIRST_AUX_HEAP: u32 = 3; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Stack { pub stack: Vec, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Heap { heap: Vec, } -#[derive(Debug, Clone)] + +#[derive(Debug, Clone, PartialEq)] pub struct Execution { // The first register, r0, is actually always zero and not really used. // Writing to it does nothing. @@ -49,8 +50,6 @@ pub struct Execution { pub use_hooks: bool, } -// Totally arbitrary, probably we will have to change it later. -pub const DEFAULT_INITIAL_GAS: u32 = 1 << 16; impl Execution { #[allow(clippy::too_many_arguments)] pub fn new( @@ -63,6 +62,7 @@ impl Execution { evm_interpreter_code_hash: [u8; 32], hook_address: u32, use_hooks: bool, + initial_gas: u32, ) -> Self { let mut registers = [TaggedValue::default(); 15]; let calldata_ptr = FatPointer { @@ -76,7 +76,7 @@ impl Execution { let context = Context::new( program_code.clone(), - u32::MAX - 0x80000000, + initial_gas, contract_address, contract_address, caller, diff --git a/src/heaps.rs b/src/heaps.rs index 6d8a9442..e02f4747 100644 --- a/src/heaps.rs +++ b/src/heaps.rs @@ -2,7 +2,7 @@ use zkevm_opcode_defs::system_params::NEW_FRAME_MEMORY_STIPEND; use crate::{eravm_error::HeapError, execution::Heap}; -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct Heaps { heaps: Vec, } diff --git a/src/lib.rs b/src/lib.rs index 753422be..73c1a451 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,6 @@ pub mod vm; pub use execution::Execution; pub use opcode::Opcode; pub use vm::EraVM; -mod rollbacks; +pub mod rollbacks; pub mod state; use zkevm_opcode_defs::Opcode as Variant; diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index f16a02d3..56391df1 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -129,7 +129,7 @@ fn decommit_code_hash( default_aa_code_hash: [u8; 32], evm_interpreter_code_hash: [u8; 32], is_constructor_call: bool, -) -> Result<(U256, bool), EraVmError> { +) -> Result<(U256, bool, u32), EraVmError> { let mut is_evm = false; let deployer_system_contract_address = Address::from_low_u64_be(DEPLOYER_SYSTEM_CONTRACT_ADDRESS_LOW as u64); @@ -193,7 +193,16 @@ fn decommit_code_hash( code_info_bytes[1] = 0; - Ok((U256::from_big_endian(&code_info_bytes), is_evm)) + let code_key = U256::from_big_endian(&code_info_bytes); + + let cost = if state.decommitted_hashes().contains(&code_key) { + 0 + } else { + let code_length_in_words = u16::from_be_bytes([code_info_bytes[2], code_info_bytes[3]]); + code_length_in_words as u32 * zkevm_opcode_defs::ERGS_PER_CODE_WORD_DECOMMITTMENT + }; + + Ok((U256::from_big_endian(&code_info_bytes), is_evm, cost)) } pub fn far_call( @@ -212,7 +221,7 @@ pub fn far_call( abi.is_constructor_call = abi.is_constructor_call && vm.current_context()?.is_kernel(); abi.is_system_call = abi.is_system_call && is_kernel(&contract_address); - let (code_key, is_evm) = decommit_code_hash( + let (code_key, is_evm, decommit_cost) = decommit_code_hash( state, contract_address, vm.default_aa_code_hash, @@ -220,6 +229,11 @@ pub fn far_call( abi.is_constructor_call, )?; + // Unlike all other gas costs, this one is not paid if low on gas. + if decommit_cost < vm.gas_left()? { + vm.decrease_gas(decommit_cost)?; + } + let FarCallParams { ergs_passed, forward_memory, diff --git a/src/rollbacks.rs b/src/rollbacks.rs index 27339a10..14aecd43 100644 --- a/src/rollbacks.rs +++ b/src/rollbacks.rs @@ -1,4 +1,7 @@ -use std::collections::{HashMap, HashSet}; +use std::{ + collections::{HashMap, HashSet}, + hash::Hash, +}; pub trait Rollbackable { type Snapshot; @@ -6,12 +9,20 @@ pub trait Rollbackable { fn snapshot(&self) -> Self::Snapshot; } -#[derive(Debug, Default)] -pub struct RollbackableHashMap { +#[derive(Debug, Default, Clone)] +pub struct RollbackableHashMap { pub map: HashMap, } -impl Rollbackable for RollbackableHashMap { +impl RollbackableHashMap { + pub fn new() -> Self { + Self { + map: HashMap::new(), + } + } +} + +impl Rollbackable for RollbackableHashMap { type Snapshot = HashMap; fn rollback(&mut self, snapshot: Self::Snapshot) { self.map = snapshot; @@ -22,11 +33,26 @@ impl Rollbackable for RollbackableHashMap { } } -#[derive(Debug, Default)] +impl Iterator for RollbackableHashMap { + type Item = (K, V); + fn next(&mut self) -> Option { + self.map.iter().next().map(|(k, v)| (k.clone(), v.clone())) + } +} + +#[derive(Debug, Default, Clone)] pub struct RollbackableVec { pub entries: Vec, } +impl RollbackableVec { + pub fn new() -> Self { + Self { + entries: Vec::new(), + } + } +} + impl Rollbackable for RollbackableVec { type Snapshot = Vec; @@ -38,7 +64,7 @@ impl Rollbackable for RollbackableVec { } } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct RollbackablePrimitive { pub value: T, } @@ -54,7 +80,7 @@ impl Rollbackable for RollbackablePrimitive { } } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct RollbackableHashSet { pub map: HashSet, } diff --git a/src/state.rs b/src/state.rs index 5a4ac386..6cc0d819 100644 --- a/src/state.rs +++ b/src/state.rs @@ -5,7 +5,11 @@ use crate::{ }, store::{Storage, StorageKey}, }; -use std::{cell::RefCell, collections::HashMap, rc::Rc}; +use std::{ + cell::RefCell, + collections::{HashMap, HashSet}, + rc::Rc, +}; use u256::{H160, U256}; use zkevm_opcode_defs::system_params::{ STORAGE_ACCESS_COLD_READ_COST, STORAGE_ACCESS_COLD_WRITE_COST, STORAGE_ACCESS_WARM_READ_COST, @@ -35,13 +39,13 @@ pub struct Event { pub tx_number: u16, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct VMState { pub storage: Rc>, - storage_changes: RollbackableHashMap, - transient_storage: RollbackableHashMap, - l2_to_l1_logs: RollbackableVec, - events: RollbackableVec, + pub storage_changes: RollbackableHashMap, + pub transient_storage: RollbackableHashMap, + pub l2_to_l1_logs: RollbackableVec, + pub events: RollbackableVec, // holds the sum of pubdata_costs pubdata: RollbackablePrimitive, pubdata_costs: RollbackableVec, @@ -52,6 +56,7 @@ pub struct VMState { // that is why we add them as rollbackable as well read_storage_slots: RollbackableHashSet, written_storage_slots: RollbackableHashSet, + decommitted_hashes: RollbackableHashSet, } impl VMState { @@ -68,6 +73,7 @@ impl VMState { refunds: RollbackableVec::::default(), read_storage_slots: RollbackableHashSet::::default(), written_storage_slots: RollbackableHashSet::::default(), + decommitted_hashes: RollbackableHashSet::::default(), } } @@ -189,8 +195,13 @@ impl VMState { } pub fn decommit(&mut self, hash: U256) -> Option> { + self.decommitted_hashes.map.insert(hash); self.storage.borrow_mut().decommit(hash) } + + pub fn decommitted_hashes(&self) -> &HashSet { + &self.decommitted_hashes.map + } } #[derive(Clone, Default, PartialEq, Debug)] diff --git a/src/store.rs b/src/store.rs index e96e43a8..557b5447 100644 --- a/src/store.rs +++ b/src/store.rs @@ -28,6 +28,8 @@ pub trait Storage: Debug { fn cost_of_writing_storage(&mut self, key: &StorageKey, value: U256) -> u32; fn is_free_storage_slot(&self, key: &StorageKey) -> bool; + + fn hash_map(&self) -> Result, StorageError>; } #[derive(Debug, Clone)] @@ -55,10 +57,14 @@ impl Storage for InitialStorageMemory { fn is_free_storage_slot(&self, _key: &StorageKey) -> bool { false } + + fn hash_map(&self) -> Result, StorageError> { + Ok(self.storage.clone()) + } } /// Error type for storage operations. -#[derive(Error, Debug)] +#[derive(Error, Debug, PartialEq)] pub enum StorageError { #[error("Key not present in storage")] KeyNotPresent, diff --git a/src/tracers/last_state_saver_tracer.rs b/src/tracers/last_state_saver_tracer.rs index 9ef4b07e..5efbd292 100644 --- a/src/tracers/last_state_saver_tracer.rs +++ b/src/tracers/last_state_saver_tracer.rs @@ -25,6 +25,7 @@ impl LastStateSaverTracer { Default::default(), 0, false, + 0, ), } } diff --git a/src/value.rs b/src/value.rs index 65383814..431b70ec 100644 --- a/src/value.rs +++ b/src/value.rs @@ -2,7 +2,7 @@ use u256::U256; /// In the zkEVM, all data in the stack and on registers is tagged to determine /// whether they are a pointer or not. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub struct TaggedValue { pub value: U256, pub is_pointer: bool, diff --git a/src/vm.rs b/src/vm.rs index 19d0faf8..2cb94089 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -56,7 +56,7 @@ pub enum ExecutionOutput { SuspendedOnHook { hook: u32, pc_to_resume_from: u16 }, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct EraVM { pub state: VMState, pub execution: Execution, From bbac76ddbf405686f83e564eb90481c2d74a9093 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 16 Aug 2024 16:32:45 -0300 Subject: [PATCH 07/57] Remove hash_map from Storage trait --- src/store.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/store.rs b/src/store.rs index 557b5447..f8171045 100644 --- a/src/store.rs +++ b/src/store.rs @@ -28,8 +28,6 @@ pub trait Storage: Debug { fn cost_of_writing_storage(&mut self, key: &StorageKey, value: U256) -> u32; fn is_free_storage_slot(&self, key: &StorageKey) -> bool; - - fn hash_map(&self) -> Result, StorageError>; } #[derive(Debug, Clone)] @@ -57,10 +55,6 @@ impl Storage for InitialStorageMemory { fn is_free_storage_slot(&self, _key: &StorageKey) -> bool { false } - - fn hash_map(&self) -> Result, StorageError> { - Ok(self.storage.clone()) - } } /// Error type for storage operations. From 5ca9edf2f24f2775d5767349ef673796bedf7404 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 22:33:36 -0300 Subject: [PATCH 08/57] Fix increment_tx_number not cleaning transient_storage --- src/op_handlers/context.rs | 7 ++++++- src/state.rs | 4 ++++ src/vm.rs | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/op_handlers/context.rs b/src/op_handlers/context.rs index fd95cdc3..0ad98561 100644 --- a/src/op_handlers/context.rs +++ b/src/op_handlers/context.rs @@ -81,7 +81,12 @@ pub fn set_context_u128(vm: &mut Execution, opcode: &Opcode) -> Result<(), EraVm Ok(()) } -pub fn increment_tx_number(vm: &mut Execution, _opcode: &Opcode) -> Result<(), EraVmError> { +pub fn increment_tx_number( + vm: &mut Execution, + _opcode: &Opcode, + state: &mut VMState, +) -> Result<(), EraVmError> { vm.tx_number += 1; + state.clear_transient_storage(); Ok(()) } diff --git a/src/state.rs b/src/state.rs index 6cc0d819..4ac50c8c 100644 --- a/src/state.rs +++ b/src/state.rs @@ -186,6 +186,10 @@ impl VMState { self.transient_storage.map.insert(key, value); } + pub(crate) fn clear_transient_storage(&mut self) { + self.transient_storage = RollbackableHashMap::default(); + } + pub fn record_l2_to_l1_log(&mut self, msg: L2ToL1Log) { self.l2_to_l1_logs.entries.push(msg); } diff --git a/src/vm.rs b/src/vm.rs index 2cb94089..19c4a5f4 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -169,7 +169,7 @@ impl EraVM { get_context_u128(&mut self.execution, &opcode) } ContextOpcode::IncrementTxNumber => { - increment_tx_number(&mut self.execution, &opcode) + increment_tx_number(&mut self.execution, &opcode, &mut self.state) } ContextOpcode::Meta => meta(&mut self.execution, &opcode, &self.state), ContextOpcode::SetContextU128 => { From 01c6eb641547e13021c8680e99dd4fe3f5ea627b Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 22:37:15 -0300 Subject: [PATCH 09/57] Fix decommit opcode Fixes decommit opcode, which was not account for if the code had already been decommitted --- src/op_handlers/opcode_decommit.rs | 24 ++++++++++++++++++++---- src/state.rs | 6 +++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/op_handlers/opcode_decommit.rs b/src/op_handlers/opcode_decommit.rs index 16f6faa4..bcf6f9eb 100644 --- a/src/op_handlers/opcode_decommit.rs +++ b/src/op_handlers/opcode_decommit.rs @@ -1,3 +1,5 @@ +use zkevm_opcode_defs::{BlobSha256Format, ContractCodeSha256Format, VersionedHashLen32}; + use crate::{ address_operands::{address_operands_read, address_operands_store}, eravm_error::{EraVmError, HeapError}, @@ -18,11 +20,25 @@ pub fn opcode_decommit( let preimage_len_in_bytes = zkevm_opcode_defs::system_params::NEW_KERNEL_FRAME_MEMORY_STIPEND; - vm.decrease_gas(extra_cost)?; + let mut buffer = [0u8; 32]; + code_hash.to_big_endian(&mut buffer); + + // gas is payed in advance + if vm.decrease_gas(extra_cost).is_err() + || (!ContractCodeSha256Format::is_valid(&buffer) && !BlobSha256Format::is_valid(&buffer)) + { + // we don't actually return an err here + vm.set_register(1, TaggedValue::zero()); + return Ok(()); + } + + let (code, was_decommited) = state.decommit(code_hash); + if was_decommited { + // refund it + vm.increase_gas(extra_cost)?; + }; - let code = state - .decommit(code_hash) - .ok_or(EraVmError::DecommitFailed)?; + let code = code.ok_or(EraVmError::DecommitFailed)?; let code_len_in_bytes = code.len() * 32; let id = vm.heaps.allocate(); diff --git a/src/state.rs b/src/state.rs index 4ac50c8c..49ec3f32 100644 --- a/src/state.rs +++ b/src/state.rs @@ -198,9 +198,9 @@ impl VMState { self.events.entries.push(event); } - pub fn decommit(&mut self, hash: U256) -> Option> { - self.decommitted_hashes.map.insert(hash); - self.storage.borrow_mut().decommit(hash) + pub fn decommit(&mut self, hash: U256) -> (Option>, bool) { + let was_decommitted = !self.decommitted_hashes.map.insert(hash); + (self.storage.borrow_mut().decommit(hash), was_decommitted) } pub fn decommitted_hashes(&self) -> &HashSet { From be5024f859e867b07517375c4d3ef53c1ee3a99d Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 22:40:06 -0300 Subject: [PATCH 10/57] Various far_call fixes Adds mandated gass, handles decommit cost err, and fixes the params of mimic call --- src/op_handlers/far_call.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index 56391df1..0c2224ba 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -1,8 +1,11 @@ use u256::{H160, U256}; use zkevm_opcode_defs::{ ethereum_types::Address, - system_params::{DEPLOYER_SYSTEM_CONTRACT_ADDRESS_LOW, EVM_SIMULATOR_STIPEND}, - FarCallOpcode, + system_params::{ + DEPLOYER_SYSTEM_CONTRACT_ADDRESS_LOW, EVM_SIMULATOR_STIPEND, + MSG_VALUE_SIMULATOR_ADDITIVE_COST, + }, + FarCallOpcode, ADDRESS_MSG_VALUE, }; use crate::{ @@ -102,6 +105,7 @@ fn far_call_params_from_register( let maximum_gas = gas_left / FAR_CALL_GAS_SCALAR_MODIFIER_DIVISOR * FAR_CALL_GAS_SCALAR_MODIFIER_DIVIDEND; + ergs_passed = ergs_passed.min(maximum_gas); source.to_little_endian(&mut args); @@ -230,8 +234,10 @@ pub fn far_call( )?; // Unlike all other gas costs, this one is not paid if low on gas. - if decommit_cost < vm.gas_left()? { + if decommit_cost <= vm.gas_left()? { vm.decrease_gas(decommit_cost)?; + } else { + return Err(EraVmError::DecommitFailed); } let FarCallParams { @@ -240,16 +246,26 @@ pub fn far_call( .. } = far_call_params_from_register(src0, vm)?; + let mandated_gas = if abi.is_system_call && src1.value == ADDRESS_MSG_VALUE.into() { + MSG_VALUE_SIMULATOR_ADDITIVE_COST + } else { + 0 + }; + + // mandated gas can surprass the 64/63 limit + let ergs_passed = ergs_passed + mandated_gas; + vm.decrease_gas(ergs_passed)?; let stipend = if is_evm { EVM_SIMULATOR_STIPEND } else { 0 }; - let ergs_passed = ergs_passed + let ergs_passed = (ergs_passed) .checked_add(stipend) .expect("stipend must not cause overflow"); let program_code = state .decommit(code_key) + .0 .ok_or(StorageError::KeyNotPresent)?; let new_heap = vm.heaps.allocate(); let new_aux_heap = vm.heaps.allocate(); @@ -273,21 +289,14 @@ pub fn far_call( )?; } FarCallOpcode::Mimic => { - let mut caller_bytes = [0; 32]; - let caller = vm.get_register(15).value; - caller.to_big_endian(&mut caller_bytes); - - let mut caller_bytes_20: [u8; 20] = [0; 20]; - for (i, byte) in caller_bytes[12..].iter().enumerate() { - caller_bytes_20[i] = *byte; - } + let caller = address_from_u256(&vm.get_register(15).value); vm.push_far_call_frame( program_code, ergs_passed, contract_address, contract_address, - H160::from(caller_bytes_20), + caller, new_heap, new_aux_heap, forward_memory.page, From cdfcd923206a2e1856388f9c13d5ddd36f261216 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 22:45:01 -0300 Subject: [PATCH 11/57] Add stipend param to frame and decrease it in panics and reverts --- src/call_frame.rs | 17 +++++++++++------ src/execution.rs | 3 +++ src/op_handlers/far_call.rs | 3 +++ src/op_handlers/ret.rs | 4 ++-- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/call_frame.rs b/src/call_frame.rs index 6ff089bb..348190e2 100644 --- a/src/call_frame.rs +++ b/src/call_frame.rs @@ -11,6 +11,7 @@ pub struct CallFrame { pub exception_handler: u64, pub sp: u32, pub snapshot: StateSnapshot, + pub stipend: u32, } #[derive(Debug, Clone, PartialEq)] @@ -54,7 +55,7 @@ impl Context { #[allow(clippy::too_many_arguments)] pub fn new( program_code: Vec, - gas_stipend: u32, + gas: u32, contract_address: Address, code_address: Address, caller: Address, @@ -65,9 +66,10 @@ impl Context { context_u128: u128, snapshot: StateSnapshot, is_static: bool, + stipend: u32, ) -> Self { Self { - frame: CallFrame::new_far_call_frame(gas_stipend, exception_handler, snapshot), + frame: CallFrame::new_far_call_frame(gas, stipend, exception_handler, snapshot), near_call_frames: vec![], contract_address, caller, @@ -89,13 +91,15 @@ impl Context { impl CallFrame { pub fn new_far_call_frame( - gas_stipend: u32, + gas: u32, + stipend: u32, exception_handler: u64, snapshot: StateSnapshot, ) -> Self { Self { pc: 0, - gas_left: Saturating(gas_stipend), + stipend: stipend, + gas_left: Saturating(gas), exception_handler, sp: 0, snapshot, @@ -106,14 +110,15 @@ impl CallFrame { pub fn new_near_call_frame( sp: u32, pc: u64, - gas_stipend: u32, + gas: u32, exception_handler: u64, snapshot: StateSnapshot, ) -> Self { Self { pc, - gas_left: Saturating(gas_stipend), + gas_left: Saturating(gas), exception_handler, + stipend: 0, sp, snapshot, } diff --git a/src/execution.rs b/src/execution.rs index 34af025b..930dc8e0 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -87,6 +87,7 @@ impl Execution { context_u128, StateSnapshot::default(), false, + 0, ); let heaps = Heaps::new(calldata); @@ -142,6 +143,7 @@ impl Execution { context_u128: u128, snapshot: StateSnapshot, is_static: bool, + stipend: u32, ) -> Result<(), EraVmError> { let new_context = Context::new( program_code, @@ -156,6 +158,7 @@ impl Execution { context_u128, snapshot, is_static, + stipend, ); self.running_contexts.push(new_context); Ok(()) diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index 0c2224ba..64908df4 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -286,6 +286,7 @@ pub fn far_call( vm.register_context_u128, snapshot, is_new_frame_static && !is_evm, + stipend, )?; } FarCallOpcode::Mimic => { @@ -304,6 +305,7 @@ pub fn far_call( vm.register_context_u128, snapshot, is_new_frame_static && !is_evm, + stipend, )?; } FarCallOpcode::Delegate => { @@ -323,6 +325,7 @@ pub fn far_call( this_context.context_u128, snapshot, is_new_frame_static && !is_evm, + stipend, )?; } }; diff --git a/src/op_handlers/ret.rs b/src/op_handlers/ret.rs index d7bfdcf9..393398c0 100644 --- a/src/op_handlers/ret.rs +++ b/src/op_handlers/ret.rs @@ -63,7 +63,7 @@ pub fn ret( vm.clear_registers(); vm.set_register(1, result); let previous_frame = vm.pop_frame()?; - vm.current_frame_mut()?.gas_left += previous_frame.gas_left; + vm.current_frame_mut()?.gas_left += previous_frame.gas_left.0 - previous_frame.stipend; if is_failure { state.rollback(previous_frame.snapshot); vm.current_frame_mut()?.pc = previous_frame.exception_handler; @@ -103,7 +103,7 @@ pub fn inexplicit_panic(vm: &mut Execution, state: &mut VMState) -> Result Date: Mon, 19 Aug 2024 22:47:14 -0300 Subject: [PATCH 12/57] Add no refund storage_read for far_call decommit_code_hash --- src/op_handlers/far_call.rs | 2 +- src/state.rs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index 64908df4..673e8de4 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -140,7 +140,7 @@ fn decommit_code_hash( let storage_key = StorageKey::new(deployer_system_contract_address, address_into_u256(address)); // reading when decommiting doesn't refund - let (code_info, _) = state.storage_read(storage_key); + let code_info = state.storage_read_with_no_refund(storage_key); let mut code_info_bytes = [0; 32]; code_info.to_big_endian(&mut code_info_bytes); diff --git a/src/state.rs b/src/state.rs index 49ec3f32..f3b0244d 100644 --- a/src/state.rs +++ b/src/state.rs @@ -93,6 +93,10 @@ impl VMState { &self.events.entries } + pub fn refunds(&self) -> &Vec { + &self.refunds.entries + } + pub fn pubdata_costs(&self) -> &Vec { &self.pubdata_costs.entries } @@ -107,6 +111,7 @@ impl VMState { pub fn storage_read(&mut self, key: StorageKey) -> (U256, u32) { let value = self.storage_read_inner(&key).unwrap_or_default(); + let storage = self.storage.borrow(); let refund = @@ -118,10 +123,24 @@ impl VMState { }; self.pubdata_costs.entries.push(0); + self.refunds.entries.push(refund); (value, refund) } + pub fn storage_read_with_no_refund(&mut self, key: StorageKey) -> U256 { + let value = self.storage_read_inner(&key).unwrap_or_default(); + let storage = self.storage.borrow(); + + if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) { + self.read_storage_slots.map.insert(key); + }; + + self.pubdata_costs.entries.push(0); + + value + } + fn storage_read_inner(&self, key: &StorageKey) -> Option { match self.storage_changes.map.get(key) { None => self.storage.borrow_mut().storage_read(key), From 29c184a85c204d673cef6288ba239c19e9b02871 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 22:47:33 -0300 Subject: [PATCH 13/57] Remove rollbacks from main context --- src/op_handlers/ret.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/op_handlers/ret.rs b/src/op_handlers/ret.rs index 393398c0..eca77d44 100644 --- a/src/op_handlers/ret.rs +++ b/src/op_handlers/ret.rs @@ -73,9 +73,10 @@ pub fn ret( Ok(false) } else { - if is_failure { - state.rollback(vm.current_frame()?.snapshot.clone()); - } + // The initial frame is not rolled back, even if it fails. + // It is the caller's job to clean up when the execution as a whole fails because + // the caller may take external snapshots while the VM is in the initial frame and + // these would break were the initial frame to be rolled back. if return_type == RetOpcode::Panic { return Ok(true); } @@ -108,7 +109,6 @@ pub fn inexplicit_panic(vm: &mut Execution, state: &mut VMState) -> Result Date: Mon, 19 Aug 2024 22:55:04 -0300 Subject: [PATCH 14/57] Reimplement vec snapshots --- src/rollbacks.rs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/rollbacks.rs b/src/rollbacks.rs index 14aecd43..3a44eebd 100644 --- a/src/rollbacks.rs +++ b/src/rollbacks.rs @@ -14,12 +14,27 @@ pub struct RollbackableHashMap { pub map: HashMap, } -impl RollbackableHashMap { +impl RollbackableHashMap { pub fn new() -> Self { Self { map: HashMap::new(), } } + + pub fn get_logs_after_snapshot( + &self, + snapshot: as Rollbackable>::Snapshot, + ) -> HashMap { + let mut changes = HashMap::new(); + + for (key, value) in self.map.iter() { + if !snapshot.contains_key(key) || snapshot.get(key).is_none() { + changes.insert(key.clone(), value.clone()); + } + } + + changes + } } impl Rollbackable for RollbackableHashMap { @@ -51,16 +66,24 @@ impl RollbackableVec { entries: Vec::new(), } } + + pub fn get_logs_after_snapshot( + &self, + snapshot: as Rollbackable>::Snapshot, + ) -> &[T] { + &self.entries[snapshot..] + } } impl Rollbackable for RollbackableVec { - type Snapshot = Vec; + type Snapshot = usize; fn rollback(&mut self, snapshot: Self::Snapshot) { - self.entries = snapshot; + self.entries.truncate(snapshot); } + fn snapshot(&self) -> Self::Snapshot { - self.entries.clone() + self.entries.len() } } From 1c9a4b2fe9d991432bf5e7709087f7995ac083ff Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 23:00:49 -0300 Subject: [PATCH 15/57] Add external snapshot(whole vm) --- src/execution.rs | 47 ++++++++++++++++++++++++++++++++++ src/state.rs | 65 ++++++++++++++++++++++++++++++++++++++++-------- src/vm.rs | 20 ++++++++++++++- 3 files changed, 121 insertions(+), 11 deletions(-) diff --git a/src/execution.rs b/src/execution.rs index 930dc8e0..a9b3c369 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -50,6 +50,22 @@ pub struct Execution { pub use_hooks: bool, } +#[derive(Debug, Clone, PartialEq)] +pub struct ExecutionSnapshot { + pub registers: [TaggedValue; 15], + pub flag_lt_of: bool, + pub flag_gt: bool, + pub flag_eq: bool, + pub running_contexts: Vec, + pub tx_number: u64, + pub heaps: Heaps, + pub register_context_u128: u128, + pub default_aa_code_hash: [u8; 32], + pub evm_interpreter_code_hash: [u8; 32], + pub hook_address: u32, + pub use_hooks: bool, +} + impl Execution { #[allow(clippy::too_many_arguments)] pub fn new( @@ -322,6 +338,37 @@ impl Execution { pub fn in_far_call(&self) -> bool { self.running_contexts.len() > 1 } + + pub fn snapshot(&self) -> ExecutionSnapshot { + ExecutionSnapshot { + default_aa_code_hash: self.default_aa_code_hash, + evm_interpreter_code_hash: self.evm_interpreter_code_hash, + flag_eq: self.flag_eq, + flag_gt: self.flag_gt, + flag_lt_of: self.flag_lt_of, + heaps: self.heaps.clone(), + hook_address: self.hook_address, + register_context_u128: self.register_context_u128, + registers: self.registers.clone(), + running_contexts: self.running_contexts.clone(), + tx_number: self.tx_number, + use_hooks: self.use_hooks, + } + } + pub fn rollback(&mut self, snapshot: ExecutionSnapshot) { + self.default_aa_code_hash = snapshot.default_aa_code_hash; + self.evm_interpreter_code_hash = snapshot.evm_interpreter_code_hash; + self.flag_eq = snapshot.flag_eq; + self.flag_gt = snapshot.flag_gt; + self.flag_lt_of = snapshot.flag_lt_of; + self.heaps = snapshot.heaps; + self.hook_address = snapshot.hook_address; + self.register_context_u128 = snapshot.register_context_u128; + self.registers = snapshot.registers; + self.running_contexts = snapshot.running_contexts; + self.tx_number = snapshot.tx_number; + self.use_hooks = snapshot.use_hooks; + } } impl Default for Stack { diff --git a/src/state.rs b/src/state.rs index f3b0244d..9ee717f1 100644 --- a/src/state.rs +++ b/src/state.rs @@ -77,6 +77,20 @@ impl VMState { } } + pub fn reset(&mut self) { + self.storage_changes = RollbackableHashMap::::default(); + self.transient_storage = RollbackableHashMap::::default(); + self.l2_to_l1_logs = RollbackableVec::::default(); + self.events = RollbackableVec::::default(); + self.pubdata = RollbackablePrimitive::::default(); + self.pubdata_costs = RollbackableVec::::default(); + self.paid_changes = RollbackableHashMap::::default(); + self.refunds = RollbackableVec::::default(); + self.read_storage_slots = RollbackableHashSet::::default(); + self.written_storage_slots = RollbackableHashSet::::default(); + self.decommitted_hashes = RollbackableHashSet::::default(); + } + pub fn storage_changes(&self) -> &HashMap { &self.storage_changes.map } @@ -231,14 +245,21 @@ impl VMState { // a copy of rollbackable fields pub struct StateSnapshot { // this casts allows us to get the Snapshot type from the Rollbackable trait - storage_changes: as Rollbackable>::Snapshot, - transient_storage: as Rollbackable>::Snapshot, - l2_to_l1_logs: as Rollbackable>::Snapshot, - events: as Rollbackable>::Snapshot, - pubdata: as Rollbackable>::Snapshot, - pubdata_costs: as Rollbackable>::Snapshot, - paid_changes: as Rollbackable>::Snapshot, - refunds: as Rollbackable>::Snapshot, + pub storage_changes: as Rollbackable>::Snapshot, + pub transient_storage: as Rollbackable>::Snapshot, + pub l2_to_l1_logs: as Rollbackable>::Snapshot, + pub events: as Rollbackable>::Snapshot, + pub pubdata: as Rollbackable>::Snapshot, + pub paid_changes: as Rollbackable>::Snapshot, +} + +pub struct FullStateSnapshot { + pub internal_snapshot: StateSnapshot, + pub pubdata_costs: as Rollbackable>::Snapshot, + pub refunds: as Rollbackable>::Snapshot, + pub decommited_hashes: as Rollbackable>::Snapshot, + pub read_storage_slots: as Rollbackable>::Snapshot, + pub written_storage_slots: as Rollbackable>::Snapshot, } impl Rollbackable for VMState { @@ -248,7 +269,9 @@ impl Rollbackable for VMState { self.storage_changes.rollback(snapshot.storage_changes); self.transient_storage.rollback(snapshot.transient_storage); self.l2_to_l1_logs.rollback(snapshot.l2_to_l1_logs); - self.events.rollback(snapshot.events) + self.events.rollback(snapshot.events); + self.pubdata.rollback(snapshot.pubdata); + self.paid_changes.rollback(snapshot.paid_changes); } fn snapshot(&self) -> Self::Snapshot { @@ -258,9 +281,31 @@ impl Rollbackable for VMState { transient_storage: self.transient_storage.snapshot(), events: self.events.snapshot(), pubdata: self.pubdata.snapshot(), - pubdata_costs: self.pubdata_costs.snapshot(), paid_changes: self.paid_changes.snapshot(), + } + } +} + +impl VMState { + pub fn external_rollback(&mut self, snapshot: FullStateSnapshot) { + self.rollback(snapshot.internal_snapshot); + self.pubdata_costs.rollback(snapshot.pubdata_costs); + self.refunds.rollback(snapshot.refunds); + self.decommitted_hashes.rollback(snapshot.decommited_hashes); + self.read_storage_slots + .rollback(snapshot.read_storage_slots); + self.written_storage_slots + .rollback(snapshot.written_storage_slots); + } + + pub fn full_state_snapshot(&self) -> FullStateSnapshot { + FullStateSnapshot { + internal_snapshot: self.snapshot(), + decommited_hashes: self.decommitted_hashes.snapshot(), + pubdata_costs: self.pubdata_costs.snapshot(), + read_storage_slots: self.read_storage_slots.snapshot(), refunds: self.refunds.snapshot(), + written_storage_slots: self.written_storage_slots.snapshot(), } } } diff --git a/src/vm.rs b/src/vm.rs index 19c4a5f4..f38adaa8 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -8,6 +8,7 @@ use zkevm_opcode_defs::{ use crate::address_operands::{address_operands_read, address_operands_store}; use crate::eravm_error::{HeapError, OpcodeError}; +use crate::execution::ExecutionSnapshot; use crate::op_handlers::add::add; use crate::op_handlers::and::and; use crate::op_handlers::aux_heap_read::aux_heap_read; @@ -41,7 +42,7 @@ use crate::op_handlers::shift::{rol, ror, shl, shr}; use crate::op_handlers::sub::sub; use crate::op_handlers::unimplemented::unimplemented; use crate::op_handlers::xor::xor; -use crate::state::VMState; +use crate::state::{FullStateSnapshot, VMState}; use crate::store::Storage; use crate::tracers::blob_saver_tracer::BlobSaverTracer; use crate::value::{FatPointer, TaggedValue}; @@ -62,6 +63,11 @@ pub struct EraVM { pub execution: Execution, } +pub struct VmSnapshot { + execution: ExecutionSnapshot, + state: FullStateSnapshot, +} + pub enum EncodingMode { Production, Testing, @@ -98,6 +104,18 @@ impl EraVM { (r, tracer) } + pub fn snapshot(&self) -> VmSnapshot { + VmSnapshot { + execution: self.execution.snapshot(), + state: self.state.full_state_snapshot(), + } + } + + pub fn rollback(&mut self, snapshot: VmSnapshot) { + self.execution.rollback(snapshot.execution); + self.state.external_rollback(snapshot.state); + } + /// Run a vm program from the given path using a custom state. /// Returns the value stored at storage with key 0 and the final vm state. pub fn program_from_file(&self, bin_path: &str) -> Result, EraVmError> { From f6f282915a2af0eedc3e44f5e86cfd0d0370d1a2 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 23:08:03 -0300 Subject: [PATCH 16/57] Add storage cache and util functio to get storage changes from initial --- src/state.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/state.rs b/src/state.rs index 9ee717f1..5f048ddf 100644 --- a/src/state.rs +++ b/src/state.rs @@ -57,6 +57,9 @@ pub struct VMState { read_storage_slots: RollbackableHashSet, written_storage_slots: RollbackableHashSet, decommitted_hashes: RollbackableHashSet, + + // Just a cache so that we don't query the db every time + pub initial_values: HashMap>, } impl VMState { @@ -74,6 +77,7 @@ impl VMState { read_storage_slots: RollbackableHashSet::::default(), written_storage_slots: RollbackableHashSet::::default(), decommitted_hashes: RollbackableHashSet::::default(), + initial_values: HashMap::default(), } } @@ -89,6 +93,7 @@ impl VMState { self.read_storage_slots = RollbackableHashSet::::default(); self.written_storage_slots = RollbackableHashSet::::default(); self.decommitted_hashes = RollbackableHashSet::::default(); + self.initial_values = HashMap::default(); } pub fn storage_changes(&self) -> &HashMap { @@ -164,6 +169,11 @@ impl VMState { pub fn storage_write(&mut self, key: StorageKey, value: U256) -> u32 { self.storage_changes.map.insert(key, value); + + self.initial_values + .entry(key) + .or_insert_with(|| self.storage.borrow_mut().storage_read(&key)); // caching the value + let mut storage = self.storage.borrow_mut(); if storage.is_free_storage_slot(&key) { @@ -239,6 +249,22 @@ impl VMState { pub fn decommitted_hashes(&self) -> &HashSet { &self.decommitted_hashes.map } + + /// returns the values that have actually changed from the initial storage + pub fn get_storage_changes(&self) -> Vec<(StorageKey, Option, U256)> { + self.storage_changes + .map + .iter() + .filter_map(|(key, &value)| { + let initial_value = self.initial_values.get(&key).unwrap_or(&None); + if initial_value.unwrap_or_default() == value { + None + } else { + Some((*key, *initial_value, value)) + } + }) + .collect() + } } #[derive(Clone, Default, PartialEq, Debug)] From 8747004c470ef0d74997e95fb3607e9737c1fcf0 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 23:41:47 -0300 Subject: [PATCH 17/57] Refactor state pub fields --- src/state.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/state.rs b/src/state.rs index 5f048ddf..f1ae665f 100644 --- a/src/state.rs +++ b/src/state.rs @@ -42,10 +42,10 @@ pub struct Event { #[derive(Debug, Clone)] pub struct VMState { pub storage: Rc>, - pub storage_changes: RollbackableHashMap, - pub transient_storage: RollbackableHashMap, - pub l2_to_l1_logs: RollbackableVec, - pub events: RollbackableVec, + storage_changes: RollbackableHashMap, + transient_storage: RollbackableHashMap, + l2_to_l1_logs: RollbackableVec, + events: RollbackableVec, // holds the sum of pubdata_costs pubdata: RollbackablePrimitive, pubdata_costs: RollbackableVec, @@ -108,10 +108,24 @@ impl VMState { &self.l2_to_l1_logs.entries } + pub fn get_l2_to_l1_logs_after_snapshot( + &self, + snapshot: as Rollbackable>::Snapshot, + ) -> &[L2ToL1Log] { + self.l2_to_l1_logs.get_logs_after_snapshot(snapshot) + } + pub fn events(&self) -> &Vec { &self.events.entries } + pub fn get_events_after_snapshot( + &self, + snapshot: as Rollbackable>::Snapshot, + ) -> &[Event] { + self.events.get_logs_after_snapshot(snapshot) + } + pub fn refunds(&self) -> &Vec { &self.refunds.entries } From 2a28162d72ecb7aee9a87ea393b6433729142486 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 19 Aug 2024 23:42:20 -0300 Subject: [PATCH 18/57] Add new function to get storage changes from snapshot or initial --- src/rollbacks.rs | 6 ++---- src/state.rs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/rollbacks.rs b/src/rollbacks.rs index 3a44eebd..5a496467 100644 --- a/src/rollbacks.rs +++ b/src/rollbacks.rs @@ -24,13 +24,11 @@ impl RollbackableHashMap { pub fn get_logs_after_snapshot( &self, snapshot: as Rollbackable>::Snapshot, - ) -> HashMap { + ) -> HashMap, V)> { let mut changes = HashMap::new(); for (key, value) in self.map.iter() { - if !snapshot.contains_key(key) || snapshot.get(key).is_none() { - changes.insert(key.clone(), value.clone()); - } + changes.insert(key.clone(), (snapshot.get(key).cloned(), value.clone())); } changes diff --git a/src/state.rs b/src/state.rs index f1ae665f..3dbfb463 100644 --- a/src/state.rs +++ b/src/state.rs @@ -279,6 +279,22 @@ impl VMState { }) .collect() } + + /// returns the values that have actually changed from the snapshot storage or the initial if the latter does not exist + /// also, a flag is also retured indicating if the value existed on the initial storage + pub fn get_storage_changes_from_snapshot( + &self, + snapshot: as Rollbackable>::Snapshot, + ) -> Vec<(StorageKey, (Option, U256, bool))> { + self.storage_changes + .get_logs_after_snapshot(snapshot) + .iter() + .map(|(key, (before, after))| { + let initial = self.initial_values.get(key).unwrap_or(&None); + (*key, (before.or(*initial), *after, initial.is_none())) + }) + .collect() + } } #[derive(Clone, Default, PartialEq, Debug)] From 41db1b5e555580db45a79f98248d9131df4db480 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 10:13:09 -0300 Subject: [PATCH 19/57] Update zksync-era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 67689581..a525e56a 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 67689581833f5994477b06c34e9d4ca41288edff +Subproject commit a525e56ab3ba6e9f1981c53cebdccee8497e5846 From fe7a666e0e5f98d978318615d9cfc2131f9280b9 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 12:10:39 -0300 Subject: [PATCH 20/57] Update era-compiler-tester submodule --- era-compiler-tester | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/era-compiler-tester b/era-compiler-tester index eab44db2..406bb803 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit eab44db21bd7b7e2ff907530133283477626ea02 +Subproject commit 406bb803e67ff1248f2bb0bf9ac4726e1466ffa1 From 18c5199278c7b7b2184b4247e54245ac5c778605 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 12:32:23 -0300 Subject: [PATCH 21/57] Address clippy warnings --- src/state.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/state.rs b/src/state.rs index 3dbfb463..31985e29 100644 --- a/src/state.rs +++ b/src/state.rs @@ -270,7 +270,7 @@ impl VMState { .map .iter() .filter_map(|(key, &value)| { - let initial_value = self.initial_values.get(&key).unwrap_or(&None); + let initial_value = self.initial_values.get(key).unwrap_or(&None); if initial_value.unwrap_or_default() == value { None } else { @@ -280,18 +280,18 @@ impl VMState { .collect() } - /// returns the values that have actually changed from the snapshot storage or the initial if the latter does not exist + /// returns the values that have actually changed from the snapshot storage or the initial if the former does not exist /// also, a flag is also retured indicating if the value existed on the initial storage pub fn get_storage_changes_from_snapshot( &self, snapshot: as Rollbackable>::Snapshot, - ) -> Vec<(StorageKey, (Option, U256, bool))> { + ) -> Vec<(StorageKey, Option, U256, bool)> { self.storage_changes .get_logs_after_snapshot(snapshot) .iter() .map(|(key, (before, after))| { let initial = self.initial_values.get(key).unwrap_or(&None); - (*key, (before.or(*initial), *after, initial.is_none())) + (*key, before.or(*initial), *after, initial.is_none()) }) .collect() } From 897eb161c88ffc6fb33af3cd5f729b67d03b2c9e Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 12:33:13 -0300 Subject: [PATCH 22/57] More clippy warnings --- src/call_frame.rs | 2 +- src/execution.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/call_frame.rs b/src/call_frame.rs index 348190e2..6efbe484 100644 --- a/src/call_frame.rs +++ b/src/call_frame.rs @@ -98,7 +98,7 @@ impl CallFrame { ) -> Self { Self { pc: 0, - stipend: stipend, + stipend, gas_left: Saturating(gas), exception_handler, sp: 0, diff --git a/src/execution.rs b/src/execution.rs index a9b3c369..fe1675c2 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -349,7 +349,7 @@ impl Execution { heaps: self.heaps.clone(), hook_address: self.hook_address, register_context_u128: self.register_context_u128, - registers: self.registers.clone(), + registers: self.registers, running_contexts: self.running_contexts.clone(), tx_number: self.tx_number, use_hooks: self.use_hooks, From 4efc0669d9885d9c4ce1ebaa72a0b47ded4fceef Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 13:12:57 -0300 Subject: [PATCH 23/57] Update zksync-era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index a525e56a..93ba1389 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit a525e56ab3ba6e9f1981c53cebdccee8497e5846 +Subproject commit 93ba13893c2f6e057ddd9c5ef7947891abb23f6c From b512b315aae5be18b639c80f710be7a7408cdcab Mon Sep 17 00:00:00 2001 From: Fran Date: Tue, 20 Aug 2024 15:04:10 -0300 Subject: [PATCH 24/57] ci: add submodule step for era --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 263c3e5a..cf2a7844 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -450,7 +450,7 @@ jobs: with: path: ${{ github.workspace }}/era_vm - - name: Setup compiler-tester submodule + - name: Setup era submodule working-directory: ${{ github.workspace }}/era_vm run: make submodules From b4c8773e51757ec70e2deedc11dd2f0b07035a2b Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Tue, 20 Aug 2024 15:05:51 -0300 Subject: [PATCH 25/57] initial commit --- src/rollbacks.rs | 56 ++++++++++++++++++++++++++--- src/state.rs | 94 ++++++++++++++++++++++++------------------------ 2 files changed, 100 insertions(+), 50 deletions(-) diff --git a/src/rollbacks.rs b/src/rollbacks.rs index 5a496467..0429dbe0 100644 --- a/src/rollbacks.rs +++ b/src/rollbacks.rs @@ -11,7 +11,7 @@ pub trait Rollbackable { #[derive(Debug, Default, Clone)] pub struct RollbackableHashMap { - pub map: HashMap, + map: HashMap, } impl RollbackableHashMap { @@ -21,6 +21,18 @@ impl RollbackableHashMap { } } + pub fn insert(&mut self, key: K, value: V) { + self.map.insert(key, value); + } + + pub fn get(&self, key: &K) -> Option<&V> { + self.map.get(key) + } + + pub fn inner_ref(&self) -> &HashMap { + &self.map + } + pub fn get_logs_after_snapshot( &self, snapshot: as Rollbackable>::Snapshot, @@ -55,7 +67,7 @@ impl Iterator for RollbackableHashMap { #[derive(Debug, Default, Clone)] pub struct RollbackableVec { - pub entries: Vec, + entries: Vec, } impl RollbackableVec { @@ -65,6 +77,14 @@ impl RollbackableVec { } } + pub fn push(&mut self, value: T) { + self.entries.push(value); + } + + pub fn entries(&self) -> &[T] { + &self.entries + } + pub fn get_logs_after_snapshot( &self, snapshot: as Rollbackable>::Snapshot, @@ -87,7 +107,7 @@ impl Rollbackable for RollbackableVec { #[derive(Debug, Default, Clone)] pub struct RollbackablePrimitive { - pub value: T, + value: T, } impl Rollbackable for RollbackablePrimitive { @@ -101,9 +121,23 @@ impl Rollbackable for RollbackablePrimitive { } } +impl RollbackablePrimitive { + pub fn new(value: T) -> Self { + Self { value } + } + + pub fn value(&self) -> T { + self.value + } + + pub fn set(&mut self, value: T) { + self.value = value; + } +} + #[derive(Debug, Default, Clone)] pub struct RollbackableHashSet { - pub map: HashSet, + map: HashSet, } impl Rollbackable for RollbackableHashSet { @@ -116,3 +150,17 @@ impl Rollbackable for RollbackableHashSet { self.map.clone() } } + +impl RollbackableHashSet { + pub fn insert(&mut self, value: K) -> bool { + self.map.insert(value) + } + + pub fn contains(&self, value: &K) -> bool { + self.map.contains(value) + } + + pub fn inner_ref(&self) -> &HashSet { + &self.map + } +} diff --git a/src/state.rs b/src/state.rs index 31985e29..b6d1dfac 100644 --- a/src/state.rs +++ b/src/state.rs @@ -97,15 +97,15 @@ impl VMState { } pub fn storage_changes(&self) -> &HashMap { - &self.storage_changes.map + self.storage_changes.inner_ref() } pub fn transient_storage(&self) -> &HashMap { - &self.transient_storage.map + self.transient_storage.inner_ref() } - pub fn l2_to_l1_logs(&self) -> &Vec { - &self.l2_to_l1_logs.entries + pub fn l2_to_l1_logs(&self) -> &[L2ToL1Log] { + self.l2_to_l1_logs.entries() } pub fn get_l2_to_l1_logs_after_snapshot( @@ -115,8 +115,8 @@ impl VMState { self.l2_to_l1_logs.get_logs_after_snapshot(snapshot) } - pub fn events(&self) -> &Vec { - &self.events.entries + pub fn events(&self) -> &[Event] { + self.events.entries() } pub fn get_events_after_snapshot( @@ -126,20 +126,21 @@ impl VMState { self.events.get_logs_after_snapshot(snapshot) } - pub fn refunds(&self) -> &Vec { - &self.refunds.entries + pub fn refunds(&self) -> &[u32] { + self.refunds.entries() } - pub fn pubdata_costs(&self) -> &Vec { - &self.pubdata_costs.entries + pub fn pubdata_costs(&self) -> &[i32] { + self.pubdata_costs.entries() } pub fn pubdata(&self) -> i32 { - self.pubdata.value + self.pubdata.value() } pub fn add_pubdata(&mut self, to_add: i32) { - self.pubdata.value += to_add; + let previous = self.pubdata.value(); + self.pubdata.set(previous + to_add); } pub fn storage_read(&mut self, key: StorageKey) -> (U256, u32) { @@ -147,16 +148,16 @@ impl VMState { let storage = self.storage.borrow(); - let refund = - if storage.is_free_storage_slot(&key) || self.read_storage_slots.map.contains(&key) { - WARM_READ_REFUND - } else { - self.read_storage_slots.map.insert(key); - 0 - }; + let refund = if storage.is_free_storage_slot(&key) || self.read_storage_slots.contains(&key) + { + WARM_READ_REFUND + } else { + self.read_storage_slots.insert(key); + 0 + }; - self.pubdata_costs.entries.push(0); - self.refunds.entries.push(refund); + self.pubdata_costs.push(0); + self.refunds.push(refund); (value, refund) } @@ -165,24 +166,24 @@ impl VMState { let value = self.storage_read_inner(&key).unwrap_or_default(); let storage = self.storage.borrow(); - if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) { - self.read_storage_slots.map.insert(key); + if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.contains(&key) { + self.read_storage_slots.insert(key); }; - self.pubdata_costs.entries.push(0); + self.pubdata_costs.push(0); value } fn storage_read_inner(&self, key: &StorageKey) -> Option { - match self.storage_changes.map.get(key) { + match self.storage_changes.get(key) { None => self.storage.borrow_mut().storage_read(key), value => value.copied(), } } pub fn storage_write(&mut self, key: StorageKey, value: U256) -> u32 { - self.storage_changes.map.insert(key, value); + self.storage_changes.insert(key, value); self.initial_values .entry(key) @@ -192,8 +193,8 @@ impl VMState { if storage.is_free_storage_slot(&key) { let refund = WARM_WRITE_REFUND; - self.refunds.entries.push(refund); - self.pubdata_costs.entries.push(0); + self.refunds.push(refund); + self.pubdata_costs.push(0); return refund; } @@ -201,18 +202,18 @@ impl VMState { // on subsequent writes, we don't charge for what has already been paid // but for the newer price, which if it is lower might end up in a refund let current_cost = storage.cost_of_writing_storage(&key, value); - let prev_cost = *self.paid_changes.map.get(&key).unwrap_or(&0); - self.paid_changes.map.insert(key, current_cost); + let prev_cost = *self.paid_changes.get(&key).unwrap_or(&0); + self.paid_changes.insert(key, current_cost); - let refund = if self.written_storage_slots.map.contains(&key) { + let refund = if self.written_storage_slots.contains(&key) { WARM_WRITE_REFUND } else { - self.written_storage_slots.map.insert(key); + self.written_storage_slots.insert(key); - if self.read_storage_slots.map.contains(&key) { + if self.read_storage_slots.contains(&key) { COLD_WRITE_AFTER_WARM_READ_REFUND } else { - self.read_storage_slots.map.insert(key); + self.read_storage_slots.insert(key); 0 } }; @@ -222,25 +223,26 @@ impl VMState { // the slots to their final values. // The only case where users may overpay is when a previous transaction ends up with a negative pubdata total. let pubdata_cost = (current_cost as i32) - (prev_cost as i32); - self.pubdata.value += pubdata_cost; - self.refunds.entries.push(refund); - self.pubdata_costs.entries.push(pubdata_cost); + let previous_pubdata = self.pubdata.value(); + self.pubdata.set(previous_pubdata + pubdata_cost); + self.refunds.push(refund); + self.pubdata_costs.push(pubdata_cost); refund } pub fn transient_storage_read(&mut self, key: StorageKey) -> U256 { - self.pubdata_costs.entries.push(0); + self.pubdata_costs.push(0); self.transient_storage - .map + .inner_ref() .get(&key) .copied() .unwrap_or_default() } pub fn transient_storage_write(&mut self, key: StorageKey, value: U256) { - self.pubdata_costs.entries.push(0); - self.transient_storage.map.insert(key, value); + self.pubdata_costs.push(0); + self.transient_storage.insert(key, value); } pub(crate) fn clear_transient_storage(&mut self) { @@ -248,26 +250,26 @@ impl VMState { } pub fn record_l2_to_l1_log(&mut self, msg: L2ToL1Log) { - self.l2_to_l1_logs.entries.push(msg); + self.l2_to_l1_logs.push(msg); } pub fn record_event(&mut self, event: Event) { - self.events.entries.push(event); + self.events.push(event); } pub fn decommit(&mut self, hash: U256) -> (Option>, bool) { - let was_decommitted = !self.decommitted_hashes.map.insert(hash); + let was_decommitted = !self.decommitted_hashes.insert(hash); (self.storage.borrow_mut().decommit(hash), was_decommitted) } pub fn decommitted_hashes(&self) -> &HashSet { - &self.decommitted_hashes.map + self.decommitted_hashes.inner_ref() } /// returns the values that have actually changed from the initial storage pub fn get_storage_changes(&self) -> Vec<(StorageKey, Option, U256)> { self.storage_changes - .map + .inner_ref() .iter() .filter_map(|(key, &value)| { let initial_value = self.initial_values.get(key).unwrap_or(&None); From a5700ea2ab693cc9245c0614f1bed8542878a19e Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Tue, 20 Aug 2024 17:24:08 -0300 Subject: [PATCH 26/57] Address review comments --- src/execution.rs | 4 +++- src/op_handlers/far_call.rs | 2 +- src/op_handlers/ret.rs | 14 ++++++++++++-- src/rollbacks.rs | 2 ++ src/state.rs | 13 +------------ 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/execution.rs b/src/execution.rs index fe1675c2..6a344b31 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -29,6 +29,7 @@ pub struct Heap { } #[derive(Debug, Clone, PartialEq)] +// represents the vm execution state pub struct Execution { // The first register, r0, is actually always zero and not really used. // Writing to it does nothing. @@ -51,6 +52,7 @@ pub struct Execution { } #[derive(Debug, Clone, PartialEq)] +// a saved state of the vm execution pub struct ExecutionSnapshot { pub registers: [TaggedValue; 15], pub flag_lt_of: bool, @@ -318,7 +320,7 @@ impl Execution { } pub fn increase_gas(&mut self, to_add: u32) -> Result<(), EraVmError> { - self.current_frame_mut()?.gas_left += to_add; + self.current_frame_mut()?.gas_left += Saturating(to_add); Ok(()) } diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index 673e8de4..bfa3e394 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -252,7 +252,7 @@ pub fn far_call( 0 }; - // mandated gas can surprass the 64/63 limit + // mandated gas can surprass the 63/64 limit let ergs_passed = ergs_passed + mandated_gas; vm.decrease_gas(ergs_passed)?; diff --git a/src/op_handlers/ret.rs b/src/op_handlers/ret.rs index eca77d44..14baad72 100644 --- a/src/op_handlers/ret.rs +++ b/src/op_handlers/ret.rs @@ -63,7 +63,12 @@ pub fn ret( vm.clear_registers(); vm.set_register(1, result); let previous_frame = vm.pop_frame()?; - vm.current_frame_mut()?.gas_left += previous_frame.gas_left.0 - previous_frame.stipend; + vm.increase_gas( + previous_frame + .gas_left + .0 + .saturating_sub(previous_frame.stipend), + )?; if is_failure { state.rollback(previous_frame.snapshot); vm.current_frame_mut()?.pc = previous_frame.exception_handler; @@ -104,7 +109,12 @@ pub fn inexplicit_panic(vm: &mut Execution, state: &mut VMState) -> Result RollbackableVec { } impl Rollbackable for RollbackableVec { + // here, we can avoid cloning and we can just store the length since we never pop entries + // and we always push at the end type Snapshot = usize; fn rollback(&mut self, snapshot: Self::Snapshot) { diff --git a/src/state.rs b/src/state.rs index 31985e29..d851b22f 100644 --- a/src/state.rs +++ b/src/state.rs @@ -82,18 +82,7 @@ impl VMState { } pub fn reset(&mut self) { - self.storage_changes = RollbackableHashMap::::default(); - self.transient_storage = RollbackableHashMap::::default(); - self.l2_to_l1_logs = RollbackableVec::::default(); - self.events = RollbackableVec::::default(); - self.pubdata = RollbackablePrimitive::::default(); - self.pubdata_costs = RollbackableVec::::default(); - self.paid_changes = RollbackableHashMap::::default(); - self.refunds = RollbackableVec::::default(); - self.read_storage_slots = RollbackableHashSet::::default(); - self.written_storage_slots = RollbackableHashSet::::default(); - self.decommitted_hashes = RollbackableHashSet::::default(); - self.initial_values = HashMap::default(); + *self = Self::new(self.storage.clone()); } pub fn storage_changes(&self) -> &HashMap { From c30ea738fa36fe859fbf142eece59437af6303a2 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Wed, 21 Aug 2024 10:07:44 -0300 Subject: [PATCH 27/57] Add cache to storage_read --- src/state.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/state.rs b/src/state.rs index d851b22f..5911ba0b 100644 --- a/src/state.rs +++ b/src/state.rs @@ -163,9 +163,17 @@ impl VMState { value } - fn storage_read_inner(&self, key: &StorageKey) -> Option { + fn storage_read_inner(&mut self, key: &StorageKey) -> Option { match self.storage_changes.map.get(key) { - None => self.storage.borrow_mut().storage_read(key), + None => { + let cached_value = *self.initial_values.get(key).unwrap_or(&None); + if cached_value.is_none() { + let value = self.storage.borrow_mut().storage_read(key); + self.initial_values.insert(*key, value); + return value; + } + cached_value + } value => value.copied(), } } From cd451dd1c8d25f4dd2c462bc7aaaa7a4ae28c4a2 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 15:37:15 -0300 Subject: [PATCH 28/57] Update zksync-era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 93ba1389..9154b1d7 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 93ba13893c2f6e057ddd9c5ef7947891abb23f6c +Subproject commit 9154b1d77fa22ec114e4d9c7e2ab05740bcd9f5b From d0c59af1a567ff5960e819ba707056718c20bc77 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 17:48:10 -0300 Subject: [PATCH 29/57] Add full tracers implementation --- src/lib.rs | 2 +- src/tracers/blob_saver_tracer.rs | 58 +++++++++++++++++--------- src/tracers/last_state_saver_tracer.rs | 30 +++++++++++-- src/tracers/print_tracer.rs | 35 ++++++++++++---- src/tracers/tracer.rs | 13 +++--- src/vm.rs | 13 +++++- 6 files changed, 110 insertions(+), 41 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 73c1a451..2409b16e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,7 @@ mod eravm_error; pub mod execution; pub mod heaps; mod op_handlers; -mod opcode; +pub mod opcode; pub mod output; mod precompiles; mod ptr_operator; diff --git a/src/tracers/blob_saver_tracer.rs b/src/tracers/blob_saver_tracer.rs index dd86ba70..9d463344 100644 --- a/src/tracers/blob_saver_tracer.rs +++ b/src/tracers/blob_saver_tracer.rs @@ -1,10 +1,5 @@ use super::tracer::Tracer; -use crate::{ - eravm_error::{EraVmError, HeapError}, - execution::Execution, - value::FatPointer, - Opcode, -}; +use crate::{execution::Execution, state::VMState, value::FatPointer, Opcode}; use std::collections::HashMap; use u256::{H160, H256, U256}; use zkevm_opcode_defs::ethereum_types::Address; @@ -53,8 +48,12 @@ fn hash_evm_bytecode(bytecode: &[u8]) -> H256 { } impl Tracer for BlobSaverTracer { - fn before_execution(&mut self, _opcode: &Opcode, vm: &mut Execution) -> Result<(), EraVmError> { - let current_callstack = vm.current_context()?; + fn before_execution(&mut self, _opcode: &Opcode, vm: &mut Execution, _state: &mut VMState) { + let current_callstack = vm.current_context(); + if current_callstack.is_err() { + return; + } + let current_callstack = current_callstack.unwrap(); // Here we assume that the only case when PC is 0 at the start of the execution of the contract. let known_code_storage_call = current_callstack.code_address == KNOWN_CODES_STORAGE_ADDRESS @@ -63,38 +62,43 @@ impl Tracer for BlobSaverTracer { if !known_code_storage_call { // Leave - return Ok(()); + return; } // Now, we need to check whether it is indeed a call to publish EVM code. let calldata_ptr = vm.get_register(1); if !calldata_ptr.is_pointer { - return Ok(()); + return; } let ptr = FatPointer::decode(calldata_ptr.value); - let data = vm - .heaps - .get(ptr.page) - .ok_or(HeapError::ReadOutOfBounds)? - .read_unaligned_from_pointer(&ptr)?; + let heap = vm.heaps.get(ptr.page); + if heap.is_none() { + return; + } + + let data = heap.unwrap().read_unaligned_from_pointer(&ptr); + if data.is_err() { + return; + } + let data = data.unwrap(); if data.len() < 64 { // Not interested - return Ok(()); + return; } let (signature, data) = data.split_at(4); if signature != PUBLISH_BYTECODE_SIGNATURE { - return Ok(()); + return; } let (_, published_bytecode) = data.split_at(64); if published_bytecode.len() % 32 != 0 { eprintln!("Invalid bytecode length"); - return Ok(()); + return; } let hash = hash_evm_bytecode(published_bytecode); @@ -106,7 +110,23 @@ impl Tracer for BlobSaverTracer { let key = U256::from_big_endian(hash.as_bytes()); self.blobs.insert(key, as_words); + } + + fn after_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + + fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} - Ok(()) + fn after_decoding( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { } } diff --git a/src/tracers/last_state_saver_tracer.rs b/src/tracers/last_state_saver_tracer.rs index 5efbd292..76a2df50 100644 --- a/src/tracers/last_state_saver_tracer.rs +++ b/src/tracers/last_state_saver_tracer.rs @@ -1,4 +1,4 @@ -use crate::eravm_error::EraVmError; +use crate::state::VMState; use crate::{execution::Execution, Opcode}; use super::tracer::Tracer; @@ -38,10 +38,32 @@ impl Default for LastStateSaverTracer { } impl Tracer for LastStateSaverTracer { - fn before_execution(&mut self, opcode: &Opcode, vm: &mut Execution) -> Result<(), EraVmError> { + fn before_execution( + &mut self, + opcode: &Opcode, + execution: &mut Execution, + _state: &mut VMState, + ) { if opcode.variant == Variant::Ret(RetOpcode::Ok) { - self.vm_state = vm.clone(); + self.vm_state = execution.clone(); } - Ok(()) + } + + fn after_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + + fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} + + fn after_decoding( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { } } diff --git a/src/tracers/print_tracer.rs b/src/tracers/print_tracer.rs index 02054e13..1c9e8d95 100644 --- a/src/tracers/print_tracer.rs +++ b/src/tracers/print_tracer.rs @@ -3,8 +3,8 @@ use zkevm_opcode_defs::Opcode as ZKOpcode; use zkevm_opcode_defs::UMAOpcode; use crate::address_operands::address_operands_read; -use crate::eravm_error::EraVmError; use crate::eravm_error::HeapError; +use crate::state::VMState; use crate::value::FatPointer; use crate::{execution::Execution, Opcode}; @@ -14,7 +14,7 @@ pub struct PrintTracer {} impl Tracer for PrintTracer { #[allow(clippy::println_empty_string)] - fn before_execution(&mut self, opcode: &Opcode, vm: &mut Execution) -> Result<(), EraVmError> { + fn before_execution(&mut self, opcode: &Opcode, vm: &mut Execution, _state: &mut VMState) { let opcode_variant = opcode.variant; const DEBUG_SLOT: u32 = 1024; @@ -25,21 +25,23 @@ impl Tracer for PrintTracer { .unwrap(); if matches!(opcode_variant, ZKOpcode::UMA(UMAOpcode::HeapWrite)) { - let (src0, src1) = address_operands_read(vm, opcode)?; + let (src0, src1) = address_operands_read(vm, opcode).unwrap(); let value = src1.value; if value == debug_magic { let fat_ptr = FatPointer::decode(src0.value); if fat_ptr.offset == DEBUG_SLOT { let how_to_print_value = vm .heaps - .get(vm.current_context()?.heap_id) - .ok_or(HeapError::ReadOutOfBounds)? + .get(vm.current_context().unwrap().heap_id) + .ok_or(HeapError::ReadOutOfBounds) + .unwrap() .read(DEBUG_SLOT + 32); let value_to_print = vm .heaps - .get(vm.current_context()?.heap_id) - .ok_or(HeapError::ReadOutOfBounds)? + .get(vm.current_context().unwrap().heap_id) + .ok_or(HeapError::ReadOutOfBounds) + .unwrap() .read(DEBUG_SLOT + 64); let print_as_hex_value = U256::from_str_radix( @@ -73,6 +75,23 @@ impl Tracer for PrintTracer { } } } - Ok(()) + } + + fn after_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + + fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} + + fn after_decoding( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { } } diff --git a/src/tracers/tracer.rs b/src/tracers/tracer.rs index 92365b98..6acb116f 100644 --- a/src/tracers/tracer.rs +++ b/src/tracers/tracer.rs @@ -1,11 +1,8 @@ -use crate::{eravm_error::EraVmError, execution::Execution, Opcode}; +use crate::{execution::Execution, state::VMState, Opcode}; pub trait Tracer { - fn before_execution( - &mut self, - _opcode: &Opcode, - _vm: &mut Execution, - ) -> Result<(), EraVmError> { - Ok(()) - } + fn before_decoding(&mut self, execution: &mut Execution, state: &mut VMState); + fn after_decoding(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); + fn before_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); + fn after_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); } diff --git a/src/vm.rs b/src/vm.rs index f38adaa8..0a5b35aa 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -145,12 +145,19 @@ impl EraVM { enc_mode: EncodingMode, ) -> Result { loop { + for tracer in tracers.iter_mut() { + tracer.before_decoding(&mut self.execution, &mut self.state); + } let opcode = match enc_mode { EncodingMode::Testing => self.execution.get_opcode_with_test_encode()?, EncodingMode::Production => self.execution.get_opcode()?, }; for tracer in tracers.iter_mut() { - tracer.before_execution(&opcode, &mut self.execution)?; + tracer.after_decoding(&opcode, &mut self.execution, &mut self.state); + } + + for tracer in tracers.iter_mut() { + tracer.before_execution(&opcode, &mut self.execution, &mut self.state); } let can_execute = self.execution.can_execute(&opcode); @@ -322,6 +329,10 @@ impl EraVM { } else { self.execution.current_frame_mut()?.pc += 1; } + + for tracer in tracers.iter_mut() { + tracer.after_execution(&opcode, &mut self.execution, &mut self.state); + } } } } From 855d043c4c81021bff3626c697cdc2bbf06c8a45 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 18:03:49 -0300 Subject: [PATCH 30/57] Refactor how the vm handles tracers --- src/tracers/mod.rs | 1 + src/tracers/no_tracer.rs | 20 ++++++++++++++++ src/vm.rs | 51 ++++++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 25 deletions(-) create mode 100644 src/tracers/no_tracer.rs diff --git a/src/tracers/mod.rs b/src/tracers/mod.rs index 99ffd5b6..a53838a4 100644 --- a/src/tracers/mod.rs +++ b/src/tracers/mod.rs @@ -1,4 +1,5 @@ pub mod blob_saver_tracer; pub mod last_state_saver_tracer; +pub mod no_tracer; pub mod print_tracer; pub mod tracer; diff --git a/src/tracers/no_tracer.rs b/src/tracers/no_tracer.rs new file mode 100644 index 00000000..bfa7d48d --- /dev/null +++ b/src/tracers/no_tracer.rs @@ -0,0 +1,20 @@ +use super::tracer::Tracer; +use crate::state::VMState; +use crate::{execution::Execution, Opcode}; + +#[derive(Default)] +pub struct NoTracer {} + +impl Tracer for NoTracer { + fn before_decoding(&mut self, execution: &mut Execution, state: &mut VMState) {} + fn after_decoding(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState) {} + fn before_execution( + &mut self, + opcode: &Opcode, + execution: &mut Execution, + state: &mut VMState, + ) { + } + fn after_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState) { + } +} diff --git a/src/vm.rs b/src/vm.rs index 0a5b35aa..4af05d9b 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -44,7 +44,7 @@ use crate::op_handlers::unimplemented::unimplemented; use crate::op_handlers::xor::xor; use crate::state::{FullStateSnapshot, VMState}; use crate::store::Storage; -use crate::tracers::blob_saver_tracer::BlobSaverTracer; +use crate::tracers::no_tracer::NoTracer; use crate::value::{FatPointer, TaggedValue}; use crate::{eravm_error::EraVmError, tracers::tracer::Tracer, Execution}; use crate::{Opcode, Variant}; @@ -82,26 +82,36 @@ impl EraVM { } /// Run a vm program with a given bytecode. - pub fn run_program_with_custom_bytecode(&mut self) -> (ExecutionOutput, BlobSaverTracer) { - self.run_opcodes() + pub fn run_program_with_custom_bytecode( + &mut self, + tracer: Option<&mut dyn Tracer>, + ) -> ExecutionOutput { + self.run_opcodes(tracer) } - pub fn run_program_with_test_encode(&mut self) -> (ExecutionOutput, BlobSaverTracer) { - let mut tracer = BlobSaverTracer::new(); + pub fn run_program_with_test_encode( + &mut self, + tracer: Option<&mut dyn Tracer>, + ) -> ExecutionOutput { let r = self - .run(&mut [Box::new(&mut tracer)], EncodingMode::Testing) + .run( + tracer.unwrap_or(&mut NoTracer::default()), + EncodingMode::Testing, + ) .unwrap_or(ExecutionOutput::Panic); - (r, tracer) + r } - fn run_opcodes(&mut self) -> (ExecutionOutput, BlobSaverTracer) { - let mut tracer = BlobSaverTracer::new(); + fn run_opcodes(&mut self, tracer: Option<&mut dyn Tracer>) -> ExecutionOutput { let r = self - .run(&mut [Box::new(&mut tracer)], EncodingMode::Production) + .run( + tracer.unwrap_or(&mut NoTracer::default()), + EncodingMode::Production, + ) .unwrap_or(ExecutionOutput::Panic); - (r, tracer) + r } pub fn snapshot(&self) -> VmSnapshot { @@ -141,25 +151,18 @@ impl EraVM { #[allow(non_upper_case_globals)] pub fn run( &mut self, - tracers: &mut [Box<&mut dyn Tracer>], + tracer: &mut dyn Tracer, enc_mode: EncodingMode, ) -> Result { loop { - for tracer in tracers.iter_mut() { - tracer.before_decoding(&mut self.execution, &mut self.state); - } + tracer.before_decoding(&mut self.execution, &mut self.state); let opcode = match enc_mode { EncodingMode::Testing => self.execution.get_opcode_with_test_encode()?, EncodingMode::Production => self.execution.get_opcode()?, }; - for tracer in tracers.iter_mut() { - tracer.after_decoding(&opcode, &mut self.execution, &mut self.state); - } - - for tracer in tracers.iter_mut() { - tracer.before_execution(&opcode, &mut self.execution, &mut self.state); - } + tracer.after_decoding(&opcode, &mut self.execution, &mut self.state); + tracer.before_execution(&opcode, &mut self.execution, &mut self.state); let can_execute = self.execution.can_execute(&opcode); if self.execution.decrease_gas(opcode.gas_cost).is_err() || can_execute.is_err() { @@ -330,9 +333,7 @@ impl EraVM { self.execution.current_frame_mut()?.pc += 1; } - for tracer in tracers.iter_mut() { - tracer.after_execution(&opcode, &mut self.execution, &mut self.state); - } + tracer.after_execution(&opcode, &mut self.execution, &mut self.state); } } } From 439f68e1dcd43116b0220778fc6f89afd2f25fd1 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 18:11:03 -0300 Subject: [PATCH 31/57] Update era-compiler-tester submodule --- era-compiler-tester | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/era-compiler-tester b/era-compiler-tester index 406bb803..3500a561 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit 406bb803e67ff1248f2bb0bf9ac4726e1466ffa1 +Subproject commit 3500a56143a279ac026fcf0a36fea95ef7671cc4 From e5ab6042bdb5665502a864fc670b47c68bbc9a11 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 18:44:05 -0300 Subject: [PATCH 32/57] Update zksync-era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 9154b1d7..ff82b18b 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 9154b1d77fa22ec114e4d9c7e2ab05740bcd9f5b +Subproject commit ff82b18b023a95b717e769af28dab2beaad260c1 From 6d1682ef79bb645fc8eedbcfbee75ca05b0c706c Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 22 Aug 2024 18:44:56 -0300 Subject: [PATCH 33/57] Address clippy warnings --- src/tracers/no_tracer.rs | 26 ++++++++++++++++++++------ src/vm.rs | 26 ++++++++++---------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/tracers/no_tracer.rs b/src/tracers/no_tracer.rs index bfa7d48d..8477a11a 100644 --- a/src/tracers/no_tracer.rs +++ b/src/tracers/no_tracer.rs @@ -6,15 +6,29 @@ use crate::{execution::Execution, Opcode}; pub struct NoTracer {} impl Tracer for NoTracer { - fn before_decoding(&mut self, execution: &mut Execution, state: &mut VMState) {} - fn after_decoding(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState) {} + fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} + + fn after_decoding( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + fn before_execution( &mut self, - opcode: &Opcode, - execution: &mut Execution, - state: &mut VMState, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, ) { } - fn after_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState) { + + fn after_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { } } diff --git a/src/vm.rs b/src/vm.rs index 4af05d9b..15862d1b 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -93,25 +93,19 @@ impl EraVM { &mut self, tracer: Option<&mut dyn Tracer>, ) -> ExecutionOutput { - let r = self - .run( - tracer.unwrap_or(&mut NoTracer::default()), - EncodingMode::Testing, - ) - .unwrap_or(ExecutionOutput::Panic); - - r + self.run( + tracer.unwrap_or(&mut NoTracer::default()), + EncodingMode::Testing, + ) + .unwrap_or(ExecutionOutput::Panic) } fn run_opcodes(&mut self, tracer: Option<&mut dyn Tracer>) -> ExecutionOutput { - let r = self - .run( - tracer.unwrap_or(&mut NoTracer::default()), - EncodingMode::Production, - ) - .unwrap_or(ExecutionOutput::Panic); - - r + self.run( + tracer.unwrap_or(&mut NoTracer::default()), + EncodingMode::Production, + ) + .unwrap_or(ExecutionOutput::Panic) } pub fn snapshot(&self) -> VmSnapshot { From 0cc8684734923fd68ebb5de7855668e0093daf66 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 23 Aug 2024 10:15:51 -0300 Subject: [PATCH 34/57] Add default function in Tracer trait --- src/tracers/blob_saver_tracer.rs | 18 ----------------- src/tracers/last_state_saver_tracer.rs | 18 ----------------- src/tracers/no_tracer.rs | 28 +------------------------- src/tracers/print_tracer.rs | 18 ----------------- src/tracers/tracer.rs | 26 ++++++++++++++++++++---- 5 files changed, 23 insertions(+), 85 deletions(-) diff --git a/src/tracers/blob_saver_tracer.rs b/src/tracers/blob_saver_tracer.rs index 9d463344..d04c30c9 100644 --- a/src/tracers/blob_saver_tracer.rs +++ b/src/tracers/blob_saver_tracer.rs @@ -111,22 +111,4 @@ impl Tracer for BlobSaverTracer { self.blobs.insert(key, as_words); } - - fn after_execution( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } - - fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} - - fn after_decoding( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } } diff --git a/src/tracers/last_state_saver_tracer.rs b/src/tracers/last_state_saver_tracer.rs index 76a2df50..220603e8 100644 --- a/src/tracers/last_state_saver_tracer.rs +++ b/src/tracers/last_state_saver_tracer.rs @@ -48,22 +48,4 @@ impl Tracer for LastStateSaverTracer { self.vm_state = execution.clone(); } } - - fn after_execution( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } - - fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} - - fn after_decoding( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } } diff --git a/src/tracers/no_tracer.rs b/src/tracers/no_tracer.rs index 8477a11a..e9f54b3c 100644 --- a/src/tracers/no_tracer.rs +++ b/src/tracers/no_tracer.rs @@ -5,30 +5,4 @@ use crate::{execution::Execution, Opcode}; #[derive(Default)] pub struct NoTracer {} -impl Tracer for NoTracer { - fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} - - fn after_decoding( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } - - fn before_execution( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } - - fn after_execution( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } -} +impl Tracer for NoTracer {} diff --git a/src/tracers/print_tracer.rs b/src/tracers/print_tracer.rs index 1c9e8d95..bbaa6b5a 100644 --- a/src/tracers/print_tracer.rs +++ b/src/tracers/print_tracer.rs @@ -76,22 +76,4 @@ impl Tracer for PrintTracer { } } } - - fn after_execution( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } - - fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} - - fn after_decoding( - &mut self, - _opcode: &Opcode, - _execution: &mut Execution, - _state: &mut VMState, - ) { - } } diff --git a/src/tracers/tracer.rs b/src/tracers/tracer.rs index 6acb116f..f866b72d 100644 --- a/src/tracers/tracer.rs +++ b/src/tracers/tracer.rs @@ -1,8 +1,26 @@ use crate::{execution::Execution, state::VMState, Opcode}; pub trait Tracer { - fn before_decoding(&mut self, execution: &mut Execution, state: &mut VMState); - fn after_decoding(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); - fn before_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); - fn after_execution(&mut self, opcode: &Opcode, execution: &mut Execution, state: &mut VMState); + fn before_decoding(&mut self, _execution: &mut Execution, _state: &mut VMState) {} + fn after_decoding( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + fn before_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } + fn after_execution( + &mut self, + _opcode: &Opcode, + _execution: &mut Execution, + _state: &mut VMState, + ) { + } } From 081d615dcbe53c428c6626a679f88f0091b716ae Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 23 Aug 2024 10:35:48 -0300 Subject: [PATCH 35/57] Address clippy warnings --- src/tracers/no_tracer.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tracers/no_tracer.rs b/src/tracers/no_tracer.rs index e9f54b3c..b060f10e 100644 --- a/src/tracers/no_tracer.rs +++ b/src/tracers/no_tracer.rs @@ -1,6 +1,4 @@ use super::tracer::Tracer; -use crate::state::VMState; -use crate::{execution::Execution, Opcode}; #[derive(Default)] pub struct NoTracer {} From ee97d09533a4b7c10e5149ee2b3861c3e6d85730 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 15:39:39 -0300 Subject: [PATCH 36/57] Address review comments --- src/call_frame.rs | 6 ++-- src/op_handlers/ret.rs | 17 ++++------- src/state.rs | 65 +++++++++++++++++++----------------------- src/vm.rs | 4 +-- 4 files changed, 39 insertions(+), 53 deletions(-) diff --git a/src/call_frame.rs b/src/call_frame.rs index 6efbe484..36f03d35 100644 --- a/src/call_frame.rs +++ b/src/call_frame.rs @@ -11,7 +11,7 @@ pub struct CallFrame { pub exception_handler: u64, pub sp: u32, pub snapshot: StateSnapshot, - pub stipend: u32, + pub stipend: Saturating, } #[derive(Debug, Clone, PartialEq)] @@ -98,7 +98,7 @@ impl CallFrame { ) -> Self { Self { pc: 0, - stipend, + stipend: Saturating(stipend), gas_left: Saturating(gas), exception_handler, sp: 0, @@ -118,7 +118,7 @@ impl CallFrame { pc, gas_left: Saturating(gas), exception_handler, - stipend: 0, + stipend: Saturating(0), sp, snapshot, } diff --git a/src/op_handlers/ret.rs b/src/op_handlers/ret.rs index 14baad72..3f3f2d2f 100644 --- a/src/op_handlers/ret.rs +++ b/src/op_handlers/ret.rs @@ -1,3 +1,5 @@ +use std::num::Saturating; + use u256::U256; use zkevm_opcode_defs::RetOpcode; @@ -63,12 +65,7 @@ pub fn ret( vm.clear_registers(); vm.set_register(1, result); let previous_frame = vm.pop_frame()?; - vm.increase_gas( - previous_frame - .gas_left - .0 - .saturating_sub(previous_frame.stipend), - )?; + vm.increase_gas((previous_frame.gas_left - previous_frame.stipend).0)?; if is_failure { state.rollback(previous_frame.snapshot); vm.current_frame_mut()?.pc = previous_frame.exception_handler; @@ -100,6 +97,7 @@ pub fn inexplicit_panic(vm: &mut Execution, state: &mut VMState) -> Result Result, written_storage_slots: RollbackableHashSet, decommitted_hashes: RollbackableHashSet, - - // Just a cache so that we don't query the db every time - pub initial_values: HashMap>, } impl VMState { @@ -77,7 +74,6 @@ impl VMState { read_storage_slots: RollbackableHashSet::::default(), written_storage_slots: RollbackableHashSet::::default(), decommitted_hashes: RollbackableHashSet::::default(), - initial_values: HashMap::default(), } } @@ -151,7 +147,9 @@ impl VMState { } pub fn storage_read_with_no_refund(&mut self, key: StorageKey) -> U256 { - let value = self.storage_read_inner(&key).unwrap_or_default(); + let value = self + .storage_read_inner(&key) + .map_or_else(|| U256::zero(), |val| val); let storage = self.storage.borrow(); if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) { @@ -163,28 +161,13 @@ impl VMState { value } - fn storage_read_inner(&mut self, key: &StorageKey) -> Option { - match self.storage_changes.map.get(key) { - None => { - let cached_value = *self.initial_values.get(key).unwrap_or(&None); - if cached_value.is_none() { - let value = self.storage.borrow_mut().storage_read(key); - self.initial_values.insert(*key, value); - return value; - } - cached_value - } - value => value.copied(), - } + fn storage_read_inner(&self, key: &StorageKey) -> Option { + self.storage_changes.map.get(key).copied() } pub fn storage_write(&mut self, key: StorageKey, value: U256) -> u32 { self.storage_changes.map.insert(key, value); - self.initial_values - .entry(key) - .or_insert_with(|| self.storage.borrow_mut().storage_read(&key)); // caching the value - let mut storage = self.storage.borrow_mut(); if storage.is_free_storage_slot(&key) { @@ -262,23 +245,31 @@ impl VMState { } /// returns the values that have actually changed from the initial storage - pub fn get_storage_changes(&self) -> Vec<(StorageKey, Option, U256)> { + pub fn get_storage_changes(&mut self) -> Vec<(StorageKey, Option, U256)> { self.storage_changes .map .iter() - .filter_map(|(key, &value)| { - let initial_value = self.initial_values.get(key).unwrap_or(&None); - if initial_value.unwrap_or_default() == value { + .filter_map(|(key, value)| { + let initial_value = self.storage_read_inner(key); + if initial_value.unwrap_or_default() == *value { None } else { - Some((*key, *initial_value, value)) + Some((*key, initial_value, value.clone())) } }) .collect() } - /// returns the values that have actually changed from the snapshot storage or the initial if the former does not exist - /// also, a flag is also retured indicating if the value existed on the initial storage + /// Retrieves the values that have changed since the snapshot was taken, or returns the initial values if no changes exist. + /// Additionally, a flag is returned to indicate whether the value was present in the initial storage. + /// + /// # Returns + /// + /// A `Vec` of tuples where each tuple contains: + /// - `StorageKey`: The key for the storage value. + /// - `Option`: The value before the change, or the initial value if no change was made. + /// - `U256`: The current value after the change. + /// - `bool`: A flag indicating whether the value existed in the initial storage (`true` if it did not exist, `false` otherwise). pub fn get_storage_changes_from_snapshot( &self, snapshot: as Rollbackable>::Snapshot, @@ -287,15 +278,15 @@ impl VMState { .get_logs_after_snapshot(snapshot) .iter() .map(|(key, (before, after))| { - let initial = self.initial_values.get(key).unwrap_or(&None); - (*key, before.or(*initial), *after, initial.is_none()) + let initial = self.storage_read_inner(key); + (*key, before.or(initial), *after, initial.is_none()) }) .collect() } } #[derive(Clone, Default, PartialEq, Debug)] -// a copy of rollbackable fields +// a copy of the state fields that get rollback on panics and reverts. pub struct StateSnapshot { // this casts allows us to get the Snapshot type from the Rollbackable trait pub storage_changes: as Rollbackable>::Snapshot, @@ -306,7 +297,8 @@ pub struct StateSnapshot { pub paid_changes: as Rollbackable>::Snapshot, } -pub struct FullStateSnapshot { +// a copy of all state fields, this type of snapshot is used only by bootloader rollbacks +pub struct ExternalStateSnapshot { pub internal_snapshot: StateSnapshot, pub pubdata_costs: as Rollbackable>::Snapshot, pub refunds: as Rollbackable>::Snapshot, @@ -340,7 +332,8 @@ impl Rollbackable for VMState { } impl VMState { - pub fn external_rollback(&mut self, snapshot: FullStateSnapshot) { + // this rollbacks are triggered by the bootloader only. + pub fn external_rollback(&mut self, snapshot: ExternalStateSnapshot) { self.rollback(snapshot.internal_snapshot); self.pubdata_costs.rollback(snapshot.pubdata_costs); self.refunds.rollback(snapshot.refunds); @@ -351,8 +344,8 @@ impl VMState { .rollback(snapshot.written_storage_slots); } - pub fn full_state_snapshot(&self) -> FullStateSnapshot { - FullStateSnapshot { + pub fn full_state_snapshot(&self) -> ExternalStateSnapshot { + ExternalStateSnapshot { internal_snapshot: self.snapshot(), decommited_hashes: self.decommitted_hashes.snapshot(), pubdata_costs: self.pubdata_costs.snapshot(), diff --git a/src/vm.rs b/src/vm.rs index f38adaa8..645df77e 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -42,7 +42,7 @@ use crate::op_handlers::shift::{rol, ror, shl, shr}; use crate::op_handlers::sub::sub; use crate::op_handlers::unimplemented::unimplemented; use crate::op_handlers::xor::xor; -use crate::state::{FullStateSnapshot, VMState}; +use crate::state::{ExternalStateSnapshot, VMState}; use crate::store::Storage; use crate::tracers::blob_saver_tracer::BlobSaverTracer; use crate::value::{FatPointer, TaggedValue}; @@ -65,7 +65,7 @@ pub struct EraVM { pub struct VmSnapshot { execution: ExecutionSnapshot, - state: FullStateSnapshot, + state: ExternalStateSnapshot, } pub enum EncodingMode { From 5baf370b1d56f3f0ed9e40d24841c69d88f50036 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 15:46:07 -0300 Subject: [PATCH 37/57] Add docs to state --- src/state.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/state.rs b/src/state.rs index 44fdd087..057582cc 100644 --- a/src/state.rs +++ b/src/state.rs @@ -235,6 +235,12 @@ impl VMState { self.events.entries.push(event); } + /// Attempts to decommit the specified `hash` and retrieves any changes made since the initial storage state. + /// # Returns + /// + /// A tuple containing: + /// - `Option>`: the contract bytecode + /// - `bool`: A boolean flag indicating whether the hash was decommitted (`true` if it was newly decommitted, `false` if it had already been decommitted). pub fn decommit(&mut self, hash: U256) -> (Option>, bool) { let was_decommitted = !self.decommitted_hashes.map.insert(hash); (self.storage.borrow_mut().decommit(hash), was_decommitted) @@ -244,7 +250,14 @@ impl VMState { &self.decommitted_hashes.map } - /// returns the values that have actually changed from the initial storage + /// Retrieves the values that have changed since the initial storage. + /// + /// # Returns + /// + /// A `Vec` of tuples where each tuple contains: + /// - `StorageKey`: The key for the storage value. + /// - `Option`: The initial value from the storage. + /// - `U256`: The current value after the change. pub fn get_storage_changes(&mut self) -> Vec<(StorageKey, Option, U256)> { self.storage_changes .map @@ -260,7 +273,7 @@ impl VMState { .collect() } - /// Retrieves the values that have changed since the snapshot was taken, or returns the initial values if no changes exist. + /// Retrieves the values that have changed since the snapshot was taken, or returns the initial values if no changes exist along the current value. /// Additionally, a flag is returned to indicate whether the value was present in the initial storage. /// /// # Returns From 75434eb253b58db85e4a4bdcd62dd8cb920e8ca2 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 16:11:14 -0300 Subject: [PATCH 38/57] Address review comments --- src/tracers/blob_saver_tracer.rs | 16 ++++------- src/tracers/print_tracer.rs | 47 ++++++++++++++++---------------- src/vm.rs | 13 +++++++-- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/tracers/blob_saver_tracer.rs b/src/tracers/blob_saver_tracer.rs index d04c30c9..c6369c62 100644 --- a/src/tracers/blob_saver_tracer.rs +++ b/src/tracers/blob_saver_tracer.rs @@ -50,10 +50,9 @@ fn hash_evm_bytecode(bytecode: &[u8]) -> H256 { impl Tracer for BlobSaverTracer { fn before_execution(&mut self, _opcode: &Opcode, vm: &mut Execution, _state: &mut VMState) { let current_callstack = vm.current_context(); - if current_callstack.is_err() { + let Ok(current_callstack) = current_callstack else { return; - } - let current_callstack = current_callstack.unwrap(); + }; // Here we assume that the only case when PC is 0 at the start of the execution of the contract. let known_code_storage_call = current_callstack.code_address == KNOWN_CODES_STORAGE_ADDRESS @@ -72,16 +71,13 @@ impl Tracer for BlobSaverTracer { } let ptr = FatPointer::decode(calldata_ptr.value); - let heap = vm.heaps.get(ptr.page); - if heap.is_none() { + let Some(heap) = vm.heaps.get(ptr.page) else { return; - } + }; - let data = heap.unwrap().read_unaligned_from_pointer(&ptr); - if data.is_err() { + let Ok(data) = heap.read_unaligned_from_pointer(&ptr) else { return; - } - let data = data.unwrap(); + }; if data.len() < 64 { // Not interested diff --git a/src/tracers/print_tracer.rs b/src/tracers/print_tracer.rs index bbaa6b5a..37db764f 100644 --- a/src/tracers/print_tracer.rs +++ b/src/tracers/print_tracer.rs @@ -19,41 +19,42 @@ impl Tracer for PrintTracer { const DEBUG_SLOT: u32 = 1024; - let debug_magic = U256::from_dec_str( + let Ok(debug_magic) = U256::from_dec_str( "33509158800074003487174289148292687789659295220513886355337449724907776218753", - ) - .unwrap(); + ) else { + return; + }; if matches!(opcode_variant, ZKOpcode::UMA(UMAOpcode::HeapWrite)) { - let (src0, src1) = address_operands_read(vm, opcode).unwrap(); + let Ok((src0, src1)) = address_operands_read(vm, opcode) else { + return; + }; let value = src1.value; if value == debug_magic { let fat_ptr = FatPointer::decode(src0.value); if fat_ptr.offset == DEBUG_SLOT { - let how_to_print_value = vm - .heaps - .get(vm.current_context().unwrap().heap_id) - .ok_or(HeapError::ReadOutOfBounds) - .unwrap() - .read(DEBUG_SLOT + 32); + let Ok(ctx) = vm.current_context() else { + return; + }; + let Some(heap) = vm.heaps.get(ctx.heap_id) else { + return; + }; + let how_to_print_value = heap.read(DEBUG_SLOT + 32); + let value_to_print = heap.read(DEBUG_SLOT + 64); - let value_to_print = vm - .heaps - .get(vm.current_context().unwrap().heap_id) - .ok_or(HeapError::ReadOutOfBounds) - .unwrap() - .read(DEBUG_SLOT + 64); - - let print_as_hex_value = U256::from_str_radix( + let Ok(print_as_hex_value) = U256::from_str_radix( "0x00debdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebde", 16, - ) - .unwrap(); - let print_as_string_value = U256::from_str_radix( + ) else { + return; + }; + + let Ok(print_as_string_value) = U256::from_str_radix( "0x00debdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdebdf", 16, - ) - .unwrap(); + ) else { + return; + }; if how_to_print_value == print_as_hex_value { print!("PRINTED: "); diff --git a/src/vm.rs b/src/vm.rs index 15862d1b..3e14eaf2 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -82,14 +82,23 @@ impl EraVM { } /// Run a vm program with a given bytecode. - pub fn run_program_with_custom_bytecode( + pub fn run_program_with_custom_bytecode(&mut self) -> ExecutionOutput { + self.run_opcodes(None) + } + + pub fn run_program_with_test_encode(&mut self) -> ExecutionOutput { + self.run(&mut NoTracer::default(), EncodingMode::Testing) + .unwrap_or(ExecutionOutput::Panic) + } + + pub fn run_program_with_custom_bytecode_and_tracer( &mut self, tracer: Option<&mut dyn Tracer>, ) -> ExecutionOutput { self.run_opcodes(tracer) } - pub fn run_program_with_test_encode( + pub fn run_program_with_test_encode_and_tracer( &mut self, tracer: Option<&mut dyn Tracer>, ) -> ExecutionOutput { From 66d412c11a2e5117bd99430f38bdc935d24d5274 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 16:12:26 -0300 Subject: [PATCH 39/57] Remove optional from run with tracers --- src/vm.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index 3e14eaf2..2289bc68 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -93,20 +93,17 @@ impl EraVM { pub fn run_program_with_custom_bytecode_and_tracer( &mut self, - tracer: Option<&mut dyn Tracer>, + tracer: &mut dyn Tracer, ) -> ExecutionOutput { - self.run_opcodes(tracer) + self.run_opcodes(Some(tracer)) } pub fn run_program_with_test_encode_and_tracer( &mut self, - tracer: Option<&mut dyn Tracer>, + tracer: &mut dyn Tracer, ) -> ExecutionOutput { - self.run( - tracer.unwrap_or(&mut NoTracer::default()), - EncodingMode::Testing, - ) - .unwrap_or(ExecutionOutput::Panic) + self.run(tracer, EncodingMode::Testing) + .unwrap_or(ExecutionOutput::Panic) } fn run_opcodes(&mut self, tracer: Option<&mut dyn Tracer>) -> ExecutionOutput { From 291e4c19848d6144d4d826503d39fbd74ff7e45e Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 16:13:51 -0300 Subject: [PATCH 40/57] Update era-compiler-tester submodule --- era-compiler-tester | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/era-compiler-tester b/era-compiler-tester index 3500a561..c44413a0 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit 3500a56143a279ac026fcf0a36fea95ef7671cc4 +Subproject commit c44413a03da700ff145787fba1b60e4b59624c8c From 02e8a90092f014eb7d3b6124b522726385f1a8be Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 16:20:02 -0300 Subject: [PATCH 41/57] Update era-compiler-tester submodule --- era-compiler-tester | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/era-compiler-tester b/era-compiler-tester index c44413a0..62ae4bce 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit c44413a03da700ff145787fba1b60e4b59624c8c +Subproject commit 62ae4bcee174e213021f36aaa8ca0bde00a3b665 From ed3784cc26c28f810bf80c355097c835eda5b8f6 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 16:45:35 -0300 Subject: [PATCH 42/57] Address clippy warnings --- src/tracers/print_tracer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tracers/print_tracer.rs b/src/tracers/print_tracer.rs index 37db764f..b486fad5 100644 --- a/src/tracers/print_tracer.rs +++ b/src/tracers/print_tracer.rs @@ -3,7 +3,6 @@ use zkevm_opcode_defs::Opcode as ZKOpcode; use zkevm_opcode_defs::UMAOpcode; use crate::address_operands::address_operands_read; -use crate::eravm_error::HeapError; use crate::state::VMState; use crate::value::FatPointer; use crate::{execution::Execution, Opcode}; From aea62fe839cac1299b31009cd1a71203147a993f Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 26 Aug 2024 17:17:49 -0300 Subject: [PATCH 43/57] Fix storage_read --- src/op_handlers/ret.rs | 2 -- src/state.rs | 13 +++++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/op_handlers/ret.rs b/src/op_handlers/ret.rs index 3f3f2d2f..ffba106a 100644 --- a/src/op_handlers/ret.rs +++ b/src/op_handlers/ret.rs @@ -1,5 +1,3 @@ -use std::num::Saturating; - use u256::U256; use zkevm_opcode_defs::RetOpcode; diff --git a/src/state.rs b/src/state.rs index 057582cc..bca3e65a 100644 --- a/src/state.rs +++ b/src/state.rs @@ -128,7 +128,9 @@ impl VMState { } pub fn storage_read(&mut self, key: StorageKey) -> (U256, u32) { - let value = self.storage_read_inner(&key).unwrap_or_default(); + let value = self + .storage_read_inner(&key) + .map_or_else(U256::zero, |val| val); let storage = self.storage.borrow(); @@ -149,7 +151,7 @@ impl VMState { pub fn storage_read_with_no_refund(&mut self, key: StorageKey) -> U256 { let value = self .storage_read_inner(&key) - .map_or_else(|| U256::zero(), |val| val); + .map_or_else(U256::zero, |val| val); let storage = self.storage.borrow(); if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) { @@ -162,7 +164,10 @@ impl VMState { } fn storage_read_inner(&self, key: &StorageKey) -> Option { - self.storage_changes.map.get(key).copied() + match self.storage_changes.map.get(key) { + None => self.storage.borrow_mut().storage_read(key), + value => value.copied(), + } } pub fn storage_write(&mut self, key: StorageKey, value: U256) -> u32 { @@ -267,7 +272,7 @@ impl VMState { if initial_value.unwrap_or_default() == *value { None } else { - Some((*key, initial_value, value.clone())) + Some((*key, initial_value, *value)) } }) .collect() From b44437328926f92e772c4d9527750332a5d2397c Mon Sep 17 00:00:00 2001 From: Juan Munoz Date: Wed, 28 Aug 2024 13:56:15 -0300 Subject: [PATCH 44/57] update zksync era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 93ba1389..96bf03de 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 93ba13893c2f6e057ddd9c5ef7947891abb23f6c +Subproject commit 96bf03de9b23cbcc8e61c20fed95ad5a72c7a262 From fc2855ac335259d093a78ea060dfd360ce7ba249 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau <76252340+MarcosNicolau@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:48:51 -0300 Subject: [PATCH 45/57] Implement statistics (#213) * Add vm statistics * Add tracking of precompiles cycles * Add storage and decommit cycles * Make opcode variant pub * Add cycles counter * Update zksync-era submodule * Address clippy warnings * Update era-compiler-tester submodule * Update zksync-era submodule * Fix initial value read in storage_changes * Expose opcode variants * Update zksync-era submodule * Address review comments --- Makefile | 68 +---------------------------- src/lib.rs | 1 + src/op_handlers/far_call.rs | 18 +++++--- src/op_handlers/log.rs | 14 ++++++ src/op_handlers/opcode_decommit.rs | 6 ++- src/op_handlers/precompile_call.rs | 10 +++-- src/opcode.rs | 8 ++-- src/precompiles/ecrecover.rs | 6 +-- src/precompiles/keccak256.rs | 10 +++-- src/precompiles/mod.rs | 5 ++- src/precompiles/secp256r1_verify.rs | 8 ++-- src/precompiles/sha256.rs | 10 +++-- src/state.rs | 11 +++++ src/statistics.rs | 21 +++++++++ src/vm.rs | 45 +++++++++++++------ zksync-era | 2 +- 16 files changed, 133 insertions(+), 110 deletions(-) create mode 100644 src/statistics.rs diff --git a/Makefile b/Makefile index 7940dd92..28a6c0c4 100644 --- a/Makefile +++ b/Makefile @@ -1,21 +1,10 @@ -.PHONY: clean lint test deps submodules bench era-test build-bench-contracts %.sol -.SILENT: %.sol +.PHONY: clean lint test deps submodules era-test LLVM_PATH?=$(shell pwd)/era-compiler-tester/target-llvm/target-final/ -ZKSYNC_ROOT=$(shell realpath ./zksync-era) -ZKSYNC_L1_CONTRACTS=$(ZKSYNC_ROOT)/contracts/l1-contracts/artifacts -ZKSYNC_L2_CONTRACTS=$(ZKSYNC_ROOT)/contracts/l2-contracts/artifacts-zk -ZKSYNC_SYS_CONTRACTS=$(ZKSYNC_ROOT)/contracts/system-contracts/artifacts-zk -ZKSYNC_BOOTLOADER_CONTRACT=$(ZKSYNC_ROOT)/contracts/system-contracts/bootloader/build/artifacts -ZKSYNC_BENCH_TEST_DATA=$(ZKSYNC_ROOT)/etc/contracts-test-data/artifacts-zk -ZKSYNC_BENCH_CONTRACTS=$(ZKSYNC_ROOT)/core/tests/vm-benchmark/deployment_benchmarks -ZKSYNC_BENCH_SOURCES=$(ZKSYNC_ROOT)/core/tests/vm-benchmark/deployment_benchmarks_sources - clean: rm -rf ./db rm -rf era-compiler-tester - rm -rf $(ZKSYNC_ROOT) lint: cargo clippy --workspace --all-features --benches --examples --tests -- -D warnings @@ -38,60 +27,5 @@ test: deps ci-test: export LLVM_SYS_170_PREFIX=$(LLVM_PATH) && $(MAKE) test -# Build the given set of zksync era contracts, -# this can be: l1, l2 or system contracts. -# These are needed for benchmarking. -define build_zk_contracts - cd $(ZKSYNC_ROOT)/contracts && \ - yarn install --frozen-lockfile && \ - $(1) -endef - -$(ZKSYNC_L1_CONTRACTS): - $(call build_zk_contracts, yarn l1 build) - -$(ZKSYNC_L2_CONTRACTS): - $(call build_zk_contracts, yarn l2 build) - -$(ZKSYNC_SYS_CONTRACTS): - $(call build_zk_contracts, yarn sc build:system-contracts) - -$(ZKSYNC_BOOTLOADER_CONTRACT): - $(call build_zk_contracts, yarn sc build:bootloader) - -$(ZKSYNC_BENCH_TEST_DATA): - touch $(ZKSYNC_ROOT)/etc/contracts-test-data - cd $(ZKSYNC_ROOT)/etc/contracts-test-data && yarn install --frozen-lockfile && yarn build - -# Steps: -# 1 - cd -# 2 - Take the given CONTRACT.sol and get its byte code -# 3 - Parse the output -# 4 - Redirect the hexstring to a CONTRACT (mind the extension-less name) to -# a file insie the contract benchmarks folder. -%.sol: - echo "Building benchmark contract: $@" - cd $(ZKSYNC_BENCH_SOURCES) && \ - zksolc --bin $@ | grep -oE '0x[0-9a-fA-F]+' > $(ZKSYNC_BENCH_CONTRACTS)/$(basename $@) - -build_bench_contracts: fibonacci_rec.sol send.sol - - -# Compile contracts and fetch submodules for the benches. -# If you get any 'missing file' errors whil running cargo bench, this is probably what you must run. -bench-setup: submodules build_bench_contracts $(ZKSYNC_BENCH_TEST_DATA) $(ZKSYNC_SYS_CONTRACTS) $(ZKSYNC_BOOTLOADER_CONTRACT) $(ZKSYNC_L1_CONTRACTS) $(ZKSYNC_L2_CONTRACTS) - -bench: - cd $(ZKSYNC_ROOT) && cargo bench --bench criterion "$(lambda|fast_vm|legacy)/(fibonacci|send)" - -bench-base: - cd $(ZKSYNC_ROOT) && cargo bench --bench criterion -- --save-baseline bench_base lambda 1>bench-compare.txt - -bench-compare: - cd $(ZKSYNC_ROOT) && cargo bench --bench criterion -- --baseline bench_base lambda 1>bench-compare.txt - -clean-contracts: - rm -rfv $(ZKSYNC_BENCH_TEST_DATA) $(ZKSYNC_SYS_CONTRACTS) $(ZKSYNC_BOOTLOADER_CONTRACT) $(ZKSYNC_L1_CONTRACTS) $(ZKSYNC_L2_CONTRACTS) - era-test: submodules cd ./zksync-era/core/lib/multivm && cargo t era_vm diff --git a/src/lib.rs b/src/lib.rs index 2409b16e..33b49d44 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,6 +8,7 @@ pub mod opcode; pub mod output; mod precompiles; mod ptr_operator; +pub mod statistics; pub mod store; pub mod tracers; pub mod utils; diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index bfa3e394..e4897186 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -14,6 +14,7 @@ use crate::{ execution::Execution, rollbacks::Rollbackable, state::VMState, + statistics::{VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES}, store::{StorageError, StorageKey}, utils::{address_into_u256, is_kernel}, value::{FatPointer, TaggedValue}, @@ -214,6 +215,7 @@ pub fn far_call( opcode: &Opcode, far_call: &FarCallOpcode, state: &mut VMState, + statistics: &mut VmStatistics, ) -> Result<(), EraVmError> { let (src0, src1) = address_operands_read(vm, opcode)?; let contract_address = address_from_u256(&src1.value); @@ -263,10 +265,14 @@ pub fn far_call( .checked_add(stipend) .expect("stipend must not cause overflow"); - let program_code = state - .decommit(code_key) - .0 - .ok_or(StorageError::KeyNotPresent)?; + let (program_code, was_decommited) = state.decommit(code_key); + + let program_code = program_code.ok_or(StorageError::KeyNotPresent)?; + if !was_decommited { + statistics.storage_application_cycles += STORAGE_READ_STORAGE_APPLICATION_CYCLES; + statistics.decommiter_cycle_from_decommit(&program_code); + } + let new_heap = vm.heaps.allocate(); let new_aux_heap = vm.heaps.allocate(); let is_new_frame_static = opcode.flag0_set || vm.current_context()?.is_static; @@ -356,14 +362,14 @@ pub fn far_call( Ok(()) } -pub(crate) struct FarCallABI { +pub struct FarCallABI { pub _gas_to_pass: u32, pub _shard_id: u8, pub is_constructor_call: bool, pub is_system_call: bool, } -pub(crate) fn get_far_call_arguments(abi: U256) -> FarCallABI { +pub fn get_far_call_arguments(abi: U256) -> FarCallABI { let _gas_to_pass = abi.0[3] as u32; let settings = (abi.0[3] >> 32) as u32; let [_, _shard_id, constructor_call_byte, system_call_byte] = settings.to_le_bytes(); diff --git a/src/op_handlers/log.rs b/src/op_handlers/log.rs index f1b8d04d..b8af16e6 100644 --- a/src/op_handlers/log.rs +++ b/src/op_handlers/log.rs @@ -2,6 +2,10 @@ use crate::{ eravm_error::EraVmError, execution::Execution, state::{L2ToL1Log, VMState}, + statistics::{ + VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES, + STORAGE_WRITE_STORAGE_APPLICATION_CYCLES, + }, store::StorageKey, value::TaggedValue, Opcode, @@ -11,10 +15,14 @@ pub fn storage_write( vm: &mut Execution, opcode: &Opcode, state: &mut VMState, + statistics: &mut VmStatistics, ) -> Result<(), EraVmError> { let key_for_contract_storage = vm.get_register(opcode.src0_index).value; let address = vm.current_context()?.contract_address; let key = StorageKey::new(address, key_for_contract_storage); + if !state.written_storage_slots().contains(&key) { + statistics.storage_application_cycles += STORAGE_WRITE_STORAGE_APPLICATION_CYCLES; + } let value = vm.get_register(opcode.src1_index).value; let refund = state.storage_write(key, value); vm.increase_gas(refund)?; @@ -25,10 +33,16 @@ pub fn storage_read( vm: &mut Execution, opcode: &Opcode, state: &mut VMState, + statistics: &mut VmStatistics, ) -> Result<(), EraVmError> { let key_for_contract_storage = vm.get_register(opcode.src0_index).value; let address = vm.current_context()?.contract_address; let key = StorageKey::new(address, key_for_contract_storage); + // we need to check if it wasn't written as well + // because when writing to storage, we need to read the slot as well + if !state.read_storage_slots().contains(&key) && !state.written_storage_slots().contains(&key) { + statistics.storage_application_cycles += STORAGE_READ_STORAGE_APPLICATION_CYCLES; + } let (value, refund) = state.storage_read(key); vm.increase_gas(refund)?; vm.set_register(opcode.dst0_index, TaggedValue::new_raw_integer(value)); diff --git a/src/op_handlers/opcode_decommit.rs b/src/op_handlers/opcode_decommit.rs index bcf6f9eb..4a530407 100644 --- a/src/op_handlers/opcode_decommit.rs +++ b/src/op_handlers/opcode_decommit.rs @@ -5,6 +5,7 @@ use crate::{ eravm_error::{EraVmError, HeapError}, execution::Execution, state::VMState, + statistics::{VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES}, value::{FatPointer, TaggedValue}, Opcode, }; @@ -13,6 +14,7 @@ pub fn opcode_decommit( vm: &mut Execution, opcode: &Opcode, state: &mut VMState, + statistics: &mut VmStatistics, ) -> Result<(), EraVmError> { let (src0, src1) = address_operands_read(vm, opcode)?; @@ -36,7 +38,9 @@ pub fn opcode_decommit( if was_decommited { // refund it vm.increase_gas(extra_cost)?; - }; + } else { + statistics.storage_application_cycles += STORAGE_READ_STORAGE_APPLICATION_CYCLES; + } let code = code.ok_or(EraVmError::DecommitFailed)?; diff --git a/src/op_handlers/precompile_call.rs b/src/op_handlers/precompile_call.rs index 613cdc51..c74801f2 100644 --- a/src/op_handlers/precompile_call.rs +++ b/src/op_handlers/precompile_call.rs @@ -15,6 +15,7 @@ use crate::{ secp256r1_verify::secp256r1_verify_function, sha256::sha256_rounds_function, }, state::VMState, + statistics::VmStatistics, value::TaggedValue, Opcode, }; @@ -23,6 +24,7 @@ pub fn precompile_call( vm: &mut Execution, opcode: &Opcode, state: &mut VMState, + statistics: &mut VmStatistics, ) -> Result<(), EraVmError> { let (src0, src1) = address_operands_read(vm, opcode)?; let aux_data = PrecompileAuxData::from_u256(src1.value); @@ -45,16 +47,16 @@ pub fn precompile_call( match address_low { KECCAK256_ROUND_FUNCTION_PRECOMPILE_ADDRESS => { - keccak256_rounds_function(abi_key, heaps)?; + statistics.keccak256_cycles += keccak256_rounds_function(abi_key, heaps)?; } SHA256_ROUND_FUNCTION_PRECOMPILE_ADDRESS => { - sha256_rounds_function(abi_key, heaps)?; + statistics.sha256_cycles += sha256_rounds_function(abi_key, heaps)?; } ECRECOVER_INNER_FUNCTION_PRECOMPILE_ADDRESS => { - ecrecover_function(abi_key, heaps)?; + statistics.ecrecover_cycles += ecrecover_function(abi_key, heaps)?; } SECP256R1_VERIFY_PRECOMPILE_ADDRESS => { - secp256r1_verify_function(abi_key, heaps)?; + statistics.secp255r1_verify_cycles += secp256r1_verify_function(abi_key, heaps)?; } _ => { // A precompile call may be used just to burn gas diff --git a/src/opcode.rs b/src/opcode.rs index 48ec936c..cb3a1199 100644 --- a/src/opcode.rs +++ b/src/opcode.rs @@ -1,14 +1,14 @@ +use crate::eravm_error::EraVmError; +use crate::eravm_error::OpcodeError; use lazy_static::lazy_static; -use zkevm_opcode_defs::Opcode as Variant; +pub use zkevm_opcode_defs::Opcode as Variant; use zkevm_opcode_defs::OpcodeVariant; use zkevm_opcode_defs::Operand; use zkevm_opcode_defs::CONDITIONAL_BITS_SHIFT; use zkevm_opcode_defs::DST_REGS_SHIFT; use zkevm_opcode_defs::OPCODES_TABLE_WIDTH; use zkevm_opcode_defs::SRC_REGS_SHIFT; - -use crate::eravm_error::EraVmError; -use crate::eravm_error::OpcodeError; +pub use zkevm_opcode_defs::{LogOpcode, RetOpcode, UMAOpcode}; #[derive(Debug, Clone)] pub enum Predicate { diff --git a/src/precompiles/ecrecover.rs b/src/precompiles/ecrecover.rs index 1a448c06..b138d87c 100644 --- a/src/precompiles/ecrecover.rs +++ b/src/precompiles/ecrecover.rs @@ -22,7 +22,7 @@ use super::*; pub struct ECRecoverPrecompile; impl Precompile for ECRecoverPrecompile { - fn execute_precompile(&mut self, query: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { + fn execute_precompile(&mut self, query: U256, heaps: &mut Heaps) -> Result { let precompile_call_params = query; let params = precompile_abi_in_log(precompile_call_params); let addr = |offset: u32| (params.output_memory_offset + offset) * 32; @@ -65,7 +65,7 @@ impl Precompile for ECRecoverPrecompile { write_heap.store(addr(0), marker); write_heap.store(addr(1), result); - Ok(()) + Ok(DEFAULT_NUM_ROUNDS) } } @@ -152,6 +152,6 @@ fn get_address_from_pk(pk: VerifyingKey) -> Result<[u8; 32], EraVmError> { Ok(address) } -pub fn ecrecover_function(abi: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { +pub fn ecrecover_function(abi: U256, heaps: &mut Heaps) -> Result { ECRecoverPrecompile.execute_precompile(abi, heaps) } diff --git a/src/precompiles/keccak256.rs b/src/precompiles/keccak256.rs index 764ee3ce..51fed976 100644 --- a/src/precompiles/keccak256.rs +++ b/src/precompiles/keccak256.rs @@ -70,7 +70,11 @@ fn hash_as_bytes32(hash: [u64; 25]) -> [u8; 32] { struct Keccak256Precompile; impl Precompile for Keccak256Precompile { - fn execute_precompile(&mut self, abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { + fn execute_precompile( + &mut self, + abi_key: U256, + heaps: &mut Heaps, + ) -> Result { let mut full_round_padding = [0u8; KECCAK_RATE_BYTES]; full_round_padding[0] = 0x01; full_round_padding[KECCAK_RATE_BYTES - 1] = 0x80; @@ -148,10 +152,10 @@ impl Precompile for Keccak256Precompile { .try_get_mut(params.memory_page_to_write)? .store(params.output_memory_offset * 32, hash); - Ok(()) + Ok(num_rounds) } } -pub fn keccak256_rounds_function(abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { +pub fn keccak256_rounds_function(abi_key: U256, heaps: &mut Heaps) -> Result { Keccak256Precompile.execute_precompile(abi_key, heaps) } diff --git a/src/precompiles/mod.rs b/src/precompiles/mod.rs index 28f90f7f..a17f5794 100644 --- a/src/precompiles/mod.rs +++ b/src/precompiles/mod.rs @@ -8,8 +8,11 @@ pub mod keccak256; pub mod secp256r1_verify; pub mod sha256; +const DEFAULT_NUM_ROUNDS: usize = 1; + pub trait Precompile: std::fmt::Debug { - fn execute_precompile(&mut self, abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError>; + fn execute_precompile(&mut self, abi_key: U256, heaps: &mut Heaps) + -> Result; } pub struct PrecompileCallABI { diff --git a/src/precompiles/secp256r1_verify.rs b/src/precompiles/secp256r1_verify.rs index d4799911..16c33b22 100644 --- a/src/precompiles/secp256r1_verify.rs +++ b/src/precompiles/secp256r1_verify.rs @@ -1,4 +1,4 @@ -use super::{precompile_abi_in_log, Precompile}; +use super::{precompile_abi_in_log, Precompile, DEFAULT_NUM_ROUNDS}; use crate::{eravm_error::EraVmError, heaps::Heaps}; use u256::U256; use zkevm_opcode_defs::p256::{ @@ -11,7 +11,7 @@ use zkevm_opcode_defs::p256::{ pub struct Secp256r1VerifyPrecompile; impl Precompile for Secp256r1VerifyPrecompile { - fn execute_precompile(&mut self, query: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { + fn execute_precompile(&mut self, query: U256, heaps: &mut Heaps) -> Result { let precompile_call_params = query; let params = precompile_abi_in_log(precompile_call_params); let addr = |offset: u32| (params.output_memory_offset + offset) * 32; @@ -52,7 +52,7 @@ impl Precompile for Secp256r1VerifyPrecompile { write_heap.store(addr(0), marker); write_heap.store(addr(1), result); - Ok(()) + Ok(DEFAULT_NUM_ROUNDS) } } @@ -90,6 +90,6 @@ pub fn secp256r1_verify_inner( } // Verifies an ECDSA signature against a message digest using a given public key. -pub fn secp256r1_verify_function(abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { +pub fn secp256r1_verify_function(abi_key: U256, heaps: &mut Heaps) -> Result { Secp256r1VerifyPrecompile.execute_precompile(abi_key, heaps) } diff --git a/src/precompiles/sha256.rs b/src/precompiles/sha256.rs index a7ee58cd..3e26c8cb 100644 --- a/src/precompiles/sha256.rs +++ b/src/precompiles/sha256.rs @@ -19,7 +19,11 @@ fn hash_as_bytes32(hash: [u32; 8]) -> [u8; 32] { pub struct Sha256Precompile; impl Precompile for Sha256Precompile { - fn execute_precompile(&mut self, abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { + fn execute_precompile( + &mut self, + abi_key: U256, + heaps: &mut Heaps, + ) -> Result { let params = precompile_abi_in_log(abi_key); let num_rounds = params.precompile_interpreted_data as usize; let mut read_addr = params.input_memory_offset; @@ -44,10 +48,10 @@ impl Precompile for Sha256Precompile { .try_get_mut(params.memory_page_to_write)? .store(write_addr, hash); - Ok(()) + Ok(num_rounds) } } -pub fn sha256_rounds_function(abi_key: U256, heaps: &mut Heaps) -> Result<(), EraVmError> { +pub fn sha256_rounds_function(abi_key: U256, heaps: &mut Heaps) -> Result { Sha256Precompile.execute_precompile(abi_key, heaps) } diff --git a/src/state.rs b/src/state.rs index 0b045b8d..7c7aee3e 100644 --- a/src/state.rs +++ b/src/state.rs @@ -127,6 +127,16 @@ impl VMState { self.pubdata.value += to_add; } + pub fn read_storage_slots(&self) -> &HashSet { + &self.read_storage_slots.map + } + + pub fn written_storage_slots(&self) -> &HashSet { + &self.written_storage_slots.map + } + + // reads shouldn't be mutable, we should consider change it to a non-mutable reference + // though that would require a refactor in the integration with the operator pub fn storage_read(&mut self, key: StorageKey) -> (U256, u32) { let value = self .storage_read_inner(&key) @@ -176,6 +186,7 @@ impl VMState { let mut storage = self.storage.borrow_mut(); if storage.is_free_storage_slot(&key) { + self.written_storage_slots.map.insert(key); let refund = WARM_WRITE_REFUND; self.refunds.entries.push(refund); self.pubdata_costs.entries.push(0); diff --git a/src/statistics.rs b/src/statistics.rs new file mode 100644 index 00000000..406899d0 --- /dev/null +++ b/src/statistics.rs @@ -0,0 +1,21 @@ +use u256::U256; + +pub const STORAGE_READ_STORAGE_APPLICATION_CYCLES: usize = 1; +pub const STORAGE_WRITE_STORAGE_APPLICATION_CYCLES: usize = 2; + +#[derive(Debug, Default, Clone, PartialEq)] +pub struct VmStatistics { + pub monotonic_counter: u32, + pub keccak256_cycles: usize, + pub ecrecover_cycles: usize, + pub sha256_cycles: usize, + pub secp255r1_verify_cycles: usize, + pub code_decommitter_cycles: usize, + pub storage_application_cycles: usize, +} + +impl VmStatistics { + pub fn decommiter_cycle_from_decommit(&mut self, code_page: &[U256]) { + self.code_decommitter_cycles += (code_page.len() + 1) / 2 + } +} diff --git a/src/vm.rs b/src/vm.rs index e40707bd..bf702a59 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -43,6 +43,7 @@ use crate::op_handlers::sub::sub; use crate::op_handlers::unimplemented::unimplemented; use crate::op_handlers::xor::xor; use crate::state::{ExternalStateSnapshot, VMState}; +use crate::statistics::VmStatistics; use crate::store::Storage; use crate::tracers::no_tracer::NoTracer; use crate::value::{FatPointer, TaggedValue}; @@ -60,11 +61,13 @@ pub enum ExecutionOutput { #[derive(Debug, Clone)] pub struct EraVM { pub state: VMState, + pub statistics: VmStatistics, pub execution: Execution, } pub struct VmSnapshot { execution: ExecutionSnapshot, + statistics: VmStatistics, state: ExternalStateSnapshot, } @@ -77,6 +80,7 @@ impl EraVM { pub fn new(execution: Execution, storage: Rc>) -> Self { Self { state: VMState::new(storage), + statistics: VmStatistics::default(), execution, } } @@ -118,12 +122,14 @@ impl EraVM { VmSnapshot { execution: self.execution.snapshot(), state: self.state.full_state_snapshot(), + statistics: self.statistics.clone(), } } pub fn rollback(&mut self, snapshot: VmSnapshot) { self.execution.rollback(snapshot.execution); self.state.external_rollback(snapshot.state); + self.statistics = snapshot.statistics; } /// Run a vm program from the given path using a custom state. @@ -225,22 +231,34 @@ impl EraVM { }, Variant::NearCall(_) => near_call(&mut self.execution, &opcode, &self.state), Variant::Log(log_variant) => match log_variant { - LogOpcode::StorageRead => { - storage_read(&mut self.execution, &opcode, &mut self.state) - } - LogOpcode::StorageWrite => { - storage_write(&mut self.execution, &opcode, &mut self.state) - } + LogOpcode::StorageRead => storage_read( + &mut self.execution, + &opcode, + &mut self.state, + &mut self.statistics, + ), + LogOpcode::StorageWrite => storage_write( + &mut self.execution, + &opcode, + &mut self.state, + &mut self.statistics, + ), LogOpcode::ToL1Message => { add_l2_to_l1_message(&mut self.execution, &opcode, &mut self.state) } - LogOpcode::PrecompileCall => { - precompile_call(&mut self.execution, &opcode, &mut self.state) - } + LogOpcode::PrecompileCall => precompile_call( + &mut self.execution, + &opcode, + &mut self.state, + &mut self.statistics, + ), LogOpcode::Event => event(&mut self.execution, &opcode, &mut self.state), - LogOpcode::Decommit => { - opcode_decommit(&mut self.execution, &opcode, &mut self.state) - } + LogOpcode::Decommit => opcode_decommit( + &mut self.execution, + &opcode, + &mut self.state, + &mut self.statistics, + ), LogOpcode::TransientStorageRead => { transient_storage_read(&mut self.execution, &opcode, &mut self.state) } @@ -254,6 +272,7 @@ impl EraVM { &opcode, &far_call_variant, &mut self.state, + &mut self.statistics, ); if res.is_err() { panic_from_far_call(&mut self.execution, &opcode)?; @@ -332,7 +351,7 @@ impl EraVM { } else { self.execution.current_frame_mut()?.pc += 1; } - + self.statistics.monotonic_counter += 1; tracer.after_execution(&opcode, &mut self.execution, &mut self.state); } } diff --git a/zksync-era b/zksync-era index 67d2d324..5312aad8 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 67d2d324b70ca28a70b0bed02e3d85c9165e7b64 +Subproject commit 5312aad8c1034e564439ce6cb6145a96c7070f9b From 5de6b4dfbc4350816a3e8ac9344c4f182f19c3fb Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Thu, 29 Aug 2024 15:02:50 -0300 Subject: [PATCH 46/57] Update zksync-era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 5312aad8..ec31b9e2 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 5312aad8c1034e564439ce6cb6145a96c7070f9b +Subproject commit ec31b9e2c1639f7480a800ac0505df62b551e521 From adc3c0e5a2ec9c840a3b8d77df0fd4fd29f24c06 Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Fri, 30 Aug 2024 16:30:21 -0300 Subject: [PATCH 47/57] Move storage param to vm run --- src/op_handlers/far_call.rs | 56 +++++++++++++++++++----------- src/op_handlers/log.rs | 12 +++++-- src/op_handlers/opcode_decommit.rs | 4 ++- src/state.rs | 54 ++++++++++++++-------------- src/vm.rs | 36 ++++++++++++------- 5 files changed, 100 insertions(+), 62 deletions(-) diff --git a/src/op_handlers/far_call.rs b/src/op_handlers/far_call.rs index e4897186..e7c1c48b 100644 --- a/src/op_handlers/far_call.rs +++ b/src/op_handlers/far_call.rs @@ -15,11 +15,13 @@ use crate::{ rollbacks::Rollbackable, state::VMState, statistics::{VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES}, - store::{StorageError, StorageKey}, + store::{Storage, StorageError, StorageKey}, utils::{address_into_u256, is_kernel}, value::{FatPointer, TaggedValue}, Opcode, }; + +use super::ret::panic_from_far_call; #[allow(dead_code)] struct FarCallParams { forward_memory: FatPointer, @@ -134,6 +136,7 @@ fn decommit_code_hash( default_aa_code_hash: [u8; 32], evm_interpreter_code_hash: [u8; 32], is_constructor_call: bool, + storage: &mut dyn Storage, ) -> Result<(U256, bool, u32), EraVmError> { let mut is_evm = false; let deployer_system_contract_address = @@ -141,7 +144,7 @@ fn decommit_code_hash( let storage_key = StorageKey::new(deployer_system_contract_address, address_into_u256(address)); // reading when decommiting doesn't refund - let code_info = state.storage_read_with_no_refund(storage_key); + let code_info = state.storage_read_with_no_refund(storage_key, storage); let mut code_info_bytes = [0; 32]; code_info.to_big_endian(&mut code_info_bytes); @@ -216,6 +219,7 @@ pub fn far_call( far_call: &FarCallOpcode, state: &mut VMState, statistics: &mut VmStatistics, + storage: &mut dyn Storage, ) -> Result<(), EraVmError> { let (src0, src1) = address_operands_read(vm, opcode)?; let contract_address = address_from_u256(&src1.value); @@ -227,45 +231,57 @@ pub fn far_call( abi.is_constructor_call = abi.is_constructor_call && vm.current_context()?.is_kernel(); abi.is_system_call = abi.is_system_call && is_kernel(&contract_address); + let FarCallParams { + ergs_passed, + forward_memory, + .. + } = far_call_params_from_register(src0, vm)?; + + let mut mandated_gas = if abi.is_system_call && src1.value == ADDRESS_MSG_VALUE.into() { + MSG_VALUE_SIMULATOR_ADDITIVE_COST + } else { + 0 + }; + let (code_key, is_evm, decommit_cost) = decommit_code_hash( state, contract_address, vm.default_aa_code_hash, vm.evm_interpreter_code_hash, abi.is_constructor_call, + storage, )?; - // Unlike all other gas costs, this one is not paid if low on gas. - if decommit_cost <= vm.gas_left()? { - vm.decrease_gas(decommit_cost)?; + if vm.decrease_gas(mandated_gas).is_err() { + vm.set_gas_left(0)?; + mandated_gas = 0; + panic_from_far_call(vm, opcode)?; } else { - return Err(EraVmError::DecommitFailed); + // Pay for decommit + // Unlike all other gas costs, this one is not paid if low on gas. + if decommit_cost <= vm.gas_left()? { + vm.decrease_gas(decommit_cost)?; + } else { + return Err(EraVmError::DecommitFailed); + } } - let FarCallParams { - ergs_passed, - forward_memory, - .. - } = far_call_params_from_register(src0, vm)?; - - let mandated_gas = if abi.is_system_call && src1.value == ADDRESS_MSG_VALUE.into() { - MSG_VALUE_SIMULATOR_ADDITIVE_COST - } else { - 0 - }; + let gas_left = vm.gas_left()?; + let maximum_gas = + gas_left / FAR_CALL_GAS_SCALAR_MODIFIER_DIVISOR * FAR_CALL_GAS_SCALAR_MODIFIER_DIVIDEND; + let ergs_passed = ergs_passed.min(maximum_gas); + vm.decrease_gas(ergs_passed)?; // mandated gas can surprass the 63/64 limit let ergs_passed = ergs_passed + mandated_gas; - vm.decrease_gas(ergs_passed)?; - let stipend = if is_evm { EVM_SIMULATOR_STIPEND } else { 0 }; let ergs_passed = (ergs_passed) .checked_add(stipend) .expect("stipend must not cause overflow"); - let (program_code, was_decommited) = state.decommit(code_key); + let (program_code, was_decommited) = state.decommit(code_key, storage); let program_code = program_code.ok_or(StorageError::KeyNotPresent)?; if !was_decommited { diff --git a/src/op_handlers/log.rs b/src/op_handlers/log.rs index b8af16e6..cf7d785f 100644 --- a/src/op_handlers/log.rs +++ b/src/op_handlers/log.rs @@ -6,7 +6,7 @@ use crate::{ VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES, STORAGE_WRITE_STORAGE_APPLICATION_CYCLES, }, - store::StorageKey, + store::{Storage, StorageKey}, value::TaggedValue, Opcode, }; @@ -16,6 +16,7 @@ pub fn storage_write( opcode: &Opcode, state: &mut VMState, statistics: &mut VmStatistics, + storage: &mut dyn Storage, ) -> Result<(), EraVmError> { let key_for_contract_storage = vm.get_register(opcode.src0_index).value; let address = vm.current_context()?.contract_address; @@ -24,7 +25,7 @@ pub fn storage_write( statistics.storage_application_cycles += STORAGE_WRITE_STORAGE_APPLICATION_CYCLES; } let value = vm.get_register(opcode.src1_index).value; - let refund = state.storage_write(key, value); + let refund = state.storage_write(key, value, storage); vm.increase_gas(refund)?; Ok(()) } @@ -34,6 +35,7 @@ pub fn storage_read( opcode: &Opcode, state: &mut VMState, statistics: &mut VmStatistics, + storage: &mut dyn Storage, ) -> Result<(), EraVmError> { let key_for_contract_storage = vm.get_register(opcode.src0_index).value; let address = vm.current_context()?.contract_address; @@ -43,7 +45,7 @@ pub fn storage_read( if !state.read_storage_slots().contains(&key) && !state.written_storage_slots().contains(&key) { statistics.storage_application_cycles += STORAGE_READ_STORAGE_APPLICATION_CYCLES; } - let (value, refund) = state.storage_read(key); + let (value, refund) = state.storage_read(key, storage); vm.increase_gas(refund)?; vm.set_register(opcode.dst0_index, TaggedValue::new_raw_integer(value)); Ok(()) @@ -82,6 +84,10 @@ pub fn add_l2_to_l1_message( ) -> Result<(), EraVmError> { let key = vm_state.get_register(opcode.src0_index).value; let value = vm_state.get_register(opcode.src1_index).value; + println!( + "L1 TO L2 regusters indexes {} {}", + opcode.src0_index, opcode.src1_index + ); let is_service = opcode.imm0 == 1; state.record_l2_to_l1_log(L2ToL1Log { key, diff --git a/src/op_handlers/opcode_decommit.rs b/src/op_handlers/opcode_decommit.rs index 4a530407..863e20f8 100644 --- a/src/op_handlers/opcode_decommit.rs +++ b/src/op_handlers/opcode_decommit.rs @@ -6,6 +6,7 @@ use crate::{ execution::Execution, state::VMState, statistics::{VmStatistics, STORAGE_READ_STORAGE_APPLICATION_CYCLES}, + store::Storage, value::{FatPointer, TaggedValue}, Opcode, }; @@ -15,6 +16,7 @@ pub fn opcode_decommit( opcode: &Opcode, state: &mut VMState, statistics: &mut VmStatistics, + storage: &mut dyn Storage, ) -> Result<(), EraVmError> { let (src0, src1) = address_operands_read(vm, opcode)?; @@ -34,7 +36,7 @@ pub fn opcode_decommit( return Ok(()); } - let (code, was_decommited) = state.decommit(code_hash); + let (code, was_decommited) = state.decommit(code_hash, storage); if was_decommited { // refund it vm.increase_gas(extra_cost)?; diff --git a/src/state.rs b/src/state.rs index 7c7aee3e..4cc37acc 100644 --- a/src/state.rs +++ b/src/state.rs @@ -5,11 +5,7 @@ use crate::{ }, store::{Storage, StorageKey}, }; -use std::{ - cell::RefCell, - collections::{HashMap, HashSet}, - rc::Rc, -}; +use std::collections::{HashMap, HashSet}; use u256::{H160, U256}; use zkevm_opcode_defs::system_params::{ STORAGE_ACCESS_COLD_READ_COST, STORAGE_ACCESS_COLD_WRITE_COST, STORAGE_ACCESS_WARM_READ_COST, @@ -41,7 +37,6 @@ pub struct Event { #[derive(Debug, Clone)] pub struct VMState { - pub storage: Rc>, storage_changes: RollbackableHashMap, transient_storage: RollbackableHashMap, l2_to_l1_logs: RollbackableVec, @@ -60,9 +55,8 @@ pub struct VMState { } impl VMState { - pub fn new(storage: Rc>) -> Self { + pub fn new() -> Self { Self { - storage, storage_changes: RollbackableHashMap::::default(), transient_storage: RollbackableHashMap::::default(), l2_to_l1_logs: RollbackableVec::::default(), @@ -78,7 +72,7 @@ impl VMState { } pub fn reset(&mut self) { - *self = Self::new(self.storage.clone()); + *self = Self::new(); } pub fn storage_changes(&self) -> &HashMap { @@ -137,13 +131,11 @@ impl VMState { // reads shouldn't be mutable, we should consider change it to a non-mutable reference // though that would require a refactor in the integration with the operator - pub fn storage_read(&mut self, key: StorageKey) -> (U256, u32) { + pub fn storage_read(&mut self, key: StorageKey, storage: &mut dyn Storage) -> (U256, u32) { let value = self - .storage_read_inner(&key) + .storage_read_inner(&key, storage) .map_or_else(U256::zero, |val| val); - let storage = self.storage.borrow(); - let refund = if storage.is_free_storage_slot(&key) || self.read_storage_slots.map.contains(&key) { WARM_READ_REFUND @@ -158,11 +150,14 @@ impl VMState { (value, refund) } - pub fn storage_read_with_no_refund(&mut self, key: StorageKey) -> U256 { + pub fn storage_read_with_no_refund( + &mut self, + key: StorageKey, + storage: &mut dyn Storage, + ) -> U256 { let value = self - .storage_read_inner(&key) + .storage_read_inner(&key, storage) .map_or_else(U256::zero, |val| val); - let storage = self.storage.borrow(); if !storage.is_free_storage_slot(&key) && !self.read_storage_slots.map.contains(&key) { self.read_storage_slots.map.insert(key); @@ -173,18 +168,21 @@ impl VMState { value } - fn storage_read_inner(&self, key: &StorageKey) -> Option { + fn storage_read_inner(&self, key: &StorageKey, storage: &mut dyn Storage) -> Option { match self.storage_changes.map.get(key) { - None => self.storage.borrow_mut().storage_read(key), + None => storage.storage_read(key), value => value.copied(), } } - pub fn storage_write(&mut self, key: StorageKey, value: U256) -> u32 { + pub fn storage_write( + &mut self, + key: StorageKey, + value: U256, + storage: &mut dyn Storage, + ) -> u32 { self.storage_changes.map.insert(key, value); - let mut storage = self.storage.borrow_mut(); - if storage.is_free_storage_slot(&key) { self.written_storage_slots.map.insert(key); let refund = WARM_WRITE_REFUND; @@ -257,9 +255,9 @@ impl VMState { /// A tuple containing: /// - `Option>`: the contract bytecode /// - `bool`: A boolean flag indicating whether the hash was decommitted (`true` if it was newly decommitted, `false` if it had already been decommitted). - pub fn decommit(&mut self, hash: U256) -> (Option>, bool) { + pub fn decommit(&mut self, hash: U256, storage: &mut dyn Storage) -> (Option>, bool) { let was_decommitted = !self.decommitted_hashes.map.insert(hash); - (self.storage.borrow_mut().decommit(hash), was_decommitted) + (storage.decommit(hash), was_decommitted) } pub fn decommitted_hashes(&self) -> &HashSet { @@ -274,12 +272,15 @@ impl VMState { /// - `StorageKey`: The key for the storage value. /// - `Option`: The initial value from the storage. /// - `U256`: The current value after the change. - pub fn get_storage_changes(&mut self) -> Vec<(StorageKey, Option, U256)> { + pub fn get_storage_changes( + &mut self, + storage: &mut dyn Storage, + ) -> Vec<(StorageKey, Option, U256)> { self.storage_changes .map .iter() .filter_map(|(key, value)| { - let initial_value = self.storage.borrow_mut().storage_read(key); + let initial_value = storage.storage_read(key); if initial_value.unwrap_or_default() == *value { None } else { @@ -302,12 +303,13 @@ impl VMState { pub fn get_storage_changes_from_snapshot( &self, snapshot: as Rollbackable>::Snapshot, + storage: &mut dyn Storage, ) -> Vec<(StorageKey, Option, U256, bool)> { self.storage_changes .get_logs_after_snapshot(snapshot) .iter() .map(|(key, (before, after))| { - let initial = self.storage.borrow_mut().storage_read(key); + let initial = storage.storage_read(key); (*key, before.or(initial), *after, initial.is_none()) }) .collect() diff --git a/src/vm.rs b/src/vm.rs index bf702a59..dfd3db90 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -1,6 +1,3 @@ -use std::cell::RefCell; -use std::rc::Rc; - use u256::U256; use zkevm_opcode_defs::{ BinopOpcode, ContextOpcode, LogOpcode, PtrOpcode, RetOpcode, ShiftOpcode, UMAOpcode, @@ -77,43 +74,53 @@ pub enum EncodingMode { } impl EraVM { - pub fn new(execution: Execution, storage: Rc>) -> Self { + pub fn new(execution: Execution) -> Self { Self { - state: VMState::new(storage), + state: VMState::new(), statistics: VmStatistics::default(), execution, } } /// Run a vm program with a given bytecode. - pub fn run_program_with_custom_bytecode(&mut self) -> ExecutionOutput { - self.run_opcodes(None) + pub fn run_program_with_custom_bytecode( + &mut self, + storage: &mut dyn Storage, + ) -> ExecutionOutput { + self.run_opcodes(None, storage) } - pub fn run_program_with_test_encode(&mut self) -> ExecutionOutput { - self.run(&mut NoTracer::default(), EncodingMode::Testing) + pub fn run_program_with_test_encode(&mut self, storage: &mut dyn Storage) -> ExecutionOutput { + self.run(&mut NoTracer::default(), EncodingMode::Testing, storage) .unwrap_or(ExecutionOutput::Panic) } pub fn run_program_with_custom_bytecode_and_tracer( &mut self, tracer: &mut dyn Tracer, + storage: &mut dyn Storage, ) -> ExecutionOutput { - self.run_opcodes(Some(tracer)) + self.run_opcodes(Some(tracer), storage) } pub fn run_program_with_test_encode_and_tracer( &mut self, tracer: &mut dyn Tracer, + storage: &mut dyn Storage, ) -> ExecutionOutput { - self.run(tracer, EncodingMode::Testing) + self.run(tracer, EncodingMode::Testing, storage) .unwrap_or(ExecutionOutput::Panic) } - fn run_opcodes(&mut self, tracer: Option<&mut dyn Tracer>) -> ExecutionOutput { + fn run_opcodes( + &mut self, + tracer: Option<&mut dyn Tracer>, + storage: &mut dyn Storage, + ) -> ExecutionOutput { self.run( tracer.unwrap_or(&mut NoTracer::default()), EncodingMode::Production, + storage, ) .unwrap_or(ExecutionOutput::Panic) } @@ -159,6 +166,7 @@ impl EraVM { &mut self, tracer: &mut dyn Tracer, enc_mode: EncodingMode, + storage: &mut dyn Storage, ) -> Result { loop { tracer.before_decoding(&mut self.execution, &mut self.state); @@ -236,12 +244,14 @@ impl EraVM { &opcode, &mut self.state, &mut self.statistics, + storage, ), LogOpcode::StorageWrite => storage_write( &mut self.execution, &opcode, &mut self.state, &mut self.statistics, + storage, ), LogOpcode::ToL1Message => { add_l2_to_l1_message(&mut self.execution, &opcode, &mut self.state) @@ -258,6 +268,7 @@ impl EraVM { &opcode, &mut self.state, &mut self.statistics, + storage, ), LogOpcode::TransientStorageRead => { transient_storage_read(&mut self.execution, &opcode, &mut self.state) @@ -273,6 +284,7 @@ impl EraVM { &far_call_variant, &mut self.state, &mut self.statistics, + storage, ); if res.is_err() { panic_from_far_call(&mut self.execution, &opcode)?; From 05c6bb96545695c993a5b7b2c918f6adc7be6e6b Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Tue, 3 Sep 2024 11:50:52 -0300 Subject: [PATCH 48/57] Update zksync era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index ec31b9e2..0e0c2577 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit ec31b9e2c1639f7480a800ac0505df62b551e521 +Subproject commit 0e0c2577f8e5efb57f7bff3fd9bc9657c7cfbb81 From beb8ef0e6c44e1d6b6c55ecf75d1efd6ed5c0244 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Tue, 3 Sep 2024 12:09:10 -0300 Subject: [PATCH 49/57] Update compiler tester submodule --- era-compiler-tester | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/era-compiler-tester b/era-compiler-tester index 62ae4bce..8f429e8b 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit 62ae4bcee174e213021f36aaa8ca0bde00a3b665 +Subproject commit 8f429e8b962cb5fb1298b2b7f40f8cfbd24f0483 From 264c0d076b9842fe89c7f2008a2ae72331c39a0d Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Tue, 3 Sep 2024 12:13:53 -0300 Subject: [PATCH 50/57] Lint --- src/state.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/state.rs b/src/state.rs index 4cc37acc..aa6b4cce 100644 --- a/src/state.rs +++ b/src/state.rs @@ -54,6 +54,12 @@ pub struct VMState { decommitted_hashes: RollbackableHashSet, } +impl Default for VMState { + fn default() -> Self { + Self::new() + } +} + impl VMState { pub fn new() -> Self { Self { From d9f581f89efdf97d5e29ec7c4fa3bae7da3caf04 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:02:28 -0300 Subject: [PATCH 51/57] Remove print --- src/op_handlers/log.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/op_handlers/log.rs b/src/op_handlers/log.rs index cf7d785f..147e8cba 100644 --- a/src/op_handlers/log.rs +++ b/src/op_handlers/log.rs @@ -84,10 +84,6 @@ pub fn add_l2_to_l1_message( ) -> Result<(), EraVmError> { let key = vm_state.get_register(opcode.src0_index).value; let value = vm_state.get_register(opcode.src1_index).value; - println!( - "L1 TO L2 regusters indexes {} {}", - opcode.src0_index, opcode.src1_index - ); let is_service = opcode.imm0 == 1; state.record_l2_to_l1_log(L2ToL1Log { key, From 1e6167a2c1a92d43fcef2b04c5cfacf734238da7 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:23:04 -0300 Subject: [PATCH 52/57] Update zksync era submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 0e0c2577..87dd05ca 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 0e0c2577f8e5efb57f7bff3fd9bc9657c7cfbb81 +Subproject commit 87dd05ca77fca5b343c5935b5b033816d9674b4a From 2a213dc0841d0d029a3366d59ab728be1906c2cb Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Wed, 4 Sep 2024 13:06:19 -0300 Subject: [PATCH 53/57] Add heap size optimization --- src/execution.rs | 42 +++++++++++++++++++++++++++++++----------- src/heaps.rs | 11 +++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/execution.rs b/src/execution.rs index 63acca84..dd9fe01d 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -26,6 +26,7 @@ pub struct Stack { #[derive(Debug, Clone, PartialEq)] pub struct Heap { heap: Vec, + size: u32, } #[derive(Debug, Clone, PartialEq)] @@ -448,20 +449,26 @@ impl Stack { impl Default for Heap { fn default() -> Self { - Self::new(vec![]) + Self::new(vec![], 0) } } impl Heap { - pub fn new(values: Vec) -> Self { - Self { heap: values } + pub fn new(values: Vec, size: u32) -> Self { + Self { heap: values, size } } - // Returns how many ergs the expand costs - pub fn expand_memory(&mut self, address: u32) -> u32 { + fn expand_memory_inner(&mut self, address: u32) { if address >= self.heap.len() as u32 { - let old_size = self.heap.len() as u32; self.heap.resize(address as usize, 0); - return MEMORY_GROWTH_ERGS_PER_BYTE * (address - old_size); + } + } + + // Returns how many ergs the expand costs + pub fn expand_memory(&mut self, address: u32) -> u32 { + if address >= self.size { + let old_size = self.size; + self.size = address; + return MEMORY_GROWTH_ERGS_PER_BYTE * (self.size - old_size); } 0 } @@ -469,13 +476,20 @@ impl Heap { pub fn store(&mut self, address: u32, value: U256) { let start = address as usize; let end = start + 32; + self.expand_memory_inner(end as u32); value.to_big_endian(&mut self.heap[start..end]); } pub fn read(&self, address: u32) -> U256 { let start = address as usize; - let end = start + 32; - U256::from_big_endian(&self.heap[start..end]) + let mut end = start + 32; + if start >= self.heap.len() { + return U256::zero(); + } + end = end.min(self.heap.len()); + let mut result = self.heap[start..end].to_vec(); + result.resize(32, 0); + U256::from_big_endian(&result) } pub fn expanded_read(&mut self, address: u32) -> (U256, u32) { @@ -485,6 +499,9 @@ impl Heap { } pub fn read_byte(&self, address: u32) -> u8 { + if address >= self.heap.len() as u32 { + return 0; + } self.heap[address as usize] } @@ -492,6 +509,9 @@ impl Heap { let mut result = U256::zero(); for i in 0..32 { let addr = pointer.start + pointer.offset + (31 - i); + if addr >= self.heap.len() as u32 { + continue; + } result |= U256::from(self.heap[addr as usize]) << (i * 8); } result @@ -504,10 +524,10 @@ impl Heap { } pub fn len(&self) -> usize { - self.heap.len() + self.size as usize } pub fn is_empty(&self) -> bool { - self.heap.is_empty() + self.size == 0 } } diff --git a/src/heaps.rs b/src/heaps.rs index e02f4747..8fadfddf 100644 --- a/src/heaps.rs +++ b/src/heaps.rs @@ -11,9 +11,10 @@ impl Heaps { pub fn new(calldata: Vec) -> Self { // The first heap can never be used because heap zero // means the current heap in precompile calls + let size = calldata.len() as u32; let heaps = vec![ Heap::default(), - Heap::new(calldata), + Heap::new(calldata, size), Heap::default(), Heap::default(), ]; @@ -23,15 +24,13 @@ impl Heaps { pub fn allocate(&mut self) -> u32 { let id = self.heaps.len() as u32; - self.heaps - .push(Heap::new(vec![0; NEW_FRAME_MEMORY_STIPEND as usize])); + self.heaps.push(Heap::new(vec![], NEW_FRAME_MEMORY_STIPEND)); id } pub fn allocate_copy(&mut self) -> u32 { - let id = self.heaps.len() as u32; - self.heaps - .push(Heap::new(vec![0; NEW_FRAME_MEMORY_STIPEND as usize])); + let id: u32 = self.heaps.len() as u32; + self.heaps.push(Heap::new(vec![], NEW_FRAME_MEMORY_STIPEND)); id } From a44f3b809cdffd851f5737fa88ec1e0ac8409a8f Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Wed, 4 Sep 2024 13:45:27 -0300 Subject: [PATCH 54/57] Update submodules --- era-compiler-tester | 2 +- zksync-era | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/era-compiler-tester b/era-compiler-tester index 8f429e8b..3cf60585 160000 --- a/era-compiler-tester +++ b/era-compiler-tester @@ -1 +1 @@ -Subproject commit 8f429e8b962cb5fb1298b2b7f40f8cfbd24f0483 +Subproject commit 3cf60585cfc6577a37aaea8d79cfa0a5b04b86c9 diff --git a/zksync-era b/zksync-era index 87dd05ca..ec01627d 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 87dd05ca77fca5b343c5935b5b033816d9674b4a +Subproject commit ec01627d482355915ba26ac7310a33349b927c70 From c4dab0e7f2deabffa7fb121b4cd995685eaa87e5 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:38:37 -0300 Subject: [PATCH 55/57] Address comments --- src/execution.rs | 10 ++++------ zksync-era | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/execution.rs b/src/execution.rs index dd9fe01d..4172fcdc 100644 --- a/src/execution.rs +++ b/src/execution.rs @@ -507,12 +507,10 @@ impl Heap { pub fn read_from_pointer(&self, pointer: &FatPointer) -> U256 { let mut result = U256::zero(); - for i in 0..32 { - let addr = pointer.start + pointer.offset + (31 - i); - if addr >= self.heap.len() as u32 { - continue; - } - result |= U256::from(self.heap[addr as usize]) << (i * 8); + let start = pointer.start + pointer.offset; + let end = self.heap.len().min((start + 32) as usize) as u32; + for (i, addr) in (start..end).enumerate() { + result |= U256::from(self.heap[addr as usize]) << ((31 - i) * 8); } result } diff --git a/zksync-era b/zksync-era index ec01627d..f6be6d9f 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit ec01627d482355915ba26ac7310a33349b927c70 +Subproject commit f6be6d9f57cbdeb768bc9ccc8fed13b875a63ed2 From 274fa7ee3175177e80196f548e353439fb3ed226 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Wed, 4 Sep 2024 17:38:47 -0300 Subject: [PATCH 56/57] Lint --- src/state.rs | 4 ++-- zksync-era | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/state.rs b/src/state.rs index 1a150e49..950f2b83 100644 --- a/src/state.rs +++ b/src/state.rs @@ -129,11 +129,11 @@ impl VMState { } pub fn read_storage_slots(&self) -> &HashSet { - &self.read_storage_slots.inner_ref() + self.read_storage_slots.inner_ref() } pub fn written_storage_slots(&self) -> &HashSet { - &self.written_storage_slots.inner_ref() + self.written_storage_slots.inner_ref() } // reads shouldn't be mutable, we should consider change it to a non-mutable reference diff --git a/zksync-era b/zksync-era index 96bf03de..858e45e4 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 96bf03de9b23cbcc8e61c20fed95ad5a72c7a262 +Subproject commit 858e45e497e00fb510ec67decac2cb5ff934e315 From 9abd7fa3bb8a63af225e17473436ef1b519d0765 Mon Sep 17 00:00:00 2001 From: Gianbelinche <39842759+gianbelinche@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:05:56 -0300 Subject: [PATCH 57/57] Update submodule --- zksync-era | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zksync-era b/zksync-era index 858e45e4..13d45a40 160000 --- a/zksync-era +++ b/zksync-era @@ -1 +1 @@ -Subproject commit 858e45e497e00fb510ec67decac2cb5ff934e315 +Subproject commit 13d45a409e50320d735310d46b9ad5bbba1ae5ab