Skip to content

Commit

Permalink
fix: Prevent coercing small spending caps to zero (#28179)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Previously there was a bug that affected the approve screen. When users
had a small spending cap (between 0.001 and 0.0001 or smaller), it was
coerced to 0.

This was caused by the method `new
Intl.NumberFormat(locale).format(spendingCap)` that applied the `1,000`
large number formatting, so the fix is to bypass it entirely for values
smaller than 1.

Additionally, these unformatted small numbers are presented in
scientific notation, so we leverage `toNonScientificString(spendingCap)`
to prevent that.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28179?quickstart=1)

## **Related issues**

Fixes:
[#28117](#28117)

## **Manual testing steps**

See original bug report.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
pedronfigueiredo authored and Gudahtt committed Oct 31, 2024
1 parent 2c9ad97 commit d267b4e
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('useApproveTokenSimulation', () => {

expect(result.current).toMatchInlineSnapshot(`
{
"formattedSpendingCap": 7,
"formattedSpendingCap": "7",
"pending": undefined,
"spendingCap": "#7",
"value": {
Expand Down Expand Up @@ -155,4 +155,70 @@ describe('useApproveTokenSimulation', () => {
}
`);
});

it('returns correct small decimal number token amount for fungible tokens', async () => {
const useIsNFTMock = jest.fn().mockImplementation(() => ({ isNFT: false }));

const useDecodedTransactionDataMock = jest.fn().mockImplementation(() => ({
pending: false,
value: {
data: [
{
name: 'approve',
params: [
{
type: 'address',
value: '0x9bc5baF874d2DA8D216aE9f137804184EE5AfEF4',
},
{
type: 'uint256',
value: 10 ** 5,
},
],
},
],
source: 'FourByte',
},
}));

(useIsNFT as jest.Mock).mockImplementation(useIsNFTMock);
(useDecodedTransactionData as jest.Mock).mockImplementation(
useDecodedTransactionDataMock,
);

const transactionMeta = genUnapprovedContractInteractionConfirmation({
address: CONTRACT_INTERACTION_SENDER_ADDRESS,
}) as TransactionMeta;

const { result } = renderHookWithProvider(
() => useApproveTokenSimulation(transactionMeta, '18'),
mockState,
);

expect(result.current).toMatchInlineSnapshot(`
{
"formattedSpendingCap": "0.0000000000001",
"pending": undefined,
"spendingCap": "0.0000000000001",
"value": {
"data": [
{
"name": "approve",
"params": [
{
"type": "address",
"value": "0x9bc5baF874d2DA8D216aE9f137804184EE5AfEF4",
},
{
"type": "uint256",
"value": 100000,
},
],
},
],
"source": "FourByte",
},
}
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useMemo } from 'react';
import { useSelector } from 'react-redux';
import { getIntlLocale } from '../../../../../../../ducks/locale/locale';
import { SPENDING_CAP_UNLIMITED_MSG } from '../../../../../constants';
import { toNonScientificString } from '../../hooks/use-token-values';
import { useDecodedTransactionData } from '../../hooks/useDecodedTransactionData';
import { useIsNFT } from './use-is-nft';

Expand Down Expand Up @@ -46,8 +47,9 @@ export const useApproveTokenSimulation = (
}, [value, decimals]);

const formattedSpendingCap = useMemo(() => {
return isNFT
? decodedSpendingCap
// formatting coerces small numbers to 0
return isNFT || decodedSpendingCap < 1
? toNonScientificString(decodedSpendingCap)
: new Intl.NumberFormat(locale).format(decodedSpendingCap);
}, [decodedSpendingCap, isNFT, locale]);

Expand Down

0 comments on commit d267b4e

Please sign in to comment.