Skip to content

fix(assets-controller): preserve EVM bridge exchange rates when currency has no CAIP mapping - #10064

Open
gomesalexandre wants to merge 2 commits into
MetaMask:mainfrom
gomesalexandre:fix_exchange_rate_bridge_currency_bailout
Open

fix(assets-controller): preserve EVM bridge exchange rates when currency has no CAIP mapping#10064
gomesalexandre wants to merge 2 commits into
MetaMask:mainfrom
gomesalexandre:fix_exchange_rate_bridge_currency_bailout

Conversation

@gomesalexandre

@gomesalexandre gomesalexandre commented Sep 1, 2026

Copy link
Copy Markdown

What it says on the box

formatExchangeRatesForBridge bailed out of the entire exchange-rate response — wiping EVM marketData/currencyRates too — when a currency lookup that's only needed by the non-EVM branch (to key conversionRates) failed to find a match in MAP_CAIP_CURRENCIES. This was broken today for at least one real, currently-supported currency (gel), and the shared map has no test coverage tying it to the actual set of supported currencies, so the gap could reopen silently.

// before — one lookup that's only used ~90 lines later, in the non-EVM branch,
// aborts the whole function including EVM output that has nothing to do with it
const currencyCaip = MAP_CAIP_CURRENCIES[selectedCurrency.toLowerCase()];
if (!currencyCaip) {
  return { conversionRates: {}, currencyRates: {}, marketData: {}, currentCurrency: selectedCurrency };
}

The real gap this closes

Cross-checked MAP_CAIP_CURRENCIES's keys against packages/core-backend/src/api/shared-types.ts's SupportedCurrency type (the authoritative "currencies this backend actually supports" list) — after adding gel, the map is now a full superset:

supported but not mapped (before this PR): ["gel"]
supported but not mapped (after this PR):  []
mapped but not supported (harmless, pre-existing, untouched): ["xag", "xau", "xdr"]

Also confirmed live against the price API formatExchangeRatesForBridge's data ultimately comes from (https://price.api.cx.metamask.io/v1/supportedVsCurrencies) — it returns 94 supported vs-currencies including gel.

The fix

  1. packages/assets-controllers/src/MultichainAssetsRatesController/constant.ts — added the missing gel: 'swift:0/iso4217:GEL' entry. This is the actual production gap; every other SupportedCurrency was already mapped.
  2. formatExchangeRatesForBridge.ts — the top-level bail-out is gone. The currency lookup only affects the non-EVM (else) branch that actually consumes it — EVM marketData/currencyRates are computed unconditionally, regardless of whether the selected currency has a CAIP mapping.

For a genuinely unmapped currency (any future gap, or caller error), the fix deliberately omits rather than mislabels the non-EVM conversionRates entry — price for a non-EVM asset is already denominated in selectedCurrency, so silently falling back to e.g. MAP_CAIP_CURRENCIES.usd (my first draft's approach, corrected after adversarial review — see receipts) would emit a gel-denominated price mislabeled as USD. That's wrong data, not a safe fallback, so it's correctly left out instead.

Mirrors the same-map-consuming sibling MultichainAssetsRatesController.ts's existing pattern of not letting an unmapped currency wipe out unrelated computed data.

Testing

New tests cover:

  • A genuinely unmapped (fictional 'xyz') currency: EVM marketData/currencyRates are still returned; the non-EVM conversionRates entry is correctly omitted (not mislabeled).
  • gel specifically, end-to-end: EVM data resolves normally, and the non-EVM conversionRates entry resolves under the correct swift:0/iso4217:GEL CAIP currency (not USD).

receipts

$ NODE_OPTIONS=--experimental-vm-modules npx jest src/utils/formatExchangeRatesForBridge.test.ts --coverage=false
# on unfixed source (git stash of the two source files): 4 failed, 19 passed
# restored: 23/23 passed

$ NODE_OPTIONS=--experimental-vm-modules npx jest --coverage=false   # full assets-controller package
Test Suites: 33 passed, 33 total
Tests:       986 passed, 986 total   # baseline (unmodified main): 981/981, +5 new

$ NODE_OPTIONS=--experimental-vm-modules npx jest src/MultichainAssetsRatesController --coverage=false   # sibling package, touches the same shared map
Test Suites: 1 passed, 1 total
Tests:       35 passed, 35 total (incl. 4 snapshots)

$ npx eslint <changed files>       # clean, no output
$ node_modules/.bin/oxfmt --check <changed files>   # all matched files use the correct format
$ yarn workspace @metamask/assets-controller run changelog:validate      # clean
$ yarn workspace @metamask/assets-controllers run changelog:validate     # clean
$ yarn workspace @metamask/assets-controller run build:types 2>&1 | grep -c "error TS"
53   # identical to unmodified main (53); all pre-existing, unrelated (bridge-controller, kyc-controller)

Codex adversarial review

Ran codex exec synchronously (not backgrounded) as a second reviewer, twice.

First pass caught a real correctness bug in my initial fix: my first draft fell back to MAP_CAIP_CURRENCIES.usd for any unmapped currency — which is semantically wrong, since a non-EVM asset's price field is already denominated in selectedCurrency (only usdPrice is always USD), so labeling a gel-denominated price as swift:0/iso4217:USD would be incorrect data. Codex also independently ran its own cross-check (SupportedCurrency type vs. MAP_CAIP_CURRENCIES keys) and found gel is the only real gap — which reframed the fix from "add a generic fallback" to "close the actual gap directly, and make the generic fallback path correctly omit instead of mislabel." Rewrote accordingly (this PR's current state).

Second pass confirmed the correctness issue is fully resolved, and caught two minor follow-ups (a formatting nit, missing changelog entries in both touched packages) — both fixed before opening.

risk

Low. packages/assets-controllers/src/MultichainAssetsRatesController/constant.ts gets one new key (gel), which is purely additive — no existing key or value changes. formatExchangeRatesForBridge.ts's behavior is unchanged for every currently-supported currency (all of which are now in the map); the change only affects the previously-broken unmapped-currency path.


Note

Low Risk
Additive gel map entry and narrowed behavior change only for previously broken unmapped-currency paths; mapped currencies behave as before.

Overview
Fixes bridge exchange-rate formatting when the selected fiat currency is missing from MAP_CAIP_CURRENCIES (notably Georgian lari / gel). Previously formatExchangeRatesForBridge returned empty marketData and currencyRates for the whole response if the CAIP lookup failed, even though that lookup is only needed for non-EVM conversionRates.

MAP_CAIP_CURRENCIES now includes gel → swift:0/iso4217:GEL. formatExchangeRatesForBridge no longer early-exits on a failed lookup; EVM marketData / currencyRates are always built, and non-EVM conversionRates are added only when currencyCaip exists—otherwise they are omitted (not mislabeled as USD).

Tests cover the fictional unmapped currency path and end-to-end gel behavior.

Reviewed by Cursor Bugbot for commit c934195. Bugbot is set up for automated code reviews on this repo. Configure here.

…ncy has no CAIP mapping

formatExchangeRatesForBridge bailed out of the whole function - wiping
EVM marketData/currencyRates too - when a currency lookup needed only
by the non-EVM branch (to key conversionRates) failed. Fixed to only
affect the non-EVM entry for a genuinely unmapped currency, preserving
EVM output. The fallback omits (rather than mislabels) the non-EVM
entry, since price is already denominated in selectedCurrency and
relabeling it as some other currency would be wrong data.

Also adds the missing `gel` (Georgian lari) entry to
MAP_CAIP_CURRENCIES - the only SupportedCurrency actually absent from
the map today, confirmed against packages/core-backend's
SupportedCurrency type and the live price API's supportedVsCurrencies
list.
@gomesalexandre
gomesalexandre marked this pull request as ready for review September 1, 2026 22:36
@gomesalexandre
gomesalexandre requested review from a team as code owners September 1, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant