fix: normalize wallet addresses to lowercase for case-insensitive comparison in attendance functions#21
Conversation
…parison in attendance functions
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
autopilot-project-manager
bot
changed the title
There was a problem hiding this comment.
Pull request overview
Normalizes Ethereum wallet addresses to lowercase within the public attendance “take” server actions to make signature/address checks and attendant lookups case-insensitive.
Changes:
- Normalize wallet addresses to lowercase before querying
attendant.walletAddressand before persisting newly-registered wallet addresses. - Normalize
verifyMessage(...)output before comparison to avoid checksum-case mismatches. - Add additional server-side logging and switch attendance-record inserts to use
returning(...)for inserted IDs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const insertResult = await db | ||
| .insert(attendanceRecord) | ||
| .values({ | ||
| attendanceRoomId: roomId, | ||
| attendantId: userInfo.id, | ||
| }) | ||
| .returning({ id: attendanceRecord.id }); | ||
|
|
||
| console.log("[takeAttendance] New attendant registered", { | ||
| attendantId: userInfo.id, | ||
| roomId, | ||
| insertedRecordId: insertResult[0]?.id, | ||
| }); | ||
|
|
||
| if (!insertResult.length) { | ||
| return { error: "Failed to save attendance record" }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Both attendance-record inserts now rely on .returning(...) and treat an empty returned array as a failure. If the underlying libsql/SQLite connection doesn’t return rows for RETURNING (or behavior changes), this will report an error even though the insert may have succeeded, which can lead to confusing UX and potential duplicate records on retry. Prefer treating “no throw” as success, or use an API that returns rowsAffected for success checks, and keep returning only for logging/observability.
| const insertResult = await db | ||
| .insert(attendanceRecord) | ||
| .values({ | ||
| attendanceRoomId: roomId, | ||
| attendantId: foundAttendant.id, | ||
| }); | ||
| }) | ||
| .returning({ id: attendanceRecord.id }); | ||
|
|
||
| return { error: null }; | ||
| console.log("[takeAttendance] Returning attendant", { | ||
| attendantId: foundAttendant.id, | ||
| roomId, | ||
| insertedRecordId: insertResult[0]?.id, | ||
| }); | ||
|
|
||
| if (!insertResult.length) { | ||
| return { error: "Failed to save attendance record" }; | ||
| } |
There was a problem hiding this comment.
The second attendance-record insert uses the same returning + empty-array failure check as above, with the same risk of reporting failure after a successful insert. Consider using a success condition based on thrown errors / rowsAffected, and keep returning optional.
| @@ -82,7 +83,7 @@ export async function getAttendantByWalletAddress( | |||
|
|
|||
| data = await db.query.attendant.findFirst({ | |||
| where: and( | |||
| eq(attendant.walletAddress, walletAddress), | |||
| eq(attendant.walletAddress, normalizedAddress), | |||
| eq(attendant.admin, room.createdBy) | |||
| ), | |||
| columns: { | |||
| @@ -95,7 +96,7 @@ export async function getAttendantByWalletAddress( | |||
| }); | |||
| } else { | |||
| data = await db.query.attendant.findFirst({ | |||
| where: eq(attendant.walletAddress, walletAddress), | |||
| where: eq(attendant.walletAddress, normalizedAddress), | |||
| columns: { | |||
There was a problem hiding this comment.
getAttendantByWalletAddress* now lowercases the input and compares it directly to attendant.walletAddress. This only becomes truly case-insensitive if the DB values are also guaranteed to be lowercase; otherwise existing attendants (or ones created/updated through other flows that don’t normalize) won’t be found. Consider either normalizing wallet addresses on every write (create/update/import) + backfilling existing rows, or changing the query to compare using SQL lower(wallet_address) so mixed-case stored values still match.
| expected: nonce, | ||
| received: previousNonce, |
There was a problem hiding this comment.
The nonce mismatch path logs both the expected nonce from Redis and the received nonce. Since the nonce is effectively a one-time authentication secret (and here appears shared per room), logging it can weaken the threat model if logs are accessible. Suggest removing the nonce values from logs (or only logging that a mismatch occurred, optionally with a request/room identifier) and keeping detailed values behind an explicit debug flag.
| expected: nonce, | |
| received: previousNonce, |
…parison in attendance functions