fix(ensnode-sdk): skip zero-address placeholders in indexed blockrange#2045
fix(ensnode-sdk): skip zero-address placeholders in indexed blockrange#2045
Conversation
Cross-namespace plugin-required datasource fields (e.g. registrar entries unused in sepolia-v2) carry address=zeroAddress, startBlock=0 purely to satisfy the typesystem. mergeBlockNumberRanges was including them, dragging the chain's indexed blockrange.startBlock to 0 even though Ponder doesn't actually index them. That propagated into backfillEndBlock = startBlock + historicalTotalBlocks - 1, computing it ~3.7M blocks low and triggering recurrent ChainIndexingStatusSnapshot validation errors mid-backfill. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d96be8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA guard in ChangesZero-Address Placeholder Filtering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: 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. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 29 seconds.Comment |
Greptile SummaryThis PR fixes a bug in Confidence Score: 5/5Safe to merge — minimal, well-tested fix with no P0 or P1 issues. The change is a single-line guard with a clear rationale, backed by a new test that directly exercises the fixed code path. All existing tests continue to pass unaffected. No regressions introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildIndexedBlockranges] --> B[For each plugin datasource name]
B --> C{datasource exists in namespace?}
C -- No --> B
C -- Yes --> D[For each contract in datasource]
D --> E{address === zeroAddress?}
E -- Yes: skip --> D
E -- No --> F[buildBlockNumberRange startBlock endBlock]
F --> G{existing range for chain?}
G -- No --> H[use contract range]
G -- Yes --> I[mergeBlockNumberRanges]
H --> J[set indexedBlockranges for chainId]
I --> J
J --> D
D --> B
B --> K[return indexedBlockranges map]
Reviews (3): Last reviewed commit: "fix lint" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates @ensnode/ensnode-sdk’s indexed blockrange calculation so placeholder datasource contracts configured with the zero address no longer affect the derived per-chain indexing range. In the broader codebase, this feeds the local Ponder client / indexing-status path that computes backfill bounds for ENS indexer chains.
Changes:
- Skip zero-address datasource contracts when building indexed blockranges.
- Add a patch changeset documenting the blockrange/backfill fix for
@ensnode/ensnode-sdk.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts |
Filters zero-address placeholder contracts out of blockrange aggregation. |
.changeset/skip-zero-address-placeholders-blockrange.md |
Records the SDK patch release note for the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts (1)
40-68: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a unit test covering the zero-address skip.
The PR explicitly notes no test was added for the new guard. Given this is the single logical change in the PR and the bug it fixes was actively causing repeated validation errors in production, a dedicated test case that verifies zero-address contracts are excluded from the computed blockrange (and don't drag
startBlockto0) would lock in the fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts` around lines 40 - 68, Add a unit test that verifies the zero-address guard in indexed-blockranges.ts skips placeholder contracts: create datasourceContracts where one contract uses zeroAddress with startBlock=0 and another uses a real address with startBlock>0, call the function (the module export that builds the indexedBlockranges which uses buildBlockNumberRange/mergeBlockNumberRanges) and assert the resulting indexedBlockranges for that chain excludes the zero-address contribution (i.e., startBlock is the non-zero contract's startBlock and the rangeType is LeftBounded or Bounded), ensuring zero-address contracts do not set startBlock to 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts`:
- Around line 40-68: Add a unit test that verifies the zero-address guard in
indexed-blockranges.ts skips placeholder contracts: create datasourceContracts
where one contract uses zeroAddress with startBlock=0 and another uses a real
address with startBlock>0, call the function (the module export that builds the
indexedBlockranges which uses buildBlockNumberRange/mergeBlockNumberRanges) and
assert the resulting indexedBlockranges for that chain excludes the zero-address
contribution (i.e., startBlock is the non-zero contract's startBlock and the
rangeType is LeftBounded or Bounded), ensuring zero-address contracts do not set
startBlock to 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f7bdf751-abf1-4478-b4cd-2f017724110e
📒 Files selected for processing (2)
.changeset/skip-zero-address-placeholders-blockrange.mdpackages/ensnode-sdk/src/shared/config/indexed-blockranges.ts
…dBlockranges Asserts a chain with one real contract and one zero-address placeholder returns a blockrange derived only from the real contract — preventing the regression where placeholders dragged startBlock to 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts`:
- Around line 160-193: Add a test that verifies buildIndexedBlockranges skips a
chain when every required datasource contract is the zero-address placeholder:
create a datasource config for a chain (use zeroAddress and startBlock: 0 for
all contracts), register it in datasourcesByName via datasourceMock and have
maybeGetDatasourceMock return it for DatasourceNames.ENSRoot (or whichever
DatasourceName used), set pluginsRequiredDatasourceNames to include that
datasource (e.g., PluginName.Subgraph -> [DatasourceNames.ENSRoot]), call
buildIndexedBlockranges(ENSNamespaceIds.Mainnet, pluginsRequiredDatasourceNames)
and assert the returned Map does not contain an entry for that chain (i.e.,
equals a Map without that chain) rather than a range starting at 0; reference
buildIndexedBlockranges, datasourceMock, maybeGetDatasourceMock, zeroAddress,
PluginName, and DatasourceNames in the test setup.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a001b85-c070-4c6d-b0a5-bcfadf07abb8
📒 Files selected for processing (1)
packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
buildIndexedBlockrangesnow skips contracts whoseaddress === zeroAddressbefore merging into a chain's indexed blockrange.Invalid ChainIndexingStatusSnapshot: latestIndexedBlock must be before or same as backfillEndBlockerrors emitted byEnsDbWriterWorkerduring sepolia-v2 backfill.Why
LegacyEthRegistrarController,WrappedEthRegistrarController,UniversalRegistrarRenewalWithReferrer) set toaddress: zeroAddress, startBlock: 0purely to satisfy the registrar plugin's typesystem. ponder doesn't actually index them.startBlock: 0intomergeBlockNumberRanges, dragging the chain'sindexedBlockrange.startBlockto0. that propagated intobackfillEndBlock = startBlock + historicalTotalBlocks - 1, which then computed ~3.7M blocks below the true chain head (because ponder'shistoricalTotalBlocksis measured from the real lowest start block, not 0).backfillEndBlockmid-backfill, the snapshot validator threw on every metadata write — ~1 error/second until realtime.Testing
pnpm -F ensnode-sdk typecheckpasses.indexed-blockranges.test.tsstill pass.Notes for Reviewer
historicalTotalBlockssomehow, this fix would have to match that behavior instead.Checklist