m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-10-22 11:22:43 +02:00
Code

Fix: advance defaults should never be used for swaps (#13617)

Browse Source
This commit is contained in:
Jyoti Puri 2022-02-15 00:37:46 +05:30 committed by ryanml
parent f4a25cb153
commit c0d03ea70a
10 changed files with 111 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
app/scripts/controllers/transactions/index.js
View File

@ -55,6 +55,11 @@ const hstInterface = new ethers.utils.Interface(abi);
const MAX_MEMSTORE_TX_LIST_SIZE = 100; // Number of transactions (by unique nonces) to keep in memory
const SWAP_TRANSACTION_TYPES = [
TRANSACTION_TYPES.SWAP,
TRANSACTION_TYPES.SWAP_APPROVAL,
];
/**
* @typedef {import('../../../../shared/constants/transaction').TransactionMeta} TransactionMeta
* @typedef {import('../../../../shared/constants/transaction').TransactionMetaMetricsEventString} TransactionMetaMetricsEventString
@ -337,9 +342,19 @@ export default class TransactionController extends EventEmitter {
*
* @param txParams
* @param origin
* @param transactionType
* @returns {txMeta}
*/
async addUnapprovedTransaction(txParams, origin) {
async addUnapprovedTransaction(txParams, origin, transactionType) {
if (
transactionType !== undefined &&
!SWAP_TRANSACTION_TYPES.includes(transactionType)
) {
throw new Error(
`TransactionController - invalid transactionType value: ${transactionType}`,
);
}
// validate
const normalizedTxParams = txUtils.normalizeTxParams(txParams);
const eip1559Compatibility = await this.getEIP1559Compatibility();
@ -381,7 +396,7 @@ export default class TransactionController extends EventEmitter {
const { type, getCodeResponse } = await this._determineTransactionType(
txParams,
);
txMeta.type = type;
txMeta.type = transactionType || type;
// ensure value
txMeta.txParams.value = txMeta.txParams.value
@ -444,7 +459,11 @@ export default class TransactionController extends EventEmitter {
if (eip1559Compatibility) {
const { eip1559V2Enabled } = this.preferencesStore.getState();
const advancedGasFeeDefaultValues = this.getAdvancedGasFee();
if (eip1559V2Enabled && Boolean(advancedGasFeeDefaultValues)) {
if (
eip1559V2Enabled &&
Boolean(advancedGasFeeDefaultValues) &&
!SWAP_TRANSACTION_TYPES.includes(txMeta.type)
) {
txMeta.userFeeLevel = CUSTOM_GAS_ESTIMATE;
txMeta.txParams.maxFeePerGas = decGWEIToHexWEI(
advancedGasFeeDefaultValues.maxBaseFee,
@ -461,7 +480,7 @@ export default class TransactionController extends EventEmitter {
// then we set maxFeePerGas and maxPriorityFeePerGas to the suggested gasPrice.
txMeta.txParams.maxFeePerGas = txMeta.txParams.gasPrice;
txMeta.txParams.maxPriorityFeePerGas = txMeta.txParams.gasPrice;
if (eip1559V2Enabled) {
if (eip1559V2Enabled && txMeta.origin !== 'metamask') {
txMeta.userFeeLevel = PRIORITY_LEVELS.DAPP_SUGGESTED;
} else {
txMeta.userFeeLevel = CUSTOM_GAS_ESTIMATE;

4
ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-defaults/advanced-gas-fee-defaults.js
View File

@ -23,7 +23,7 @@ const AdvancedGasFeeDefaults = () => {
const t = useI18nContext();
const dispatch = useDispatch();
const {
hasErrors,
gasErrors,
maxBaseFee,
maxPriorityFeePerGas,
} = useAdvancedGasFeePopoverContext();
@ -86,7 +86,7 @@ const AdvancedGasFeeDefaults = () => {
checked={isDefaultSettingsSelected}
className="advanced-gas-fee-defaults__checkbox"
onClick={handleUpdateDefaultSettings}
disabled={hasErrors}
disabled={gasErrors.maxFeePerGas || gasErrors.maxPriorityFeePerGas}
/>
<Typography variant={TYPOGRAPHY.H7} color={COLORS.UI4} margin={0}>
{isDefaultSettingsSelected

15
ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/base-fee-input/base-fee-input.js
View File

@ -2,7 +2,10 @@ import React, { useCallback, useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
import { HIGH_FEE_WARNING_MULTIPLIER } from '../../../../../pages/send/send.constants';
import { PRIORITY_LEVELS } from '../../../../../../shared/constants/gas';
import {
EDIT_GAS_MODES,
PRIORITY_LEVELS,
} from '../../../../../../shared/constants/gas';
import { SECONDARY } from '../../../../../helpers/constants/common';
import {
bnGreaterThan,
@ -47,7 +50,12 @@ const validateBaseFee = (value, gasFeeEstimates, maxPriorityFeePerGas) => {
const BaseFeeInput = () => {
const t = useI18nContext();
const { gasFeeEstimates, estimateUsed, maxFeePerGas } = useGasFeeContext();
const {
gasFeeEstimates,
estimateUsed,
maxFeePerGas,
editGasMode,
} = useGasFeeContext();
const {
maxPriorityFeePerGas,
setErrorValue,
@ -68,7 +76,8 @@ const BaseFeeInput = () => {
const [baseFee, setBaseFee] = useState(() => {
if (
estimateUsed !== PRIORITY_LEVELS.CUSTOM &&
advancedGasFeeValues?.maxBaseFee
advancedGasFeeValues?.maxBaseFee &&
editGasMode !== EDIT_GAS_MODES.SWAPS
) {
return advancedGasFeeValues.maxBaseFee;
}

28
ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/base-fee-input/basefee-input.test.js → ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/base-fee-input/base-fee-input.test.js
View File

@ -1,7 +1,10 @@
import React from 'react';
import { fireEvent, screen } from '@testing-library/react';
import { GAS_ESTIMATE_TYPES } from '../../../../../../shared/constants/gas';
import {
EDIT_GAS_MODES,
GAS_ESTIMATE_TYPES,
} from '../../../../../../shared/constants/gas';
import { renderWithProvider } from '../../../../../../test/lib/render-helpers';
import mockEstimates from '../../../../../../test/data/mock-estimates.json';
import mockState from '../../../../../../test/data/mock-state.json';
@ -20,7 +23,7 @@ jest.mock('../../../../../store/actions', () => ({
removePollingTokenFromAppState: jest.fn(),
}));
const render = (txProps) => {
const render = (txProps, contextProps) => {
const store = configureStore({
metamask: {
...mockState.metamask,
@ -43,6 +46,7 @@ const render = (txProps) => {
userFeeLevel: 'custom',
...txProps,
}}
{...contextProps}
>
<AdvancedGasFeePopoverContextProvider>
<BaseFeeInput />
@ -56,17 +60,33 @@ describe('BaseFeeInput', () => {
it('should renders advancedGasFee.baseFee value if current estimate used is not custom', () => {
render({
userFeeLevel: 'high',
txParams: {
maxFeePerGas: '0x2E90EDD000',
},
});
expect(document.getElementsByTagName('input')[0]).toHaveValue(100);
});
it('should not advancedGasFee.baseFee value for swaps', () => {
render(
{
userFeeLevel: 'high',
txParams: {
maxFeePerGas: '0x2E90EDD000',
},
},
{ editGasMode: EDIT_GAS_MODES.SWAPS },
);
expect(document.getElementsByTagName('input')[0]).toHaveValue(200);
});
it('should renders baseFee values from transaction if current estimate used is custom', () => {
render({
txParams: {
maxFeePerGas: '0x174876E800',
maxFeePerGas: '0x2E90EDD000',
},
});
expect(document.getElementsByTagName('input')[0]).toHaveValue(100);
expect(document.getElementsByTagName('input')[0]).toHaveValue(200);
});
it('should show current value of estimatedBaseFee in subtext', () => {
render({

9
ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/priority-fee-input/priority-fee-input.js
View File

@ -2,7 +2,10 @@ import React, { useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
import { HIGH_FEE_WARNING_MULTIPLIER } from '../../../../../pages/send/send.constants';
import { PRIORITY_LEVELS } from '../../../../../../shared/constants/gas';
import {
EDIT_GAS_MODES,
PRIORITY_LEVELS,
} from '../../../../../../shared/constants/gas';
import { SECONDARY } from '../../../../../helpers/constants/common';
import { decGWEIToHexWEI } from '../../../../../helpers/utils/conversions.util';
import { getAdvancedGasFeeValues } from '../../../../../selectors';
@ -49,6 +52,7 @@ const PriorityFeeInput = () => {
setMaxPriorityFeePerGas,
} = useAdvancedGasFeePopoverContext();
const {
editGasMode,
estimateUsed,
gasFeeEstimates,
maxPriorityFeePerGas,
@ -63,7 +67,8 @@ const PriorityFeeInput = () => {
const [priorityFee, setPriorityFee] = useState(() => {
if (
estimateUsed !== PRIORITY_LEVELS.CUSTOM &&
advancedGasFeeValues?.priorityFee
advancedGasFeeValues?.priorityFee &&
editGasMode !== EDIT_GAS_MODES.SWAPS
) {
return advancedGasFeeValues.priorityFee;
}

24
ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/priority-fee-input/priority-fee-input.test.js
View File

@ -1,7 +1,10 @@
import React from 'react';
import { fireEvent, screen } from '@testing-library/react';
import { GAS_ESTIMATE_TYPES } from '../../../../../../shared/constants/gas';
import {
EDIT_GAS_MODES,
GAS_ESTIMATE_TYPES,
} from '../../../../../../shared/constants/gas';
import { renderWithProvider } from '../../../../../../test/lib/render-helpers';
import mockEstimates from '../../../../../../test/data/mock-estimates.json';
import mockState from '../../../../../../test/data/mock-state.json';
@ -20,7 +23,7 @@ jest.mock('../../../../../store/actions', () => ({
removePollingTokenFromAppState: jest.fn(),
}));
const render = (txProps) => {
const render = (txProps, contextProps) => {
const store = configureStore({
metamask: {
...mockState.metamask,
@ -43,6 +46,7 @@ const render = (txProps) => {
userFeeLevel: 'custom',
...txProps,
}}
{...contextProps}
>
<AdvancedGasFeePopoverContextProvider>
<PriorityfeeInput />
@ -56,10 +60,26 @@ describe('PriorityfeeInput', () => {
it('should renders advancedGasFee.priorityfee value if current estimate used is not custom', () => {
render({
userFeeLevel: 'high',
txParams: {
maxFeePerGas: '0x2E90EDD000',
},
});
expect(document.getElementsByTagName('input')[0]).toHaveValue(100);
});
it('should not advancedGasFee.baseFee value for swaps', () => {
render(
{
userFeeLevel: 'high',
txParams: {
maxFeePerGas: '0x2E90EDD000',
},
},
{ editGasMode: EDIT_GAS_MODES.SWAPS },
);
expect(document.getElementsByTagName('input')[0]).toHaveValue(200);
});
it('should renders priorityfee value from transaction if current estimate used is custom', () => {
render({
txParams: {

1
ui/components/app/advanced-gas-fee-popover/context/advancedGasFeePopover.js
View File

@ -29,6 +29,7 @@ export const AdvancedGasFeePopoverContextProvider = ({ children }) => {
gasLimit,
hasErrors:
errors.maxFeePerGas || errors.maxPriorityFeePerGas || errors.gasLimit,
gasErrors: errors,
maxFeePerGas,
maxPriorityFeePerGas,
setErrorValue,

2
ui/components/app/edit-gas-fee-popover/edit-gas-item/useGasItemFeeDetails.js
View File

@ -58,7 +58,7 @@ export const useGasItemFeeDetails = (priorityLevel) => {
if (estimateUsed === PRIORITY_LEVELS.CUSTOM) {
maxFeePerGas = maxFeePerGasValue;
maxPriorityFeePerGas = maxPriorityFeePerGasValue;
} else if (advancedGasFeeValues) {
} else if (advancedGasFeeValues && editGasMode !== EDIT_GAS_MODES.SWAPS) {
maxFeePerGas = advancedGasFeeValues.maxBaseFee;
maxPriorityFeePerGas = advancedGasFeeValues.priorityFee;
}

7
ui/ducks/swaps/swaps.js
View File

@ -857,6 +857,7 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => {
addUnapprovedTransaction(
{ ...approveTxParams, amount: '0x0' },
'metamask',
TRANSACTION_TYPES.SWAP_APPROVAL,
),
);
await dispatch(setApproveTxId(approveTxMeta.id));
@ -881,7 +882,11 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => {
}
const tradeTxMeta = await dispatch(
addUnapprovedTransaction(usedTradeTxParams, 'metamask'),
addUnapprovedTransaction(
usedTradeTxParams,
'metamask',
TRANSACTION_TYPES.SWAP,
),
);
dispatch(setTradeTxId(tradeTxMeta.id));

21
ui/store/actions.js
View File

@ -698,18 +698,23 @@ export function updateTransaction(txData, dontShowLoadingIndicator) {
};
}
export function addUnapprovedTransaction(txParams, origin) {
export function addUnapprovedTransaction(txParams, origin, type) {
log.debug('background.addUnapprovedTransaction');
return () => {
return new Promise((resolve, reject) => {
background.addUnapprovedTransaction(txParams, origin, (err, txMeta) => {
if (err) {
reject(err);
return;
}
resolve(txMeta);
});
background.addUnapprovedTransaction(
txParams,
origin,
type,
(err, txMeta) => {
if (err) {
reject(err);
return;
}
resolve(txMeta);
},
);
});
};
}