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(anvil): set storage.best_number correctly #9215

Merged
merged 8 commits into from
Oct 30, 2024

Conversation

yash-atreya
Copy link
Member

@yash-atreya yash-atreya commented Oct 28, 2024

Motivation

Closes #9053 + Closes #8493

This arises when --fork-url and --load-state are used together.

Solution

  • Introduce account_at in ForkedStorage which stores the state of accounts at specific block numbers.
  • Populate account_at using the provided state file.

Solution is a lot simpler.

  • We are correctly loading the data to the db:

    if !self.db.write().await.load_state(state.clone())? {

  • However, the storage.best_number i.e head of the chain is not correctly set when --load-state is used with --fork-url. We continue to set storage.best_number = state.block.number which is incorrect as the best_number should be fork_block_number.

  • This issue is observed when fork_block_number > state.block_number. In this case while all state data has been correctly loaded to the db, we fetch via the provider due to this check.

    if fork.predates_fork(number) {

  • Setting storage.best_number = fork_block_number fixes this

  • Test

@grandizzy
Copy link
Collaborator

grandizzy commented Oct 29, 2024

@yash-atreya I added a test here, basically dumping state locally from gh issue recreate and reloading / make sure balance properly retrieved, feel free to include in PR if OK

yash/load-forked-state...grandizzy:test-for-9215

yash-atreya and others added 5 commits October 30, 2024 13:34
This reverts commit b650f56.
---------

Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
@yash-atreya
Copy link
Member Author

@yash-atreya I added a test here, basically dumping state locally from gh issue recreate and reloading / make sure balance properly retrieved, feel free to include in PR if OK

yash/load-forked-state...grandizzy:test-for-9215

@grandizzy thanks for this.

I've added a more comprehensive test. ptal.

@yash-atreya yash-atreya changed the title fix(anvil): introduce account_at in ForkedStorage fix(anvil): set storage.best_number correctly Oct 30, 2024
@grandizzy
Copy link
Collaborator

@yash-atreya I suspect this is supposed to close #8493 too? If so, could you please update description to include it? thank you

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

lgtm!

crates/anvil/src/eth/backend/mem/mod.rs Outdated Show resolved Hide resolved
@yash-atreya yash-atreya enabled auto-merge (squash) October 30, 2024 11:26
@yash-atreya yash-atreya merged commit 45d5997 into master Oct 30, 2024
21 checks passed
@yash-atreya yash-atreya deleted the yash/load-forked-state branch October 30, 2024 11:40
rplusq pushed a commit to rplusq/foundry that referenced this pull request Nov 29, 2024
* feat(`anvil`): persist accounts in `ForkedStorage`

* load accounts

* test

* fix(`anvil`): override storage.best_number with fork_block_num if loading state on a fork url

* Revert "load accounts"

This reverts commit b650f56.

* Revert "feat(`anvil`): persist accounts in `ForkedStorage`"

This reverts commit 456da15.

* nit

---------

Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>

* nit

---------

Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
@grandizzy grandizzy added T-bug Type: bug C-anvil Command: anvil labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-anvil Command: anvil T-bug Type: bug
Projects
Status: Completed
4 participants