fix: source current round timing on-chain instead of the subgraph#731
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe current-round API now reads round metadata and block numbers directly from the on-chain ChangesCurrent round API
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens the “Current Round” tracker against subgraph stalls by sourcing round timing directly from the on-chain RoundsManager, ensuring round calculations stay internally consistent even when subgraph indexing halts.
Changes:
- Update
/api/current-roundto readcurrentRound,currentRoundStartBlock,currentRoundInitialized, andblockNumfromRoundsManagervial2PublicClient. - Reintroduce overdue-round display guards in the RoundStatus UI (clamping remaining blocks/progress and showing “awaiting an orchestrator” copy).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pages/api/current-round.tsx | Switches current-round timing fields from subgraph-derived values to on-chain RoundsManager reads. |
| components/RoundStatus/index.tsx | Restores overdue-round UI guards and clamps to prevent negative blocks remaining / >100% progress. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [id, startBlock, initialized, currentL1Block, currentL2Block] = | ||
| await Promise.all([ | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRound", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRoundStartBlock", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRoundInitialized", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "blockNum", | ||
| }), | ||
| l2PublicClient.getBlockNumber(), | ||
| ]); |
There was a problem hiding this comment.
This shuold be handled internally in the client call.
| const blocksRemaining = useMemo( | ||
| () => | ||
| currentRoundInfo?.initialized && protocol | ||
| ? +protocol.roundLength - | ||
| (+Number(currentRoundInfo.currentL1Block) - | ||
| +Number(currentRoundInfo.startBlock)) | ||
| protocol | ||
| ? Math.max(+protocol.roundLength - blocksSinceCurrentRoundStart, 0) | ||
| : 0, | ||
| [protocol, currentRoundInfo] | ||
| [protocol, blocksSinceCurrentRoundStart] | ||
| ); |
5b83552 to
4de4dfe
Compare
ed26d0e to
6ecffb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pages/api/current-round.tsx`:
- Around line 29-48: Replace the parallel readContract calls in the
current-round data-loading flow with l2PublicClient.multicall, preserving the
same five contract function reads and result ordering. Configure the multicall
with allowFailure: false so all values are evaluated from one atomic snapshot,
then continue destructuring the returned results into id, startBlock,
initialized, currentL1Block, and currentL2Block.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ab085f1-2f31-4a49-8be1-250754e153c3
📒 Files selected for processing (1)
pages/api/current-round.tsx
| const [id, startBlock, initialized, currentL1Block, currentL2Block] = | ||
| await Promise.all([ | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRound", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRoundStartBlock", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "currentRoundInitialized", | ||
| }), | ||
| l2PublicClient.readContract({ | ||
| ...contract, | ||
| functionName: "blockNum", | ||
| }), | ||
| l2PublicClient.getBlockNumber(), | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guarantee the "single snapshot" claimed in the PR description.
Problem: The PR states that round data is read using a "single snapshot" to avoid inconsistencies. However, mapping multiple readContract calls into Promise.all executes separate RPC requests (unless batch: { multicall: true } is enabled globally on l2PublicClient).
Why it matters: Independent RPC requests can be routed to differently-synced load-balanced nodes or straddle a new block, resulting in torn state (e.g., startBlock from block blockNum from block
Suggested fix: Use l2PublicClient.multicall with allowFailure: false to guarantee all contract reads evaluate atomically in a single eth_call.
🛠 Proposed fix
- const [id, startBlock, initialized, currentL1Block, currentL2Block] =
- await Promise.all([
- l2PublicClient.readContract({
- ...contract,
- functionName: "currentRound",
- }),
- l2PublicClient.readContract({
- ...contract,
- functionName: "currentRoundStartBlock",
- }),
- l2PublicClient.readContract({
- ...contract,
- functionName: "currentRoundInitialized",
- }),
- l2PublicClient.readContract({
- ...contract,
- functionName: "blockNum",
- }),
- l2PublicClient.getBlockNumber(),
- ]);
+ const [[id, startBlock, initialized, currentL1Block], currentL2Block] =
+ await Promise.all([
+ l2PublicClient.multicall({
+ contracts: [
+ { ...contract, functionName: "currentRound" },
+ { ...contract, functionName: "currentRoundStartBlock" },
+ { ...contract, functionName: "currentRoundInitialized" },
+ { ...contract, functionName: "blockNum" },
+ ],
+ allowFailure: false,
+ }),
+ l2PublicClient.getBlockNumber(),
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [id, startBlock, initialized, currentL1Block, currentL2Block] = | |
| await Promise.all([ | |
| l2PublicClient.readContract({ | |
| ...contract, | |
| functionName: "currentRound", | |
| }), | |
| l2PublicClient.readContract({ | |
| ...contract, | |
| functionName: "currentRoundStartBlock", | |
| }), | |
| l2PublicClient.readContract({ | |
| ...contract, | |
| functionName: "currentRoundInitialized", | |
| }), | |
| l2PublicClient.readContract({ | |
| ...contract, | |
| functionName: "blockNum", | |
| }), | |
| l2PublicClient.getBlockNumber(), | |
| ]); | |
| const [[id, startBlock, initialized, currentL1Block], currentL2Block] = | |
| await Promise.all([ | |
| l2PublicClient.multicall({ | |
| contracts: [ | |
| { ...contract, functionName: "currentRound" }, | |
| { ...contract, functionName: "currentRoundStartBlock" }, | |
| { ...contract, functionName: "currentRoundInitialized" }, | |
| { ...contract, functionName: "blockNum" }, | |
| ], | |
| allowFailure: false, | |
| }), | |
| l2PublicClient.getBlockNumber(), | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/api/current-round.tsx` around lines 29 - 48, Replace the parallel
readContract calls in the current-round data-loading flow with
l2PublicClient.multicall, preserving the same five contract function reads and
result ordering. Configure the multicall with allowFailure: false so all values
are evaluated from one atomic snapshot, then continue destructuring the returned
results into id, startBlock, initialized, currentL1Block, and currentL2Block.
6ecffb8 to
bb5185d
Compare
0b60de7 to
d8696d7
Compare
/current-round mixed sources: round id/startBlock/initialized came from the subgraph, but the current block from a live RPC call. When subgraph indexing stalls the stale round is compared against a live block, so the tracker shows a stale round number with counters running past the round length. Read the timing from the RoundsManager contract (currentRound, currentRoundStartBlock, currentRoundInitialized, blockNum) via l2PublicClient, keeping startBlock and the current block consistent and independent of subgraph health. Response shape is unchanged; aggregate stats still use the subgraph. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d8696d7 to
ab7af01
Compare
…#735) * fix: source round length and lock state on-chain in the round tracker RoundStatus still mixed sources after #731: round timing came from the RoundsManager via /current-round, but roundLength and the lock state came from the subgraph (protocol.roundLength / protocol.lockPeriod). The lock state was re-derived client-side (blocksRemaining <= lockPeriod) because the subgraph, being event-driven, can't keep a per-block boolean fresh. Extend /api/current-round to also return roundLength and locked (from RoundsManager.roundLength and currentRoundLocked), and read both from useCurrentRoundData. The lock state now comes straight from currentRoundLocked — one authoritative snapshot instead of re-deriving it — and the round tracker (ring, blocks remaining, lock badge) is fully on-chain and consistent. The spinner now only waits on the on-chain round data, so the tracker renders even when the subgraph is fully down; the subgraph-only stats (fees, rewards, supply) are hidden in that case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: drop redundant comment in RoundStatus Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: show a loading skeleton for the round lock badge isRoundLocked falls back to false before currentRoundInfo resolves, so the header badge asserted "Initialized" while the round data was still loading. Render a skeleton until currentRoundInfo is available, matching the spinner in the content area below. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Read the current round's timing (
id,startBlock,initialized, current block) directly from theRoundsManagercontract vial2PublicClient, instead of from the subgraph.Why
/current-roundpreviously readid/startBlock/initializedfrom the subgraph but the current block from a live RPC call. That mixes two sources: the round state depends on subgraph freshness, and when subgraph indexing stalls the stale round is compared against a live block, so the tracker shows a stale round number and its counters run past the round length.Reading
currentRoundStartBlockandblockNumfrom the same contract in one snapshot makes the computation self-consistent and independent of subgraph health. Response shape is unchanged, no client/type changes.Aggregate stats (fees, rewards, total supply) still come from the subgraph; they have no cheap on-chain equivalent, and a stale fee number isn't misleading the way a wrong round number is.
The old subgraph null-guards (
No current round found/No block number found) are dropped, contract reads always return a value, and RPC failures surface via the existingtry/catch.The display-clamp changes originally bundled here are split into #732 so this data-source change can be reverted independently.
Testing
tsc/eslint clean on the changed file/api/current-roundreturns on-chain-matching values with a live block