Skip to content

STR-1308: EVM state diff(snapshot) prototype #800

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

Merged
merged 21 commits into from
May 21, 2025

Conversation

evgenyzdanovich
Copy link
Contributor

@evgenyzdanovich evgenyzdanovich commented May 5, 2025

Description

A prototype impl of EVM DA using state "snapshot" (without compression).

What's meant by "snapshot" is: current impl of EVM DA does not use diffs, but rather sets a new state every time (without diffing currently).

The impl also:

  • defines broad structs and mechanism for capturing state related pieces.
  • contains a tested implementation of state reconstruction based only on batch snapshots (diffs in the future).

P.S. Once basic DA diff library is settled, an additional lift to use diffs should be somewhat minimal.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added (where necessary) tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@evgenyzdanovich evgenyzdanovich requested review from a team as code owners May 5, 2025 20:43
@evgenyzdanovich evgenyzdanovich changed the title Str-1308: EE state diff prototype STR-1308: EE state diff prototype May 5, 2025
Copy link

codecov bot commented May 5, 2025

Codecov Report

Attention: Patch coverage is 60.69652% with 237 lines in your changes missing coverage. Please review.

Project coverage is 52.43%. Comparing base (34479ee) to head (1edbb1d).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crates/reth/statediff/src/state.rs 0.00% 90 Missing ⚠️
crates/reth/exex/src/state_diff_exex.rs 0.00% 39 Missing ⚠️
crates/reth/rpc/src/rpc.rs 0.00% 26 Missing ⚠️
crates/reth/chainspec/src/lib.rs 0.00% 22 Missing ⚠️
crates/reth/statediff/src/account.rs 79.61% 21 Missing ⚠️
bin/alpen-reth/src/main.rs 0.00% 19 Missing ⚠️
crates/reth/statediff/src/block.rs 0.00% 10 Missing ⚠️
crates/reth/db/src/rocksdb/db.rs 90.47% 8 Missing ⚠️
crates/reth/statediff/src/lib.rs 99.04% 2 Missing ⚠️
@@            Coverage Diff             @@
##             main     #800      +/-   ##
==========================================
+ Coverage   52.23%   52.43%   +0.20%     
==========================================
  Files         298      304       +6     
  Lines       32747    33319     +572     
==========================================
+ Hits        17106    17472     +366     
- Misses      15641    15847     +206     
Files with missing lines Coverage Δ
bin/datatool/src/util.rs 0.00% <ø> (ø)
crates/mpt/src/lib.rs 70.70% <ø> (ø)
crates/proof-impl/evm-ee-stf/src/db.rs 48.18% <ø> (ø)
crates/proof-impl/evm-ee-stf/src/lib.rs 98.79% <ø> (ø)
crates/proof-impl/evm-ee-stf/src/primitives.rs 100.00% <ø> (ø)
crates/proof-impl/evm-ee-stf/src/processor.rs 62.50% <ø> (ø)
crates/reth/exex/src/prover_exex.rs 0.00% <ø> (ø)
crates/reth/statediff/src/lib.rs 99.04% <99.04%> (ø)
crates/reth/db/src/rocksdb/db.rs 91.30% <90.47%> (-0.70%) ⬇️
crates/reth/statediff/src/block.rs 0.00% <0.00%> (ø)
... and 6 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

github-actions bot commented May 5, 2025

Commit: 38ba260

SP1 Execution Results

program cycles success
Bitcoin Blockspace 71,660
EVM EE STF 277,604
CL STF 167,016
Checkpoint 4,252

Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

I'm not sure what from this design comes out of how the exex API is designed. Including tracking in the BatchStateDiff structure feels undesirable, see comments

I know this is a prototype and that's why we're using serde for it, but that seems like a huge overhead. At least adding #[serde(rename = ...)] to fields to make them encode as single chars would slim this down and skipping serializing entries in AccountInfo if they're unchanged.

Should we also include in the prototype a checker/applicator that applies the diff on top of a previous state and ensures it matches a final state? It would be nice to have that since that's how I imagine the flow in the proof would be more like, instead of recomputing the diff while checking the blocks and checking it matches a hash passed.

@MdTeach MdTeach self-requested a review May 8, 2025 06:50
@evgenyzdanovich
Copy link
Contributor Author

@delbonis @MdTeach added some logic to reconstruct the state root based off of the batch diffs.
Please note - the logic is very raw and not yet tested, but feel free to take a look and leave suggestions.

I'm working on proper testing of reconstructing logic - both unit and esp fntests.

@evgenyzdanovich evgenyzdanovich force-pushed the str-1308-ee-state-diff-prototype branch from d5e9a14 to aef42c7 Compare May 14, 2025 16:52
@evgenyzdanovich evgenyzdanovich requested a review from a team as a code owner May 14, 2025 16:52
@evgenyzdanovich evgenyzdanovich requested a review from delbonis May 14, 2025 16:52
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

Somewhat more minor changes.

I am curious about how this will lead into DA based sync? Do we have an alternate set of processing stages for that? I'm not sure what infra there is.

@evgenyzdanovich evgenyzdanovich force-pushed the str-1308-ee-state-diff-prototype branch from df53270 to 2f573bf Compare May 18, 2025 16:51
@evgenyzdanovich evgenyzdanovich requested a review from a team as a code owner May 18, 2025 16:51
@evgenyzdanovich
Copy link
Contributor Author

I am curious about how this will lead into DA based sync? Do we have an alternate set of processing stages for that? I'm not sure what infra there is.

@delbonis I have taken a deep look at it, but it seems we gotta build a set of processing stages (given that we have plans to allow DA based sync)

Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

I think all the major issues I looked at earlier have been dealt with so I'm approving this now, but I'm not sure what the next steps are and if those maybe make sense to do here and now. Do you have an idea of what else is needed here?

@delbonis
Copy link
Contributor

Like obviously sync is a next step but that feels like we'd want it to be a different PR. And/or making a more advanced RPC control interface.

Copy link
Contributor

@bewakes bewakes left a comment

Choose a reason for hiding this comment

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

Just got a chance to look closely into this today. Seems like a lot of things are already in place. Great work!

@evgenyzdanovich
Copy link
Contributor Author

Do you have an idea of what else is needed here?

@delbonis
I think this piece of work is self-contained and finished enough, so it can be merged.

Next steps:

  • Once Add diff library #807 is complete, I'll adjust this piece of work to be DIFFS (right now state is not quite diffing, it's rather setting new value everytime).
  • DA based Sync (@barakshani and I converge on the fact it shouldn't be a top priority at the moment).

Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

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

ACK 1edbb1d

@storopoli storopoli enabled auto-merge May 20, 2025 13:54
@barakshani
Copy link
Contributor

LGTM!

@evgenyzdanovich evgenyzdanovich changed the title STR-1308: EE state diff prototype STR-1308: EE state diff(snapshot) prototype May 20, 2025
@evgenyzdanovich evgenyzdanovich changed the title STR-1308: EE state diff(snapshot) prototype STR-1308: EVM state diff(snapshot) prototype May 20, 2025
@storopoli storopoli added this pull request to the merge queue May 21, 2025
Merged via the queue into main with commit 5afc95b May 21, 2025
18 checks passed
@storopoli storopoli deleted the str-1308-ee-state-diff-prototype branch May 21, 2025 02:20
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.

6 participants