feat(ensapi): DomainResolver.effective (ENSv1 + ENSv2)#2265
Conversation
Adds `DomainResolver.effective`, the Resolver that ENS Forward Resolution (ENSIP-10) lands on for a Domain, identified from indexed data via findResolverWithIndex. Renames getDomainResolver -> getDomainAssignedResolver to disambiguate from the new effective-resolver lookup.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 277717d 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 |
|
Warning Review limit reached
More reviews will be available in 31 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds DomainResolver.effective via an exported resolver finder and ENSv2 namegraph walker, new getDomainAssignedResolver/getDomainEffectiveResolver helpers, GraphQL wiring, SKILL docs and changeset, integration tests, and devnet fixture/seed support. ChangesDomainResolver.effective Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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. Comment |
|
@greptile review |
Greptile SummaryThis PR adds
Confidence Score: 5/5This PR is safe to merge. The changes are well-scoped, the ENSv1/ENSv2 fork is guarded by an existing datasource-presence check, and both the new namegraph walk and the effective-resolver lookup are covered by integration tests validated against the ENSv2 devnet. The logic is cleanly decomposed: No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DomainResolver.effective] --> B[getDomainEffectiveResolver]
B --> C{domain.canonicalName?}
C -- null --> D[return null]
C -- present --> E[getRootRegistry]
E --> F[findResolverWithIndex]
F --> G{ENSv2Root datasource present?}
G -- No --> H[findResolverWithIndexENSv1\nnamehash-keyed DRR lookup]
G -- Yes --> I[findResolverWithIndexENSv2\nnamegraph walk]
H --> J{DRR found?}
I --> K[forwardWalkDisjointNamegraph\nRecursive CTE by labelHash]
K --> L{deepest ancestor\nwith resolver?}
L -- none --> M[NULL_RESULT]
L -- found --> N[activeResolver + activeName]
J -- none --> M
J -- found --> N
N --> O[makeResolverId\nregistry.chainId + activeResolver]
M --> D
Reviews (6): Last reviewed commit: "refactor(ensapi): rename forward-walk-na..." | Re-trigger Greptile |
Greptile SummaryAdds
Confidence Score: 4/5Safe to merge; the new field is additive and nullable, the rename is internal-only, and all generated artifacts are consistent. The implementation is clean and well-guarded: the canonicalName null-check correctly short-circuits for non-canonical domains, findResolverWithIndex already handles the ENSIP-10 hierarchy walk correctly for ENSv1, and the ResolverId return flows naturally into the existing loadableObjectRef dataloader. The one thing worth noting is that findResolverWithIndex is documented (via its own TODO) as ENSv1-only — ENSv2 canonical domains will silently get null for effective, which is a silent false-negative rather than a crash, and is a known pre-existing limitation being tracked separately. apps/ensapi/src/omnigraph-api/lib/get-domain-resolver.ts warrants a second look if/when ENSv2 resolver support is added, to ensure getDomainEffectiveResolver branches correctly by domain type. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as GraphQL Client
participant DR as DomainResolver (Pothos)
participant GER as getDomainEffectiveResolver
participant GAR as getDomainAssignedResolver
participant DB as ensDb
participant FRI as findResolverWithIndex
participant RL as ResolverRef Loader
C->>DR: "query { domain { resolver { assigned effective } } }"
DR->>GAR: getDomainAssignedResolver(domainId)
GAR->>DB: domainResolverRelation.findFirst(domainId, with: resolver)
DB-->>GAR: DRR with resolver object
GAR-->>DR: "Resolver | undefined"
DR->>GER: getDomainEffectiveResolver(domainId)
GER->>DB: "domain.findFirst(id=domainId, with: registry)"
DB-->>GER: "Domain { canonicalName, registry }"
alt canonicalName is null
GER-->>DR: null
else canonicalName present
GER->>FRI: findResolverWithIndex(registry, canonicalName)
FRI->>DB: domainResolverRelation.findMany(registry + name hierarchy)
DB-->>FRI: DRR records
FRI-->>GER: "{ activeResolver: Address | null }"
alt no active resolver
GER-->>DR: null
else resolver found
GER-->>DR: ResolverId
end
end
DR->>RL: load([ResolverId])
RL->>DB: resolver.findMany(ids)
DB-->>RL: Resolver[]
RL-->>DR: "Resolver | null"
DR-->>C: "{ assigned: Resolver, effective: Resolver | null }"
Reviews (2): Last reviewed commit: "docs: trim DomainResolver.effective desc..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR extends the ENSAPI Omnigraph GraphQL surface to expose a Domain’s ENSIP-10 effective resolver (derived by walking the canonical name hierarchy within the domain’s registry using indexed Domain↔Resolver relationships), complementing the existing directly-assigned resolver field.
Changes:
- Adds
DomainResolver.effective: Resolverto the Omnigraph API and generated SDL. - Introduces
getDomainEffectiveResolver(domainId)and renames the existing assigned-resolver helper togetDomainAssignedResolver. - Exports
findResolverWithIndexfor reuse by Omnigraph API logic, and adds a Changeset for anensapipatch release.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/enssdk/src/omnigraph/generated/schema.graphql | Generated SDL updated to include DomainResolver.effective. |
| apps/ensapi/src/omnigraph-api/schema/domain-resolver.ts | Adds the new effective resolver field and wires it to the new lib helper. |
| apps/ensapi/src/omnigraph-api/lib/get-domain-resolver.ts | Splits assigned vs effective resolver lookup into separate exported helpers. |
| apps/ensapi/src/lib/protocol-acceleration/find-resolver.ts | Exports findResolverWithIndex for external use by Omnigraph API. |
| .changeset/domain-resolver-effective.md | Declares an ensapi patch release for the new API field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Make findResolverWithIndex ENSv2-aware so DomainResolver.effective works across both data models. It now forks by namespace: ENSv1-only namespaces keep the namehash + Domain-Resolver-Relation lookup (Protocol Acceleration data only), while ENSv2 namespaces walk the namegraph by labelHash and return the deepest ancestor Domain with an assigned Resolver. - Extract the disjoint namegraph walk into a shared forward-walk-namegraph module, reused by findResolverWithIndex and getDomainIdByInterpretedName. - Add getRootRegistry (AccountId) and DRY getRootRegistryId through it. - Seed noresolver.parent.eth (no Resolver under a Resolver-bearing parent) to exercise effective-Resolver fallback, plus an integration test covering assigned===effective and the fallback case. The ENSv2 walk depends on the Unigraph-maintained domain table; the fork preserves protocol-acceleration-in-isolation for ENSv1 until the plugins are folded (TODO(fold-protocol-acceleration) markers added).
shrugs
changed the title
![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
♻️ Duplicate comments (1)
packages/ensskills/skills/omnigraph/SKILL.md (1)
142-142:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove implementation detail from the effective field description.
The description includes "identified by walking the name hierarchy within the Domain's Registry", but this phrase was requested to be dropped in a previous review and is inconsistent with the authoritative schema source in
apps/ensapi/src/omnigraph-api/schema/domain-resolver.ts, which describes the field as simply "The Resolver that ENS Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its effective Resolver."📝 Align with schema source
-- effective: Resolver — The Resolver that ENS Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its _effective_ Resolver, identified by walking the name hierarchy within the Domain's Registry. Null when no active Resolver exists or the Domain is not in the canonical nametree. +- effective: Resolver — The Resolver that ENS Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its _effective_ Resolver. Null when no active Resolver exists or the Domain is not in the Canonical Nametree.🤖 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/ensskills/skills/omnigraph/SKILL.md` at line 142, Update the "effective" field description in packages/ensskills/skills/omnigraph/SKILL.md to match the authoritative schema in apps/ensapi/src/omnigraph-api/schema/domain-resolver.ts by removing the implementation detail phrase "identified by walking the name hierarchy within the Domain's Registry" so the description reads only "The Resolver that ENS Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its effective Resolver" and preserve the note about null when no active Resolver exists or the Domain is not in the canonical nametree.
🤖 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 `@apps/ensapi/src/lib/protocol-acceleration/forward-walk-namegraph.ts`:
- Around line 34-35: The helper forwardWalkDisjointNamegraph currently accepts
paths longer than the CTE limit and can silently truncate them; add an explicit
guard at the start of forwardWalkDisjointNamegraph that checks if path.length >
MAX_SUPPORTED_NAME_DEPTH and rejects (throw an Error or return a clear failure)
instead of proceeding, and apply the same guard to the other related helper(s)
in this file (the block covering lines 42-75) so that findResolverWithIndexENSv2
cannot resolve with an over-depth path; reference the constant
MAX_SUPPORTED_NAME_DEPTH and the function name forwardWalkDisjointNamegraph (and
mirror this check in the sibling function(s) in the same file).
In `@apps/ensapi/src/omnigraph-api/lib/get-domain-resolver.ts`:
- Around line 34-51: The code currently uses getRootRegistry(namespace) and then
calls findResolverWithIndex(registry, domain.canonicalName) and makeResolverId({
chainId: registry.chainId, address: activeResolver }), which pins resolution to
the root registry; instead load the Domain's registry from the Domain record you
fetched (e.g., domain.registry or domain.registryId) and pass that domain
registry into findResolverWithIndex and makeResolverId so both the lookup and
the resulting resolver id use the Domain's own registry rather than the root
registry.
---
Duplicate comments:
In `@packages/ensskills/skills/omnigraph/SKILL.md`:
- Line 142: Update the "effective" field description in
packages/ensskills/skills/omnigraph/SKILL.md to match the authoritative schema
in apps/ensapi/src/omnigraph-api/schema/domain-resolver.ts by removing the
implementation detail phrase "identified by walking the name hierarchy within
the Domain's Registry" so the description reads only "The Resolver that ENS
Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its effective
Resolver" and preserve the note about null when no active Resolver exists or the
Domain is not in the canonical nametree.
🪄 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: f1c8774f-25a8-4649-9dfd-75b1f648902a
⛔ Files ignored due to path filters (2)
packages/enssdk/src/omnigraph/generated/introspection.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**
📒 Files selected for processing (12)
.changeset/domain-resolver-effective.mdapps/ensapi/src/lib/protocol-acceleration/find-resolver.tsapps/ensapi/src/lib/protocol-acceleration/forward-walk-namegraph.tsapps/ensapi/src/omnigraph-api/lib/get-domain-by-interpreted-name.tsapps/ensapi/src/omnigraph-api/lib/get-domain-resolver.tsapps/ensapi/src/omnigraph-api/schema/domain-resolver.integration.test.tsapps/ensapi/src/omnigraph-api/schema/domain-resolver.tspackages/ensnode-sdk/src/shared/root-registry.tspackages/ensskills/skills/omnigraph/SKILL.mdpackages/integration-test-env/src/devnet/fixtures.tspackages/integration-test-env/src/seed/effective-resolver-fallback.tspackages/integration-test-env/src/seed/index.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Guard against paths longer than MAX_SUPPORTED_NAME_DEPTH so an over-depth name throws rather than resolving against a CTE-truncated ancestor path.
|
@greptile review |
…k-disjoint-namegraph Match the file name to its contents (forwardWalkDisjointNamegraph + helpers).
| effective: t.field({ | ||
| description: | ||
| "The Resolver that ENS Forward Resolution (ENSIP-10) lands on for this Domain — i.e. its _effective_ Resolver. Null when no active Resolver exists or the Domain is not in the Canonical Nametree.", | ||
| type: ResolverRef, | ||
| nullable: true, | ||
| resolve: (domainId) => getDomainEffectiveResolver(domainId), |
Summary
Adds
DomainResolver.effectiveto the Omnigraph API — the Resolver that ENS Forward Resolution (ENSIP-10) lands on for a Domain, identified from indexed data. Complements the existingDomainResolver.assigned(the Domain's directly-assigned Resolver).To make this work against ENSv2 data (where Domains are keyed by StorageId, not namehash),
findResolverWithIndexis made ENSv2-aware.Changes
Effective resolver
get-domain-resolver.ts— addsgetDomainEffectiveResolver(resolves viafindResolverWithIndexfrom the root registry); renamesgetDomainResolver→getDomainAssignedResolver.domain-resolver.tsexposes the nullableeffective: Resolverfield.ENSv1/ENSv2 fork in
findResolverWithIndexENSv2Rootdatasource) keep the namehash + Domain-Resolver-Relation lookup, depending only on Protocol Acceleration data — so it still works when the Protocol Acceleration plugin runs in isolation.forwardWalkDisjointNamegraph) and return the deepest ancestor Domain with an assigned Resolver — which also yields ENSIP-10 fallback for free.domaintable; the fork preserves the ENSv1 protocol-acceleration-only path until the plugins are folded.TODO(fold-protocol-acceleration)markers prep the codebase for that refactor (the ENSv1 path then deletes and the walk handles both).forward-walk-namegraphmodule, now reused byfindResolverWithIndexandgetDomainIdByInterpretedName(DRY).ensnode-sdk: addsgetRootRegistry(AccountId, ENSv2-preferring) and DRYsgetRootRegistryIdthrough it.Test fixture + integration test
noresolver.parent.eth(no Resolver, under the Resolver-bearingparent.eth) to exercise effective-Resolver fallback.assigned === effectiveand the fallback case.Validation
pnpm generate, rootpnpm typecheck,pnpm lint✅pnpm test --project ensapi✅ (288)assigned === effectivefor resolver-bearing names, andnoresolver.parent.eth→assigned: null,effective=parent.eth's resolver.