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): reset from fork to another fork #8768

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Conversation

yash-atreya
Copy link
Member

@yash-atreya yash-atreya commented Aug 29, 2024

Motivation

Fixes #8684

Resetting from an existing fork to another fork would not update underlying ForkedDatabase, resulting in an invalid state for the new fork.

Solution

reset the underlying the ForkedDatabase using node_config.setup_fork_db_and_config

@yash-atreya yash-atreya changed the title fix(anvil): reset from fork to another fork fix(anvil): reset from fork to another fork Aug 29, 2024
@yash-atreya yash-atreya enabled auto-merge (squash) August 29, 2024 11:52
// reset storage
*self.blockchain.storage.write() = BlockchainStorage::forked(
fork.block_number(),
fork.block_hash(),
fork.total_difficulty(),
);
self.states.write().clear();
self.db.write().await.clear();
// self.db.write().await.clear();
Copy link
Member

Choose a reason for hiding this comment

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

why do we no longer need this?

Copy link
Member Author

@yash-atreya yash-atreya Sep 6, 2024

Choose a reason for hiding this comment

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

Correct me if I'm wrong but we no longer need this because when we reset to a new forked state from a non-fork/old-fork state, we use self.db(Backend.db) as an intermediate step to set the database in ClientFork, here and here.

So even when we clear - self.db.write().await.clear();, RPC calls work because they refer to the database in the fork + foundry_fork_db::SharedBackend.

@grandizzy
Copy link
Collaborator

hey @yash-atreya, wonder if you got the chance to test it with arbitrum forks too, there's a special case with block L1/L2 Not sure if it needs to be addressed here too but think it worth flagging

// If Arbitrum, apply chain specifics to converted block.
if let Ok(
NamedChain::Arbitrum |
NamedChain::ArbitrumGoerli |
NamedChain::ArbitrumNova |
NamedChain::ArbitrumTestnet,
) = NamedChain::try_from(self.env.read().env.cfg.chain_id)
{
// Block number is the best number.
block.header.number = Some(self.best_number());
// Set `l1BlockNumber` field.
block.other.insert("l1BlockNumber".to_string(), number.into());
}

@yash-atreya yash-atreya merged commit 872e2f3 into master Sep 6, 2024
20 checks passed
@yash-atreya yash-atreya deleted the yash/fix-8684 branch September 6, 2024 15:06
github-merge-queue bot pushed a commit to hyperlane-xyz/hyperlane-monorepo that referenced this pull request Sep 9, 2024
fix: temporarily disable dry-run in cli-advanced-e2e
- until we fix the problem with our call to `resetFork` after a dry-run
deployment
- our CI uses the latest nightly foundry releases, and this recent PR
foundry-rs/foundry#8768 has broken the fork
reset. Ironically, this PR actually is a bugfix for something that was
broken that we relied on.

Examples of failures:
-
https://github.com/hyperlane-xyz/hyperlane-monorepo/actions/runs/10772795435/job/29873480020?pr=4441#step:12:133
-
https://github.com/hyperlane-xyz/hyperlane-monorepo/actions/runs/10751285630/job/29818541086?pr=4448

Note that the `pi_with_core_chain` tests don't fail because they do not
do a dry run
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.

bug(anvil): anvil_reset seems to not reset the addresses contract code
3 participants