state: Avoid redundant lookups and allocations#1584
Conversation
- Skip the initial state storage query for accounts without initial storage (e.g. newly created contracts): the value is guaranteed to be zero. A constructor storing N slots previously made N pointless StateView queries. - Bind the sender account reference once in execute_message() instead of three get() lookups per value-transferring call. - Look up the 0x03 quirk account on the revert path only for revisions where the quirk applies (Spurious Dragon and later). - Return early from access_account() for precompiles: they are always warm (EIP-2929) and no state entry is needed for them. The access list warming loop in transition() must now materialize the account itself before warming storage keys, as the access list may name a precompile together with storage keys (covered by a new test). As a side effect untouched precompile addresses no longer appear in the state diff as no-op modified/deleted entries. - Build the EIP-7702 delegation designation in a stack buffer instead of concatenating temporary heap allocations per authorization. - Reserve StateDiff::modified_accounts, pass bytes_view by value.
|
The execution-spec-tests / precompiles-gmp / precompiles-libsecp256k1 failures were a real bug in the first push: the access-list warming loop in Fixed by materializing the account in the warming loop when storage keys are present, and added a unit test ( |
chfast
left a comment
chfast
left a comment
There was a problem hiding this comment.
Generally these look good (except for inline comments), but please submit them as separate PRs (you can reuse this PR for one of them).
| const auto is_03_touched = acc_03 != nullptr && acc_03->erase_if_empty; | ||
| // The 0x03 quirk applies only from Spurious Dragon, don't look the account up earlier. | ||
| bool is_03_touched = false; | ||
| if (m_rev >= EVMC_SPURIOUS_DRAGON) |
There was a problem hiding this comment.
This is not worth it because we mostly care about the performance in the recent forks. However, you can check if it is enough to apply the quirk only in the single fork (where it happen on Mainnet).
| if (missing) | ||
| // Query the initial state only if the account may have storage there; | ||
| // otherwise the initial value is guaranteed to be zero (the default). | ||
| if (missing && acc.has_initial_storage) |
There was a problem hiding this comment.
This looks like edge case which in unlikely in practice and we don't use .has_initial_storage as it is only needed for CREATE collisions (another edge case).
2 participants
A collection of small optimizations on the state transition hot paths:
StateView::get_storage()queries.execute_message()instead of threeget()lookups per value-transferring call (references tounordered_mapelements are stable, inserting the recipient doesn't invalidate them).access_account()for precompiles: they are always warm (EIP-2929) and no state entry is needed for them. Note a small observable difference: untouched precompile addresses no longer appear in the state diff as no-op modified/deleted entries (the resulting post-state is identical).StateDiff::modified_accounts, passbytes_viewby value.