fix(ensapi): correct ensv2 fallback behavior#2290
Conversation
The namegraph walk (domain(by: name)) fell back to the ENSv1 disjoint namegraph whenever the ENSv2 Root walk didn't exact-match. UniversalResolverV2 has no such registry-level fallback: its only ENSv1 fallback is the ENSV1Resolver mirror Resolver set within the ENSv2 namegraph on reserved entries, which the walk already follows. Revert the fallback so the walk faithfully models UR2. Faithfully reserve the ENSv1 devnet fixtures in the ENSv2 ETHRegistry (resolver = ENSV1Resolver), mirroring migration, so domain(by: name) returns the preferred ENSv2 Domain. Update the integration tests accordingly.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR removes the ENSv2→ENSv1 fallback behavior from namegraph resolution and replaces it with ENSv2 reserved-entry mirroring: ENSv1 names are seeded into the ENSv2 ETHRegistry via ENSV1Resolver, the resolution logic no longer attempts a secondary ENSv1-root walk, and tests verify that ENSv1-only names now resolve through their ENSv2 reserved entries. ChangesENSv1 Fallback Removal and ENSv2 Reserved-Entry Migration
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
shrugs
changed the title
There was a problem hiding this comment.
Pull request overview
Aligns ENSApi’s namegraph walk (domain(by: name) and related lookups) with UniversalResolverV2 semantics by removing an artificial ENSv1 registry-level fallback when the ENSv2 root walk doesn’t exact-match, and updates the devnet seed + integration assertions to reflect the reserved-entry mechanism (ENSv2 entry pointing at ENSV1Resolver).
Changes:
- Removed the ENSv1 disjoint-namegraph fallback from
forwardWalkNamegraph’s internal walk logic, preserving only resolver-mediated hops (e.g.ENSv1Resolver,ENSv2Resolver, bridged resolvers). - Updated devnet seeding so ENSv1 fixtures are also registered in the ENSv2
ETHRegistrywithresolver = ENSV1Resolver, mirroring the migration “reserved entry” shape used for UR2 resolution. - Updated integration tests to assert ENSv1 fixture lookups resolve to the reserved ENSv2 Domain IDs (and literal canonical names), matching the new walk behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/integration-test-env/src/seed/registered-names.ts | Reserves ENSv1 fixture names in ENSv2 ETHRegistry via ENSV1Resolver to mirror migration behavior under UR2. |
| apps/ensapi/src/test/integration/devnet-names.ts | Clarifies that ENSv1 fixtures are reserved in ENSv2; keeps a dedicated list for ENSv1-targeted assertions. |
| apps/ensapi/src/omnigraph-api/schema/query.integration.test.ts | Updates domain(by: name) integration assertions so ENSv1 fixture names resolve to the reserved ENSv2 Domains/IDs. |
| apps/ensapi/src/omnigraph-api/lib/get-domain-by-interpreted-name.ts | Removes the fabricated ENSv1 fallback when ENSv2 doesn’t exact-match, keeping only resolver-driven hops. |
| .changeset/walk-namegraph-no-ensv1-fallback.md | Adds a changeset documenting the behavior change as a patch for ensapi. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR reverts a fabricated ENSv1 fallback in the namegraph walk (
Confidence Score: 5/5Safe to merge — the fallback removal is a targeted, well-reasoned revert, and the devnet seed + tests are updated to stay consistent with the new protocol-accurate behavior. All four changed files move in lockstep: the walk is corrected, the devnet seed mirrors it, and the tests are updated to assert the now-preferred ENSv2 domain. The removed code path had no legitimate protocol backing per UR2, so its absence does not regress real behaviour. No files require special attention. The seed change adds one sequential registerEthName call per ENSv1 fixture and the test merges two cases into one — both are low-risk mechanical changes. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["forwardWalkNamegraph(name)"] --> B["walkNamegraphFromRegistry(ENSv2 Root)"]
B --> C{Exact match in ENSv2?}
C -- "Yes + has resolver" --> D["Return ENSv2 Domain"]
C -- "No match" --> E["Find deepest resolver"]
E --> F{Resolver type?}
F -- "Bridged Resolver" --> G["Recurse to target registry"]
F -- "ENSv1Resolver" --> H["Recurse to ENSv1 Root"]
F -- "ENSv2Resolver" --> I["Recurse to ENSv2 Root"]
F -- "None / other" --> J["Not resolvable"]
H --> K["Return ENSv1 Domain"]
Reviews (1): Last reviewed commit: "chore: remove changeset" | Re-trigger Greptile |
What
The namegraph walk (
domain(by: name)and related Name→Domain lookups) fell back to the ENSv1 disjoint namegraph whenever the ENSv2 Root walk didn't exact-match (added in #2286). UniversalResolverV2 has no registry-level ENSv1 fallback. Its only ENSv1 fallback is theENSV1Resolver(a mirror Resolver) set within the ENSv2 namegraph on reserved entries — which the walk already follows via its existingENSv1Resolverhop. I confirmed this againstcontracts-v2(LibRegistry.findResolverwalks only the ENSv2 registry tree; the overview docs describe theENSV1Resolverreserved-entry mechanism).So a name present only in ENSv1 and not reserved in ENSv2 is simply not resolvable under UR2 — the fallback fabricated resolution the protocol doesn't have.
Changes
get-domain-by-interpreted-name.ts(file is byte-identical to its pre-feat(ensapi): improve seedDevnet - add names with contenthash #2286 state) so the walk faithfully models UR2.resolver = ENSV1Resolver), mirroring migration.domain(by: name)now returns the preferred ENSv2 Domain.domain(by: name)ENSv1 blocks (which only passed via the fallback) now assert the reserved ENSv2 Domain id + literal canonical name. Thedomains(where: name eq)direct-lookup and ENSv1 virtual-registry tests are untouched.Notes
ETHRegistrarcommit-reveal (owner = seed account) rather than a migration-controllerowner=0reserved registration. Indistinguishable to the walk/indexer (what matters is the ENSv2 entry withresolver = ENSV1Resolver), but a simplification of the true reserved-entry shape.test:integration:ci) — not exercised locally.Verify
pnpm -F ensapi typecheck✓ ·pnpm -F @ensnode/integration-test-env typecheck✓ ·pnpm lint✓ ·pnpm test --project ensapi→ 293 passed