Replay archival genesis - storage re-key + account history seed by mlukanova · Pull Request #18 · raylsnetwork/axyl

mlukanova · 2026-07-07T10:23:24Z

Summary

Fixes issue with archival nodes inconsistent responses for genesis values.

Closes #676

Surface areas touched

Breaking / compatibility

None.

Test plan

Consistent responses to requests for genesis contract state similar to:
cast call 0x07e17e17e17e17e17e17e17e17e17e17e17e17e1 "getValidators(uint8)" 6 -r https://mainnet-archive.rayls.com/

[ ]
[ ]

github-actions · 2026-07-07T10:23:37Z

Claude finished @mlukanova's task in 2m 40s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a maintenance command (--fix-genesis-history) to correct two v2 archive defects in offline mode: (1) StoragesHistory re-keying from plain slot to keccak256(slot), and (2) seeding AccountsHistory with genesis block entries for immutable system contracts. The implementation is well-structured and the two new methods are correctly gated behind #[cfg(feature = "archive-replay")]. The overall approach is sound. A few issues are worth addressing before merge.

Issues

Bug risk — `fix_genesis_account_history`: `ShardedKey::last` is unsafe for accounts with post-genesis changesets

crates/execution/evm/src/reth_env/init.rs:529-542

let key = ShardedKey::last(*addr);
let existing = rocksdb.get::<reth_db::tables::AccountsHistory>(key.clone())?;
let merged = match existing {
    Some(existing_list) => {
        let blocks: Vec<u64> =
            core::iter::once(block).chain(existing_list.iter()).collect();
        BlockNumberList::new(blocks) ...
    }
    None => BlockNumberList::new([block])...
};

ShardedKey::last(addr) targets the shard with highest_block_number == u64::MAX — the "current" shard holding recent blocks. For an account that has multiple shards (e.g. one for blocks 1–500 and one for 501–∞), inserting block 0 into the u64::MAX shard corrupts the shard structure: the earliest shard (keyed at 500) remains untouched and block 0 ends up in the wrong bucket.

The intent (per the docstring) is to fix only immutable system contracts that never change after genesis and thus have exactly one shard. But there is no runtime guard preventing this path from running on a multi-shard account. The already_fixed check only skips accounts whose history already contains block 0 in some shard — a multi-shard account that legitimately lacks a block-0 entry would pass the filter and have its u64::MAX shard mutated.

Suggested fix: add an early-continue if more than one shard exists for the address, or assert that only one shard is present before writing. Fix this →

Correctness — `fix_genesis_history`: old plain-key entries are never removed

crates/execution/evm/src/reth_env/init.rs:563-635

The method inserts new hashed-key entries via insert_storage_history but leaves the original plain-key entries written by reth's genesis writer untouched. This doesn't cause incorrect reads (v2 reads by hashed key and will find the new entries), but:

The stale plain entries consume disk space and will be iterated by any full-table scan (including this method's own idempotency scan on a second run).
Tools that dump StoragesHistory will see duplicate-looking entries at genesis block — one under the plain slot and one under keccak256(slot).

Suggested fix: after the provider_rw.commit() on line 632, open a second write transaction and delete the genesis entries from StoragesHistory that are still keyed by plain slots (identifiable as entries at block whose key is NOT keccak256 of any genesis slot). Fix this →

Performance — Full table scans for idempotency in both methods

Both fix_genesis_history (line 597) and fix_genesis_account_history (line 498) iterate the entire StoragesHistory / AccountsHistory table to build the already_fixed set. For a large production archive these tables can be very large (millions of entries across all historic shards), making each run of --fix-genesis-history expensive even when there is nothing to do.

The idempotency check only needs to verify whether each specific (addr, hashed_slot) / addr key has genesis block 0. Since both sets are bounded by genesis.alloc, targeted point-reads would be O(k) where k = |genesis_alloc| rather than O(N) over the entire table.

Suggested approach: for each address/slot in genesis.alloc, directly call rocksdb.get::<AccountsHistory>(ShardedKey::last(addr)) and check if the result contains(block), instead of scanning all shards. Fix this →

Minor — `map_or(false, ...)` clippy lint

crates/execution/evm/src/reth_env/init.rs:619

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

This pattern is flagged by clippy as clippy::map_unwrap_or. Prefer:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

Minor — Empty test plan

The PR body has an empty test plan. Even for a maintenance-path fix, listing the manual steps (e.g. "run --fix-genesis-history on a known-bad archive, then query a genesis-seeded storage slot via eth_getStorageAt at an early block and verify non-zero result") would help reviewers and operators verify correctness.

What looks good

The --fix-genesis-history flag is cleanly separated from the replay path — the snapshot env is never opened, which is efficient for a maintenance-only run.
The early-return ordering (fix_genesis_history → unwind_to → replay) is logical.
Both methods correctly no-op for v1 (plain) storage via use_hashed_state() check.
fix_genesis_account_history uses a read-merge-write so existing post-genesis shards are preserved rather than overwritten.
insert_storage_history is called via reth's public API rather than direct DB writes, which keeps the indexing logic consistent.
Both methods are idempotent (second run is a no-op, even if imperfect in efficiency).
Signal handling and graceful-stop path are unchanged and remain correct.

github-actions · 2026-07-07T10:23:46Z

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results


Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

github-actions · 2026-07-07T10:26:59Z

Claude finished @mlukanova's task in 2m 26s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a --fix-genesis-history maintenance command that corrects two v2 archive defects offline: (1) StoragesHistory re-keying from plain slot to keccak256(slot), and (2) seeding AccountsHistory with genesis block entries for immutable system contracts. The structure and intent are sound. The previous automated review covered the key issues well — this review confirms those findings and adds one new usability issue.

Issues

Bug risk — `fix_genesis_account_history`: `ShardedKey::last` is unsafe for accounts with post-genesis changesets

crates/execution/evm/src/reth_env/init.rs:526

let key = ShardedKey::last(*addr);
let existing = rocksdb.get::<reth_db::tables::AccountsHistory>(key.clone())?;
let merged = match existing {
    Some(existing_list) => {
        let blocks: Vec<u64> =
            core::iter::once(block).chain(existing_list.iter()).collect();
        BlockNumberList::new(blocks)...
    }
    None => BlockNumberList::new([block])...
};

ShardedKey::last(addr) targets the shard with highest_block_number == u64::MAX. For accounts with multiple shards (e.g. blocks 0–500 in one shard, 501–∞ in another), inserting block 0 into the u64::MAX shard corrupts the shard structure — the earliest shard (keyed at 500) remains untouched and block 0 ends up in the wrong bucket.

The docstring correctly limits the intent to "accounts that produce no changesets" (immutable system contracts with exactly one shard). But there is no runtime guard preventing this from running on a multi-shard account. The already_fixed set only skips accounts whose history already contains block 0 — a multi-shard account legitimately missing a block-0 entry would pass the filter and be mutated incorrectly.

Suggested fix: before calling ShardedKey::last, count the shards for each address by iterating with a prefix filter. Skip with a warn! if more than one shard exists.

Fix this →

Correctness — `fix_genesis_history`: stale plain-key entries are never removed

crates/execution/evm/src/reth_env/init.rs:626-628

let provider_rw = self.provider_factory.database_provider_rw()?;
insert_storage_history(&provider_rw, rehashed.iter().map(|(a, acc)| (a, acc)), block)?;
provider_rw.commit()?;

The hashed-key entries are inserted correctly, but the original plain-slot entries written by reth's genesis writer remain untouched. Reads are unaffected (v2 looks up by hashed key and finds the new entries), and the idempotency check is also unaffected (it correctly matches hashed-key entries in already_fixed on the next run). However:

Stale plain-slot entries consume disk space permanently.
Any tool that does a full StoragesHistory scan will see what appear to be duplicate entries at genesis block — one under the plain slot and one under keccak256(slot).
The existing heal_rocksdb_history_after_snapshot scanner iterates this table and will see the extra entries on every startup.

Suggested fix: after provider_rw.commit(), open a second write transaction and delete the StoragesHistory entries still keyed by plain slots at the genesis block. These can be identified from genesis.alloc — for each address/slot pair, delete the entry keyed by (addr, plain_slot) at block.

Fix this →

Performance — Full table scans in both methods

crates/execution/evm/src/reth_env/init.rs:495-505 and 591-601

Both fix_genesis_account_history and fix_genesis_history iterate the entire AccountsHistory / StoragesHistory table to build their already_fixed sets. On a production archive these tables contain millions of entries across all historic shards, making the idempotency check expensive on every run — even when there's nothing to do.

Since both sets are bounded by genesis.alloc, targeted point-reads are O(k) where k = |genesis_alloc| rather than O(N) over the whole table:

// Instead of scanning all shards:
for addr in genesis.alloc.keys() {
    let key = ShardedKey::last(*addr);
    if let Some(list) = rocksdb.get::<AccountsHistory>(key)? {
        if list.contains(block) {
            already_fixed.insert(*addr);
        }
    }
}

Fix this →

Usability — `--snapshot-datadir` is required even in fix-only mode

bin/rayls-replay/src/main.rs:39-43 and 191-199

// main.rs: fix_genesis_history exits before snapshot_evm is ever created (line 217)
if cli.fix_genesis_history {
    archive_evm.fix_genesis_history()?;
    archive_evm.fix_genesis_account_history()?;
    return Ok(());
}
// snapshot_evm is opened only after this point...

The --fix-genesis-history maintenance exit fires before the snapshot env is ever opened (correctly). However, --snapshot-datadir is still a required clap argument with no default, so operators must pass a dummy path:

rayls-replay --archive-out /data/archive --snapshot-datadir /dev/null --fix-genesis-history

Suggested fix: mark snapshot_datadir as required = false (or add required_unless_present = "fix_genesis_history") so the maintenance mode only needs --archive-out.

Fix this →

Minor — `map_or(false, ...)` clippy lint

crates/execution/evm/src/reth_env/init.rs:615

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

Clippy flags map_or(false, ...) as clippy::map_unwrap_or. Prefer:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

What looks good

The maintenance exit path fires before the snapshot env is opened — correct and efficient.
Both methods are correctly gated behind #[cfg(feature = "archive-replay")].
Both methods correctly no-op for v1 (plain) storage via use_hashed_state() guard.
The re-key in fix_genesis_history builds the hashed alloc correctly: address stays plain, slot is keccak256(slot), matching the v2 read path.
fix_genesis_account_history uses a read-merge-write with core::iter::once(block).chain(existing_list.iter()) — prepending block 0 is always sorted-safe since it's the smallest possible block number.
insert_storage_history is called via reth's public API rather than direct DB writes, keeping indexing logic consistent with the rest of the stack.
RocksDB batch is used for the account history write (line 524) — appropriate for bulk RocksDB-only updates without needing a full provider transaction.
The fix_genesis_history → unwind_to → replay ordering in main.rs is logical.
Signal handling and graceful-stop path are untouched and remain correct.

…gesets present)

github-actions · 2026-07-07T10:59:37Z

Claude finished @mlukanova's task in 3m 27s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR adds a --fix-genesis-history maintenance command that corrects two v2 archive defects offline: (1) StoragesHistory re-keying from plain slot to keccak256(slot), and (2) seeding AccountsHistory with genesis block entries for accounts that produce no changesets. The most recent commit (88016d8) addressed the critical multi-shard safety bug from the prior reviews. Several issues from those reviews remain open.

What the latest commit fixed

fix_genesis_account_history: Multi-shard guard is now correct (init.rs:497–542)

The single-pass scan now collects both already_fixed and shard_counts, and to_fix is restricted to accounts with shard_count <= 1. This prevents the earlier bug where inserting block 0 into the u64::MAX shard of a multi-shard account would leave the earlier shards untouched and corrupt the shard boundaries.

One additional positive: the BlockNumberList merge is safe. core::iter::once(block).chain(existing_list.iter()) prepends genesis block 0, which is always ≤ any existing block in the list, so sorted order is preserved and BlockNumberList::new will not reject it.

Remaining issues

Bug — `open_db(&consensus_db)` fires before the fix-mode early return

bin/rayls-replay/src/main.rs:166 vs 191

let consensus_store = open_db(&consensus_db);   // line 166 — always runs

// ...
if cli.fix_genesis_history {                     // line 191 — early return
    archive_evm.fix_genesis_history()?;
    ...
    return Ok(());
}

consensus_db defaults to snapshot_datadir/consensus-db. In fix mode the snapshot DB is never queried, but open_db is still called unconditionally before the early return. Depending on what open_db does with a non-existent or invalid path, this can fail or silently create unexpected directories when an operator passes a dummy --snapshot-datadir. This compounds the usability issue below — even a placeholder value may not be safe.

Suggested fix: move the consensus_store = open_db(...) line to below the fix_genesis_history early return block (along with any other snapshot-only setup that is currently before it). Fix this →

Correctness — `fix_genesis_history`: stale plain-key entries never removed

crates/execution/evm/src/reth_env/init.rs:652–655

The method inserts new hashed-key entries but leaves the original plain-slot entries (written by reth's genesis writer) intact. This does not affect v2 reads (which look up by keccak256(slot) and find the new entries), but:

heal_rocksdb_history_after_snapshot iterates all of StoragesHistory on every node startup and will scan these stale entries on each boot.
Any full-table dump or diagnostic tool will show apparent duplicate entries at genesis block — one under the plain slot, one under keccak256(slot).
Stale entries consume disk space permanently.

Suggested fix: after provider_rw.commit(), open a second write transaction and delete the plain-slot entries at genesis block. Build the deletion keys from genesis.alloc: for each (addr, plain_slot) pair, remove the StoragesHistory entry keyed by (addr, plain_slot) at block block. Fix this →

Performance — Full table scan in `fix_genesis_history` idempotency check

crates/execution/evm/src/reth_env/init.rs:617–627

fix_genesis_history iterates the entire StoragesHistory table to build already_fixed. On a production archive this table can contain millions of entries across all historic shards, making the idempotency check expensive even when there is nothing to do.

fix_genesis_account_history already collects both already_fixed and shard_counts in a single pass (good), but the set is still O(N) over all shards rather than O(k) over genesis alloc.

For both methods the check only needs to verify whether each specific genesis key has block 0. Since the set is bounded by genesis.alloc, targeted point-reads are possible:

// Instead of scanning all shards, for each genesis address/slot:
let key = StorageShardedKey::last(*addr, hashed_slot);
if let Some(list) = rocksdb.get::<StoragesHistory>(key)? {
    if list.contains(block) { already_fixed.insert((*addr, hashed_slot)); }
}

Fix this →

Usability — `--snapshot-datadir` required even in fix-only mode

bin/rayls-replay/src/main.rs:43

snapshot_datadir: PathBuf is a required argument with no default. In fix mode the snapshot env is never opened, so operators must either pass the real snapshot path (which may not be co-located with the archive) or a dummy value. As described above, even a dummy value risks triggering open_db on a bad path.

Suggested fix: make snapshot_datadir optional (Option<PathBuf>) or add required_unless_present = "fix_genesis_history" so the maintenance mode only needs --archive-out. Update downstream references to handle the Option. Fix this →

Minor — `map_or(false, …)` clippy lint

crates/execution/evm/src/reth_env/init.rs:641

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

Clippy flags map_or(false, …) as clippy::map_unwrap_or. Prefer:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

What looks good

The multi-shard safety bug from prior reviews is correctly fixed: shard_counts and already_fixed are gathered in a single scan pass, and the <= 1 guard prevents writing block 0 into the wrong shard bucket.
Both methods correctly short-circuit on !use_hashed_state() — safe no-op for v1 archives.
The BlockNumberList merge order is correct: prepending genesis block 0 to an existing sorted list preserves sort order, so BlockNumberList::new will not fail.
keccak256(slot) usage matches how v2 reads StoragesHistory — the re-keyed entries will be found by the read path.
The fix fires before snapshot_evm is ever opened — no unnecessary snapshot DB access.
Both methods are gated behind #[cfg(feature = "archive-replay")].
The fix_genesis_history → unwind_to → replay ordering in main.rs is logical.

github-actions · 2026-07-07T11:01:01Z

Claude finished @mlukanova's task in 2m 20s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a --fix-genesis-history maintenance command that corrects two v2 archive defects offline:

StoragesHistory re-keying from plain slot → keccak256(slot)
Seeding AccountsHistory with genesis block 0 entries for immutable system contracts

The latest commit (88016d8) addresses the top bug risk identified in previous reviews — ShardedKey::last being unsafe for accounts with multiple history shards — by adding a combined single-pass scan that tracks both idempotency state and shard counts. That fix is correct and well-structured. Three issues remain.

Issues

Correctness — Stale plain-key entries are never removed from `StoragesHistory`

crates/execution/evm/src/reth_env/init.rs:652-654

let provider_rw = self.provider_factory.database_provider_rw()?;
insert_storage_history(&provider_rw, rehashed.iter().map(|(a, acc)| (a, acc)), block)?;
provider_rw.commit()?;

fix_genesis_history inserts hashed-key entries correctly, but the original plain-slot entries that reth's genesis writer created remain in StoragesHistory. They are keyed by (addr, plain_slot) while the new entries are keyed by (addr, keccak256(slot)) — different keys, so both persist permanently. Consequences:

Stale entries consume disk space and will be iterated by every full-table scan of StoragesHistory (including heal_rocksdb_history_after_snapshot and this method's own idempotency scan on a second run, though the idempotency logic itself stays correct since it matches on the hashed key).
Any diagnostic tool that dumps StoragesHistory will see apparent duplicates at the genesis block — one entry under the plain slot, one under keccak256(slot).

Suggested fix: after provider_rw.commit(), open a second write transaction and delete the StoragesHistory entries keyed by plain slots at the genesis block. The full set of plain keys to delete is deterministic: iterate genesis.alloc and for each address/slot pair, delete (addr, plain_slot) at block. Fix this →

Minor — `map_or(false, ...)` is a clippy warning

crates/execution/evm/src/reth_env/init.rs:641

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

clippy::map_unwrap_or flags map_or(false, pred) — prefer the semantically equivalent:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

Minor — `--snapshot-datadir` is required even in fix-only mode

bin/rayls-replay/src/main.rs:43

The fix_genesis_history exit fires before the snapshot env is opened (correctly), but --snapshot-datadir is still a required positional argument. Operators must supply a path even when they only want to run the fix. Since genesis_path and parameters_path fall back to cli.snapshot_datadir, passing --genesis and --parameters explicitly can work around it today, but the UX isn't obvious.

Suggested fix: mark snapshot_datadir as Option<PathBuf> with required_unless_present = "fix_genesis_history" and guard the path-construction code to error clearly if it's absent and fix mode is not set. Fix this →

What looks good

Multi-shard safety (fixed in latest commit): The combined single-pass scan in fix_genesis_account_history that collects both already_fixed and shard_counts correctly addresses the prior bug: accounts with > 1 shard are logged and skipped, preventing block 0 from being inserted into the wrong shard.
The --fix-genesis-history flag exits before the snapshot env is opened — efficient and correct.
Both methods are correctly gated behind #[cfg(feature = "archive-replay")] and no-op for v1 via the use_hashed_state() guard.
The fix_genesis_history → unwind_to → replay ordering in main.rs is logical.
fix_genesis_account_history correctly prepends block 0 to the existing shard's list (sorted-safe since 0 is always the smallest possible block number).
insert_storage_history is called via reth's public API — consistent with the rest of the stack.
RocksDB batch is used for account history writes — appropriate for bulk RocksDB-only updates without a full provider transaction.
Signal handling and graceful-stop path are untouched.
Both methods are idempotent (second run is a no-op).

github-actions · 2026-07-07T13:38:25Z

Claude finished @mlukanova's task in 3m 0s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a --fix-genesis-history maintenance command correcting two v2 archive defects: (1) StoragesHistory re-keying from plain slot → keccak256(slot), and (2) seeding AccountsHistory with genesis block entries for immutable system contracts. The multi-shard safety bug flagged in prior reviews was addressed in commit 88016d8. Three issues remain open after reviewing the current code.

What the latest commit fixed

fix_genesis_account_history: multi-shard safety guard (init.rs:497–542)

The single-pass scan now collects both already_fixed and shard_counts simultaneously. The to_fix filter restricts to shard_counts <= 1, preventing block 0 from being written into the u64::MAX shard of a multi-shard account. The BlockNumberList merge is sorted-safe: core::iter::once(0).chain(existing_list.iter()) always preserves ascending order since 0 is the minimum block number.

Remaining issues

Correctness — Stale plain-key entries never removed from `StoragesHistory`

crates/execution/evm/src/reth_env/init.rs:652–655

let provider_rw = self.provider_factory.database_provider_rw()?;
insert_storage_history(&provider_rw, rehashed.iter().map(|(a, acc)| (a, acc)), block)?;
provider_rw.commit()?;

New hashed-key entries are inserted correctly, but the original plain-slot entries written by reth's genesis writer remain permanently. Both key sets coexist: (addr, plain_slot) and (addr, keccak256(slot)). v2 reads are unaffected (they look up by hashed key), but:

heal_rocksdb_history_after_snapshot iterates all of StoragesHistory on every startup and will scan stale entries on each boot.
Any diagnostic dump of StoragesHistory shows apparent duplicates at the genesis block.
The idempotency scan on a second run of this method also traverses them unnecessarily.

Suggested fix: after provider_rw.commit(), open a second write transaction and delete the plain-slot StoragesHistory entries at block. These are deterministic — iterate genesis.alloc, and for each (addr, plain_slot) pair, delete the shard entry keyed by (addr, plain_slot). Fix this →

Bug — `open_db` called before the fix-mode early return

bin/rayls-replay/src/main.rs:166 vs 191

let consensus_store = open_db(&consensus_db);  // line 166 — always runs

// ...
if cli.fix_genesis_history {                    // line 191 — early return
    archive_evm.fix_genesis_history()?;
    ...
    return Ok(());
}

consensus_db defaults to cli.snapshot_datadir.join("consensus-db") (line 144). In fix-only mode, consensus_store is never used — but open_db is called unconditionally. If an operator passes a dummy --snapshot-datadir to work around the required-argument issue, open_db may fail, create unexpected directories, or open a DB at a garbage path depending on its implementation.

Suggested fix: move let consensus_store = open_db(&consensus_db) to after the fix_genesis_history block (after line 200), alongside the rest of the snapshot-only setup. Fix this →

Usability — `--snapshot-datadir` required even in fix-only mode

bin/rayls-replay/src/main.rs:43

snapshot_datadir: PathBuf is a required clap argument with no default. The --fix-genesis-history exit fires before the snapshot env is opened (correctly), but operators still must supply a path. This compounds the open_db issue above — even a placeholder value may trigger side effects.

Suggested fix: change to Option<PathBuf> with required_unless_present = "fix_genesis_history", and update downstream path construction to unwrap or error clearly when absent and fix mode is not set. Fix this →

Performance — Full table scans for idempotency in both methods

init.rs:501–508 (fix_genesis_account_history) and 617–627 (fix_genesis_history)

Both methods iterate all of AccountsHistory / StoragesHistory to build their already_fixed sets. On a production archive these tables span millions of entries. The check only needs to verify whether each genesis address/slot already has a block-0 entry — since both sets are bounded by genesis.alloc, targeted point-reads are O(k) rather than O(N):

// Instead of scanning all shards, for each genesis slot:
let key = StorageShardedKey::last(*addr, hashed_slot);
if let Some(list) = rocksdb.get::<StoragesHistory>(key)? {
    if list.contains(block) { already_fixed.insert((*addr, hashed_slot)); }
}

Note: fix_genesis_account_history must still count shards to enforce the <= 1 guard, so a prefix iterator per address remains necessary there — but the already_fixed part of that scan can use a point-read instead. Fix this →

Minor — `map_or(false, ...)` clippy lint

crates/execution/evm/src/reth_env/init.rs:641

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

Clippy flags map_or(false, pred) as clippy::map_unwrap_or. Prefer:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

What looks good

Multi-shard safety bug is correctly fixed: shard_counts and already_fixed collected in a single pass; accounts with > 1 shard are logged and skipped.
Both methods no-op for v1 archives via the use_hashed_state() guard.
The --fix-genesis-history exit fires before snapshot_evm is opened — correct and efficient.
keccak256(slot) usage in re-keying matches the v2 read path exactly (address plain, slot hashed).
fix_genesis_account_history uses raw RocksDB batch writes — appropriate for this bulk update without needing a full provider transaction.
fix_genesis_history uses insert_storage_history via reth's public API — consistent with the stack.
Both methods are idempotent (second run is a no-op).
The fix_genesis_history → unwind_to → replay ordering in main.rs is logical.
Signal handling and graceful-stop path are untouched.

github-actions · 2026-07-08T08:32:45Z

Claude finished @mlukanova's task in 2m 35s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a --fix-genesis-history maintenance command correcting two v2 archive defects: (1) StoragesHistory re-keying from plain slot → keccak256(slot), and (2) seeding AccountsHistory with genesis block 0 entries for immutable system contracts. Additionally, both fixes are applied automatically at the end of a fresh replay (lines 280–281 in main.rs).

The multi-shard safety bug from prior reviews was addressed in commit 88016d8 and is correctly implemented. Three medium-severity issues remain open, plus two minor items.

Issues

Bug — `open_db` fires before the fix-mode early return

bin/rayls-replay/src/main.rs:166 vs 191

let consensus_store = open_db(&consensus_db);   // line 166 — always runs

// ...
if cli.fix_genesis_history {                     // line 191 — early return
    archive_evm.fix_genesis_history()?;
    ...
    return Ok(());
}

consensus_db defaults to cli.snapshot_datadir.join("consensus-db"). In fix-only mode the consensus store is never used (its first actual use is in run_replay at line 246), but open_db is called unconditionally. An operator passing a dummy --snapshot-datadir (the workaround forced by the missing optional argument) will trigger open_db against a non-existent path, which may fail, create unexpected directories, or silently succeed but leave garbage state.

Fix: move let consensus_store = open_db(&consensus_db); to after the fix_genesis_history block (after line 200) and after the unwind_to block (after line 215), since consensus_store is only needed by run_replay. Fix this →

Correctness — Stale plain-key entries never removed from `StoragesHistory`

crates/execution/evm/src/reth_env/init.rs:652–654

let provider_rw = self.provider_factory.database_provider_rw()?;
insert_storage_history(&provider_rw, rehashed.iter().map(|(a, acc)| (a, acc)), block)?;
provider_rw.commit()?;

New hashed-key entries are inserted correctly, but the original plain-slot entries written by reth's genesis writer remain permanently. Both key sets now coexist: (addr, plain_slot) and (addr, keccak256(slot)). v2 reads are unaffected (they look up by hashed key and find the new entries), but:

heal_rocksdb_history_after_snapshot iterates all of StoragesHistory on every node startup — it will scan these stale entries on each boot forever.
fix_genesis_history itself scans all of StoragesHistory for its idempotency check (line 618) — stale plain entries pass through this scan on every subsequent run.
Any diagnostic dump shows apparent duplicate entries at genesis block.

Fix: after provider_rw.commit(), open a second write transaction and delete the StoragesHistory entries still keyed by plain slots at genesis block. The full set of plain keys is deterministic: iterate genesis.alloc, and for each (addr, plain_slot) pair, delete the shard entry keyed by (addr, plain_slot). Fix this →

Performance — Full table scan in `fix_genesis_history` idempotency check

crates/execution/evm/src/reth_env/init.rs:617–627

let already_fixed: HashSet<(Address, B256)> = {
    let rocksdb = self.provider_factory.rocksdb_provider();
    let mut set = HashSet::new();
    for entry in rocksdb.iter::<reth_db::tables::StoragesHistory>()? {
        let (key, value) = entry?;
        if value.contains(block) {
            set.insert((key.address, key.sharded_key.key));
        }
    }
    set
};

This iterates the entire StoragesHistory table. On a production archive this spans millions of entries across all historic shards. This is expensive even when there's nothing to do. More critically, fix_genesis_history is also called automatically after every successful replay completion (line 280 in main.rs), meaning a full table scan runs at the end of every replay — after the table has just been fully populated.

Since the check only needs to verify whether each specific (addr, hashed_slot) already has a block-0 entry, and the set is bounded by genesis.alloc, targeted point-reads are O(k) where k = |genesis slots|:

// Instead of iterating the whole table:
let key = StorageShardedKey::last(*addr, hashed_slot);
if let Some(list) = rocksdb.get::<StoragesHistory>(key)? {
    if list.contains(block) { already_fixed.insert((*addr, hashed_slot)); }
}

Fix this →

Usability — `--snapshot-datadir` required even in fix-only mode

bin/rayls-replay/src/main.rs:43

snapshot_datadir: PathBuf is a required positional argument. In fix mode the snapshot env is never opened (correctly). However, operators must still supply a path — and since genesis_path and parameters_path are derived from snapshot_datadir at lines 146–151, they must either provide the real snapshot path or rely on the embedded config fallback. Combined with the open_db issue above, even a syntactically valid placeholder path can have side effects.

Fix: change snapshot_datadir to Option<PathBuf> with required_unless_present = "fix_genesis_history" and update lines 144–151 to unwrap or error clearly when absent and fix mode is not set. Fix this →

Minor — `map_or(false, ...)` clippy lint

crates/execution/evm/src/reth_env/init.rs:641

rehashed.retain(|(_, account)| account.storage.as_ref().map_or(false, |s| !s.is_empty()));

clippy::map_unwrap_or flags map_or(false, pred). Prefer:

rehashed.retain(|(_, account)| account.storage.as_ref().is_some_and(|s| !s.is_empty()));

Fix this →

What looks good

Multi-shard safety (fixed in 88016d8): The single-pass scan in fix_genesis_account_history correctly collects both already_fixed and shard_counts simultaneously; accounts with > 1 shard are logged and skipped, preventing block 0 from landing in the wrong shard.
Both methods are correctly gated behind #[cfg(feature = "archive-replay")] and no-op for v1 via the use_hashed_state() guard.
The --fix-genesis-history exit fires before snapshot_evm is ever opened — correct.
keccak256(slot) re-keying matches the v2 read path exactly (address plain, slot hashed).
fix_genesis_account_history uses raw RocksDB batch writes — appropriate for bulk updates without a full provider transaction.
fix_genesis_history uses insert_storage_history via reth's public API — consistent with the rest of the stack.
BlockNumberList merge in fix_genesis_account_history is sorted-safe: prepending block 0 always preserves ascending order.
Both methods are idempotent.
Both fixes are applied automatically post-replay (lines 280–281) in addition to the standalone --fix-genesis-history path.
Signal handling and graceful-stop path are untouched.

github-actions · 2026-07-08T13:16:46Z

Claude finished @mlukanova's task in 3m 22s —— View job

PR Review

Gathering context
Reading changed files
Analyzing code
Posting review

Summary

This PR introduces a --fix-genesis-history maintenance command correcting two v2 archive defects: (1) StoragesHistory re-keying from plain slot → keccak256(slot), and (2) seeding AccountsHistory with genesis block 0 entries for immutable system contracts. Both fixes are also applied automatically at the end of a successful replay.

The latest commit addresses three issues from prior reviews. Two open bugs and two minor issues remain.

What the latest commit fixed

fix_genesis_history: O(N) full table scan replaced with O(k) per-slot lookups (init.rs:625–648)

The idempotency check now uses rocksdb.storage_history_shards(*addr, *slot) per genesis slot instead of iterating the entire StoragesHistory table. This is O(k) where k = number of genesis storage slots — bounded and fast even post-replay.

fix_genesis_history: New has_other_history safety guard (init.rs:637–666)

The code now distinguishes two skip cases: slots already re-keyed at the genesis block (already_fixed) and slots with any post-genesis hashed history (has_other_history). This prevents insert_storage_history's upsert from overwriting the last shard of a slot that has legitimate post-genesis writes. The in-code comment explains the invariant well. The tradeoff — genesis entries for non-static slots remain unreachable under the hashed key — is correct and preferable to silent data corruption.

map_or(false, ...) clippy lint fixed (init.rs:677)

Now uses is_some_and as previously suggested.

Remaining issues

Bug — `open_db` fires before the fix-mode early return

bin/rayls-replay/src/main.rs:166 vs. 191

let consensus_store = open_db(&consensus_db);   // line 166 — runs unconditionally

// ...
if cli.fix_genesis_history {                     // line 191 — early return
    archive_evm.fix_genesis_history()?;
    ...
    return Ok(());
}

consensus_db defaults to cli.snapshot_datadir.join("consensus-db"). In fix-only mode the consensus store is never referenced (its first use is inside run_replay at line 246), but open_db is called unconditionally before the early return. An operator forced to pass a dummy --snapshot-datadir will trigger open_db against a non-existent or invalid path.

Suggested fix: Move let consensus_store = open_db(&consensus_db) to after both the fix_genesis_history and unwind_to early-return blocks (i.e., after line 215), since consensus_store is only consumed by run_replay. Fix this →

Correctness — Stale plain-key entries never removed from `StoragesHistory`

crates/execution/evm/src/reth_env/init.rs:688–691

let provider_rw = self.provider_factory.database_provider_rw()?;
insert_storage_history(&provider_rw, rehashed.iter().map(|(a, acc)| (a, acc)), block)?;
provider_rw.commit()?;

New hashed-key entries are inserted correctly, but the original plain-slot entries from reth's genesis writer coexist permanently. They are keyed as (addr, plain_slot) while new entries are (addr, keccak256(slot)) — distinct keys, so both persist. v2 reads are unaffected (they look up by hashed key), but:

Any diagnostic tool that dumps StoragesHistory will see apparent duplicate entries at the genesis block.
fix_genesis_account_history's full table scan (and other full-table consumers) will traverse them on every run.
Stale entries consume disk space permanently.

Suggested fix: After provider_rw.commit(), open a second write transaction and delete the plain-slot StoragesHistory entries at block. The set is deterministic — build it from the rehashed vec (the set of slots that were actually re-keyed) and delete (addr, plain_slot) for each. Fix this →

Performance — `fix_genesis_account_history` still scans the entire `AccountsHistory` table

crates/execution/evm/src/reth_env/init.rs:501–508

for entry in rocksdb.iter::<reth_db::tables::AccountsHistory>()? {
    let (key, value) = entry?;
    *counts.entry(key.key).or_insert(0) += 1;
    if value.contains(block) {
        fixed.insert(key.key);
    }
}

This single-pass scan is efficient for what it does, but it iterates all entries across every account's every shard. On a production archive this can be millions of rows. Critically, fix_genesis_account_history is now called after every successful replay completion (line 281 in main.rs), so this O(N) scan runs at the end of each replay — after the table has just been fully built.

The shard-count check requires knowing all shards per address, so a full-table scan is the simplest approach. However, scoping the iteration to genesis addresses via per-address prefix scans would reduce complexity to O(k × avg_shards) where k = |genesis.alloc|. The idempotency point-read could then use the same prefix scan.

Fix this →

Usability — `--snapshot-datadir` required even in fix-only mode

bin/rayls-replay/src/main.rs:43

snapshot_datadir: PathBuf is a required positional argument with no default. In fix mode the snapshot env is never opened (correctly), but operators must still pass a path. Combined with the open_db issue above, even a syntactically valid placeholder can cause side effects.

Suggested fix: Change to Option<PathBuf> with required_unless_present = "fix_genesis_history" and guard the path-construction code at lines 144–151 to error clearly when absent and fix mode is not set. Fix this →

What looks good

fix_genesis_history idempotency is now O(k) via storage_history_shards per genesis slot — no longer a full table scan.
The has_other_history guard correctly prevents insert_storage_history's upsert from destroying post-genesis shard data; the tradeoff is documented in the code comment.
The fix_genesis_history → unwind_to → replay ordering in main.rs is logical.
Both fixes fire before snapshot_evm is ever opened.
Both methods correctly no-op for v1 archives via the use_hashed_state() guard.
Multi-shard safety in fix_genesis_account_history remains correct: combined single-pass collects already_fixed and shard_counts; accounts with > 1 shard are skipped.
Both fixes are now applied automatically at the end of a successful replay (lines 280–281), so new archive nodes get them without a separate maintenance step.
Both methods are idempotent.
Signal handling and graceful-stop path are untouched.

mlukanova · 2026-07-09T10:15:56Z

Closed - moved to #24

Replay archival genesis - storage re-key + account history seed

13e4365

mlukanova requested review from bronxyz, kstoykov and procdump as code owners July 7, 2026 10:23

Fmt

fb7fed5

Skip genesis accounts with multiple history shards (post-genesis chan…

88016d8

…gesets present)

Fmt

ffa1f72

Merge branch 'main' into fix/storages-history

05a6162

Add history fix for new archival nodes

a51a854

Add safety guard for post-genesis writes

31b416b

procdump mentioned this pull request Jul 9, 2026

fix(replay): repair genesis history for v2 (StorageV2) archives #24

Open

mlukanova closed this Jul 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replay archival genesis - storage re-key + account history seed#18

Replay archival genesis - storage re-key + account history seed#18
mlukanova wants to merge 7 commits into
mainfrom
fix/storages-history

mlukanova commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

mlukanova commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mlukanova commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Surface areas touched

Breaking / compatibility

Test plan

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

Issues

Bug risk — fix_genesis_account_history: ShardedKey::last is unsafe for accounts with post-genesis changesets

Correctness — fix_genesis_history: old plain-key entries are never removed

Performance — Full table scans for idempotency in both methods

Minor — map_or(false, ...) clippy lint

Minor — Empty test plan

What looks good

Uh oh!

github-actions Bot commented Jul 7, 2026

Security Scan - Code

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

Issues

Bug risk — fix_genesis_account_history: ShardedKey::last is unsafe for accounts with post-genesis changesets

Correctness — fix_genesis_history: stale plain-key entries are never removed

Performance — Full table scans in both methods

Usability — --snapshot-datadir is required even in fix-only mode

Minor — map_or(false, ...) clippy lint

What looks good

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

What the latest commit fixed

Remaining issues

Bug — open_db(&consensus_db) fires before the fix-mode early return

Correctness — fix_genesis_history: stale plain-key entries never removed

Performance — Full table scan in fix_genesis_history idempotency check

Usability — --snapshot-datadir required even in fix-only mode

Minor — map_or(false, …) clippy lint

What looks good

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

Issues

Correctness — Stale plain-key entries are never removed from StoragesHistory

Minor — map_or(false, ...) is a clippy warning

Minor — --snapshot-datadir is required even in fix-only mode

What looks good

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

What the latest commit fixed

Remaining issues

Correctness — Stale plain-key entries never removed from StoragesHistory

Bug — open_db called before the fix-mode early return

Usability — --snapshot-datadir required even in fix-only mode

Performance — Full table scans for idempotency in both methods

Minor — map_or(false, ...) clippy lint

What looks good

Uh oh!

github-actions Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

Issues

Bug — open_db fires before the fix-mode early return

Correctness — Stale plain-key entries never removed from StoragesHistory

Performance — Full table scan in fix_genesis_history idempotency check

mlukanova commented Jul 7, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Bug risk — `fix_genesis_account_history`: `ShardedKey::last` is unsafe for accounts with post-genesis changesets

Correctness — `fix_genesis_history`: old plain-key entries are never removed

Minor — `map_or(false, ...)` clippy lint

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Bug risk — `fix_genesis_account_history`: `ShardedKey::last` is unsafe for accounts with post-genesis changesets

Correctness — `fix_genesis_history`: stale plain-key entries are never removed

Usability — `--snapshot-datadir` is required even in fix-only mode

Minor — `map_or(false, ...)` clippy lint

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Bug — `open_db(&consensus_db)` fires before the fix-mode early return

Correctness — `fix_genesis_history`: stale plain-key entries never removed

Performance — Full table scan in `fix_genesis_history` idempotency check

Usability — `--snapshot-datadir` required even in fix-only mode

Minor — `map_or(false, …)` clippy lint

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Correctness — Stale plain-key entries are never removed from `StoragesHistory`

Minor — `map_or(false, ...)` is a clippy warning

Minor — `--snapshot-datadir` is required even in fix-only mode

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Correctness — Stale plain-key entries never removed from `StoragesHistory`

Bug — `open_db` called before the fix-mode early return

Usability — `--snapshot-datadir` required even in fix-only mode

Minor — `map_or(false, ...)` clippy lint

github-actions Bot commented Jul 8, 2026 •

edited

Loading

Bug — `open_db` fires before the fix-mode early return

Correctness — Stale plain-key entries never removed from `StoragesHistory`

Performance — Full table scan in `fix_genesis_history` idempotency check

Usability — `--snapshot-datadir` required even in fix-only mode

Minor — `map_or(false, ...)` clippy lint

github-actions Bot commented Jul 8, 2026 •

edited

Loading

Bug — `open_db` fires before the fix-mode early return

Correctness — Stale plain-key entries never removed from `StoragesHistory`

Performance — `fix_genesis_account_history` still scans the entire `AccountsHistory` table

Usability — `--snapshot-datadir` required even in fix-only mode