fix: derive indexedChainIds from static plugin metadata#1766
fix: derive indexedChainIds from static plugin metadata#1766
Conversation
…exedChainIds` in plugins that conditionally index multiple chains (ex: 'protocol-acceleration').
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0ad286b The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDerives indexedChainIds by iterating each plugin's declared datasource names (using maybeGetDatasource) instead of namespace-only datasources; exposes allDatasourceNames on plugins, renames public type to EnsIndexerConfig in schema/validation, adds test helpers, and adds a changeset for a minor release. Changes
Sequence Diagram(s)sequenceDiagram
participant ConfigBuilder as Config Builder
participant PluginRegistry as Plugin Registry
participant DatasourceResolver as maybeGetDatasource
participant Datasource as Datasource (chain.id)
ConfigBuilder->>PluginRegistry: for each configured plugin\nread plugin.allDatasourceNames
loop per datasourceName
PluginRegistry->>DatasourceResolver: maybeGetDatasource(namespace, datasourceName)
DatasourceResolver-->>Datasource: resolve datasource or undefined
alt datasource exists
Datasource-->>ConfigBuilder: return chain.id
else datasource missing
DatasourceResolver-->>ConfigBuilder: return undefined (skip)
end
end
Note right of ConfigBuilder: collect unique chain.ids\nset config.indexedChainIds
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Pull request overview
This PR fixes how EnsIndexerConfig.indexedChainIds is derived by switching from “required datasource names” to the plugin’s actual generated Ponder config, which better reflects chains indexed via optional/conditional datasources (notably protocol-acceleration).
Changes:
- Update
derive_indexedChainIdsto callplugin.createPonderConfig(config)and extract chain IDs fromponderConfig.chains. - Update plugin helper typings around
createPonderConfigand remove an unused helper. - Add/refactor config tests to stub RPC URLs per-namespace and add explicit
.indexedChainIdscoverage; add a changeset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/ensindexer/src/lib/plugin-helpers.ts | Adjusts plugin helper typing surface for createPonderConfig and removes old required-datasource helper. |
| apps/ensindexer/src/config/derived-params.ts | Changes indexedChainIds derivation to use the plugin’s Ponder config chains as source of truth. |
| apps/ensindexer/src/config/config.test.ts | Adds stubRpcUrlsForNamespace and new .indexedChainIds test coverage, plus RPC stub fixes. |
| .changeset/tiny-friends-lie.md | Declares a minor release for the indexedChainIds derivation fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Greptile SummaryThis PR fixes a correctness bug in The fix introduces a new Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ENSIndexerConfigSchema.parse] --> B[.transform: derive_indexedChainIds]
B --> C{For each active plugin}
C --> D[plugin.allDatasourceNames\nrequired + optional]
D --> E{maybeGetDatasource\nnamespace, name}
E -->|exists| F[indexedChainIds.add\ndatasource.chain.id]
E -->|undefined| G[skip — optional\ndatasource missing]
F --> H[return config + indexedChainIds]
G --> H
H --> I[.check: invariant_requiredDatasourcesSubsetOfAll\nrequiredDatasourceNames ⊆ allDatasourceNames]
I --> J[.check: invariant_requiredDatasources\nrequired DSs present in namespace]
J --> K[.check: invariant_rpcConfigsSpecifiedForRootChain]
K --> L[.check: invariant_validContractConfigs]
L --> M[.check: invariant_isSubgraphCompatibleRequirements]
M --> N[.check: invariant_rpcConfigsSpecifiedForIndexedChains\nfor each id in indexedChainIds]
N --> O[.check: invariant_globalBlockrange\nindexedChainIds.size ≤ 1 if blockrange set]
O --> P[EnsIndexerConfig ✓]
|
![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: 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 `@apps/ensindexer/src/lib/plugin-helpers.ts`:
- Around line 70-74: The createPonderConfig parameter types are inconsistent:
ENSIndexerPlugin.createPonderConfig currently takes Omit<EnsIndexerConfig,
"indexedChainIds"> while BuildPluginOptions.createPonderConfig takes the full
EnsIndexerConfig, causing a mismatch when createPlugin returns the options as an
ENSIndexerPlugin. Make both signatures use the same type (pick either
EnsIndexerConfig or Omit<EnsIndexerConfig, "indexedChainIds">) and update the
declaration in ENSIndexerPlugin.createPonderConfig or
BuildPluginOptions.createPonderConfig accordingly so createPonderConfig has the
identical parameter type in both places (references: createPonderConfig,
ENSIndexerPlugin, BuildPluginOptions, createPlugin).
🪄 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: 22a24cfe-1406-42b8-8dff-be5777b673da
📒 Files selected for processing (4)
.changeset/tiny-friends-lie.mdapps/ensindexer/src/config/config.test.tsapps/ensindexer/src/config/derived-params.tsapps/ensindexer/src/lib/plugin-helpers.ts
|
@greptile |
There was a problem hiding this comment.
Pull request overview
Fixes EnsIndexerConfig.indexedChainIds derivation so it accounts for chains indexed via optional datasources (not just “required” ones), and updates related helpers/tests to reflect the broader datasource set.
Changes:
- Introduces
allDatasourceNameson plugins and derivesindexedChainIdsby resolving those datasources against the active namespace. - Updates indexed blockrange derivation to accept (required + optional) datasource names and skip datasources not present in the namespace.
- Expands config tests with
.indexedChainIdscoverage and adds a namespace-based RPC_URL stubbing helper.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Accepts broader datasource sets and skips namespace-missing datasources when deriving per-chain blockranges. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Updates expectations to match “skip missing datasource” behavior. |
| apps/ensindexer/src/plugins/tokenscope/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/subgraph/plugins/threedns/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/subgraph/plugins/lineanames/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/subgraph/plugins/basenames/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/registrars/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/plugins/protocol-acceleration/plugin.ts | Sets allDatasourceNames to include optional multi-chain datasources (bug driver). |
| apps/ensindexer/src/plugins/ensv2/plugin.ts | Adds allDatasourceNames metadata for chain derivation. |
| apps/ensindexer/src/lib/plugin-helpers.ts | Extends plugin typing/factory to require allDatasourceNames; adds getPluginsAllDatasourceNames. |
| apps/ensindexer/src/lib/local-ponder-client.ts | Uses all datasource names when building indexed blockranges. |
| apps/ensindexer/src/config/validations.ts | Validates RPC configs using derived indexedChainIds; updates global blockrange invariant to use the set. |
| apps/ensindexer/src/config/derived-params.ts | Derives indexedChainIds via allDatasourceNames + namespace datasource resolution. |
| apps/ensindexer/src/config/config.test.ts | Adds .indexedChainIds tests and stubRpcUrlsForNamespace() helper. |
| apps/ensindexer/src/config/config.schema.ts | Moves derived-param transform earlier so later invariants can depend on indexedChainIds. |
| .changeset/tiny-friends-lie.md | Adds a changeset entry describing the indexedChainIds fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- use Omit<EnsIndexerConfig, "indexedChainIds"> for createPonderConfig param - update config.schema.ts inline docs to reflect new pipeline order - rephrase RPC validation error message - add @ensnode/ensnode-sdk patch to changeset Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shrugs
changed the title
indexedChainIds derivationThere was a problem hiding this comment.
Pull request overview
This PR fixes incorrect derivation of EnsIndexerConfig.indexedChainIds by switching from “required datasources only” to “all datasources a plugin may index (required + optional)” via new static plugin metadata, and updates downstream logic to align with that derivation across namespaces.
Changes:
- Add
allDatasourceNamesto the plugin interface and update plugins to declare it. - Update
derive_indexedChainIdsto resolveallDatasourceNamesviamaybeGetDatasource, and reorder the Zod pipeline so derivation happens before invariant checks. - Update
buildIndexedBlockranges/local-ponder-clientto use all datasource names and skip namespace-missing datasources; adjust unit tests accordingly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Accepts “all datasource names” and skips datasources missing from a namespace instead of throwing. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Updates expectations to validate “skip missing datasource” behavior. |
| apps/ensindexer/src/lib/plugin-helpers.ts | Extends plugin interface/options with allDatasourceNames and renames helper to return all datasource names. |
| apps/ensindexer/src/lib/local-ponder-client.ts | Uses getPluginsAllDatasourceNames() when building indexed blockranges. |
| apps/ensindexer/src/config/derived-params.ts | Re-derives indexedChainIds using plugin.allDatasourceNames + maybeGetDatasource. |
| apps/ensindexer/src/config/validations.ts | Simplifies invariants to operate directly on config.indexedChainIds. |
| apps/ensindexer/src/config/config.schema.ts | Reorders Zod pipeline: .transform(derive_indexedChainIds) before .check() invariants. |
| apps/ensindexer/src/config/config.test.ts | Adds/updates tests for cross-namespace indexedChainIds derivation and stubs RPC URLs by namespace. |
| apps/ensindexer/src/plugins/**/plugin.ts | Adds allDatasourceNames declarations to plugins (required-only for most; full set for multi/optional plugins). |
| .changeset/tiny-friends-lie.md | Publishes changes (ensindexer minor, ensnode-sdk patch). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…asourceNames Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@greptile review |
…nvariant test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates ENSIndexer config derivation to compute indexedChainIds from static plugin metadata (allDatasourceNames, including optional datasources), enabling correct multi-namespace + optional-datasource behavior (notably for protocol-acceleration) without needing to execute createPonderConfig.
Changes:
- Add
allDatasourceNamesto the plugin interface and populate it across all plugins. - Rework
derive_indexedChainIdsto resolve chains viamaybeGetDatasource(plugin.allDatasourceNames)and reorder the Zod pipeline so derived values exist before invariants. - Adjust indexed blockrange calculation to skip missing datasources (matching derivation semantics) and update tests accordingly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Skip missing datasources when aggregating blockranges. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Update test expectations to match “skip missing datasource” behavior. |
| apps/ensindexer/src/lib/plugin-helpers.ts | Extend plugin type to require allDatasourceNames; expose helper to map plugins → all datasource names. |
| apps/ensindexer/src/lib/local-ponder-client.ts | Use all datasource names when building indexed blockranges. |
| apps/ensindexer/src/config/derived-params.ts | Derive indexedChainIds from allDatasourceNames via maybeGetDatasource. |
| apps/ensindexer/src/config/validations.ts | Simplify invariants to depend on config.indexedChainIds; add subset invariant for required vs all datasource names. |
| apps/ensindexer/src/config/config.schema.ts | Reorder Zod pipeline: .transform(derive_indexedChainIds) before .check(...) invariants. |
| apps/ensindexer/src/config/config.test.ts | Add/adjust tests for cross-namespace chain derivation + subset invariant; add RPC-url stubbing helper. |
| apps/ensindexer/src/plugins/**/plugin.ts | Populate allDatasourceNames across plugins (required-only for most; expanded for multi-datasource plugins). |
| .changeset/tiny-friends-lie.md | Release notes for ensindexer + ensnode-sdk versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ecification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Looks good! 👍
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect indexedChainIds derivation for plugins that optionally index additional datasources/chains by introducing static plugin metadata (allDatasourceNames) and using it to derive indexed chains without executing plugin createPonderConfig.
Changes:
- Add
allDatasourceNames(required + optional datasources) to theENSIndexerPlugininterface and populate it across all plugins. - Rework
derive_indexedChainIdsto resolveallDatasourceNamesviamaybeGetDatasource, and reorder the config zod pipeline to derive before invariant checks. - Update indexed blockrange calculation and config tests to align with the new “skip missing datasource in namespace” behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Skip missing datasources (namespace-dependent) when building indexed blockranges. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Update expectations to match “skip missing datasource” behavior. |
| apps/ensindexer/src/plugins/tokenscope/plugin.ts | Populate allDatasourceNames for TokenScope plugin. |
| apps/ensindexer/src/plugins/subgraph/plugins/threedns/plugin.ts | Populate allDatasourceNames for ThreeDNS plugin. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/plugin.ts | Populate allDatasourceNames for Subgraph plugin. |
| apps/ensindexer/src/plugins/subgraph/plugins/lineanames/plugin.ts | Populate allDatasourceNames for Lineanames plugin. |
| apps/ensindexer/src/plugins/subgraph/plugins/basenames/plugin.ts | Populate allDatasourceNames for Basenames plugin. |
| apps/ensindexer/src/plugins/registrars/plugin.ts | Populate allDatasourceNames for Registrars plugin. |
| apps/ensindexer/src/plugins/protocol-acceleration/plugin.ts | Populate allDatasourceNames (required + optional) for Protocol Acceleration plugin. |
| apps/ensindexer/src/plugins/ensv2/plugin.ts | Populate allDatasourceNames (required + optional) for ENSv2 plugin. |
| apps/ensindexer/src/lib/plugin-helpers.ts | Extend plugin types/factory to require allDatasourceNames; add helper to fetch all datasource names per plugin. |
| apps/ensindexer/src/lib/local-ponder-client.ts | Use all datasource names when building indexed blockranges. |
| apps/ensindexer/src/config/validations.ts | Add subset invariant and simplify invariants to use config.indexedChainIds directly. |
| apps/ensindexer/src/config/derived-params.ts | Derive indexedChainIds from plugin.allDatasourceNames resolved via maybeGetDatasource. |
| apps/ensindexer/src/config/config.test.ts | Add/adjust tests for indexedChainIds derivation and helper for stubbing RPC env vars by namespace. |
| apps/ensindexer/src/config/config.schema.ts | Run .transform(derive_indexedChainIds) before .check() invariants so checks can use derived values. |
| .changeset/tiny-friends-lie.md | Version bumps and release note for indexedChainIds derivation fix. |
Comments suppressed due to low confidence (1)
apps/ensindexer/src/config/validations.ts:96
- Typo in the global blockrange invariant message: "unintentially" should be "unintentionally". Since this string is surfaced to users during validation failures, fixing the spelling will make the guidance clearer.
input: config,
message: `ENSIndexer's behavior when indexing _multiple chains_ with a _specific blockrange_ is considered undefined (for now). If you're using this feature, you're likely interested in snapshotting at a specific END_BLOCK, and may have unintentially activated plugins that source events from multiple chains. The config currently is:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@greptile |
Reviewer Focus
allDatasourceNameson the plugin interface and howderive_indexedChainIdsuses it.transform()now runs before.check()invariantsbuildIndexedBlockranges(ensnode-sdk) now skips missing datasources instead of throwingProblem
derive_indexedChainIdsused required (not optional) datasources to determine indexed chains, leading to incorrectindexedChainIdsin plugins like protocol-acceleration with optional datasourcesWhat Changed
allDatasourceNamesfield toENSIndexerPlugin— declares all datasources a plugin may index (required + optional)derive_indexedChainIdsto resolveplugin.allDatasourceNamesviamaybeGetDatasource.transform(derive_indexedChainIds)runs first, then.check()invariants useindexedChainIdsdirectlyinvariant_rpcConfigsSpecifiedForIndexedChainsandinvariant_globalBlockrangeto useconfig.indexedChainIdsdirectlybuildIndexedBlockranges+local-ponder-client.tsto use all datasource names (not just required)allDatasourceNamesto all 8 pluginsTesting
Risk
allDatasourceNamesChecklist
ensindexerminor,@ensnode/ensnode-sdkpatch)