Skip to content

feat: fall back to ENS for voter table and public profile names#335

Merged
tnrdd merged 4 commits into
mainfrom
feat/ens-fallback
Jul 2, 2026
Merged

feat: fall back to ENS for voter table and public profile names#335
tnrdd merged 4 commits into
mainfrom
feat/ens-fallback

Conversation

@tnrdd

@tnrdd tnrdd commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Anywhere the app renders a wallet's platform name, the display now falls back to its ENS name before the truncated address, matching what the comms feeds and the Permissions table already do. Order everywhere: platform name, then ENS name, then truncated address.

Changes

  • Voter table (Flow Councils membership): the Name column now falls back to ENS when a voter has no platform profile name. ENS resolution is scoped to the visible page (up to 50 rows) so a large roster does not fan out thousands of mainnet reverse lookups.
  • Public profile page (/profile/[address]): the heading falls back to ENS between the platform display name and the truncated address.

Already correct (unchanged)

Permissions table, comms feed messages, useProfileDisplayName, and AccountDropdown.

Out of scope

The flow-splitters and flow-guilds graphs and activity feeds resolve ENS but never use the platform name (the opposite gap), so they were left as-is.

@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
platform Ready Ready Preview, Comment Jul 2, 2026 5:21am

Request Review

@gaston-review gaston-review 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.

Gaston Review

Verdict: Comment

Score: ████████░░ 8/10

Pull Request Summary

This PR adds ENS name resolution as a fallback in two places: the VoterTable (flow councils membership) and the public profile page. When a user doesn't have a platform display name, it now tries to resolve their ENS name before falling back to a truncated address. The VoterTable implementation is smart about only resolving ENS for the current page of voters (up to 50) rather than the entire roster.

Review Summary

🟡 Warning — Truthy check changed to nullish coalescing for profile name
In VoterTable, the old code used name ? name : truncateAddress(account) which treats empty strings as falsy (falling through to truncateAddress). The new code uses name ?? ensByAddress?.[account]?.name ?? truncateAddress(account) which only checks for null/undefined. If the profile API ever returns an empty string for a name, the old code would show a truncated address, but the new code would render an empty cell. Depending on the API contract this may not be an issue, but it is a behavior change.

🔵 Suggestion — Inconsistent fallback operators between VoterTable and PublicProfile
VoterTable uses ?? (nullish coalescing) for the ENS fallback chain, while PublicProfile uses || (logical OR). Both should use the same operator for the same semantic operation. Since most of the codebase uses ?? for ENS lookups, the PublicProfile should probably use ?? as well — or if empty-string protection is desired, both should use ||.

Clean, focused change. The pagination-scoped ENS resolution in VoterTable is a good design choice. The stable-key memoization pattern avoids unnecessary re-fetches. I have one behavioral concern around the operator change from truthy to nullish coalescing, and a minor inconsistency between the two files.

📌 2 inline comments
🟡 Warning: 1 · ⚪ Nitpick: 1


🔍 Reviewed by Gaston

Comment thread src/app/flow-councils/membership/VoterTable.tsx Outdated
Comment thread src/app/profile/[address]/public-profile.tsx Outdated

@gaston-review gaston-review 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.

Gaston Review

Verdict: Approved

Score: ████████░░ 8/10

Pull Request Summary

This PR adds ENS name resolution as a fallback for display names in two places: the VoterTable (flow councils membership roster) and the public profile page. When a user lacks a platform display name, it resolves their ENS name before falling through to a truncated address. The VoterTable implementation scopes ENS resolution to the current page of voters only, avoiding thousands of unnecessary mainnet reverse lookups.

Both concerns from the prior review are addressed: the fallback chain now uses || consistently in both files, preserving the original empty-string-falls-through behavior. Key casing is correct — VoterTable lowercases account before lookup, and PublicProfile uses address.toLowerCase() for the ENS map key while the hook normalizes internally. Clean, well-scoped change that's ready to ship.


🔍 Reviewed by Gaston

@gaston-review gaston-review 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.

Gaston Review

Verdict: Approved

Score: ████████░░ 8/10

Pull Request Summary

This PR centralizes the display-name fallback chain (profile name → ENS name → truncated address) into a shared resolveDisplayName utility used by VoterTable, PublicProfile, and MessageItem. The useEnsResolution hook gets a significant upgrade: module-level caching to avoid redundant RPC calls when paging back and forth, an avatars option to skip avatar resolution where unneeded, and a proper cancellation pattern for stale async results. VoterTable scopes ENS resolution to the visible page only and debounces filter inputs to avoid fanning out lookups per keystroke.

Both concerns from the first review (inconsistent ?? vs || operators, empty-string fallthrough behavior) are resolved — || is used consistently throughout. The caching strategy in useEnsResolution is well-designed: successful lookups are cached, failures are left uncached for retry, and the cancellation flag correctly prevents stale state writes without discarding cache-worthy data. The debounce + page-scoped ENS resolution in VoterTable is a smart performance call. Clean, production-ready work.

📌 2 inline comments
⚪ Nitpick: 2


🔍 Reviewed by Gaston

Comment thread src/hooks/useEnsResolution.ts
Comment thread src/app/flow-councils/components/chat/MessageItem.tsx

@gaston-review gaston-review 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.

Gaston Review

Verdict: Approved

Score: ████████░░ 8/10

Pull Request Summary

This PR adds ENS name resolution as a fallback for display names in VoterTable, PublicProfile, and MessageItem, centralizing the fallback chain (profile name → ENS name → truncated address) into a shared resolveDisplayName utility. The useEnsResolution hook gets module-level caching with a 10k-entry cap and oldest-first eviction, an avatars option to skip avatar lookups, and a cancellation pattern for stale async results. VoterTable scopes ENS resolution to the visible page and debounces filter inputs.

The latest push addresses the one remaining nitpick from the prior review — the module-level caches are now capped at 10,000 entries with FIFO eviction. The implementation is correct and minimal. This PR has been through three review rounds, all prior concerns are resolved, and the code is production-ready.


🔍 Reviewed by Gaston

@tnrdd tnrdd merged commit 21d8c2d into main Jul 2, 2026
2 checks passed
@tnrdd tnrdd deleted the feat/ens-fallback branch July 2, 2026 05:30
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.

1 participant