Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update EIP-2935: update for Prague devnet1 #8577

Merged
merged 4 commits into from
May 17, 2024
Merged

Conversation

s1na
Copy link
Contributor

@s1na s1na commented May 17, 2024

This PR makes the following changes to the EIP:

  • Reverts changes to the BLOCKHASH opcode so it retains its prior behavior. I.e. gas cost of 20 stays and it will not read from the contract state anymore. The idea is we should change the implementation of BLOCKHASH at the transition to Verkle tries.
  • Backfilling logic is removed. It will take roughly 1 day for the contract storage to fill up with 8192 block hashes.
  • EIP-2929 rules clarified w.r.t history contract

@s1na s1na requested a review from eth-bot as a code owner May 17, 2024 14:19
@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-core labels May 17, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented May 17, 2024

✅ All reviewers have approved.

@eth-bot eth-bot added the a-review Waiting on author to review label May 17, 2024
@eth-bot eth-bot changed the title EIP-2935: update for Prague devnet1 Update EIP-2935: update for Prague devnet1 May 17, 2024
@eth-bot eth-bot enabled auto-merge (squash) May 17, 2024 15:33
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 3948040 into ethereum:master May 17, 2024
10 checks passed
Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One post-merge nit, LGTM otherwise


Since now `BLOCKHASH` is served from state, the clients now **additionally** charge the corresponding warm or cold `SLOAD` costs. For verkle based networks this would imply doing and bundling corresponding accesses (and gas charges) of `SLOAD`.
The gas cost of the `BLOCKHASH` opcode is unchanged. Importantly the processing at the beginning of the block, i.e. `process_block_hash_history`, will not warm the `HISTORY_STORAGE_ADDRESS` account or its storage slots as per [EIP-2929](./eip-2929.md) rules. As such the first call to the contract will pay for warming up the account and storage slots it accesses.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the first call to the contract is in tx context, not block context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is your concern talking about the first call in this section at all? Or rather, how would you clarify this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As such the first call to the contract will pay for warming up the account and storage slots it accesses."

It is not super clear that we are talking about the "first call in a tx to the contract" and not "first call in a block to the contract" (it could be misinterpreted). However, anyone with knowledge of EIP2929 will know that this is inside the tx context and not the block context.

So as nit, I would clarify it with "As such the first call within a transaction to the contract"...

somnathb1 pushed a commit to erigontech/erigon that referenced this pull request Jun 26, 2024
somnathb1 pushed a commit to erigontech/erigon that referenced this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-review Waiting on author to review c-update Modifies an existing proposal s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants