feat(rpc): impl Filecoin.ChainGetTipSetFinalityStatus#6811
Merged
hanabi1224 merged 20 commits intomainfrom Apr 7, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements the Changes
Sequence DiagramsequenceDiagram
actor Client
participant RPC Handler
participant EC Finality Calculator
participant Blockstore/Chain
Client->>RPC Handler: Filecoin.ChainGetTipSetFinalityStatus()
RPC Handler->>RPC Handler: Check cached threshold
alt Cache Miss
RPC Handler->>Blockstore/Chain: Get current head tipset
RPC Handler->>Blockstore/Chain: Walk parent epochs (building chain)
RPC Handler->>EC Finality Calculator: find_threshold_depth(parent_epochs)
EC Finality Calculator-->>RPC Handler: threshold_depth
RPC Handler->>Blockstore/Chain: Get tipset at threshold height
RPC Handler->>RPC Handler: Cache result by head
end
alt Both EC and F3 finalized exist
RPC Handler->>RPC Handler: Select finalized_tip_set by epoch
else EC only or F3 only
RPC Handler->>RPC Handler: Use whichever is present
end
RPC Handler-->>Client: ChainFinalityStatus {ec_finality_threshold_depth, ec_finalized_tip_set, f3_finalized_tip_set, finalized_tip_set, head}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
hanabi1224
force-pushed
the
hm/ChainGetTipSetFinalityStatus
branch
from
50e2ab7 to
00abf12
Compare
hanabi1224
force-pushed
the
hm/ChainGetTipSetFinalityStatus
branch
from
59f3462 to
6db0a02
Compare
hanabi1224
requested review from
LesnyRumcajs 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: 2
🧹 Nitpick comments (2)
src/rpc/methods/chain.rs (2)
1236-1245: Redundant.clone()on tipset.The
tsis already owned from theOk(ts)binding, so the.clone()is unnecessary.Proposed fix
let finalized = if depth >= 0 && let Ok(ts) = ctx.chain_index().tipset_by_height( (head.epoch() - depth).max(0), head, ResolveNullTipset::TakeOlder, ) { - Some(ts.clone()) + Some(ts) } else { None };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/chain.rs` around lines 1236 - 1245, The code constructs `finalized` by matching `let Ok(ts) = ctx.chain_index().tipset_by_height(...)` but then calls `ts.clone()` even though `ts` is already moved into the `Ok(ts)` binding; remove the redundant `.clone()` and use `ts` directly when returning `Some(ts)` in the `finalized` assignment (the change affects the block that references `finalized`, the `let Ok(ts)` pattern, `ctx.chain_index().tipset_by_height`, `head`, and `ResolveNullTipset::TakeOlder`).
1261-1267: Consider async blocking for cache miss path.On a cache miss,
get_finality_statuswalks ~905 epochs through the blockstore and runs a binary search with floating-point probability calculations viafind_threshold_depth. While blockstore lookups are cached and the binary search is bounded (O(log 450)), this synchronous work could briefly block the async runtime on every new block when the cache invalidates.Cache hits will be frequent in practice since the cache invalidates only when
heaviest_tipsetchanges, which happens infrequently compared to typical RPC request rates. However, if high RPC concurrency is a concern for your load patterns, consider usingtokio::task::spawn_blockingfor the cache miss path, consistent with similar CPU-bound work likeeth.rs::execution_trace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/chain.rs` around lines 1261 - 1267, The handler currently calls get_finality_status synchronously inside handle, which can perform heavy CPU/blocking work on cache misses; change handle to offload that work to a blocking thread by invoking tokio::task::spawn_blocking (or equivalent) for get_finality_status and awaiting its JoinHandle so the async runtime isn't blocked on the ~905-epoch walk and binary search; update the handle function to call spawn_blocking(|| Self::get_finality_status(&ctx)) and await the result, mirroring the pattern used for other CPU-bound work like eth.rs::execution_trace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/chain/ec_finality/calculator/mod.rs`:
- Around line 27-29: Update the test comment in the EC finality calculator tests
to reflect the new BISECT_HIGH value (450) instead of 200: locate the comment
above the all-ones chain test in tests.rs (referencing the BISECT/BisectHigh
behavior) and change its text to mention "BisectHigh=450" or "BISECT_HIGH=450"
(e.g., "// All-1s chain is too degraded to achieve 2^-30 within the bisect
search range (BisectHigh=450), so threshold is not found"); no logic changes
needed—the test already references the BISECT_HIGH constant.
In `@src/rpc/methods/chain/types.rs`:
- Around line 14-16: The doc comment block above the type that begins "Describes
how the node is currently determining finality" has a regular comment line ("//
combining probabilistic...") which breaks the doc comment flow; change that line
to a doc comment ("/// combining probabilistic...") so the entire comment block
is contiguous and will appear in generated documentation for the associated
type/enum in types.rs.
---
Nitpick comments:
In `@src/rpc/methods/chain.rs`:
- Around line 1236-1245: The code constructs `finalized` by matching `let Ok(ts)
= ctx.chain_index().tipset_by_height(...)` but then calls `ts.clone()` even
though `ts` is already moved into the `Ok(ts)` binding; remove the redundant
`.clone()` and use `ts` directly when returning `Some(ts)` in the `finalized`
assignment (the change affects the block that references `finalized`, the `let
Ok(ts)` pattern, `ctx.chain_index().tipset_by_height`, `head`, and
`ResolveNullTipset::TakeOlder`).
- Around line 1261-1267: The handler currently calls get_finality_status
synchronously inside handle, which can perform heavy CPU/blocking work on cache
misses; change handle to offload that work to a blocking thread by invoking
tokio::task::spawn_blocking (or equivalent) for get_finality_status and awaiting
its JoinHandle so the async runtime isn't blocked on the ~905-epoch walk and
binary search; update the handle function to call spawn_blocking(||
Self::get_finality_status(&ctx)) and await the result, mirroring the pattern
used for other CPU-bound work like eth.rs::execution_trace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3713c0e0-a502-4285-a18b-3d12c21fceb8
⛔ Files ignored due to path filters (1)
src/rpc/snapshots/forest__rpc__tests__rpc__v2.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
scripts/tests/api_compare/.envscripts/tests/api_compare/filter-list-gatewaysrc/chain/ec_finality/calculator/mod.rssrc/chain/ec_finality/calculator/tests.rssrc/chain/ec_finality/mod.rssrc/chain/mod.rssrc/rpc/methods/chain.rssrc/rpc/methods/chain/types.rssrc/rpc/mod.rssrc/tool/subcommands/api_cmd/api_compare_tests.rssrc/tool/subcommands/api_cmd/test_snapshots.txt
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Member
|
no green checkmark, no review! |
LesnyRumcajs
reviewed
Apr 7, 2026
LesnyRumcajs
reviewed
Apr 7, 2026
LesnyRumcajs
reviewed
Apr 7, 2026
LesnyRumcajs
reviewed
Apr 7, 2026
LesnyRumcajs
approved these changes
Apr 7, 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
Part of #6769
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Release Notes
New Features
Filecoin.ChainGetTipSetFinalityStatusRPC method for querying tip set finality information, including EC and F3 finalization thresholds, finalized tip sets, and current chain head details.Chores