Skip to content

feat: resolve ENS names and reject misshaped files on voter CSV import#332

Open
tnrdd wants to merge 2 commits into
mainfrom
feat/voter-import-ens
Open

feat: resolve ENS names and reject misshaped files on voter CSV import#332
tnrdd wants to merge 2 commits into
mainfrom
feat/voter-import-ens

Conversation

@tnrdd

@tnrdd tnrdd commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Voter CSV import had two failure modes that produced silent no-ops:

  • A roster whose wallet wasn't in the first column (e.g. Name, Wallet, Email) had every row skipped with no feedback.
  • ENS names (name.eth) in the address column were rejected as invalid addresses.

Changes

  • ENS resolution: .eth names in the address column are resolved to addresses (mainnet) before validation/sync.
  • Shape validation: files with no address in the first column are now rejected with a clear, dismissible error linking the template, instead of silently skipping every row.
  • Added src/lib/ens.ts (resolveEnsNames) plus pure helpers isEnsName / collectEnsNames / applyEnsResolutions / validateCsvShape, with unit tests.

Verification

  • pnpm vitest run (voterCsv): 15 passing
  • pnpm typecheck, pnpm lint, pnpm prettier: clean

@vercel

vercel Bot commented Jun 30, 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 Jun 30, 2026 1:12pm

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: Approved

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

Pull Request Summary

This PR adds ENS name resolution during voter CSV import — ENS names in the address column are resolved to wallet addresses before the sync runs — and rejects files that don't match the expected template shape (address in the first column). New pure utility functions are well-tested, and the UI surfaces progress and errors clearly.

Review Summary

🟡 Warning — Async Papa.parse callback has no error boundary
The complete callback in handleCsvImport is now async but has no try-catch. If resolveEnsNames (or anything else in the callback) throws, the 'Resolving ENS names…' note stays permanently visible with no way to dismiss it, and there's an unhandled promise rejection. Wrap the async body in try/catch and show the error via setImportError.

🔵 Suggestion — ENS client missing batch config that exists elsewhere
The existing useEnsResolution.ts hook creates its mainnet client with batch: { batchSize: 100, wait: 10 }. The new ensClient in src/lib/ens.ts doesn't batch, so a CSV with many ENS names fires one RPC call per name. Consider adding the same batch config, or even sharing the client.

Clean, well-structured PR. Logic is properly separated into pure functions with good test coverage. The main concern is the unguarded async callback: if resolveEnsNames unexpectedly throws, the "Resolving ENS names…" message gets stuck with no way for the user to recover. The ENS client should also pick up the same batching config used elsewhere. Neither is a blocker, but the missing try-catch is the kind of thing that bites you in prod.

📌 2 inline comments
🟡 Warning: 1 · 🔵 Suggestion: 1


🔍 Reviewed by Gaston

Comment thread src/app/flow-councils/membership/VoterTable.tsx
Comment thread src/lib/ens.ts

@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 during voter CSV import, validates that uploaded files match the expected template shape, and extracts a shared batched mainnet client used by both the new resolver and the existing useEnsResolution hook.

Both concerns from the prior review are addressed cleanly: the async callback now has a try-catch that surfaces errors in the UI, and the ENS client is extracted to src/lib/ens.ts with batching config, shared across both call sites. The pure utility functions have solid test coverage, the error UX is sensible (dismissible alert with a template link), and the skipEmptyLines addition is a nice touch. Good work — ship it.


🔍 Reviewed by Gaston

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