fix(ensapi): delegate to UniversalResolver for unaccelerated forward resolution#2268
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 62ae07d 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 |
|
Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes. Warning Review limit reached
More reviews will be available in 30 minutes and 36 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 (1)
📝 WalkthroughWalkthroughForward Resolution is refactored to centralize UniversalResolver delegation: a new helper function standardizes delegated calls, temporary ENSv2 bailout logic is replaced with unified conditional delegation, and final-stage resolution now routes to UniversalResolver on the ENS Root Chain or conditionally to the active resolver on shadow chains with ENSIP-10 compatibility checks. ChangesUniversalResolver delegation centralization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 SummaryThis PR fixes forward resolution in ENSApi so that on-chain resolver behavior gated on
Confidence Score: 5/5Safe to merge. The change correctly unifies all non-indexed forward resolution through the on-chain UniversalResolver, which is the only path that can faithfully reproduce caller-sensitive resolver behavior. The refactoring is well-scoped: resolveViaUniversalResolver is a thin wrapper around the existing executeOperations + withEnsProtocolStep machinery, the removed isExtendedResolver probe and requiresWildcardSupport guard are now subsumed by the UR itself, and the ENSv2 bailout is cleanly folded into the same branch. All 288 tests pass and there are no dangling imports or unused variables. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant resolveForward
participant _resolveForward
participant resolveViaUniversalResolver
participant Acceleration
participant RootChainUR as Root Chain UniversalResolver
Client->>resolveForward: resolve(name, selection)
resolveForward->>_resolveForward: "(name, selection, {registry: ENSRoot})"
alt Non-accelerated OR ENSv2 namespace (Step 0)
_resolveForward->>resolveViaUniversalResolver: (name, ops, rootChainClient)
resolveViaUniversalResolver->>RootChainUR: ENSIP-10 resolve() via CCIP-Read
RootChainUR-->>resolveViaUniversalResolver: resolved operations
resolveViaUniversalResolver-->>_resolveForward: operations
_resolveForward-->>Client: records response
else Accelerated path
_resolveForward->>_resolveForward: Step 1: findResolver
alt Bridged Resolver (e.g. Basenames/Lineanames)
_resolveForward->>_resolveForward: recurse with shadow registry
end
_resolveForward->>Acceleration: Step 3: ENSIP-19 / Static Resolver passes
Acceleration-->>_resolveForward: partially/fully resolved ops
alt All operations resolved
_resolveForward-->>Client: records response (early return)
else Remaining unresolved ops (Step 4)
_resolveForward->>resolveViaUniversalResolver: (name, remaining ops, rootChainClient)
resolveViaUniversalResolver->>RootChainUR: ENSIP-10 resolve() via CCIP-Read
RootChainUR-->>resolveViaUniversalResolver: resolved operations
resolveViaUniversalResolver-->>_resolveForward: operations
_resolveForward-->>Client: records response
end
end
Reviews (3): Last reviewed commit: "fix: delegate all RPC fallback to Univer..." | 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 ENSApi forward resolution to match on-chain behavior by delegating unresolved (non-accelerated) resolution to the ENS Root Chain UniversalResolver, addressing cases where off-chain reconstruction cannot reproduce msg.sender-gated resolver behavior (e.g. the ENSv2-readiness canary).
Changes:
- Added a
resolveViaUniversalResolver()helper to execute remaining forward-resolution operations via the Root ChainUniversalResolver’sresolve(). - Changed forward resolution flow so that when acceleration can’t be attempted (or ENSv2 indexing is present), resolution delegates wholesale to the
UniversalResolver. - For accelerated requests, kept indexed acceleration for known patterns, then delegated remaining Root Chain operations to
UniversalResolverwhile preserving direct-resolution behavior on shadow Registry chains.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/ensapi/src/lib/resolution/forward-resolution.ts | Routes unaccelerated (and Root Chain remainder) forward resolution through the Root Chain UniversalResolver to ensure protocol-faithful results. |
| .changeset/forward-resolution-delegate-universal-resolver.md | Adds a patch changeset documenting the behavioral change for forward resolution. |
💡 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}
- step 4 always delegates to the root-chain UniversalResolver (correctly resolves shadow-Registry names via the original input instead of calling an L2 resolver through the root-chain client) - coerce ENSv2 namespace check to boolean - remove now-orphaned imports (isExtendedResolver, withSpanAsync, getENSRootChainId)
|
@greptile review |
Problem
ur.integration-test.eth(the ENS ENSv2-Readiness canary) resolved to the legacy sentinel0x1111…1111through ENSApi, while the on-chainUniversalResolverreturns the ready sentinel0x2222…2222.Root cause: the canary's
URTestResolvergates itsIExtendedResolversupport onmsg.sender == UniversalResolver.implementation(). It reveals its extended shape (→resolve()→0x2222) only to the canonical UR implementation, and its legacy shape (→addr()→0x1111) to everyone else. ENSApi reconstructed resolution off-chain (findResolver+ a direct call to the discovered resolver, gated on a full-gassupportsInterface), so it could never reproduce the UR's answer. This is not reproducible off-chain by design — not a gas/caller trick we can mimic.Fix
Delegate to the on-chain
UniversalResolverwhenever ENSApi resolves on the ENS Root Chain and cannot satisfy the records purely from indexed data:The shadow-Registry path is unchanged, so cross-chain ENSv1 resolution (navigate L1 → bridged resolver → L2 shadow Registry DRRs → resolve from the L2 resolver) is preserved.
Validation
pnpm typecheck,pnpm lint, andpnpm test --project ensapi(288/288) all pass.