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 EVM bytecode to add the system set update op #8596

Merged
merged 5 commits into from
Jun 9, 2024

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented May 25, 2024

TODO

@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-core labels May 25, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented May 25, 2024

✅ All reviewers have approved.

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

there is no function dispatcher, which I understand is a solidity-level concept, but that means that a code written in solidity will either have to a) use a low-level call function written in yul, or b) this contract will have to ignore the first 4 bytes of the calldata, just in case they are added by a solidity call.

Also, this effectively says that unless you're the system contract, you're going a get. I would rather do it the right way, but I would check with matt how the 4788 contract does it.

@g11tech
Copy link
Contributor Author

g11tech commented May 25, 2024

Also, this effectively says that unless you're the system contract, you're going a get. I would rather do it the right way, but I would check with matt how the 4788 contract does it.

this is exactly how 4788 does it
image

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

i added a comment about not sanitizing the input of set but apart from that, I think this is good to merge.

EIPS/eip-2935.md Show resolved Hide resolved

Corresponding bytecode:
`60203611603157600143035f35116029575f356120000143116029576120005f3506545f5260205ff35b5f5f5260205ff35b5f5ffd00`
Similar to [EIP-4788](./eip-4788.md) above contract provides a `set` mechanism which clients can choose to update the block's parent hash in the ring buffer instead of direct state update. For this clients should call this contract with `SYSTEM_ADDRESS` as caller and provide `32` bytes parent block hash as input.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Similar to [EIP-4788](./eip-4788.md) above contract provides a `set` mechanism which clients can choose to update the block's parent hash in the ring buffer instead of direct state update. For this clients should call this contract with `SYSTEM_ADDRESS` as caller and provide `32` bytes parent block hash as input.
Similar to [EIP-4788](./eip-4788.md) above contract provides a `set` mechanism which clients can choose to update the block's parent hash in the ring buffer instead of direct state update. For this clients should call this contract with `SYSTEM_ADDRESS` as caller and provide `32` bytes parent block hash as input.
Note that the inputs to the `set` method are not sanitized and it is the caller's responsibility to ensure that the ring buffer isn't overrun. This choice was made to support a change in the ring buffer size without having to redeploy a new contract.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will also need to change the get method as well. but there is another consideration:

the contracts that would directly call the contract would break if we modify ring buffer length and expcted them to mod the input in a new way

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 9fcafab into ethereum:master Jun 9, 2024
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants