fix: drop zero-address placeholders, merge eth RegistrarControllers#2049
fix: drop zero-address placeholders, merge eth RegistrarControllers#2049
Conversation
…gistrarControllers Removes `LegacyEthRegistrarController`, `WrappedEthRegistrarController`, and `UniversalRegistrarRenewalWithReferrer` placeholder entries (`address: zeroAddress, startBlock: 0`) from sepolia-v2 and `UniversalRegistrarRenewalWithReferrer` from ens-test-env. These were registered with Ponder, which dragged each chain's effective startBlock to 0 and caused `historicalTotalBlocks` to overshoot by ~3.7M, producing `Block 14473749 not found` on sepolia-v2. In `registrars`, `subgraph`, and `ensv2` plugins, replaces per-controller Ponder contract entries with a single `RegistrarController` entry per chain that uses `AnyRegistrarControllerABI` (now also includes `UniversalRegistrarRenewalWithReferrer`). Controller addresses are combined into one chain entry via the new `mergedChainConfigForContracts` helper, with `pickContracts` for namespace-conditional lookup. Handlers dispatch by long-form event signature, mirroring `ensv2/handlers/ensv1/RegistrarController.ts`. Reverts the zero-address skip in `buildIndexedBlockranges` from #2045 — the underlying placeholders no longer exist. `getContractsByManagedName` now treats the optional ENSRoot controllers as `maybeGetDatasourceContract` lookups and filters absent ones. Closes #2048: the `ensv2` plugin's previous `chain: { …spread, …spread, …spread }` pattern silently overwrote each controller's chain-id-keyed config (only the last spread per chain id survived; on mainnet only `UnwrappedEthRegistrarController` was indexed). The new `mergedChainConfigForContracts` helper combines addresses correctly via `address: Address[]`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: ff228ca 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 (3)
📝 WalkthroughWalkthroughThis PR eliminates zero-address placeholder registrar controller contracts and consolidates their handling into merged per-chain controller configurations. It adds helper functions to unify contract address and blockrange logic, updates event handlers to dispatch from a single merged ABI, removes the zero-address skip workaround from blockrange calculation, and treats optional controllers as maybe-lookups instead of required ones. ChangesPlaceholder Elimination & Controller Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
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)
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 27 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes zero-address placeholder contracts from datasource configs and updates indexer plugins to use a single merged RegistrarController Ponder entry per chain (dispatching by long-form event signature), fixing a chain-id spread override bug in the ENSv2 plugin and avoiding startBlock=0 backfill issues.
Changes:
- Remove zero-address placeholder controller entries from
sepolia-v2(and URRWR fromens-test-env) and update managed-name contract lookups to treat these controllers as optional. - Merge ENSRoot RegistrarControllers into a single Ponder contract entry per chain across
subgraph,registrars, andensv2plugins usingAnyRegistrarControllerABIand new helperspickContracts+mergedChainConfigForContracts. - Revert the
buildIndexedBlockrangeszero-address skip workaround and update tests/changesets accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/managed-names.ts | Treat ENSRoot controllers as optional via maybeGetDatasourceContract(...).filter(...). |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Removes zero-address placeholder skipping logic (workaround no longer needed). |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Removes the test that asserted zero-address skipping behavior. |
| packages/datasources/src/sepolia-v2.ts | Drops placeholder controller entries (Legacy/Wrapped/URRWR) from sepolia-v2 ENSRoot datasource. |
| packages/datasources/src/ens-test-env.ts | Removes URRWR placeholder entry from ens-test-env datasource. |
| packages/datasources/src/lib/AnyRegistrarControllerABI.ts | Adds URRWR ABI into merged controller ABI so RenewalReferred is available. |
| apps/ensindexer/src/lib/ponder-helpers.ts | Adds pickContracts and mergedChainConfigForContracts helpers for merged Ponder chain config. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/plugin.ts | Replaces per-controller entries with a single merged RegistrarController contract entry. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registrar.ts | Switches controller handlers to dispatch by long-form event signature under merged entry. |
| apps/ensindexer/src/plugins/registrars/plugin.ts | Replaces four ethnames controller entries with one merged Ethnames_RegistrarController. |
| apps/ensindexer/src/plugins/registrars/event-handlers.ts | Consolidates ethnames controller handler attachment into one file. |
| apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_UniversalRegistrarRenewalWithReferrer.ts | Removed; logic moved into merged controller handler. |
| apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_RegistrarController.ts | Consolidates ethnames controller + URRWR event handling with merged dispatch. |
| apps/ensindexer/src/plugins/ensv2/plugin.ts | Fixes #2048 by merging per-chain controller configs instead of overwriting via spreads. |
| .changeset/skip-zero-address-placeholders-blockrange.md | Removed (replaced by new combined changeset). |
| .changeset/eliminate-zero-address-placeholders.md | New changeset covering datasource/indexer/sdk changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR fixes two compounding bugs on sepolia-v2: (1) zero-address placeholder contracts were dragging Ponder's effective indexed Confidence Score: 5/5Safe to merge — all long-form signatures were verified against ABIs, arg remappings are correct, and the empty-array guard is in place All long-form event signatures were cross-checked against the ABI files and are exact matches. The arg remapping in each handler (label↔labelhash, cost vs baseCost+premium) is correct for every controller. The mergedChainConfigForContracts helper guards against empty input. The pickContracts helper correctly handles absent contracts for namespace-conditional entries. The buildIndexedBlockranges revert is safe because zero-address placeholders no longer exist. No P0 or P1 issues found. No files require special attention. The long-form signature correctness is the only manual-verification risk and has been confirmed against the ABI sources. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Ponder: Ethnames_RegistrarController\n(AnyRegistrarControllerABI, merged addresses)"]
A -->|"NameRegistered(string name, bytes32 indexed label, address indexed owner, uint256 cost, uint256 expires)"| B["LegacyEthRegistrarController handler\n(cost, no premium, no referral)"]
A -->|"NameRegistered(string name, bytes32 indexed label, address indexed owner, uint256 baseCost, uint256 premium, uint256 expires)"| C["WrappedEthRegistrarController handler\n(baseCost+premium, no referral)"]
A -->|"NameRegistered(string label, bytes32 indexed labelhash, address indexed owner, uint256 baseCost, uint256 premium, uint256 expires, bytes32 referrer)"| D["UnwrappedEthRegistrarController handler\n(baseCost+premium, with referral)"]
A -->|"NameRenewed(string name, bytes32 indexed label, uint256 cost, uint256 expires)"| E["Legacy+Wrapped NameRenewed handler\n(cost, no premium, no referral)"]
A -->|"NameRenewed(string label, bytes32 indexed labelhash, uint256 cost, uint256 expires, bytes32 referrer)"| F["UnwrappedEthRegistrarController NameRenewed\n(cost, no premium, with referral)"]
A -->|"RenewalReferred (short form)"| G["UniversalRegistrarRenewalWithReferrer handler"]
subgraph "pickContracts + mergedChainConfigForContracts"
H["LegacyEthRegistrarController address"] --> I["Merged address[]"]
J["WrappedEthRegistrarController address"] --> I
K["UnwrappedEthRegistrarController address"] --> I
L["URRWR address (optional)"] --> I
end
I --> A
Reviews (4): Last reviewed commit: "fix: bot notes (loop 2) — undefined addr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/lib/ponder-helpers.ts`:
- Around line 191-194: The calculation of minStartBlock using contracts.reduce
can yield Number.POSITIVE_INFINITY when contracts is empty; update the function
in ponder-helpers.ts that computes minStartBlock (where minStartBlock and
contracts are used) to defensively handle an empty contracts array by either
throwing a clear error/ assertion or returning early with a sensible default;
specifically check if contracts.length === 0 before the reduce and raise an
explanatory exception or return a configured fallback so the chain config never
receives Infinity.
In `@apps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registrar.ts`:
- Around line 111-128: The premium-registration handlers rebuild event.args from
scratch causing emitted fields like owner and expires to be dropped; in the
addOnchainEventListener callbacks (see namespaceContract(...) match and the
async handler that calls handleNameRegisteredByController) merge the original
event.args with the overrides instead of replacing it — e.g. create args as {
...event.args, label: event.args.name, labelHash: event.args.label, cost:
event.args.baseCost + event.args.premium } so owner/expires and any other
emitted fields are preserved; apply the same merge fix to the other premium
branch (the similar handler around the other occurrence).
🪄 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: 014b5165-eb3a-47e8-afa6-56ce4ad25a34
📒 Files selected for processing (16)
.changeset/eliminate-zero-address-placeholders.md.changeset/skip-zero-address-placeholders-blockrange.mdapps/ensindexer/src/lib/ponder-helpers.tsapps/ensindexer/src/plugins/ensv2/plugin.tsapps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_RegistrarController.tsapps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_UniversalRegistrarRenewalWithReferrer.tsapps/ensindexer/src/plugins/registrars/event-handlers.tsapps/ensindexer/src/plugins/registrars/plugin.tsapps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registrar.tsapps/ensindexer/src/plugins/subgraph/plugins/subgraph/plugin.tspackages/datasources/src/ens-test-env.tspackages/datasources/src/lib/AnyRegistrarControllerABI.tspackages/datasources/src/sepolia-v2.tspackages/ensnode-sdk/src/shared/config/indexed-blockranges.test.tspackages/ensnode-sdk/src/shared/config/indexed-blockranges.tspackages/ensnode-sdk/src/shared/managed-names.ts
💤 Files with no reviewable changes (5)
- packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts
- packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts
- apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_UniversalRegistrarRenewalWithReferrer.ts
- packages/datasources/src/ens-test-env.ts
- .changeset/skip-zero-address-placeholders-blockrange.md
🚀 Preview Packages -
|
| addOnchainEventListener( | ||
| namespaceContract(pluginName, "Ethnames_LegacyEthRegistrarController:NameRenewed"), | ||
| namespaceContract( | ||
| pluginName, |
There was a problem hiding this comment.
Because of the deduplication in the diff, this diff is going to look odd, but read the file in isolation for clarity.
… empty input Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…et typo Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
| export function pickContracts( | ||
| contracts: Record<string, ContractConfig>, | ||
| names: readonly string[], |
There was a problem hiding this comment.
I had an idea that maybe it's possible to make something like
contracts: Record<T, ContractConfig>,
names: T[]But I lost to compiler, I just dont understand how typescript works...
| args: { | ||
| // this field is the labelhash, not the label | ||
| label: labelHash, | ||
| // rename to labelHash |
There was a problem hiding this comment.
if this is TODO, does it make sense to add // TODO: ? At first I thought this is prompt for AI autocompletion 😄
Reviewer Focus (Read This First)
registrars+subgraphplugins: one Ponder entry per chain usingAnyRegistrarControllerABI, dispatch by long-form event signatureensv2/plugin.ts:RegistrarController— closes ENSv2 plugin silently overrides RegistrarController address per chain #2048; only the lastchainConfigForContract(...)spread per chain id was surviving (mainnet was indexing onlyUnwrappedEthRegistrarController)mergedChainConfigForContractsandpickContractsinapps/ensindexer/src/lib/ponder-helpers.tsProblem & Motivation
three problems compound on sepolia-v2:
LegacyEthRegistrarController,WrappedEthRegistrarController, andUniversalRegistrarRenewalWithReferrerasaddress: zeroAddress, startBlock: 0placeholders to satisfy the typesystembuildIndexedBlockrangesfilter raised the publicindexedBlockrange.startBlockbut didn't fix ponder's view;historicalTotalBlocks - startBlockovershot by ~3.7M, producingBlock 14473749 not foundon every metadata writediscovered mid-investigation: ensv2 plugin's
RegistrarControllerwas silently dropping all but the lastchainConfigForContract(...)spread per chain id (#2048).What Changed (Concrete)
LegacyEthRegistrarController,WrappedEthRegistrarController,UniversalRegistrarRenewalWithReferrerplaceholder entries from sepolia-v2 ENSRoot; removedUniversalRegistrarRenewalWithReferrerfrom ens-test-env.UniversalRegistrarRenewalWithReferrertoAnyRegistrarControllerABIso itsRenewalReferredevent is part of the merged dispatch.RegistrarControllerentry per chain. addresses combined via newmergedChainConfigForContracts(…, pickContracts(contracts, [names])).NameRegistered(string label, bytes32 indexed labelhash, …, bytes32 referrer));RenewalReferreduses the short form (unique name in merged abi).Ethnames_RegistrarController.ts; updatedevent-handlers.ts.RegistrarControllernow combines per-chain controller addresses viamergedChainConfigForContractsinstead of multiple chain-id-keyed spreads that overwrote each other.buildIndexedBlockrangesfrom fix(ensnode-sdk): skip zero-address placeholders in indexed blockrange #2045 (placeholders no longer exist); removed the corresponding test; replaced the changeset.getContractsByManagedNamenow usesmaybeGetDatasourceContract+.filter(...)for the optional ENSRoot controllers, matching the existing Basenames/Lineanames pattern.Design & Planning
design was iterated in conversation. key decisions:
start by deleting placeholders with no guards and observe what ponder complains about, to validate scope
when type errors propagate into unrelated handlers (tokenscope), recognize that contract-name-keyed conditional spreads collapse
EventNamesinference and route around itprefer the "merge controllers under one Ponder entry" pattern (mirroring
apps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.ts) over amaybePonderContractEntry-style cast helperdiscovery of ENSv2 plugin silently overrides RegistrarController address per chain #2048 mid-flight; folded its fix in via the same
mergedChainConfigForContractshelperplanning artifacts: none (conversation-driven)
reviewed / approved by: not yet
Self-Review
mergedChainConfigForContracts,pickContractsmaybePonderContractEntryhelper; per-controller ABI re-exports added then dropped within this branchother things caught after first pass:
RenewalReferredinitially registered with long-form sig — switched to short form (unique name in merged abi).filter((c): c is ContractConfig => …)predicate failed on union of specific contract types; introducedpickContractswhich casts toContractConfig | undefinedfirstCross-Codebase Alignment
zeroAddress,LegacyEthRegistrarController,WrappedEthRegistrarController,UniversalRegistrarRenewalWithReferrer,chainConfigForContract,AnyRegistrarControllerABIregistrars/plugin.ts(no namespace-conditional issue; no chain-id override).apps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.tswas the precedent for the long-form-signature dispatchregistrars/plugin.tsfor consistency. not load-bearing todayDownstream & Consumer Impact
@ensnode/datasourcesdrops the three placeholder entries on sepolia-v2 + URRWR on ens-test-env. consumers must treat the optional ENSRoot controllers as possibly-absent.AnyRegistrarControllerABInow includesRenewalReferred. ponder contract entry names changed across the three plugins (e.g.Ethnames_LegacyEthRegistrarController→Ethnames_RegistrarController)mergedChainConfigForContractsvschainConfigForContract— explicit about merge semantics; the singular form's "spread three into one chain block" pattern was the source of ENSv2 plugin silently overrides RegistrarController address per chain #2048pickContracts(contracts, [names])reads naturally as the input tomergedChainConfigForContractsTesting Evidence
pnpm -F ensindexer typecheckcleanpnpm test --project ensindexer --project ensnode-sdk --project datasources— 219 / 219pnpm lintcleanPLUGINS=subgraph,ensv2,protocol-acceleration pnpm devagainst sepolia-v2 —Started backfill indexing chain=99911155111 block_range=[3702721,10771028],cache_rate=100%, indexer progressing without the originalBlock 14473749 not foundcrashpackages/datasources/src/abis/root/*Scope Reductions
registrars/plugin.tsinto the merged pattern — no namespace-conditional issue, no current bugRenewalReferredhandler today; including its address would index events that no handler consumesRisk Analysis
mergeAbispreserves overloaded events with distinct topic hashes. ponder dispatches per long-form signature when an event name has overloadsregistrars,subgraph, orensv2plugins re-indexes (handler/contract changes are implicit re-index per project convention). the ENSv2 plugin silently overrides RegistrarController address per chain #2048 fix means mainnetensv2will pick up Legacy/Wrapped historicals previously dropped, lengthening the first historical phase on those deploymentsPre-Review Checklist (Blocking)