Skip to content

[#88] Replace recoverMessageAddress with verifyMessage#96

Merged
realproject7 merged 2 commits intomainfrom
task/88-verify-message
Mar 15, 2026
Merged

[#88] Replace recoverMessageAddress with verifyMessage#96
realproject7 merged 2 commits intomainfrom
task/88-verify-message

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

Fixes #88

  • Replaced recoverMessageAddress() with publicClient.verifyMessage() in src/app/api/ratings/route.ts
  • verifyMessage handles both EOA and EIP-1271 contract wallet signatures, enabling Smart contract wallet users (Safe, Argent, etc.) to submit ratings
  • Added address field to request body so the caller provides their address for verification

Checklist

  • Replace recoverMessageAddress with verifyMessage
  • npm run lint passes
  • npm run typecheck passes

Test plan

  • Submit a rating with an EOA wallet — signature should verify correctly
  • Submit a rating with a contract wallet (Safe) — should now succeed instead of failing
  • Submit with invalid signature — should return "Invalid signature" error

🤖 Generated with Claude Code

…allet support

Uses publicClient.verifyMessage() which handles both EOA and EIP-1271
contract wallet signatures, enabling Smart contract wallet users (Safe,
Argent, etc.) to submit ratings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Summary

The signature verification change is directionally correct, but this PR introduces a request-shape regression that makes rating submissions fail from the current UI.

Findings

  • [high] The API now requires address, but the existing rating client does not send it, so every browser submission will hit Missing address, signature, or message.
    • File: src/app/api/ratings/route.ts:68
    • Suggestion: Either include address in the POST body from src/components/RatingWidget.tsx:120, or preserve backward-compatible verification behavior in the API while updating the client in the same PR.

Decision

Requesting changes because the current diff breaks the only in-repo caller for /api/ratings, so issue #88 is not fully fixed yet.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

T2b Review: APPROVE

Change Summary

Single file changed: src/app/api/ratings/route.ts (+12 / -7). Replaces recoverMessageAddress() with publicClient.verifyMessage() to support EIP-1271 contract wallet signatures, and adds a caller-supplied address field to the request body.

Review Against Criteria

1. Correctly replaces recoverMessageAddress with publicClient.verifyMessage?
Yes. The recoverMessageAddress import is removed. publicClient.verifyMessage({ address, message, signature }) is used correctly, handling both EOA (ecrecover) and EIP-1271 (on-chain isValidSignature). The publicClient is a proper viem PublicClient connected to Base/Base Sepolia.

2. Contract addresses imported (not hardcoded)?
No hardcoded addresses in this diff. Token address comes from the database; rater address from the request body. Clean.

3. Security issues?
None blocking. The address is now caller-supplied, but verifyMessage still cryptographically proves the claimed address produced the signature (EOA) or that the contract confirms it (EIP-1271). The signed message format still binds to a specific action, preventing replay.

4. Minimal and focused?
Yes. One file, surgical change. Branch naming and PR body follow conventions.

Nits (non-blocking)

# Severity Description
1 Nit No isAddress() validation on the caller-supplied address before passing to verifyMessage. Fails safely (catch block returns 400) but gives an opaque "Failed to verify signature" error instead of "Invalid address format". Nice-to-have for DX.

LGTM — correct EIP-1271 migration, security model preserved, minimal diff.

Fixes client-side regression: the caller must now send the wallet
address so the API can use verifyMessage for EIP-1271 support.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

T2b Review — APPROVE

Correct migration from recoverMessageAddress() to publicClient.verifyMessage() — supports EIP-1271 contract wallet signatures (Safe, Argent, etc.).

  • No hardcoded addresses
  • Minimal, focused change (+12/-7, single file)
  • No security issues with the verification logic

One non-blocking nit: no isAddress() pre-validation on the caller-supplied address field — fails safely but gives an opaque error. Fine to address later.

LGTM ✓

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: APPROVE

Summary

The PR now correctly switches rating signature verification to publicClient.verifyMessage() and updates the existing client to send the claimed address, so the EIP-1271 fix is complete without breaking the current submission flow.

Findings

  • No blocking findings.

Decision

Approving because the change now matches issue #88, stays minimal, and the refreshed lint/typecheck run passed.

@realproject7 realproject7 merged commit 45ecd47 into main Mar 15, 2026
1 check passed
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.

[Bug] Rating API uses recoverMessageAddress instead of verifyMessage

2 participants