-
Notifications
You must be signed in to change notification settings - Fork 1
feat: support ankr as a balance provider #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR integrates Ankr multichain RPC API for fetching token balances. It introduces a new datasource module with Ankr API configuration, a TokenBalancesRepositoryAnkr class implementing token balance retrieval via Ankr's JSON-RPC interface, and updates the factory to use the Ankr implementation instead of Alchemy. Includes error handling, request timeouts, and response validation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant TokenBalancesRepositoryAnkr
participant requestBalanceData
participant AnkrAPI
participant ResponseValidator
Client->>TokenBalancesRepositoryAnkr: getTokenBalances({address, chainId})
TokenBalancesRepositoryAnkr->>TokenBalancesRepositoryAnkr: Validate chain support & map to blockchain ID
TokenBalancesRepositoryAnkr->>requestBalanceData: Call requestBalanceData(address, blockchain)
requestBalanceData->>requestBalanceData: Create AbortController + timeout
requestBalanceData->>AnkrAPI: POST JSON-RPC request (getAccountBalance)
AnkrAPI-->>requestBalanceData: Return response
requestBalanceData-->>TokenBalancesRepositoryAnkr: Return Response
TokenBalancesRepositoryAnkr->>ResponseValidator: Validate response structure
ResponseValidator-->>TokenBalancesRepositoryAnkr: Validation result
TokenBalancesRepositoryAnkr->>TokenBalancesRepositoryAnkr: Aggregate balances by token address
TokenBalancesRepositoryAnkr-->>Client: Return TokenBalancesResponse (Map<address, balance>)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Key areas requiring attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
libs/repositories/src/repos/TokenBalancesRepository/TokenBalancesRepositoryAnkr.ts (3)
63-66: Consider including chainId in error message for debugging.When the chain is unsupported, the error message doesn't indicate which chain was requested, making debugging harder.
if (!blockchain) { - throw new Error('Unsupported chain'); + throw new Error(`Unsupported chain: ${chainId}`); }
81-106: Minor: Redundant zero check.Line 90 checks
balance !== '0'and line 93 checksbalanceBigInt !== 0n. The second check is sufficient sinceBigInt('0') === 0n.const balance = asset.balanceRawInteger; - if (balance && balance !== '0') { + if (balance) { try { const balanceBigInt = BigInt(balance); if (balanceBigInt !== 0n) {
109-143: Timeout abort throws generic error - consider explicit timeout handling.When the request times out,
controller.abort()causes fetch to throw anAbortError, which will propagate as-is. Wrapping this in a timeout-specific error would improve debugging.try { const response = await fetch(apiUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(requestBody), signal: controller.signal, }); return response; + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + throw new Error(`Ankr API request timed out after ${REQUEST_TIMEOUT_MS}ms`); + } + throw error; } finally { clearTimeout(timeoutId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/repositories/src/datasources/ankr.ts(1 hunks)libs/repositories/src/index.ts(1 hunks)libs/repositories/src/repos/TokenBalancesRepository/TokenBalancesRepositoryAnkr.ts(1 hunks)libs/services/src/factories.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/repositories/src/repos/TokenBalancesRepository/TokenBalancesRepositoryAnkr.ts (3)
libs/repositories/src/repos/TokenBalancesRepository/TokenBalancesRepository.ts (3)
TokenBalancesRepository(12-14)TokenBalanceParams(5-8)TokenBalancesResponse(10-10)libs/repositories/src/datasources/ankr.ts (3)
ANKR_CLIENT_NETWORK_MAPPING(10-25)getAnkrApiUrl(27-32)ANKR_API_KEY(3-3)libs/shared/src/logger.ts (1)
logger(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: docker (apps/notification-producer)
- GitHub Check: docker (apps/twap)
- GitHub Check: docker (apps/telegram)
- GitHub Check: docker (apps/api)
🔇 Additional comments (4)
libs/repositories/src/datasources/ankr.ts (1)
1-33: LGTM!The Ankr datasource configuration is well-structured with clear network mapping and a straightforward URL builder. The use of
nullfor unsupported chains (LENS, PLASMA) allows for explicit handling in the repository layer.libs/services/src/factories.ts (1)
150-152: LGTM!Clean swap from Alchemy to Ankr implementation. The factory pattern allows for this seamless provider change without affecting consumers.
libs/repositories/src/index.ts (1)
49-49: LGTM!Export follows the established pattern for TokenBalancesRepository implementations.
libs/repositories/src/repos/TokenBalancesRepository/TokenBalancesRepositoryAnkr.ts (1)
36-55: LGTM!The type guard provides solid runtime validation of the API response structure before processing.
| type AnkrGetAccountBalanceResponse = { | ||
| jsonrpc: string; | ||
| id: number; | ||
| result: { | ||
| totalBalanceUsd: string; | ||
| assets: AnkrAsset[]; | ||
| nextPageToken?: string; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Ankr getAccountBalance API documentation pagination nextPageToken
💡 Result:
Short answer — getAccountBalance supports cursor pagination: include pageSize and pageToken in the request; the response includes result.nextPageToken which you pass as pageToken in the next request to fetch the next page. Use the *_raw / low-level endpoints or pass pageSize/pageToken yourself; some higher-level SDK helpers may auto-paginate for you. [1][2]
References
- Ankr Token API — ankr_getAccountBalance (shows pageSize, pageToken request fields and nextPageToken in result). [1]
- Ankr SDK docs / GitHub (notes that *_raw methods expose manual pagination via pageSize/pageToken; non-_raw methods may auto-paginate). [2]
Pagination not handled — may return incomplete results.
The AnkrGetAccountBalanceResponse type includes nextPageToken indicating the API supports pagination, but getTokenBalances only processes the first page of results. Wallets with many token holdings may receive incomplete balance data.
Consider implementing pagination loop:
+ private async fetchAllBalances(
+ address: string,
+ blockchain: string
+ ): Promise<AnkrAsset[]> {
+ const allAssets: AnkrAsset[] = [];
+ let pageToken: string | undefined;
+
+ do {
+ const response = await this.requestBalanceData(address, blockchain, pageToken);
+ if (!response.ok) {
+ throw new Error(`Ankr API error: ${response.status} ${response.statusText}`);
+ }
+ const asJson = await response.json();
+ if (!isAnkrGetAccountBalanceResponse(asJson)) {
+ throw new Error('Invalid Ankr response');
+ }
+ allAssets.push(...asJson.result.assets);
+ pageToken = asJson.result.nextPageToken;
+ } while (pageToken);
+
+ return allAssets;
+ }Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.