Support EIP-2929: Gas cost increases for state access opcodes #571
Conversation
yperbasis
force-pushed
the
eip-2929
branch
2 times, most recently
from
9424768 to
7a1b2f3
Compare
include/evmc/mocked_host.hpp
Outdated
| @@ -156,6 +156,15 @@ class MockedHost : public Host | |||
| return accounts.count(addr) != 0; | |||
| } | |||
|
|
|||
| /// Access an account (EIP-2929). | |||
| evmc_access_status access_account(const address&) noexcept override { return EVMC_COLD_ACCESS; } | |||
There was a problem hiding this comment.
For later: we should implement some basic map of access to also return "warm" status. This will help with code coverage in evmone.
chfast
force-pushed
the
eip-2929
branch
3 times, most recently
from
74daa4b to
5304359
Compare
| EXPECT_EQ(b[OP_CALLCODE].gas_cost, 100); | ||
| EXPECT_EQ(b[OP_DELEGATECALL].gas_cost, 100); | ||
| EXPECT_EQ(b[OP_STATICCALL].gas_cost, 100); | ||
| EXPECT_EQ(b[OP_SLOAD].gas_cost, 100); | ||
| } |
There was a problem hiding this comment.
Would also add selfdestruct/sstore to this list. They are special cases.
There was a problem hiding this comment.
SELFDESTRUCT doesn't charge WARM_STORAGE_READ_COST (EIP-2929).
SSTORE had complicated pricing logic even prior to EIP-2929; I decided not to change the previous decision to assign 0 gas cost to it in evmc.
There was a problem hiding this comment.
Fine to add these though for clarity (i.e. to document what you have just written).
include/evmc/evmc.h
Outdated
| /** Access account callback function (EIP-2929). */ | ||
| evmc_access_account_fn access_account; | ||
|
|
||
| /** Access storage callback function (EIP-2929). */ |
There was a problem hiding this comment.
Do we really need to state EIP-2929 in every place? I am fine with it for now, but perhaps in the long term we don't need it?
| * @param address The address of the account. | ||
| * @return 0 if cold access, 1 if warm access. | ||
| */ | ||
| int accessAccount(byte[] address); |
There was a problem hiding this comment.
Could use boolean if it is a bool. accountExists does that.
There was a problem hiding this comment.
I used enum for clarity. It's not immediately obvious whether true means cold access or warm access.
There was a problem hiding this comment.
In C yes, but in Java it seems to be only a number :)
There was a problem hiding this comment.
We can leave a todo here to change this to enum or bool.
| @@ -126,6 +126,8 @@ mod tests { | |||
|
|
|||
| let host = ::evmc_sys::evmc_host_interface { | |||
| account_exists: None, | |||
| access_account: None, | |||
There was a problem hiding this comment.
It just occurred to me account_exists should be after the accessing ones if we consider some lexical ordering?
There was a problem hiding this comment.
Well, the ordering here is not lexical, but loosely logical. In
account_exists
access_account
access_storage
get_storage
...
access_account & access_storage are together between account and storage operations.
There was a problem hiding this comment.
I would vote for lexical ordering.
There was a problem hiding this comment.
I'm also not sure what is the point of bringing up "lexical" ordering. For logical ordering, they more belong to the transaction context, not state access (although name indicates otherwise). For me you can put them last after emit_log.
There was a problem hiding this comment.
The other entries seemed to be lexically ordered, in most cases. I don't really care, it was just a remark.
There was a problem hiding this comment.
I didn't notice that others are not ordered, then I don't care.
There was a problem hiding this comment.
Still putting them last for historical and weak ABI stability seems a good option. I can do that in the final rebase.
chfast
force-pushed
the
eip-2929
branch
7 times, most recently
from
addb263 to
d4f426a
Compare
Create new instruction metrics/costs table for Berlin EVM revision. Use warm access costs from EIP-2929 "Gas cost increases for state access opcodes": https://eips.ethereum.org/EIPS/eip-2929.
Add methods access_account() and access_storage() to evmc_host_interface. These are to support EIP-2929 "Gas cost increases for state access opcodes" https://eips.ethereum.org/EIPS/eip-2929. This also defines EVMC_ACCESS_{WARM,COLD} constants for access status representation. Adjustments to Go/Java/Rust bindings are to be done in consecutive commits. Co-authored-by: Paweł Bylica <chfast@gmail.com>
Implement Java bindings for access_account() and access_storage() Host methods.
Implement Go bindings for access_account() and access_storage() Host methods.
Implement Rust bindings for access_account() and access_storage() Host methods.
ethereum/evmone#289 & erigontech/silkworm#161 provide an example of EIP-2929 implementation.