From a6965ecac5b06d6f87f8d8819007cca48e248f20 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 29 Oct 2024 14:40:07 +0000 Subject: [PATCH] fix: incorrect standard swap gas fee estimation (#28127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **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** Smart Standard ## **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. --- .../swaps/prepare-swap-page/review-quote.js | 3 +- .../prepare-swap-page/review-quote.test.js | 57 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/ui/pages/swaps/prepare-swap-page/review-quote.js b/ui/pages/swaps/prepare-swap-page/review-quote.js index 13d11a93cd1f..680ece113f5d 100644 --- a/ui/pages/swaps/prepare-swap-page/review-quote.js +++ b/ui/pages/swaps/prepare-swap-page/review-quote.js @@ -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) { diff --git a/ui/pages/swaps/prepare-swap-page/review-quote.test.js b/ui/pages/swaps/prepare-swap-page/review-quote.test.js index cacd52ca47ed..1e4ab9199226 100644 --- a/ui/pages/swaps/prepare-swap-page/review-quote.test.js +++ b/ui/pages/swaps/prepare-swap-page/review-quote.test.js @@ -10,6 +10,7 @@ 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( @@ -17,11 +18,18 @@ jest.mock( () => () => '', ); +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 { @@ -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, ); @@ -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(, store); + + await act(() => { + // Intentionally empty + }); + + expect(getSwap1559GasFeeEstimatesMock).toHaveBeenCalledWith( + expect.any(Object), + null, + ESTIMATED_BASE_FEE_MOCK, + CHAIN_IDS.MAINNET, + ); + }); }); });