feat(api): implement the ChainGetFinalizedTipset API
#6133
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
WalkthroughReplaces the stubbed ChainGetFinalizedTipset handler with real finality resolution: computes EC finality epoch from the heaviest tipset, attempts F3 finality via a new helper, and falls back to EC finality on errors. Expands RPC handler contexts to require Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant RPC as ChainGetFinalizedTipset
participant Ctx as Node/Ctx
participant CI as ChainIndex
participant F3 as F3 service
Client->>RPC: ChainGetFinalizedTipset()
RPC->>Ctx: fetch heaviest tipset
RPC->>CI: compute EC finality epoch
RPC->>F3: request latest certificate + head
alt F3 certificate present and head >= EC epoch
RPC->>CI: load tipset at F3 head
CI-->>RPC: F3 finalized tipset
RPC-->>Client: return F3 finalized tipset
else F3 missing/invalid/error
Note over RPC: warn and fallback to EC finality
RPC->>CI: load tipset at EC finality epoch
CI-->>RPC: EC finalized tipset
RPC-->>Client: return EC finalized tipset
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (1)
src/rpc/methods/chain.rs (1)
151-165: Include error context in the fallback warning.When we drop into the EC path we lose the original failure reason, which makes field debugging harder. Please log the error (e.g. with
tracing::warn!(error = %err, ...)) before falling back.- match get_f3_finality_tipset(&ctx, ec_finality_epoch).await { + match get_f3_finality_tipset(&ctx, ec_finality_epoch).await { Ok(f3_tipset) => { tracing::debug!("Using F3 finalized tipset at epoch {}", f3_tipset.epoch()); Ok((*f3_tipset).clone()) } - Err(_) => { - // fallback to ec finality - tracing::warn!("F3 finalization unavailable, falling back to EC finality"); + Err(error) => { + // fallback to ec finality + tracing::warn!( + error = %error, + "F3 finalization unavailable, falling back to EC finality" + ); let ec_tipset = ctx.chain_index().tipset_by_height( ec_finality_epoch, head, ResolveNullTipset::TakeOlder, )?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/rpc/methods/chain.rs(1 hunks)src/rpc/methods/f3.rs(2 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
src/rpc/reflect/mod.rs (1)
request(250-260)src/rpc/client.rs (1)
request(272-285)
src/rpc/methods/chain.rs (1)
src/blocks/tipset.rs (2)
epoch(306-308)epoch(543-545)
src/rpc/methods/f3.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: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: All lint checks
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
hanabi1224
reviewed
Oct 8, 2025
src/rpc/methods/chain.rs
Outdated
| (): Self::Params, | ||
| ) -> Result<Self::Ok, ServerError> { | ||
| let head = ctx.chain_store().heaviest_tipset(); | ||
| let ec_finality_epoch = head.epoch() - ctx.chain_config().policy.chain_finality; |
There was a problem hiding this comment.
Suggested change
| let ec_finality_epoch = head.epoch() - ctx.chain_config().policy.chain_finality; | |
| let ec_finality_epoch = (head.epoch() - ctx.chain_config().policy.chain_finality).max(0); |
elmattic
reviewed
Oct 8, 2025
| match get_f3_finality_tipset(&ctx, ec_finality_epoch).await { | ||
| Ok(f3_tipset) => { | ||
| tracing::debug!("Using F3 finalized tipset at epoch {}", f3_tipset.epoch()); | ||
| Ok((*f3_tipset).clone()) |
There was a problem hiding this comment.
Could we avoid cloning here?
There was a problem hiding this comment.
I don't think so f3_tipset wrapped in Arc so we will have to clone only.
hanabi1224
reviewed
Oct 8, 2025
| .await | ||
| .map_err(|e| anyhow::anyhow!("Failed to get F3 certificate: {}", e))?; | ||
|
|
||
| let f3_finalized_head = f3_finalized_cert.chain_head(); |
There was a problem hiding this comment.
f3_finalized_cert.chain_head() could be newer than chain head during catchup. Lotus does not seem to handle that case, should we handle it? e.g. marking finalized tipset in F3.Finalize RPC method instead.
There was a problem hiding this comment.
We could track this in a separate issue, for now the logic matches Lotus
akaladarshi
force-pushed
the
akaladarshi/add-chain-get-finalized-tipset-api
branch
from
b215a7d to
de68dc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/rpc/methods/chain.rs (1)
143-167: LGTM! Well-structured finality resolution with F3 primary and EC fallback.The implementation correctly:
- Computes EC finality epoch with
.max(0)to prevent negative values- Attempts F3 finality first via the new helper
- Falls back to EC finality on errors with appropriate logging
Optional: Log the discarded error for easier debugging.
Currently, the error from
get_f3_finality_tipset(line 156) is silently discarded. Consider logging it at debug level to aid troubleshooting:match get_f3_finality_tipset(&ctx, ec_finality_epoch).await { Ok(f3_tipset) => { tracing::debug!("Using F3 finalized tipset at epoch {}", f3_tipset.epoch()); Ok((*f3_tipset).clone()) } - Err(_) => { + Err(e) => { // fallback to ec finality - tracing::warn!("F3 finalization unavailable, falling back to EC finality"); + tracing::warn!("F3 finalization unavailable, falling back to EC finality: {}", e); let ec_tipset = ctx.chain_index().tipset_by_height(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/rpc/methods/chain.rs(1 hunks)src/rpc/methods/f3.rs(2 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tool/subcommands/api_cmd/api_compare_tests.rs
- src/tool/subcommands/api_cmd/test_snapshots.txt
🧰 Additional context used
🧬 Code graph analysis (2)
src/rpc/methods/chain.rs (2)
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)src/blocks/tipset.rs (2)
epoch(306-308)epoch(543-545)
src/rpc/methods/f3.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: tests
- GitHub Check: tests-release
- GitHub Check: All lint checks
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
🔇 Additional comments (3)
src/rpc/methods/f3.rs (2)
701-704: LGTM! Signature update enables cross-thread async usage.Adding
Send + Sync + 'staticbounds aligns with other async RPC handlers (e.g.,StateCall,StateReplayinstate.rs) and is necessary for calling this method fromChainGetFinalizedTipsetinchain.rs.
954-954: LGTM! Visibility change supports cross-module usage.Exposing
get_rpc_http_clientpublicly enables the newChainGetFinalizedTipsetimplementation inchain.rsto interact with the F3 sidecar.src/rpc/methods/chain.rs (1)
170-197: LGTM! Robust F3 finality helper with proper validation.The helper correctly:
- Fetches the latest F3 certificate via
F3GetLatestCertificate::handle- Validates that F3 finality is not behind EC finality (lines 180-186)
- Loads the corresponding tipset from the chain index
- Provides descriptive error messages for debugging
hanabi1224
approved these changes
Oct 9, 2025
elmattic
approved these changes
Oct 9, 2025
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:
ChainGetFinalizedTipsetAPIbasiccheck since it is dependent on F3 and the latest tipset.Reference issue to close (if applicable)
Closes #6113
Other information and links
Change checklist
Summary by CodeRabbit