bluealloy
Ethereum tests failures when upgrading from revm 9 to 14
When upgrading our project from revm 9 to 14, the following tests from the Ethereum suite fail with invalid state root:
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stRevertTest/RevertPrefoundEmptyCallOOG.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stRevertTest/RevertPrefoundEmptyOOG.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stRevertTest/TouchToEmptyAccountRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stSStoreTest/InitCollision.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stStaticCall/static_call_OOG_additionalGasCosts2.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_CALLCODE_ToEmpty_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_CALLCODE_ToOneStorageKey_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_CALL_ToEmpty_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_CALL_ToOneStorageKey_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_DELEGATECALL_ToEmpty_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_DELEGATECALL_ToOneStorageKey_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_SUICIDE_ToEmpty_OOGRevert.json
ethereum_tests/12.2/BlockchainTests/GeneralStateTests/stZeroCallsRevert/ZeroValue_SUICIDE_ToOneStorageKey_OOGRevert.json
This is the function that executes the transactions:
pub(crate) fn transact_commit_with_inspect<DB>(
env: Env,
db: DB,
) -> ExecutionResult
where
DB: Database + DatabaseCommit,
{
let mut evm = revm::Evm::builder()
.with_db(db)
.with_env_with_handler_cfg(EnvWithHandlerCfg::new_with_spec_id(
env.into(),
FORK_SPEC_ID, // Paris fork
))
.append_handler_register(revm::inspector_handle_register)
.build();
evm.transact_commit().unwrap() // error handling omitted
}
revm dependency is declared as:
# Using "9.0" works as expected
revm = { version = "14.0", default-features = false, features = [
"optional_eip3607",
"serde",
] }
Would you happen to have any clue? Is there anything we should look into? It seems that all failing tests are somehow related to a tx revert
Same issue with:
revm = { version = "14.0", default-features = true, features = [
"serde",
"arbitrary",
"asm-keccak",
"dev",
"kzg-rs",
] }
There is EIP (https://eips.ethereum.org/EIPS/eip-7610) that changed how an Account is considered empty, in essence, it requires storage to be empty.
In Revm we don't have that check as it is close to impossible to trigger on mainnet as there are only few empty accounts with storage and you would need to get a hash collision to trigger it.
Statetest are handmade with some storage, there is list in Revm here: https://github.com/bluealloy/revm/blob/e477c7fb578acd55d1f7a95b1a80392d8ede9d22/bins/revme/src/cmd/statetest/runner.rs#L115-L125
@rakita We have investigated more and we can reproduce the error with every REVM version >= 10.
Even by ignoring the InitialCollision.json test as shown in your code, there are still 12 other tests that fail with revm 10 and work as expected with revm 9:
GeneralStateTests/stRevertTest/RevertPrefoundEmptyCallOOG.json
GeneralStateTests/stRevertTest/RevertPrefoundEmptyOOG.json
GeneralStateTests/stRevertTest/TouchToEmptyAccountRevert.json
GeneralStateTests/stSStoreTest/InitCollision.json
GeneralStateTests/stStaticCall/static_call_OOG_additionalGasCosts2.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_CALLCODE_ToEmpty_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_CALLCODE_ToOneStorageKey_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_CALL_ToEmpty_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_CALL_ToOneStorageKey_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_DELEGATECALL_ToEmpty_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_DELEGATECALL_ToOneStorageKey_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_SUICIDE_ToEmpty_OOGRevert.json
GeneralStateTests/stZeroCallsRevert/ZeroValue_SUICIDE_ToOneStorageKey_OOGRevert.json
Any idea?
@rakita Our tests are broken by this commit: https://github.com/bluealloy/revm/commit/9955d9faa5ca4cefc30bde3eb37ba11af7ef474d
After that commit the DatabaseCommit::commit fn is called with a different set of changes. For example, in the case of the BlockchainTests/GeneralStateTests/stRevertTest/RevertPrefoundEmptyOOG.json test, the set of changes before the commit is:
{
0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba: Account {
info: AccountInfo {
balance: 0,
nonce: 0,
code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470,
code: None,
},
storage: {},
status: AccountStatus(
Touched,
),
},
0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b: Account {
info: AccountInfo {
balance: 999990700000,
nonce: 1,
code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470,
code: None,
},
storage: {},
status: AccountStatus(
Touched,
),
},
0xa000000000000000000000000000000000000000: Account {
info: AccountInfo {
balance: 1,
nonce: 0,
code_hash: 0x7b5b62ce77e883bfb4a35d856b28fe6d3d38b2d689920b7f6ff96a1d8df3cbea,
code: Some(
LegacyRaw(
0x602060006000f0600055622fffff60002000,
),
),
},
storage: {},
status: AccountStatus(
0x0,
),
},
}
but after that commit the changes are:
{
0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b: Account {
info: AccountInfo {
balance: 999990700000,
nonce: 1,
code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470,
code: None,
},
storage: {},
status: AccountStatus(
Touched,
),
},
0xa000000000000000000000000000000000000000: Account {
info: AccountInfo {
balance: 1,
nonce: 0,
code_hash: 0x7b5b62ce77e883bfb4a35d856b28fe6d3d38b2d689920b7f6ff96a1d8df3cbea,
code: Some(
LegacyRaw(
0x602060006000f0600055622fffff60002000,
),
),
},
storage: {
0: EvmStorageSlot {
original_value: 0,
present_value: 0,
is_cold: true,
},
},
status: AccountStatus(
0x0,
),
},
0x7db299e0885c85039f56fa504a13dd8ce8a56aa7: Account {
info: AccountInfo {
balance: 0,
nonce: 0,
code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470,
code: None,
},
storage: {},
status: AccountStatus(
Cold,
),
},
0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba: Account {
info: AccountInfo {
balance: 0,
nonce: 0,
code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470,
code: None,
},
storage: {},
status: AccountStatus(
Touched,
),
},
}
Should we ignore everything marked as cold?
Should we ignore everything marked as cold?
This is the solution. It is more performant if I don't remove the item from the hashmap, and it is helpful (a nice feature) to see all loaded accounts.
For info, I am doing a lot of refactoring in the current main branch (PR https://github.com/bluealloy/revm/pull/1865). I am not sure about your timeline, but first version of this change should be made in a next few weeks. This is part of making the Revm an Evm Framework.
Should we ignore everything marked as cold?
This is the solution.
Great! Should we also ignore storage slots with is_cold: true?
Great! Should we also ignore storage slots with is_cold: true?
Delayed answer, but yes. if is_cold is found it means storage or account reverted its access.