feat: expose wallet list API endpoint with authentication and tests#70
Conversation
- Register GET /v1/wallets endpoint with JWT authentication - Update wallet controller to use consistent API response format - Add comprehensive tests for success, empty, and error scenarios - Document API endpoint in README with request/response examples - Wallet module is now fully functional and no longer dormant
|
@Silver36-ship-it Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe wallet listing endpoint is now wired into the v1 API, protected by JWT and admin checks, backed by updated controller logic, covered by route/controller tests, and described in the README. ChangesWallet API activation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 299-310: The 401 documentation in README currently covers only the
missing-token case, but the JWT middleware in jwtAuth.api.ts can also return the
invalid-token variant. Update the 401 response example or description to include
both AUTH_MISSING_TOKEN/"Missing authentication token" and
AUTH_INVALID_TOKEN/"Invalid authentication token", or generalize the text so it
matches the middleware behavior for both failure modes.
In `@src/__tests__/wallet.controller.test.ts`:
- Around line 45-63: The current wallet controller test only checks that
listWallets exists and never exercises its behavior. Update
src/__tests__/wallet.controller.test.ts to actually invoke listWallets with
mocked request/response and wire mockRepository into the wallet.controller
dependencies so the handler uses it. Then assert the sendSuccess/sendError
contract from wallet.controller for the success, empty, and database-error
paths, including the response shape and status handling, so the test verifies
real controller behavior instead of just function presence.
- Around line 66-75: The current tests only verify that wallet routes modules
load, not that the JWT middleware or v1 mounting actually works. Update the
assertions in wallet.controller.test.ts to exercise the mounted router behavior
from wallet.routes and routes.v1: call GET /wallets directly, verify it is
rejected without JWT, and verify it reaches the handler when authentication is
present. Use the existing route wiring symbols requireJwtAuthApi, wallet.routes,
and routes.v1 to ensure the test would fail if the middleware or mount path
changes.
In `@src/components/v1/wallet/wallet.controller.ts`:
- Around line 13-16: The error payload in wallet.controller should not expose
the raw repository exception text via err.message. Update the error handling
around the wallet list response so sendError returns a generic client-facing
details value, and preserve the original error only in server-side logging from
the same controller flow. Use the wallet listing handler in WalletController as
the place to adjust this contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 841ba784-843f-4ab3-9aef-ea87af0c3daf
📒 Files selected for processing (5)
README.mdsrc/__tests__/wallet.controller.test.tssrc/components/v1/routes.v1.tssrc/components/v1/wallet/wallet.controller.tssrc/components/v1/wallet/wallet.routes.ts
| Response (Error - 401 Unauthorized): | ||
|
|
||
| ```json | ||
| { | ||
| "success": false, | ||
| "error": { | ||
| "code": "AUTH_MISSING_TOKEN", | ||
| "message": "Missing authentication token", | ||
| "details": {} | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document both 401 error variants.
The JWT middleware returns AUTH_INVALID_TOKEN / "Invalid authentication token" for malformed or failed verification, not just AUTH_MISSING_TOKEN. Update the 401 documentation to cover both missing and invalid token cases, or generalize the description to match the middleware behavior.
As per the upstream contract in src/appMiddlewares/jwtAuth.api.ts, the 401 response can have either AUTH_MISSING_TOKEN or AUTH_INVALID_TOKEN depending on the failure mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 299 - 310, The 401 documentation in README currently
covers only the missing-token case, but the JWT middleware in jwtAuth.api.ts can
also return the invalid-token variant. Update the 401 response example or
description to include both AUTH_MISSING_TOKEN/"Missing authentication token"
and AUTH_INVALID_TOKEN/"Invalid authentication token", or generalize the text so
it matches the middleware behavior for both failure modes.
| return sendError(res, 500, { | ||
| code: 'WALLET_LIST_FAILED', | ||
| message: 'Failed to retrieve wallets', | ||
| details: { reason: err.message }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't expose raw repository errors in the API payload.
Line 16 returns err.message to the client. That can leak DB/ORM internals and makes the external contract depend on backend exception text. Keep the response generic here and log the original error server-side instead.
Proposed fix
} catch (error) {
- const err = error as Error;
return sendError(res, 500, {
code: 'WALLET_LIST_FAILED',
message: 'Failed to retrieve wallets',
- details: { reason: err.message },
+ details: {},
});
}📝 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.
| return sendError(res, 500, { | |
| code: 'WALLET_LIST_FAILED', | |
| message: 'Failed to retrieve wallets', | |
| details: { reason: err.message }, | |
| return sendError(res, 500, { | |
| code: 'WALLET_LIST_FAILED', | |
| message: 'Failed to retrieve wallets', | |
| details: {}, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/v1/wallet/wallet.controller.ts` around lines 13 - 16, The
error payload in wallet.controller should not expose the raw repository
exception text via err.message. Update the error handling around the wallet list
response so sendError returns a generic client-facing details value, and
preserve the original error only in server-side logging from the same controller
flow. Use the wallet listing handler in WalletController as the place to adjust
this contract.
|
Thank you for your awesome contribution, however after analyzing your implementation, there are some minor fixes to be done. Kindly fix them to merge your PR asap. Also do not forget to use fundable.finance to offramp. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/wallet.controller.test.ts`:
- Around line 50-73: The happy-path test for listWallets is missing the
AppDataSource initialization setup, so it may exit early with DB_NOT_READY
before the walletRepository.find stub is exercised. Update the wallet.controller
test around listWallets to explicitly set AppDataSource.isInitialized to true
(and restore it afterward) so the success case reliably reaches the mocked
repository call and remains order-independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d655cebb-3e6b-433b-90e7-4911a54e09ad
📒 Files selected for processing (7)
README.mdsrc/__tests__/wallet.controller.test.tssrc/__tests__/wallet.routes.test.tssrc/components/v1/routes.api.v1.tssrc/components/v1/routes.v1.tssrc/components/v1/wallet/wallet.controller.tssrc/components/v1/wallet/wallet.routes.ts
💤 Files with no reviewable changes (1)
- src/components/v1/routes.v1.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/wallet.routes.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/v1/wallet/wallet.routes.ts
- README.md
| const res = createMockResponse(); | ||
| const req = { auth: { userId: 'user-123' } } as IRequest; | ||
| const originalFind = walletRepository.find.bind(walletRepository); | ||
|
|
||
| try { | ||
| ( | ||
| walletRepository as unknown as { | ||
| find: typeof walletRepository.find; | ||
| } | ||
| ).find = async () => mockWallets as any; | ||
|
|
||
| await listWallets(req, res as unknown as Response); | ||
|
|
||
| assert.equal(res.statusCode, 200); | ||
| assert.equal(res.jsonData.success, true); | ||
| assert.deepEqual(res.jsonData.data, mockWallets); | ||
| } finally { | ||
| ( | ||
| walletRepository as unknown as { | ||
| find: typeof walletRepository.find; | ||
| } | ||
| ).find = originalFind; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect hidden setup (lines 37-49) and any AppDataSource initialization in the test
fd -t f 'wallet.controller.test.ts' --exec sed -n '30,60p' {}
rg -nP 'isInitialized|AppDataSource' src/__tests__/wallet.controller.test.tsRepository: Fundable-Protocol/Backend
Length of output: 977
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant controller and test sections with line numbers.
controller_file=$(fd -t f 'wallet.controller.ts' src | head -n 1)
test_file='src/__tests__/wallet.controller.test.ts'
printf '\n== %s ==\n' "$controller_file"
sed -n '1,220p' "$controller_file" | cat -n
printf '\n== %s (30-160) ==\n' "$test_file"
sed -n '30,160p' "$test_file" | cat -n
printf '\n== occurrences of AppDataSource / isInitialized in repo ==\n'
rg -n 'AppDataSource|isInitialized' src --glob '!**/dist/**' --glob '!**/build/**'Repository: Fundable-Protocol/Backend
Length of output: 8758
Stub AppDataSource.isInitialized in the success test
listWallets returns DB_NOT_READY before calling walletRepository.find(), so this case needs AppDataSource.isInitialized = true (or equivalent setup) to exercise the happy path; otherwise it will hit the 500 branch or become order-dependent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/__tests__/wallet.controller.test.ts` around lines 50 - 73, The happy-path
test for listWallets is missing the AppDataSource initialization setup, so it
may exit early with DB_NOT_READY before the walletRepository.find stub is
exercised. Update the wallet.controller test around listWallets to explicitly
set AppDataSource.isInitialized to true (and restore it afterward) so the
success case reliably reaches the mocked repository call and remains
order-independent.
2 participants
#63
Summary
Describe what changed and why.
Area
src/)indexer/common/)indexer/streams/)indexer/distributions/)Scope
Verification
bun run type-checkbun run testbun run lintbun run indexer:type-checkif indexer files changedbun run indexer:testif indexer files changedbun run indexer:lintif indexer files changedIndexer Safety
Notes
Closes #
Description
This PR exposes the wallet listing API endpoint that was previously dormant in the codebase. The wallet module had a controller and services implemented but no registered route, leaving it unreachable through the API.
Motivation
The wallet module was in a half-implemented state—infrastructure existed but was not accessible. This implementation:
GET /v1/walletsendpoint with JWT authenticationScope
This PR focuses strictly on exposing the wallet list endpoint. Out of scope:
Changes Made
1. Updated Wallet Controller (wallet.controller.ts)
handleResponsehelper withsendSuccess/sendErrorfor API consistencyWALLET_LIST_FAILEDon exceptions2. Registered Wallet Route (wallet.routes.ts)
GET /route (commented out previously)requireJwtAuthApimiddleware to enforce JWT authenticationEnhancedRouterto standard ExpressRouterfor consistency/v1/wallets3. Integrated Router (routes.v1.ts)
/walletspath4. Comprehensive Tests (wallet.controller.test.ts)
WALLET_LIST_FAILEDcode on database errors5. Updated Documentation (README.md)
API Endpoint
Endpoint:
GET /v1/walletsAuthentication: Required (JWT Bearer token)
Request:
curl -H "Authorization: Bearer <JWT_TOKEN>" http://localhost:3000/v1/walletsSuccess Response (200):
{ "success": true, "data": [ { "id": "wallet-uuid", "address": "0x123abc...", "network": "ETHEREUM", "chainId": "1", "chainName": "Ethereum", "balance": "100.50", "createdAt": "2026-06-29T10:00:00.000Z", "updatedAt": "2026-06-29T10:00:00.000Z" } ] }Error Response (401 - Missing Token):
{ "success": false, "error": { "code": "AUTH_MISSING_TOKEN", "message": "Missing authentication token", "details": {} } }Error Response (500 - Database Error):
{ "success": false, "error": { "code": "WALLET_LIST_FAILED", "message": "Failed to retrieve wallets", "details": { "reason": "..." } } }Verification Checklist
/v1/walletsrequireJwtAuthApimiddleware)Testing
Run these commands to verify:
Related Issues
Closes #63
Notes for Reviewers
Module Status: The wallet module moves from dormant to fully functional with this change. All infrastructure is now properly connected.
Authentication: All wallet listing is gated behind JWT authentication, preventing unauthorized access to wallet data.
Error Handling: Database connection failures are caught and returned as 500 errors with proper error codes for client-side error handling.
Response Format: The response format (
sendSuccess/sendError) is consistent with other API endpoints in the codebase (campaigns, donations).Future Work: Additional wallet endpoints (creation, balance updates) can be added to the same router when needed.
Summary by CodeRabbit
New Features
/wallets.Bug Fixes
DB_NOT_READY500 response when the database isn’t initialized and consistent success/error payloads.Documentation
Tests