Skip to content

Commit

Permalink
fix: incorrect standard swap gas fee estimation (#28127)
Browse files Browse the repository at this point in the history
## **Description**

Fix incorrect non-smart gas fee estimations due to the use of an empty
estimated base fee.

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

## **Related issues**

Fixes: #28088 

## **Manual testing steps**

See issue.

## **Screenshots/Recordings**

### **Before**

### **After**

<img width="354" alt="Smart"
src="https://github.com/user-attachments/assets/c1c604d3-9b03-4e4f-b819-9c21f230f789">

<img width="353" alt="Standard"
src="https://github.com/user-attachments/assets/47acd4d1-c618-4d61-afdc-65ce827454a2">

## **Pre-merge author checklist**

- [x] 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).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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
matthewwalsh0 committed Oct 30, 2024
1 parent 8a695e4 commit a6965ec
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 1 deletion.
3 changes: 2 additions & 1 deletion ui/pages/swaps/prepare-swap-page/review-quote.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ export default function ReviewQuote({ setReceiveToAmount }) {
);
const smartTransactionFees = useSelector(getSmartTransactionFees, isEqual);
const swapsNetworkConfig = useSelector(getSwapsNetworkConfig, shallowEqual);
const { estimatedBaseFee = '0' } = useGasFeeEstimates();
const { gasFeeEstimates: networkGasFeeEstimates } = useGasFeeEstimates();
const { estimatedBaseFee = '0' } = networkGasFeeEstimates ?? {};

const gasFeeEstimates = useAsyncResult(async () => {
if (!networkAndAccountSupports1559) {
Expand Down
57 changes: 57 additions & 0 deletions ui/pages/swaps/prepare-swap-page/review-quote.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,26 @@ import {
} from '../../../../test/jest';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { getSwap1559GasFeeEstimates } from '../swaps.util';
import { getNetworkConfigurationByNetworkClientId } from '../../../store/actions';
import ReviewQuote from './review-quote';

jest.mock(
'../../../components/ui/info-tooltip/info-tooltip-icon',
() => () => '<InfoTooltipIcon />',
);

jest.mock('../../../store/actions', () => ({
...jest.requireActual('../../../store/actions'),
getNetworkConfigurationByNetworkClientId: jest.fn(),
}));

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

const ESTIMATED_BASE_FEE_MOCK = '1234';

const middleware = [thunk];
const createProps = (customProps = {}) => {
return {
Expand All @@ -31,6 +39,15 @@ const createProps = (customProps = {}) => {
};

describe('ReviewQuote', () => {
const getNetworkConfigurationByNetworkClientIdMock = jest.mocked(
getNetworkConfigurationByNetworkClientId,
);

beforeEach(() => {
jest.resetAllMocks();
getNetworkConfigurationByNetworkClientIdMock.mockResolvedValue(undefined);
});

const getSwap1559GasFeeEstimatesMock = jest.mocked(
getSwap1559GasFeeEstimates,
);
Expand Down Expand Up @@ -210,5 +227,45 @@ describe('ReviewQuote', () => {
expect(getByText('Max fee:')).toBeInTheDocument();
expect(getByText('$8.15')).toBeInTheDocument();
});

it('extracts estimated base fee from network gas fee estimates', async () => {
getNetworkConfigurationByNetworkClientIdMock.mockResolvedValueOnce({
chainId: CHAIN_IDS.MAINNET,
});

smartDisabled1559State.metamask.gasFeeEstimatesByChainId = {
[CHAIN_IDS.MAINNET]: {
gasFeeEstimates: {
estimatedBaseFee: ESTIMATED_BASE_FEE_MOCK,
},
},
};

getSwap1559GasFeeEstimatesMock.mockResolvedValueOnce({
estimatedBaseFee: '0x1',
tradeGasFeeEstimates: {
maxFeePerGas: '0x2',
maxPriorityFeePerGas: '0x3',
baseAndPriorityFeePerGas: '0x123456789123',
},
approveGasFeeEstimates: undefined,
});

const store = configureMockStore(middleware)(smartDisabled1559State);
const props = createProps();

renderWithProvider(<ReviewQuote {...props} />, store);

await act(() => {
// Intentionally empty
});

expect(getSwap1559GasFeeEstimatesMock).toHaveBeenCalledWith(
expect.any(Object),
null,
ESTIMATED_BASE_FEE_MOCK,
CHAIN_IDS.MAINNET,
);
});
});
});

0 comments on commit a6965ec

Please sign in to comment.