feat: add sol/btc tokens to price api#206
Conversation
📝 WalkthroughWalkthroughAddress validation regex broadened; CoinGecko platform mappings expanded; address normalization switched to Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UsdRepo as UsdRepository
participant Coingecko
participant SDK as Cow SDK
Client->>UsdRepo: request USD price (chain, tokenAddress?, platform)
UsdRepo->>SDK: getAddressKey(tokenAddress) / getSupportedCoingeckoChainId(chain)
SDK-->>UsdRepo: addressOrPlatform, chainId
alt chainId unsupported
UsdRepo-->>Client: null
else chainId supported
alt addressOrPlatform is platform
UsdRepo->>Coingecko: getMarketDataByPlatformId(platform)
else addressOrPlatform is address
UsdRepo->>Coingecko: getMarketDataByTokenAddress(contract_address=addressOrPlatform)
end
Coingecko-->>UsdRepo: market data / price
UsdRepo-->>Client: USD price or null
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
libs/repositories/src/datasources/coingecko.ts (1)
31-32: Add documentation for synthetic chain IDs.Lines 31–32 use undocumented magic numbers (1000000000, 1000000001) as synthetic chain IDs for non-EVM blockchains. A brief comment explaining their purpose and numbering scheme would improve maintainability.
Suggested documentation
export const COINGECKO_PLATFORMS: Record<number, string | undefined> = { ...SUPPORTED_COINGECKO_PLATFORMS, + // Synthetic chain IDs for non-EVM platforms (starting at 1 billion to avoid EVM collisions) [1000000000]: 'bitcoin', [1000000001]: 'solana',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/repositories/src/datasources/coingecko.ts` around lines 31 - 32, Add a brief comment above the synthetic chain ID mappings explaining that the numeric keys 1000000000 and 1000000001 are intentionally reserved synthetic IDs representing non-EVM chains (e.g., Bitcoin and Solana), describe the numbering scheme (large reserved range starting at 1_000_000_000 for non-EVM chains), and note that these IDs are not on-chain but used internally by the Coingecko mapping object (the entries with keys [1000000000] and [1000000001]); update the comment near the mapping in coingecko.ts so future maintainers understand their purpose and must not collide with real chain IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/repositories/src/datasources/coingecko.ts`:
- Around line 31-32: Add a brief comment above the synthetic chain ID mappings
explaining that the numeric keys 1000000000 and 1000000001 are intentionally
reserved synthetic IDs representing non-EVM chains (e.g., Bitcoin and Solana),
describe the numbering scheme (large reserved range starting at 1_000_000_000
for non-EVM chains), and note that these IDs are not on-chain but used
internally by the Coingecko mapping object (the entries with keys [1000000000]
and [1000000001]); update the comment near the mapping in coingecko.ts so future
maintainers understand their purpose and must not collide with real chain IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45809208-ed49-48c2-a9a4-c16168a02771
⛔ Files ignored due to path filters (1)
libs/repositories/src/gen/cow/cow-api-types.tsis excluded by!**/gen/**
📒 Files selected for processing (4)
apps/api/src/app/schemas.tslibs/repositories/src/datasources/coingecko.tslibs/repositories/src/repos/UsdRepository/UsdRepositoryCoingecko.tslibs/repositories/src/utils/coingeckoUtils.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/repositories/src/utils/coingeckoUtils.ts (1)
62-62: Outdated comment.The comment says "Only SupportedChainIds are supported" but the validation now also accepts
AdditionalTargetChainIdvalues.📝 Suggested comment update
- // Only SupportedChainIds are supported + // Only SupportedChainId and AdditionalTargetChainId values are supported🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/repositories/src/utils/coingeckoUtils.ts` at line 62, Update the outdated inline comment that says "Only SupportedChainIds are supported" to reflect that validation now accepts both SupportedChainIds and AdditionalTargetChainId values; locate the comment near the validation logic in coingeckoUtils.ts (references to SupportedChainIds and AdditionalTargetChainId) and replace it with a brief accurate note such as "Supports SupportedChainIds and AdditionalTargetChainId values" so the comment matches current behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/repositories/src/utils/coingeckoUtils.ts`:
- Line 62: Update the outdated inline comment that says "Only SupportedChainIds
are supported" to reflect that validation now accepts both SupportedChainIds and
AdditionalTargetChainId values; locate the comment near the validation logic in
coingeckoUtils.ts (references to SupportedChainIds and AdditionalTargetChainId)
and replace it with a brief accurate note such as "Supports SupportedChainIds
and AdditionalTargetChainId values" so the comment matches current behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c614a03-065b-45cc-88ee-45001c90eac5
📒 Files selected for processing (1)
libs/repositories/src/utils/coingeckoUtils.ts
| * | ||
| * @default false | ||
| */ | ||
| fullBalanceCheck: boolean; |
There was a problem hiding this comment.
it's automatically generated, looks like somebody forgot to build and commit it in previous pr
| * } | ||
| */ | ||
| pattern: '^(0x)?[a-fA-F0-9\\.:]{3,80}$', | ||
| pattern: '^(0x)?[a-zA-Z0-9\\.:]{3,80}$', |
There was a problem hiding this comment.
As we are already using oneOf, maybe it would be better to break this down into multiple RegExps intead of a single catch-all one, so that this validation is a bit more strict.
Summary by CodeRabbit
New Features
Bug Fixes