Skip to content

feat: ENS name search in the main search bar - #832

Closed
barnabasbusa wants to merge 2 commits into
masterfrom
bbusa/ens-search
Closed

feat: ENS name search in the main search bar#832
barnabasbusa wants to merge 2 commits into
masterfrom
bbusa/ens-search

Conversation

@barnabasbusa

Copy link
Copy Markdown
Collaborator

Summary

Adds forward ENS resolution (vitalik.eth → address) to the main search bar, building on the existing reverse-resolution (address → name) subsystem — no new dependencies.

  • Typing a complete ENS name shows an "ENS Names" typeahead section with the resolved address (plus a Contract badge when applicable); selecting it goes to /address/0x…
  • Submitting the search form with an ENS name redirects straight to the address page (checked after the numeric/hex resolvers, before validator-name/graffiti fallbacks; falls through if the name doesn't resolve)
  • Subdomains (sub.name.eth), non-.eth TLDs and emoji names are supported; a shape check (complete name, TLD label ≥ 3 chars) avoids per-keystroke eth_calls while typing

ENS is always resolved against mainnet

ENS lives on Ethereum mainnet, so lookups now always use the dedicated ensResolver.endpointsdefaulting to https://ethereum-rpc.publicnode.com when unset. The previous fallback to the local execution pool is removed: on devnets/testnets that pool serves a chain without an ENS deployment, so it could never resolve anything. This applies to the existing reverse resolution too. Operators who don't want the public-RPC dependency can point ensResolver.endpoints at their own mainnet node.

Implementation notes

  • services/ensresolver_ens.go: new resolveForward()registry.resolver(namehash(name))resolver.addr(node), reusing the existing namehash/calldata/eth_call helpers and registry priority order
  • services/ensresolver.go: exported ResolveEnsName() with its own LRU cache (positive/negative entries honor refreshPositive/refreshNegative), 10s call timeout
  • handlers/search.go: ENS branch in the full-search resolver + new ens search-ahead type; the global 0x strip is skipped for ENS queries since labels like 0xdeadbeef.eth are legal
  • Feature is gated on ensResolver.enabled && executionIndexer.enabled (results redirect to /address, which requires the indexer)
  • ENS names in typeahead suggestions are HTML-escaped (same as graffiti results)

Same known limitations as the existing reverse path: lowercase normalization only (no UTS-46/ENSIP-15), no wildcard/CCIP-read (ENSIP-10) resolvers.

Testing

  • go build ./... and go vet clean
  • Name-shape regex verified against vitalik.eth, sub.vitalik.eth, 0xdeadbeef.eth, nick.xyz, emoji names, and partial-keystroke negatives

Adds forward ENS resolution (name -> address) on top of the existing
reverse-resolution subsystem, wired into the header search:

- typing a complete ENS name (e.g. vitalik.eth) shows an "ENS Names"
  typeahead section with the resolved address; selecting it (or
  submitting the search) redirects to the /address page
- new "ens" search-ahead type; the global 0x-strip is skipped for it
  since ENS labels may legitimately contain "0x"
- forward results are cached in a dedicated LRU (honoring the existing
  refreshPositive/refreshNegative intervals) plus the page cache
- gated on both ensResolver.enabled and executionIndexer.enabled
  (resolved names redirect to /address, which needs the indexer)

ENS lookups now always run against Ethereum mainnet: the configured
ensResolver.endpoints, defaulting to a public mainnet RPC
(ethereum-rpc.publicnode.com) when unset. The local execution pool is
no longer used as fallback - on devnets/testnets it serves a chain
without an ENS deployment.
redpandabot[bot]

This comment was marked as outdated.

@redpandabot redpandabot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Adds forward ENS name resolution (name→address) to the main search bar and full-search submit path, gated on ensResolver+execution indexer, with a complete-name regex to avoid per-keystroke eth_calls and HTML-escaping consistent with the graffiti path. The resolution/escaping/caching logic traces cleanly; the main thing I'd fix is the newly-concurrent use of the unsynchronized dedicated ENS RPC client init.

Issues

  • 🟡 services/ensresolver.go:481dedicated ENS rpc client Initialize() is now called concurrently (data race) — see the thread on that line

Reviewed @ 2690e8db
"Senior engineering is mostly knowing what not to build."

Comment thread services/ensresolver.go
}
e.dedicatedInit.Do(e.initDedicatedClients)
for _, ec := range e.dedicatedClients {
if err := ec.client.Initialize(ctx); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 dedicated ENS rpc client Initialize() is now called concurrently (data race)

On master, getEthClient() was only reached from the single worker loop (processBatch); this PR adds ResolveEnsName(), which runs on HTTP handler goroutines (SearchAhead/Search) and also calls getEthClient(), so ec.client.Initialize() (clients/execution/rpc/executionapi.go:89) can run concurrently from handler threads and the worker. Initialize() does an unlocked check-then-act on ec.ethClient and writes ec.rpcClient/ec.ethClient (lines 103-104) while GetEthClient() reads them — a fresh data race, and each racing caller passes the nil check and dials/leaks an extra RPC connection. Couldn't run go test -race in this sandbox (no Go toolchain), so impact assessment is static: both callers still get a usable client, so it's connection churn plus race-detector noise rather than a functional break — guarding Initialize() with its own mutex/sync.Once would fix it.

@barnabasbusa

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants