feat(ui): resolve ens over the chain, offchain names by service - #346
mariano-aguero wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Findings outside the diff
These sit on lines this PR did not change, but this PR is what makes them wrong. They cannot be posted as inline comments.
- [WARNING]
.github:2— Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: amannn/action-semantic-pull-request@v5. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
Reviewer callouts (1)
Non-blocking observations. Nothing here needs to change before merge.
.github:1— "The ubuntu-latest label will migrate to Ubuntu 26 beginning October 19, 2026. For more information, see actions/runner-images#14748"
|
Closing this in favour of a smaller split. The two bugs it fixed stand on their own and are now in #348, where the shared public client refuses offchain lookups, and #349, where reverse answers are keyed by address and chain, each with tests that fail without the fix. Reading the reverse record over the chain is not needed for either one: it costs a request per address plus a records read where the service answers in a single call, and the keyless case that started this branch is being handled server side, which also covers the offchain names a client read cannot reach on its own. #347 carries the jsdom test fix that travelled along here and was never part of the same change. |
What
ENS names and avatars on the signing, permission and onboarding screens now resolve over the chain, through the universal resolver, instead of through the
/ens/v2/reverseservice. A name whose resolver is offchain still goes to that service, which is the one hop that may follow a CCIP gateway.Three things change for whoever reads a dialog:
ccipRead: false, so a signing screen never fetches a host that a resolver named. Avatars render through the ENS metadata proxy for the same reason.Case by case:
ccipRead: falsegetEnsText, rendered through the ENS metadata proxyThe diff also carries a jsdom fix in
apps/keys-jaw-id/src/lib/session-manager.test.ts, unrelated to ENS: on Node 25 the shared setup puts an in-memory stub instorageAreaand jsdom'sStorageEventconstructor rejects it. Say so if you would rather see it on its own branch.Why
Two of the three changes above are bugs on a screen where the user is about to sign something, and they are the reason to merge this.
The wrong name per chain is the visible one. Reverse resolution was keyed by address alone, so two chains asking for the same address collapsed onto one entry and whichever answered last was rendered on both rows. An address with a name set on Base showed that name on a mainnet transaction.
The CCIP exposure is the quieter one. Following an offchain lookup means fetching the urls that a resolver revert names, and a resolver is chosen by whoever owns the name or the address. A
toin a call the user is about to sign can point the page at a host of its choosing, and a certificate error there taints the page and blocks the passkey ceremony in strict browsers.ccipRead: falsemakes that refusal explicit, and the names that do need a gateway are asked of a server that can talk to one. The avatar goes through the ENS metadata proxy on the same reasoning: the record is an arbitrary url and the page renders it.Dropping the service hop for onchain names is the smaller half. It removes a dependency for something the wallet can do itself, and it is one request less per dialog on the path almost every name takes.
This branch started from ENGR-1277, which read the same work as the fix for a dApp served without an api key getting no names. That premise was wrong and the ticket is canceled:
usePasskeysanduseLoginin the keys app fall back toNEXT_PUBLIC_API_KEYwhen the dApp passes none, so therpcUrlreverse resolution is handed always carries a key. The 400 that started it came from a url built by hand. Nothing in the diff depends on that framing, and the two bugs above stand on their own.One leftover from it is worth a look:
carriesApiKeyinreverseResolve.tsskips the offchain hop when the url has no key. If the url always has one, that branch is unreachable and can go. Left in for now because it is the only thing standing between a keyless url and a request that would certainly fail.What it costs
Resolving N addresses used to be one request with the records inlined. It is now N reverse reads, batched into a multicall, plus a text record read per name, behind a two second budget and a sixty second memory so a re-render does not ask again. Whatever has not arrived by the budget renders as hex, which is what already happened when a node did not answer.
Three fixes landed after the first round of review, each with a test that fails without it. The default record is read up front and awaited only when the chain's own record has no name, so on the paths that drop it the rejection is marked handled instead of surfacing as an unhandled rejection on the signing page. The offchain requests are caught per chain, so a request that fails leaves its own addresses unanswered rather than discarding the names another chain already returned. And
TransactionDialogresolves every address against the one chain it shows, which is what its display maps and its child sections are keyed by.Closes N/A. ENGR-1277 is canceled and #345 is closed, neither by this PR.
How to test
bun install bunx nx run-many -t typecheck lint test --projects=@jaw.id/ui bunx nx build @jaw.id/uiThe three fixes are pinned by tests that fail against the commit before them. Reverting
packages/ui/src/utils/reverseResolve.tsto that commit and rerunningbunx vitest run src/utils/reverseResolve.test.tsinsidepackages/uigives 3 failed, 18 passed, two of them reporting the rejection nobody handled. Worth reading in that file:plainClient, which exists because avi.fnhooks the promise it returns and marks a dropped rejection as handled, which is what hid this bug from the original suite.Live, against a public node, with
ccipRead: falseas the shared client now has it:The last two are the hinge of the split: an offchain name announces itself with the selector the code watches for, and an onchain one resolves without the page leaving our transport.
Worth a second pair of eyes: that selector check is exercised in the suite against a constructed error, and the live check above ran on a plain client. The client the dialogs use has
batch: { multicall: true }, where the revert arrives as thereturnDataof anaggregate3entry. If the shape differs there, an offchain name falls through as unanswered and never reaches the service, which would take.justan.idsubnames with it. Confirm against the batched client before approving.In a dialog: open a transaction whose counterparty has an onchain ENS name and see the name and avatar, with no request to
api.justaname.id/ens/v2/reversefor it in the network tab.Checklist
nx releasetakes the version bump from its typebunx prettier --check .andbunx nx affected -t lint test typecheck build api-checkpass, which is what CI runs@jaw.id/core's public API is unchanged, orbunx nx api-update @jaw.id/coreran and the report diff is part of this PRDocs are deliberately untouched:
concepts/ens.mdxdescribes subnames resolving through a CCIP-Read gateway, which stays true for the person reading it.