Skip to content

fix(transaction-pay-controller): convert exchange rate numbers to strings before passing to BigNumber#8808

Open
OGPoyraz wants to merge 4 commits into
mainfrom
ogp/significant-number-fix-tpc
Open

fix(transaction-pay-controller): convert exchange rate numbers to strings before passing to BigNumber#8808
OGPoyraz wants to merge 4 commits into
mainfrom
ogp/significant-number-fix-tpc

Conversation

@OGPoyraz
Copy link
Copy Markdown
Member

@OGPoyraz OGPoyraz commented May 14, 2026

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), three raw number values from external sources are passed directly to BigNumber:

// 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)

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)

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

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:

"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

Changelog

@metamask/transaction-pay-controller

  • Fixed: Crash when exchange rate numbers exceed 15 significant digits by converting to strings before passing to BigNumber

Note

Low Risk
Low risk, localized change to exchange-rate arithmetic that only alters input coercion to BigNumber to prevent runtime exceptions; outputs remain string rates.

Overview
Prevents transaction-pay-controller crashes when fiat/exchange rates exceed BigNumber’s 15-significant-digit limit by coercing tokenToNativeRate, nativeToUsdRate, and nativeToFiatRate to strings before BigNumber construction/multiplication in getTokenFiatRate.

Updates the package changelog to document the fix.

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

OGPoyraz added 2 commits May 14, 2026 11:38
…ings before passing to BigNumber

BigNumber throws when a JS number has more than 15 significant digits.
In getTokenFiatRate, three number values from external sources are
passed to BigNumber constructor and .multipliedBy():

- tokenToNativeRate (from marketData) -> new BigNumber()
- nativeToUsdRate (from CurrencyRateController) -> .multipliedBy()
- nativeToFiatRate (from CurrencyRateController) -> .multipliedBy()

All three can exceed 15 significant digits, especially for
weak-currency locales where conversion rates are large numbers
like 40115252.21304121 (16 significant digits).

Wrapping in String() avoids the crash since BigNumber's string
inputs have no precision limit.
@OGPoyraz OGPoyraz marked this pull request as ready for review May 14, 2026 09:49
@OGPoyraz OGPoyraz requested review from a team as code owners May 14, 2026 09:49
@OGPoyraz OGPoyraz temporarily deployed to default-branch May 14, 2026 09:49 — with GitHub Actions Inactive
@OGPoyraz OGPoyraz added this pull request to the merge queue May 14, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch May 14, 2026
vinistevam
vinistevam previously approved these changes May 14, 2026
@OGPoyraz OGPoyraz added this pull request to the merge queue May 15, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 15, 2026
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.

3 participants