Skip to content

Commit

Permalink
Fix check of EIP-3607 transaction for internal calls (#1018)
Browse files Browse the repository at this point in the history
* 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.

* fix eip-3607 tests

* apply review suggtions
  • Loading branch information
koushiro authored Mar 15, 2023
1 parent 5af12e9 commit 8a5492f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 7 deletions.
8 changes: 7 additions & 1 deletion frame/evm/src/runner/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,20 @@ where
>,
) -> (ExitReason, R),
{
// 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.
//
// 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 !<AccountCodes<T>>::get(source).is_empty() || precompiles.is_precompile(source) {
if is_transactional
&& (!<AccountCodes<T>>::get(source).is_empty() || precompiles.is_precompile(source))
{
return Err(RunnerError {
error: Error::<T>::TransactionMustComeFromEOA,
weight,
Expand Down
50 changes: 44 additions & 6 deletions frame/evm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Test as Config>::Runner::call(
// Contract address.
H160::from_str("1000000000000000000000000000000000000001").unwrap(),
Expand All @@ -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
&<Test as Config>::config().clone(),
) {
Err(RunnerError {
Expand All @@ -582,12 +583,31 @@ fn eip3607_transaction_from_contract_should_fail() {
}) => (),
_ => panic!("Should have failed"),
}

// internal call
assert!(<Test as Config>::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
&<Test as Config>::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 <Test as Config>::Runner::call(
// Precompile address.
H160::from_str("0000000000000000000000000000000000000001").unwrap(),
Expand All @@ -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
&<Test as Config>::config().clone(),
) {
Err(RunnerError {
Expand All @@ -609,5 +629,23 @@ fn eip3607_transaction_from_precompile_should_fail() {
}) => (),
_ => panic!("Should have failed"),
}

// internal call
assert!(<Test as Config>::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
&<Test as Config>::config().clone(),
)
.is_ok());
});
}

0 comments on commit 8a5492f

Please sign in to comment.