From eebfff0037f331841ff503f9f0b2d46f2480ef51 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Wed, 20 Nov 2024 16:49:56 +1100 Subject: [PATCH 1/2] move impersonation state into its own subcomponent --- src/node/eth.rs | 36 ++++++++++------------- src/node/impersonate.rs | 61 +++++++++++++++++++++++++++++++++++++++ src/node/in_memory.rs | 30 ++++++++----------- src/node/in_memory_ext.rs | 48 +++++++++++++----------------- src/node/mod.rs | 1 + 5 files changed, 109 insertions(+), 67 deletions(-) create mode 100644 src/node/impersonate.rs diff --git a/src/node/eth.rs b/src/node/eth.rs index 4b8275f0..65ef02e0 100644 --- a/src/node/eth.rs +++ b/src/node/eth.rs @@ -171,22 +171,16 @@ impl InMemoryNo l2_tx.set_input(bytes, hash); + if !self + .impersonation + .is_impersonating(&l2_tx.common_data.initiator_address) { - let reader = self - .inner - .read() - .map_err(|_| anyhow::anyhow!("Failed to acquire read lock for accounts."))?; - if !reader - .impersonated_accounts - .contains(&l2_tx.common_data.initiator_address) - { - let err = format!( - "Initiator address {:?} is not allowed to perform transactions", - l2_tx.common_data.initiator_address - ); - tracing::error!("{err}"); - return Err(TransparentError(err).into()); - } + let err = format!( + "Initiator address {:?} is not allowed to perform transactions", + l2_tx.common_data.initiator_address + ); + tracing::error!("{err}"); + return Err(TransparentError(err).into()); } self.seal_block(vec![l2_tx], system_contracts) @@ -2949,7 +2943,7 @@ mod tests { .filters .add_block_filter() .expect("failed adding block filter"); - inner.impersonated_accounts.insert(H160::repeat_byte(0x1)); + inner.impersonation.impersonate(H160::repeat_byte(0x1)); inner.rich_accounts.insert(H160::repeat_byte(0x1)); inner .previous_states @@ -2970,7 +2964,7 @@ mod tests { blocks: inner.blocks.clone(), block_hashes: inner.block_hashes.clone(), filters: inner.filters.clone(), - impersonated_accounts: inner.impersonated_accounts.clone(), + impersonated_accounts: inner.impersonation.impersonated_accounts(), rich_accounts: inner.rich_accounts.clone(), previous_states: inner.previous_states.clone(), raw_storage: storage.raw_storage.clone(), @@ -3055,7 +3049,7 @@ mod tests { .filters .add_block_filter() .expect("failed adding block filter"); - inner.impersonated_accounts.insert(H160::repeat_byte(0x1)); + inner.impersonation.impersonate(H160::repeat_byte(0x1)); inner.rich_accounts.insert(H160::repeat_byte(0x1)); inner .previous_states @@ -3077,7 +3071,7 @@ mod tests { blocks: inner.blocks.clone(), block_hashes: inner.block_hashes.clone(), filters: inner.filters.clone(), - impersonated_accounts: inner.impersonated_accounts.clone(), + impersonated_accounts: inner.impersonation.impersonated_accounts(), rich_accounts: inner.rich_accounts.clone(), previous_states: inner.previous_states.clone(), raw_storage: storage.raw_storage.clone(), @@ -3108,7 +3102,7 @@ mod tests { .filters .add_pending_transaction_filter() .expect("failed adding pending transaction filter"); - inner.impersonated_accounts.insert(H160::repeat_byte(0x2)); + inner.impersonation.impersonate(H160::repeat_byte(0x2)); inner.rich_accounts.insert(H160::repeat_byte(0x2)); inner .previous_states @@ -3148,7 +3142,7 @@ mod tests { assert_eq!(expected_snapshot.filters, inner.filters); assert_eq!( expected_snapshot.impersonated_accounts, - inner.impersonated_accounts + inner.impersonation.impersonated_accounts() ); assert_eq!(expected_snapshot.rich_accounts, inner.rich_accounts); assert_eq!(expected_snapshot.previous_states, inner.previous_states); diff --git a/src/node/impersonate.rs b/src/node/impersonate.rs new file mode 100644 index 00000000..4a99187e --- /dev/null +++ b/src/node/impersonate.rs @@ -0,0 +1,61 @@ +use std::collections::HashSet; +use std::sync::{Arc, RwLock}; +use zksync_types::Address; + +/// Manages impersonated accounts across the system. +/// +/// Clones always agree on the set of impersonated accounts and updating one affects all other +/// instances. +#[derive(Clone, Debug, Default)] +pub struct ImpersonationManager { + state: Arc>>, +} + +impl ImpersonationManager { + /// Starts impersonation for the provided account. + /// + /// Returns `true` if the account was not impersonated before. + pub fn impersonate(&self, addr: Address) -> bool { + tracing::trace!(?addr, "start impersonation"); + let mut state = self + .state + .write() + .expect("ImpersonationManager lock is poisoned"); + state.insert(addr) + } + + /// Stops impersonation for the provided account. + /// + /// Returns `true` if the account was impersonated before. + pub fn stop_impersonating(&self, addr: &Address) -> bool { + tracing::trace!(?addr, "stop impersonation"); + self.state + .write() + .expect("ImpersonationManager lock is poisoned") + .remove(addr) + } + + /// Returns whether the provided account is currently impersonated. + pub fn is_impersonating(&self, addr: &Address) -> bool { + self.state + .read() + .expect("ImpersonationManager lock is poisoned") + .contains(addr) + } + + /// Returns all accounts that are currently being impersonated. + pub fn impersonated_accounts(&self) -> HashSet
{ + self.state + .read() + .expect("ImpersonationManager lock is poisoned") + .clone() + } + + /// Overrides currently impersonated accounts with the provided value. + pub fn set_impersonated_accounts(&self, accounts: HashSet
) { + *self + .state + .write() + .expect("ImpersonationManager lock is poisoned") = accounts; + } +} diff --git a/src/node/in_memory.rs b/src/node/in_memory.rs index 772c0911..2a1e17f6 100644 --- a/src/node/in_memory.rs +++ b/src/node/in_memory.rs @@ -46,6 +46,7 @@ use zksync_types::{ use zksync_utils::{bytecode::hash_bytecode, h256_to_account_address, h256_to_u256, u256_to_h256}; use zksync_web3_decl::error::Web3Error; +use crate::node::impersonate::ImpersonationManager; use crate::node::time::TimestampManager; use crate::{ bootloader_debug::{BootloaderDebug, BootloaderDebugTracer}, @@ -220,7 +221,7 @@ pub struct InMemoryNodeInner { pub config: TestNodeConfig, pub console_log_handler: ConsoleLogHandler, pub system_contracts: SystemContracts, - pub impersonated_accounts: HashSet
, + pub impersonation: ImpersonationManager, pub rich_accounts: HashSet, /// Keeps track of historical states indexed via block hash. Limited to [MAX_PREVIOUS_STATES]. pub previous_states: IndexMap>, @@ -294,7 +295,7 @@ impl InMemoryNodeInner { &updated_config.system_contracts_options, updated_config.use_evm_emulator, ), - impersonated_accounts: Default::default(), + impersonation: Default::default(), rich_accounts: HashSet::new(), previous_states: Default::default(), observability, @@ -329,7 +330,7 @@ impl InMemoryNodeInner { &config.system_contracts_options, config.use_evm_emulator, ), - impersonated_accounts: Default::default(), + impersonation: Default::default(), rich_accounts: HashSet::new(), previous_states: Default::default(), observability, @@ -457,7 +458,7 @@ impl InMemoryNodeInner { let initiator_address = request_with_gas_per_pubdata_overridden .from .unwrap_or_default(); - let impersonating = self.impersonated_accounts.contains(&initiator_address); + let impersonating = self.impersonation.is_impersonating(&initiator_address); let system_contracts = self .system_contracts .contracts_for_fee_estimate(impersonating) @@ -769,17 +770,6 @@ impl InMemoryNodeInner { vm.execute(InspectExecutionMode::OneTx) } - /// Sets the `impersonated_account` field of the node. - /// This field is used to override the `tx.initiator_account` field of the transaction in the `run_l2_tx` method. - pub fn set_impersonated_account(&mut self, address: Address) -> bool { - self.impersonated_accounts.insert(address) - } - - /// Clears the `impersonated_account` field of the node. - pub fn stop_impersonating_account(&mut self, address: Address) -> bool { - self.impersonated_accounts.remove(&address) - } - /// Archives the current state for later queries. pub fn archive_state(&mut self) -> Result<(), String> { if self.previous_states.len() > MAX_PREVIOUS_STATES as usize { @@ -824,7 +814,7 @@ impl InMemoryNodeInner { blocks: self.blocks.clone(), block_hashes: self.block_hashes.clone(), filters: self.filters.clone(), - impersonated_accounts: self.impersonated_accounts.clone(), + impersonated_accounts: self.impersonation.impersonated_accounts(), rich_accounts: self.rich_accounts.clone(), previous_states: self.previous_states.clone(), raw_storage: storage.raw_storage.clone(), @@ -851,7 +841,8 @@ impl InMemoryNodeInner { self.blocks = snapshot.blocks; self.block_hashes = snapshot.block_hashes; self.filters = snapshot.filters; - self.impersonated_accounts = snapshot.impersonated_accounts; + self.impersonation + .set_impersonated_accounts(snapshot.impersonated_accounts); self.rich_accounts = snapshot.rich_accounts; self.previous_states = snapshot.previous_states; storage.raw_storage = snapshot.raw_storage; @@ -898,6 +889,7 @@ pub struct InMemoryNode { #[allow(dead_code)] pub(crate) system_contracts_options: system_contracts::Options, pub(crate) time: TimestampManager, + pub(crate) impersonation: ImpersonationManager, } fn contract_address_from_tx_result(execution_result: &VmExecutionResultAndLogs) -> Option { @@ -924,11 +916,13 @@ impl InMemoryNode { let system_contracts_options = config.system_contracts_options; let inner = InMemoryNodeInner::new(fork, observability, config); let time = inner.time.clone(); + let impersonation = inner.impersonation.clone(); InMemoryNode { inner: Arc::new(RwLock::new(inner)), snapshots: Default::default(), system_contracts_options, time, + impersonation, } } @@ -1051,7 +1045,7 @@ impl InMemoryNode { .inner .read() .map_err(|_| anyhow::anyhow!("Failed to acquire read lock"))?; - Ok(if inner.impersonated_accounts.contains(&tx_initiator) { + Ok(if inner.impersonation.is_impersonating(&tx_initiator) { tracing::info!("🕵️ Executing tx from impersonated account {tx_initiator:?}"); inner .system_contracts diff --git a/src/node/in_memory_ext.rs b/src/node/in_memory_ext.rs index f63c2ce2..03aec802 100644 --- a/src/node/in_memory_ext.rs +++ b/src/node/in_memory_ext.rs @@ -303,36 +303,26 @@ impl InMemoryNo } pub fn impersonate_account(&self, address: Address) -> Result { - self.get_inner() - .write() - .map_err(|err| anyhow!("failed acquiring lock: {:?}", err)) - .map(|mut writer| { - if writer.set_impersonated_account(address) { - tracing::info!("🕵️ Account {:?} has been impersonated", address); - true - } else { - tracing::info!("🕵️ Account {:?} was already impersonated", address); - false - } - }) + if self.impersonation.impersonate(address) { + tracing::info!("🕵️ Account {:?} has been impersonated", address); + Ok(true) + } else { + tracing::info!("🕵️ Account {:?} was already impersonated", address); + Ok(false) + } } pub fn stop_impersonating_account(&self, address: Address) -> Result { - self.get_inner() - .write() - .map_err(|err| anyhow!("failed acquiring lock: {:?}", err)) - .map(|mut writer| { - if writer.stop_impersonating_account(address) { - tracing::info!("🕵️ Stopped impersonating account {:?}", address); - true - } else { - tracing::info!( - "🕵️ Account {:?} was not impersonated, nothing to stop", - address - ); - false - } - }) + if self.impersonation.stop_impersonating(&address) { + tracing::info!("🕵️ Stopped impersonating account {:?}", address); + Ok(true) + } else { + tracing::info!( + "🕵️ Account {:?} was not impersonated, nothing to stop", + address + ); + Ok(false) + } } pub fn set_code(&self, address: Address, code: String) -> Result<()> { @@ -502,18 +492,20 @@ mod tests { config: Default::default(), console_log_handler: Default::default(), system_contracts: Default::default(), - impersonated_accounts: Default::default(), + impersonation: Default::default(), rich_accounts: Default::default(), previous_states: Default::default(), observability: None, }; let time = old_inner.time.clone(); + let impersonation = old_inner.impersonation.clone(); let node = InMemoryNode:: { inner: Arc::new(RwLock::new(old_inner)), snapshots: old_snapshots, system_contracts_options: old_system_contracts_options, time, + impersonation, }; let address = Address::from_str("0x36615Cf349d7F6344891B1e7CA7C72883F5dc049").unwrap(); diff --git a/src/node/mod.rs b/src/node/mod.rs index bf8a00d7..7cd35a24 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -8,6 +8,7 @@ mod eth; mod evm; mod fee_model; mod hardhat; +mod impersonate; mod in_memory; mod in_memory_ext; mod net; From 9ed9f17270fafbdfa10074dfb77793bf3612fc66 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Wed, 20 Nov 2024 16:55:04 +1100 Subject: [PATCH 2/2] use frozen lockfile for contracts --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c7360a39..88d0d8b0 100644 --- a/Makefile +++ b/Makefile @@ -4,8 +4,8 @@ fetch-contracts: # Build the system contracts build-contracts: - cd contracts/system-contracts && yarn; yarn install; yarn build; - cd contracts/l2-contracts && yarn; yarn install; yarn build; + cd contracts/system-contracts && yarn install --frozen-lockfile; yarn build; + cd contracts/l2-contracts && yarn install --frozen-lockfile; yarn build; ./scripts/refresh_contracts.sh # Clean the system contracts