Conversation
…bases. `read_chain_id_from_db` deserialized the full `ChainConfig` from the existing DB, which fails on pre-v10 data because `terminal_total_difficulty` (a u128 exceeding u64::MAX on mainnet) was stored via serde_json's default encoding that goes through f64 and loses integer precision, while v10's new `hex_str_opt` deserializer strictly requires a parseable u128 number or hex string. The migration check then silently swallowed the error via `.ok()?` and `migrate_datadir_if_needed` fell through to starting an empty DB under the new network-specific subdirectory, orphaning the user's synced state without any log message. Extract only `chain_id` from the stored JSON (the only field the migration check actually needs) so unrelated `ChainConfig` schema evolution doesn't block otherwise-valid migrations. Replace every silent `.ok()?` in `read_chain_id_from_db` with a matched error that logs the specific failure, and add a user-facing warning in `migrate_datadir_if_needed` when a valid metadata.json is present but the chain ID can't be read, instructing the operator how to move the data manually.
ilitteri
requested review from
a team,
ManuelBilbao and
avilagaston9
as code owners
🤖 Kimi Code ReviewThis PR bumps the version from 9.0.0 to 10.0.0 across the workspace and improves database migration robustness between versions. The changes are clean and the error handling improvements are well-designed. Summary of Changes
Code Review
|
Greptile SummaryThis PR bumps the workspace version from 9.0.0 to 10.0.0 across all Cargo manifests and updates the Confidence Score: 5/5Safe to merge — version bump is mechanical and the hotfix is well-targeted with no regressions. All changes are either version number updates or a narrowly scoped, well-commented fix to a silent failure in the migration read path. The partial-deserialization approach is sound (camelCase mapping matches ChainConfig serialization), warnings are correctly ordered in the log stream, and the PR was verified against a real 449 GB v9 mainnet database. No files require special attention.
|
| Filename | Overview |
|---|---|
| crates/storage/store.rs | Hotfix: replaced silent .ok()? chain in read_chain_id_from_db with explicit match arms emitting warn! logs, and swapped full ChainConfig deserialization for a minimal ChainIdOnly struct to tolerate v9 terminal_total_difficulty format differences. |
| cmd/ethrex/initializers.rs | Added actionable warn! message when read_chain_id_from_db returns None, guiding operators on how to recover a pre-v10 database that cannot be migrated automatically. |
| Cargo.toml | Workspace version bumped from 9.0.0 to 10.0.0. |
| docs/CLI.md | Updated --builder.extra-data default from "ethrex 9.0.0" to "ethrex 10.0.0" in both the ethrex and ethrex L2 sections. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[migrate_datadir_if_needed] --> B{base_datadir == network_datadir?}
B -- Yes --> Z[return: no-op]
B -- No --> C{has_valid_db network_datadir?}
C -- Yes --> Z
C -- No --> D{has_valid_db base_datadir?}
D -- No --> Z
D -- Yes --> E{network subdirs exist?}
E -- Yes --> Z
E -- No --> F[read_chain_id_from_db]
F --> G{RocksDB open?}
G -- Err --> H[warn: failed to open]
H --> Z
G -- Ok --> I{begin_read?}
I -- Err --> J[warn: failed to begin read]
J --> Z
I -- Ok --> K{get chain config bytes?}
K -- None --> L[warn: entry not found]
L --> Z
K -- Err --> M[warn: read failed]
M --> Z
K -- Some --> N{deserialize ChainIdOnly - chain_id only}
N -- Err --> O[warn: deserialize failed]
O --> Z
N -- Ok --> P[return Some chain_id]
P --> Q{chain_id matches expected?}
Q -- No --> R[warn: chain ID mismatch]
R --> Z
Q -- Yes --> S{no_migrate flag?}
S -- Yes --> T[info: skipping]
T --> Z
S -- No --> U[Migrate: move files to network_datadir]
Reviews (1): Last reviewed commit: "Fix silent datadir migration failure whe..." | Re-trigger Greptile
🤖 Claude Code ReviewI now have all the context needed. Here is the full review. PR Review:
|
Lines of code reportTotal lines added: Detailed view |
There was a problem hiding this comment.
Pull request overview
Prepares the v10.0.0 release by bumping workspace/package versions and updating documentation, while also including a hotfix to improve diagnosability of datadir migration by logging specific chain-config read failures.
Changes:
- Bump versions from
9.0.0to10.0.0across the workspace, guest programs, and quote-gen; refresh lockfiles. - Update CLI docs default
--builder.extra-datatoethrex 10.0.0. - Improve
read_chain_id_from_dbfailure handling by logging warnings per failure mode and deserializing onlychain_idfrom storedChainConfig.
Reviewed changes
Copilot reviewed 5 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Workspace version bump to 10.0.0. |
| Cargo.lock | Refresh root lockfile to reflect 10.0.0 workspace crates. |
| tooling/Cargo.lock | Refresh tooling lockfile for 10.0.0 workspace crate versions. |
| docs/CLI.md | Update documented default for --builder.extra-data to ethrex 10.0.0. |
| crates/storage/store.rs | Hotfix: log warn-level diagnostics on chain-id read failures; deserialize only chain_id. |
| cmd/ethrex/initializers.rs | Emit operator-facing warning when chain ID cannot be read during migration gating. |
| crates/l2/tee/quote-gen/Cargo.toml | Bump quote-gen crate version to 10.0.0. |
| crates/l2/tee/quote-gen/Cargo.lock | Refresh lockfile for quote-gen dependencies to 10.0.0 workspace versions. |
| crates/vm/levm/bench/revm_comparison/Cargo.lock | Refresh bench lockfile for 10.0.0 workspace versions. |
| crates/guest-program/bin/sp1/Cargo.toml | Bump guest SP1 package version to 10.0.0. |
| crates/guest-program/bin/sp1/Cargo.lock | Refresh guest SP1 lockfile to 10.0.0 workspace versions. |
| crates/guest-program/bin/risc0/Cargo.toml | Bump guest RISC0 package version to 10.0.0. |
| crates/guest-program/bin/risc0/Cargo.lock | Refresh guest RISC0 lockfile to 10.0.0 workspace versions. |
| crates/guest-program/bin/openvm/Cargo.toml | Bump guest OpenVM package version to 10.0.0. |
| crates/guest-program/bin/openvm/Cargo.lock | Refresh guest OpenVM lockfile to 10.0.0 workspace versions. |
| crates/guest-program/bin/zisk/Cargo.toml | Bump guest ZisK package version to 10.0.0. |
| crates/guest-program/bin/zisk/Cargo.lock | Refresh guest ZisK lockfile to 10.0.0 workspace versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| thread::JoinHandle, | ||
| }; | ||
| use tracing::{debug, error, info}; | ||
| use tracing::{debug, error, info, warn}; |
🤖 Codex Code Review
No blocking findings in the runtime code changes. The narrowed I couldn’t run the targeted Rust tests in this environment because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
The DB_SCHEMA array runs as a single transaction during init_db(). On an existing v9 database the latest_sent table has two columns (_id, batch), but the INSERT statement tried to supply three values (including the new verified_at column added in v10). This caused a SQLite error that rolled back the entire schema transaction, preventing the subsequent ALTER TABLE migration from ever executing. By specifying column names in the INSERT, the statement works on both the old 2-column table and the new 3-column table (verified_at gets its DEFAULT 0). The ALTER TABLE migration then adds the column to existing databases as intended.
…/store.rs. The `warn!` calls in `read_chain_id_from_db` live inside `#[cfg(feature = "rocksdb")]`, so when `Lint L2` runs `cargo clippy --workspace --features l2,l2-sql` (without the rocksdb feature) the top-level `use tracing::warn` was unused and failed with `-D warnings`.
tracing imports so the cfg-gated `use tracing::warn;` precedes the
unconditional `use tracing::{debug, error, info};`, as rustfmt expects.
Motivation
Prepare the v10.0.0 release by bumping the workspace version and merging the release branch back to main. The branch also carries a hotfix found during pre-release testing on the mainnet snapsync test servers.
Description
Cargo.toml, the guest-programCargo.tomlfiles (sp1, risc0, openvm, zisk) andcrates/l2/tee/quote-gen/Cargo.toml; refresh lockfiles.--builder.extra-datadefault indocs/CLI.mdtoethrex 10.0.0in both ethrex and ethrex L2 sections.read_chain_id_from_dbnow extracts onlychain_idfrom the storedChainConfig(unaffected by the newhex_str_optattribute onterminal_total_difficulty) and each failure mode logs awarninstead of being swallowed by.ok()?. Verified on a real v9 mainnet database (449 GB): migrated into the new~/.local/share/ethrex/mainnet/layout and resumed at the same block without re-sync.Checklist
STORE_SCHEMA_VERSION— N/A, no storage schema change (hotfix only affects the migration read path).