Skip to content

Fix Sepolia deposit contract issue #2132

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.

Sign up for GitHub

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

Jump to bottom
Closed
mpaulucci opened this issue Mar 5, 2025 · 1 comment · Fixed by #2832
Closed

Fix Sepolia deposit contract issue #2132

mpaulucci opened this issue Mar 5, 2025 · 1 comment · Fixed by #2832
Assignees
@fmoletta
Labels
L1 Ethereum client pectra Pectra (Prague + Electra) hardfork

Comments

@mpaulucci
Copy link
Collaborator

mpaulucci commented Mar 5, 2025 •
edited
Loading

There was an issue when the clients hardforked to Pectra in Sepolia related to the contract address and the logs that are emmited.

This is the change to the EIP: ethereum/EIPs#9453

This is the fix on Geth: ethereum/go-ethereum#31317

This test should pass: ethereum/execution-spec-tests@9084db0#diff-423832c4eebde83349472ade321b211f34bd8659b6a677207da2ae0b78b462e9R22

This is a blog post explaining the issue: https://mariusvanderwijden.github.io/blog/2025/03/08/Sepolia/
@mpaulucci mpaulucci added L1 Ethereum client pectra Pectra (Prague + Electra) hardfork labels Mar 5, 2025
@mpaulucci mpaulucci moved this to Todo in ethrex_l1 May 16, 2025
@mpaulucci
Copy link
Collaborator Author

I think this comment might be related: ethereum/execution-spec-tests#1607 (comment)

@DiegoCivi DiegoCivi assigned DiegoCivi and unassigned DiegoCivi May 16, 2025
@fmoletta fmoletta self-assigned this May 19, 2025
@fmoletta fmoletta mentioned this issue May 19, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue May 19, 2025
@fmoletta
fix(l1): add deposit request layout validations + return invalid depo…
c716b18 
…sit request error (#2832)

**Motivation**
Currently, when we fail to parse a deposit request we simply ignore it
and keep the rest of the deposits, relying on the request hash check
afterwards to notice the missing deposit request. This PR handles the
error earlier and returns the appropriate `InvalidDepositRequest Error`.
This will provide better debugging information and also more accurate
testing via tools such as `execution-spec-tests` which rely on specific
error returns.
We also were not correctly validating the layout according to the
[EIP](https://eips.ethereum.org/EIPS/eip-6110), as we were only checking
the total size and not the size and offset of each request field
<!-- Why does this pull request exist? What are its goals? -->

**Description**
* Check that the full layout of deposit requests is valid (aka the
internal sizes and offsets of the encoded data)
* Handle errors when parsing deposit requests
* Check log topic matches deposit topic before parsing a request as a
deposit request
<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves #111, Resolves #222 -->

Allows us to address review comment made on execution-specs-test PR
ethereum/execution-spec-tests#1607 + also closes
#2132
@github-project-automation github-project-automation bot moved this from Todo to Done in ethrex_l1 May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fmoletta fmoletta

Labels
L1 Ethereum client pectra Pectra (Prague + Electra) hardfork
Projects
ethrex_l1
Archived in project
Milestone
No milestone
3 participants
@mpaulucci @DiegoCivi @fmoletta