Avoid refetching empty live RPC ranges#220
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRpcDataStream now centralizes invalidation decisions via new helpers, tracks the last emitted empty block ( ChangesLive Block Empty-Block Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/protocol/tests/rpc-data-stream.test.ts (1)
98-149: ⚡ Quick winAdd a regression test for reorgs after an empty live block.
This PR also changes invalidation to consider
lastEmptyBlockNumberand clear it on reorg, but the new test only covers straight head growth. A case where blockNis emitted as empty and then a reorg invalidatesNwould lock in the new invalidation path.🤖 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 `@packages/protocol/tests/rpc-data-stream.test.ts` around lines 98 - 149, Add a regression test that simulates an empty live block followed by a reorg to ensure the new invalidation path clears lastEmptyBlockNumber and triggers proper refetch: create an EmptyLiveHeadConfig and RpcClient, open streamData({ finality: "accepted", filter: ["logs"], startingCursor: { orderKey: 1n }}), advance the head to emit an empty block (verifying fetchBlockRangeCalls and that config.lastEmptyBlockNumber is set), then simulate a reorg by setting config.headBlock to a lower value (invalidating that empty block) and assert that config.lastEmptyBlockNumber is cleared and that subsequent iterator.next() causes fetchBlockRange to be called again for the correct start/max block; reference EmptyLiveHeadConfig, RpcClient, streamData, fetchBlockRangeCalls, and lastEmptyBlockNumber to locate where to add assertions.
🤖 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 `@packages/protocol/src/rpc/data-stream.ts`:
- Around line 483-491: The termination check currently uses state.cursor to
decide whether to stop, but lastProcessedLiveBlock(state) can advance beyond
state.cursor when an empty accepted block updates state.lastEmptyBlockNumber;
update the termination logic in shouldContinue() (or wherever
options.endingCursor is compared) to compare options.endingCursor against
lastProcessedLiveBlock(state) instead of state.cursor so the stream stops when
the processed-live position reaches the requested endingCursor; ensure you
reference lastProcessedLiveBlock(state), state.lastEmptyBlockNumber,
state.cursor.orderKey and options.endingCursor when making the change.
---
Nitpick comments:
In `@packages/protocol/tests/rpc-data-stream.test.ts`:
- Around line 98-149: Add a regression test that simulates an empty live block
followed by a reorg to ensure the new invalidation path clears
lastEmptyBlockNumber and triggers proper refetch: create an EmptyLiveHeadConfig
and RpcClient, open streamData({ finality: "accepted", filter: ["logs"],
startingCursor: { orderKey: 1n }}), advance the head to emit an empty block
(verifying fetchBlockRangeCalls and that config.lastEmptyBlockNumber is set),
then simulate a reorg by setting config.headBlock to a lower value (invalidating
that empty block) and assert that config.lastEmptyBlockNumber is cleared and
that subsequent iterator.next() causes fetchBlockRange to be called again for
the correct start/max block; reference EmptyLiveHeadConfig, RpcClient,
streamData, fetchBlockRangeCalls, and lastEmptyBlockNumber to locate where to
add assertions.
🪄 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: b73dac99-1265-4168-a9b7-940cb58ad9b6
📒 Files selected for processing (2)
packages/protocol/src/rpc/data-stream.tspackages/protocol/tests/rpc-data-stream.test.ts
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 `@packages/protocol/src/rpc/data-stream.ts`:
- Around line 482-490: The resume start for fetching the next live range should
use lastProcessedLiveBlock(state) instead of directly using
state.cursor.orderKey; update the logic that currently computes the next start
as state.cursor.orderKey + 1n to compute lastProcessedLiveBlock(state) + 1n so
that after an accepted empty head block (when state.lastEmptyBlockNumber >
state.cursor.orderKey) the fetch resumes from max(cursor,
lastEmptyBlockNumber)+1; change the reference at the place that builds the
head-range fetch start (currently using state.cursor.orderKey + 1n) to call
lastProcessedLiveBlock(state) and add 1n.
🪄 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: 0fec8b04-b7f8-4a8e-985f-88822cc47600
📒 Files selected for processing (2)
change/@apibara-protocol-cefba257-a3ad-451e-b3d6-13613ecf7903.jsonpackages/protocol/src/rpc/data-stream.ts
✅ Files skipped from review due to trivial changes (1)
- change/@apibara-protocol-cefba257-a3ad-451e-b3d6-13613ecf7903.json
fracek
force-pushed
the
fix/live-empty-range-refetch
branch
from
758d25b to
fc0b1c6
Compare
fracek
force-pushed
the
fix/live-empty-range-refetch
branch
from
fc0b1c6 to
3d285f9
Compare
2 participants
Summary
Fix repeated live RPC range scans after emitting empty accepted blocks.
When live filtering reaches the current head with no matching data, the stream emits an empty accepted block but
intentionally does not advance
state.cursor. Subsequent live scans were still starting fromcursor + 1, so sparsefilters could repeatedly refetch previously scanned empty blocks as the head advanced.
This changes live range fetching to resume from
max(cursor, lastEmptyBlockNumber) + 1, treatslastEmptyBlockNumber === headas being at head, and clears the empty-block marker on reorg invalidation.Testing
vitestis unavailable.