feat(treasury): accurate multi-currency balance conversion (#1827)#2379
Open
webguru-hypha wants to merge 7 commits into
Open
feat(treasury): accurate multi-currency balance conversion (#1827)#2379webguru-hypha wants to merge 7 commits into
webguru-hypha wants to merge 7 commits into
Conversation
Contributor
WalkthroughAdds supported profile currencies, persisted currency preferences, fiat conversion utilities, converted asset balances, reference-price handling, currency-aware treasury formatting, and localized currency/token-price UI. ChangesCurrency conversion
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProfileForm
participant PeopleAPI
participant AssetRoute
participant CoinGecko
participant Treasury
ProfileForm->>PeopleAPI: save selected currency
Treasury->>AssetRoute: request assets with currency
AssetRoute->>CoinGecko: resolve USD conversion rate
CoinGecko-->>AssetRoute: return currency rate
AssetRoute-->>Treasury: return converted assets and balance
Treasury-->>Treasury: format balance and token price
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@apps/web/src/app/api/v1/people/`[personSlug]/assets/route.ts:
- Around line 257-259: The referencePrice fallback is used as USD even when
referenceCurrency is non-USD. Update the map in the asset pricing logic to
retain referenceCurrency alongside referencePrice, then convert the fallback
price to USD before it contributes to usdEqual and balance, reusing the existing
currency-conversion helper or rate source.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/assets/route.ts:
- Around line 84-86: Update the referencePrice conversion in the token mapping
to check `token.referencePrice != null` before calling `Number`, returning null
otherwise. Keep this consistent with the existing `t.referencePrice != null`
check to prevent undefined values from becoming NaN in `dbTokens` and the API
response.
In `@packages/core/src/common/currency.ts`:
- Around line 32-42: Update CURRENCY_SYMBOLS so every SupportedCurrency maps to
its proper Unicode currency symbol, including EUR, GBP, JPY, CNY, CAD, CHF, AUD,
and HKD, keeping USD as '$'. Ensure getCurrencySymbol consumers receive
consistent symbol values.
In `@packages/core/src/common/server/get-fiat-conversion-rate.ts`:
- Around line 33-40: Add an AbortSignal.timeout() option to the CoinGecko fetch
call in getUsdConversionRate, using an appropriate finite timeout alongside the
existing headers so stalled requests abort and reach the function’s catch
handling.
In `@packages/core/src/people/types.ts`:
- Line 12: Replace the bare string currency types in the Person and
EditPersonInput interfaces with the supported currency type exported from
`@hypha-platform/common`, such as CurrencyCode or SupportedCurrency, and add the
required import.
In `@packages/epics/src/people/components/edit-person-section.tsx`:
- Around line 449-485: Replace the hardcoded Select placeholder in the currency
FormField with tProfile('editForm.placeholders.currency'), adding that key to
all required i18n message files. Update the currency fallback in the Select
value to use imported DEFAULT_CURRENCY instead of the string literal 'USD'.
In `@packages/epics/src/treasury/components/assets/asset-card.tsx`:
- Around line 191-201: Replace the hardcoded “Token Price” text in the asset
card rendering with the localized TreasuryTab.assetCard.tokenPrice translation,
supplying formatted price and currency values. Pass the component’s lang
argument to formatCurrencyValue(referencePrice, lang), and add the tokenPrice
message key with matching placeholders to every locale.
In `@packages/epics/src/treasury/hooks/use-assets.ts`:
- Around line 48-49: Remove the duplicate referenceCurrency property from the
AssetItem type/interface, keeping the existing declaration and retaining
referencePrice unchanged.
In `@packages/i18n/src/messages/es.json`:
- Around line 1478-1479: Add the missing editForm.labels.currency translation to
every locale JSON file (de.json, en.json, fr.json, pt.json, and es.json),
placing it consistently alongside the existing editForm.labels.location entry
and using the appropriate localized “Currency” value.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aab42109-28f5-4535-a6a2-27622bcb0bfb
📒 Files selected for processing (25)
apps/web/src/app/api/v1/people/[personSlug]/assets/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/assets/route.tspackages/core/src/common/currency.tspackages/core/src/common/index.tspackages/core/src/common/server/get-fiat-conversion-rate.tspackages/core/src/common/server/index.tspackages/core/src/common/server/web3-rpc/get-token-meta.tspackages/core/src/governance/types.tspackages/core/src/people/server/queries.tspackages/core/src/people/types.tspackages/core/src/people/validation.tspackages/epics/src/people/components/edit-person-section.tsxpackages/epics/src/treasury/components/assets/asset-card.tsxpackages/epics/src/treasury/hooks/use-assets-section.tspackages/epics/src/treasury/hooks/use-assets.tspackages/epics/src/treasury/hooks/use-user-assets-section.tspackages/epics/src/treasury/hooks/use-user-assets.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.jsonpackages/storage-postgres/migrations/0064_people_currency.sqlpackages/storage-postgres/migrations/meta/_journal.jsonpackages/storage-postgres/src/schema/people.ts
Rebase onto main and resolve currency conflicts including NZD support. Space treasury reads currency from the client query param instead of getDb/findSelf, while profile assets keep the authenticated user connection. Co-authored-by: Cursor <cursoragent@cursor.com>
webguru-hypha
force-pushed
the
feat/1827-currency-conversion-v2
branch
from
July 10, 2026 12:01
b157432 to
d6f84b1
Compare
Convert non-USD reference prices to USD before balance totals, align currency symbols, add fiat fetch timeout, tighten types, and complete i18n for profile currency and token price labels. Co-authored-by: Cursor <cursoragent@cursor.com>
Validate DB currency strings with isSupportedCurrency before assigning to Person.currency so check-types passes after the typed currency field. Co-authored-by: Cursor <cursoragent@cursor.com>
 |
|
🔗 Custom preview URL: https://pr-2379.preview-app.hypha.earth |
Replace hardcoded USD fallback in the currency select with the shared DEFAULT_CURRENCY constant to address the remaining CodeRabbit review. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/common/server/web3-rpc/get-token-meta.ts (2)
140-142: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid
anyin the catch block. Useunknownand guard witherror instanceof Errorbefore readingmessage.🤖 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 `@packages/core/src/common/server/web3-rpc/get-token-meta.ts` around lines 140 - 142, Update the catch clause in the token metadata fetch logic to use unknown instead of any, and only access error.message after an error instanceof Error guard; provide a safe fallback message for non-Error values while preserving the existing logging and rethrow behavior.Source: Learnings
53-55: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMatch stable-token metadata by address, not symbol.
dbTokens?.find(...)can attach another row’sreferencePrice/referenceCurrency(andiconUrl) when symbols collide. Fall back to symbol only for rows withoutaddress.🤖 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 `@packages/core/src/common/server/web3-rpc/get-token-meta.ts` around lines 53 - 55, Update the token lookup in the metadata retrieval function to prefer matching by normalized token address, preventing symbol collisions from selecting another row’s metadata. Use symbol matching only as a fallback for database rows whose address is absent, while preserving the existing behavior for tokens without an address.apps/web/src/app/api/v1/spaces/[spaceSlug]/assets/route.ts (1)
114-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the untyped error and balance array. Use
unknownfor the caught RPC error andAwaited<ReturnType<typeof getTokenBalancesByAddress>>forexternalTokens;anyhere skips type checking in the error and asset-parsing paths.🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/assets/route.ts around lines 114 - 116, The catch block uses an unsafe any type and the balance array lacks an explicit inferred RPC result type. In the relevant asset route handler, change the caught error in the catch block to unknown and type externalTokens as Awaited<ReturnType<typeof getTokenBalancesByAddress>>; update error and asset-parsing logic to narrow unknown safely before accessing properties.Source: Learnings
🤖 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 `@apps/web/src/app/api/v1/people/`[personSlug]/assets/route.ts:
- Around line 412-417: Update the asset-list handler to accept
`PaginationParams` from `core/common`, paginate the sorted assets before
constructing the response, and return a `PaginatedResponse` containing the page
data and pagination metadata. Ensure the pagination is applied after sorting and
that the balance calculation remains consistent with the intended full or
paginated dataset.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/assets/route.ts:
- Around line 300-303: The asset-list endpoint currently returns all assets
without pagination. Update its handler to accept PaginationParams from
core/common, apply the requested page and limit after sorting the aggregated
assets, and return a PaginatedResponse<T> containing the paginated assets plus
total/count metadata while preserving balance and currency fields as
appropriate.
In `@packages/core/src/common/server/get-fiat-conversion-rate.ts`:
- Around line 1-8: Extract the CoinGecko request from the conversion logic into
a new server-only client module named coingecko-client.ts, exporting one
function such as fetchUsdPriceRates. Move the URL construction, timeout, JSON
parsing, and non-2xx error handling there; update get-fiat-conversion-rate.ts to
import it and retain only caching, validation, and fallback behavior.
- Around line 22-57: Update fetchUsdConversionRate to iterate over all rates in
the CoinGecko response, validate each positive finite numeric value, and store
every valid currency rate in conversionCache using getConversionCacheKey.
Preserve returning the requested currency’s rate and retain the existing error
when that requested rate is missing or invalid.
In `@packages/epics/src/treasury/components/assets/asset-card.tsx`:
- Around line 191-202: Separate the USD market-rate currency from the configured
reference-price currency in the asset-card data flow. Update the upstream
token-price computation and component props to provide a distinct
tokenPriceCurrency (USD or the selected display currency), use it for tokenPrice
on the existing market-rate row, and keep referenceCurrency only for formatting
referencePrice in the new row.
---
Outside diff comments:
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/assets/route.ts:
- Around line 114-116: The catch block uses an unsafe any type and the balance
array lacks an explicit inferred RPC result type. In the relevant asset route
handler, change the caught error in the catch block to unknown and type
externalTokens as Awaited<ReturnType<typeof getTokenBalancesByAddress>>; update
error and asset-parsing logic to narrow unknown safely before accessing
properties.
In `@packages/core/src/common/server/web3-rpc/get-token-meta.ts`:
- Around line 140-142: Update the catch clause in the token metadata fetch logic
to use unknown instead of any, and only access error.message after an error
instanceof Error guard; provide a safe fallback message for non-Error values
while preserving the existing logging and rethrow behavior.
- Around line 53-55: Update the token lookup in the metadata retrieval function
to prefer matching by normalized token address, preventing symbol collisions
from selecting another row’s metadata. Use symbol matching only as a fallback
for database rows whose address is absent, while preserving the existing
behavior for tokens without an address.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 074f576e-84ef-4247-aec6-8f17b19a0df0
📒 Files selected for processing (24)
apps/web/src/app/api/v1/people/[personSlug]/assets/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/assets/route.tspackages/core/src/common/currency.tspackages/core/src/common/index.tspackages/core/src/common/server/get-fiat-conversion-rate.tspackages/core/src/common/server/index.tspackages/core/src/common/server/web3-rpc/get-token-meta.tspackages/core/src/people/server/queries.tspackages/core/src/people/types.tspackages/core/src/people/validation.tspackages/epics/src/people/components/edit-person-section.tsxpackages/epics/src/treasury/components/assets/asset-card.tsxpackages/epics/src/treasury/hooks/use-assets-section.tspackages/epics/src/treasury/hooks/use-assets.tspackages/epics/src/treasury/hooks/use-user-assets-section.tspackages/epics/src/treasury/hooks/use-user-assets.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.jsonpackages/storage-postgres/migrations/0067_people_currency.sqlpackages/storage-postgres/migrations/meta/_journal.jsonpackages/storage-postgres/src/schema/people.ts
Comment on lines
+412
to
+417
| const usdBalance = sorted.reduce((sum, asset) => sum + asset.usdEqual, 0); | ||
|
|
||
| return NextResponse.json({ | ||
| assets: sorted, | ||
| balance: sorted.reduce((sum, asset) => sum + asset.usdEqual, 0), | ||
| balance: usdBalance * usdConversionRate, | ||
| currency, |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Paginate this asset-list response.
This route returns every asset after per-token RPC work. Add PaginationParams input and return PaginatedResponse<T> to bound response size and request cost.
As per coding guidelines, “All list endpoints must use PaginatedResponse<T> and PaginationParams from core/common for pagination.”
🤖 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 `@apps/web/src/app/api/v1/people/`[personSlug]/assets/route.ts around lines 412
- 417, Update the asset-list handler to accept `PaginationParams` from
`core/common`, paginate the sorted assets before constructing the response, and
return a `PaginatedResponse` containing the page data and pagination metadata.
Ensure the pagination is applied after sorting and that the balance calculation
remains consistent with the intended full or paginated dataset.
Source: Coding guidelines
Comment on lines
300
to
+303
| return NextResponse.json({ | ||
| assets: sorted, | ||
| balance: sorted.reduce((sum, asset) => sum + asset.usdEqual, 0), | ||
| balance: usdBalance * usdConversionRate, | ||
| currency, |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Paginate this asset-list response.
This endpoint aggregates every token and performs per-token chain reads before returning the full list. Accept PaginationParams and return PaginatedResponse<T>.
As per coding guidelines, “All list endpoints must use PaginatedResponse<T> and PaginationParams from core/common for pagination.”
🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/assets/route.ts around lines 300
- 303, The asset-list endpoint currently returns all assets without pagination.
Update its handler to accept PaginationParams from core/common, apply the
requested page and limit after sorting the aggregated assets, and return a
PaginatedResponse<T> containing the paginated assets plus total/count metadata
while preserving balance and currency fields as appropriate.
Source: Coding guidelines
Comment on lines
+1
to
+8
| import 'server-only'; | ||
| import NodeCache from 'node-cache'; | ||
| import { | ||
| DEFAULT_CURRENCY, | ||
| SupportedCurrency, | ||
| resolveSupportedCurrency, | ||
| SUPPORTED_CURRENCIES, | ||
| } from '../currency'; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
File naming doesn't follow the <api-name>-client.ts convention for external API clients.
As per coding guidelines for packages/core/src/**/server/**/*.ts: "External API clients should be plain fetch with a thin server-only wrapper; one exported function per API call, throws on non-2xx responses; implement as separate files under packages/core/src/common/server/<api-name>-client.ts following the Bridge or Alchemy client patterns."
Consider extracting the CoinGecko fetch call into packages/core/src/common/server/coingecko-client.ts (exporting a single function that throws on non-2xx), and keeping the caching/conversion logic in this file or a domain utility.
♻️ Proposed structure
// packages/core/src/common/server/coingecko-client.ts
import 'server-only';
import { SUPPORTED_CURRENCIES } from '../currency';
type FiatConversionResponse = { usd?: Record<string, number> };
const getSupportedQuoteCurrencies = (): string =>
SUPPORTED_CURRENCIES.map((currency) => currency.toLowerCase()).join(',');
export async function fetchUsdPriceRates(): Promise<FiatConversionResponse> {
const response = await fetch(
`https://api.coingecko.com/api/v3/simple/price?ids=usd&vs_currencies=${getSupportedQuoteCurrencies()}`,
{
headers: { Accept: 'application/json' },
signal: AbortSignal.timeout(10_000),
},
);
if (!response.ok) {
throw new Error(`Failed to fetch fiat rates: ${response.statusText}`);
}
return (await response.json()) as FiatConversionResponse;
}Then get-fiat-conversion-rate.ts imports fetchUsdPriceRates and handles caching, validation, and fallback.
🤖 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 `@packages/core/src/common/server/get-fiat-conversion-rate.ts` around lines 1 -
8, Extract the CoinGecko request from the conversion logic into a new
server-only client module named coingecko-client.ts, exporting one function such
as fetchUsdPriceRates. Move the URL construction, timeout, JSON parsing, and
non-2xx error handling there; update get-fiat-conversion-rate.ts to import it
and retain only caching, validation, and fallback behavior.
Source: Coding guidelines
Comment on lines
+22
to
+57
| async function fetchUsdConversionRate(currency: SupportedCurrency) { | ||
| if (currency === DEFAULT_CURRENCY) { | ||
| return 1; | ||
| } | ||
|
|
||
| const cacheKey = getConversionCacheKey(currency); | ||
| const cachedRate = conversionCache.get<number>(cacheKey); | ||
| if (cachedRate !== undefined) { | ||
| return cachedRate; | ||
| } | ||
|
|
||
| const response = await fetch( | ||
| `https://api.coingecko.com/api/v3/simple/price?ids=usd&vs_currencies=${getSupportedQuoteCurrencies()}`, | ||
| { | ||
| headers: { | ||
| Accept: 'application/json', | ||
| }, | ||
| signal: AbortSignal.timeout(10_000), | ||
| }, | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch fiat rates: ${response.statusText}`); | ||
| } | ||
|
|
||
| const data = (await response.json()) as FiatConversionResponse; | ||
| const normalizedCurrency = currency.toLowerCase(); | ||
| const rate = data.usd?.[normalizedCurrency]; | ||
|
|
||
| if (typeof rate !== 'number' || !Number.isFinite(rate) || rate <= 0) { | ||
| throw new Error(`Invalid conversion rate for currency "${currency}"`); | ||
| } | ||
|
|
||
| conversionCache.set(cacheKey, rate); | ||
| return rate; | ||
| } |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Cache all rates from the single CoinGecko response to avoid redundant API calls.
fetchUsdConversionRate requests all supported currencies in one API call (getSupportedQuoteCurrencies() joins all of them), but only caches the one requested. If a different currency is requested next, it triggers another full API call to CoinGecko. Since the response already contains all rates, cache them all in one pass.
♻️ Proposed fix: cache all valid rates from a single response
const data = (await response.json()) as FiatConversionResponse;
+
+ // Cache all valid rates from this single API response
+ if (data.usd) {
+ for (const supportedCurrency of SUPPORTED_CURRENCIES) {
+ if (supportedCurrency === DEFAULT_CURRENCY) continue;
+ const rate = data.usd[supportedCurrency.toLowerCase()];
+ if (typeof rate === 'number' && Number.isFinite(rate) && rate > 0) {
+ conversionCache.set(getConversionCacheKey(supportedCurrency), rate);
+ }
+ }
+ }
+
const normalizedCurrency = currency.toLowerCase();
const rate = data.usd?.[normalizedCurrency];
if (typeof rate !== 'number' || !Number.isFinite(rate) || rate <= 0) {
throw new Error(`Invalid conversion rate for currency "${currency}"`);
}
- conversionCache.set(cacheKey, rate);
return rate;📝 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.
Suggested change
| async function fetchUsdConversionRate(currency: SupportedCurrency) { | |
| if (currency === DEFAULT_CURRENCY) { | |
| return 1; | |
| } | |
| const cacheKey = getConversionCacheKey(currency); | |
| const cachedRate = conversionCache.get<number>(cacheKey); | |
| if (cachedRate !== undefined) { | |
| return cachedRate; | |
| } | |
| const response = await fetch( | |
| `https://api.coingecko.com/api/v3/simple/price?ids=usd&vs_currencies=${getSupportedQuoteCurrencies()}`, | |
| { | |
| headers: { | |
| Accept: 'application/json', | |
| }, | |
| signal: AbortSignal.timeout(10_000), | |
| }, | |
| ); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch fiat rates: ${response.statusText}`); | |
| } | |
| const data = (await response.json()) as FiatConversionResponse; | |
| const normalizedCurrency = currency.toLowerCase(); | |
| const rate = data.usd?.[normalizedCurrency]; | |
| if (typeof rate !== 'number' || !Number.isFinite(rate) || rate <= 0) { | |
| throw new Error(`Invalid conversion rate for currency "${currency}"`); | |
| } | |
| conversionCache.set(cacheKey, rate); | |
| return rate; | |
| } | |
| const data = (await response.json()) as FiatConversionResponse; | |
| // Cache all valid rates from this single API response | |
| if (data.usd) { | |
| for (const supportedCurrency of SUPPORTED_CURRENCIES) { | |
| if (supportedCurrency === DEFAULT_CURRENCY) continue; | |
| const rate = data.usd[supportedCurrency.toLowerCase()]; | |
| if (typeof rate === 'number' && Number.isFinite(rate) && rate > 0) { | |
| conversionCache.set(getConversionCacheKey(supportedCurrency), rate); | |
| } | |
| } | |
| } | |
| const normalizedCurrency = currency.toLowerCase(); | |
| const rate = data.usd?.[normalizedCurrency]; | |
| if (typeof rate !== 'number' || !Number.isFinite(rate) || rate <= 0) { | |
| throw new Error(`Invalid conversion rate for currency "${currency}"`); | |
| } | |
| return rate; |
🤖 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 `@packages/core/src/common/server/get-fiat-conversion-rate.ts` around lines 22
- 57, Update fetchUsdConversionRate to iterate over all rates in the CoinGecko
response, validate each positive finite numeric value, and store every valid
currency rate in conversionCache using getConversionCacheKey. Preserve returning
the requested currency’s rate and retain the existing error when that requested
rate is missing or invalid.
Comment on lines
+191
to
+202
| {referencePrice !== null && | ||
| referencePrice !== undefined && | ||
| referenceCurrency && ( | ||
| <div className="w-full flex flex-row gap-1"> | ||
| <Text className="text-1 text-neutral-11"> | ||
| {tTreasury('assetCard.tokenPrice', { | ||
| price: formatCurrencyValue(referencePrice, lang), | ||
| currency: referenceCurrency, | ||
| })} | ||
| </Text> | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the USD market rate separate from the reference-price currency.
tokenPrice is used upstream to compute usdEqual, so it is USD. The existing row on Lines 141-146 labels it with referenceCurrency; this new row then shows the raw configured value, producing conflicting values for EUR/GBP-priced tokens. Return/render a distinct tokenPriceCurrency (USD or display currency), while reserving referenceCurrency for referencePrice.
🤖 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 `@packages/epics/src/treasury/components/assets/asset-card.tsx` around lines
191 - 202, Separate the USD market-rate currency from the configured
reference-price currency in the asset-card data flow. Update the upstream
token-price computation and component props to provide a distinct
tokenPriceCurrency (USD or the selected display currency), use it for tokenPrice
on the existing market-rate row, and keep referenceCurrency only for formatting
referencePrice in the new row.
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
1 participant
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.
Summary
people.currencymigrationCloses #1827
Test plan
0064_people_currency.sqllocallyMade with Cursor
Summary by CodeRabbit