fix: guard RpcDataSource against invalid decimals to prevent NaN bala…

packages/assets-controller/CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `RpcDataSource` no longer writes `{ amount: "NaN" }` entries into `assetsBalance` when state holds malformed native metadata ([#8781](https://github.com/MetaMask/core/pull/8781))
+  - Existing native metadata in `assetsInfo` is now only reused when its `decimals` field is a finite, non-negative number (via a new `#hasValidDecimals` guard). Stale entries like `{ type: 'native', decimals: null, name: '', symbol: '' }` or `{ decimals: -1, ... }` are replaced with the chain-status stub (`{ type: 'native', symbol/name: chainStatus.nativeCurrency, decimals: 18 }`) so balance conversion has a usable `decimals` and consumers never see a negative `decimals` in `assetsInfo`.
+  - `decimals: 0` is treated as valid; `name` and `symbol` are not required.
+  - Decimals resolution in `#handleBalanceUpdate` and the manual-fetch path no longer relies on `??` to fall through from state to pipeline metadata. A new `#pickValidDecimals` helper picks the first source whose `decimals` is finite and non-negative, so a stale `decimals: NaN` (or `decimals: -1`) in state can no longer shadow the chain-status stub's `decimals: 18` and silently produce `amount: '0'` while `assetsInfo` reports `decimals: 18`.
+  - `#convertToHumanReadable` now defensively returns `'0'` when `decimals` isn't a finite non-negative number or when the raw balance can't be parsed, matching the existing safe fallback used in the error path.
+
 ## [7.1.1]
 
 ### Changed

packages/assets-controller/src/data-sources/RpcDataSource.test.ts

@@ -1553,6 +1553,225 @@ describe('RpcDataSource', () => {
     });
   });
 
+  describe('native metadata validation (#hasValidDecimals)', () => {
+    const NATIVE_ASSET_ID = 'eip155:1/slip44:60' as Caip19AssetId;
+
+    const subscribeAndEmit = async (
+      stateNativeMeta: unknown,
+      onAssetsUpdate: jest.Mock,
+    ): Promise<void> => {
+      let balanceUpdateCallback:
+        | ((result: BalanceFetchResult) => void | Promise<void>)
+        | null = null;
+      jest
+        .spyOn(BalanceFetcher.prototype, 'setOnBalanceUpdate')
+        .mockImplementation(function (this: BalanceFetcher, callback) {
+          balanceUpdateCallback = callback;
+        });
+
+      await withController(
+        {
+          actionHandlerOverrides: {
+            'AssetsController:getState': () => ({
+              ...getDefaultAssetsControllerState(),
+              assetsInfo: { [NATIVE_ASSET_ID]: stateNativeMeta },
+            }),
+          },
+        },
+        async ({ controller }) => {
+          await controller.subscribe({
+            request: createDataRequest(),
+            subscriptionId: 'test-sub',
+            isUpdate: false,
+            onAssetsUpdate,
+          });
+          await balanceUpdateCallback?.(
+            createBalanceFetchResult({
+              balances: [
+                {
+                  assetId: NATIVE_ASSET_ID,
+                  balance: '1000000000000000000',
+                } as BalanceFetchResult['balances'][0],
+              ],
+            }),
+          );
+        },
+      );
+    };
+
+    it('falls back to chain-status stub when state native metadata has null decimals', async () => {
+      const onAssetsUpdate = jest.fn();
+      await subscribeAndEmit(
+        {
+          aggregators: ['dynamic'],
+          decimals: null,
+          name: '',
+          symbol: '',
+          type: 'native',
+        },
+        onAssetsUpdate,
+      );
+
+      expect(onAssetsUpdate).toHaveBeenCalled();
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(response.assetsInfo?.[NATIVE_ASSET_ID]).toStrictEqual({
+        type: 'native',
+        symbol: 'ETH',
+        name: 'ETH',
+        decimals: 18,
+      });
+      expect(
+        response.assetsBalance?.[MOCK_ACCOUNT_ID]?.[NATIVE_ASSET_ID]?.amount,
+      ).toBe('1');
+    });
+
+    it('falls back to chain-status stub when state native metadata has NaN decimals', async () => {
+      const onAssetsUpdate = jest.fn();
+      await subscribeAndEmit(
+        {
+          decimals: Number.NaN,
+          name: 'ETH',
+          symbol: 'ETH',
+          type: 'native',
+        },
+        onAssetsUpdate,
+      );
+
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(response.assetsInfo?.[NATIVE_ASSET_ID]?.decimals).toBe(18);
+      // Regression: `decimals: NaN` in state must not bypass the pipeline's
+      // valid `decimals: 18` via `??`, otherwise the amount silently becomes
+      // '0' even though `assetsInfo` reports the correct decimals.
+      expect(
+        response.assetsBalance?.[MOCK_ACCOUNT_ID]?.[NATIVE_ASSET_ID]?.amount,
+      ).toBe('1');
+    });
+
+    it('keeps existing native metadata when decimals is 0 (valid)', async () => {
+      const onAssetsUpdate = jest.fn();
+      const existing = {
+        decimals: 0,
+        name: '',
+        symbol: '',
+        type: 'native' as const,
+      };
+      await subscribeAndEmit(existing, onAssetsUpdate);
+
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(response.assetsInfo?.[NATIVE_ASSET_ID]).toStrictEqual(existing);
+    });
+
+    it('keeps existing native metadata when name/symbol are missing but decimals is valid', async () => {
+      const onAssetsUpdate = jest.fn();
+      const existing = {
+        decimals: 18,
+        type: 'native' as const,
+      };
+      await subscribeAndEmit(existing, onAssetsUpdate);
+
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(response.assetsInfo?.[NATIVE_ASSET_ID]).toStrictEqual(existing);
+    });
+
+    it('falls back to chain-status stub when state native metadata has negative decimals', async () => {
+      const onAssetsUpdate = jest.fn();
+      await subscribeAndEmit(
+        {
+          decimals: -1,
+          name: 'X',
+          symbol: 'X',
+          type: 'native',
+        },
+        onAssetsUpdate,
+      );
+
+      const [response] = onAssetsUpdate.mock.calls[0];
+      // Regression: `#hasValidDecimals` previously accepted negative
+      // decimals (only checked `Number.isFinite`), so consumers saw stale
+      // `decimals: -1` in `assetsInfo` while the balance silently became
+      // `'0'`. The metadata guard must reject negatives so the chain-status
+      // stub takes over and the balance resolves correctly.
+      expect(response.assetsInfo?.[NATIVE_ASSET_ID]).toStrictEqual({
+        type: 'native',
+        symbol: 'ETH',
+        name: 'ETH',
+        decimals: 18,
+      });
+      expect(
+        response.assetsBalance?.[MOCK_ACCOUNT_ID]?.[NATIVE_ASSET_ID]?.amount,
+      ).toBe('1');
+    });
+  });
+
+  describe('convertToHumanReadable guards', () => {
+    const NATIVE_ASSET_ID = 'eip155:1/slip44:60' as Caip19AssetId;
+
+    const runFetchWithStateMetadata = async (
+      stateNativeMeta: unknown,
+      rawBalance: string,
+    ): Promise<{
+      onAssetsUpdate: jest.Mock;
+    }> => {
+      let balanceUpdateCallback:
+        | ((result: BalanceFetchResult) => void | Promise<void>)
+        | null = null;
+      jest
+        .spyOn(BalanceFetcher.prototype, 'setOnBalanceUpdate')
+        .mockImplementation(function (this: BalanceFetcher, callback) {
+          balanceUpdateCallback = callback;
+        });
+
+      const onAssetsUpdate = jest.fn();
+      await withController(
+        {
+          actionHandlerOverrides: {
+            'AssetsController:getState': () => ({
+              ...getDefaultAssetsControllerState(),
+              assetsInfo: { [NATIVE_ASSET_ID]: stateNativeMeta },
+            }),
+          },
+        },
+        async ({ controller }) => {
+          await controller.subscribe({
+            request: createDataRequest(),
+            subscriptionId: 'test-sub',
+            isUpdate: false,
+            onAssetsUpdate,
+          });
+          await balanceUpdateCallback?.(
+            createBalanceFetchResult({
+              balances: [
+                {
+                  assetId: NATIVE_ASSET_ID,
+                  balance: rawBalance,
+                } as BalanceFetchResult['balances'][0],
+              ],
+            }),
+          );
+        },
+      );
+
+      return { onAssetsUpdate };
+    };
+
+    it('returns "0" amount when raw balance is not a number', async () => {
+      const { onAssetsUpdate } = await runFetchWithStateMetadata(
+        {
+          decimals: 18,
+          name: 'ETH',
+          symbol: 'ETH',
+          type: 'native',
+        },
+        'not-a-number',
+      );
+
+      const [response] = onAssetsUpdate.mock.calls[0];
+      expect(
+        response.assetsBalance?.[MOCK_ACCOUNT_ID]?.[NATIVE_ASSET_ID]?.amount,
+      ).toBe('0');
+    });
+  });
+
   describe('handleDetectionUpdate (via callback)', () => {
     it('invokes onAssetsUpdate when TokenDetector callback runs', async () => {
       let detectionUpdateCallback:

packages/assets-controller/src/data-sources/RpcDataSource.ts

@@ -346,12 +346,30 @@ export class RpcDataSource extends AbstractDataSource<
   /**
    * Convert a raw balance to human-readable format using decimals.
    *
+   * Returns `'0'` when either input is invalid (e.g. `decimals` is `null`,
+   * `NaN`, negative or non-finite, or `rawBalance` cannot be parsed as a
+   * number). Defaulting to a fixed decimals value would silently produce
+   * wrong amounts; `'0'` keeps state safe and never lets `NaN` leak in.
+   *
    * @param rawBalance - The raw balance string.
    * @param decimals - The number of decimals for the token.
-   * @returns The human-readable balance string.
+   * @returns The human-readable balance string, or `'0'` when inputs are invalid.
    */
   #convertToHumanReadable(rawBalance: string, decimals: number): string {
+    if (!Number.isFinite(decimals) || decimals < 0) {
+      log('Invalid decimals — defaulting balance to "0"', {
+        rawBalance,
+        decimals,
+      });
+      return '0';
+    }
+
     const rawAmount = new BigNumberJS(rawBalance);
+    if (!rawAmount.isFinite()) {
+      log('Invalid raw balance — defaulting to "0"', { rawBalance, decimals });
+      return '0';
+    }
+
     const divisor = new BigNumberJS(10).pow(decimals);
     return rawAmount.dividedBy(divisor).toFixed();
   }
@@ -383,7 +401,7 @@ export class RpcDataSource extends AbstractDataSource<
         // enriched by the price/info API with image, description, etc.
         // Only emit a minimal stub when there's nothing in state yet,
         // so we don't clobber that richer metadata on every balance refresh.
-        if (existingMeta) {
+        if (this.#hasValidDecimals(existingMeta)) {
           assetsInfo[balance.assetId] = existingMeta;
         } else {
           const chainStatus = this.#chainStatuses[chainId];
@@ -406,6 +424,50 @@ export class RpcDataSource extends AbstractDataSource<
     return assetsInfo;
   }
 
+  /**
+   * Type guard for metadata whose `decimals` is safe to use for balance
+   * conversion.
+   *
+   * Mirrors the validity rules in `#convertToHumanReadable` (finite and
+   * non-negative). Keeping these in sync ensures that whenever the metadata
+   * guard accepts a value, the balance guard will also accept it — so we
+   * never end up emitting metadata with `decimals: -1` while silently
+   * defaulting the balance to `'0'`.
+   *
+   * @param metadata - The metadata to check.
+   * @returns `true` if `decimals` is a finite, non-negative number.
+   */
+  #hasValidDecimals(
+    metadata: AssetMetadata | undefined,
+  ): metadata is AssetMetadata {
+    return Boolean(
+      metadata && Number.isFinite(metadata.decimals) && metadata.decimals >= 0,
+    );
+  }
+
+  /**
+   * Pick the first valid `decimals` value from a list of metadata sources.
+   *
+   * `??` only short-circuits on `null`/`undefined`, so a stale state entry
+   * with `decimals: NaN` would otherwise win over a later source that holds
+   * a correct value (e.g. the chain-status stub produced by
+   * `#collectMetadataForBalances`). This helper treats `NaN`, negative, and
+   * non-finite values as missing so the next source can supply a usable one.
+   *
+   * @param metadatas - Metadata candidates in priority order.
+   * @returns The first finite `decimals` value, or `undefined` if none are valid.
+   */
+  #pickValidDecimals(
+    ...metadatas: (AssetMetadata | undefined)[]
+  ): number | undefined {
+    for (const metadata of metadatas) {
+      if (this.#hasValidDecimals(metadata)) {
+        return metadata.decimals;
+      }
+    }
+    return undefined;
+  }
+
   /**
    * Handle balance update from BalanceFetcher.
    *
@@ -436,7 +498,7 @@ export class RpcDataSource extends AbstractDataSource<
     for (const balance of normalizedBalances) {
       const stateMetadata = existingMetadata[balance.assetId];
       const pipelineMetadata = assetsInfo[balance.assetId];
-      const decimals = stateMetadata?.decimals ?? pipelineMetadata?.decimals;
+      const decimals = this.#pickValidDecimals(stateMetadata, pipelineMetadata);
 
       if (decimals === undefined) {
         continue;
@@ -1012,8 +1074,10 @@ export class RpcDataSource extends AbstractDataSource<
           for (const balance of normalizedBalances) {
             const stateMetadata = existingMetadata[balance.assetId];
             const pipelineMetadata = assetsInfo[balance.assetId];
-            let decimals: number | undefined =
-              stateMetadata?.decimals ?? pipelineMetadata?.decimals;
+            let decimals: number | undefined = this.#pickValidDecimals(
+              stateMetadata,
+              pipelineMetadata,
+            );
 
             if (decimals === undefined) {
               const parsed = parseCaipAssetType(balance.assetId);
@@ -1054,7 +1118,7 @@ export class RpcDataSource extends AbstractDataSource<
           // nothing is in state yet, so we don't clobber that richer metadata.
           const existingNativeMeta =
             this.#getExistingAssetsMetadata()[nativeAssetId];
-          if (existingNativeMeta) {
+          if (this.#hasValidDecimals(existingNativeMeta)) {
             assetsInfo[nativeAssetId] = existingNativeMeta;
           } else {
             const chainStatus = this.#chainStatuses[chainId];