Skip to content

fix(anvil): restore backwards compatibility for state dumps missing block/best_block_number keys - #16639

Closed
gomesalexandre wants to merge 1 commit into
foundry-rs:masterfrom
gomesalexandre:fix_loadstate_serde_default
Closed

fix(anvil): restore backwards compatibility for state dumps missing block/best_block_number keys#16639
gomesalexandre wants to merge 1 commit into
foundry-rs:masterfrom
gomesalexandre:fix_loadstate_serde_default

Conversation

@gomesalexandre

Copy link
Copy Markdown
Contributor

`SerializableState` (`crates/anvil/src/eth/backend/db.rs`) has two fields documented as "Option for backwards compatibility" that are actually hard-required by serde, while a third field with the identical doc comment correctly isn't:

```rust
/// Note: This is an Option for backwards compatibility: #5460
#[serde(deserialize_with = "deserialize_block_env_compat")] // <- missing #[serde(default)]
pub block: Option,

#[serde(deserialize_with = "deserialize_best_block_number_compat")] // <- missing #[serde(default)]
pub best_block_number: Option,

/// Note: This is an Option for backwards compatibility.
#[serde(default)] // <- has it
pub historical_states: Option,
```

serde's missing-field-defaults-to-`None` behavior only applies automatically when a field uses its own plain `Deserialize` impl. Adding `#[serde(deserialize_with = "...")]` without `#[serde(default)]` makes the field hard-required — the custom deserializer never runs if the key is absent, so you get a hard parse error instead of graceful `None`-defaulting.

Live-verified consequence

Loading an older state-dump JSON file that lacks the `block`/`best_block_number` keys entirely fails hard via both `anvil --load-state ` and the `anvil_loadState` RPC method (`error: failed to parse json file: missing field `block` at line 1 column 115`). The decisive control: the same file with the keys present-but-`null` loads fine — this is purely "absent key" vs "key present with default", nothing to do with the compat deserializers themselves.

The consumer is already written to expect `None`: `crates/anvil/src/eth/backend/mem/mod.rs`'s `load_state` does `let mut block_env = state.block.take();` then `if let Some(block) = &mut block_env { ... }`, and defaults `best_number` via `state.best_block_number.unwrap_or(...)`.

Regression origin

`#[serde(deserialize_with = "...")]` landed in #11179 ("v1.2 state load compatibility") whose explicit goal was to widen compatibility with old dump formats — but it didn't also add `#[serde(default)]`, the opposite of the PR's own intent. Pre-#11179 these fields were bare `pub block: Option` with no custom deserializer, which correctly defaulted to `None` on a missing key.

Precedent

#8501 ("`anvil_loadState` fails to decode state dump after upgrading") → #8752 ("fix(anvil): backwards compatible dumps"), merged — same bug class (an older dump file failing to load after an anvil upgrade), different field.

Why no test caught it

The closest existing test (`test_backward_compatibility_optional_fields_deserialization_v1_2`) deliberately omits three other fields to test compat, but always supplies `block` and `best_block_number` — and every `test-data/state-dump*.json` fixture carries both keys too.

Fix

Add `#[serde(default)]` to both fields, restoring the pre-#11179 graceful-degrade behavior the doc comments already claim exists.

Honest trade-off: this restores a silent partial load (rather than a loud parse error) if someone typos or truncates the `block` key in a hand-edited/corrupted dump file. This is exactly the pre-#11179 behavior, so precedent supports it as acceptable, but it's worth stating explicitly rather than presenting this as a risk-free one-word diff.

Testing

  • `test_backward_compatibility_missing_block_and_best_block_number_v1_2`: direct deserialization test proving a truly-absent-key document parses to `None`/`None`, identical to the present-and-`null` control case.
  • `test_load_state_missing_block_and_best_block_number_v1_2`: end-to-end test spawning a real Anvil node via `with_init_state_path` off a missing-keys dump file, verifying it boots and applies account state correctly — the real `load_state` code path, not just isolated deserialization.
  • Verified real red→green: reverted just the fix, confirmed both new tests fail on unfixed code (panic on missing field), restored the fix, confirmed both pass.
  • All 5 `backward_compatibility_*` tests and all 31 `state::` tests pass.
  • `cargo check -p anvil --tests` clean. `cargo fmt --check` clean on stable (nightly unavailable in this environment to verify against, same disclosure as prior PRs in this campaign).
  • Note: `cargo clippy -p anvil -- -D warnings` fails on a completely unmodified `master` checkout too (two pre-existing `missing_const_for_fn` findings in `mem/mod.rs`'s monad-feature shim functions, unrelated to this diff) — confirmed via `git stash` before touching anything, so not a regression introduced here.

receipts

no runtime UI change — this is a backend/RPC-level bug; the added integration test (spawning a real Anvil node off a hand-crafted missing-keys state file and verifying it boots + applies account state) is the receipt.

…lock/best_block_number keys

serde's missing-field default only applies when a field uses its own plain
Deserialize impl. block and best_block_number both use
#[serde(deserialize_with = "...")] without #[serde(default)], making them
hard-required despite being documented as Option for backwards compatibility
(the identical doc comment on historical_states correctly has #[serde(default)]).

An older state-dump file that omits these keys entirely (not null, absent)
now fails to load via both `anvil --load-state` and `anvil_loadState`, even
though load_state (mem/mod.rs) is written to handle None for both fields.

Regression from foundry-rs#11179 (v1.2 state load compatibility), whose goal was to
WIDEN compatibility with old dump formats.

Adds #[serde(default)] to both fields, restoring the pre-foundry-rs#11179 behavior
the doc comments already claim.

Trade-off disclosed: this restores a silent-partial-load on a truncated/
typo'd block key rather than a hard error - matching pre-foundry-rs#11179 behavior,
not a new risk.
@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

Copy link
Copy Markdown
Member

dont need this

@figtracer figtracer closed this Sep 4, 2026
@github-project-automation github-project-automation Bot moved this to Done in Foundry Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants