spec: Add ChainGetFinalizedTipset API
#6114
Merged
Conversation
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
akaladarshi
requested review from
LesnyRumcajs and
sudo-shashank
and removed request for
a team
WalkthroughAdds a new public RPC method Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as RPC Server
participant CH as Chain RPC Handler
C->>S: Filecoin.ChainGetFinalizedTipSet()
S->>CH: invoke ChainGetFinalizedTipset::handle()
note right of CH: handler is stubbed / not implemented<br/>metadata: prefer F3 finality, fallback to EC
CH-->>S: Error (stubbed)
S-->>C: JSON-RPC error response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ 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)
🔇 Additional comments (1)
Comment |
![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 (2)
src/rpc/methods/chain.rs (2)
130-150: Rename type to ChainGetFinalizedTipSet; tighten docs; align API_PATHS style.
- Type name uses “Tipset” but the codebase consistently uses “TipSet” in public RPC type names (e.g., ChainGetTipSet). Recommend renaming for consistency and to avoid confusion.
- Improve the doc comment grammar and casing for F3.
- Prefer constructing BitFlags without the macro for consistency/readability.
Apply:
-/// Returns the latest finalized tipset. -/// It uses the current F3 instance to determine the finalized tipset. -/// If F3 is operational and finalizing in this node. If not, it will fall back -/// to the Expected Consensus (EC) finality definition of head - 900 epochs. +/// Returns the latest finalized tipset. +/// Uses the current F3 instance to determine the finalized tipset when F3 is operational on this node; +/// otherwise falls back to the Expected Consensus (EC) definition (head - 900 epochs). - -pub enum ChainGetFinalizedTipset {} -impl RpcMethod<0> for ChainGetFinalizedTipset { +pub enum ChainGetFinalizedTipSet {} +impl RpcMethod<0> for ChainGetFinalizedTipSet { const NAME: &'static str = "Filecoin.ChainGetFinalizedTipSet"; const PARAM_NAMES: [&'static str; 0] = []; - const API_PATHS: BitFlags<ApiPaths> = make_bitflags!(ApiPaths::V1); + const API_PATHS: BitFlags<ApiPaths> = BitFlags::from_flag(ApiPaths::V1); const PERMISSION: Permission = Permission::Read; const DESCRIPTION: Option<&'static str> = Some( - "Returns the latest finalized tipset. It uses the f3 instance or Expected Consensus (EC) definition (head - 900 epochs) to get the finalized tipset.", + "Returns the latest finalized tipset. It uses the F3 instance or Expected Consensus (EC) definition (head - 900 epochs) to get the finalized tipset.", ); type Params = (); type Ok = Tipset; async fn handle(_ctx: Ctx<impl Blockstore>, (): Self::Params) -> Result<Self::Ok, ServerError> { - Err(anyhow::anyhow!("ChainGetFinalizedTipset is not yet implemented").into()) + Err(anyhow::anyhow!("ChainGetFinalizedTipSet is not yet implemented").into()) } }
5-5: Avoid importing the macro; use BitFlags::from_flag instead.Drop make_bitflags and use BitFlags::from_flag(ApiPaths::V1) to keep imports lean and consistent with the rest of the file.
-use enumflags2::{BitFlags, make_bitflags}; +use enumflags2::BitFlags;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/rpc/methods/chain.rs(2 hunks)src/rpc/mod.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: hanabi1224
PR: ChainSafe/forest#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.
🧬 Code graph analysis (1)
src/rpc/methods/chain.rs (1)
src/rpc/methods/state.rs (16)
handle(109-114)handle(132-138)handle(151-157)handle(172-178)handle(196-205)handle(223-231)handle(248-255)handle(272-282)handle(298-305)handle(335-465)handle(484-492)handle(508-538)handle(555-561)handle(577-597)handle(615-624)handle(641-663)
⏰ 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 Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: cargo-publish-dry-run
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: All lint checks
- GitHub Check: Build forest binaries on Linux AMD64
🔇 Additional comments (1)
src/rpc/mod.rs (1)
68-68: Ignore suggested rename — repository still uses ChainGetFinalizedTipsetThe repo defines
pub enum ChainGetFinalizedTipset(src/rpc/methods/chain.rs) and the registration in src/rpc/mod.rs:68 uses that same name; the suggested change toChainGetFinalizedTipSetis not present.Likely an incorrect or invalid review comment.
LesnyRumcajs
requested changes
Sep 24, 2025
src/rpc/methods/chain.rs
Outdated
| const API_PATHS: BitFlags<ApiPaths> = make_bitflags!(ApiPaths::V1); | ||
| const PERMISSION: Permission = Permission::Read; | ||
| const DESCRIPTION: Option<&'static str> = Some( | ||
| "Returns the latest finalized tipset. It uses the f3 instance or Expected Consensus (EC) definition (head - 900 epochs) to get the finalized tipset.", |
There was a problem hiding this comment.
Suggested change
| "Returns the latest finalized tipset. It uses the f3 instance or Expected Consensus (EC) definition (head - 900 epochs) to get the finalized tipset.", | |
| "Returns the latest F3 finalized tipset, or falls back to EC finality if F3 is not operational on the node or if the F3 finalized tipset is further back than EC finalized tipset.", |
Let's just use the text from filecoin-project/FIPs#1199?
src/rpc/methods/chain.rs
Outdated
Comment on lines
130
to
133
| /// Returns the latest finalized tipset. | ||
| /// It uses the current F3 instance to determine the finalized tipset. | ||
| /// If F3 is operational and finalizing in this node. If not, it will fall back | ||
| /// to the Expected Consensus (EC) finality definition of head - 900 epochs. |
There was a problem hiding this comment.
Suggested change
| /// Returns the latest finalized tipset. | |
| /// It uses the current F3 instance to determine the finalized tipset. | |
| /// If F3 is operational and finalizing in this node. If not, it will fall back | |
| /// to the Expected Consensus (EC) finality definition of head - 900 epochs. |
redundant
src/rpc/methods/chain.rs
Outdated
| type Ok = Tipset; | ||
|
|
||
| async fn handle(_ctx: Ctx<impl Blockstore>, (): Self::Params) -> Result<Self::Ok, ServerError> { | ||
| Err(anyhow::anyhow!("ChainGetFinalizedTipset is not yet implemented").into()) |
There was a problem hiding this comment.
There's a dedicated method for this.
Lines 63 to 74 in 22d626c
There was a problem hiding this comment.
Thank 🙏, Didn't knew about this.
Done.
LesnyRumcajs
approved these changes
Sep 24, 2025
sudo-shashank
approved these changes
Sep 30, 2025
sudo-shashank
deleted the
akaladarshi/add-f3-finalised-tipset-api-spec
branch
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
ChainGetFinalizedTipsetAPIforest-toolto generate API spec. It will be implemented in a follow up PR tracking here ImplementChainGetFinalizedTipsetAPI #6113Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Chores