fix(f3): skip importing F3 data when initial power table is not set in the F3 manifest #6397
Conversation
…n the F3 manifest
WalkthroughThis PR updates the go-jsonrpc dependency from v0.9.0 to v0.10.0 and adds a conditional guard in the F3 snapshot import logic that checks for the presence of initial_power_table in the manifest before proceeding with the import. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Comment |
hanabi1224
requested review from
LesnyRumcajs and
akaladarshi
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/daemon/db_util.rs (1)
251-254: Consider logging the F3 CID for better traceability.The warning message clearly explains why the import is skipped. For debugging and audit purposes, consider including the
f3_cidin the log message to provide complete context about which F3 data is being skipped.📝 Optional enhancement to include F3 CID in warning
{ - // To avoid importing old/wrong F3 data without initial power table check tracing::warn!( - "skipped importing F3 data as the initial power table CID is not set in the current manifest" + "Skipped importing F3 data (CID: {}) as the initial power table CID is not set in the current manifest", + f3_cid );
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
f3-sidecar/go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
f3-sidecar/go.modsrc/daemon/db_util.rs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: src/daemon/db_util.rs:236-259
Timestamp: 2025-08-11T14:00:47.060Z
Learning: In Forest's snapshot import (`src/daemon/db_util.rs`), when F3 CID is present in snapshot metadata but the actual F3 data is missing, this should cause a hard error as it indicates snapshot corruption. Only errors during the F3 import process itself (after data is successfully retrieved) should be non-fatal and logged.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-11T14:00:47.060Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: src/daemon/db_util.rs:236-259
Timestamp: 2025-08-11T14:00:47.060Z
Learning: In Forest's snapshot import (`src/daemon/db_util.rs`), when F3 CID is present in snapshot metadata but the actual F3 data is missing, this should cause a hard error as it indicates snapshot corruption. Only errors during the F3 import process itself (after data is successfully retrieved) should be non-fatal and logged.
Applied to files:
src/daemon/db_util.rs
📚 Learning: 2025-10-17T14:24:47.046Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.
Applied to files:
src/daemon/db_util.rs
📚 Learning: 2025-08-06T14:36:08.654Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: src/daemon/db_util.rs:244-259
Timestamp: 2025-08-06T14:36:08.654Z
Learning: The `import_f3_snapshot` function already prints success messages internally, so additional success logging at the call site is not needed.
Applied to files:
src/daemon/db_util.rs
📚 Learning: 2026-01-05T12:54:40.850Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/cron_state.rs:8-8
Timestamp: 2026-01-05T12:54:40.850Z
Learning: In Rust code reviews, do not derive Eq for a struct if any field does not implement Eq (e.g., types from external dependencies). If a type like CronStateLotusJson includes fields wrapping external dependencies that lack Eq, derive PartialEq (or implement PartialEq manually) but avoid deriving Eq. This ensures comparisons compile and reflect actual equivalence semantics. When needed, consider implementing custom PartialEq (and possibly Eq) only after ensuring all fields (or wrappers) implement Eq, or keep PartialEq-only if full equality semantics cannot be expressed.
Applied to files:
src/daemon/db_util.rs
📚 Learning: 2026-01-05T12:56:13.802Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/evm_state.rs:41-44
Timestamp: 2026-01-05T12:56:13.802Z
Learning: In Rust codebases (e.g., Forest), do not add #[cfg(test)] to functions already annotated with #[test]. The #[test] attribute ensures the function is compiled only for tests, so a separate #[cfg(test)] is redundant and can be removed if present. Apply this check to all Rust files that contain #[test] functions.
Applied to files:
src/daemon/db_util.rs
🧬 Code graph analysis (1)
src/daemon/db_util.rs (1)
src/f3/mod.rs (2)
get_f3_sidecar_params(37-89)import_f3_snapshot(120-144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Coverage
- GitHub Check: tests-release
- GitHub Check: All lint checks
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
🔇 Additional comments (3)
src/daemon/db_util.rs (2)
247-249: LGTM: Conditional guard aligns with PR objectives.The check correctly determines whether the initial power table is configured before proceeding with F3 import. This prevents importing potentially stale or incorrect F3 data during the calibnet F3 reset.
255-276: LGTM: Existing F3 import logic preserved with correct error handling.The F3 import flow maintains the established error handling patterns:
- Hard error if F3 data is missing despite CID being present (Line 258), indicating snapshot corruption
- Non-fatal error logging if the import process fails (Line 273-274), allowing the snapshot import to proceed
Based on learnings, this error handling strategy is correct and intentional.
f3-sidecar/go.mod (1)
7-7: No blocking issues identified with the version upgrade.The go-jsonrpc v0.10.0 release (Jan 8, 2026) is a feature addition that introduces a method name formatter option for client handlers. No breaking changes or security advisories were found for this version, making it a straightforward upgrade.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
LesnyRumcajs
deleted the
hm/f3-import-enforce-initial-power-table-check
branch
Summary of changes
(Part of F3 reset on calibnet)
To avoid importing old/wrong F3 data without initial power table check
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.