Commit b8d1270
authored
fix(transaction-pay-controller): convert exchange rate numbers to strings before passing to BigNumber (#8808)
## Explanation
Fixes a crash in `transaction-pay-controller` caused by `BigNumber`
throwing when receiving a JavaScript `number` with more than 15
significant digits.
### Root Cause
`BigNumber` enforces a strict 15 significant digit limit on `number`
inputs — both in the constructor (`new BigNumber(number)`) and in math
methods (`.multipliedBy(number)`, `.times(number)`, `.plus(number)`,
etc.) — because IEEE 754 floats lose precision beyond that.
In `getTokenFiatRate`
([`token.ts`](https://github.com/MetaMask/core/blob/main/packages/transaction-pay-controller/src/utils/token.ts#L215-L221)),
three raw `number` values from external sources are passed directly to
BigNumber:
```typescript
// tokenToNativeRate: number — from marketData[chainId][address].price
// nativeToUsdRate: number — from CurrencyRateController state
// nativeToFiatRate: number — from CurrencyRateController state
const usdRate = new BigNumber(tokenToNativeRate ?? 1) // ❌ constructor
.multipliedBy(nativeToUsdRate) // ❌ method arg
.toString(10);
const fiatRate = new BigNumber(tokenToNativeRate ?? 1) // ❌ constructor
.multipliedBy(nativeToFiatRate) // ❌ method arg
.toString(10);
```
### When does this crash?
**Case 1: Large conversion rate (weak-currency locales like VND, IDR)**
```js
new BigNumber(40115252.21304121) // ❌ 16 significant digits → THROWS
new BigNumber("40115252.21304121") // ✅ string input → no limit
// .multipliedBy() also validates:
bn.multipliedBy(40115252.21304121) // ❌ THROWS
bn.multipliedBy("40115252.21304121") // ✅ no limit
```
**Case 2: Very precise small price (micro-cap tokens)**
```js
new BigNumber(0.00000123456789012345) // ❌ 17 significant digits → THROWS
new BigNumber("0.00000123456789012345") // ✅ string input → no limit
```
**Case 3: Near-peg stablecoin with API floating point noise**
```js
new BigNumber(1.0000000000000002) // ❌ 17 significant digits → THROWS
new BigNumber("1.0000000000000002") // ✅ string input → no limit
```
Real user state logs confirm CurrencyRateController stores values
exceeding 15 significant digits for weak-currency locales:
```json
"currencyRates": {
"ETH": { "conversionRate": 40115252.21304121 },
"BNB": { "conversionRate": 11259865.090939905 }
}
```
This happens because `CurrencyRateController.boundedPrecisionNumber`
uses `toFixed(9)` which bounds **decimal** digits, not **significant**
digits. For large numbers, 8+ integer digits + 9 decimal digits = 17+
significant digits.
### The Fix
Wrap all three values in `String()` before passing to BigNumber.
BigNumber's string constructor and string method arguments have no
precision limit.
`getTokenFiatRate` is the single entry point where raw `number` values
from CurrencyRateController and marketData enter the
transaction-pay-controller. Everything downstream operates on
`FiatRates` (`{ fiatRate: string, usdRate: string }`), so fixing this
one function protects the entire controller.
### Related
- Extension fix:
MetaMask/metamask-extension#42674
- Upstream root cause:
[`CurrencyRateController.boundedPrecisionNumber`](https://github.com/MetaMask/core/blob/main/packages/assets-controllers/src/CurrencyRateController.ts#L111-L112)
should use `toPrecision(15)` instead of `toFixed(9)` (separate issue for
@metamask/assets-controllers)
## Changelog
### `@metamask/transaction-pay-controller`
- **Fixed**: Crash when exchange rate numbers exceed 15 significant
digits by converting to strings before passing to BigNumber
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: small, localized change to exchange-rate arithmetic that
only affects how values are passed into `BigNumber` (number→string) and
should preserve outputs while preventing runtime throws.
>
> **Overview**
> Prevents a runtime crash in `getTokenFiatRate` by converting
exchange-rate inputs to strings before constructing/operating on
`BigNumber`, avoiding the 15-significant-digit limit on JS `number`
inputs.
>
> Adds a matching `Unreleased` changelog entry noting the fix.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
d140b4e. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 10ae080 commit b8d1270
2 files changed
Lines changed: 5 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
212 | 212 | | |
213 | 213 | | |
214 | 214 | | |
215 | | - | |
216 | | - | |
| 215 | + | |
| 216 | + | |
217 | 217 | | |
218 | 218 | | |
219 | | - | |
220 | | - | |
| 219 | + | |
| 220 | + | |
221 | 221 | | |
222 | 222 | | |
223 | 223 | | |
| |||
0 commit comments