Skip to content

Commit

Permalink
fix: fix approve flow on swap (#11532)
Browse files Browse the repository at this point in the history 
## **Description**

PR to fix the approve flow when users swap tokens.

## **Related issues**

Fixes: #10834

## **Manual testing steps**

1. Switch to base network
2. Swap WETH against any other token, exp USDC
3. See approval screen; it should have the buttons enabled and you
should be able to see token icon and spending cap

## **Screenshots/Recordings**

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

### **Before**

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


https://github.com/user-attachments/assets/d8be6306-be74-4c6c-8463-567f35ee891e


### **After**

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


https://github.com/user-attachments/assets/8382cc83-e4e7-42ce-b913-fcac7e906d4b


## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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
@sahar-fehri
sahar-fehri authored Oct 10, 2024
1 parent a133d1c commit b0ef1a7
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 9 deletions.
2 changes: 1 addition & 1 deletion ...Views/confirmations/components/ApproveTransactionReview/__snapshots__/index.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ exports[`ApproveTransactionModal render matches snapshot 1`] = `
},
]
}
testID=""
testID="Confirm"
>
<Text
style={
Expand Down
16 changes: 8 additions & 8 deletions app/components/Views/confirmations/components/ApproveTransactionReview/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { getApproveNavbar } from '../../../../UI/Navbar';
import { connect } from 'react-redux';
import { getHost } from '../../../../../util/browser';
import {
safeToChecksumAddress,
getAddressAccountType,
getTokenDetails,
shouldShowBlockExplorer,
Expand Down Expand Up @@ -389,7 +388,10 @@ class ApproveTransactionReview extends PureComponent {
decodeApproveData(data);
const encodedDecimalAmount = hexToBN(encodedHexAmount).toString();

const contract = tokenList[safeToChecksumAddress(to)];
// The tokenList addresses we get from state are not checksum addresses
// also, the tokenList we get does not contain the tokenStandard, so even if the token exists in tokenList we will
// need to fetch it using getTokenDetails
const contract = tokenList[to];
if (tokenAllowanceState) {
const {
tokenSymbol: symbol,
Expand All @@ -405,7 +407,7 @@ class ApproveTransactionReview extends PureComponent {
tokenBalance = balance;
tokenStandard = standard;
createdSpendCap = isReadyToApprove;
} else if (!contract) {
} else {
try {
const result = await getTokenDetails(to, from, encodedDecimalAmount);

Expand All @@ -428,12 +430,9 @@ class ApproveTransactionReview extends PureComponent {
);
}
} catch (e) {
tokenSymbol = 'ERC20 Token';
tokenDecimals = 18;
tokenSymbol = contract?.symbol || 'ERC20 Token';
tokenDecimals = contract?.decimals || 18;
}
} else {
tokenSymbol = contract.symbol;
tokenDecimals = contract.decimals;
}

const approveAmount = fromTokenMinimalUnit(
Expand Down Expand Up @@ -895,6 +894,7 @@ class ApproveTransactionReview extends PureComponent {
onConfirmPress={this.onConfirmPress}
confirmDisabled={shouldDisableConfirmButton}
confirmButtonState={this.getConfirmButtonState()}
confirmTestID="Confirm"
>
<View style={styles.actionViewChildren}>
<ScrollView nestedScrollEnabled>
Expand Down
142 changes: 142 additions & 0 deletions app/components/Views/confirmations/components/ApproveTransactionReview/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ import ApproveTransactionModal from '.';
import { backgroundState } from '../../../../../util/test/initial-root-state';
import { renderScreen } from '../../../../../util/test/renderWithProvider';
import { SET_APPROVAL_FOR_ALL_SIGNATURE } from '../../../../../util/transactions';
import { cloneDeep } from 'lodash';
import { fireEvent, waitFor } from '@testing-library/react-native';

import {
getTokenDetails,
} from '../../../../../util/address';

jest.mock('../../../../../util/address', () => ({
...jest.requireActual('../../../../../util/address'),
getTokenDetails: jest.fn()
}));



jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
Expand All @@ -19,6 +32,11 @@ jest.mock('../../../../../core/Engine', () => ({
KeyringController: {
getOrAddQRKeyring: async () => ({ subscribe: () => ({}) }),
},
AssetsContractController: {
getERC20BalanceOf: jest
.fn()
.mockResolvedValue(0x0186a0),
}
},
controllerMessenger: {
subscribe: jest.fn(),
Expand Down Expand Up @@ -74,4 +92,128 @@ describe('ApproveTransactionModal', () => {
);
expect(toJSON()).toMatchSnapshot();
});

it('Approve button is enabled when standard is defined', async () => {
const mockGetTokenDetails = getTokenDetails as jest.Mock;
mockGetTokenDetails.mockReturnValue({
standard: 'ERC20'
});
const state = cloneDeep(initialState);
state.engine.backgroundState.AccountTrackerController.accounts = [];
state.engine.backgroundState.TokenListController = {
tokenList: {
'0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f': {
address: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
symbol: 'SNX',
decimals: 18,
name: 'Synthetix Network Token',
iconUrl:
'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png',
type: 'erc20',
aggregators: [
'Aave',
],
occurrences: 10,
fees: {
'0x5fd79d46eba7f351fe49bff9e87cdea6c821ef9f': 0,
'0xda4ef8520b1a57d7d63f1e249606d1a459698876': 0,
},
},
}
};

state.transaction = {
to: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
origin: 'test-dapp',
chainId: '0x1',
txParams: {
to: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
from: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
data,
origin: 'test-dapp',
},
data,
};
const mockOnConfirm = jest.fn();
const {getByTestId } = renderScreen(

() => (
// eslint-disable-next-line react/react-in-jsx-scope
<ApproveTransactionModal
onConfirm={mockOnConfirm}
/>
),
{ name: 'Approve' },
{ state },
);

expect(mockGetTokenDetails).toHaveBeenCalled();
fireEvent.press(getByTestId('Confirm'));
expect(mockOnConfirm).toHaveBeenCalled();

await waitFor(() => {
const isDisabled = getByTestId('Confirm').props.disabled;
expect(isDisabled).toBe(false);
});
});

it('Approve button is disabled when standard is undefined', async () => {
const mockGetTokenDetails = getTokenDetails as jest.Mock;
mockGetTokenDetails.mockReturnValue({});
const state = cloneDeep(initialState);
state.engine.backgroundState.AccountTrackerController.accounts = [];
state.engine.backgroundState.TokenListController = {
tokenList: {
'0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f': {
address: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
symbol: 'SNX',
decimals: 18,
name: 'Synthetix Network Token',
iconUrl:
'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png',
type: 'erc20',
aggregators: [
'Aave',
],
occurrences: 10,
fees: {
'0x5fd79d46eba7f351fe49bff9e87cdea6c821ef9f': 0,
'0xda4ef8520b1a57d7d63f1e249606d1a459698876': 0,
},
},
}
};

state.transaction = {
to: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
origin: 'test-dapp',
chainId: '0x1',
txParams: {
to: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
from: '0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f',
data,
origin: 'test-dapp',
},
data,
};
const mockOnConfirm = jest.fn();
const {getByTestId } = renderScreen(

() => (
// eslint-disable-next-line react/react-in-jsx-scope
<ApproveTransactionModal
onConfirm={mockOnConfirm}
/>
),
{ name: 'Approve' },
{ state },
);

expect(mockGetTokenDetails).toHaveBeenCalled();
await waitFor(() => {
const isDisabled = getByTestId('Confirm').props.disabled;
expect(isDisabled).toBe(true);
});
});

});

0 comments on commit b0ef1a7

Please sign in to comment.