fix: fix backfill with null tipsets present#6409
Merged
hanabi1224 merged 1 commit intomainfrom Jan 12, 2026
Merged
Conversation
Contributor
WalkthroughThis pull request adds a changelog entry documenting a backfill fix for null tipsets and modifies the indexing command to clamp the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
LesnyRumcajs
force-pushed
the
fix-null-ts-backfill
branch
from
36ff519 to
8904876
Compare
LesnyRumcajs
requested review from
hanabi1224 and
sudo-shashank
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/tool/subcommands/index_cmd.rs (1)
97-97: Consider updating the displayed "From epoch" to reflect the clamped value.When the
fromepoch is clamped due to null tipsets at head, the output will display the original user-specified value rather than the actual clamped starting epoch. This could be slightly confusing if users notice the discrepancy.♻️ Suggested fix
- println!("From epoch: {}", from.unwrap_or_else(|| head_ts.epoch())); + let display_from = from.map(|f| std::cmp::min(f, head_ts.epoch())).unwrap_or_else(|| head_ts.epoch()); + println!("From epoch: {}", display_from);Alternatively, compute the clamped value earlier and reuse it:
+ let effective_from = from.map(|f| std::cmp::min(f, head_ts.epoch())); + println!("Database path: {}", db_root_dir.display()); - println!("From epoch: {}", from.unwrap_or_else(|| head_ts.epoch())); + println!("From epoch: {}", effective_from.unwrap_or_else(|| head_ts.epoch())); println!("{spec}"); println!("Head epoch: {}", head_ts.epoch()); - let from_ts = if let Some(from) = from { - // ensure from epoch is not greater than head epoch. This can happen if the - // assumed head is actually a null tipset. - let from = std::cmp::min(*from, head_ts.epoch()); + let from_ts = if let Some(from) = effective_from { chain_store.chain_index().tipset_by_height( from, head_ts, ResolveNullTipset::TakeOlder, )? } else { head_ts };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdsrc/tool/subcommands/index_cmd.rs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5944
File: src/chain/store/index.rs:0-0
Timestamp: 2025-08-18T03:09:47.932Z
Learning: In Forest's tipset_by_height caching implementation, hanabi1224 prefers performance-conscious solutions that leverage finality guarantees rather than expensive chain walking for fork detection. The approach of constraining cache lookups to finalized epochs (using CHECKPOINT_INTERVAL >= CHAIN_FINALITY) provides fork safety without the performance cost of ancestry verification.
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2025-08-25T13:35:24.230Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5969
File: src/tool/subcommands/snapshot_cmd.rs:412-412
Timestamp: 2025-08-25T13:35:24.230Z
Learning: In src/tool/subcommands/snapshot_cmd.rs, the +1 in `last_epoch = ts.epoch() - epochs as i64 + 1` fixes an off-by-1 bug where specifying --check-stateroots=N would validate N+1 epochs instead of N epochs, causing out-of-bounds errors when the snapshot contains only N recent state roots.
Applied to files:
src/tool/subcommands/index_cmd.rs
📚 Learning: 2025-09-16T12:55:26.955Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6079
File: src/chain_sync/metrics.rs:6-7
Timestamp: 2025-09-16T12:55:26.955Z
Learning: HEAD_EPOCH references in shell scripts (like scripts/tests/calibnet_eth_mapping_check.sh) that extract data from `forest-cli info show` are unrelated to Rust metrics constants with the same name and should not be flagged when metrics cleanup is performed.
Applied to files:
src/tool/subcommands/index_cmd.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/tool/subcommands/index_cmd.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/tool/subcommands/index_cmd.rs
📚 Learning: 2025-08-18T03:09:47.932Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5944
File: src/chain/store/index.rs:0-0
Timestamp: 2025-08-18T03:09:47.932Z
Learning: In Forest's tipset_by_height caching implementation, hanabi1224 prefers performance-conscious solutions that leverage finality guarantees rather than expensive chain walking for fork detection. The approach of constraining cache lookups to finalized epochs (using CHECKPOINT_INTERVAL >= CHAIN_FINALITY) provides fork safety without the performance cost of ancestry verification.
Applied to files:
CHANGELOG.md
⏰ 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). (8)
- GitHub Check: Coverage
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: All lint checks
- GitHub Check: rubocop
🔇 Additional comments (2)
CHANGELOG.md (1)
52-53: LGTM!The changelog entry is properly formatted, concise, and correctly placed under the "Fixed" section.
src/tool/subcommands/index_cmd.rs (1)
101-112: LGTM!The fix correctly addresses the null tipset issue by clamping the
fromepoch to not exceed the head epoch. The explanatory comment clearly documents the edge case being handled. The use ofstd::cmp::mincombined withResolveNullTipset::TakeOlderensures robust behavior when dealing with null tipsets.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
hanabi1224
approved these changes
Jan 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
due to the null tipset https://calibration.filscan.io/en/height/3363360/
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.