From e9c8ed3b152135bc058f8c50b6cb44ef01960ff0 Mon Sep 17 00:00:00 2001 From: koushiro Date: Tue, 14 Mar 2023 15:26:44 +0800 Subject: [PATCH 1/3] Improve check of EIP-3607 Only check the restrictions of EIP-3607 if the source of the EVM operation is from an external transaction. If the source of this EVM operation is from an internal call, like from `eth_call` or `eth_estimateGas` RPC, we will skip the checks for the EIP-3607. --- frame/evm/src/runner/stack.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index be39912a48..f1d474e514 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -130,18 +130,23 @@ where >, ) -> (ExitReason, R), { - // EIP-3607: https://eips.ethereum.org/EIPS/eip-3607 - // Do not allow transactions for which `tx.sender` has any code deployed. - // - // We extend the principle of this EIP to also prevent `tx.sender` to be the address - // of a precompile. While mainnet Ethereum currently only has stateless precompiles, - // projects using Frontier can have stateful precompiles that can manage funds or - // which calls other contracts that expects this precompile address to be trustworthy. - if !>::get(source).is_empty() || precompiles.is_precompile(source) { - return Err(RunnerError { - error: Error::::TransactionMustComeFromEOA, - weight, - }); + // Only check the restrictions of EIP-3607 if the source of the EVM operation is from an external transaction. + // If the source of this EVM operation is from an internal call, like from `eth_call` or `eth_estimateGas` RPC, + // we will skip the checks for the EIP-3607. + if is_transactional { + // EIP-3607: https://eips.ethereum.org/EIPS/eip-3607 + // Do not allow transactions for which `tx.sender` has any code deployed. + // + // We extend the principle of this EIP to also prevent `tx.sender` to be the address + // of a precompile. While mainnet Ethereum currently only has stateless precompiles, + // projects using Frontier can have stateful precompiles that can manage funds or + // which calls other contracts that expects this precompile address to be trustworthy. + if !>::get(source).is_empty() || precompiles.is_precompile(source) { + return Err(RunnerError { + error: Error::::TransactionMustComeFromEOA, + weight, + }); + } } let (total_fee_per_gas, _actual_priority_fee_per_gas) = From a3726566f262815205284b88bd1a44d7e597365b Mon Sep 17 00:00:00 2001 From: koushiro Date: Tue, 14 Mar 2023 17:12:48 +0800 Subject: [PATCH 2/3] fix eip-3607 tests --- frame/evm/src/tests.rs | 50 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 1e8202ea43..b12ff74468 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -559,8 +559,9 @@ fn runner_max_fee_per_gas_gte_max_priority_fee_per_gas() { } #[test] -fn eip3607_transaction_from_contract_should_fail() { +fn eip3607_transaction_from_contract() { new_test_ext().execute_with(|| { + // external transaction match ::Runner::call( // Contract address. H160::from_str("1000000000000000000000000000000000000001").unwrap(), @@ -572,8 +573,8 @@ fn eip3607_transaction_from_contract_should_fail() { None, None, Vec::new(), - false, // non-transactional - true, // must be validated + true, // transactional + false, // not sure be validated &::config().clone(), ) { Err(RunnerError { @@ -582,12 +583,31 @@ fn eip3607_transaction_from_contract_should_fail() { }) => (), _ => panic!("Should have failed"), } + + // internal call + assert!(::Runner::call( + // Contract address. + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + None, + None, + None, + Vec::new(), + false, // non-transactional + true, // must be validated + &::config().clone(), + ) + .is_ok()); }); } #[test] -fn eip3607_transaction_from_precompile_should_fail() { +fn eip3607_transaction_from_precompile() { new_test_ext().execute_with(|| { + // external transaction match ::Runner::call( // Precompile address. H160::from_str("0000000000000000000000000000000000000001").unwrap(), @@ -599,8 +619,8 @@ fn eip3607_transaction_from_precompile_should_fail() { None, None, Vec::new(), - false, // non-transactional - true, // must be validated + true, // transactional + false, // not sure be validated &::config().clone(), ) { Err(RunnerError { @@ -609,5 +629,23 @@ fn eip3607_transaction_from_precompile_should_fail() { }) => (), _ => panic!("Should have failed"), } + + // internal call + assert!(::Runner::call( + // Contract address. + H160::from_str("0000000000000000000000000000000000000001").unwrap(), + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + None, + None, + None, + Vec::new(), + false, // non-transactional + true, // must be validated + &::config().clone(), + ) + .is_ok()); }); } From 5e7305641b59fbac1f40b900f7866ee90ca19512 Mon Sep 17 00:00:00 2001 From: koushiro Date: Tue, 14 Mar 2023 23:42:54 +0800 Subject: [PATCH 3/3] apply review suggtions --- frame/evm/src/runner/stack.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index f1d474e514..3193b1c049 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -133,20 +133,21 @@ where // Only check the restrictions of EIP-3607 if the source of the EVM operation is from an external transaction. // If the source of this EVM operation is from an internal call, like from `eth_call` or `eth_estimateGas` RPC, // we will skip the checks for the EIP-3607. - if is_transactional { - // EIP-3607: https://eips.ethereum.org/EIPS/eip-3607 - // Do not allow transactions for which `tx.sender` has any code deployed. - // - // We extend the principle of this EIP to also prevent `tx.sender` to be the address - // of a precompile. While mainnet Ethereum currently only has stateless precompiles, - // projects using Frontier can have stateful precompiles that can manage funds or - // which calls other contracts that expects this precompile address to be trustworthy. - if !>::get(source).is_empty() || precompiles.is_precompile(source) { - return Err(RunnerError { - error: Error::::TransactionMustComeFromEOA, - weight, - }); - } + // + // EIP-3607: https://eips.ethereum.org/EIPS/eip-3607 + // Do not allow transactions for which `tx.sender` has any code deployed. + // + // We extend the principle of this EIP to also prevent `tx.sender` to be the address + // of a precompile. While mainnet Ethereum currently only has stateless precompiles, + // projects using Frontier can have stateful precompiles that can manage funds or + // which calls other contracts that expects this precompile address to be trustworthy. + if is_transactional + && (!>::get(source).is_empty() || precompiles.is_precompile(source)) + { + return Err(RunnerError { + error: Error::::TransactionMustComeFromEOA, + weight, + }); } let (total_fee_per_gas, _actual_priority_fee_per_gas) =