MetaMask / MetaMask/metamask-mobile

Engine.hasFunds() doubles ethFiat and ignores tokenFiat after fiat-balance refactor

Open
#30,576 0 comments 0 reactions 0 assignees View on GitHub
external-contributor INVALID-ISSUE-TEMPLATE
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

### What is this about?

`Engine.hasFunds()` currently computes `totalFiatBalance` as `fiatBalance.ethFiat + fiatBalance.ethFiat`, even though `getTotalEvmFiatAccountBalance()` returns separate `ethFiat` and `tokenFiat` values.

Current code in `app/core/Engine/Engine.ts`:

```ts
const fiatBalance = this.getTotalEvmFiatAccountBalance() || 0;
const totalFiatBalance = fiatBalance.ethFiat + fiatBalance.ethFiat;
return totalFiatBalance > 0 || tokenFound || nfts.length > 0;
```

This helper is used to gate backup / wallet-protection flows in:

- `app/components/Views/ProtectWalletMandatoryModal/ProtectWalletMandatoryModal.tsx`
- `app/components/Views/ManualBackupStep1/index.tsx`
- `app/components/Views/AccountBackupStep1/index.js`

### Why this looks like a regression

Before [#8572](https://github.com/MetaMask/metamask-mobile/pull/8572), `getTotalFiatAccountBalance()` returned a single combined fiat number and `hasFunds()` compared that numeric total directly.

In [#8572](https://github.com/MetaMask/metamask-mobile/pull/8572), the helper was refactored to return `{ ethFiat, tokenFiat }`, and `hasFunds()` was updated in the same change. The current `ethFiat + ethFiat` expression appears to have been introduced there and then carried forward through later refactors.

Later history preserved the same logic:

- [#12366](https://github.com/MetaMask/metamask-mobile/pull/12366) moved `Engine.ts` to `app/core/Engine/Engine.ts`
- [#14278](https://github.com/MetaMask/metamask-mobile/pull/14278) renamed `getTotalFiatAccountBalance()` to `getTotalEvmFiatAccountBalance()`

### Scenario

One concrete false-negative case:

- account has token fiat value
- `ethFiat === 0`
- `TokenBalancesController.tokenBalances` is missing or stale, so `tokenFound === false`
- `getTotalEvmFiatAccountBalance()` can still derive `tokenFiat` from token objects / `item.balance`
- `hasFunds()` returns `false`, allowing “Remind me later” / backup skip paths even though token fiat is present

`tokenFound` masks many token-holding cases, so this is not catastrophic, but the fiat calculation itself still looks wrong and can affect backup / wallet-protection gating.

### Technical Details

Relevant history:

- `3c9e448e64` (`Make backup required when user has funds`) introduced `hasFunds()` with a single numeric fiat total
- `108da1cec1` / [#8572](https://github.com/MetaMask/metamask-mobile/pull/8572) changed the helper to return `{ ethFiat, tokenFiat }` and introduced `ethFiat + ethFiat`
- `62a9a426d2` / [#14278](https://github.com/MetaMask/metamask-mobile/pull/14278) renamed the helper but preserved the same `hasFunds()` logic

There does not appear to be direct engine-level regression coverage for `hasFunds()` itself today; the surrounding screen tests generally mock `Engine.hasFunds()` as a boolean.

### Acceptance Criteria

- `hasFunds()` uses `ethFiat + tokenFiat`
- add regression coverage for a token-only / token-fiat-present case
- ideally include a case where raw `tokenBalances` is missing or stale but `getTotalEvmFiatAccountBalance()` still has token fiat, since that is the clearest false-negative path
- backup / wallet-protection flows continue to behave as expected for native-only, token-only, and zero-balance wallets

### References

- `app/core/Engine/Engine.ts`
- `app/components/Views/ProtectWalletMandatoryModal/ProtectWalletMandatoryModal.tsx`
- `app/components/Views/ManualBackupStep1/index.tsx`
- `app/components/Views/AccountBackupStep1/index.js`
- [#8572](https://github.com/MetaMask/metamask-mobile/pull/8572)
- [#12366](https://github.com/MetaMask/metamask-mobile/pull/12366)
- [#14278](https://github.com/MetaMask/metamask-mobile/pull/14278)

Contributor guide

Open the contributing guide

Research direction

Start in app/core/Engine/Engine.ts by locating hasFunds() and inspect existing Engine tests or search for tests covering this helper. Add regression coverage for native-only, token-only, token-fiat-present with missing or stale tokenBalances, and zero-balance cases. Run the relevant Engine test suite and confirm the backup and wallet-protection flows retain their expected gating behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
react-native, typescript
Domain
mobile-dev, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.