diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb85c5..6808c8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,19 +158,33 @@ pre-release development phases (see [`docs/ROADMAP.md`](docs/ROADMAP.md)). many simulations against the same snapshot without reallocating: it clears the per-simulation dirty layer (keeping the snapshot `Arc`, `ext_db`, and the reusable shared-memory buffer), reading the pristine snapshot again and behaving - exactly like a freshly-built overlay. The 64 KB shared-memory buffer is also + exactly like a freshly-built overlay. The 64 KiB shared-memory buffer is also recycled across the build→transact→revert call methods (stored as a plain `Vec`, so the overlay stays `Send`). - **Configurable EVM shared-memory pre-allocation** — `SharedMemoryCapacity` - (`Fixed(usize)` / `Auto`, default `Fixed(64_000)`) set via + (`Fixed(usize)` / `Auto`, default `Fixed(64 * 1024)` / 65,536 bytes) set via `EvmCacheBuilder::shared_memory_capacity`. `Fixed` pins the per-context working- memory buffer (general users running wide fan-outs of small simulations can lower it to cut per-overlay memory; the previous behavior is the default); `Auto` sizes it from the chain state loaded at build time (e.g. a bincode state file), clamped - to a 64 kB floor / 4 MiB ceiling. The resolved size is readable via + to a 64 KiB floor / 4 MiB ceiling. The resolved size is readable via `EvmCache::shared_memory_capacity()` and is propagated to every snapshot so snapshot-backed overlays pre-allocate the same amount. `with_cache_capacity` is the lower-level constructor behind the builder setter. +- **Explicit cold-account materialization** — `StateUpdate::AccountUpsert` and + `StateUpdate::account_upsert(...)` intentionally materialize an account absent + from both layers. Normal `StateUpdate::Account` patches are now cold-aware and + surface skipped cold patches through `StateDiff.skipped_accounts: + Vec`. +- **Invalidating layer-2 mutation wrapper** — `EvmCache::with_blockchain_db_mut` + runs a synchronous direct `BlockchainDb` mutation and invalidates the Phase 5 + memoized COW base automatically after the closure returns. +- **Exact access-list RLP data-gas helper** — + `access_list::access_list_rlp_data_gas(&AccessList)` returns the EIP-2930 RLP + calldata gas for an access list and backs the L2 profitability calculation. +- **Versioned on-disk cache envelope** — binary EVM state, bytecode, + `ImmutableDataCache`, and V3 tick snapshot cache files now start with + crate-specific magic bytes plus a `u32` version before the bincode payload. ### Changed @@ -183,7 +197,25 @@ pre-release development phases (see [`docs/ROADMAP.md`](docs/ROADMAP.md)). layer-2 bulk write now marks the touched addresses dirty for the memoized copy-on-write base. The write itself is still a direct backend (layer-2) write with the same semantics; only the receiver mutability changed. - +- **Raw layer-2 handles were renamed to unchecked accessors** (Phase 5) — + `EvmCache::blockchain_db()` is now `unchecked_blockchain_db()` and + `EvmCache::backend()` is now `unchecked_backend()`. The rename makes the + bypass explicit; use `with_blockchain_db_mut` for synchronous direct writes that + should automatically invalidate the snapshot base. +- **Persistence APIs now return `Result<()>`** — `cache::save_binary_state`, + `PrefetchRegistry::save`, and `EvmCache::flush` report serialization, + directory-creation, and write failures to explicit callers. `Drop` remains + best-effort and logs `flush()` errors. +- **Block re-pins clear stale context** — `set_block` sets `block_number` only + for concrete numeric pins, clears it for tag/hash/`None` pins, and clears stale + `basefee` on block changes and on non-concrete pin calls that can drift under + the same tag. `repin_to_block` follows the same no-stale-basefee rule; callers + refresh `NUMBER`/`BASEFEE` via `set_block_context` after fetching the new + header. +- **Legacy raw-bincode cache files are treated as misses** — the versioned cache + envelope intentionally rejects unversioned `evm_state.bin`, `bytecodes.bin`, + `immutable_data.bin`, and `v3_tick_snapshots.bin` payloads rather than trying + to deserialize ambiguous layouts. - Simulation entry points that distinguish failure modes return `SimulationResult` (`Result`), separating decoded reverts, EVM halts, and host errors. `SimulationErrorKind` remains as a deprecated alias. @@ -199,6 +231,19 @@ pre-release development phases (see [`docs/ROADMAP.md`](docs/ROADMAP.md)). ### Fixed +- **Cold absolute account patches no longer mask on-chain accounts.** + `StateUpdate::Account` on an account absent from both layers now skips instead + of writing `AccountInfo::default()` fields through the shared backend. The + skipped patch is visible in `StateDiff.skipped_accounts`; intentional cold + creation uses `StateUpdate::AccountUpsert`. +- **Access-list profitability no longer conflates provider failures with + unprofitable lists.** `SmartAccessList::into_access_list_if_profitable` and + `access_list_if_profitable` now propagate provider/pricing failures as `Err` + and reserve `Ok(None)` for empty, zero-priced, or genuinely unprofitable lists. +- **`simulate_call_with_balance_deltas` now reports a real access list.** It + extracts the EIP-2930 touched account/slot list from the EVM journal before + commit/revert, including the pre/post `balanceOf` reads and the simulated call, + instead of returning `AccessList::default()`. - **`cached_storage_value` silent-corruption bug** (Phase 3 §16.0, audit HIGH + MED). For a storage slot absent from an overlay account whose revm `account_state` is `StorageCleared` or `NotExisting`, the accessor now returns @@ -214,7 +259,9 @@ pre-release development phases (see [`docs/ROADMAP.md`](docs/ROADMAP.md)). **skips both layer writes** (returning an empty diff) when no field actually changes, instead of unconditionally inserting `AccountInfo::default()` into the shared backend for an all-`None` (or value-unchanged) patch on an absent address. - A real field change still materializes the backend account (unchanged intent). + Phase 5 later tightened this further: real field changes on cold accounts now + skip through `StateDiff.skipped_accounts` unless the caller uses + `StateUpdate::AccountUpsert`. - **`account_state`-awareness extended to the snapshot + account-info paths** (Phase 3 fix-review, HIGH + MED). A follow-up adversarial review found the §16.0 `cached_storage_value` fix had not been propagated to two sibling read paths: diff --git a/Cargo.lock b/Cargo.lock index db4da26..dc8eaf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1852,6 +1852,7 @@ dependencies = [ "alloy-node-bindings", "alloy-primitives", "alloy-provider", + "alloy-rlp", "alloy-rpc-client", "alloy-rpc-types-eth", "alloy-sol-types", diff --git a/Cargo.toml b/Cargo.toml index 4a928d4..5f7cf9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ alloy-eips = "1.0.38" alloy-network = "1.0.38" alloy-primitives = { version = "1.4", features = ["map"] } alloy-provider = "1.0.38" +alloy-rlp = "0.3" alloy-rpc-client = "1.0.38" alloy-rpc-types-eth = "1.0.38" alloy-sol-types = "1.4" diff --git a/README.md b/README.md index f106255..2d97a5c 100644 --- a/README.md +++ b/README.md @@ -217,10 +217,10 @@ println!("installed {} bytes at {}", etched.code_size, etched.target_address); ## Benchmarks -Criterion benchmarks live in [`benches/`](benches). The offline benches are the -baseline against which the planned copy-on-write snapshot rewrite (roadmap -Pillar A) will be measured, so they exercise the real hot paths at a range of -cache sizes: +Criterion benchmarks live in [`benches/`](benches). The offline benches exercise +the current hot paths at a range of cache sizes, including the Phase 5 +copy-on-write snapshot implementation and retained deep-clone baselines where +useful for A/B comparison: | Bench | Measures | | --- | --- | diff --git a/benches/simulation.rs b/benches/simulation.rs index 82a8bd2..aca01e1 100644 --- a/benches/simulation.rs +++ b/benches/simulation.rs @@ -10,13 +10,15 @@ //! via `inject_storage_batch` (`populated_cache_layer2`), the way a fork cache //! actually holds it. For each size it benches both the COW `create_snapshot` //! and the retained `create_snapshot_deep_clone`. The deep clone is an O(total -//! state) copy, so its cost slopes up with the index size; the COW path folds -//! only the (empty) hot layer over an `Arc`-shared memoized base, so after the -//! base is warm it should stay roughly **flat** across sizes. +//! state) copy, so its cost slopes up with the index size; the COW path shares +//! the memoized base and avoids cloning total storage slots. It still scans +//! accounts and new layer-1 entries, so it should be much flatter than the deep +//! clone, especially as slots/account grows, but not strictly flat by account +//! count. //! - **`resnapshot_hot_loop`.** Warms the base with one snapshot, applies a small //! `apply_updates` layer-1 mutation, then measures `create_snapshot`. This is -//! the memoization win: ≈ O(changed) and flat across cold-index size, vs. the -//! deep clone's slope. +//! the memoization win: the COW path avoids cloning cold storage slots but +//! remains sensitive to account scans and new layer-1 entries. //! - **`overlay_fanout`.** Measures fanning one frozen snapshot out into many //! isolated simulations, comparing a fresh `EvmOverlay::new` per sim against a //! single `reset()`-recycled overlay (Pillar A.2). @@ -81,9 +83,9 @@ fn populated_cache_layer2(rt: &Runtime, accounts: usize, slots_per: usize) -> Ev /// A/B snapshot creation across cold-index sizes: the COW `create_snapshot` vs. /// the retained `create_snapshot_deep_clone`, both over a layer-2-seeded index. /// -/// The deep clone slopes up with the index; the COW path, after a warm-up -/// snapshot has memoized the base, should stay roughly flat (the hot layer is -/// empty, so it is an `Arc` handle copy plus the O(accounts) growth scan). +/// The deep clone slopes up with total slots; the COW path, after a warm-up +/// snapshot has memoized the base, avoids cloning those slots but still pays the +/// O(accounts) growth scan. fn bench_create_snapshot(c: &mut Criterion) { let rt = Runtime::new().unwrap(); let mut group = c.benchmark_group("create_snapshot"); @@ -115,7 +117,8 @@ fn bench_create_snapshot(c: &mut Criterion) { /// The memoization win: a hot re-snapshot loop. Warm the base once, apply a /// *small* layer-1 mutation, then measure `create_snapshot`. Cost should track -/// the changed state (≈ flat across cold-index size), unlike the deep clone. +/// account scanning plus new layer-1 entries, staying much flatter than the deep +/// clone as cold storage grows. fn bench_resnapshot_hot_loop(c: &mut Criterion) { let rt = Runtime::new().unwrap(); let mut group = c.benchmark_group("resnapshot_hot_loop"); diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index 329c2a8..9a72081 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -1,128 +1,116 @@ # Known issues & limitations A living triage list of bugs, smells, and limitations surfaced during the -publication-readiness review. Items here are **flagged, not fixed** — the test -suite deliberately pins *current* behavior, so changing any of these is a -conscious, reviewable decision (and a `CHANGELOG.md` entry). +publication-readiness review. Items here are either **remaining limitations** or +**recently-fixed issues kept for auditability**; behavior-changing fixes should +carry red/green tests and a `CHANGELOG.md` entry. Confidence legend: **[V]** verified against the source during review; **[R]** reported by the review and worth confirming before acting. -## Correctness / behavior to review - -1. **[V] Silent persistence failures.** `cache::save_binary_state`, - `PrefetchRegistry::save`, and `ImmutableDataCache::save` log a warning on I/O - error but return `()`, so callers cannot detect a failed write (full disk, - permissions, partial flush). Consider returning `Result<()>` (a breaking - change worth taking pre-1.0). Tested today only insofar as the happy-path - round-trip succeeds. - -2. **[V] Access-list L2 profitability uses an approximate gas model.** In - `access_list.rs`, `into_access_list_if_profitable` / `access_list_if_profitable` - estimate L1 calldata cost with hand-rolled RLP-overhead constants - (`4 * 16` per address, `16` per key, `3 * 16` for the list header). This is an - intentional heuristic, not a precise EIP-2930 serialization cost — verify it - against real serialized sizes before relying on the profitability verdict for - anything other than a rough gate. The two functions also duplicate this logic - (a maintenance hazard: a fix to one must be mirrored). - -3. **[V] Profitability swallows provider errors.** The same functions catch all - provider errors and return `Ok(None)`, which is indistinguishable from - "computed: not profitable." A caller cannot tell a skipped check (RPC down) - from a real negative. Consider a result type that distinguishes the two. - -4. **[R] `set_block` with a tag leaves `block.number` stale.** Only - `BlockId::Number(n)` syncs the `NUMBER` opcode value; pinning to a tag (e.g. - `BlockId::latest()`) leaves the previously-set number in the block env. Either - resolve tags to a concrete number at pin time or document the constraint - loudly. - -5. **[R] Duplicate custom-error selectors shadow silently.** `RevertDecoder` +## Recently fixed by `codex/phase-5-known-issues-top5` + +1. **[FIXED] Cold absolute `Account` patches no longer materialize unknown + accounts.** `StateUpdate::Account` is cold-aware: a partial patch against an + address absent from both layers is skipped, does not write a default backend + account, and is surfaced through `StateDiff.skipped_accounts`. Intentional + cold materialization is now explicit via `StateUpdate::AccountUpsert` / + `StateUpdate::account_upsert(...)`. + +2. **[FIXED] Block-context drift after re-pinning.** `set_block` now sets + `block_number` only for concrete numeric pins and clears it for tag/hash/`None` + pins. Block changes, plus non-concrete pin calls that can drift under the same + tag, clear stale `basefee`; callers refresh `NUMBER`/`BASEFEE` together with + `set_block_context` after fetching the new header. + +3. **[FIXED] Synchronous layer-2 escape hatches have an invalidating wrapper.** + Raw handles are now visibly named `unchecked_blockchain_db()` / + `unchecked_backend()`, and `EvmCache::with_blockchain_db_mut(...)` runs a + synchronous `BlockchainDb` mutation and invalidates the COW snapshot base + automatically. + +4. **[FIXED] Explicit persistence failures are observable.** + `cache::save_binary_state`, `PrefetchRegistry::save`, and + `EvmCache::flush` now return `anyhow::Result<()>`. `Drop` remains best-effort + and logs flush errors. + +5. **[FIXED] Access-list profitability uses exact EIP-2930 RLP bytes.** + Arbitrum profitability now centralizes data-gas accounting in + `access_list_rlp_data_gas(...)` and provider/pricing failures propagate as + `Err`, leaving `Ok(None)` for empty/zero-priced/unprofitable lists. + +6. **[FIXED] `simulate_call_with_balance_deltas` now returns the touched access + list.** The pre/post `balanceOf` reads and simulated call share one EVM + journal, and the method now extracts the EIP-2930 access list before + commit/revert, matching the transfer-inspector simulation path. + +7. **[FIXED] On-disk cache files carry magic bytes and a version number.** + `binary_state`, `bytecode`, `ImmutableDataCache`, and V3 tick snapshots now + write a crate-specific magic header plus version `1` before the bincode + payload. Unknown magic/version values and legacy raw-bincode files are treated + as cache misses. + +8. **[FIXED] `call_raw_with_access_list` did not revert its checkpoint on a + transact error.** Both `EvmCache::call_raw_with_access_list` and + `EvmOverlay::call_raw_with_access_list_with` now match on the `transact_one` + result and `checkpoint_revert` on **every** path (success and host error), + matching `call_raw` / `simulate_with_transfer_tracking`. A host-level transact + error no longer leaves the overlay checkpoint un-reverted. + +## Remaining open issues ranked by unexpected-result risk + +1. **[R] Duplicate custom-error selectors shadow silently.** `RevertDecoder` registration replaces an existing entry for the same 4-byte selector with no warning, so an accidental double-registration silently wins. Consider a debug-level log or a `try_register` that reports collisions. -6. **[R] ERC20 `Transfer` decoding assumes the standard layout.** `inspector.rs` +2. **[R] ERC20 `Transfer` decoding assumes the standard layout.** `inspector.rs` reads `from`/`to` from indexed topics and `value` from the first 32 data - bytes. Non-standard or packed `Transfer` encodings parse incorrectly. Also, an - address that appears as both `from` and `to` in one transfer is both - subtracted and added (a semantically-invalid self-transfer is not rejected). + bytes. Non-standard or packed `Transfer` encodings may parse incorrectly or be + skipped. A self-transfer where `from == to` nets to zero for that owner; this + is now documented at the call site. -7. **[R] Panic codes above `u64::MAX` are dropped.** `decode_solidity_panic` +3. **[V] `SystemTime::now().unwrap()` panic risk in EVM construction.** + `build_evm` / `make_local_context` (and the overlay equivalents) call + `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()` when no timestamp + override is set, which panics if the system clock is before the Unix epoch. + Setting an explicit timestamp avoids it; consider a saturating fallback. + +4. **[R] Panic codes above `u64::MAX` are dropped.** `decode_solidity_panic` converts out-of-range panic codes to `None`. Real compiler-emitted panic codes - are single-byte constants, so this is benign in practice; now documented at the + are single-byte constants, so this is benign in practice and documented at the call site. -8. **[V] `simulate_call_with_balance_deltas` returns an empty access list.** It - sets `CallSimulationResult.access_list = AccessList::default()`, unlike - `simulate_with_transfer_tracking` which populates it via `extract_access_list`. - Either the field is meaningless on this path or the population was missed — - the docs now state the field is empty here; reconcile before relying on it. - -9. **[FIXED] `call_raw_with_access_list` did not revert its checkpoint on a - transact error.** Both `EvmCache::call_raw_with_access_list` and - `EvmOverlay::call_raw_with_access_list_with` now match on the `transact_one` - result and `checkpoint_revert` on **every** path (success and host error), - matching `call_raw` / `simulate_with_transfer_tracking`. A host-level transact - error no longer leaves the overlay checkpoint un-reverted. - -10. **[V] `SystemTime::now().unwrap()` panic risk in EVM construction.** - `build_evm` / `make_local_context` (and the overlay equivalents) call - `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()` when no timestamp - override is set, which panics if the system clock is before the Unix epoch. - Setting an explicit timestamp avoids it; consider a saturating fallback. - -18. **[V] Cold absolute `Account` patch masks the real on-chain account.** A - *partial* absolute [`StateUpdate::Account`] patch (e.g. balance-only) applied - to an address absent from **both** cache layers writes default values for the - un-patched fields (nonce `0`, empty code) through the shared BlockchainDb - backend as authoritative — pre-empting a later RPC fetch of the real account - (`apply_account_patch` materializes the backend account on any real change, by - design / spec §5.2). This is a live-fork footgun for callers reconstructing an - account from one event field. Mitigations: fetch+seed the account first, or use - the relative `StateUpdate::BalanceDelta` / `EvmCache::modify_account_balance` - (Phase 3 §16.5), which are cold-aware (a cold target is skipped and surfaced in - `StateDiff.skipped_balances`, never materialized). A no-op patch (no field - actually changes) does **not** materialize anything (Phase 3 §16.1 fix). The - rustdoc on `apply_update` / `StateUpdate::Account` / `AccountPatch` carries a - `# Warning` to this effect. - ## Code-quality nits -11. **[V] Dead branch in `i128_to_u256`** (`cache/storage_keys.rs`): both the +5. **[V] Dead branch in `i128_to_u256`** (`cache/storage_keys.rs`): both the `value >= 0` and `else` arms evaluate the identical `U256::from(value as u128)`. The two's-complement cast is correct for both signs, so the `if`/`else` can collapse to one line (keep the explanatory comment). -12. **[R] V3 tick-snapshot keys serialize as strings.** `V3PoolTickSnapshot` +6. **[R] V3 tick-snapshot keys serialize as strings.** `V3PoolTickSnapshot` stringifies `i16`/`i32` tick/word keys for bincode, then `parse()`s them back in `to_tick_bitmap`/`to_ticks`, silently dropping any key that fails to parse. A native integer-keyed encoding would be faster and would not fail silently. -13. **[V] On-disk caches have no version header.** `binary_state`, `bytecode`, - `metadata` (`ImmutableDataCache`), and `tick_snapshot` all persist raw bincode - with no magic bytes or version field, so a struct-layout change silently - invalidates every existing cache file (decoded as a miss). A version header - would enable detection/migration. - -14. **[R] Balancer pool id keyed by `Debug` formatting.** `ImmutableDataCache` +7. **[R] Balancer pool id keyed by `Debug` formatting.** `ImmutableDataCache` keys `balancer_pools` by `format!("{:?}", pool_id)`. `Debug` output is not a stable encoding contract; a hex encoding would be safer for a persisted key. ## API ergonomics -15. **[R] `snapshot()` vs `create_snapshot()`.** `snapshot()` returns a low-level +8. **[R] `snapshot()` vs `create_snapshot()`.** `snapshot()` returns a low-level `revm::database::Cache` for in-place `restore()`; `create_snapshot()` returns an `Arc` for cross-thread fan-out. The names don't convey the difference. Docs now cross-reference them (see the rustdoc), but a rename could be considered pre-1.0. -16. **[R] Process-global cache speed mode.** `set_cache_speed_mode` / +9. **[R] Process-global cache speed mode.** `set_cache_speed_mode` / `cache_speed_mode` are a process-wide `static`, so two caches in one process cannot tune concurrency independently. Phase 1 moved configuration toward per-instance (`EvmCacheBuilder::cache_config`); the global setter remains. -17. **[V] `SpeculativeSim` consumption contract.** Both `validate()` and +10. **[V] `SpeculativeSim` consumption contract.** Both `validate()` and `into_optimistic()` take `self` by value, so double-consumption is unreachable under normal ownership. Internally `validate()` uses `.expect("validation handle taken twice")` (defensive) while `into_optimistic()` no-ops if the @@ -148,29 +136,35 @@ Confidence legend: **[V]** verified against the source during review; kept as the A/B benchmark baseline and the read-equivalence reference; the `create_snapshot` group in `benches/simulation.rs` measures both. Decisions and the cost model are in [`phase-5-spec.md`](phase-5-spec.md) / `ROADMAP.md`. -- **[V] Memoized-base staleness at the layer-2 escape hatches (Phase 5).** The +- **Layer-2 unchecked accessors remain an explicit contract boundary (Phase 5).** The snapshot base's growth scan is count/absence-based, which is sufficient for the supported writers: the crate's own mutators (`apply_update`, `inject_storage_batch`, the `inject_*` helpers, purges, code overrides) explicitly mark the base dirty, and the `foundry-fork-db` `SharedBackend` lazy fetch is append-only at a fixed block (it only inserts on a cache miss, never overwrites in place — a load-bearing - invariant noted in `refresh_base`). The one gap, surfaced by the Phase 5 - adversarial review: a **direct, out-of-band write through the public - `blockchain_db()` / `backend()` handles** that *overwrites an existing slot value - at an unchanged slot count* is invisible to the scan, so a subsequent - `create_snapshot` may reuse a stale base (`create_snapshot_deep_clone` always - re-reads and would diverge). This is a contract boundary, not an internal bug — no - in-crate path triggers it, and both accessors are documented as bypassing the - two-layer model. Mitigation: call the new - [`EvmCache::invalidate_snapshot_base`] after any direct layer-2 write through those - handles (or re-pin via `set_block`); the rustdoc on both accessors and the hook - carries this warning, and `tests/cow_snapshot.rs` - (`invalidate_snapshot_base_rehonest_after_escape_hatch_write`) pins it. + invariant noted in `refresh_base`). Direct out-of-band writes through the + `unchecked_blockchain_db()` / `unchecked_backend()` handles still bypass the + normal write funnel by design. For synchronous `BlockchainDb` map writes, prefer + [`EvmCache::with_blockchain_db_mut`], which invalidates the base automatically + after the closure returns. If using the unchecked handle directly, call + [`EvmCache::invalidate_snapshot_base`] after the write lands and before the next + snapshot (or re-pin via `set_block`). For + `SharedBackend::insert_or_update_storage` / `insert_or_update_address`, the call + only enqueues work on the backend handler; `invalidate_snapshot_base()` does not + wait for that queued update. First synchronize or read back until the expected + value is visible in `BlockchainDb` / through the backend, then invalidate before + creating the snapshot. The rustdoc on both accessors and the hook carries this + warning, and `tests/cow_snapshot.rs` + (`invalidate_snapshot_base_rehonest_after_escape_hatch_write`, + `invalidate_snapshot_base_rehonest_after_existing_account_write`, + `with_blockchain_db_mut_rehonest_after_storage_overwrite`, + `with_blockchain_db_mut_rehonest_after_account_overwrite`) + pins it. - **`protocols` not yet extracted.** The DeFi surface is feature-gated but still - in-crate; `cargo test --no-default-features` is not yet supported because some - unit tests assume the default feature. Extraction into `evm-amm-state` is - planned (roadmap), blocked partly by `ImmutableDataCache` coupling generic - token-decimals with V2/V3/Balancer pool metadata. + in-crate. The generic core builds and tests with `--no-default-features`, but + extraction into `evm-amm-state` is still planned (roadmap), blocked partly by + `ImmutableDataCache` coupling generic token-decimals with V2/V3/Balancer pool + metadata. - **Event-driven sync (roadmap Pillar B) — reader/writer halves done; live WS transport is not.** The Phase 3 **writer half** (`StateUpdate` + `apply_update`/`apply_updates`) and the Phase 4 **reader half** (the `events` diff --git a/docs/phase-2-spec.md b/docs/phase-2-spec.md index c61f19c..baff6b2 100644 --- a/docs/phase-2-spec.md +++ b/docs/phase-2-spec.md @@ -54,7 +54,7 @@ evaluation sims only. `storage_batch_fetcher() -> Option<&StorageBatchFetchFn>`, `inject_storage_batch(&[(Address,U256,U256)])`, `purge_pool_storage`, `purge_pool_slots`, `call_raw_with`/`TxConfig`, `CallSimulationResult`, - `blockchain_db()`, `db_mut()`. + `unchecked_blockchain_db()`, `db_mut()`. - `cache::EvmOverlay` / `cache::EvmSnapshot` (`overlay.rs`/`snapshot.rs`): `EvmOverlay::new(Arc, Option)`, `call_raw`, `simulate_with_transfer_tracking`. `EvmOverlay` is `Send`. diff --git a/docs/phase-3-spec.md b/docs/phase-3-spec.md index 199eaf5..e51886c 100644 --- a/docs/phase-3-spec.md +++ b/docs/phase-3-spec.md @@ -358,7 +358,9 @@ in a new `tests/state_update.rs` (reuse `tests/common`). `nonce/code_hash == None`. 6. **Account code patch:** patch code; assert `code_hash` recomputed (`Bytecode::hash_slow`), `code_hash` delta recorded; balance/nonce preserved. -7. **Account create:** patch an absent account → materialized with patched fields. +7. **Cold account patch:** patch an absent account → skipped and surfaced in + `StateDiff.skipped_accounts`; explicit `AccountUpsert` materializes with + patched fields. 8. **Purge Account / AllStorage / Slots:** correct layers cleared; `PurgeRecord` counts (`slots_removed`, `account_removed`) correct on both layers. 9. **`apply_updates` fold + merge:** a mixed batch (Slot, Account, Purge) → @@ -645,7 +647,7 @@ the `SlotDelta`/`modify_slot` base read (HIGH) and `apply_slot`'s `old`/predicat **and** no backend value) stays skip-and-surface. Add a test for the hot-zero case (it is currently the untested seam between Decision-4 skip and apply). -### 16.1 No-op `Account` patch must not materialize a backend account (audit LOW) +### 16.1 No-op / cold `Account` patch must not materialize a backend account (audit LOW; tightened in Phase 5) `apply_account_patch` (src/cache/mod.rs ~1331-1340) writes the patched `AccountInfo` into the backend **unconditionally**, so an all-`None` (or @@ -654,23 +656,21 @@ otherwise no-change) patch on an address absent from both layers inserts diff — breaking no-op parity with the Slot path and (per the cold-account hazard) masking a future RPC fetch. **Fix (LOCKED):** compute the change first; **only write-through when at least one field actually changes** (i.e. skip both layer -writes and return `None` when the patched `info` equals the loaded base). A real -field change on an absent address still materializes the backend account (the -existing intended behavior — keep `apply_account_patch_materializes_absent_account` -green). Add a no-op idempotence test (patching balance to its current value ⇒ -empty diff, no backend account materialized). - -### 16.2 Cold absolute-`Account`-patch hazard — document (audit LOW) - -A *partial* absolute `Account` patch on a cold (un-fetched) address writes default -nonce/code through the shared backend, masking the real on-chain account. This is -spec-locked §5.2 behavior, **not** changed here, but it is an undocumented -live-fork footgun. **Fix (LOCKED, docs only):** add a `### Known issues` entry in -`docs/KNOWN_ISSUES.md` and a prominent `# Warning` doc paragraph on -`apply_update` / `StateUpdate::Account` / `AccountPatch` stating that a partial -patch on an address absent from both cache layers writes default nonce/code as -authoritative (pre-empting RPC), so callers must fetch+seed the account first, or -use `StateUpdate::BalanceDelta` (§16.5) for relative native-balance tracking. +writes and return `None` when the patched `info` equals the loaded base). +Phase 5 tightened the cold-account contract further: a real field change on an +address absent from both layers now skips and records `SkippedAccountPatch` in +`StateDiff.skipped_accounts`; explicit materialization uses +`StateUpdate::AccountUpsert`. Add no-op and cold-skip idempotence tests (patching +balance to its current value ⇒ empty diff; cold balance patch ⇒ no backend account +materialized). + +### 16.2 Cold absolute-`Account`-patch hazard — fixed in Phase 5 (audit LOW) + +A *partial* absolute `Account` patch on a cold (un-fetched) address used to write +default nonce/code through the shared backend, masking the real on-chain account. +Phase 5 changed this contract: `StateUpdate::Account` is cold-aware and records a +`SkippedAccountPatch` instead; callers that intentionally want a synthetic/default +account use `StateUpdate::AccountUpsert`. ### 16.3 `serde` on the vocabulary (audit HIGH gap) @@ -778,7 +778,7 @@ Add tests (in `tests/state_update.rs` unless noted). Each must assert the materialized where the spec says none should be (mirror `apply_slot_no_overlay_account_is_not_materialized`). - **Backend-only account patch:** seed an account only in the backend - (`blockchain_db().accounts().write().insert`), patch balance, assert + (`unchecked_blockchain_db().accounts().write().insert`), patch balance, assert `AccountChange.balance == Some((old,new))`, backend updated, overlay still absent. - **Nonce-only** and **multi-field (balance+nonce+code)** patches: assert the respective `AccountChange` fields are `Some`/`None` correctly. diff --git a/examples/prefetch_registry.rs b/examples/prefetch_registry.rs index 3c57366..690092b 100644 --- a/examples/prefetch_registry.rs +++ b/examples/prefetch_registry.rs @@ -19,7 +19,7 @@ use alloy_primitives::{Address, U256}; use evm_fork_cache::StorageAccessList; use evm_fork_cache::prefetch_registry::PrefetchRegistry; -fn main() { +fn main() -> anyhow::Result<()> { let pool = Address::repeat_byte(0xAA); let vault_a = Address::repeat_byte(0x01); let vault_b = Address::repeat_byte(0x02); @@ -45,7 +45,7 @@ fn main() { // Persist to disk (bincode) and reload — the shape survives the round trip. let path = std::env::temp_dir().join("evm_fork_cache_example_prefetch.bin"); - registry.save(&path); + registry.save(&path)?; let loaded = PrefetchRegistry::load(&path); let aggregated = loaded.phase_slots("pool_refresh"); @@ -61,4 +61,5 @@ fn main() { ); let _ = std::fs::remove_file(&path); + Ok(()) } diff --git a/src/access_list.rs b/src/access_list.rs index a84c99e..bf9df24 100644 --- a/src/access_list.rs +++ b/src/access_list.rs @@ -15,8 +15,9 @@ use alloy_eips::eip2930::{AccessList, AccessListItem}; use alloy_network::Network; use alloy_primitives::{Address, B256, U256, address}; use alloy_provider::Provider; +use alloy_rlp::Encodable; use alloy_sol_types::{SolCall, sol}; -use anyhow::Result; +use anyhow::{Context as _, Result}; use revm::context::result::ExecutionResult; use tracing::{debug, info}; @@ -141,36 +142,18 @@ impl SmartAccessList { /// - **L2 savings**: `100 gas * entry_count * perArbGas`, where each address /// and each storage key counts as one entry (the EIP-2929 warm-vs-cold /// access discount). - /// - **L1 cost**: `l1_data_gas * l1_base_fee`, where `l1_data_gas` sums the - /// per-byte calldata gas ([`l1_data_gas_for_bytes`]) of every address and - /// key plus a fixed RLP-framing surcharge. - /// - /// # Cost model is approximate - /// - /// The RLP-overhead constants — roughly `4 * 16` gas per address entry, - /// `16` gas per storage key, and `3 * 16` gas for the top-level list headers - /// — are a deliberate **approximation**, not the exact EIP-2930 RLP - /// serialization cost. They assume worst-case non-zero framing bytes and do - /// not account for the real RLP length-prefix sizing, address/key sharing, - /// or rollup-specific compression. Treat this as a rough profitability gate, - /// not a precise gas accounting: a list near the break-even point may be - /// classified either way. + /// - **L1 cost**: `l1_data_gas * l1_base_fee`, where `l1_data_gas` is the + /// exact per-byte calldata gas ([`l1_data_gas_for_bytes`]) of the EIP-2930 + /// RLP-encoded access list. /// /// # Errors /// - /// Returns `Err` only if the call wrapper itself surfaces a non-recoverable - /// error; in practice provider/pricing failures do **not** error. + /// Returns `Err` if the provider/pricing queries fail. /// /// Returns `Ok(None)` when: /// - the list is empty, - /// - the `ArbGasInfo` pricing or L1-base-fee query fails (the error is logged - /// at `debug` and swallowed — see below), /// - either the L2 or L1 gas price reads as zero, or /// - the estimated L1 cost meets or exceeds the L2 savings (not profitable). - /// - /// A `None` returned because a provider query failed is **indistinguishable** - /// from a `None` returned because the list was genuinely unprofitable: both - /// surface as a skipped access list, not as an error. pub async fn into_access_list_if_profitable( self, provider: &P, @@ -182,21 +165,15 @@ impl SmartAccessList { // Query ArbGasInfo for current pricing let arb = ArbGasInfo::new(ARB_GAS_INFO, provider); let prices_call = arb.getPricesInWei(); - let prices = match prices_call.call().await { - Ok(p) => p, - Err(e) => { - debug!(error = %e, "Failed to query ArbGasInfo prices, skipping access list"); - return Ok(None); - } - }; + let prices = prices_call + .call() + .await + .context("failed to query ArbGasInfo prices for access-list profitability")?; let l1_fee_call = arb.getL1BaseFeeEstimate(); - let l1_base_fee = match l1_fee_call.call().await { - Ok(fee) => fee, - Err(e) => { - debug!(error = %e, "Failed to query L1 base fee, skipping access list"); - return Ok(None); - } - }; + let l1_base_fee = l1_fee_call + .call() + .await + .context("failed to query ArbGasInfo L1 base fee for access-list profitability")?; let l2_gas_price = prices.perArbGas; @@ -205,46 +182,14 @@ impl SmartAccessList { return Ok(None); } - // Calculate aggregate L2 savings and L1 cost - let mut total_entries: u64 = 0; - let mut total_l1_data_gas: u64 = 0; - - for item in &self.items { - total_entries += 1; - total_l1_data_gas += l1_data_gas_for_bytes(item.address.as_slice()); - // RLP overhead per address entry (~3-4 bytes, assume non-zero) - total_l1_data_gas += 4 * 16; - - for key in &item.storage_keys { - total_entries += 1; - total_l1_data_gas += l1_data_gas_for_bytes(key.as_slice()); - // RLP length prefix (1 byte, non-zero) - total_l1_data_gas += 16; - } - } - // Top-level RLP list headers (~3 bytes) - total_l1_data_gas += 3 * 16; - - // L2 savings: 100 gas per entry × L2 gas price - let l2_savings_wei = U256::from(total_entries) * U256::from(100) * l2_gas_price; - // L1 cost: serialized data gas × L1 base fee - let l1_cost_wei = U256::from(total_l1_data_gas) * l1_base_fee; - - let profitable = l2_savings_wei > l1_cost_wei; - - info!( - entries = total_entries, - items = self.items.len(), - l2_savings_wei = %l2_savings_wei, - l1_cost_wei = %l1_cost_wei, - l2_gas_price_gwei = %format_gwei(l2_gas_price), - l1_base_fee_gwei = %format_gwei(l1_base_fee), - profitable, - "Access list profitability check" - ); - - if profitable { - Ok(Some(AccessList(self.items))) + let access_list = AccessList(self.items); + if log_access_list_profitability( + &access_list, + l2_gas_price, + l1_base_fee, + "Access list profitability check", + ) { + Ok(Some(access_list)) } else { Ok(None) } @@ -261,31 +206,14 @@ impl SmartAccessList { /// [`SmartAccessList::into_access_list_if_profitable`] for a pre-built /// [`AccessList`]; the two share the same cost model and break-even comparison. /// -/// # Cost model is approximate -/// -/// As with [`SmartAccessList::into_access_list_if_profitable`], the L1 cost is -/// estimated from per-byte calldata gas ([`l1_data_gas_for_bytes`]) plus fixed -/// RLP-framing surcharges (`4 * 16` gas per address, `16` gas per key, `3 * 16` -/// gas for the top-level headers). Those framing constants are an -/// **approximation**, not the exact EIP-2930 RLP serialization cost: they assume -/// worst-case non-zero bytes and ignore real length-prefix sizing and -/// rollup-specific compression. Treat the result as a rough profitability gate. -/// /// # Errors /// -/// Returns `Err` only if the call wrapper itself surfaces a non-recoverable -/// error; in practice provider/pricing failures do **not** error. +/// Returns `Err` if the provider/pricing queries fail. /// /// Returns `Ok(None)` when: /// - the list is empty, -/// - the `ArbGasInfo` pricing or L1-base-fee query fails (the error is logged at -/// `debug` and swallowed), /// - either the L2 or L1 gas price reads as zero, or /// - the estimated L1 cost meets or exceeds the L2 savings (not profitable). -/// -/// A `None` returned because a provider query failed is **indistinguishable** -/// from a `None` returned because the list was genuinely unprofitable: both -/// surface as a skipped access list, not as an error. pub async fn access_list_if_profitable( access_list: AccessList, provider: &P, @@ -296,20 +224,16 @@ pub async fn access_list_if_profitable( // Query ArbGasInfo for current pricing let arb = ArbGasInfo::new(ARB_GAS_INFO, provider); - let prices = match arb.getPricesInWei().call().await { - Ok(p) => p, - Err(e) => { - debug!(error = %e, "Failed to query ArbGasInfo prices, skipping access list"); - return Ok(None); - } - }; - let l1_base_fee = match arb.getL1BaseFeeEstimate().call().await { - Ok(fee) => fee, - Err(e) => { - debug!(error = %e, "Failed to query L1 base fee, skipping access list"); - return Ok(None); - } - }; + let prices = arb + .getPricesInWei() + .call() + .await + .context("failed to query ArbGasInfo prices for access-list profitability")?; + let l1_base_fee = arb + .getL1BaseFeeEstimate() + .call() + .await + .context("failed to query ArbGasInfo L1 base fee for access-list profitability")?; let l2_gas_price = prices.perArbGas; @@ -318,45 +242,12 @@ pub async fn access_list_if_profitable( return Ok(None); } - // Calculate aggregate L2 savings and L1 cost - let mut total_entries: u64 = 0; - let mut total_l1_data_gas: u64 = 0; - - for item in &access_list.0 { - total_entries += 1; - total_l1_data_gas += l1_data_gas_for_bytes(item.address.as_slice()); - // RLP overhead per address entry (~3-4 bytes, assume non-zero) - total_l1_data_gas += 4 * 16; - - for key in &item.storage_keys { - total_entries += 1; - total_l1_data_gas += l1_data_gas_for_bytes(key.as_slice()); - // RLP length prefix (1 byte, non-zero) - total_l1_data_gas += 16; - } - } - // Top-level RLP list headers (~3 bytes) - total_l1_data_gas += 3 * 16; - - // L2 savings: 100 gas per entry × L2 gas price - let l2_savings_wei = U256::from(total_entries) * U256::from(100) * l2_gas_price; - // L1 cost: serialized data gas × L1 base fee - let l1_cost_wei = U256::from(total_l1_data_gas) * l1_base_fee; - - let profitable = l2_savings_wei > l1_cost_wei; - - info!( - entries = total_entries, - items = access_list.0.len(), - l2_savings_wei = %l2_savings_wei, - l1_cost_wei = %l1_cost_wei, - l2_gas_price_gwei = %format_gwei(l2_gas_price), - l1_base_fee_gwei = %format_gwei(l1_base_fee), - profitable, - "Simulation access list profitability check" - ); - - if profitable { + if log_access_list_profitability( + &access_list, + l2_gas_price, + l1_base_fee, + "Simulation access list profitability check", + ) { Ok(Some(access_list)) } else { Ok(None) @@ -483,6 +374,49 @@ pub fn l1_data_gas_for_bytes(data: &[u8]) -> u64 { .sum() } +/// Exact L1 calldata gas for the EIP-2930 RLP encoding of an access list. +pub fn access_list_rlp_data_gas(access_list: &AccessList) -> u64 { + let mut encoded = Vec::with_capacity(access_list.length()); + access_list.encode(&mut encoded); + l1_data_gas_for_bytes(&encoded) +} + +fn access_list_entry_count(access_list: &AccessList) -> u64 { + access_list + .0 + .iter() + .map(|item| 1 + item.storage_keys.len() as u64) + .sum() +} + +fn log_access_list_profitability( + access_list: &AccessList, + l2_gas_price: U256, + l1_base_fee: U256, + message: &'static str, +) -> bool { + let total_entries = access_list_entry_count(access_list); + let total_l1_data_gas = access_list_rlp_data_gas(access_list); + let l2_savings_wei = U256::from(total_entries) * U256::from(100) * l2_gas_price; + let l1_cost_wei = U256::from(total_l1_data_gas) * l1_base_fee; + let profitable = l2_savings_wei > l1_cost_wei; + + info!( + entries = total_entries, + items = access_list.0.len(), + l1_data_gas = total_l1_data_gas, + l2_savings_wei = %l2_savings_wei, + l1_cost_wei = %l1_cost_wei, + l2_gas_price_gwei = %format_gwei(l2_gas_price), + l1_base_fee_gwei = %format_gwei(l1_base_fee), + profitable, + check = message, + "Access list profitability check" + ); + + profitable +} + /// Filter already-warm and excluded addresses from an access list, then apply /// it to the transaction request. /// @@ -571,4 +505,53 @@ mod tests { let addr = Address::repeat_byte(0xFF); assert_eq!(l1_data_gas_for_bytes(addr.as_slice()), 320); } + + #[test] + fn access_list_rlp_data_gas_uses_exact_eip2930_encoding() { + let access_list = AccessList(vec![AccessListItem { + address: Address::ZERO, + storage_keys: Vec::new(), + }]); + + // RLP([[zero_address, []]]) = d7 d6 94 <20 zero bytes> c0. + // Four non-zero framing bytes cost 64 gas; twenty zero address bytes cost + // 80 gas. The old fixed-overhead approximation returned 192. + assert_eq!(access_list_rlp_data_gas(&access_list), 144); + } + + #[tokio::test] + async fn access_list_profitability_provider_error_returns_err() { + use alloy_network::Ethereum; + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let provider = RootProvider::::new(RpcClient::mocked(Asserter::new())); + let access_list = AccessList(vec![AccessListItem { + address: Address::repeat_byte(0xAA), + storage_keys: Vec::new(), + }]); + + let err = access_list_if_profitable(access_list, &provider) + .await + .expect_err("provider failures must be distinguishable from unprofitable lists"); + assert!( + err.to_string().contains("ArbGasInfo") || err.to_string().contains("provider"), + "unexpected error: {err:#}" + ); + } + + #[tokio::test] + async fn access_list_profitability_empty_list_still_returns_none() { + use alloy_network::Ethereum; + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let provider = RootProvider::::new(RpcClient::mocked(Asserter::new())); + let result = access_list_if_profitable(AccessList::default(), &provider) + .await + .expect("empty list must not query provider"); + assert!(result.is_none()); + } } diff --git a/src/cache/binary_state.rs b/src/cache/binary_state.rs index 1deef61..ccfb3d0 100644 --- a/src/cache/binary_state.rs +++ b/src/cache/binary_state.rs @@ -5,20 +5,25 @@ //! and write a compact binary file. On load, we populate BlockchainDb directly, //! then seed bytecodes from the separate bytecodes.bin cache. //! -//! The file format is raw bincode with no version header or magic bytes, so it -//! is not migratable: a cache written by a build with a different struct layout -//! decodes as a failure (cache miss) rather than being upgraded in place. +//! The file format is a tiny crate-specific envelope (magic bytes + version) +//! followed by bincode payload. Unknown magic/version values are cache misses. use std::path::Path; use std::time::Instant; use alloy_primitives::map::HashMap; use alloy_primitives::{Address, B256, U256}; +use anyhow::{Context as _, Result}; use foundry_fork_db::BlockchainDb; use revm::state::AccountInfo; use serde::{Deserialize, Serialize}; use tracing::{debug, warn}; +use super::versioned; + +const BINARY_STATE_MAGIC: &[u8; 8] = b"EFCSTAT\0"; +const BINARY_STATE_VERSION: u32 = 1; + /// Binary-serializable EVM state. Stores accounts without bytecode (bytecodes /// are loaded separately from bytecodes.bin) and all storage slots. #[derive(Serialize, Deserialize)] @@ -42,16 +47,14 @@ struct BinaryAccountInfo { /// and persisted separately to `bytecodes.bin`; the saved account info keeps /// only the `code_hash`. /// -/// Errors are logged at `warn` level and otherwise swallowed: serialization -/// failures, parent-directory creation failures, and write failures all return -/// without signalling to the caller, so a failed save is indistinguishable from -/// a successful one at the call site. +/// Returns an error if serialization, parent-directory creation, or writing +/// fails, so explicit flush callers can distinguish a successful save from a +/// stale or missing on-disk cache. /// -/// The on-disk format is raw bincode with no version header, so it is not -/// forward/backward compatible: a file written by a build with a different -/// layout will fail to decode on load (treated as a cache miss) rather than +/// The on-disk format carries magic bytes and a version number before the +/// bincode payload. Unknown versions are treated as a cache miss rather than /// being migrated. -pub fn save_binary_state(blockchain_db: &BlockchainDb, path: &Path) { +pub fn save_binary_state(blockchain_db: &BlockchainDb, path: &Path) -> Result<()> { let start = Instant::now(); let accounts: Vec<(Address, BinaryAccountInfo)> = blockchain_db @@ -79,27 +82,28 @@ pub fn save_binary_state(blockchain_db: &BlockchainDb, path: &Path) { let state = BinaryEvmState { accounts, storage }; - match bincode::serialize(&state) { - Ok(data) => { - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - match std::fs::write(path, &data) { - Ok(()) => { - let ms = start.elapsed().as_millis(); - debug!( - accounts = state.accounts.len(), - storage_contracts = state.storage.len(), - bytes = data.len(), - save_ms = ms, - "Saved binary EVM state" - ); - } - Err(e) => warn!(error = %e, "Failed to write binary EVM state"), - } - } - Err(e) => warn!(error = %e, "Failed to serialize binary EVM state"), + let data = versioned::encode( + BINARY_STATE_MAGIC, + BINARY_STATE_VERSION, + &state, + "binary EVM state", + )?; + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create binary EVM state directory {parent:?}"))?; } + std::fs::write(path, &data) + .with_context(|| format!("failed to write binary EVM state to {path:?}"))?; + + let ms = start.elapsed().as_millis(); + debug!( + accounts = state.accounts.len(), + storage_contracts = state.storage.len(), + bytes = data.len(), + save_ms = ms, + "Saved binary EVM state" + ); + Ok(()) } /// Load binary EVM state and populate the BlockchainDb. @@ -109,9 +113,8 @@ pub fn save_binary_state(blockchain_db: &BlockchainDb, path: &Path) { /// Bytecodes should be seeded separately from bytecodes.bin. /// /// Returns `false` (rather than erroring) when `path` cannot be read or its -/// contents fail to decode as the expected bincode layout; a decode failure is -/// logged at `warn` level. Because the format carries no version header, a file -/// written by an incompatible build is reported as a decode failure here. +/// contents fail the magic/version check or fail to decode as the expected +/// bincode layout; failures are logged at `warn` level. pub fn load_binary_state(blockchain_db: &BlockchainDb, path: &Path) -> bool { let start = Instant::now(); @@ -120,12 +123,14 @@ pub fn load_binary_state(blockchain_db: &BlockchainDb, path: &Path) -> bool { Err(_) => return false, }; - let state: BinaryEvmState = match bincode::deserialize(&data) { - Ok(s) => s, - Err(e) => { - warn!(?e, "Failed to decode binary EVM state, starting fresh"); - return false; - } + let Some(state) = versioned::decode::( + &data, + BINARY_STATE_MAGIC, + BINARY_STATE_VERSION, + "binary EVM state", + ) else { + warn!("Failed to decode binary EVM state, starting fresh"); + return false; }; let account_count = state.accounts.len(); @@ -229,8 +234,18 @@ mod tests { } // Save - save_binary_state(&db, &path); + save_binary_state(&db, &path).expect("save binary state"); assert!(path.exists()); + let bytes = std::fs::read(&path).expect("read saved state"); + assert!( + bytes.starts_with(b"EFCSTAT\0"), + "binary state cache must carry a magic header" + ); + assert_eq!( + &bytes[8..12], + &1u32.to_le_bytes(), + "binary state cache must carry an explicit version" + ); // Load into a fresh db let meta2 = BlockchainDbMeta::default(); @@ -265,6 +280,24 @@ mod tests { let _ = std::fs::remove_dir(&dir); } + #[test] + fn save_binary_state_reports_write_failures() { + let dir = std::env::temp_dir().join("evm_fork_cache_test_binary_state_write_error"); + let _ = std::fs::remove_dir_all(&dir); + let _ = std::fs::remove_file(&dir); + std::fs::write(&dir, b"not a directory").expect("create file path conflict"); + + let db = BlockchainDb::new(BlockchainDbMeta::default(), None); + let path = dir.join("state.bin"); + let err = save_binary_state(&db, &path).expect_err("save must report write failure"); + assert!( + err.to_string().contains("directory") || err.to_string().contains("Not a directory"), + "unexpected error: {err:#}" + ); + + let _ = std::fs::remove_file(&dir); + } + #[test] fn test_load_missing_file_returns_false() { let meta = BlockchainDbMeta::default(); @@ -289,4 +322,25 @@ mod tests { let _ = std::fs::remove_file(&path); let _ = std::fs::remove_dir(&dir); } + + #[test] + fn load_legacy_raw_bincode_returns_false() { + let dir = std::env::temp_dir().join("evm_fork_cache_test_binary_state_legacy"); + let path = dir.join("legacy.bin"); + let _ = std::fs::create_dir_all(&dir); + let legacy = BinaryEvmState { + accounts: Vec::new(), + storage: Vec::new(), + }; + std::fs::write(&path, bincode::serialize(&legacy).unwrap()).unwrap(); + + let db = BlockchainDb::new(BlockchainDbMeta::default(), None); + assert!( + !load_binary_state(&db, &path), + "unversioned legacy bincode must be treated as a cache miss" + ); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } } diff --git a/src/cache/bytecode.rs b/src/cache/bytecode.rs index a894a58..1ef00bc 100644 --- a/src/cache/bytecode.rs +++ b/src/cache/bytecode.rs @@ -6,10 +6,9 @@ //! entries are used to re-seed the `code` of accounts that were restored //! without it. //! -//! Each entry's bytes are hex-encoded for the serde representation, but the -//! file is written as raw bincode with no version header, so a cache written by -//! an incompatible build fails to decode (cache miss) rather than being -//! migrated. +//! Each entry's bytes are hex-encoded for the serde representation. The file is +//! written as a crate-specific versioned envelope followed by bincode payload, so +//! incompatible versions are detected as cache misses. use std::collections::HashMap; use std::path::Path; @@ -18,7 +17,11 @@ use alloy_primitives::Address; use anyhow::Result; use foundry_fork_db::BlockchainDb; use serde::{Deserialize, Serialize}; -use tracing::warn; + +use super::versioned; + +const BYTECODE_CACHE_MAGIC: &[u8; 8] = b"EFCBYTE\0"; +const BYTECODE_CACHE_VERSION: u32 = 1; /// Serializable bytecode cache entry. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -38,15 +41,16 @@ pub(crate) struct BytecodeCache { impl BytecodeCache { /// Load bytecode cache from disk (binary format). /// - /// Returns `None` if `path` cannot be read or its contents fail to decode as - /// bincode for this type; a decode failure is logged at `warn` level. The - /// format carries no version header, so a file from an incompatible build is - /// reported as `None`. + /// Returns `None` if `path` cannot be read, fails the magic/version check, or + /// fails to decode as bincode for this type. pub(crate) fn load(path: &Path) -> Option { let data = std::fs::read(path).ok()?; - bincode::deserialize(&data) - .inspect_err(|e| warn!("Failed to parse bytecode cache (bincode): {}", e)) - .ok() + versioned::decode( + &data, + BYTECODE_CACHE_MAGIC, + BYTECODE_CACHE_VERSION, + "bytecode cache", + ) } /// Save bytecode cache to disk (binary format). @@ -62,7 +66,12 @@ impl BytecodeCache { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - let data = bincode::serialize(self)?; + let data = versioned::encode( + BYTECODE_CACHE_MAGIC, + BYTECODE_CACHE_VERSION, + self, + "bytecode cache", + )?; std::fs::write(path, data)?; Ok(()) } @@ -117,6 +126,16 @@ mod tests { }, ); cache.save(&path).expect("save bytecode cache"); + let bytes = std::fs::read(&path).expect("read saved bytecode cache"); + assert!( + bytes.starts_with(b"EFCBYTE\0"), + "bytecode cache must carry a magic header" + ); + assert_eq!( + &bytes[8..12], + &1u32.to_le_bytes(), + "bytecode cache must carry an explicit version" + ); let loaded = BytecodeCache::load(&path).expect("load bytecode cache"); assert_eq!( @@ -128,6 +147,26 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn load_legacy_raw_bincode_is_none() { + let path = temp_path("legacy"); + let mut cache = BytecodeCache::default(); + cache.contracts.insert( + Address::repeat_byte(0x42), + BytecodeCacheEntry { + bytecode: vec![0x60, 0x00], + }, + ); + std::fs::write(&path, bincode::serialize(&cache).unwrap()).expect("write legacy cache"); + + assert!( + BytecodeCache::load(&path).is_none(), + "unversioned legacy bincode must be treated as a cache miss" + ); + + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + #[test] fn load_missing_file_is_none() { assert!(BytecodeCache::load(std::path::Path::new("/nonexistent/bytecodes.bin")).is_none()); diff --git a/src/cache/metadata.rs b/src/cache/metadata.rs index 6ab9472..670de08 100644 --- a/src/cache/metadata.rs +++ b/src/cache/metadata.rs @@ -12,10 +12,14 @@ use std::path::{Path, PathBuf}; use alloy_primitives::{Address, B256, U256}; use anyhow::Result; use serde::{Deserialize, Serialize}; -use tracing::warn; use std::collections::HashSet; +use super::versioned; + +const IMMUTABLE_CACHE_MAGIC: &[u8; 8] = b"EFCMETA\0"; +const IMMUTABLE_CACHE_VERSION: u32 = 1; + /// Configuration for disk-based caching of EVM state. /// /// Enables on-disk persistence of fetched fork state. Cache files are laid out @@ -158,18 +162,17 @@ pub struct ImmutableDataCache { impl ImmutableDataCache { /// Load immutable data cache from disk (binary format). /// - /// Returns `None` if `path` cannot be read or the contents are not valid - /// bincode for this type (a parse failure is logged at `warn` level and - /// swallowed). Callers should treat `None` as "no cache yet" and start fresh. - /// - /// Note: the on-disk format is bincode with no version header, so a cache - /// written by an incompatible build deserializes as a parse failure (`None`) - /// rather than being migrated. + /// Returns `None` if `path` cannot be read, fails the magic/version check, or + /// the payload is not valid bincode for this type. Callers should treat + /// `None` as "no cache yet" and start fresh. pub fn load(path: &Path) -> Option { let data = std::fs::read(path).ok()?; - bincode::deserialize(&data) - .inspect_err(|e| warn!("Failed to parse immutable data cache (bincode): {}", e)) - .ok() + versioned::decode( + &data, + IMMUTABLE_CACHE_MAGIC, + IMMUTABLE_CACHE_VERSION, + "immutable data cache", + ) } /// Save immutable data cache to disk (binary format). @@ -185,7 +188,12 @@ impl ImmutableDataCache { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - let data = bincode::serialize(self)?; + let data = versioned::encode( + IMMUTABLE_CACHE_MAGIC, + IMMUTABLE_CACHE_VERSION, + self, + "immutable data cache", + )?; std::fs::write(path, data)?; Ok(()) } diff --git a/src/cache/mod.rs b/src/cache/mod.rs index ceed40e..5ae04a2 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -8,6 +8,7 @@ pub mod snapshot; mod storage_keys; #[cfg(feature = "protocols")] mod tick_snapshot; +mod versioned; pub use binary_state::{load_binary_state, save_binary_state}; pub use metadata::{ @@ -50,7 +51,7 @@ use alloy_primitives::{Address, B256, Bytes, I256, Log, TxKind, U256, keccak256} use alloy_provider::{Provider, network::AnyNetwork}; use alloy_rpc_types_eth::TransactionRequest; use alloy_sol_types::{SolCall, SolValue, sol}; -use anyhow::{Result, anyhow}; +use anyhow::{Context as _, Result, anyhow}; use foundry_fork_db::{BlockchainDb, SharedBackend, cache::BlockchainDbMeta}; use revm::{ Context, ExecuteCommitEvm, ExecuteEvm, InspectEvm, MainBuilder, MainContext, @@ -67,8 +68,8 @@ use crate::errors::{SimError, SimulationError, SimulationResult}; use crate::freshness::SlotChange; use crate::inspector::TransferInspector; use crate::state_update::{ - AccountChange, AccountPatch, PurgeRecord, PurgeScope, SkippedBalanceDelta, SkippedDelta, - SkippedMask, SlotDelta, StateDiff, StateUpdate, + AccountChange, AccountPatch, PurgeRecord, PurgeScope, SkippedAccountPatch, SkippedBalanceDelta, + SkippedDelta, SkippedMask, SlotDelta, StateDiff, StateUpdate, }; use bytecode::BytecodeCache; @@ -183,6 +184,10 @@ fn write_slot_into( } } +fn account_patch_is_empty(patch: &AccountPatch) -> bool { + patch.balance.is_none() && patch.nonce.is_none() && patch.code.is_none() +} + static CACHE_SPEED_MODE: AtomicU8 = AtomicU8::new(CacheSpeedMode::Slow as u8); /// Runtime tuning profile for cache-side batch storage fetches. @@ -358,7 +363,8 @@ where /// Set how much EVM shared memory to pre-allocate per simulation context. /// - /// Defaults to [`SharedMemoryCapacity::Fixed`]`(64_000)` (today's behavior). + /// Defaults to [`SharedMemoryCapacity::Fixed`] with `64 * 1024` bytes + /// (65,536 bytes). /// Use `Fixed(n)` to pin a size, or [`SharedMemoryCapacity::Auto`] to size it /// from the chain state loaded at [`build`](Self::build) time (e.g. a bincode /// state file supplied via [`cache_config`](Self::cache_config)). See @@ -390,10 +396,10 @@ type InspectorCacheEvm<'a, INSP> = revm::MainnetEvm< >; /// Default initial capacity for the EVM shared-memory (working-memory) buffer. -/// 64 kB, chosen from profiling a state-heavy workload (16x the revm default of -/// 4 kB) so simulations rarely reallocate. Exposed for tuning via +/// 64 KiB (65,536 bytes), chosen from profiling a state-heavy workload (16x the +/// revm default of 4 KiB) so simulations rarely reallocate. Exposed for tuning via /// [`SharedMemoryCapacity`]. -const DEFAULT_SHARED_MEMORY_CAPACITY: usize = 64_000; +const DEFAULT_SHARED_MEMORY_CAPACITY: usize = 64 * 1024; /// How much EVM shared memory (per-context working memory) to pre-allocate for /// simulations. @@ -405,11 +411,12 @@ const DEFAULT_SHARED_MEMORY_CAPACITY: usize = 64_000; /// this much memory per overlay, so general users may want a smaller `Fixed` size, /// while state-heavy users can raise it or let it auto-size from the loaded state. /// -/// The default is `Fixed(64_000)` (today's behavior). Configure it on +/// The default is `Fixed(64 * 1024)` (65,536 bytes). Configure it on /// [`EvmCacheBuilder::shared_memory_capacity`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SharedMemoryCapacity { - /// Pre-allocate exactly this many bytes. The [`Default`] is `Fixed(64_000)`. + /// Pre-allocate exactly this many bytes. The [`Default`] is + /// `Fixed(64 * 1024)`. Fixed(usize), /// Size the buffer from the amount of chain state loaded into the cache at /// construction (e.g. from a bincode state file via @@ -429,7 +436,8 @@ impl Default for SharedMemoryCapacity { } impl SharedMemoryCapacity { - /// Floor for [`Auto`](Self::Auto) (and the default fixed size): 64 kB. + /// Floor for [`Auto`](Self::Auto) (and the default fixed size): 64 KiB + /// (65,536 bytes). pub const MIN_AUTO: usize = DEFAULT_SHARED_MEMORY_CAPACITY; /// Ceiling for [`Auto`](Self::Auto): 4 MiB. A simulation that needs more than /// this still works — revm grows the buffer past it on demand. @@ -1147,56 +1155,59 @@ impl EvmCache { /// /// Call this after loading AMMs and running simulations to speed up subsequent runs. /// The cache is also automatically flushed when the EvmCache is dropped. - pub fn flush(&self) { + pub fn flush(&self) -> Result<()> { if let Some(cfg) = &self.cache_config { // Save EVM state to binary cache (bincode format) let binary_path = cfg.binary_state_cache_path(); - binary_state::save_binary_state(&self.blockchain_db, &binary_path); + binary_state::save_binary_state(&self.blockchain_db, &binary_path) + .with_context(|| format!("failed to save binary state cache to {binary_path:?}"))?; // Save bytecode cache let bytecode_path = cfg.bytecode_cache_path(); let mut bytecode_cache = BytecodeCache::load(&bytecode_path).unwrap_or_default(); bytecode_cache.merge_from_db(&self.blockchain_db); - if let Err(e) = bytecode_cache.save(&bytecode_path) { - warn!(error = %e, "Failed to save bytecode cache"); - } else { - debug!( - count = bytecode_cache.contracts.len(), - path = ?bytecode_path, - "Updated bytecode cache (binary format)" - ); - } + bytecode_cache + .save(&bytecode_path) + .with_context(|| format!("failed to save bytecode cache to {bytecode_path:?}"))?; + debug!( + count = bytecode_cache.contracts.len(), + path = ?bytecode_path, + "Updated bytecode cache (binary format)" + ); // Save the immutable data cache let immutable_path = cfg.immutable_cache_path(); - if let Err(e) = self.immutable_cache.save(&immutable_path) { - warn!(error = %e, "Failed to save immutable data cache"); - } else { - debug!( - token_decimals = self.immutable_cache.token_decimals.len(), - v2_pools = self.immutable_cache.v2_pools.len(), - v3_pools = self.immutable_cache.v3_pools.len(), - balancer_pools = self.immutable_cache.balancer_pools.len(), - path = ?immutable_path, - "Updated immutable data cache" - ); - } + self.immutable_cache + .save(&immutable_path) + .with_context(|| { + format!("failed to save immutable data cache to {immutable_path:?}") + })?; + debug!( + token_decimals = self.immutable_cache.token_decimals.len(), + v2_pools = self.immutable_cache.v2_pools.len(), + v3_pools = self.immutable_cache.v3_pools.len(), + balancer_pools = self.immutable_cache.balancer_pools.len(), + path = ?immutable_path, + "Updated immutable data cache" + ); // Save the V3 tick snapshot cache (needed for liquidity validation) #[cfg(feature = "protocols")] { let tick_snapshot_path = cfg.tick_snapshot_cache_path(); - if let Err(e) = self.tick_snapshot_cache.save(&tick_snapshot_path) { - warn!(error = %e, "Failed to save V3 tick snapshot cache"); - } else { - debug!( - snapshots = self.tick_snapshot_cache.len(), - path = ?tick_snapshot_path, - "Updated V3 tick snapshot cache" - ); - } + self.tick_snapshot_cache + .save(&tick_snapshot_path) + .with_context(|| { + format!("failed to save V3 tick snapshot cache to {tick_snapshot_path:?}") + })?; + debug!( + snapshots = self.tick_snapshot_cache.len(), + path = ?tick_snapshot_path, + "Updated V3 tick snapshot cache" + ); } } + Ok(()) } /// Get the cache configuration, if any. @@ -1208,46 +1219,73 @@ impl EvmCache { self.cache_config.as_ref() } - /// Get a reference to the underlying [`BlockchainDb`] (the layer-2 backend - /// store of accounts, storage, and bytecodes). + /// Run a synchronous direct mutation against the underlying [`BlockchainDb`] + /// and invalidate the memoized snapshot base afterwards. + /// + /// This is the preferred escape hatch for unavoidable layer-2 map writes such + /// as `accounts().write().insert(...)` or `storage().write().insert(...)`. + /// The closure still bypasses the CacheDB overlay and the normal write funnel, + /// so use higher-level mutators when they can express the change. Unlike + /// [`unchecked_blockchain_db`](Self::unchecked_blockchain_db), this wrapper + /// keeps the copy-on-write snapshot base honest automatically after in-place + /// overwrites whose map cardinality does not change. + pub fn with_blockchain_db_mut(&mut self, f: impl FnOnce(&BlockchainDb) -> R) -> R { + let result = f(&self.blockchain_db); + self.invalidate_base(); + result + } + + /// Get an unchecked reference to the underlying [`BlockchainDb`] (the layer-2 + /// backend store of accounts, storage, and bytecodes). /// /// This exposes an internal store and bypasses the cache's two-layer - /// consistency model: reads here see only the backend layer, not the - /// CacheDB overlay, and any writes performed through it skip the overlay. - /// Prefer the higher-level accessors; use with care. + /// consistency model: reads here see only the backend layer, not the CacheDB + /// overlay, and any writes performed through it skip the overlay. Prefer + /// higher-level accessors or [`with_blockchain_db_mut`](Self::with_blockchain_db_mut) + /// for direct synchronous writes. /// /// # Snapshot base - /// Writing layer 2 directly through this handle also bypasses the memoized - /// copy-on-write snapshot base (Pillar A): an **in-place value overwrite at an - /// unchanged slot count** is invisible to the [`create_snapshot`](Self::create_snapshot) - /// growth scan (which is count/absence-based — the lazily-fetched backend only - /// ever *appends*, so that is sufficient for the supported write paths), and a - /// later `create_snapshot` may reuse a stale base. After a direct layer-2 write - /// through this handle, call + /// Writing layer 2 directly through this unchecked handle also bypasses the + /// memoized copy-on-write snapshot base (Pillar A). The next + /// [`create_snapshot`](Self::create_snapshot) only performs a count/absence + /// growth scan over layer 2, which catches lazy RPC-populated accounts/slots + /// because that path only appends at a fixed block. It does **not** catch + /// direct in-place changes where cardinality is unchanged: overwriting an + /// existing storage slot, or changing an existing account's info/code/balance + /// without adding a new account, can leave a stale snapshot base. After such a + /// direct write, call /// [`invalidate_snapshot_base`](Self::invalidate_snapshot_base) (or re-pin via /// [`set_block`](Self::set_block)) before the next snapshot. Writes via the /// crate's own mutators (`inject_storage_batch`, `apply_update`, the `inject_*` /// helpers, the purges) keep the base honest automatically. - pub fn blockchain_db(&self) -> &BlockchainDb { + pub fn unchecked_blockchain_db(&self) -> &BlockchainDb { &self.blockchain_db } - /// Get a reference to the underlying [`SharedBackend`] (the lazy RPC-backed - /// fetcher shared across clones). + /// Get an unchecked reference to the underlying [`SharedBackend`] (the lazy + /// RPC-backed fetcher shared across clones). /// - /// This exposes an internal and bypasses the cache's two-layer consistency + /// This exposes an internal handle and bypasses the cache's two-layer consistency /// model: it reads/fetches directly without consulting the CacheDB overlay. /// Prefer the higher-level accessors; use with care. /// /// # Snapshot base - /// `SharedBackend::insert_or_update_storage` / `insert_or_update_address` rewrite - /// layer-2 entries **in place**, which (unlike the append-only lazy fetch) can - /// leave the memoized copy-on-write snapshot base stale at an unchanged slot - /// count. After such a direct write, call + /// Lazy RPC fetches through this backend only append missing accounts/slots at + /// the pinned block, so the snapshot growth scan catches them without an + /// explicit invalidation. Direct `SharedBackend::insert_or_update_storage` / + /// `insert_or_update_address` calls are different: they enqueue a background + /// handler request that can rewrite layer-2 entries **in place**, leaving the + /// memoized copy-on-write base stale at an unchanged slot/account count. + /// + /// If you use those helpers directly, first synchronize with the backend + /// handler by reading back the updated account/slot through `SharedBackend` + /// (for example via `basic_ref` / `storage_ref`), then call /// [`invalidate_snapshot_base`](Self::invalidate_snapshot_base) before the next - /// [`create_snapshot`](Self::create_snapshot). The lazy RPC fetch path needs no - /// such call (it only ever appends, which the snapshot growth scan catches). - pub fn backend(&self) -> &SharedBackend { + /// [`create_snapshot`](Self::create_snapshot). Calling + /// `invalidate_snapshot_base` immediately after `insert_or_update_*` is not, by + /// itself, a guarantee that the queued update has been applied before the next + /// snapshot. + pub fn unchecked_backend(&self) -> &SharedBackend { &self.backend } @@ -1376,28 +1414,23 @@ impl EvmCache { /// layers (no RPC), apply each `Some` patch field (recomputing the code hash /// when `code` is set), then write through with the same layer policy. /// Records an [`AccountChange`] with `Some((old, new))` only for fields - /// that changed. + /// that changed. If the account is cold (absent from both layers), apply + /// nothing and surface a [`SkippedAccountPatch`] in + /// `diff.skipped_accounts`. + /// - [`StateUpdate::AccountUpsert`] — same patch semantics, but intentionally + /// materializes a cold/default account when absent from both layers. /// - [`StateUpdate::Purge`] — dispatch to the matching purge layer logic and /// record a [`PurgeRecord`]. /// /// # Warning — relative updates can be skipped /// - /// A relative [`SlotDelta`](StateUpdate::SlotDelta) / - /// [`BalanceDelta`](StateUpdate::BalanceDelta) targeting a **cold** address is - /// *dropped, not applied* (applying it against an unknown base would corrupt - /// state). Because a skip produces no change, it is invisible to the - /// changes-only [`StateDiff::is_empty`] / [`StateDiff::len`] success check, so - /// after applying relative updates the caller **must** inspect - /// [`StateDiff::has_skipped`] (or `diff.skipped` / `diff.skipped_balances`) and - /// fetch+seed the cold target — a silently-dropped balance update can break - /// conservation. - /// - /// # Warning — cold absolute `Account` patches - /// - /// A partial absolute [`StateUpdate::Account`] patch on an address absent from - /// both layers writes default nonce/code through the backend as authoritative, - /// masking a real RPC fetch. Fetch+seed the account first, or use - /// [`StateUpdate::BalanceDelta`] for relative native-balance tracking. + /// A cold-aware update targeting a **cold** address is *dropped, not applied* + /// unless it is an explicit [`StateUpdate::AccountUpsert`]. Because a skip + /// produces no change, it is invisible to the changes-only + /// [`StateDiff::is_empty`] / [`StateDiff::len`] success check, so after + /// applying cold-aware updates the caller **must** inspect + /// [`StateDiff::has_skipped`] (or the `skipped_*` fields) and fetch+seed the + /// cold target. /// /// ```no_run /// # use alloy_primitives::{Address, U256}; @@ -1489,7 +1522,17 @@ impl EvmCache { } } StateUpdate::Account { address, patch } => { - if let Some(change) = self.apply_account_patch(*address, patch) { + match self.apply_account_patch(*address, patch, false) { + Ok(Some(change)) => diff.accounts.push(change), + Ok(None) => {} + Err(skipped) => diff.skipped_accounts.push(skipped), + } + } + StateUpdate::AccountUpsert { address, patch } => { + if let Some(change) = self + .apply_account_patch(*address, patch, true) + .expect("AccountUpsert never skips cold account patches") + { diff.accounts.push(change); } } @@ -1872,10 +1915,22 @@ impl EvmCache { &mut self, address: Address, patch: &AccountPatch, - ) -> Option { - // 1. Current info from the cached layers only (overlay ▸ backend ▸ - // default). No RPC: apply is a write, not a fetch. - let mut info = self.loaded_account_info(address).unwrap_or_default(); + allow_cold_upsert: bool, + ) -> std::result::Result, SkippedAccountPatch> { + // 1. Current info from the cached layers only (overlay ▸ backend). No RPC: + // apply is a write, not a fetch. A partial patch on a cold account is + // skipped unless the caller explicitly chose AccountUpsert. + let mut info = match self.loaded_account_info(address) { + Some(info) => info, + None if account_patch_is_empty(patch) => return Ok(None), + None if allow_cold_upsert => AccountInfo::default(), + None => { + return Err(SkippedAccountPatch { + address, + patch: patch.clone(), + }); + } + }; let old_balance = info.balance; let old_nonce = info.nonce; @@ -1906,14 +1961,14 @@ impl EvmCache { code_hash: (old_code_hash != info.code_hash).then_some((old_code_hash, info.code_hash)), }; if change.balance.is_none() && change.nonce.is_none() && change.code_hash.is_none() { - return None; + return Ok(None); } // 4. Write-through, mirroring the slot policy: backend always; overlay // only if an overlay account already exists (do not materialize one). self.write_account_info_through(address, info); - Some(change) + Ok(Some(change)) } /// Dispatch a [`PurgeScope`] to the matching layer logic (§5.3), returning a @@ -2283,14 +2338,23 @@ impl EvmCache { /// /// The crate's own mutators keep the base honest automatically. This is the /// **escape-hatch re-honest hook**: call it after writing layer 2 directly - /// through [`blockchain_db`](Self::blockchain_db) or - /// [`backend`](Self::backend) — those bypass the write funnel, and an in-place - /// value overwrite at an unchanged slot count is invisible to the snapshot - /// growth scan (it is count/absence-based, which suffices for the append-only - /// lazy-fetch path but not for an out-of-band overwrite). Calling this before - /// the next snapshot guarantees it reflects the direct write rather than a - /// stale memoized value. Over-invalidation is always safe (Decision D2); the - /// only cost is one full base rebuild on the next snapshot. + /// through [`unchecked_blockchain_db`](Self::unchecked_blockchain_db) or + /// [`unchecked_backend`](Self::unchecked_backend) — those bypass the write + /// funnel, and in-place changes at unchanged cardinality are invisible to the + /// snapshot growth scan. + /// That includes overwriting an existing storage slot and changing an existing + /// account's info/code/balance without adding a new account. Lazy RPC-populated + /// data does not need this call because it only appends accounts/slots, which + /// the growth scan catches. + /// + /// When using `SharedBackend::insert_or_update_*` through + /// [`unchecked_backend`](Self::unchecked_backend), remember those helpers only + /// enqueue a background update. Synchronize/read back the update through + /// `SharedBackend` before the next snapshot; `invalidate_snapshot_base` alone + /// is not a backend-handler synchronization point. Once the direct write is + /// present, calling this before the next snapshot guarantees it reflects that + /// write rather than a stale memoized value. Over-invalidation is always safe + /// (Decision D2); the only cost is one full base rebuild on the next snapshot. pub fn invalidate_snapshot_base(&mut self) { self.invalidate_base(); } @@ -2337,7 +2401,7 @@ impl EvmCache { // only add a new account (caught by the absence check) or a new slot (caught // by the count check). An in-place value overwrite at unchanged length is // invisible here; the controlled writers therefore call `mark_base_dirty` - // explicitly, and a direct out-of-band write via `blockchain_db()`/`backend()` + // explicitly, and a direct out-of-band write via `unchecked_blockchain_db()`/`unchecked_backend()` // must call `invalidate_snapshot_base`. If a future foundry-fork-db bump makes // the lazy path overwrite-in-place, this scan must gain a value/version check. { @@ -2598,18 +2662,24 @@ impl EvmCache { /// To prevent the EVM block context from silently diverging from the pinned /// block, when `block` is a concrete `BlockId::Number(Number(n))` this also /// updates `block_number` (the `NUMBER` opcode) to `n`. For tag-based block - /// ids (`latest`, `pending`, hashes, etc.) the height is not statically known, - /// so `block_number` is left unchanged. - /// - /// `basefee` (the `BASEFEE` opcode) is **not** refreshed here because deriving - /// it requires fetching the block header, which this synchronous method cannot - /// do. Callers that change blocks should refresh it via - /// [`set_block_context`](Self::set_block_context) (e.g. after fetching the new - /// header). Prefer [`repin_to_block`](Self::repin_to_block) when re-pinning to + /// ids (`latest`, `pending`, hashes, etc.) and `None`, the height is not + /// statically known, so `block_number` is cleared. + /// + /// `basefee` (the `BASEFEE` opcode) is **cleared on every block change** and + /// on every non-concrete tag/hash/`None` pin call because deriving it requires + /// fetching the block header, which this synchronous method cannot do. Callers + /// that change blocks should refresh it via + /// [`set_block_context`](Self::set_block_context) after fetching the new + /// header. Prefer [`repin_to_block`](Self::repin_to_block) when re-pinning to /// a concrete height, since it keeps `block_number` and the pinned block in /// lockstep. pub fn set_block(&mut self, block: Option) { - if self.block != block { + let changed = self.block != block; + let concrete_number = match block { + Some(BlockId::Number(BlockNumberOrTag::Number(n))) => Some(n), + _ => None, + }; + if changed { self.block = block; // Re-pinning replaces layer 2 wholesale (state at a new block): the // memoized base must be rebuilt from scratch on the next snapshot. @@ -2617,14 +2687,16 @@ impl EvmCache { if let Some(block_id) = block { let _ = self.backend.set_pinned_block(block_id); *self.batch_block_id.lock().unwrap() = block_id; - // Keep the EVM `NUMBER` opcode aligned with the pinned block so the - // two cannot silently diverge. Only a concrete height is meaningful; - // tags (latest/pending/hash) leave `block_number` untouched. - if let BlockId::Number(BlockNumberOrTag::Number(n)) = block_id { - self.block_number = Some(n); - } } } + if changed || concrete_number.is_none() { + self.basefee = None; + } + + // Keep the EVM `NUMBER` opcode aligned with the pin. Only a concrete + // height is meaningful; tags, hashes, and no explicit pin clear it so a + // stale number from an earlier concrete block cannot leak into simulation. + self.block_number = concrete_number; } /// Get the block that RPC fetches are currently pinned to. @@ -2656,9 +2728,10 @@ impl EvmCache { /// Get the block number used for EVM simulations (the `NUMBER` opcode). /// - /// Fetched from the pinned block's header at construction and kept in - /// lockstep with the pin by [`set_block`](Self::set_block) / - /// [`repin_to_block`](Self::repin_to_block). `None` means revm falls back + /// Fetched from the pinned block's header at construction. Concrete-number + /// pins set it via [`set_block`](Self::set_block) / + /// [`repin_to_block`](Self::repin_to_block); tag/hash/`None` pins clear it + /// because their height is not statically known. `None` means revm falls back /// to `0`, which can steer contracts that branch on `block.number` down a /// different code path. Override directly via /// [`set_block_context`](Self::set_block_context). @@ -2669,10 +2742,12 @@ impl EvmCache { /// Get the base fee per gas used for EVM simulations (the `BASEFEE` opcode). /// /// Fetched from the pinned block's header at construction. `None` means - /// revm falls back to `0`. Unlike `block_number` this is **not** refreshed - /// by [`set_block`](Self::set_block); refresh it with - /// [`set_block_context`](Self::set_block_context) after fetching a new - /// header if `BASEFEE` accuracy matters. + /// revm falls back to `0`. This is cleared by [`set_block`](Self::set_block) + /// / [`repin_to_block`](Self::repin_to_block) when the pin changes, and by + /// non-concrete tag/hash/`None` pin calls because those can drift without a + /// concrete number in the API. Refresh it with + /// [`set_block_context`](Self::set_block_context) after fetching a new header + /// if `BASEFEE` accuracy matters. pub fn basefee(&self) -> Option { self.basefee } @@ -2717,16 +2792,12 @@ impl EvmCache { /// /// Updates the SharedBackend pinned block, the batch fetcher block, and the /// EVM block context (`NUMBER` opcode) in lockstep. The current `basefee` is - /// preserved; callers should refresh it via - /// [`set_block_context`](Self::set_block_context) after fetching the new + /// cleared because it cannot be refreshed synchronously; callers should set it + /// via [`set_block_context`](Self::set_block_context) after fetching the new /// block header if `BASEFEE` accuracy matters. pub fn repin_to_block(&mut self, block_number: u64) { let old_block = self.block; - // `set_block` already updates `block_number` for a concrete height; the - // explicit `set_block_context` below preserves `basefee` and keeps the - // re-pin atomic and self-documenting. self.set_block(Some(BlockId::Number(block_number.into()))); - self.set_block_context(Some(block_number), self.basefee); if let Some(BlockId::Number(BlockNumberOrTag::Number(old_num))) = old_block { let drift = block_number.saturating_sub(old_num); @@ -3719,8 +3790,9 @@ impl EvmCache { /// reverted. Unlike /// [`simulate_with_transfer_tracking`](Self::simulate_with_transfer_tracking), /// this measures deltas via pre/post balance reads (not transfer-event - /// inspection) and the returned - /// [`access_list`](CallSimulationResult::access_list) is always empty. + /// inspection). The returned [`access_list`](CallSimulationResult::access_list) + /// includes the accounts and slots touched by the pre/post `balanceOf` reads + /// and the simulated call. /// /// # Errors /// Returns an error if building the tx env fails, if a pre/post @@ -3768,11 +3840,13 @@ impl EvmCache { token_deltas.insert(*token, I256::from_raw(post) - I256::from_raw(pre)); } - Ok((gas_used, token_deltas, logs, output)) + let access_list = extract_access_list(&evm.journaled_state.state); + + Ok((gas_used, token_deltas, logs, output, access_list)) })(); match result { - Ok((gas_used, token_deltas, logs, output)) => { + Ok((gas_used, token_deltas, logs, output, access_list)) => { if commit { evm.commit_inner(); } else { @@ -3783,7 +3857,7 @@ impl EvmCache { gas_used, token_deltas, logs, - access_list: AccessList::default(), + access_list, output, }) } @@ -4320,7 +4394,9 @@ impl Drop for EvmCache { fn drop(&mut self) { if self.cache_config.is_some() { debug!("Flushing EVM cache on drop"); - self.flush(); + if let Err(e) = self.flush() { + warn!(error = %e, "Failed to flush EVM cache on drop"); + } } } } @@ -4352,7 +4428,7 @@ mod shared_memory_capacity_tests { #[test] fn default_is_fixed_64k() { - assert_eq!(Cap::default(), Cap::Fixed(64_000)); + assert_eq!(Cap::default(), Cap::Fixed(64 * 1024)); } #[test] @@ -4365,7 +4441,7 @@ mod shared_memory_capacity_tests { fn auto_floors_clamps_and_scales() { // Nothing / little loaded → floor. assert_eq!(Cap::Auto.resolve(0), Cap::MIN_AUTO); - assert_eq!(Cap::Auto.resolve(1_000), Cap::MIN_AUTO); // 16 KB < 64 KB floor + assert_eq!(Cap::Auto.resolve(1_000), Cap::MIN_AUTO); // 16 KiB < 64 KiB floor // Linear region (16 bytes/slot). assert_eq!(Cap::Auto.resolve(10_000), 160_000); assert_eq!(Cap::Auto.resolve(100_000), 1_600_000); @@ -4863,6 +4939,126 @@ mod core_tests { assert_eq!(cache.basefee(), None); } + #[test] + fn set_block_latest_clears_stale_block_context() { + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let asserter = Asserter::new(); + let client = RpcClient::mocked(asserter); + let provider = RootProvider::::new(client); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + + let mut cache = rt.block_on(EvmCache::new(Arc::new(provider), None)); + cache.set_block_context(Some(148_252_680), Some(50)); + + cache.set_block(Some(BlockId::latest())); + + assert_eq!( + cache.block_number(), + None, + "tag pins must not retain a stale NUMBER context" + ); + assert_eq!( + cache.basefee(), + None, + "set_block cannot refresh BASEFEE synchronously, so it must clear stale values" + ); + } + + #[test] + fn set_block_none_clears_stale_context_even_when_pin_unchanged() { + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let asserter = Asserter::new(); + let client = RpcClient::mocked(asserter); + let provider = RootProvider::::new(client); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + + let mut cache = rt.block_on(EvmCache::new(Arc::new(provider), None)); + cache.set_block_context(Some(148_252_680), Some(50)); + + cache.set_block(None); + + assert_eq!( + cache.block_number(), + None, + "None pins must not retain a stale NUMBER context" + ); + assert_eq!( + cache.basefee(), + None, + "None pins can drift like tags, so stale BASEFEE must be cleared" + ); + } + + #[test] + fn set_block_number_sets_number_and_clears_stale_basefee() { + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let asserter = Asserter::new(); + let client = RpcClient::mocked(asserter); + let provider = RootProvider::::new(client); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + + let mut cache = rt.block_on(EvmCache::new(Arc::new(provider), None)); + cache.set_block_context(Some(100), Some(50)); + + cache.set_block(Some(BlockId::Number(BlockNumberOrTag::Number(200)))); + + assert_eq!(cache.block_number(), Some(200)); + assert_eq!( + cache.basefee(), + None, + "set_block cannot refresh BASEFEE synchronously, so it must clear stale values" + ); + } + + #[test] + fn repin_to_block_clears_stale_basefee() { + use alloy_provider::RootProvider; + use alloy_rpc_client::RpcClient; + use alloy_transport::mock::Asserter; + + let asserter = Asserter::new(); + let client = RpcClient::mocked(asserter); + let provider = RootProvider::::new(client); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + + let mut cache = rt.block_on(EvmCache::new(Arc::new(provider), None)); + cache.set_block_context(Some(100), Some(50)); + + cache.repin_to_block(200); + + assert_eq!(cache.block_number(), Some(200)); + assert_eq!( + cache.basefee(), + None, + "repin_to_block must not carry stale BASEFEE across blocks" + ); + } + #[test] fn test_build_evm_applies_block_context() { use alloy_provider::RootProvider; @@ -4918,8 +5114,8 @@ mod core_tests { let block_num = Some(148_252_680u64); let basefee_val = Some(50u64); let child = EvmCache::from_backend( - parent.backend().clone(), - parent.blockchain_db().clone(), + parent.unchecked_backend().clone(), + parent.unchecked_blockchain_db().clone(), parent.block(), 42161, block_num, diff --git a/src/cache/tick_snapshot.rs b/src/cache/tick_snapshot.rs index e5f6f5a..145f7b2 100644 --- a/src/cache/tick_snapshot.rs +++ b/src/cache/tick_snapshot.rs @@ -12,7 +12,11 @@ use std::path::Path; use alloy_primitives::{Address, U256}; use anyhow::Result; use serde::{Deserialize, Serialize}; -use tracing::warn; + +use super::versioned; + +const TICK_SNAPSHOT_CACHE_MAGIC: &[u8; 8] = b"EFCTICK\0"; +const TICK_SNAPSHOT_CACHE_VERSION: u32 = 1; /// Per-tick liquidity state for a UniswapV3-style concentrated-liquidity pool. /// @@ -151,15 +155,16 @@ pub struct V3TickSnapshotCache { impl V3TickSnapshotCache { /// Load tick snapshot cache from disk (binary format). /// - /// Returns `None` if `path` cannot be read or its contents fail to decode as - /// bincode for this type; a decode failure is logged at `warn` level and - /// treated as a cache miss. The format has no version header, so a file from - /// an incompatible build also yields `None`. + /// Returns `None` if `path` cannot be read, fails the magic/version check, or + /// fails to decode as bincode for this type. pub fn load(path: &Path) -> Option { let data = std::fs::read(path).ok()?; - bincode::deserialize(&data) - .inspect_err(|e| warn!("Failed to parse V3 tick snapshot cache (bincode): {}", e)) - .ok() + versioned::decode( + &data, + TICK_SNAPSHOT_CACHE_MAGIC, + TICK_SNAPSHOT_CACHE_VERSION, + "V3 tick snapshot cache", + ) } /// Save tick snapshot cache to disk (binary format). @@ -175,7 +180,12 @@ impl V3TickSnapshotCache { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - let data = bincode::serialize(self)?; + let data = versioned::encode( + TICK_SNAPSHOT_CACHE_MAGIC, + TICK_SNAPSHOT_CACHE_VERSION, + self, + "V3 tick snapshot cache", + )?; std::fs::write(path, data)?; Ok(()) } diff --git a/src/cache/versioned.rs b/src/cache/versioned.rs new file mode 100644 index 0000000..0ed9681 --- /dev/null +++ b/src/cache/versioned.rs @@ -0,0 +1,66 @@ +use serde::{Serialize, de::DeserializeOwned}; +use tracing::warn; + +use anyhow::{Context as _, Result}; + +const VERSION_BYTES: usize = 4; + +pub(crate) fn encode( + magic: &[u8; 8], + version: u32, + value: &T, + label: &'static str, +) -> Result> { + let payload = + bincode::serialize(value).with_context(|| format!("failed to serialize {label}"))?; + let mut data = Vec::with_capacity(magic.len() + VERSION_BYTES + payload.len()); + data.extend_from_slice(magic); + data.extend_from_slice(&version.to_le_bytes()); + data.extend_from_slice(&payload); + Ok(data) +} + +pub(crate) fn decode( + data: &[u8], + magic: &[u8; 8], + version: u32, + label: &'static str, +) -> Option { + let header_len = magic.len() + VERSION_BYTES; + if data.len() < header_len { + warn!( + cache = label, + bytes = data.len(), + "Cache file is missing version header; treating as cache miss" + ); + return None; + } + + if &data[..magic.len()] != magic { + warn!( + cache = label, + "Cache file has unrecognized magic header; treating as cache miss" + ); + return None; + } + + let version_start = magic.len(); + let found_version = u32::from_le_bytes( + data[version_start..header_len] + .try_into() + .expect("version slice length is fixed"), + ); + if found_version != version { + warn!( + cache = label, + expected = version, + found = found_version, + "Cache file version mismatch; treating as cache miss" + ); + return None; + } + + bincode::deserialize(&data[header_len..]) + .inspect_err(|e| warn!(cache = label, error = %e, "Failed to parse cache payload")) + .ok() +} diff --git a/src/lib.rs b/src/lib.rs index 15406b8..f138977 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,6 +128,6 @@ pub use freshness::{ SpeculativeSim, Validation, Validity, WallClock, }; pub use state_update::{ - AccountChange, AccountPatch, PurgeRecord, PurgeScope, SkippedBalanceDelta, SkippedDelta, - SkippedMask, SlotDelta, StateDiff, StateUpdate, + AccountChange, AccountPatch, PurgeRecord, PurgeScope, SkippedAccountPatch, SkippedBalanceDelta, + SkippedDelta, SkippedMask, SlotDelta, StateDiff, StateUpdate, }; diff --git a/src/prefetch_registry.rs b/src/prefetch_registry.rs index ba5ab86..04fa1ab 100644 --- a/src/prefetch_registry.rs +++ b/src/prefetch_registry.rs @@ -16,6 +16,7 @@ use std::collections::{HashMap, HashSet}; use std::path::Path; use alloy_primitives::{Address, U256}; +use anyhow::{Context as _, Result}; use serde::{Deserialize, Serialize}; use tracing::{debug, info, warn}; @@ -80,36 +81,27 @@ impl PrefetchRegistry { /// Persist the registry to `path` in bincode format, creating parent /// directories as needed. /// - /// This is best-effort: I/O and serialization failures (unwritable parent - /// directory, failed write, or a serialization error) are logged at `warn` - /// and swallowed rather than returned, so a save failure leaves stale or - /// missing on-disk data that [`load`](Self::load) will silently treat as an - /// empty registry on the next cycle. - pub fn save(&self, path: &Path) { - if let Some(parent) = path.parent() - && let Err(e) = std::fs::create_dir_all(parent) - { - warn!(error = %e, "Failed to create prefetch registry directory"); - return; - } - match bincode::serialize(self) { - Ok(data) => { - if let Err(e) = std::fs::write(path, data) { - warn!(error = %e, "Failed to persist prefetch registry"); - } else { - let total_slots: usize = - self.phases.values().map(|al| al.slots.len()).sum::() - + self - .keyed_phases - .values() - .flat_map(|m| m.values()) - .map(|al| al.slots.len()) - .sum::(); - debug!(total_slots, "Saved prefetch registry"); - } - } - Err(e) => warn!(error = %e, "Failed to serialize prefetch registry"), + /// Returns an error if the parent directory cannot be created, serialization + /// fails, or the write fails. + pub fn save(&self, path: &Path) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("failed to create prefetch registry directory {parent:?}") + })?; } + let data = bincode::serialize(self).context("failed to serialize prefetch registry")?; + std::fs::write(path, data) + .with_context(|| format!("failed to persist prefetch registry to {path:?}"))?; + + let total_slots: usize = self.phases.values().map(|al| al.slots.len()).sum::() + + self + .keyed_phases + .values() + .flat_map(|m| m.values()) + .map(|al| al.slots.len()) + .sum::(); + debug!(total_slots, "Saved prefetch registry"); + Ok(()) } /// Record the aggregated access list for `phase`, **overwriting** any access @@ -352,7 +344,7 @@ mod tests { sal.slots.insert((key, U256::from(99))); registry.record_keyed("per_target", key, sal); - registry.save(&path); + registry.save(&path).expect("save registry"); let loaded = PrefetchRegistry::load(&path); assert_eq!(loaded.phases.len(), 1); @@ -374,6 +366,26 @@ mod tests { let _ = std::fs::remove_dir(&dir); } + #[test] + fn save_reports_write_failures() { + let dir = std::env::temp_dir().join("evm_fork_cache_test_prefetch_registry_write_error"); + let _ = std::fs::remove_dir_all(&dir); + let _ = std::fs::remove_file(&dir); + std::fs::write(&dir, b"not a directory").expect("create file path conflict"); + + let registry = PrefetchRegistry::default(); + let path = dir.join("registry.bin"); + let err = registry + .save(&path) + .expect_err("save must report write failure"); + assert!( + err.to_string().contains("directory") || err.to_string().contains("Not a directory"), + "unexpected error: {err:#}" + ); + + let _ = std::fs::remove_file(&dir); + } + #[test] fn test_load_missing_file_returns_default() { let path = std::path::Path::new("/tmp/nonexistent_prefetch_registry.bin"); diff --git a/src/state_update.rs b/src/state_update.rs index 165bff5..2641023 100644 --- a/src/state_update.rs +++ b/src/state_update.rs @@ -15,23 +15,31 @@ //! - [`StateUpdate::Slot`] — set a single storage slot, authoritative across //! both cache layers. //! - [`StateUpdate::Account`] — apply a partial [`AccountPatch`] -//! (`balance`/`nonce`/`code`, each optional). +//! (`balance`/`nonce`/`code`, each optional) to an already-known account. +//! - [`StateUpdate::AccountUpsert`] — intentionally materialize a cold account +//! from a partial [`AccountPatch`]. //! - [`StateUpdate::Purge`] — drop cached state at a [`PurgeScope`] so the next //! read re-fetches. //! //! # The dual-layer write-through policy //! -//! [`apply_update`](crate::cache::EvmCache::apply_update) applies a `Slot` or -//! `Account` write-through with one consistent rule: the BlockchainDb backend -//! (layer 2) is written **always**; the CacheDB overlay (layer 1) is written -//! **only if an overlay account already exists** for the address. A new overlay -//! account is never materialized for a slot/account write — the read path falls -//! through to the backend for an absent overlay entry, so a backend-only write -//! is authoritative, and materializing an overlay entry would pollute layer 1 -//! and could shadow later RPC reads. (This mirrors the established +//! [`apply_update`](crate::cache::EvmCache::apply_update) applies `Slot` writes +//! through with one consistent rule: the BlockchainDb backend (layer 2) is +//! written **always**; the CacheDB overlay (layer 1) is written **only if an +//! overlay account already exists** for the address. A new overlay account is +//! never materialized for a slot write — the read path falls through to the +//! backend for an absent overlay entry, so a backend-only write is authoritative, +//! and materializing an overlay entry would pollute layer 1 and could shadow +//! later RPC reads. (This mirrors the established //! [`inject_storage_batch_fresh`](crate::cache::EvmCache::inject_storage_batch_fresh) //! semantics.) //! +//! `Account` patches follow the same overlay-if-present write-through policy +//! once the account is already present in either layer. If the account is absent +//! from **both** layers, the patch is skipped and surfaced in +//! [`StateDiff::skipped_accounts`]. Use [`StateUpdate::AccountUpsert`] when the +//! caller intentionally wants to materialize a cold/default account. +//! //! # The output //! //! Every apply returns a [`StateDiff`] of the changes it actually made: the @@ -86,21 +94,13 @@ //! Because a cold-skipped relative update produces **no** change, it is invisible //! to the natural [`StateDiff::is_empty`] / [`StateDiff::len`] success check (those //! are changes-only). A caller applying relative updates **must** therefore check -//! [`StateDiff::has_skipped`] (or inspect [`skipped`](StateDiff::skipped) / -//! [`skipped_balances`](StateDiff::skipped_balances)) — a cold target was dropped, -//! not applied, and a silently-dropped balance update can break conservation. -//! [`StateDiff::is_fully_applied`] and [`StateDiff::skipped_len`] are the -//! companions. -//! -//! # Warning — cold absolute `Account` patches -//! -//! A *partial* absolute [`StateUpdate::Account`] patch (e.g. balance-only) on an -//! address absent from **both** cache layers writes default nonce/code through the -//! shared backend as authoritative, pre-empting a real RPC fetch. Fetch+seed the -//! account first, or prefer [`StateUpdate::BalanceDelta`] for relative -//! native-balance tracking. See the warnings on -//! [`apply_update`](crate::cache::EvmCache::apply_update), -//! [`StateUpdate::Account`], and [`AccountPatch`]. +//! [`StateDiff::has_skipped`] (or inspect [`skipped`](StateDiff::skipped), +//! [`skipped_balances`](StateDiff::skipped_balances), +//! [`skipped_masks`](StateDiff::skipped_masks), or +//! [`skipped_accounts`](StateDiff::skipped_accounts)) — a cold target was +//! dropped, not applied, and a silently-dropped balance/account update can break +//! conservation. [`StateDiff::is_fully_applied`] and +//! [`StateDiff::skipped_len`] are the companions. //! //! # Boundary — events are Phase 4 //! @@ -246,21 +246,32 @@ pub enum StateUpdate { /// The bits to write (only the bits selected by `mask` are applied). value: U256, }, - /// Patch an account's balance/nonce/code (partial — see [`AccountPatch`]). + /// Patch an already-known account's balance/nonce/code (partial — see + /// [`AccountPatch`]). /// - /// # Warning - /// - /// A partial absolute patch (e.g. balance-only) on an address absent from - /// **both** cache layers writes default nonce/code through the shared backend - /// as authoritative, pre-empting a real RPC fetch. Fetch+seed the account - /// first, or use [`StateUpdate::BalanceDelta`] for relative native-balance - /// tracking. + /// Cold-aware: if the account is absent from **both** layers, the patch is not + /// applied and is surfaced in [`StateDiff::skipped_accounts`] as a + /// [`SkippedAccountPatch`]. Use [`StateUpdate::AccountUpsert`] when + /// materializing a cold/default account is intentional. Account { /// Account to patch. address: Address, /// The partial mutation: each `Some` field overwrites, `None` leaves it. patch: AccountPatch, }, + /// Apply an [`AccountPatch`], materializing a cold account when needed. + /// + /// This is the explicit escape hatch for callers that really do want a + /// default account to become authoritative in the backend (for example a + /// synthetic test account). Normal event-derived account patches should use + /// [`StateUpdate::Account`] so a cold account is skipped instead of masking a + /// future RPC fetch. + AccountUpsert { + /// Account to patch or create. + address: Address, + /// The partial mutation: each `Some` field overwrites, `None` leaves it. + patch: AccountPatch, + }, /// Purge cached state for `address` at `scope`; the next read re-fetches. Purge { /// Account whose cached state is purged. @@ -337,6 +348,16 @@ impl StateUpdate { Self::Account { address, patch } } + /// Construct a [`StateUpdate::AccountUpsert`] from a prebuilt + /// [`AccountPatch`]. + /// + /// Use this only when materializing an account absent from both layers is the + /// desired behavior. For normal patches to known accounts, use + /// [`account`](Self::account). + pub fn account_upsert(address: Address, patch: AccountPatch) -> Self { + Self::AccountUpsert { address, patch } + } + /// Construct a [`StateUpdate::Purge`] for `address` at `scope`. pub fn purge(address: Address, scope: PurgeScope) -> Self { Self::Purge { address, scope } @@ -360,11 +381,10 @@ impl StateUpdate { /// # Warning /// /// Applying an absolute patch with [`StateUpdate::Account`] on an address absent -/// from **both** cache layers writes default values for the un-patched fields -/// (e.g. nonce `0`, empty code) through the shared backend as authoritative, -/// masking a later RPC fetch of the real on-chain account. Fetch+seed the account -/// first, or use [`StateUpdate::BalanceDelta`] for relative native-balance -/// tracking. +/// from **both** cache layers is skipped and surfaced in +/// [`StateDiff::skipped_accounts`]. Use [`StateUpdate::AccountUpsert`] only when +/// default values for un-patched fields (e.g. nonce `0`, empty code) should become +/// authoritative in the backend. /// /// ``` /// use alloy_primitives::{Bytes, U256}; @@ -448,18 +468,19 @@ pub enum PurgeScope { /// changes are recorded, so a no-op write yields a [`Default`] (empty) diff. /// /// The struct is `#[non_exhaustive]`: it has grown fields pre-1.0 -/// ([`skipped`](Self::skipped), [`skipped_balances`](Self::skipped_balances)) and -/// may grow more. Construct it via [`Default`] + field assignment, never an -/// exhaustive struct literal. +/// ([`skipped`](Self::skipped), [`skipped_balances`](Self::skipped_balances), +/// [`skipped_masks`](Self::skipped_masks), and +/// [`skipped_accounts`](Self::skipped_accounts)) and may grow more. Construct it +/// via [`Default`] + field assignment, never an exhaustive struct literal. /// /// # Checking for skips /// /// [`is_empty`](Self::is_empty) / [`len`](Self::len) are **changes-only**, so a -/// cold-skipped relative update ([`SlotDelta`](StateUpdate::SlotDelta) / -/// [`BalanceDelta`](StateUpdate::BalanceDelta)) is invisible to them. After -/// applying relative updates, check [`has_skipped`](Self::has_skipped) (or -/// inspect [`skipped`](Self::skipped) / [`skipped_balances`](Self::skipped_balances)) -/// — a cold target was dropped, not applied. +/// cold-skipped update ([`SlotDelta`](StateUpdate::SlotDelta) / +/// [`BalanceDelta`](StateUpdate::BalanceDelta) / [`Account`](StateUpdate::Account)) +/// is invisible to them. After applying cold-aware updates, check +/// [`has_skipped`](Self::has_skipped) (or inspect the `skipped_*` fields) — a cold +/// target was dropped, not applied. #[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[non_exhaustive] pub struct StateDiff { @@ -484,6 +505,10 @@ pub struct StateDiff { /// bits could not be preserved. Like [`skipped`](Self::skipped) this is /// informational metadata, not a change. pub skipped_masks: Vec, + /// Account patches ([`StateUpdate::Account`]) that were **not** applied + /// because the account was absent from both layers. Like + /// [`skipped`](Self::skipped) this is informational metadata, not a change. + pub skipped_accounts: Vec, } impl StateDiff { @@ -515,12 +540,16 @@ impl StateDiff { !self.skipped.is_empty() || !self.skipped_balances.is_empty() || !self.skipped_masks.is_empty() + || !self.skipped_accounts.is_empty() } /// Total number of skipped relative/masked updates (`skipped` + - /// `skipped_balances` + `skipped_masks`). + /// `skipped_balances` + `skipped_masks` + `skipped_accounts`). pub fn skipped_len(&self) -> usize { - self.skipped.len() + self.skipped_balances.len() + self.skipped_masks.len() + self.skipped.len() + + self.skipped_balances.len() + + self.skipped_masks.len() + + self.skipped_accounts.len() } /// Whether every relative update in the apply was applied (none skipped). @@ -535,7 +564,8 @@ impl StateDiff { /// Used by [`apply_updates`](crate::cache::EvmCache::apply_updates) to merge /// per-update diffs; the concatenation preserves order, so two writes to the /// same slot record their `old → new` history in sequence. The `skipped`, - /// `skipped_balances`, and `skipped_masks` metadata are concatenated too. + /// `skipped_balances`, `skipped_masks`, and `skipped_accounts` metadata are + /// concatenated too. pub fn merge(&mut self, other: StateDiff) { self.slots.extend(other.slots); self.accounts.extend(other.accounts); @@ -543,6 +573,7 @@ impl StateDiff { self.skipped.extend(other.skipped); self.skipped_balances.extend(other.skipped_balances); self.skipped_masks.extend(other.skipped_masks); + self.skipped_accounts.extend(other.skipped_accounts); } } @@ -642,6 +673,22 @@ pub struct SkippedMask { pub value: U256, } +/// An account patch ([`StateUpdate::Account`]) that could not be applied because +/// the account is absent from **both** cache layers. +/// +/// A partial patch against a cold account is skipped rather than applied against +/// [`AccountInfo::default`](revm::state::AccountInfo::default), because default +/// nonce/code would become authoritative and mask a later RPC fetch. It is +/// surfaced here so the caller can fetch+seed the account and retry, or opt in to +/// materialization with [`StateUpdate::AccountUpsert`]. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct SkippedAccountPatch { + /// Account whose patch was skipped. + pub address: Address, + /// The patch that was not applied. + pub patch: AccountPatch, +} + #[cfg(test)] mod tests { use super::*; @@ -688,6 +735,13 @@ mod tests { patch: AccountPatch::default().balance(U256::from(9)), } ); + assert_eq!( + StateUpdate::account_upsert(a, AccountPatch::default().balance(U256::from(9))), + StateUpdate::AccountUpsert { + address: a, + patch: AccountPatch::default().balance(U256::from(9)), + } + ); assert_eq!( StateUpdate::purge(a, PurgeScope::Account), StateUpdate::Purge { diff --git a/tests/cache_state.rs b/tests/cache_state.rs index cdbb636..8c55b88 100644 --- a/tests/cache_state.rs +++ b/tests/cache_state.rs @@ -8,13 +8,13 @@ mod common; -use alloy_primitives::{Address, Bytes, I256, U256}; -use alloy_sol_types::SolValue; +use alloy_primitives::{Address, B256, Bytes, I256, U256, keccak256}; +use alloy_sol_types::{SolCall, SolValue}; use anyhow::{Context, Result}; use revm::state::{AccountInfo, Bytecode}; use common::{ - MOCK_ERC20_BALANCE_SLOT, balance_of, install_default_account, install_mock_erc20, + MOCK_ERC20_BALANCE_SLOT, MockERC20, balance_of, install_default_account, install_mock_erc20, mock_erc20_creation_code, mock_erc20_runtime, setup_cache, transfer, }; use evm_fork_cache::cache::TxConfig; @@ -123,6 +123,75 @@ async fn simulation_reports_balance_deltas() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn balance_delta_simulation_reports_access_list() -> Result<()> { + let mut cache = setup_cache().await?; + let token = Address::repeat_byte(0x44); + let owner = Address::repeat_byte(0x55); + let recipient = Address::repeat_byte(0x66); + + install_default_account(&mut cache, Address::ZERO); + install_default_account(&mut cache, owner); + install_default_account(&mut cache, recipient); + install_mock_erc20(&mut cache, token); + + let balance_slot = U256::from(MOCK_ERC20_BALANCE_SLOT); + cache.insert_mapping_storage_slot(token, balance_slot, owner, U256::from(1_000u64))?; + cache.insert_mapping_storage_slot(token, balance_slot, recipient, U256::ZERO)?; + + let transfer_call = MockERC20::transferCall { + to: recipient, + amount: U256::from(250u64), + }; + let result = cache.simulate_call_with_balance_deltas( + owner, + token, + Bytes::from(transfer_call.abi_encode()), + owner, + [token], + false, + )?; + + assert_eq!( + result.token_deltas.get(&token), + Some(&-I256::from_raw(U256::from(250u64))) + ); + + let owner_balance_slot = B256::from(U256::from_be_bytes( + keccak256((owner, balance_slot).abi_encode()).0, + )); + let recipient_balance_slot = B256::from(U256::from_be_bytes( + keccak256((recipient, balance_slot).abi_encode()).0, + )); + let token_item = result + .access_list + .0 + .iter() + .find(|item| item.address == token) + .expect("access list includes the token account"); + assert!( + token_item.storage_keys.contains(&owner_balance_slot), + "access list includes owner's balance slot" + ); + assert!( + token_item.storage_keys.contains(&recipient_balance_slot), + "access list includes recipient's balance slot" + ); + + assert_eq!( + balance_of(&mut cache, token, owner)?, + U256::from(1_000u64), + "non-committing simulation must not change owner balance" + ); + assert_eq!( + balance_of(&mut cache, token, recipient)?, + U256::ZERO, + "non-committing simulation must not change recipient balance" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn set_erc20_balance_with_slot_scan_finds_balance_slot() -> Result<()> { let mut cache = setup_cache().await?; @@ -235,7 +304,7 @@ async fn two_layer_cache_staleness_requires_full_purge() -> Result<()> { // Clearing ONLY the backend leaves the overlay serving stale data. { - let mut storage = cache.blockchain_db().storage().write(); + let mut storage = cache.unchecked_blockchain_db().storage().write(); storage.remove(&token); } assert_eq!( diff --git a/tests/cow_snapshot.rs b/tests/cow_snapshot.rs index 697e7f1..892e154 100644 --- a/tests/cow_snapshot.rs +++ b/tests/cow_snapshot.rs @@ -219,7 +219,7 @@ async fn cow_snapshot_matches_deep_clone_through_mutations() -> Result<()> { // `BlockchainDb` from inside foundry-fork-db, bypassing our write funnel): // a brand-new account+slot, and a NEW slot on the existing `pool`. { - let bdb = cache.blockchain_db(); + let bdb = cache.unchecked_blockchain_db(); bdb.storage() .write() .entry(pool3) @@ -275,7 +275,7 @@ async fn cow_snapshot_matches_deep_clone_through_mutations() -> Result<()> { } /// Escape-hatch re-honest hook (adversarial-review finding). A direct, out-of-band -/// layer-2 write through `blockchain_db()` that overwrites an existing slot at an +/// layer-2 write through `unchecked_blockchain_db()` that overwrites an existing slot at an /// unchanged slot count is the one mutation the count-based growth scan cannot see, /// so the memoized base can go stale. `invalidate_snapshot_base()` must restore /// read-equivalence with the deep-clone reference. @@ -290,7 +290,7 @@ async fn invalidate_snapshot_base_rehonest_after_escape_hatch_write() -> Result< // Out-of-band overwrite at unchanged length (bypasses the write funnel). { - let bdb = cache.blockchain_db(); + let bdb = cache.unchecked_blockchain_db(); bdb.storage() .write() .entry(pool) @@ -311,6 +311,195 @@ async fn invalidate_snapshot_base_rehonest_after_escape_hatch_write() -> Result< Ok(()) } +/// Escape-hatch re-honest hook for account-map overwrites. A direct update of an +/// existing layer-2 account's balance/code at an unchanged account count is also +/// invisible to the count/absence growth scan, so callers must invalidate the +/// memoized base after the direct write lands. +#[tokio::test(flavor = "multi_thread")] +async fn invalidate_snapshot_base_rehonest_after_existing_account_write() -> Result<()> { + let mut cache = setup_cache().await?; + let account = Address::repeat_byte(0xA1); + let code_v1 = Bytecode::new_raw(Bytes::from(vec![0x60u8, 0x01])); + let code_v2 = Bytecode::new_raw(Bytes::from(vec![0x60u8, 0x02, 0x60, 0x03])); + let h1 = code_v1.hash_slow(); + let h2 = code_v2.hash_slow(); + assert_ne!(h1, h2); + + let original = AccountInfo { + balance: U256::from(111u64), + nonce: 1, + code_hash: h1, + code: Some(code_v1.clone()), + account_id: None, + }; + let updated = AccountInfo { + balance: U256::from(222u64), + nonce: 2, + code_hash: h2, + code: Some(code_v2.clone()), + account_id: None, + }; + + { + let bdb = cache.unchecked_blockchain_db(); + bdb.accounts().write().insert(account, original.clone()); + } + let warm = cache.create_snapshot(); // memoize the base with `original`. + let mut ov_warm = EvmOverlay::new(Arc::clone(&warm), None); + let warm_info = ov_warm + .basic(account) + .expect("warm basic") + .expect("warm account"); + assert_eq!(warm_info.balance, original.balance); + assert_eq!(warm_info.nonce, original.nonce); + assert_eq!(warm_info.code_hash, h1); + assert_eq!( + ov_warm + .code_by_hash(h1) + .expect("warm code") + .original_bytes(), + code_v1.original_bytes() + ); + + // Out-of-band account overwrite at unchanged account count (bypasses the + // write funnel and is not detectable by the growth scan). + { + let bdb = cache.unchecked_blockchain_db(); + let mut accounts = bdb.accounts().write(); + assert!( + accounts.contains_key(&account), + "test must update an existing account" + ); + let len_before = accounts.len(); + accounts.insert(account, updated.clone()); + assert_eq!( + accounts.len(), + len_before, + "test must keep the account count unchanged" + ); + } + + cache.invalidate_snapshot_base(); + let cow = cache.create_snapshot(); + let deep = cache.create_snapshot_deep_clone(); + let mut ov_cow = EvmOverlay::new(Arc::clone(&cow), None); + let mut ov_deep = EvmOverlay::new(Arc::clone(&deep), None); + + let cow_basic = ov_cow.basic(account).expect("cow basic"); + let deep_basic = ov_deep.basic(account).expect("deep basic"); + assert!( + account_eq(&cow_basic, &deep_basic), + "invalidate_snapshot_base must re-honest the base after an out-of-band account write: cow={cow_basic:?} deep={deep_basic:?}" + ); + let cow_info = cow_basic.expect("updated cow account"); + assert_eq!(cow_info.balance, updated.balance); + assert_eq!(cow_info.nonce, updated.nonce); + assert_eq!(cow_info.code_hash, h2); + assert_eq!( + ov_cow.code_by_hash(h2).expect("cow h2").original_bytes(), + ov_deep.code_by_hash(h2).expect("deep h2").original_bytes(), + "updated code hash must match the deep clone" + ); + assert_eq!( + ov_cow.code_by_hash(h2).expect("cow h2").original_bytes(), + code_v2.original_bytes() + ); + assert!( + ov_deep.code_by_hash(h1).expect("deep h1").is_empty(), + "sanity: deep clone drops the unreferenced old hash" + ); + assert_eq!( + ov_cow.code_by_hash(h1).expect("cow h1").original_bytes(), + ov_deep.code_by_hash(h1).expect("deep h1").original_bytes(), + "the old code hash must not linger after invalidation" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn with_blockchain_db_mut_rehonest_after_storage_overwrite() -> Result<()> { + let mut cache = setup_cache().await?; + let pool = Address::repeat_byte(0x78); + let slot = U256::from(0u64); + + cache.inject_storage_batch(&[(pool, slot, U256::from(111u64))]); + let _warm = cache.create_snapshot(); + + cache.with_blockchain_db_mut(|bdb| { + bdb.storage() + .write() + .entry(pool) + .or_default() + .insert(slot, U256::from(222u64)); + }); + + let cow = cache.create_snapshot(); + let deep = cache.create_snapshot_deep_clone(); + assert_eq!( + cow.storage_value(pool, slot), + deep.storage_value(pool, slot), + "with_blockchain_db_mut must invalidate the COW base after storage writes" + ); + assert_eq!(cow.storage_value(pool, slot), Some(U256::from(222u64))); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn with_blockchain_db_mut_rehonest_after_account_overwrite() -> Result<()> { + let mut cache = setup_cache().await?; + let account = Address::repeat_byte(0xA2); + let code_v1 = Bytecode::new_raw(Bytes::from(vec![0x60u8, 0x01])); + let code_v2 = Bytecode::new_raw(Bytes::from(vec![0x60u8, 0x02])); + let h1 = code_v1.hash_slow(); + let h2 = code_v2.hash_slow(); + let original = AccountInfo { + balance: U256::from(111u64), + nonce: 1, + code_hash: h1, + code: Some(code_v1), + account_id: None, + }; + let updated = AccountInfo { + balance: U256::from(222u64), + nonce: 2, + code_hash: h2, + code: Some(code_v2.clone()), + account_id: None, + }; + + cache.with_blockchain_db_mut(|bdb| { + bdb.accounts().write().insert(account, original); + }); + let _warm = cache.create_snapshot(); + + cache.with_blockchain_db_mut(|bdb| { + let mut accounts = bdb.accounts().write(); + let len_before = accounts.len(); + accounts.insert(account, updated.clone()); + assert_eq!(accounts.len(), len_before); + }); + + let cow = cache.create_snapshot(); + let deep = cache.create_snapshot_deep_clone(); + let mut ov_cow = EvmOverlay::new(Arc::clone(&cow), None); + let mut ov_deep = EvmOverlay::new(Arc::clone(&deep), None); + let cow_basic = ov_cow.basic(account).expect("cow basic"); + let deep_basic = ov_deep.basic(account).expect("deep basic"); + assert!( + account_eq(&cow_basic, &deep_basic), + "with_blockchain_db_mut must invalidate the COW base after account writes: cow={cow_basic:?} deep={deep_basic:?}" + ); + assert_eq!( + cow_basic.expect("updated cow account").balance, + updated.balance + ); + assert_eq!( + ov_cow.code_by_hash(h2).expect("cow h2").original_bytes(), + code_v2.original_bytes() + ); + Ok(()) +} + /// Regression (review finding P2): the COW partial rebuild must not leave a stale /// `code_by_hash` entry when a base account is recoded or purged. Warm the base /// with a code-bearing account, recode it in layer 2, dirty it via a controlled @@ -330,7 +519,7 @@ async fn cow_code_index_matches_deep_clone_after_base_account_recoded() -> Resul // Seed a code-bearing account (code_v1) directly into the cold base (layer 2), // then re-honest the memoized base. let put_account = |cache: &EvmCache, code: &Bytecode, hash| { - cache.blockchain_db().accounts().write().insert( + cache.unchecked_blockchain_db().accounts().write().insert( contract, AccountInfo { balance: U256::from(1u64), diff --git a/tests/event_pipeline.rs b/tests/event_pipeline.rs index d18d224..bc6b2be 100644 --- a/tests/event_pipeline.rs +++ b/tests/event_pipeline.rs @@ -9,7 +9,7 @@ //! //! Layering vocabulary mirrors `tests/state_update.rs`: //! - **layer 1 / overlay** = the CacheDB overlay (`db_mut().cache.accounts`). -//! - **layer 2 / backend** = the BlockchainDb backend (`blockchain_db()`). +//! - **layer 2 / backend** = the BlockchainDb backend (`unchecked_blockchain_db()`). mod common; @@ -41,7 +41,7 @@ fn mapping_slot(owner: Address, mapping_slot: u64) -> U256 { /// Value of a slot in the BlockchainDb backend (layer 2) only. fn backend_slot(cache: &EvmCache, addr: Address, slot: U256) -> Option { cache - .blockchain_db() + .unchecked_blockchain_db() .storage() .read() .get(&addr) diff --git a/tests/freshness.rs b/tests/freshness.rs index 73607d0..aa2455a 100644 --- a/tests/freshness.rs +++ b/tests/freshness.rs @@ -248,7 +248,7 @@ async fn purge_account_drops_account_and_storage_from_both_layers() -> Result<() ); // Account gone from the backend accounts map. { - let accounts = cache.blockchain_db().accounts().read(); + let accounts = cache.unchecked_blockchain_db().accounts().read(); assert!(!accounts.contains_key(&token), "backend account removed"); } @@ -1185,6 +1185,12 @@ async fn run_unverified_on_fetcher_error() -> Result<()> { // T3 (part 2): into_optimistic aborts the validation task. The fetcher WOULD // queue a correction (it reports a changed value), so if the abort failed we // would observe a non-zero pending queue. We assert it stays 0. +// +// Determinism mirrors the Drop-abort test below: the validator is allowed to +// reach the synchronous fetch, but the gated fetch cannot return until after +// `into_optimistic()` has set the cancel flag. That makes the product guarantee +// precise: a cancel observed at the post-fetch checkpoint suppresses all +// side-effects, including pending corrections and re-run accounting. #[tokio::test(flavor = "multi_thread")] async fn run_into_optimistic_aborts_validation() -> Result<()> { let token = Address::repeat_byte(0x44); @@ -1192,11 +1198,12 @@ async fn run_into_optimistic_aborts_validation() -> Result<()> { let recipient = Address::repeat_byte(0x66); let mut cache = cache_with_balance(token, owner, U256::from(1000)).await?; + let gate = Gate::new(); // A CHANGED value: if the validator ran, it would queue a correction. - cache.set_storage_batch_fetcher(stub_fetcher(HashMap::from([( - (token, balance_slot_for(owner)), - U256::from(50), - )]))); + cache.set_storage_batch_fetcher(gated_tracking_fetcher( + HashMap::from([((token, balance_slot_for(owner)), U256::from(50))]), + gate.clone(), + )); let mut controller = FreshnessController::new(FreshnessRegistry::new(), AlwaysVerify); let sim = controller.run( @@ -1208,6 +1215,7 @@ async fn run_into_optimistic_aborts_validation() -> Result<()> { )], )?; let results = sim.into_optimistic(); // aborts the background validation + gate.release(); assert_eq!(results.len(), 1); // Give any (incorrectly) surviving task a chance to run, then assert no @@ -1572,8 +1580,8 @@ async fn run_unverified_without_fetcher() -> Result<()> { // A `from_backend` cache exposes no fetcher (no provider captured). let base = cache_with_balance(token, owner, U256::from(1000)).await?; let mut cache = EvmCache::from_backend( - base.backend().clone(), - base.blockchain_db().clone(), + base.unchecked_backend().clone(), + base.unchecked_blockchain_db().clone(), None, base.chain_id(), None, diff --git a/tests/serialization_roundtrip.rs b/tests/serialization_roundtrip.rs index 71850cf..27bdc32 100644 --- a/tests/serialization_roundtrip.rs +++ b/tests/serialization_roundtrip.rs @@ -87,6 +87,16 @@ fn immutable_data_cache_round_trips() { let len_before = cache.len(); cache.save(&path).expect("save immutable cache"); + let bytes = std::fs::read(&path).expect("read immutable cache file"); + assert!( + bytes.starts_with(b"EFCMETA\0"), + "immutable cache must carry a magic header" + ); + assert_eq!( + &bytes[8..12], + &1u32.to_le_bytes(), + "immutable cache must carry an explicit version" + ); let loaded = ImmutableDataCache::load(&path).expect("load immutable cache"); // Counts and scalar values survive the round trip. @@ -118,6 +128,20 @@ fn immutable_data_cache_round_trips() { assert_eq!(bal.last_change_block, U256::from(18_000_000u64)); } +#[test] +fn immutable_data_cache_load_legacy_raw_bincode_is_none() { + let dir = TempDir::new("immutable_legacy"); + let path = dir.path("legacy_immutable_data.bin"); + let mut cache = ImmutableDataCache::default(); + cache.set_token_decimals(Address::repeat_byte(0xA1), 6); + std::fs::write(&path, bincode::serialize(&cache).unwrap()).expect("write legacy cache"); + + assert!( + ImmutableDataCache::load(&path).is_none(), + "unversioned legacy bincode must be treated as a cache miss" + ); +} + #[test] fn immutable_data_cache_load_missing_file_is_none() { let dir = TempDir::new("immutable_missing"); @@ -180,6 +204,16 @@ mod tick_snapshots { assert_eq!(cache.len(), 1); cache.save(&path).expect("save tick cache"); + let bytes = std::fs::read(&path).expect("read tick cache file"); + assert!( + bytes.starts_with(b"EFCTICK\0"), + "tick snapshot cache must carry a magic header" + ); + assert_eq!( + &bytes[8..12], + &1u32.to_le_bytes(), + "tick snapshot cache must carry an explicit version" + ); let loaded = V3TickSnapshotCache::load(&path).expect("load tick cache"); let snap = loaded.get(pool).expect("snapshot present"); @@ -190,6 +224,24 @@ mod tick_snapshots { assert_eq!(snap.to_ticks(), ticks, "ticks survive round trip"); } + #[test] + fn v3_tick_snapshot_cache_load_legacy_raw_bincode_is_none() { + let dir = TempDir::new("v3_ticks_legacy"); + let path = dir.path("legacy_v3_tick_snapshots.bin"); + let pool = Address::repeat_byte(0x77); + let mut cache = V3TickSnapshotCache::default(); + cache.set( + pool, + V3PoolTickSnapshot::from_pool_data(&HashMap::new(), &HashMap::new(), 0, 0), + ); + std::fs::write(&path, bincode::serialize(&cache).unwrap()).expect("write legacy cache"); + + assert!( + V3TickSnapshotCache::load(&path).is_none(), + "unversioned legacy bincode must be treated as a cache miss" + ); + } + #[test] fn v3_tick_snapshot_silently_drops_unparseable_keys() { // Pin the documented behavior (KNOWN_ISSUES): a string key that does not diff --git a/tests/shared_memory_capacity.rs b/tests/shared_memory_capacity.rs index 08a9bf4..f3fc029 100644 --- a/tests/shared_memory_capacity.rs +++ b/tests/shared_memory_capacity.rs @@ -36,8 +36,8 @@ async fn default_capacity_is_fixed_64k() -> Result<()> { let cache = EvmCacheBuilder::new(mock_provider()).build().await; assert_eq!( cache.shared_memory_capacity(), - 64_000, - "the default must be Fixed(64_000)" + 65_536, + "the default must be Fixed(64 * 1024)" ); Ok(()) } @@ -54,7 +54,7 @@ async fn fixed_capacity_is_honored() -> Result<()> { #[tokio::test(flavor = "multi_thread")] async fn auto_capacity_with_no_loaded_state_falls_back_to_floor() -> Result<()> { - // No cache_config → nothing loaded → Auto resolves to the 64 KB floor. + // No cache_config → nothing loaded → Auto resolves to the 64 KiB floor. let cache = EvmCacheBuilder::new(mock_provider()) .shared_memory_capacity(SharedMemoryCapacity::Auto) .build() @@ -69,7 +69,7 @@ async fn auto_capacity_with_no_loaded_state_falls_back_to_floor() -> Result<()> /// The headline: `Auto` sizes the buffer from the chain state in a loaded bincode /// state file. A first cache persists 10 000 storage slots; a second cache built /// with `Auto` over the same `CacheConfig` loads them and pre-allocates -/// `10_000 * 16 = 160_000` bytes (vs. the 64 KB default). +/// `10_000 * 16 = 160_000` bytes (vs. the 64 KiB default). #[tokio::test(flavor = "multi_thread")] async fn auto_capacity_scales_with_loaded_binary_state() -> Result<()> { let dir = unique_cache_dir("auto"); @@ -86,7 +86,7 @@ async fn auto_capacity_scales_with_loaded_binary_state() -> Result<()> { .map(|i| (token, U256::from(i), U256::from(i + 1))) .collect(); cache.inject_storage_batch(&batch); - cache.flush(); // writes evm_state.bin + cache.flush()?; // writes evm_state.bin } // Second cache: Auto over the same config loads the 10k slots and sizes from them. @@ -104,11 +104,34 @@ async fn auto_capacity_scales_with_loaded_binary_state() -> Result<()> { // A Fixed override ignores the loaded state. let fixed = EvmCacheBuilder::new(mock_provider()) .cache_config(cfg.clone()) - .shared_memory_capacity(SharedMemoryCapacity::Fixed(64_000)) + .shared_memory_capacity(SharedMemoryCapacity::Fixed(64 * 1024)) .build() .await; - assert_eq!(fixed.shared_memory_capacity(), 64_000); + assert_eq!(fixed.shared_memory_capacity(), 65_536); let _ = std::fs::remove_dir_all(&dir); Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn flush_reports_unwritable_cache_paths() -> Result<()> { + let path_conflict = unique_cache_dir("flush_error"); + std::fs::write(&path_conflict, b"not a directory")?; + let cfg = CacheConfig::new(&path_conflict, 1, Default::default(), Default::default()); + let cache = EvmCacheBuilder::new(mock_provider()) + .cache_config(cfg) + .build() + .await; + + let err = cache + .flush() + .expect_err("flush must report persistence failures"); + let rendered = format!("{err:#}"); + assert!( + rendered.contains("directory") || rendered.contains("Not a directory"), + "unexpected error: {rendered}" + ); + + let _ = std::fs::remove_file(&path_conflict); + Ok(()) +} diff --git a/tests/state_update.rs b/tests/state_update.rs index 2ee024e..815758e 100644 --- a/tests/state_update.rs +++ b/tests/state_update.rs @@ -10,7 +10,7 @@ //! - **layer 1 / overlay** = the CacheDB overlay (`db_mut().cache.accounts`), //! which wins on reads. //! - **layer 2 / backend** = the BlockchainDb backend -//! (`blockchain_db().storage()` / `.accounts()`). +//! (`unchecked_blockchain_db().storage()` / `.accounts()`). mod common; @@ -22,8 +22,8 @@ use common::{ }; use evm_fork_cache::cache::EvmCache; use evm_fork_cache::{ - AccountPatch, PurgeScope, SkippedBalanceDelta, SkippedDelta, SkippedMask, SlotChange, - SlotDelta, StateDiff, StateUpdate, + AccountPatch, PurgeScope, SkippedAccountPatch, SkippedBalanceDelta, SkippedDelta, SkippedMask, + SlotChange, SlotDelta, StateDiff, StateUpdate, }; use revm::state::{AccountInfo, Bytecode}; @@ -52,7 +52,7 @@ fn overlay_slot(cache: &mut EvmCache, addr: Address, slot: U256) -> Option /// Value of a slot in the BlockchainDb backend (layer 2) only. fn backend_slot(cache: &EvmCache, addr: Address, slot: U256) -> Option { cache - .blockchain_db() + .unchecked_blockchain_db() .storage() .read() .get(&addr) @@ -87,7 +87,7 @@ fn overlay_nonce(cache: &mut EvmCache, addr: Address) -> Option { /// Backend (layer 2) balance for `addr`, if a backend account exists. fn backend_balance(cache: &EvmCache, addr: Address) -> Option { cache - .blockchain_db() + .unchecked_blockchain_db() .accounts() .read() .get(&addr) @@ -133,6 +133,13 @@ fn state_update_constructors_produce_expected_variants() { patch: AccountPatch::default().balance(U256::from(9)), } ); + assert_eq!( + StateUpdate::account_upsert(a, AccountPatch::default().balance(U256::from(9))), + StateUpdate::AccountUpsert { + address: a, + patch: AccountPatch::default().balance(U256::from(9)), + } + ); assert_eq!( StateUpdate::purge(a, PurgeScope::Account), StateUpdate::Purge { @@ -341,15 +348,46 @@ async fn apply_account_code_patch_recomputes_hash() -> Result<()> { } #[tokio::test] -async fn apply_account_patch_materializes_absent_account() -> Result<()> { - // An account absent from both layers is created (in the backend) by a patch, - // and the value is readable. +async fn apply_account_patch_on_cold_account_is_skipped_and_surfaced() -> Result<()> { + // A partial Account patch against a cold account must not materialize a + // default backend account, because that would mask the real on-chain account. let addr = Address::repeat_byte(0x88); let mut cache = setup_cache().await?; assert!(!overlay_has_account(&mut cache, addr)); assert_eq!(backend_balance(&cache, addr), None); - let diff = cache.apply_update(&StateUpdate::balance(addr, U256::from(1234))); + let patch = AccountPatch::default().balance(U256::from(1234)); + let diff = cache.apply_update(&StateUpdate::account(addr, patch.clone())); + + assert_eq!( + backend_balance(&cache, addr), + None, + "cold patch must not materialize a backend account" + ); + assert!(diff.accounts.is_empty()); + assert_eq!( + diff.skipped_accounts, + vec![SkippedAccountPatch { + address: addr, + patch + }] + ); + assert!(diff.has_skipped()); + assert_eq!(diff.skipped_len(), 1); + Ok(()) +} + +#[tokio::test] +async fn account_upsert_intentionally_materializes_absent_account() -> Result<()> { + let addr = Address::repeat_byte(0x88); + let mut cache = setup_cache().await?; + assert!(!overlay_has_account(&mut cache, addr)); + assert_eq!(backend_balance(&cache, addr), None); + + let diff = cache.apply_update(&StateUpdate::account_upsert( + addr, + AccountPatch::default().balance(U256::from(1234)), + )); assert_eq!(backend_balance(&cache, addr), Some(U256::from(1234))); assert_eq!(diff.accounts.len(), 1); @@ -357,6 +395,7 @@ async fn apply_account_patch_materializes_absent_account() -> Result<()> { diff.accounts[0].balance, Some((U256::ZERO, U256::from(1234))) ); + assert!(diff.skipped_accounts.is_empty()); Ok(()) } @@ -394,7 +433,7 @@ async fn apply_purge_account_clears_both_layers() -> Result<()> { "backend storage gone" ); { - let accounts = cache.blockchain_db().accounts().read(); + let accounts = cache.unchecked_blockchain_db().accounts().read(); assert!(!accounts.contains_key(&token), "backend account removed"); } assert_eq!(diff.purged.len(), 1); @@ -1303,7 +1342,7 @@ async fn account_patch_on_backend_only_account_does_not_materialize_overlay() -> let acct = Address::repeat_byte(0x91); let mut cache = setup_cache().await?; // Seed only the backend (the cold-prefetched, layer-2-only case). - cache.blockchain_db().accounts().write().insert( + cache.unchecked_blockchain_db().accounts().write().insert( acct, AccountInfo { balance: U256::from(100), @@ -1582,7 +1621,7 @@ async fn account_patch_normalizes_zero_code_hash_across_layers() -> Result<()> { use revm::primitives::KECCAK_EMPTY; let acct = Address::repeat_byte(0x7f); let mut cache = setup_cache().await?; - cache.blockchain_db().accounts().write().insert( + cache.unchecked_blockchain_db().accounts().write().insert( acct, AccountInfo { balance: U256::from(1), @@ -1594,7 +1633,7 @@ async fn account_patch_normalizes_zero_code_hash_across_layers() -> Result<()> { cache.apply_update(&StateUpdate::balance(acct, U256::from(2))); let backend_hash = cache - .blockchain_db() + .unchecked_blockchain_db() .accounts() .read() .get(&acct)