Skip to content

feat(phase-2): wallet_contacts — data model + API (#67)#73

Draft
sacha-l wants to merge 2 commits intodevelopfrom
feat/wallet-contacts-p2-01
Draft

feat(phase-2): wallet_contacts — data model + API (#67)#73
sacha-l wants to merge 2 commits intodevelopfrom
feat/wallet-contacts-p2-01

Conversation

@sacha-l
Copy link
Copy Markdown
Collaborator

@sacha-l sacha-l commented Apr 30, 2026

Summary

Phase 2 P2-01 — identity-layer plumbing. Adds the wallet_contacts table keyed by SS58 address with an optional email and a single notifications_enabled flag. Surfaced through:

  • GET /api/wallet-contacts/:address — public, returns { email_set, notifications_enabled } only. Never leaks the raw email (asserted in tests).
  • PUT /api/wallet-contacts/:address — SIWS-gated for the matching wallet via a new requireOwnWallet middleware. Partial updates: fields not present in the body are preserved.

Closes #67.

What's in the diff

Two commits:

  • f37ac2d — initial implementation (8 new files: migration + repo + service + controller + routes + 3 test files; 3 modified: middleware, validation utils, server.js mount).
  • d3e0c09 — review-blocker fixes:
    1. Added the SIWS domain check (DISABLE_SIWS_DOMAIN_CHECK / EXPECTED_DOMAIN) to requireOwnWallet, matching requireAdmin.
    2. Made upsertByWallet a true partial update (read-modify-write using 'key' in fields to distinguish omitted from explicit-null) — fixed a data-loss bug where PUT { email: 'x' } would silently reset notifications_enabled to true.
    3. Added a regression test that catches the data-loss bug; added an expect(validateEmail).not.toHaveBeenCalled() assertion on the no-email PUT path.

New surface

  • Migration: supabase/migrations/20260430000000_create_wallet_contacts.sql — table + partial index WHERE email IS NOT NULL.
  • Middleware: requireOwnWallet in auth.middleware.js — SIWS-verifies + asserts the signer's pubkey matches the route param's :address pubkey (prefix-agnostic via u8aToHex(decodeAddress(...))). Honors dev-bypass. New SIWS statement "Update notification preferences for wallet on Stadium".
  • Validator: validateEmail in validation.js — RFC 5321 simplified, returns the project-standard { valid, error, normalised } envelope; lowercases + trims into normalised.

Test plan

  • cd server && npm test89/89 passing (was 80/80 on develop; this PR adds 9 server-side tests across 3 files, plus the regression test).
  • cd client && npm run build — clean.
  • cd client && npm run lint — 0 errors, 0 warnings.

UI verification (stadium-tester)

Not applicable for this PR — server-only API, no UI surface.

Per the issue's scope ("No UI yet — UI lands in P2-05"), there's nothing for stadium-tester to drive. The mock-mode preview short-circuits client API calls; this PR doesn't add client API calls. UI verification will run on P2-05 when NotificationsCard.tsx and the focus-refetch wiring land.

The issue's 5 ## Test scenarios are server endpoint behaviour. Each is mapped to a Vitest assertion below:

# Issue scenario Vitest coverage
1 GET /api/wallet-contacts/<unknown>200 { email_set: false, notifications_enabled: true } wallet-contact.controller.test.js — "GET unknown wallet returns default shape"
2 PUT /api/wallet-contacts/<address> matching signer + valid email → 200, GET reflects email_set: true wallet-contact.controller.test.js — "PUT happy path" + wallet-contact.repository.test.jsupsertByWallet round-trip
3 PUT signed by mismatched wallet → 403 wallet-contact.middleware.test.js — "signer mismatch returns 403 with reason: not-your-wallet"
4 PUT with malformed email → 400 (impl returns 422 per the project's field-validation convention; matches program.controller.js) wallet-contact.controller.test.js — "PUT with malformed email returns 422"
5 GET response body never contains email key wallet-contact.controller.test.jsexpect(body.data).not.toHaveProperty('email') on both empty + populated cases

Manual QA from the issue (real-wallet smoke) is for the human reviewer to walk before merge. Runs against local dev with a real wallet — outside the agent's scope per workflow.

Backlog items logged

Appended to docs/improvement-backlog.md:

  • requireTeamMemberOrAdmin lacks the same SIWS domain check that requireAdmin (and now requireOwnWallet) enforce. Pre-existing inconsistency, separate hardening pass.

Invariants verified

  • BYPASS_ADMIN_CHECK still false (untouched).
  • All admin/auth-protected routes still use middleware from auth.middleware.js — new PUT route uses requireOwnWallet declared there.
  • No new console.log/warn/error in client (no client changes).
  • No new Mongoose imports under server/api/**.
  • Server is ESM throughout.
  • No changes to existing Phase 1 schemas — purely additive migration.
  • Public GET response object never contains the email key — enforced at the service boundary, asserted in controller tests.

Adds the wallet_contacts table (migration), repository, service, controller,
and routes. GET /:address is public and returns only { email_set, notifications_enabled }.
PUT /:address is guarded by requireOwnWallet (new SIWS middleware). validateEmail
added to server/api/utils/validation.js. 85 tests passing.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

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

Project Deployment Actions Updated (UTC)
stadium Ready Ready Preview, Comment Apr 30, 2026 1:12am

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.

revamp-P2-01: wallet_contacts — data model + API

1 participant