feat: proxy getTokenDetails to fullnode thin_wallet/token API#372
Merged
andreabadesso merged 3 commits intomasterfrom Mar 3, 2026
Merged
feat: proxy getTokenDetails to fullnode thin_wallet/token API#372andreabadesso merged 3 commits intomasterfrom
andreabadesso merged 3 commits intomasterfrom
Conversation
Instead of querying the database for token info, supply, transactions and authorities, proxy the request to the fullnode's thin_wallet/token endpoint and map the response to preserve the existing API contract.
📝 WalkthroughWalkthroughToken details endpoint now proxies to the fullnode via fullnode.getTokenDetails(tokenId) instead of DB lookups; DB connection is closed in a finally block. Tests were updated to mock fullnode.getTokenDetails and to cover success and validation paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Wallet as WalletService
participant Fullnode as Fullnode
participant Thin as ThinWalletAPI
participant DB as DB
Client->>Wallet: GET /wallet/tokens/:token_id/details
Wallet->>Fullnode: getTokenDetails(token_id)
Fullnode->>Thin: GET thin_wallet/token?id=token_id
Thin-->>Fullnode: 200 { data: token details }
Fullnode-->>Wallet: token details
Wallet->>DB: closeDbConnection(mysql)
Wallet-->>Client: 200 { success:true, details: {...} }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wallet-service/src/api/tokens.ts`:
- Around line 68-90: Wrap the call to fullnode.getTokenDetails in a
try/catch/finally so closeDbConnection(mysql) is always invoked in the finally
block, and validate the response before returning success: check that data
exists and contains expected fields (e.g., data.name, data.symbol, data.total);
if the fullnode response is falsy or missing required fields or threw, return a
non-200 statusCode with success: false and an explanatory message instead of
returning success: true. Specifically update the code around
fullnode.getTokenDetails and closeDbConnection to ensure errors are caught,
logged/propagated, and DB closure always runs.
In `@packages/wallet-service/tests/api.test.ts`:
- Around line 1530-1563: The test fails to assert that the second proxied call
forwarded the correct token id, risking false positives; after calling
getTokenDetails with event made by makeGatewayEventWithAuthorizer (using
TX_IDS[1]) add an assertion that mockFullnodeResponse was called with TX_IDS[1]
(similar to the earlier
expect(mockFullnodeResponse).toHaveBeenCalledWith(TX_IDS[0])), referencing the
mockFullnodeResponse mock, the getTokenDetails invocation, and TX_IDS[1] to
ensure the forwarded argument is validated.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/wallet-service/src/api/tokens.tspackages/wallet-service/src/fullnode.tspackages/wallet-service/tests/api.test.ts
- Wrap fullnode call in try/finally to ensure DB connection is always closed - Check data.success before returning success response; return TOKEN_NOT_FOUND if fullnode reports failure - Assert second proxied token id with toHaveBeenNthCalledWith
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wallet-service/src/api/tokens.ts`:
- Around line 72-75: The token-not-found branches in
packages/wallet-service/src/api/tokens.ts call closeDbAndGetError (e.g., the if
(!data?.success) branch and the similar 98-100 branch) which duplicates DB
closing because a finally block later also closes mysql; change those branches
to return the error response without closing the DB (i.e., don’t call
closeDbAndGetError — return the equivalent ApiError payload directly or call a
non-closing helper) and let the existing finally block perform the single
mysql.close; update references to closeDbAndGetError in these branches so only
the finally closes the connection.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/wallet-service/src/api/tokens.tspackages/wallet-service/tests/api.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/wallet-service/tests/api.test.ts
Return error response directly instead of using closeDbAndGetError, since the finally block already handles closing the DB connection.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wallet-service/src/api/tokens.ts`:
- Around line 103-105: The get function currently opens a DB connection and
returns results but never closes it; mirror the pattern used in getTokenDetails
by adding a try/finally (or a finally block after the existing return) that
calls closeDbConnection(mysql) to always release the connection; locate the get
function in tokens.ts and ensure closeDbConnection(mysql) is invoked in a
finally block (same as in getTokenDetails) so the mysql connection is closed on
all execution paths.
Comment on lines
+103
to
+105
| } finally { | ||
| await closeDbConnection(mysql); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/wallet-service/src/api/tokens.ts | head -120Repository: HathorNetwork/hathor-wallet-service
Length of output: 2764
🏁 Script executed:
rg -n "closeDbConnection|closeDb" packages/wallet-service/src/api/ -C2Repository: HathorNetwork/hathor-wallet-service
Length of output: 50391
🏁 Script executed:
rg -n "getDbConnection" packages/wallet-service/src/ -A10Repository: HathorNetwork/hathor-wallet-service
Length of output: 47728
🏁 Script executed:
fd "connection|db" packages/wallet-service/src/ -t f | grep -E "\.(ts|js)$"Repository: HathorNetwork/hathor-wallet-service
Length of output: 123
🏁 Script executed:
# Let me check if there are any tests or documentation about how the connection pool handles closing
rg -i "serverless.*mysql" packages/wallet-service/src/ -A2 | head -40Repository: HathorNetwork/hathor-wallet-service
Length of output: 2861
🏁 Script executed:
# Check if closeDbConnection is called in error handlers
rg "closeDbConnection|closeDbAndGetError" packages/wallet-service/src/api/utils.ts -A3 -B3Repository: HathorNetwork/hathor-wallet-service
Length of output: 661
Add connection closing to the get function for consistency and resource management.
The get function (lines 26-38) should close the database connection like getTokenDetails does. Currently, get performs a database query but never calls closeDbConnection(mysql), while getTokenDetails properly closes the connection in a finally block. This inconsistency can lead to connection pool exhaustion and resource leaks on warm Lambda instances where the module-level connection persists across invocations. Add a finally block after the return statement to ensure the connection is properly released.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wallet-service/src/api/tokens.ts` around lines 103 - 105, The get
function currently opens a DB connection and returns results but never closes
it; mirror the pattern used in getTokenDetails by adding a try/finally (or a
finally block after the existing return) that calls closeDbConnection(mysql) to
always release the connection; locate the get function in tokens.ts and ensure
closeDbConnection(mysql) is invoked in a finally block (same as in
getTokenDetails) so the mysql connection is closed on all execution paths.
pedroferreira1
approved these changes
Mar 3, 2026
github-project-automation
Bot
moved this from In Review (WIP)
to Waiting to be deployed
in Hathor Network
This was referenced Mar 4, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of querying the database for token info, supply, transactions and authorities, proxy the request to the fullnode's
thin_wallet/tokenendpoint and map the response to preserve the existing API contract.Motivation
The
getTokenDetailsendpoint was querying multiple database tables (token info, total supply, total transactions, mint/melt authority UTXOs) to assemble the response. By proxying directly to the fullnode, we get real-time token data while keeping the same response format so existing wallets are unaffected.Acceptance Criteria
GET /wallet/tokens/{token_id}/detailsendpoint proxies to the fullnode'sthin_wallet/token?id={token_id}APIsuccess,details.tokenInfo,details.totalSupply,details.totalTransactions,details.authorities.mint,details.authorities.melt)Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets mergedSummary by CodeRabbit
Refactor
Bug Fixes
Tests