Skip to content

Commit

Permalink
Replace useSafeGasEstimatePolling with usePolling (#23010)
Browse files Browse the repository at this point in the history
## **Description**
Added `usePolling` hook and used it within `useSafeGasEstimatePolling`
hook.

This replaces where gas estimates are coming from for the confirmation
flow.

## **Related issues**

Fixes: MetaMask/MetaMask-planning#2014

## **Manual testing steps**

1. Hit send button
2. get to confirmation screen
3. see that gas estimation updates

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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: Shane Jonas <[email protected]>
Co-authored-by: Alex Donesky <[email protected]>
  • Loading branch information
3 people authored Mar 8, 2024
1 parent 6c810aa commit 1f89a65
Show file tree
Hide file tree
Showing 54 changed files with 1,949 additions and 1,262 deletions.
10 changes: 10 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2941,6 +2941,10 @@ export default class MetamaskController extends EventEmitter {
this.networkController.getEIP1559Compatibility.bind(
this.networkController,
),
getNetworkConfigurationByNetworkClientId:
this.networkController.getNetworkConfigurationByNetworkClientId.bind(
this.networkController,
),
// PreferencesController
setSelectedAddress: (address) => {
const account = this.accountsController.getAccountByAddress(address);
Expand Down Expand Up @@ -3421,6 +3425,12 @@ export default class MetamaskController extends EventEmitter {
),

// GasFeeController
gasFeeStartPollingByNetworkClientId:
gasFeeController.startPollingByNetworkClientId.bind(gasFeeController),

gasFeeStopPollingByPollingToken:
gasFeeController.stopPollingByPollingToken.bind(gasFeeController),

getGasFeeEstimatesAndStartPolling:
gasFeeController.getGasFeeEstimatesAndStartPolling.bind(
gasFeeController,
Expand Down
1 change: 0 additions & 1 deletion development/ts-migration-dashboard/files-to-convert.json
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,6 @@
"ui/hooks/useIncrementedGasFees.js",
"ui/hooks/useOriginMetadata.js",
"ui/hooks/usePrevious.js",
"ui/hooks/useSafeGasEstimatePolling.js",
"ui/hooks/useSegmentContext.js",
"ui/hooks/useShouldAnimateGasEstimations.js",
"ui/hooks/useShouldShowSpeedUp.js",
Expand Down
37 changes: 37 additions & 0 deletions test/data/mock-send-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"importNftsModal": { "open": false },
"gasIsLoading": false,
"isLoading": false,
"importTokensModalOpen": false,
"modal": {
"open": false,
"modalState": {
Expand All @@ -24,6 +25,9 @@
"name": null
}
},
"showIpfsModalOpen": false,
"showKeyringRemovalSnapModal": false,
"showWhatsNewPopup": false,
"warning": null,
"alertOpen": false
},
Expand Down Expand Up @@ -90,13 +94,46 @@
"priorityFeeTrend": "down",
"networkCongestion": 0.90625
},
"gasFeeEstimatesByChainId": {
"0x5": {
"gasEstimateType": "fee-market",
"gasFeeEstimates": {
"low": {
"minWaitTimeEstimate": 180000,
"maxWaitTimeEstimate": 300000,
"suggestedMaxPriorityFeePerGas": "3",
"suggestedMaxFeePerGas": "53"
},
"medium": {
"minWaitTimeEstimate": 15000,
"maxWaitTimeEstimate": 60000,
"suggestedMaxPriorityFeePerGas": "7",
"suggestedMaxFeePerGas": "70"
},
"high": {
"minWaitTimeEstimate": 0,
"maxWaitTimeEstimate": 15000,
"suggestedMaxPriorityFeePerGas": "10",
"suggestedMaxFeePerGas": "100"
},
"estimatedBaseFee": "50",
"historicalBaseFeeRange": ["28.533098435", "70.351148354"],
"baseFeeTrend": "up",
"latestPriorityFeeRange": ["1", "40"],
"historicalPriorityFeeRange": ["0.1458417", "700.156384646"],
"priorityFeeTrend": "down",
"networkCongestion": 0.90625
}
}
},
"snaps": [{}],
"preferences": {
"hideZeroBalanceTokens": false,
"showFiatInTestnets": false,
"showTestNetworks": true,
"useNativeCurrencyAsPrimaryCurrency": true
},
"seedPhraseBackedUp": null,
"ensResolutionsByAddress": {},
"isAccountMenuOpen": false,
"isUnlocked": true,
Expand Down
32 changes: 32 additions & 0 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,38 @@
"participateInMetaMetrics": false,
"gasEstimateType": "fee-market",
"showBetaHeader": false,
"gasFeeEstimatesByChainId": {
"0x5": {
"gasEstimateType": "fee-market",
"gasFeeEstimates": {
"low": {
"minWaitTimeEstimate": 180000,
"maxWaitTimeEstimate": 300000,
"suggestedMaxPriorityFeePerGas": "3",
"suggestedMaxFeePerGas": "53"
},
"medium": {
"minWaitTimeEstimate": 15000,
"maxWaitTimeEstimate": 60000,
"suggestedMaxPriorityFeePerGas": "7",
"suggestedMaxFeePerGas": "70"
},
"high": {
"minWaitTimeEstimate": 0,
"maxWaitTimeEstimate": 15000,
"suggestedMaxPriorityFeePerGas": "10",
"suggestedMaxFeePerGas": "100"
},
"estimatedBaseFee": "50",
"historicalBaseFeeRange": ["28.533098435", "70.351148354"],
"baseFeeTrend": "up",
"latestPriorityFeeRange": ["1", "40"],
"historicalPriorityFeeRange": ["0.1458417", "700.156384646"],
"priorityFeeTrend": "down",
"networkCongestion": 0.90625
}
}
},
"permissionHistory": {},
"gasFeeEstimates": {
"low": {
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/snaps/test-snap-txinsights-v2.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ describe('Test Snap TxInsights-v2', function () {
WINDOW_TITLES.Dialog,
windowHandles,
);

await driver.findClickableElement({
text: 'Confirm',
tag: 'button',
});

await driver.waitForSelector({
text: 'Insights Example Snap',
tag: 'button',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ const MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_HEX_WEI =
MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_BN_WEI.toString(16);

jest.mock('../../../store/actions', () => ({
gasFeeStartPollingByNetworkClientId: jest
.fn()
.mockResolvedValue('pollingToken'),
gasFeeStopPollingByPollingToken: jest.fn(),
getNetworkConfigurationByNetworkClientId: jest.fn().mockImplementation(() =>
Promise.resolve({
chainId: '0x5',
}),
),
disconnectGasFeeEstimatePoller: jest.fn(),
getGasFeeTimeEstimate: jest.fn().mockImplementation(() => Promise.resolve()),
getGasFeeEstimatesAndStartPolling: jest
Expand Down Expand Up @@ -85,6 +94,14 @@ const render = (
featureFlags: { advancedInlineGas: true },
gasFeeEstimates:
mockEstimates[GasEstimateTypes.feeMarket].gasFeeEstimates,
gasFeeEstimatesByChainId: {
...mockState.metamask.gasFeeEstimatesByChainId,
'0x5': {
...mockState.metamask.gasFeeEstimatesByChainId['0x5'],
gasFeeEstimates:
mockEstimates[GasEstimateTypes.feeMarket].gasFeeEstimates,
},
},
},
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { waitFor } from '@testing-library/react';
import { act, waitFor } from '@testing-library/react';
import { TransactionStatus } from '@metamask/transaction-controller';
import { GAS_LIMITS } from '../../../../shared/constants/gas';
import { renderWithProvider } from '../../../../test/lib/render-helpers';
Expand All @@ -10,8 +10,13 @@ import TransactionListItemDetails from '.';

jest.mock('../../../store/actions.ts', () => ({
tryReverseResolveAddress: () => jest.fn(),
getGasFeeEstimatesAndStartPolling: jest.fn().mockResolvedValue(),
addPollingTokenToAppState: jest.fn(),
gasFeeStartPollingByNetworkClientId: jest
.fn()
.mockResolvedValue('pollingToken'),
gasFeeStopPollingByPollingToken: jest.fn(),
getNetworkConfigurationByNetworkClientId: jest
.fn()
.mockResolvedValue({ chainId: '0x5' }),
}));

let mockGetCustodianTransactionDeepLink = jest.fn();
Expand All @@ -22,34 +27,34 @@ jest.mock('../../../store/institutional/institution-background', () => ({
}),
}));

describe('TransactionListItemDetails Component', () => {
const transaction = {
history: [],
id: 1,
status: TransactionStatus.confirmed,
txParams: {
from: '0x1',
gas: GAS_LIMITS.SIMPLE,
gasPrice: '0x3b9aca00',
nonce: '0xa4',
to: '0x2',
value: '0x2386f26fc10000',
},
metadata: {
note: 'some note',
},
custodyId: '1',
};

const transactionGroup = {
transactions: [transaction],
primaryTransaction: transaction,
initialTransaction: transaction,
const transaction = {
history: [],
id: 1,
status: TransactionStatus.confirmed,
txParams: {
from: '0x1',
gas: GAS_LIMITS.SIMPLE,
gasPrice: '0x3b9aca00',
nonce: '0xa4',
hasRetried: false,
hasCancelled: false,
};

to: '0x2',
value: '0x2386f26fc10000',
},
metadata: {
note: 'some note',
},
custodyId: '1',
};

const transactionGroup = {
transactions: [transaction],
primaryTransaction: transaction,
initialTransaction: transaction,
nonce: '0xa4',
hasRetried: false,
hasCancelled: false,
};

const render = async (overrideProps) => {
const rpcPrefs = {
blockExplorerUrl: 'https://customblockexplorer.com/',
};
Expand All @@ -69,70 +74,52 @@ describe('TransactionListItemDetails Component', () => {
transactionStatus: () => <div></div>,
blockExplorerLinkText,
rpcPrefs,
...overrideProps,
};

it('should render title with title prop', async () => {
const mockStore = configureMockStore([thunk])(mockState);
const mockStore = configureMockStore([thunk])(mockState);

let result;

const { queryByText } = renderWithProvider(
<TransactionListItemDetails {...props} />,
mockStore,
);
await act(
async () =>
(result = renderWithProvider(
<TransactionListItemDetails {...props} />,
mockStore,
)),
);

return result;
};

describe('TransactionListItemDetails Component', () => {
it('should render title with title prop', async () => {
const { queryByText } = await render();

await waitFor(() => {
expect(queryByText(props.title)).toBeInTheDocument();
expect(queryByText('Test Transaction Details')).toBeInTheDocument();
});
});

describe('Retry button', () => {
it('should render retry button with showRetry prop', () => {
const retryProps = {
...props,
showRetry: true,
};

const mockStore = configureMockStore([thunk])(mockState);

const { queryByTestId } = renderWithProvider(
<TransactionListItemDetails {...retryProps} />,
mockStore,
);
it('should render retry button with showRetry prop', async () => {
const { queryByTestId } = await render({ showRetry: true });

expect(queryByTestId('rety-button')).toBeInTheDocument();
});
});

describe('Cancel button', () => {
it('should render cancel button with showCancel prop', () => {
const retryProps = {
...props,
showCancel: true,
};

const mockStore = configureMockStore([thunk])(mockState);

const { queryByTestId } = renderWithProvider(
<TransactionListItemDetails {...retryProps} />,
mockStore,
);
it('should render cancel button with showCancel prop', async () => {
const { queryByTestId } = await render({ showCancel: true });

expect(queryByTestId('cancel-button')).toBeInTheDocument();
});
});

describe('Speedup button', () => {
it('should render speedup button with showSpeedUp prop', () => {
const retryProps = {
...props,
showSpeedUp: true,
};

const mockStore = configureMockStore([thunk])(mockState);

const { queryByTestId } = renderWithProvider(
<TransactionListItemDetails {...retryProps} />,
mockStore,
);
it('should render speedup button with showSpeedUp prop', async () => {
const { queryByTestId } = await render({ showSpeedUp: true });

expect(queryByTestId('speedup-button')).toBeInTheDocument();
});
Expand All @@ -144,9 +131,7 @@ describe('TransactionListItemDetails Component', () => {
.fn()
.mockReturnValue({ url: 'https://url.com' });

const mockStore = configureMockStore([thunk])(mockState);

renderWithProvider(<TransactionListItemDetails {...props} />, mockStore);
await render({ showCancel: true });

await waitFor(() => {
const custodianViewButton = document.querySelector(
Expand All @@ -173,15 +158,10 @@ describe('TransactionListItemDetails Component', () => {
primaryTransaction: newTransaction,
initialTransaction: newTransaction,
};
const mockStore = configureMockStore([thunk])(mockState);

const { queryByText } = renderWithProvider(
<TransactionListItemDetails
{...props}
transactionGroup={newTransactionGroup}
/>,
mockStore,
);
const { queryByText } = await render({
transactionGroup: newTransactionGroup,
});

await waitFor(() => {
expect(queryByText('some note')).toBeInTheDocument();
Expand Down
Loading

0 comments on commit 1f89a65

Please sign in to comment.