chore: code cleanup and formatting for middleware files #18
Conversation
WalkthroughType annotations were tightened in auth middleware for wallet signature verification. Rate limiting middleware received minor formatting/comment updates and a slightly safer walletAddress extraction in the key generator. No control flow or exported API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/middleware/auth.ts (1)
27-27: Avoid asserting a concrete body shape; either keep it unknown or type the request BodyAsserting
{ walletAddress: string; message: string; signature: string }can mislead TS since the values can still beundefinedat runtime. Two safer options:
- Minimal: destructure from a generic record and keep your existing runtime typeof checks.
- Better: type the Fastify request Body so the middleware is sound at compile time.
Minimal change (within this line):
-const { walletAddress, message, signature } = request.body as { walletAddress: string; message: string; signature: string }; +const { walletAddress, message, signature } = request.body as Record<string, unknown>;Stronger typing (outside this line; adjust the function signature):
export async function verifyWalletSignature( - request: FastifyRequest, + request: FastifyRequest<{ Body: { walletAddress: string; message: string; signature: string } }>, reply: FastifyReply ) {src/middleware/rateLimit.ts (4)
54-61: Type the rate‑limit context instead of anyUse a minimal structural type to document usage (or import the plugin’s exported context type if available).
-export const rateLimitErrorHandler = (request: FastifyRequest, context: any) => { +export const rateLimitErrorHandler = (request: FastifyRequest, context: { ttl: number }) => {If the plugin exports a dedicated type, prefer it:
// import type { RateLimitContext } from '@fastify/rate-limit'; // export const rateLimitErrorHandler = (_req: FastifyRequest, context: RateLimitContext) => { ... }If you want me to confirm the exact exported type from @fastify/rate-limit, I can look it up.
4-7: Fix duplicated docblock openingsThere’s a repeated
/**line inside each block. Clean these up for clarity.-/** -/** +/**Also applies to: 50-53, 63-66, 82-85, 104-107
9-16: Safer env parsing for max values (radix + fallback)Current
parseInt(process.env.X || 'N')lacks a radix and will propagateNaNif X is set but invalid. Suggest a tiny helper and switch callers.Add once near the top:
const envInt = (name: string, fallback: number) => { const v = process.env[name]; if (v == null || v === '') return fallback; const n = Number.parseInt(v, 10); return Number.isNaN(n) ? fallback : n; };Then replace e.g.:
- max: parseInt(process.env.RATE_LIMIT_GENERAL_MAX || '100'), + max: envInt('RATE_LIMIT_GENERAL_MAX', 100),(Apply similarly to auth, apiKeyCreation, registration, health.)
Also applies to: 17-24, 25-32, 33-40, 41-48
112-115: Minor: drop unnecessary return
preHandlercan justawaitand fall through;return;is redundant.- // Custom logic can be added here if needed - return; + // Custom logic can be added here if needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/middleware/auth.ts(1 hunks)src/middleware/rateLimit.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/middleware/auth.ts (1)
src/utils/verifySignature.ts (1)
verifySignature(10-25)
src/middleware/rateLimit.ts (1)
test/utils/verifySignature.test.ts (1)
wallet(169-196)
🔇 Additional comments (1)
src/middleware/rateLimit.ts (1)
88-101: Formatting/trailing commas: LGTMConsistent trailing commas and minor formatting improve diffs and readability. No behavior changes.
| if (request.body && typeof request.body === 'object' && 'walletAddress' in request.body) { | ||
| walletAddress = (request.body as any).walletAddress; | ||
| walletAddress = (request.body as { walletAddress?: string }).walletAddress; | ||
| } | ||
| if (!walletAddress && request.headers['x-wallet-address']) { | ||
| walletAddress = String(request.headers['x-wallet-address']); | ||
| } | ||
| // If wallet address is available, use IP + wallet for more granular control | ||
| if (walletAddress) { | ||
| return `${ip}:${walletAddress.toLowerCase()}`; | ||
| } | ||
| // Fallback to just IP | ||
| return ip; |
There was a problem hiding this comment.
Guard walletAddress before calling toLowerCase to prevent runtime crashes
walletAddress from the body can be non‑string (e.g., number/array), which would throw on .toLowerCase(). Add a type check (and optionally trim) before usage. Also gate the final branch on a string.
let walletAddress: string | undefined;
if (request.body && typeof request.body === 'object' && 'walletAddress' in request.body) {
- walletAddress = (request.body as { walletAddress?: string }).walletAddress;
+ const raw = (request.body as { walletAddress?: unknown }).walletAddress;
+ if (typeof raw === 'string' && raw.trim() !== '') {
+ walletAddress = raw;
+ }
}
if (!walletAddress && request.headers['x-wallet-address']) {
walletAddress = String(request.headers['x-wallet-address']);
}
- if (walletAddress) {
+ if (typeof walletAddress === 'string' && walletAddress) {
return `${ip}:${walletAddress.toLowerCase()}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (request.body && typeof request.body === 'object' && 'walletAddress' in request.body) { | |
| walletAddress = (request.body as any).walletAddress; | |
| walletAddress = (request.body as { walletAddress?: string }).walletAddress; | |
| } | |
| if (!walletAddress && request.headers['x-wallet-address']) { | |
| walletAddress = String(request.headers['x-wallet-address']); | |
| } | |
| // If wallet address is available, use IP + wallet for more granular control | |
| if (walletAddress) { | |
| return `${ip}:${walletAddress.toLowerCase()}`; | |
| } | |
| // Fallback to just IP | |
| return ip; | |
| if (request.body && typeof request.body === 'object' && 'walletAddress' in request.body) { | |
| const raw = (request.body as { walletAddress?: unknown }).walletAddress; | |
| if (typeof raw === 'string' && raw.trim() !== '') { | |
| walletAddress = raw; | |
| } | |
| } | |
| if (!walletAddress && request.headers['x-wallet-address']) { | |
| walletAddress = String(request.headers['x-wallet-address']); | |
| } | |
| if (typeof walletAddress === 'string' && walletAddress) { | |
| return `${ip}:${walletAddress.toLowerCase()}`; | |
| } | |
| return ip; |
🤖 Prompt for AI Agents
In src/middleware/rateLimit.ts around lines 70 to 79, the code calls
walletAddress.toLowerCase() without ensuring walletAddress is a string; if the
body/header contains a non-string (number, array, object) this will throw. Fix
by checking typeof walletAddress === 'string' (and optionally trimming) before
calling toLowerCase; when reading from request.headers coerce to string only
after verifying it's a string-like value or use String(...) and then guard that
the resulting value is a non-empty string before lowercasing; if walletAddress
is not a valid string, fall back to returning ip.
Summary
This pull request performs code cleanup and formatting improvements for the following middleware files:
rateLimit.tsauth.tsChanges
Removed duplicate imports and unused code.
Fixed malformed block comments for clarity and documentation consistency.
Improved type usage and formatting for better readability and maintainability.
Ensured both files follow consistent code style and best practices.
Summary by CodeRabbit
No user-facing functionality changed; behavior and public interfaces remain the same.