Skip to content

Conversation

@JereSalo
Copy link
Contributor

@JereSalo JereSalo commented Dec 22, 2025

Motivation

  • Main goal was to -> Fix problems with ethrex-replay when asking for a witness to non-ethrex nodes.

Now, given that it turns out to be a vulnerability for the L2 we will change the approach, it's more important to guarantee safety in the Guest Program than to replay with non-ethrex nodes for now. We will try to achieve both though.

Description

Description of Replay with non-ethrex nodes problem:

When using a non-ethrex node as RPC in ethrex-replay it will happen sometimes that we will request for a node that's not completely necessary for that block's execution, so in this case instead of failing (as we were doing before) we default to an empty value because the node wasn't relevant in the first place.
In all the cases I've seen so far the storage slot didn't exist, but the non-ethrex witness didn't provide the nodes to check it by traversing nibble by nibble until we found 0x80 corresponding to that path. It just simply didn't give us the node.
The fix for replay is to default to empty values if we don't have them but this is not correct for production, so we will try to find a good solution that satisfies both replay and the L2, prioritizing correctness in the L2 first.

Before this PR we were doing for accounts and storage "If you don't find them, default to empty values because they are not important". This works for replay but is a vulnerability in production (e.g. for an Ethrex L2) so we are going to be strict and see if we can be loose with replay only.

@JereSalo JereSalo self-assigned this Dec 22, 2025
@github-actions github-actions bot added L1 Ethereum client L2 Rollup client labels Dec 22, 2025
@github-actions
Copy link

github-actions bot commented Dec 22, 2025

Lines of code report

Total lines added: 0
Total lines removed: 7
Total lines changed: 7

Detailed view
+-------------------------------------------------------+-------+------+
| File                                                  | Lines | Diff |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/block_execution_witness.rs | 343   | -7   |
+-------------------------------------------------------+-------+------+

@JereSalo JereSalo marked this pull request as ready for review December 22, 2025 21:24
@JereSalo JereSalo requested a review from a team as a code owner December 22, 2025 21:24
Copilot AI review requested due to automatic review settings December 22, 2025 21:24
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Dec 22, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes handling of incomplete execution witnesses when querying non-ethrex nodes in ethrex-replay. Instead of failing when witness nodes are missing for accounts or storage slots that aren't critical to block execution, the code now defaults to empty values and logs debug messages to aid in troubleshooting.

Key Changes:

  • Modified error handling in get_account_state and get_storage_slot to return empty values instead of propagating errors when trie nodes are missing
  • Added debug logging to help identify incomplete witnesses when debugging state root mismatches in the L2 Prover

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JereSalo JereSalo changed the title fix(l1,l2): default to empty values in Guest Program if they aren't in the witness fix(l1,l2): make GuestProgramState more strict when information is missing Dec 23, 2025
@JereSalo JereSalo marked this pull request as draft December 23, 2025 20:02
@ethrex-project-sync ethrex-project-sync bot moved this from In Review to In Progress in ethrex_l1 Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client L2 Rollup client

Projects

Status: In Progress
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants