From e10fcf5d0bdbe0994e7f9805b1adc2142b63c565 Mon Sep 17 00:00:00 2001 From: dancoombs Date: Thu, 7 Sep 2023 11:38:30 -0400 Subject: [PATCH] fix(sim): updated associated storage on deploy rules, RPC error codes --- src/common/simulation.rs | 66 +++++++++++++++--------------- src/rpc/eth/error.rs | 8 ++-- src/rpc/eth/mod.rs | 7 ++-- test/spec-tests/bundler-spec-tests | 2 +- 4 files changed, 43 insertions(+), 40 deletions(-) diff --git a/src/common/simulation.rs b/src/common/simulation.rs index 596084218..5ecc47aff 100644 --- a/src/common/simulation.rs +++ b/src/common/simulation.rs @@ -162,6 +162,17 @@ where &entry_point_out, self.sim_settings, ); + + if let Some(f) = entity_infos.get(EntityType::Factory) { + tracing::info!( + "Factory found at {:?}. Is staked? {}", + f.address, + f.is_staked + ); + } else { + tracing::info!("No factory found"); + } + let is_unstaked_wallet_creation = entity_infos .get(EntityType::Factory) .filter(|factory| !factory.is_staked) @@ -253,9 +264,7 @@ where for &slot in slots { let restriction = get_storage_restriction(GetStorageRestrictionArgs { slots_by_address: &tracer_out.associated_slots_by_address, - entity, is_unstaked_wallet_creation, - entry_point_address: self.entry_point.address(), entity_address: entity_info.address, sender_address, accessed_address: address, @@ -477,10 +486,20 @@ pub enum SimulationViolation { UsedForbiddenOpcode(Entity, Address, ViolationOpCode), #[display("{0.kind} uses banned precompile: {2:?} in contract {1:?}")] UsedForbiddenPrecompile(Entity, Address, Address), + #[display( + "{0.kind} tried to access code at {1} during validation, but that address is not a contract" + )] + AccessedUndeployedContract(Entity, Address), #[display("factory may only call CREATE2 once during initialization")] FactoryCalledCreate2Twice(Address), #[display("{0.kind} accessed forbidden storage at address {1:?} during validation")] InvalidStorageAccess(Entity, StorageSlot), + #[display("{0.kind} called entry point method other than depositTo")] + CalledBannedEntryPointMethod(Entity), + #[display("{0.kind} must not send ETH during validation (except from account to entry point)")] + CallHadValue(Entity), + #[display("code accessed by validation has changed since the last time validation was run")] + CodeHashChanged, #[display("{0.kind} must be staked")] NotStaked(Entity, U256, U256), #[display("reverted while simulating {0} validation")] @@ -489,18 +508,8 @@ pub enum SimulationViolation { DidNotRevert, #[display("simulateValidation should have 3 parts but had {0} instead. Make sure your EntryPoint is valid")] WrongNumberOfPhases(u32), - #[display("{0.kind} must not send ETH during validation (except from account to entry point)")] - CallHadValue(Entity), #[display("ran out of gas during {0.kind} validation")] OutOfGas(Entity), - #[display( - "{0.kind} tried to access code at {1} during validation, but that address is not a contract" - )] - AccessedUndeployedContract(Entity, Address), - #[display("{0.kind} called entry point method other than depositTo")] - CalledBannedEntryPointMethod(Entity), - #[display("code accessed by validation has changed since the last time validation was run")] - CodeHashChanged, #[display("aggregator signature validation failed")] AggregatorValidationFailed, } @@ -615,9 +624,7 @@ enum StorageRestriction { #[derive(Clone, Copy, Debug)] struct GetStorageRestrictionArgs<'a> { slots_by_address: &'a AssociatedSlotsByAddress, - entity: Entity, is_unstaked_wallet_creation: bool, - entry_point_address: Address, entity_address: Address, sender_address: Address, accessed_address: Address, @@ -627,34 +634,29 @@ struct GetStorageRestrictionArgs<'a> { fn get_storage_restriction(args: GetStorageRestrictionArgs) -> StorageRestriction { let GetStorageRestrictionArgs { slots_by_address, - entity, is_unstaked_wallet_creation, - entry_point_address, entity_address, sender_address, accessed_address, slot, + .. } = args; if accessed_address == sender_address { StorageRestriction::Allowed } else if slots_by_address.is_associated_slot(sender_address, slot) { - if is_unstaked_wallet_creation - && entity.kind != EntityType::Account - && accessed_address != entry_point_address - { - // We deviate from the letter of ERC-4337 to allow an unstaked - // sender to access its own associated storage during account - // creation, based on discussion with the ERC authors. - // - // We also deviate by allowing unstaked access to the sender's - // associated storage on the entry point during account creation. - // Without this, several spec tests fail because the `SimpleWallet` - // used in the tests deposits in its constructor, which causes the - // factory to access the sender's associated storage on the entry - // point. - StorageRestriction::NeedsStake - } else { + tracing::info!( + "Accessed associated storage slot: {:?} entity_address {:?}", + slot, + entity_address + ); + + // Allow entities to access associated storage unless its during an unstaked wallet creation + if !is_unstaked_wallet_creation { + tracing::info!("Allowing access to associated storage, not unstaked wallet creation"); StorageRestriction::Allowed + } else { + tracing::info!("Disallowing access to associated storage, unstaked wallet creation"); + StorageRestriction::NeedsStake } } else if accessed_address == entity_address || slots_by_address.is_associated_slot(entity_address, slot) diff --git a/src/rpc/eth/error.rs b/src/rpc/eth/error.rs index 7f6423326..ebf5b0473 100644 --- a/src/rpc/eth/error.rs +++ b/src/rpc/eth/error.rs @@ -46,9 +46,9 @@ pub enum EthRpcError { /// Opcode violation #[error("{0} uses banned opcode: {1:?}")] OpcodeViolation(EntityType, Opcode), - /// Precompile violation, maps to Opcode Violation - #[error("{0} uses banned precompile: {1:?}")] - PrecompileViolation(EntityType, Address), + /// Used for other simulation violations that map to Opcode Violations + #[error("{0}")] + OpcodeViolationMap(SimulationError), /// Invalid storage access, maps to Opcode Violation #[error("{0} accesses inaccessible storage at address: {1:?} slot: {2:#032x}")] InvalidStorageAccess(EntityType, Address, U256), @@ -150,7 +150,7 @@ impl From for RpcError { rpc_err_with_data(PAYMASTER_VALIDATION_REJECTED_CODE, msg, data) } EthRpcError::OpcodeViolation(_, _) - | EthRpcError::PrecompileViolation(_, _) + | EthRpcError::OpcodeViolationMap(_) | EthRpcError::InvalidStorageAccess(_, _, _) => rpc_err(OPCODE_VIOLATION_CODE, msg), EthRpcError::OutOfTimeRange(data) => { rpc_err_with_data(OUT_OF_TIME_RANGE_CODE, msg, data) diff --git a/src/rpc/eth/mod.rs b/src/rpc/eth/mod.rs index edf6bc0df..79d750141 100644 --- a/src/rpc/eth/mod.rs +++ b/src/rpc/eth/mod.rs @@ -661,9 +661,10 @@ impl From for EthRpcError { SimulationViolation::UsedForbiddenOpcode(entity, _, op) => { Self::OpcodeViolation(entity.kind, op.clone().0) } - SimulationViolation::UsedForbiddenPrecompile(entity, _, precompile) => { - Self::PrecompileViolation(entity.kind, *precompile) - } + SimulationViolation::UsedForbiddenPrecompile(_, _, _) + | SimulationViolation::AccessedUndeployedContract(_, _) + | SimulationViolation::CalledBannedEntryPointMethod(_) + | SimulationViolation::CallHadValue(_) => Self::OpcodeViolationMap(value), SimulationViolation::FactoryCalledCreate2Twice(_) => { Self::OpcodeViolation(EntityType::Factory, Opcode::CREATE2) } diff --git a/test/spec-tests/bundler-spec-tests b/test/spec-tests/bundler-spec-tests index 9a733c020..2e81f94f1 160000 --- a/test/spec-tests/bundler-spec-tests +++ b/test/spec-tests/bundler-spec-tests @@ -1 +1 @@ -Subproject commit 9a733c0204a5ccd0f518022ddc6180313a8fca52 +Subproject commit 2e81f94f1ee21a4a30a90f84b2a11beaef733965