Skip to content

Conversation

@yash-atreya
Copy link
Contributor

@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
Contributor 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!

@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

Development

Successfully merging this pull request may close these issues.

bug(anvil): nonce too low after anvil restarts with state bug(anvil): --fork-url and --load-state are incompatible

4 participants