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

fix(l2): replace ExecutionDB from_exec() impl to use a CacheDB based approach #1709

Merged
merged 27 commits into from
Jan 24, 2025

Conversation

xqft
Copy link
Contributor

@xqft xqft commented Jan 14, 2025

Motivation

A ExecutionDB is created from a block's pre-execution but the current implementation is incorrect and not all needed data gets stored. The goal is to replace this with another approach, using an auxiliary "caching" database that will retrieve data from a Store or an RPC endpoint (like in #1705) during the pre-execution of a block, finally resulting in a db that contains all the needed execution data.

Description

  • replace ExecutionDB::from_exec() to use the CacheDB, rename it to from_store()
  • some refactors

@xqft xqft marked this pull request as ready for review January 15, 2025 19:58
@xqft xqft requested a review from a team as a code owner January 15, 2025 19:58
Copy link

github-actions bot commented Jan 15, 2025

| File                                                                              | Lines | Diff |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/l2/proposer/prover_server.rs               | 451   | +1   |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/l2/prover/zkvm/interface/risc0/src/main.rs | 44    | +3   |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/l2/prover/zkvm/interface/sp1/src/main.rs   | 45    | +3   |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/l2/prover/zkvm/interface/src/lib.rs        | 164   | +44  |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/vm/db.rs                                   | 142   | +45  |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/vm/errors.rs                               | 119   | +4   |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/vm/execution_db.rs                         | 227   | +43  |
+-----------------------------------------------------------------------------------+-------+------+
| /home/runner/work/ethrex/ethrex/crates/vm/vm.rs                                   | 818   | -1   |
+-----------------------------------------------------------------------------------+-------+------+

Total lines added: +143
Total lines removed: 1
Total lines changed: 144

Copy link
Contributor

@fborello-lambda fborello-lambda left a comment

Choose a reason for hiding this comment

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

LGTM, the prover compiles and the test make perf-risc0 passes.

We have to keep in mind that the blocks used for testing contain just plain transfers.

I think that #1705 will provide more insight and test-cases.

@xqft xqft marked this pull request as draft January 17, 2025 15:06
@xqft xqft changed the title fix(l2): replace ExecutionDB from_exec() with IndexDB fix(l2): replace ExecutionDB from_exec() with CacheDB Jan 20, 2025
@xqft xqft marked this pull request as ready for review January 20, 2025 20:43
@xqft xqft changed the title fix(l2): replace ExecutionDB from_exec() with CacheDB fix(l2): replace ExecutionDB from_exec() impl to use a CacheDB based approach Jan 20, 2025
@xqft xqft added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit 95b653c Jan 24, 2025
23 checks passed
@xqft xqft deleted the l2/touched_state branch January 24, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants