Skip to content

bump evm + remove EIP-3607 extension for precompiles #1032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nanocryk wants to merge 3 commits into from
Closed

bump evm + remove EIP-3607 extension for precompiles #1032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6ee59c
bump evm + remove EIP-3607 extension for precompiles
nanocryk Mar 30, 2023
62c4e83
fmt
nanocryk Mar 30, 2023
5915143
remove test for precompile
nanocryk Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions frame/evm/src/runner/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use evm::{
executor::stack::{Accessed, StackExecutor, StackState as StackStateT, StackSubstateMetadata},
ExitError, ExitReason, Transfer,
};
use fp_evm::{CallInfo, CreateInfo, ExecutionInfo, Log, PrecompileSet, Vicinity};
use fp_evm::{CallInfo, CreateInfo, ExecutionInfo, Log, Vicinity};
use frame_support::traits::{Currency, ExistenceRequirement, Get};
use sp_core::{H160, H256, U256};
use sp_runtime::traits::UniqueSaturatedInto;
Expand Down Expand Up @@ -136,14 +136,7 @@ where
//
// 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
&& (!<AccountCodes<T>>::get(source).is_empty() || precompiles.is_precompile(source))
{
if is_transactional && !<AccountCodes<T>>::get(source).is_empty() {
Copy link
Copy Markdown
Contributor Author

@nanocryk nanocryk Mar 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_precompile now takes an extra argument being the remaining gas, and if not OOG will return the bool and the cost of performing that check. This function is used in only 2 places in frontier and evm:

  • evm: inside is_cold, which occurs when we interact with a smart contract (getting code, doing a subcall)
  • frontier (where this comment is): to know if the from address is the one of a precompile. EIP-3607 forbids from to be the address of a smart contract, and we extended it in frontier so that it cannot be the one of a precompile. The precompile part is NOT part of the EIP.

Now it is required to take into account the cost of calling is_precompile, which is less trivial than the change in the EVM. In this frontier code it is a non trivial change, I thus suggest we get rid of that part to keep things simple (we could maybe deduct the cost from the gas limit or whatever, but it may have unwanted side effects).

return Err(RunnerError {
error: Error::<T>::TransactionMustComeFromEOA,
weight,
Expand Down
46 changes: 0 additions & 46 deletions frame/evm/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,49 +605,3 @@ fn eip3607_transaction_from_contract() {
.is_ok());
});
}

#[test]
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(),
H160::from_str("1000000000000000000000000000000000000001").unwrap(),
Vec::new(),
U256::from(1u32),
1000000,
None,
None,
None,
Vec::new(),
true, // transactional
false, // not sure be validated
&<Test as Config>::config().clone(),
) {
Err(RunnerError {
error: Error::TransactionMustComeFromEOA,
..
}) => (),
_ => 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());
});
}