ethereum / ethereum/execution-specs

Track gas more appropriately prior to state access in specs

Open
#1,867 3 comments 0 reactions 1 assignee Claimed by @fselmo View on GitHub
A-spec-specs C-bug stale
Dominant language
Python
Stars
1.2k
Forks
505
Avg merge
2d 14h
Merged PRs (30d)
116

Description

The specs currently `charge_gas` only after accounting for state access gas costs while accessing state at the same time, without knowing whether or not we have enough gas for the state access (e.g. we calculate access cost [here](https://github.com/ethereum/execution-specs/blob/forks/osaka/src/ethereum/forks/osaka/vm/instructions/system.py#L376-L380), then proceed to read the account within [access_delegation](https://github.com/ethereum/execution-specs/blob/forks/osaka/src/ethereum/forks/osaka/vm/instructions/system.py#L388) and [is_account_alive](https://github.com/ethereum/execution-specs/blob/forks/osaka/src/ethereum/forks/osaka/vm/instructions/system.py#L392), before charging gas [here](https://github.com/ethereum/execution-specs/blob/forks/osaka/src/ethereum/forks/osaka/vm/instructions/system.py#L402) and making sure we had enough to access the account in the first place).

With the implementation for block-level access lists in `eips/amsterdam/eip-7928`, this revealed inconsistencies with how clients behave and what is read at what point across certain gas boundaries. This has been addressed, for the most part, within the logic in the EIP-7928 implementation branch (e.g., we [check_gas here](https://github.com/ethereum/execution-specs/blob/eips/amsterdam/eip-7928/src/ethereum/forks/amsterdam/vm/instructions/system.py#L426-L429) before to make sure we have enough gas for all "static" checks that don't require state access first, before accessing state). The need for these changes revealed a necessity for the specs to be correct across all forks when it comes to static checks / gas accounting prior to any state access.

These changes should separated from the EIP-7928 logic and PR'd to the development branch (likely `forks/amsterdam`) in an effort to agree on the design for this and apply it to all forks. Once the design is agreed upon for the active development banch, we should apply them across all forks and rebase them into EIP-7928, effectively replacing this patching of the logic that is currently there.

---

### Updated TODO:

- [x] Merge the `eips/amsterdam/eip-7928` (includes gas accounting changes) into `forks/amsterdam`
- Apply the proper gas accounting to all forks, using `ethereum/forks/amsterdam` as the "template" (closed by #2903)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.