Fix eip2929 implementation

rust-ethereum / evm

Pure Rust implementation of Ethereum Virtual Machine

Apache License 2.0

1.16k stars 355 forks source link

Fix eip2929 implementation #280

Closed RomarQ closed 3 months ago

RomarQ commented 4 months ago

Includes a fix related to EIP-2929, where some addresses were incorrectly considered cold and consumed extra gas. I identified this issue when running the tests located at ethtests/GeneralStateTests/Cancun/stEIP1153-transientStorage, which were failing because the gas costs did not match.

RomarQ commented 3 months ago

@sorpaas can you have a look at these changes?

I splitted these changes from the #278 to make the review easier.

koushiro commented 3 months ago

@RomarQ could you split this PR into three PRs?

PR1: fix eip1559
PR2: fix eip2929 (and update jsontests in CI)
PR3: add cancun config

koushiro commented 3 months ago

Could you add ethtests/GeneralStateTests/Cancun/stEIP1153-transientStorage into jsontests in CI

RomarQ commented 3 months ago

@RomarQ could you split this PR into three PRs?

PR1: fix eip1559

PR2: fix eip2929 (and update jsontests in CI)

PR3: add cancun config

Done

[x] https://github.com/rust-ethereum/evm/pull/289: fix eip1559
[x] https://github.com/rust-ethereum/evm/pull/280: fix eip2929 (and update jsontests in CI)
[x] https://github.com/rust-ethereum/evm/pull/288: add cancun config

RomarQ commented 3 months ago

Could you add ethtests/GeneralStateTests/Cancun/stEIP1153-transientStorage into jsontests in CI

This depends on https://github.com/rust-ethereum/evm/pull/288.

We can test it this way:

cargo run --release --verbose -p jsontests -- jsontests/res/ethtests/GeneralStateTests/Cancun/stEIP1153-transientStorage --debug

RomarQ commented 3 months ago

Once we enable tests for Cancun fork here: https://github.com/rust-ethereum/evm/blob/8168ca150abce577a4fb78d2da0918d229939b9e/jsontests/src/run.rs#L106

if we run all the tests, some will fail, mainly because there are still a few incompatibilities after berlin (gas related). I only fixed a few, there are more.

koushiro commented 3 months ago

Once we enable tests for Cancun fork here:

https://github.com/rust-ethereum/evm/blob/8168ca150abce577a4fb78d2da0918d229939b9e/jsontests/src/run.rs#L106

if we run all the tests, some will fail, mainly because there are still a few incompatibilities after berlin (gas related). I only fixed a few, there are more.

We could fix those incompatibilities in future PRs, let's merge this PR first