Skip to content

fix(evm): don't gate deleteStateSnapshot on backend init in CowBackend - #16640

Open
gomesalexandre wants to merge 2 commits into
foundry-rs:masterfrom
gomesalexandre:fix_cow_delete_state_snapshot_fuzz
Open

fix(evm): don't gate deleteStateSnapshot on backend init in CowBackend#16640
gomesalexandre wants to merge 2 commits into
foundry-rs:masterfrom
gomesalexandre:fix_cow_delete_state_snapshot_fuzz

Conversation

@gomesalexandre

Copy link
Copy Markdown
Contributor

`vm.deleteStateSnapshot`/`vm.deleteStateSnapshots` silently return `false`/no-op for a snapshot that genuinely exists, whenever the delete is the FIRST cheatcode call of a test/fuzz run - e.g. a snapshot taken in `setUp()`:

function setUp() public { id = vm.snapshotState(); }

function testFuzz_a(uint256 x) public {
    assertTrue(vm.deleteStateSnapshot(id));   // FAILS - returns false
}

function testFuzz_b(uint256 x) public {
    vm.snapshotState();                       // any mutating cheatcode first
    assertTrue(vm.deleteStateSnapshot(id));   // passes - same id, different answer
}

Same operation, same snapshot id, and the answer flips depending on unrelated prior cheatcode history within the run. `vm.deleteStateSnapshots()` (delete-all) is `void`-returning, so its no-op failure mode was completely silent - no assertion could ever catch it.

Per the spec (`crates/cheatcodes/spec/src/vm.rs`), `deleteStateSnapshot` should return `false` only if the snapshot does not exist - it does exist here.

Root cause

`CowBackend::delete_state_snapshot` (`crates/evm/core/src/backend/cow.rs`) gated backend access via `initialized_backend_mut()`, which returned `None` whenever `pending_init` was still `Some` - i.e. before ANY mutating cheatcode had run in the current call. But `pending_init` only tracks whether the deferred `Backend::initialize()` setup call has happened yet; it has nothing to do with whether a snapshot exists. A snapshot taken in a prior, separate execution (like `setUp()`) is already present in the deep-cloned `state_snapshots` map regardless of `pending_init`. `revert_state`, a sibling method, proves this - it goes through `backend_mut()` unconditionally and correctly finds the same snapshot id.

Fix

Check existence on the possibly-still-`Cow::Borrowed` backend first, via the read-only `state_snapshots()` accessor - no clone triggered - and only call `backend_mut()` (which clones-on-write and may run the deferred `initialize()`) when there's actually something to remove. This preserves the original intent of avoiding an unnecessary clone on a fuzz iteration that never really mutates the backend, while fixing the false-negative on a genuine hit. Same pattern applied to `delete_state_snapshots()`, backed by a new `StateSnapshots::is_empty()` accessor. The now-unused `initialized_backend_mut()` helper (its only two callers were these two methods) was removed as dead code.

Testing

  • Added a new Solidity fixture, `StateSnapshotDeleteFromSetUpTest`, reproducing the exact scenario: a snapshot taken in `setUp()`, deleted (singular and plural forms) as the very first cheatcode call of the test body, plus a `testFuzz_` variant proving it holds on every fuzz run (256/256 passing).
  • Real red-before-green: reverted just the fix (`git stash` on `cow.rs`/`state_snapshot.rs`), confirmed all 3 new tests genuinely fail on unfixed code with `VM::deleteStateSnapshot(0) -> false` and a subsequent `revertToState` proving the snapshot was never removed; restored the fix, confirmed green.
  • Full `StateSnapshots.t.sol` suite (15 tests across all three contracts) passes, no regressions to existing snapshot/revert behavior.
  • `cargo clippy -p foundry-evm-core -- -D warnings` clean.
  • `cargo fmt --check` clean on stable - nightly (used by this repo's CI) wasn't available in this environment to verify against.
  • Synchronous Codex adversarial review: no Critical/High/Medium/Low findings. It independently verified `Backend::initialize()` (which `backend_mut()` may trigger) never touches `state_snapshots`, and confirmed `initialized_backend_mut()` had exactly these two callers with nothing else depending on its removed timing gate.

receipts

no runtime UI change - the Solidity fixture's real red-before-green run against the built `forge` binary is the receipt for this PR (cheatcode-level fix, not a runnable app).

vm.deleteStateSnapshot / vm.deleteStateSnapshots silently returned
false / no-op for a snapshot that genuinely exists, whenever it's the
first cheatcode call of a fuzz run - e.g. a snapshot taken in setUp().

CowBackend::delete_state_snapshot gated access via
initialized_backend_mut(), which returned None while pending_init was
still Some (i.e. before any mutating cheatcode had run in the current
fuzz iteration). But pending_init only tracks deferred
Backend::initialize() setup, not whether a snapshot exists - a
snapshot taken in a prior, separate execution is already present in
the deep-cloned state_snapshots map regardless.

Fix: check existence on the possibly-still-borrowed backend via the
read-only state_snapshots() accessor before calling backend_mut(),
so a miss never forces an unnecessary clone and a hit still gets a
correct answer. Same pattern applied to the plural
delete_state_snapshots(), using a new StateSnapshots::is_empty().
Removed the now-dead initialized_backend_mut() helper.
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

✅ Changelog found

The deterministic check will validate the changed entry.

@figtracer
figtracer enabled auto-merge (squash) September 4, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants