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 cache path during anvil_reset without fork url #9729

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nbaztec
Copy link
Contributor

@nbaztec nbaztec commented Jan 21, 2025

Motivation

The test test_reset_dev_account_nonce would fail on subsequent runs because anvil_reset when run without a fork_url would not update the cache path in SharedBackend and BlockchainDb. As such, after reset, the rpc calls would write to the initial block's cache file.

Solution

Reset the ForkedDatabase correctly even when the fork url is missing.

Note

Unsure why this doesn't fail on the CI, but it would consistently fail locally on Ubuntu 24.04

@grandizzy grandizzy added T-bug Type: bug C-anvil Command: anvil labels Jan 30, 2025
@grandizzy grandizzy changed the title fix: reset cache path during anvil_reset without fork url fix(anvil): reset cache path during anvil_reset without fork url Jan 30, 2025
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.

makes sense, thank you! left couple of nits

@@ -542,6 +542,27 @@ impl Backend {
*self.fork.write() = Some(fork);
*self.env.write() = env;
} else {
// If rpc url is unspecified, then update the fork with the new block number and
Copy link
Collaborator

Choose a reason for hiding this comment

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

this makes sense, can you please externalize this code (which is done also in the case of rpc url specified) in a helper fn / closure and reuse?

if let Some(fork_url) = forking.json_rpc_url {
// Set the fork block number
let mut node_config = self.node_config.write().await;
node_config.fork_choice = Some(ForkChoice::Block(fork_block_number));

async fn get_block_from_cache_path(api: &mut EthApi) -> u64 {
let db = api.backend.get_db().read().await;
let cache_debug = format!("{:?}", db.maybe_inner().unwrap().cache());
let re = regex::Regex::new(r#"JsonBlockCacheDB \{ cache_path: Some\("([^"]+)"\)"#).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we simplify this by starting node with cache_path config?

/// Sets the path where states are cached
#[must_use]
pub fn with_cache_path(mut self, cache_path: Option<PathBuf>) -> Self {
self.cache_path = cache_path;
self
}

Copy link
Contributor Author

@nbaztec nbaztec Jan 30, 2025

Choose a reason for hiding this comment

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

The test actually verifies that the cache path is updated upon reset, the regex exists because cache_path is currently private (so only obtainable via fmt::Debug). If we expose that property we can get rid of the regex and check it straight away

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: No status
Development

Successfully merging this pull request may close these issues.

2 participants