fix: replace last usages of legacy gas fee polling with polling by ne…

…tworkClientId (#24065)

<!--
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**

#23010 made initial
changes to replace the GasFeeController's legacy gas fee polling in
favor of polling via networkClientId. It was discovered that there are
still lingering instances of the legacy gas fee polling which were
causing double polling to occur since the legacy and networkClientId
based polling run on separate loops. This PR fixes this by replacing
those remaining legacy gas fee polling usages.

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

## **Related issues**

Fixes: #23010

## **Manual testing steps**

1. Open background console -> Network tab
2. Open Extension Popup -> send -> enter address and amount -> next
3. Background console network tab should now start showing one single
network request every 10s to the gas api endpoint


## **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 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
- [ ] I’ve included tests if applicable
- [ ] 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.

---------

Co-authored-by: Alex <[email protected]>

app/scripts/metamask-controller.js

@@ -3522,14 +3522,6 @@ export default class MetamaskController extends EventEmitter {
       gasFeeStopPollingByPollingToken:
         gasFeeController.stopPollingByPollingToken.bind(gasFeeController),
 
-      getGasFeeEstimatesAndStartPolling:
-        gasFeeController.getGasFeeEstimatesAndStartPolling.bind(
-          gasFeeController,
-        ),
-
-      disconnectGasFeeEstimatePoller:
-        gasFeeController.disconnectPoller.bind(gasFeeController),
-
       getGasFeeTimeEstimate:
         gasFeeController.getTimeEstimate.bind(gasFeeController),
 
@@ -5687,7 +5679,6 @@ export default class MetamaskController extends EventEmitter {
    */
   onClientClosed() {
     try {
-      this.gasFeeController.stopPolling();
       this.gasFeeController.stopAllPolling();
       this.appStateController.clearPollingTokens();
     } catch (error) {
@@ -5707,7 +5698,6 @@ export default class MetamaskController extends EventEmitter {
     const pollingTokensToDisconnect =
       this.appStateController.store.getState()[appStatePollingTokenType];
     pollingTokensToDisconnect.forEach((pollingToken) => {
-      this.gasFeeController.disconnectPoller(pollingToken);
       this.gasFeeController.stopPollingByPollingToken(pollingToken);
       this.appStateController.removePollingToken(
         pollingToken,

ui/components/app/cancel-speedup-popover/cancel-speedup-popover.test.js

@@ -57,11 +57,7 @@ jest.mock('../../../store/actions', () => ({
       chainId: '0x5',
     }),
   ),
-  disconnectGasFeeEstimatePoller: jest.fn(),
   getGasFeeTimeEstimate: jest.fn().mockImplementation(() => Promise.resolve()),
-  getGasFeeEstimatesAndStartPolling: jest
-    .fn()
-    .mockImplementation(() => Promise.resolve()),
   addPollingTokenToAppState: jest.fn(),
   removePollingTokenFromAppState: jest.fn(),
   updateTransactionGasFees: () => ({ type: 'UPDATE_TRANSACTION_PARAMS' }),

ui/components/app/transaction-list-item/transaction-list-item.component.test.js

@@ -68,7 +68,6 @@ jest.mock('../../../hooks/useGasFeeEstimates', () => ({
 
 setBackgroundConnection({
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
 });
 
 jest.mock('react', () => {

ui/ducks/send/send.js

@@ -46,11 +46,10 @@ import {
   getSelectedInternalAccount,
   getSelectedInternalAccountWithBalance,
   getUnapprovedTransactions,
+  getSelectedNetworkClientId,
 } from '../../selectors';
 import {
-  disconnectGasFeeEstimatePoller,
   displayWarning,
-  getGasFeeEstimatesAndStartPolling,
   hideLoadingIndication,
   showLoadingIndication,
   updateEditableParams,
@@ -64,6 +63,8 @@ import {
   updateTransactionSendFlowHistory,
   getCurrentNetworkEIP1559Compatibility,
   getLayer1GasFee,
+  gasFeeStopPollingByPollingToken,
+  gasFeeStartPollingByNetworkClientId,
 } from '../../store/actions';
 import { setCustomGasLimit } from '../gas/gas.duck';
 import {
@@ -601,6 +602,7 @@ export const initializeSendState = createAsyncThunk(
      */
     const state = thunkApi.getState();
     const isNonStandardEthChain = getIsNonStandardEthChain(state);
+    const selectedNetworkClientId = getSelectedNetworkClientId(state);
     const chainId = getCurrentChainId(state);
     let eip1559support = checkNetworkAndAccountSupports1559(state);
     if (eip1559support === undefined) {
@@ -632,7 +634,9 @@ export const initializeSendState = createAsyncThunk(
     let gasEstimatePollToken = null;
 
     // Instruct the background process that polling for gas prices should begin
-    gasEstimatePollToken = await getGasFeeEstimatesAndStartPolling();
+    gasEstimatePollToken = await gasFeeStartPollingByNetworkClientId(
+      selectedNetworkClientId,
+    );
 
     addPollingTokenToAppState(gasEstimatePollToken);
 
@@ -2284,7 +2288,7 @@ export function resetSendState() {
     dispatch(actions.resetSendState());
 
     if (state[name].gasEstimatePollToken) {
-      await disconnectGasFeeEstimatePoller(state[name].gasEstimatePollToken);
+      await gasFeeStopPollingByPollingToken(state[name].gasEstimatePollToken);
       removePollingTokenFromAppState(state[name].gasEstimatePollToken);
     }
   };

ui/ducks/send/send.test.js

@@ -130,8 +130,8 @@ describe('Send Slice', () => {
       .spyOn(Actions, 'estimateGas')
       .mockImplementation(() => Promise.resolve('0x0'));
     jest
-      .spyOn(Actions, 'getGasFeeEstimatesAndStartPolling')
-      .mockImplementation(() => Promise.resolve());
+      .spyOn(Actions, 'gasFeeStartPollingByNetworkClientId')
+      .mockImplementation(() => Promise.resolve('pollToken'));
     jest
       .spyOn(Actions, 'updateTokenType')
       .mockImplementation(() => Promise.resolve({ isERC721: false }));

ui/pages/confirmations/components/advanced-gas-fee-popover/advanced-gas-fee-input-subtext/advanced-gas-fee-input-subtext.test.js

@@ -14,8 +14,6 @@ jest.mock('../../../../../store/actions', () => ({
       chainId: '0x5',
     }),
   ),
-  disconnectGasFeeEstimatePoller: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn().mockResolvedValue(null),
   addPollingTokenToAppState: jest.fn(),
   removePollingTokenFromAppState: jest.fn(),
 }));

ui/pages/confirmations/components/confirm-page-container/confirm-page-container-container.test.js

@@ -159,10 +159,6 @@ const props = {
 };
 
 jest.mock('../../../../store/actions', () => ({
-  disconnectGasFeeEstimatePoller: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest
-    .fn()
-    .mockImplementation(() => Promise.resolve()),
   getNetworkConfigurationByNetworkClientId: jest.fn().mockImplementation(() => {
     return Promise.resolve({ chainId: '0x5' });
   }),

ui/pages/confirmations/confirm-send-ether/confirm-send-ether.test.js

@@ -20,7 +20,6 @@ setBackgroundConnection({
     }),
   ),
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   promisifiedBackground: jest.fn(),
   tryReverseResolveAddress: jest.fn(),
   getNextNonce: jest.fn(),

ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js

@@ -37,10 +37,10 @@ import TransactionDetailItem from '../components/transaction-detail-item/transac
 import LoadingHeartBeat from '../../../components/ui/loading-heartbeat';
 import LedgerInstructionField from '../components/ledger-instruction-field';
 import {
-  disconnectGasFeeEstimatePoller,
-  getGasFeeEstimatesAndStartPolling,
   addPollingTokenToAppState,
   removePollingTokenFromAppState,
+  gasFeeStartPollingByNetworkClientId,
+  gasFeeStopPollingByPollingToken,
 } from '../../../store/actions';
 
 import { MIN_GAS_LIMIT_DEC } from '../send/send.constants';
@@ -173,6 +173,7 @@ export default class ConfirmTransactionBase extends Component {
     isSmartTransaction: PropTypes.bool,
     smartTransactionsOptInStatus: PropTypes.bool,
     currentChainSupportsSmartTransactions: PropTypes.bool,
+    selectedNetworkClientId: PropTypes.string,
   };
 
   state = {
@@ -929,7 +930,7 @@ export default class ConfirmTransactionBase extends Component {
   _beforeUnloadForGasPolling = () => {
     this._isMounted = false;
     if (this.state.pollingToken) {
-      disconnectGasFeeEstimatePoller(this.state.pollingToken);
+      gasFeeStopPollingByPollingToken(this.state.pollingToken);
       removePollingTokenFromAppState(this.state.pollingToken);
     }
   };
@@ -970,15 +971,17 @@ export default class ConfirmTransactionBase extends Component {
      * This makes a request to get estimates and begin polling, keeping track of the poll
      * token in component state.
      * It then disconnects polling upon componentWillUnmount. If the hook is unmounted
-     * while waiting for `getGasFeeEstimatesAndStartPolling` to resolve, the `_isMounted`
+     * while waiting for `gasFeeStartPollingByNetworkClientId` to resolve, the `_isMounted`
      * flag ensures that a call to disconnect happens after promise resolution.
      */
-    getGasFeeEstimatesAndStartPolling().then((pollingToken) => {
+    gasFeeStartPollingByNetworkClientId(
+      this.props.selectedNetworkClientId,
+    ).then((pollingToken) => {
       if (this._isMounted) {
         addPollingTokenToAppState(pollingToken);
         this.setState({ pollingToken });
       } else {
-        disconnectGasFeeEstimatePoller(pollingToken);
+        gasFeeStopPollingByPollingToken(pollingToken);
         removePollingTokenFromAppState(this.state.pollingToken);
       }
     });

ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.container.js

@@ -54,6 +54,7 @@ import {
   getUnapprovedTransactions,
   getInternalAccountByAddress,
   getApprovedAndSignedTransactions,
+  getSelectedNetworkClientId,
 } from '../../../selectors';
 import {
   getCurrentChainSupportsSmartTransactions,
@@ -134,6 +135,7 @@ const mapStateToProps = (state, ownProps) => {
   } = ownProps;
   const { id: paramsTransactionId } = params;
   const isMainnet = getIsMainnet(state);
+  const selectedNetworkClientId = getSelectedNetworkClientId(state);
 
   ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
   const envType = getEnvironmentType();
@@ -312,6 +314,7 @@ const mapStateToProps = (state, ownProps) => {
     nextNonce,
     mostRecentOverviewPage: getMostRecentOverviewPage(state),
     isMainnet,
+    selectedNetworkClientId,
     isEthGasPrice,
     noGasPrice,
     supportsEIP1559,

ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js

@@ -44,7 +44,6 @@ setBackgroundConnection({
     }),
   ),
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   promisifiedBackground: jest.fn(),
   tryReverseResolveAddress: jest.fn(),
   getNextNonce: jest.fn(),

ui/pages/confirmations/confirm-transaction/confirm-transaction.test.js

@@ -43,9 +43,7 @@ const mockState = {
 
 setBackgroundConnection({
   addPollingTokenToAppState: jest.fn(),
-  disconnectGasFeeEstimatePoller: jest.fn(),
   getContractMethodData: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   removePollingTokenFromAppState: jest.fn(),
   setDefaultHomeActiveTabName: jest.fn(),
 });

ui/pages/confirmations/confirm-transaction/confirm-transaction.transaction.test.js

@@ -1,6 +1,7 @@
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
+import { act } from '@testing-library/react';
 import * as Actions from '../../../store/actions';
 import { renderWithProvider } from '../../../../test/lib/render-helpers';
 import { setBackgroundConnection } from '../../../store/background-connection';
@@ -18,18 +19,21 @@ const middleware = [thunk];
 
 setBackgroundConnection({
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
+  gasFeeStartPollingByNetworkClientId: jest.fn(),
+  gasFeeStopPollingByPollingToken: jest.fn(),
   promisifiedBackground: jest.fn(),
   tryReverseResolveAddress: jest.fn(),
   getNextNonce: jest.fn(),
   addKnownMethodData: jest.fn(),
+  addPollingTokenToAppState: jest.fn(),
+  removePollingTokenFromAppState: jest.fn(),
 });
 
 describe('Confirm Transaction', () => {
   const unapprovedTransactionId = Object.keys(
     mockState.metamask.transactions,
   )[0];
-  it('should render correct information for approve transaction with value', () => {
+  it('should render correct information for approve transaction with value', async () => {
     jest
       .spyOn(Actions, 'gasFeeStartPollingByNetworkClientId')
       .mockResolvedValue(null);
@@ -39,13 +43,19 @@ describe('Confirm Transaction', () => {
         txData: mockState.metamask.transactions[0],
       },
     });
-    const { getByText, getByRole } = renderWithProvider(
-      <ConfirmTransaction actionKey="confirm" />,
-      store,
-      `${CONFIRM_TRANSACTION_ROUTE}/${unapprovedTransactionId}${CONFIRM_SEND_ETHER_PATH}`,
+    let result;
+
+    await act(
+      async () =>
+        (result = renderWithProvider(
+          <ConfirmTransaction actionKey="confirm" />,
+          store,
+          `${CONFIRM_TRANSACTION_ROUTE}/${unapprovedTransactionId}${CONFIRM_SEND_ETHER_PATH}`,
+        )),
     );
-    expect(getByText('0xb19Ac...f0c5e')).toBeInTheDocument();
-    expect(getByRole('button', { name: 'Details' })).toBeInTheDocument();
-    expect(getByRole('button', { name: 'Hex' })).toBeInTheDocument();
+
+    expect(result.getByText('0xb19Ac...f0c5e')).toBeInTheDocument();
+    expect(result.getByRole('button', { name: 'Details' })).toBeInTheDocument();
+    expect(result.getByRole('button', { name: 'Hex' })).toBeInTheDocument();
   });
 });

ui/pages/confirmations/send/send.test.js

@@ -47,7 +47,6 @@ jest.mock('react-router-dom', () => {
 
 setBackgroundConnection({
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   promisifiedBackground: jest.fn(),
 });
 

ui/pages/confirmations/token-allowance/token-allowance.test.js

@@ -138,15 +138,11 @@ const mockShowModal = jest.fn();
 const mockedState = jest.mocked(state);
 
 jest.mock('../../../store/actions', () => ({
-  disconnectGasFeeEstimatePoller: jest.fn(),
   getGasFeeTimeEstimate: jest.fn().mockImplementation(() => Promise.resolve()),
   gasFeeStartPollingByNetworkClientId: jest
     .fn()
     .mockResolvedValue('pollingToken'),
   gasFeeStopPollingByPollingToken: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest
-    .fn()
-    .mockImplementation(() => Promise.resolve()),
   addPollingTokenToAppState: jest.fn(),
   removePollingTokenFromAppState: jest.fn(),
   getNetworkConfigurationByNetworkClientId: jest.fn().mockImplementation(() =>

ui/pages/swaps/fee-card/fee-card.test.js

@@ -52,7 +52,6 @@ const generateUseSelectorRouter = () => (selector) => {
 
 setBackgroundConnection({
   getGasFeeTimeEstimate: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   createTransactionEventFragment: jest.fn(),
 });
 

ui/pages/swaps/prepare-swap-page/review-quote.test.js

@@ -40,7 +40,6 @@ setBackgroundConnection({
   resetPostFetchState: jest.fn(),
   safeRefetchQuotes: jest.fn(),
   setSwapsErrorKey: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   updateTransaction: jest.fn(),
   getGasFeeTimeEstimate: jest.fn(),
   setSwapsQuotesPollingLimitEnabled: jest.fn(),

ui/pages/swaps/view-quote/view-quote.test.js

@@ -48,7 +48,6 @@ setBackgroundConnection({
   resetPostFetchState: jest.fn(),
   safeRefetchQuotes: jest.fn(),
   setSwapsErrorKey: jest.fn(),
-  getGasFeeEstimatesAndStartPolling: jest.fn(),
   updateTransaction: jest.fn(),
   getGasFeeTimeEstimate: jest.fn(),
   setSwapsQuotesPollingLimitEnabled: jest.fn(),

ui/store/actions.ts

@@ -4410,30 +4410,6 @@ export async function updateTokenType(
   return undefined;
 }
 
-/**
- * initiates polling for gas fee estimates.
- *
- * @returns a unique identify of the polling request that can be used
- * to remove that request from consideration of whether polling needs to
- * continue.
- */
-export function getGasFeeEstimatesAndStartPolling(): Promise<string> {
-  return submitRequestToBackground('getGasFeeEstimatesAndStartPolling');
-}
-
-/**
- * Informs the GasFeeController that a specific token is no longer requiring
- * gas fee estimates. If all tokens unsubscribe the controller stops polling.
- *
- * @param pollToken - Poll token received from calling
- * `getGasFeeEstimatesAndStartPolling`.
- */
-export function disconnectGasFeeEstimatePoller(pollToken: string) {
-  return submitRequestToBackground('disconnectGasFeeEstimatePoller', [
-    pollToken,
-  ]);
-}
-
 export async function addPollingTokenToAppState(pollingToken: string) {
   return submitRequestToBackground('addPollingTokenToAppState', [
     pollingToken,