fix(services): prevent incorrect token decimals fallback causing amount calculation errors#1712
fix(services): prevent incorrect token decimals fallback causing amount calculation errors#1712
Conversation
…nt calculation errors Previously, when Alchemy RPC failed to fetch token decimals, the code silently returned 18 decimals as a default. This caused severe calculation errors for tokens with different decimals (e.g., USDC with 6 decimals would have amounts off by 10^12). Root cause: The Alchemy provider's getTokenDecimals returned 18 without throwing when RPC returned empty data, meaning retries never triggered and the calling code incorrectly assumed it received valid decimals. Changes: - alchemy-rpc.provider.ts: Throw error on invalid RPC response to trigger retries and propagate failure to caller instead of silently returning 18 - config-utils.ts: Add KNOWN_TOKEN_DECIMALS map with verified decimals for all tokens in specificChainTokens (USDC=6, USDT=6, WETH=18, etc.) - config-utils.ts: Fix incorrect Base USDT address (was DAI with 18 decimals) - rpc-spot.provider.ts: Use KNOWN_TOKEN_DECIMALS as fallback when RPC fails, reject swap for unknown tokens instead of guessing 18 - rpc-spot-integration.test.ts: Add test verifying all 25 token decimals against on-chain data via Alchemy RPC The new flow: 1. Try RPC with retries (existing circuit breaker + retry logic) 2. If fails → use KNOWN_TOKEN_DECIMALS lookup 3. If unknown token → reject the swap (safer than guessing) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📊 Test Coverage Report
|
There was a problem hiding this comment.
Pull request overview
This PR hardens token-decimal handling to prevent incorrect 18-decimal fallbacks from corrupting swap amount calculations, especially for tokens like USDC/USDT with 6 decimals. It changes the Alchemy RPC provider to fail loudly on invalid decimals responses and introduces a curated, tested map of known token decimals that RpcSpotProvider uses as a safe fallback before ultimately rejecting ambiguous swaps.
Changes:
- Updated
AlchemyRpcProvider.getTokenDecimalsto throw on empty/invalid RPC results, leveraging retries and propagating failures instead of silently defaulting to 18 decimals. - Introduced
TOKEN_DECIMALS_BY_TYPE,KNOWN_TOKEN_DECIMALS, andgetKnownTokenDecimalsderived fromspecificChainTokens, plus fixed the Base USDT address to the correct bridged USDT contract. - Updated
RpcSpotProviderswap detection and receipt-based amount calculation to useKNOWN_TOKEN_DECIMALSafter RPC failures and to reject swaps when decimals cannot be reliably determined, with new integration and provider tests to validate behavior and known-decimal correctness.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/services/src/providers/spot-live/rpc-spot.provider.ts |
Adds KNOWN_TOKEN_DECIMALS-based fallback and rejection logic in receipt-based amount calculation and swap detection, ensuring swaps are only accepted when token decimals are known. |
packages/services/src/providers/spot-live/alchemy-rpc.provider.ts |
Changes getTokenDecimals to treat empty/invalid RPC responses as errors (with retries and structured logging) and rethrow, removing the unsafe 18-decimal default. |
packages/services/src/providers/__tests__/rpc-spot-integration.test.ts |
Adds an integration test (skipped without API key) to verify all entries in KNOWN_TOKEN_DECIMALS against on-chain decimals, ensuring the hardcoded map stays accurate over time. |
packages/services/src/providers/__tests__/alchemy-rpc.provider.test.ts |
Adjusts decimals tests to match new behavior, including asserting that native ETH (zero address) now causes getTokenDecimals to throw as it is not an ERC20. |
packages/services/src/lib/config-utils.ts |
Fixes the Base USDT address, defines TOKEN_DECIMALS_BY_TYPE, builds KNOWN_TOKEN_DECIMALS from specificChainTokens, and exposes getKnownTokenDecimals for safe, centralized decimal fallbacks. |
| const fromDecimals = | ||
| tokenDecimals.get(outbound.tokenAddress) ?? | ||
| getKnownTokenDecimals(outbound.tokenAddress); | ||
| const toDecimals = | ||
| tokenDecimals.get(inbound.tokenAddress) ?? | ||
| getKnownTokenDecimals(inbound.tokenAddress); | ||
|
|
||
| // Reject swap if we can't determine decimals for either token | ||
| // This prevents incorrect amount calculations (e.g., USDC with 6 decimals | ||
| // being treated as 18 would be off by 10^12) | ||
| if (fromDecimals === undefined || toDecimals === undefined) { | ||
| this.logger.warn( | ||
| { | ||
| txHash: receipt.transactionHash, | ||
| fromToken: outbound.tokenAddress, | ||
| toToken: inbound.tokenAddress, | ||
| hasFromDecimals: fromDecimals !== undefined, | ||
| hasToDecimals: toDecimals !== undefined, | ||
| }, | ||
| "[RpcSpotProvider] Cannot determine decimals for swap tokens - rejecting", | ||
| ); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The updated detectSwapFromReceiptLogs logic now rejects swaps when decimals cannot be determined for either token (falling back to getKnownTokenDecimals before giving up), but there are no tests asserting this rejection behavior. To guard against regressions in this safety check, it would be helpful to add a unit test that constructs a receipt with ERC20 transfers for tokens absent from tokenDecimals and KNOWN_TOKEN_DECIMALS, and verifies that the method returns null and emits the expected warning log (optional but recommended).
There was a problem hiding this comment.
Thanks for the suggestion! I've added a unit test in commit 501d699 that verifies the rejection behavior when decimals cannot be determined for unknown tokens.
The test:
- Sets up transfers with unknown tokens (addresses not in
KNOWN_TOKEN_DECIMALS) - Mocks
getTokenDecimalsto fail (simulating RPC failure) - Verifies the swap is rejected (returns 0 trades)
- Verifies the warning is logged
I also fixed the rejection logic in rpc-spot.provider.ts - previously when calculateAmountFromReceiptLog returned null, the code would silently continue with an incorrect amount. Now it properly rejects the swap with a warning log.
Add unit test to verify that swaps are rejected when decimals cannot be determined for unknown tokens (RPC fails and token not in KNOWN_TOKEN_DECIMALS). Also fix the rejection logic in rpc-spot.provider to actually reject swaps when calculateAmountFromReceiptLog returns null, instead of silently continuing with an incorrect amount calculated using 0 decimals. This ensures the full fallback chain works correctly: 1. Try RPC with retries 2. Fall back to KNOWN_TOKEN_DECIMALS 3. Reject swap if both fail (prevents incorrect amounts) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| export function getKnownTokenDecimals( | ||
| tokenAddress: string, | ||
| ): number | undefined { | ||
| return KNOWN_TOKEN_DECIMALS.get(tokenAddress.toLowerCase()); | ||
| } |
There was a problem hiding this comment.
This introduces a second exported getKnownTokenDecimals helper (the other lives in lib/blockchain-math-utils.ts and is keyed by token symbol), but the two functions have different input semantics (address vs symbol). Having two identically named helpers with different expectations is likely to cause confusion or incorrect imports in the future; consider renaming this address-based variant (e.g., getKnownTokenDecimalsByAddress) or consolidating known-decimals logic into a single shared utility.
| const knownDecimals = getKnownTokenDecimals(tokenAddress); | ||
| if (knownDecimals !== undefined) { | ||
| this.logger.warn( | ||
| { tokenAddress, chain, knownDecimals, error }, | ||
| "[RpcSpotProvider] RPC decimals fetch failed, using known token decimals", | ||
| ); | ||
| return parseFloat( | ||
| formatTokenAmount(rawValue.toString(), knownDecimals), | ||
| ); |
There was a problem hiding this comment.
The new fallback path that uses getKnownTokenDecimals(tokenAddress) when rpcProvider.getTokenDecimals throws does not appear to be covered by tests: existing tests either let getTokenDecimals succeed or, in the new "decimals fallback and rejection" case, make it fail for an unknown token so the fallback isn’t exercised. Given this branch is the safety net for transient RPC failures on known tokens, consider adding a unit test that simulates getTokenDecimals rejecting for a token present in KNOWN_TOKEN_DECIMALS and asserts that the amount is computed via the fallback and an appropriate warning is logged.
Summary
Fixes a critical bug where failed Alchemy RPC calls would silently return 18 decimals as default, causing severe calculation errors for tokens with different decimals (e.g., USDC with 6 decimals would have amounts off by 10^12).
Original Incident
Competition:
a4d9fae6-e169-47c7-a261-eb9a397a6143Agent:
grok 4 vision(8359ab64-2f9c-4db0-9d02-16d6d2b54b32)Transaction:
0x5a817fb0e6bfa035638f94297368f679066378ef9ee77b50542fdb0c7a3b08caTimestamp: 2026-01-16 15:16:03 UTC
On-Chain Reality (from Basescan)
What Our System Recorded
Impact
0.036 WETH ($120) while only debiting ~0 USDCRoot Cause
The Alchemy provider
getTokenDecimalsreturned 18 without throwing when RPC returned empty data:119198739divided by10^18=0.000000000119198739(should be10^6=119.198739)The Bug Path
Changes
alchemy-rpc.provider.tsconfig-utils.tsKNOWN_TOKEN_DECIMALSmap derived fromspecificChainTokensrpc-spot.provider.tsKNOWN_TOKEN_DECIMALS→ reject swaprpc-spot-integration.test.tsalchemy-rpc.provider.test.tsThe New Flow
Test plan
Database Investigation
To find potentially affected trades in the database:
🤖 Generated with Claude Code