diff --git a/Cargo.lock b/Cargo.lock index bd51a3c319..31b772cce4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2161,7 +2161,7 @@ dependencies = [ [[package]] name = "evm" version = "0.41.2" -source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#aeff7f361687b4c6a7fcbe1cf6e4fe5f2aea32b5" +source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#842749e2a0a0319ebbe2a2fbda07b6b749ad6d95" dependencies = [ "auto_impl", "environmental", @@ -2181,7 +2181,7 @@ dependencies = [ [[package]] name = "evm-core" version = "0.41.0" -source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#aeff7f361687b4c6a7fcbe1cf6e4fe5f2aea32b5" +source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#842749e2a0a0319ebbe2a2fbda07b6b749ad6d95" dependencies = [ "parity-scale-codec", "primitive-types", @@ -2192,7 +2192,7 @@ dependencies = [ [[package]] name = "evm-gasometer" version = "0.41.0" -source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#aeff7f361687b4c6a7fcbe1cf6e4fe5f2aea32b5" +source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#842749e2a0a0319ebbe2a2fbda07b6b749ad6d95" dependencies = [ "environmental", "evm-core", @@ -2203,7 +2203,7 @@ dependencies = [ [[package]] name = "evm-runtime" version = "0.41.0" -source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#aeff7f361687b4c6a7fcbe1cf6e4fe5f2aea32b5" +source = "git+https://github.com/moonbeam-foundation/evm?branch=moonbeam-polkadot-stable2409#842749e2a0a0319ebbe2a2fbda07b6b749ad6d95" dependencies = [ "auto_impl", "environmental", diff --git a/frame/evm/precompile/dispatch/src/mock.rs b/frame/evm/precompile/dispatch/src/mock.rs index 98d21c8978..ef5031384e 100644 --- a/frame/evm/precompile/dispatch/src/mock.rs +++ b/frame/evm/precompile/dispatch/src/mock.rs @@ -217,6 +217,10 @@ impl PrecompileHandle for MockHandle { &self.context } + fn origin(&self) -> H160 { + unimplemented!() + } + fn is_static(&self) -> bool { unimplemented!() } diff --git a/frame/evm/test-vector-support/src/lib.rs b/frame/evm/test-vector-support/src/lib.rs index cd4252c9db..1cd5a8eecb 100644 --- a/frame/evm/test-vector-support/src/lib.rs +++ b/frame/evm/test-vector-support/src/lib.rs @@ -111,6 +111,10 @@ impl PrecompileHandle for MockHandle { &self.context } + fn origin(&self) -> H160 { + unimplemented!() + } + fn is_static(&self) -> bool { self.is_static } diff --git a/precompiles/src/evm/handle.rs b/precompiles/src/evm/handle.rs index 7cc73bc65c..029256517a 100644 --- a/precompiles/src/evm/handle.rs +++ b/precompiles/src/evm/handle.rs @@ -177,6 +177,10 @@ mod tests { unimplemented!() } + fn origin(&self) -> sp_core::H160 { + unimplemented!() + } + fn is_static(&self) -> bool { true } diff --git a/precompiles/src/precompile_set.rs b/precompiles/src/precompile_set.rs index 2c20d78cd8..bf85a29486 100644 --- a/precompiles/src/precompile_set.rs +++ b/precompiles/src/precompile_set.rs @@ -21,7 +21,6 @@ //! default and must be disabled explicely through type annotations. use crate::{ - evm::handle::PrecompileHandleExt, solidity::{codec::String, revert::revert}, EvmResult, }; @@ -304,13 +303,11 @@ impl PrecompileChecks for CallableByPrecompile { #[derive(PartialEq)] #[cfg_attr(feature = "std", derive(Debug))] pub enum AddressType { - /// The code stored at the address is less than 5 bytes, but not well known. - Unknown, /// No code is stored at the address, therefore is EOA. EOA, /// The 5-byte magic constant for a precompile is stored at the address. Precompile, - /// The code is greater than 5-bytes, potentially a Smart Contract. + /// Every address that is not a EOA or a Precompile is potentially a Smart Contract. Contract, } @@ -319,39 +316,18 @@ pub fn get_address_type( handle: &mut impl PrecompileHandle, address: H160, ) -> Result { - // AccountCodesMetadata: - // Blake2128(16) + H160(20) + CodeMetadata(40) - handle.record_db_read::(76)?; - let code_len = pallet_evm::Pallet::::account_code_metadata(address).size; - - // 0 => either EOA or precompile without dummy code - if code_len == 0 { + // It is an Externally Owned Account (EOA) + // - When the transaction origin is equal to the address + if handle.origin() == address { return Ok(AddressType::EOA); } - // dummy code is 5 bytes long, so any other len means it is a contract. - if code_len != 5 { - return Ok(AddressType::Contract); - } - - // check code matches dummy code - handle.record_db_read::(code_len as usize)?; - let code = pallet_evm::AccountCodes::::get(address); - if code == [0x60, 0x00, 0x60, 0x00, 0xfd] { + // Check if address is a precompile + if let Ok(true) = is_precompile_or_fail::(address, handle.remaining_gas()) { return Ok(AddressType::Precompile); } - Ok(AddressType::Unknown) -} - -fn is_address_eoa_or_precompile( - handle: &mut impl PrecompileHandle, - address: H160, -) -> Result { - match get_address_type::(handle, address)? { - AddressType::EOA | AddressType::Precompile => Ok(true), - _ => Ok(false), - } + Ok(AddressType::Contract) } /// Common checks for precompile and precompile sets. @@ -375,19 +351,23 @@ fn common_checks( u32::from_be_bytes(buffer) }); - // Is this selector callable from a smart contract? - let callable_by_smart_contract = - C::callable_by_smart_contract(caller, selector).unwrap_or(false); - if !callable_by_smart_contract && !is_address_eoa_or_precompile::(handle, caller)? { - return Err(revert("Function not callable by smart contracts")); - } + let caller_address_type = get_address_type::(handle, caller)?; // Is this selector callable from a precompile? let callable_by_precompile = C::callable_by_precompile(caller, selector).unwrap_or(false); - if !callable_by_precompile && is_precompile_or_fail::(caller, handle.remaining_gas())? { + let is_precompile = caller_address_type == AddressType::Precompile; + if !callable_by_precompile && is_precompile { return Err(revert("Function not callable by precompiles")); } + // Is this selector callable from a smart contract? + let callable_by_smart_contract = + C::callable_by_smart_contract(caller, selector).unwrap_or(false); + let is_smart_contract = caller_address_type == AddressType::Contract; + if !callable_by_smart_contract && is_smart_contract { + return Err(revert("Function not callable by smart contracts")); + } + Ok(()) } @@ -463,6 +443,10 @@ impl<'a, H: PrecompileHandle> PrecompileHandle for RestrictiveHandle<'a, H> { self.handle.context() } + fn origin(&self) -> H160 { + self.handle.origin() + } + fn is_static(&self) -> bool { self.handle.is_static() } diff --git a/precompiles/src/testing/execution.rs b/precompiles/src/testing/execution.rs index e8279ee7dd..1f53c6a3c8 100644 --- a/precompiles/src/testing/execution.rs +++ b/precompiles/src/testing/execution.rs @@ -72,6 +72,11 @@ impl<'p, P: PrecompileSet> PrecompilesTester<'p, P> { } } + pub fn with_origin(mut self, origin: impl Into) -> Self { + self.handle.origin = origin.into(); + self + } + pub fn with_value(mut self, value: impl Into) -> Self { self.handle.context.apparent_value = value.into(); self diff --git a/precompiles/src/testing/handle.rs b/precompiles/src/testing/handle.rs index 0228bcca43..7702de1978 100644 --- a/precompiles/src/testing/handle.rs +++ b/precompiles/src/testing/handle.rs @@ -77,6 +77,7 @@ pub type SubcallHandle = Box; /// Mock handle to write tests for precompiles. pub struct MockHandle { + pub origin: H160, pub gas_limit: u64, pub gas_used: u64, pub logs: Vec, @@ -90,6 +91,7 @@ pub struct MockHandle { impl MockHandle { pub fn new(code_address: H160, context: Context) -> Self { Self { + origin: context.caller, gas_limit: u64::MAX, gas_used: 0, logs: vec![], @@ -193,6 +195,10 @@ impl PrecompileHandle for MockHandle { &self.context } + fn origin(&self) -> H160 { + self.origin + } + /// Is the precompile call is done statically. fn is_static(&self) -> bool { self.is_static diff --git a/precompiles/tests-external/lib.rs b/precompiles/tests-external/lib.rs index fd5a18e4a5..81a980c329 100644 --- a/precompiles/tests-external/lib.rs +++ b/precompiles/tests-external/lib.rs @@ -146,7 +146,21 @@ impl MockPrecompile { } } -struct MockPrecompileHandle; +#[derive(Default)] +struct MockPrecompileHandle { + origin: H160, + remaining_gas: u64, +} +impl MockPrecompileHandle { + fn with_origin(mut self, origin: H160) -> Self { + self.origin = origin; + self + } + fn with_remaining_gas(mut self, remaining_gas: u64) -> Self { + self.remaining_gas = remaining_gas; + self + } +} impl PrecompileHandle for MockPrecompileHandle { fn call( &mut self, @@ -176,7 +190,7 @@ impl PrecompileHandle for MockPrecompileHandle { fn refund_external_cost(&mut self, _ref_time: Option, _proof_size: Option) {} fn remaining_gas(&self) -> u64 { - unimplemented!() + self.remaining_gas } fn log(&mut self, _: H160, _: Vec, _: Vec) -> Result<(), evm::ExitError> { @@ -195,6 +209,10 @@ impl PrecompileHandle for MockPrecompileHandle { unimplemented!() } + fn origin(&self) -> H160 { + self.origin + } + fn is_static(&self) -> bool { true } @@ -325,6 +343,25 @@ fn default_checks_revert_when_called_by_contract() { }) } +#[test] +fn default_checks_revert_when_called_by_contract_during_construction() { + ExtBuilder::default().build().execute_with(|| { + // Condition: tx.origin != msg.sender + precompiles() + .prepare_test(Zero, H160::from_low_u64_be(1), PCall::success {}) + .with_origin(Alice) + .with_subcall_handle(|Subcall { .. }| panic!("there should be no subcall")) + .execute_reverts(|r| r == b"Function not callable by smart contracts"); + + // Condition: tx.origin == msg.sender + precompiles() + .prepare_test(Alice, H160::from_low_u64_be(1), PCall::success {}) + .with_origin(Alice) + .with_subcall_handle(|Subcall { .. }| panic!("there should be no subcall")) + .execute_returns(()) + }) +} + #[test] fn default_checks_revert_when_doing_subcall() { ExtBuilder::default().build().execute_with(|| { @@ -385,10 +422,12 @@ fn subcalls_works_when_allowed() { #[test] fn get_address_type_works_for_eoa() { ExtBuilder::default().build().execute_with(|| { - let addr = H160::repeat_byte(0x1d); + let externally_owned_account: H160 = Alice.into(); + let mut handle = MockPrecompileHandle::default().with_origin(externally_owned_account); + assert_eq!( AddressType::EOA, - get_address_type::(&mut MockPrecompileHandle, addr).expect("OOG") + get_address_type::(&mut handle, externally_owned_account).expect("OOG") ); }) } @@ -396,47 +435,32 @@ fn get_address_type_works_for_eoa() { #[test] fn get_address_type_works_for_precompile() { ExtBuilder::default().build().execute_with(|| { - let addr = H160::repeat_byte(0x1d); - pallet_evm::AccountCodes::::insert(addr, vec![0x60, 0x00, 0x60, 0x00, 0xfd]); - assert_eq!( - AddressType::Precompile, - get_address_type::(&mut MockPrecompileHandle, addr).expect("OOG") - ); + let precompiles: Vec = Precompiles::::used_addresses_h160().collect(); + // We expect 4 precompiles + assert_eq!(precompiles.len(), 4); + + let mut handle = MockPrecompileHandle::default(); + precompiles.iter().cloned().for_each(|precompile| { + assert_eq!( + AddressType::Precompile, + get_address_type::(&mut handle, precompile).expect("OOG") + ); + }); }) } #[test] fn get_address_type_works_for_smart_contract() { ExtBuilder::default().build().execute_with(|| { - let addr = H160::repeat_byte(0x1d); - - // length > 5 - pallet_evm::AccountCodes::::insert( - addr, - vec![0x60, 0x00, 0x60, 0x00, 0xfd, 0xff, 0xff], - ); - assert_eq!( - AddressType::Contract, - get_address_type::(&mut MockPrecompileHandle, addr).expect("OOG") - ); + let externally_owned_account: H160 = Alice.into(); + let other_address = H160::repeat_byte(0x1d); + let mut handle = MockPrecompileHandle::default() + .with_origin(externally_owned_account) + .with_remaining_gas(100); - // length < 5 - pallet_evm::AccountCodes::::insert(addr, vec![0x60, 0x00, 0x60]); assert_eq!( AddressType::Contract, - get_address_type::(&mut MockPrecompileHandle, addr).expect("OOG") - ); - }) -} - -#[test] -fn get_address_type_works_for_unknown() { - ExtBuilder::default().build().execute_with(|| { - let addr = H160::repeat_byte(0x1d); - pallet_evm::AccountCodes::::insert(addr, vec![0x11, 0x00, 0x60, 0x00, 0xfd]); - assert_eq!( - AddressType::Unknown, - get_address_type::(&mut MockPrecompileHandle, addr).expect("OOG") + get_address_type::(&mut handle, other_address).expect("OOG") ); }) }