From 28093b0c593400161a36c9b17e30e122810e57ad Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 26 Jun 2023 16:13:16 -0230 Subject: [PATCH] Fix fallback gas estimation (#19746) * Fix fallback gas estimation Our fallback gas estimation was failing due to a bug in the `@metamask/controller-utils` package. This was causing gas estimation to fail completely on certain networks (those not supported by our gas estimation APIs and non EIP-1559 compatibile), and it was causing the fallback gas estimation operation (in case our API was down) to fail across all networks. Fixes https://github.com/MetaMask/metamask-extension/issues/19735 * Add e2e tests E2E tests have been added to capture gas estimation. Cases are added for our API, for the fallback estimate, and for non-EIP-1559 estimates. As part of this work, the legacy gas API had to be disabled. This was being used in e2e tests but was dead code in production. It needed to be disabled to ensure the code under test was reachable. * Fix gas API referenced in e2e test * Update unit test snapshots --- app/scripts/metamask-controller.js | 7 +- package.json | 2 +- test/e2e/tests/gas-estimates.spec.js | 171 ++++++++++++++++++ .../confirm-gas-display.test.js.snap | 1 + .../confirm-gas-display.js | 8 +- .../confirm-legacy-gas-display.js | 9 +- .../app/gas-details-item/gas-details-item.js | 7 +- .../transaction-detail-item.component.js | 7 +- .../confirm-send-ether.test.js.snap | 1 + .../confirm-transaction-base.test.js.snap | 1 + .../send-content.component.test.js.snap | 1 + yarn.lock | 12 +- 12 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 test/e2e/tests/gas-estimates.spec.js diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 7b6b049d1..1c848efae 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -535,13 +535,8 @@ export default class MetamaskController extends EventEmitter { ), getCurrentAccountEIP1559Compatibility: this.getCurrentAccountEIP1559Compatibility.bind(this), - legacyAPIEndpoint: `${gasApiBaseUrl}/networks//gasPrices`, EIP1559APIEndpoint: `${gasApiBaseUrl}/networks//suggestedGasFees`, - getCurrentNetworkLegacyGasAPICompatibility: () => { - const { chainId } = - this.networkController.store.getState().providerConfig; - return process.env.IN_TEST || chainId === CHAIN_IDS.MAINNET; - }, + getCurrentNetworkLegacyGasAPICompatibility: () => false, getChainId: () => this.networkController.store.getState().providerConfig.chainId, }); diff --git a/package.json b/package.json index fff0ea2f6..5cb4be44e 100644 --- a/package.json +++ b/package.json @@ -233,7 +233,7 @@ "@metamask/base-controller": "^3.0.0", "@metamask/browser-passworder": "^4.1.0", "@metamask/contract-metadata": "^2.3.1", - "@metamask/controller-utils": "^4.0.0", + "@metamask/controller-utils": "^4.0.1", "@metamask/design-tokens": "^1.9.0", "@metamask/desktop": "^0.3.0", "@metamask/eth-json-rpc-infura": "^8.1.0", diff --git a/test/e2e/tests/gas-estimates.spec.js b/test/e2e/tests/gas-estimates.spec.js new file mode 100644 index 000000000..a7d121f31 --- /dev/null +++ b/test/e2e/tests/gas-estimates.spec.js @@ -0,0 +1,171 @@ +const { + convertToHexValue, + withFixtures, + logInWithBalanceValidation, +} = require('../helpers'); +const FixtureBuilder = require('../fixture-builder'); + +describe('Gas estimates generated by MetaMask', function () { + const baseGanacheOptions = { + accounts: [ + { + secretKey: + '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', + balance: convertToHexValue(25000000000000000000), + }, + ], + }; + const preLondonGanacheOptions = { + ...baseGanacheOptions, + hardfork: 'berlin', + }; + const postLondonGanacheOptions = { + ...baseGanacheOptions, + hardfork: 'london', + }; + + describe('Send on a network that is EIP-1559 compatible', function () { + it('show expected gas defaults', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: postLondonGanacheOptions, + title: this.test.title, + }, + async ({ driver, ganacheServer }) => { + await driver.navigate(); + await logInWithBalanceValidation(driver, ganacheServer); + + await driver.clickElement('[data-testid="eth-overview-send"]'); + + await driver.fill( + 'input[placeholder="Enter public address (0x) or ENS name"]', + '0x2f318C334780961FB129D2a6c30D0763d9a5C970', + ); + + await driver.fill('.unit-input__input', '1'); + + // Check that the gas estimation is what we expect + await driver.findElement({ + cass: '[data-testid="confirm-gas-display"]', + text: '0.00043983', + }); + }, + ); + }); + + it('show expected gas defaults when API is down', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: postLondonGanacheOptions, + testSpecificMock: (mockServer) => { + mockServer + .forGet( + 'https://gas-api.metaswap.codefi.network/networks/1337/suggestedGasFees', + ) + .thenCallback(() => { + return { + json: { + error: 'cannot get gas prices for chain id 1337', + }, + statusCode: 503, + }; + }); + }, + title: this.test.title, + }, + async ({ driver, ganacheServer }) => { + await driver.navigate(); + await logInWithBalanceValidation(driver, ganacheServer); + + await driver.clickElement('[data-testid="eth-overview-send"]'); + + await driver.fill( + 'input[placeholder="Enter public address (0x) or ENS name"]', + '0x2f318C334780961FB129D2a6c30D0763d9a5C970', + ); + + await driver.fill('.unit-input__input', '1'); + + // Check that the gas estimation is what we expect + await driver.findElement({ + cass: '[data-testid="confirm-gas-display"]', + text: '0.00043983', + }); + }, + ); + }); + + it('show expected gas defaults when the network is not supported', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: postLondonGanacheOptions, + testSpecificMock: (mockServer) => { + mockServer + .forGet( + 'https://gas-api.metaswap.codefi.network/networks/1337/suggestedGasFees', + ) + .thenCallback(() => { + return { + statusCode: 422, + }; + }); + }, + title: this.test.title, + }, + async ({ driver, ganacheServer }) => { + await driver.navigate(); + await logInWithBalanceValidation(driver, ganacheServer); + + await driver.clickElement('[data-testid="eth-overview-send"]'); + + await driver.fill( + 'input[placeholder="Enter public address (0x) or ENS name"]', + '0x2f318C334780961FB129D2a6c30D0763d9a5C970', + ); + + await driver.fill('.unit-input__input', '1'); + + // Check that the gas estimation is what we expect + await driver.findElement({ + cass: '[data-testid="confirm-gas-display"]', + text: '0.00043983', + }); + }, + ); + }); + }); + + describe('Send on a network that is not EIP-1559 compatible', function () { + it('show expected gas defaults', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: preLondonGanacheOptions, + title: this.test.title, + }, + async ({ driver, ganacheServer }) => { + await driver.navigate(); + await logInWithBalanceValidation(driver, ganacheServer); + + await driver.clickElement('[data-testid="eth-overview-send"]'); + + await driver.fill( + 'input[placeholder="Enter public address (0x) or ENS name"]', + '0x2f318C334780961FB129D2a6c30D0763d9a5C970', + ); + + await driver.fill('.unit-input__input', '1'); + + // Check that the gas estimation is what we expect + await driver.findElement({ + cass: '[data-testid="confirm-gas-display"]', + text: '0.000042', + }); + }, + ); + }); + }); +}); diff --git a/ui/components/app/confirm-gas-display/__snapshots__/confirm-gas-display.test.js.snap b/ui/components/app/confirm-gas-display/__snapshots__/confirm-gas-display.test.js.snap index a993864ba..edd5c141f 100644 --- a/ui/components/app/confirm-gas-display/__snapshots__/confirm-gas-display.test.js.snap +++ b/ui/components/app/confirm-gas-display/__snapshots__/confirm-gas-display.test.js.snap @@ -4,6 +4,7 @@ exports[`ConfirmGasDisplay should match snapshot 1`] = `
{ checkNetworkAndAccountSupports1559, ); const supportsEIP1559 = networkAndAccountSupports1559 && !isLegacyTxn; + const dataTestId = 'confirm-gas-display'; return supportsEIP1559 ? ( - + ) : ( - + ); }; diff --git a/ui/components/app/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.js b/ui/components/app/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.js index 124fbb0d9..15e0abfb6 100644 --- a/ui/components/app/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.js +++ b/ui/components/app/confirm-gas-display/confirm-legacy-gas-display/confirm-legacy-gas-display.js @@ -1,4 +1,5 @@ import React from 'react'; +import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; import { useI18nContext } from '../../../../hooks/useI18nContext'; @@ -31,7 +32,7 @@ import { Icon, IconName } from '../../../component-library'; const renderHeartBeatIfNotInTest = () => process.env.IN_TEST ? null : ; -const ConfirmLegacyGasDisplay = () => { +const ConfirmLegacyGasDisplay = ({ 'data-testid': dataTestId } = {}) => { const t = useI18nContext(); // state selectors @@ -55,6 +56,7 @@ const ConfirmLegacyGasDisplay = () => { return [ { return ( @@ -186,4 +189,8 @@ const ConfirmLegacyGasDisplay = () => { ); }; +ConfirmLegacyGasDisplay.propTypes = { + 'data-testid': PropTypes.string, +}; + export default ConfirmLegacyGasDisplay; diff --git a/ui/components/app/gas-details-item/gas-details-item.js b/ui/components/app/gas-details-item/gas-details-item.js index 10e8ca459..7d5afd287 100644 --- a/ui/components/app/gas-details-item/gas-details-item.js +++ b/ui/components/app/gas-details-item/gas-details-item.js @@ -24,7 +24,10 @@ import { useDraftTransactionWithTxParams } from '../../../hooks/useDraftTransact import { PriorityLevels } from '../../../../shared/constants/gas'; import GasDetailsItemTitle from './gas-details-item-title'; -const GasDetailsItem = ({ userAcknowledgedGasMissing = false }) => { +const GasDetailsItem = ({ + 'data-testid': dataTestId, + userAcknowledgedGasMissing = false, +}) => { const t = useI18nContext(); const draftTransaction = useSelector(getCurrentDraftTransaction); const transactionData = useDraftTransactionWithTxParams(); @@ -64,6 +67,7 @@ const GasDetailsItem = ({ userAcknowledgedGasMissing = false }) => { return ( } detailTitleColor={TextColor.textDefault} detailText={ @@ -137,6 +141,7 @@ const GasDetailsItem = ({ userAcknowledgedGasMissing = false }) => { }; GasDetailsItem.propTypes = { + 'data-testid': PropTypes.string, userAcknowledgedGasMissing: PropTypes.bool, }; diff --git a/ui/components/app/transaction-detail-item/transaction-detail-item.component.js b/ui/components/app/transaction-detail-item/transaction-detail-item.component.js index a90bad16a..ed1578b62 100644 --- a/ui/components/app/transaction-detail-item/transaction-detail-item.component.js +++ b/ui/components/app/transaction-detail-item/transaction-detail-item.component.js @@ -14,6 +14,7 @@ import { } from '../../../helpers/constants/design-system'; export default function TransactionDetailItem({ + 'data-testid': dataTestId, detailTitle = '', detailText, detailTitleColor = Color.textDefault, @@ -24,7 +25,7 @@ export default function TransactionDetailItem({ flexWidthValues = false, }) { return ( -
+