Skip to content

test(l2): add state reconstruction test to the CI#2255

Merged
jrchatruc merged 47 commits into
mainfrom
state-reconstruct-ci
Mar 26, 2025
Merged

test(l2): add state reconstruction test to the CI#2255
jrchatruc merged 47 commits into
mainfrom
state-reconstruct-ci

Conversation

@ManuelBilbao

Copy link
Copy Markdown
Contributor

Motivation

We want to check that the state diff reconstruction doesn't break on PRs.

Description

Added some tests that reconstruct the state from 3 blobs, which include balance and nonce diffs, and an ERC20 contract "deployment" with balance diffs.

@ManuelBilbao ManuelBilbao force-pushed the state-reconstruct-ci branch from e1f2cad to 610fc21 Compare March 20, 2025 14:46
@ManuelBilbao ManuelBilbao marked this pull request as ready for review March 20, 2025 15:16
@ManuelBilbao ManuelBilbao requested a review from a team as a code owner March 20, 2025 15:16

@fborello-lambda fborello-lambda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing! Left one minor comment

Comment thread crates/l2/Makefile
Comment on lines +182 to +188
rm -rf $$PWD/store
cargo run --release --manifest-path ../../cmd/ethrex_l2/Cargo.toml --bin ethrex_l2 -- config create ci --default
cargo run --release --manifest-path ../../cmd/ethrex_l2/Cargo.toml --bin ethrex_l2 -- config set ci
cargo run --release --manifest-path ../../cmd/ethrex_l2/Cargo.toml --bin ethrex_l2 -- stack reconstruct -g ../../test_data/genesis-l2.json -b ../../test_data/blobs/ -s $$PWD/store -c 0x0007a881CD95B1484fca47615B64803dad620C8d
cargo b --manifest-path ../../Cargo.toml --release
make init-l2 ethrex_L2_DEV_LIBMDBX=$$PWD/store 2>&1 >/dev/null &
cargo test state_reconstruct --release

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've tested it out locally, but i had to wait for the l2 to start.
I've used a sleep after the make init-l2 which is not ideal.
In this case, i wouldn't init the metrics. I would comment the init-metrics target:

init-l2: # init-metrics ## 🚀 Initializes an L2 Lambda ethrex Client

I would also add a target to run the state_reconstruction test locally, just copying that part.

What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When i've tested it locally i had another chain, which produced some conflicts, i thought it was a timing issue.

However, i would remove the init-metrics target when using init-l2 anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done d78f96b

Comment thread crates/l2/Makefile Outdated
Co-authored-by: Federico Borello <156438142+fborello-lambda@users.noreply.github.com>
Comment thread crates/l2/Makefile
Comment thread crates/l2/Makefile
Comment thread crates/networking/rpc/clients/eth/mod.rs Outdated
Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
Base automatically changed from state-reconstruct to main March 21, 2025 20:53
@jrchatruc jrchatruc enabled auto-merge March 25, 2025 22:23
@jrchatruc jrchatruc added this pull request to the merge queue Mar 26, 2025
Merged via the queue into main with commit d1faf8b Mar 26, 2025
@jrchatruc jrchatruc deleted the state-reconstruct-ci branch March 26, 2025 15:22
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->
We want to check that the state diff reconstruction doesn't break on
PRs.

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
Added some tests that reconstruct the state from 3 blobs, which include
balance and nonce diffs, and an ERC20 contract "deployment" with balance
diffs.

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

---------

Co-authored-by: Federico Borello <156438142+fborello-lambda@users.noreply.github.com>
Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
Co-authored-by: Javier Chatruc <jrchatruc@gmail.com>
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