Skip to content

Commit

Permalink
chore: update gas fee controller to 15.1.2 (#9693)
Browse files Browse the repository at this point in the history
## **Description**

This PR upgrades `gas-fee-controller` from version 13 to version 15.1.2
and fixes the breaking changes associated with the upgrade.

## **Related issues**

Fixes: MetaMask/mobile-planning#1785

## **Manual testing steps**

1. Create a transaction to send tokens from one wallet to another.
Verify that the gas fee looks agreeable.

## **Screenshots/Recordings**
### Regular Transactions
Before:
![Screenshot 2024-05-20 at 6 04
52 PM](https://github.com/MetaMask/metamask-mobile/assets/6249205/2da41805-2fe5-4382-9f3b-c3465f20af7a)

After:
![Screenshot 2024-05-20 at 5 54
26 PM](https://github.com/MetaMask/metamask-mobile/assets/6249205/dd8b5960-0d73-45c2-b0b7-b07cc29b64c2)

### Dapp Transaction
Before: 
![Screenshot 2024-05-21 at 5 59
42 PM](https://github.com/MetaMask/metamask-mobile/assets/6249205/3934e897-fc70-4f51-9c75-41c603aecf0b)

After:
![Screenshot 2024-05-21 at 6 12
47 PM](https://github.com/MetaMask/metamask-mobile/assets/6249205/09e25ef7-4b5b-4cd2-ae16-36ab66953fab)

Reported prices (app vs Etherscan)
ETH Mainnet: .00017 ETH vs 0.00017017
Linea mainnet: < .00001 ETH vs .000001 ETH
Sepolia testnet: .00023 ETH vs 0.00022519

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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: tommasini <tommasini15@gmail.com>
Co-authored-by: Aslau Mario-Daniel <marioaslau@gmail.com>
  • Loading branch information
3 people authored Jun 6, 2024
1 parent cef2d09 commit 31fbfa4
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 30 deletions.
13 changes: 2 additions & 11 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,26 +618,20 @@ class Engine {
networkController.state.selectedNetworkClientId,
);
const gasFeeController = new GasFeeController({
//@ts-expect-error Misalign types because of Base Controller version
messenger: this.controllerMessenger.getRestricted({
name: 'GasFeeController',
allowedActions: [
`${networkController.name}:getNetworkClientById`,
`${networkController.name}:getEIP1559Compatibility`,
`${networkController.name}:getState`,
],
allowedEvents: [AppConstants.NETWORK_DID_CHANGE_EVENT],
}),
getProvider: () =>
// @ts-expect-error at this point in time the provider will be defined by the `networkController.initializeProvider`
networkController.getProviderAndBlockTracker().provider,
onNetworkDidChange: (listener) =>
this.controllerMessenger.subscribe(
AppConstants.NETWORK_DID_CHANGE_EVENT,
listener,
),
getCurrentNetworkEIP1559Compatibility: async () =>
(await networkController.getEIP1559Compatibility()) ?? false,
getChainId: () => networkController.state.providerConfig.chainId,
getCurrentNetworkLegacyGasAPICompatibility: () => {
const chainId = networkController.state.providerConfig.chainId;
return (
Expand All @@ -647,10 +641,7 @@ class Engine {
);
},
clientId: AppConstants.SWAPS.CLIENT_ID,
legacyAPIEndpoint:
'https://gas.api.cx.metamask.io/networks/<chain_id>/gasPrices',
EIP1559APIEndpoint:
'https://gas.api.cx.metamask.io/networks/<chain_id>/suggestedGasFees',
infuraAPIKey: process.env.MM_INFURA_PROJECT_ID || NON_EMPTY,
});

const phishingController = new PhishingController({
Expand Down
98 changes: 98 additions & 0 deletions app/store/migrations/042.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import migration from './042';
import { merge } from 'lodash';
import initialRootState from '../../util/test/initial-root-state';
import { captureException } from '@sentry/react-native';

const oldState = {
engine: {
backgroundState: {
GasFeeController: {
gasFeeEstimates: {},
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'none',
gasFeeEstimatesByChainId: {},
},
},
},
};

const expectedNewState = {
engine: {
backgroundState: {
GasFeeController: {
gasFeeEstimates: {},
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'none',
gasFeeEstimatesByChainId: {},
nonRPCGasFeeApisDisabled: false,
},
},
},
};

jest.mock('@sentry/react-native', () => ({
captureException: jest.fn(),
}));
const mockedCaptureException = jest.mocked(captureException);

describe('Migration #42', () => {
beforeEach(() => {
jest.restoreAllMocks();
jest.resetAllMocks();
});

const invalidStates = [
{
state: merge({}, initialRootState, {
engine: null,
}),
errorMessage:
"FATAL ERROR: Migration 42: Invalid engine state error: 'object'",
scenario: 'engine state is invalid',
},
{
state: merge({}, initialRootState, {
engine: {
backgroundState: null,
},
}),
errorMessage:
"FATAL ERROR: Migration 42: Invalid engine backgroundState error: 'object'",
scenario: 'backgroundState is invalid',
},
{
state: merge({}, initialRootState, {
engine: {
backgroundState: {
GasFeeController: null,
},
},
}),
errorMessage:
"FATAL ERROR: Migration 42: Invalid GasFeeController state error: 'null'",
scenario: 'GasFeeController state is invalid',
},
];

for (const { errorMessage, scenario, state } of invalidStates) {
it(`should capture exception if ${scenario}`, async () => {
const newState = await migration(state);

expect(newState).toStrictEqual(state);
expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error));
expect(mockedCaptureException.mock.calls[0][0].message).toBe(
errorMessage,
);
});
}

it('should contain new property nonRPCGasFeeApisDisabled = false in GasFeeController state ', async () => {
const newState = await migration(oldState);
expect(newState).toStrictEqual(expectedNewState);

expect(
// @ts-expect-error: ignore for testing purposes: new state is type unknown
newState.engine.backgroundState.GasFeeController.nonRPCGasFeeApisDisabled,
).toEqual(false);
});
});
32 changes: 32 additions & 0 deletions app/store/migrations/042.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { captureException } from '@sentry/react-native';
import { isObject } from '@metamask/utils';
import { ensureValidState } from './util';

/**
* Migration to update state of GasFeeController
*
* @param state Persisted Redux state
* @returns
*/
export default function migrate(state: unknown) {
if (!ensureValidState(state, 42)) {
return state;
}

const gasFeeControllerState = state.engine.backgroundState.GasFeeController;

if (!isObject(gasFeeControllerState)) {
captureException(
new Error(
`FATAL ERROR: Migration 42: Invalid GasFeeController state error: '${JSON.stringify(
gasFeeControllerState,
)}'`,
),
);
return state;
}

gasFeeControllerState.nonRPCGasFeeApisDisabled = false;

return state;
}
2 changes: 2 additions & 0 deletions app/store/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import migration38 from './038';
import migration39 from './039';
import migration40 from './040';
import migration41 from './041';
import migration42 from './042';

type MigrationFunction = (state: unknown) => unknown;
type AsyncMigrationFunction = (state: unknown) => Promise<unknown>;
Expand Down Expand Up @@ -96,6 +97,7 @@ export const migrationList: MigrationsList = {
39: migration39,
40: migration40,
41: migration41,
42: migration42,
};

// Enable both synchronous and asynchronous migrations
Expand Down
3 changes: 2 additions & 1 deletion app/util/test/initial-background-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@
"gasFeeEstimates": {},
"estimatedGasFeeTimeBounds": {},
"gasEstimateType": "none",
"gasFeeEstimatesByChainId": {}
"gasFeeEstimatesByChainId": {},
"nonRPCGasFeeApisDisabled": false
},
"ApprovalController": {
"pendingApprovals": {},
Expand Down
1 change: 1 addition & 0 deletions e2e/fixtures/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ class FixtureBuilder {
estimatedGasFeeTimeBounds: {},
gasEstimateType: 'none',
gasFeeEstimatesByChainId: {},
nonRPCGasFeeApisDisabled: false,
},
TokenDetectionController: {},
NftDetectionController: {},
Expand Down
15 changes: 15 additions & 0 deletions e2e/specs/confirmations/advanced-gas-fees.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import WalletActionsModal from '../../pages/modals/WalletActionsModal';
import TestHelpers from '../../helpers';
import Assertions from '../../utils/Assertions';
import { AmountViewSelectorsText } from '../../selectors/SendFlow/AmountView.selectors';
import NetworkListModal from '../../pages/modals/NetworkListModal';
import NetworkEducationModal from '../../pages/modals/NetworkEducationModal';
import { CustomNetworks } from '../../resources/networks.e2e';

const VALID_ADDRESS = '0xebe6CcB6B55e1d094d9c58980Bc10Fed69932cAb';

Expand All @@ -36,8 +39,20 @@ describe(SmokeConfirmations('Advanced Gas Fees and Priority Tests'), () => {

// Check that we are on the wallet screen
await WalletView.isVisible();

await WalletView.tapNetworksButtonOnNavBar();
await TestHelpers.delay(2000);
await NetworkListModal.changeNetworkTo(
CustomNetworks.Sepolia.providerConfig.nickname,
);
await Assertions.checkIfVisible(NetworkEducationModal.container);
await NetworkEducationModal.tapGotItButton();
await Assertions.checkIfNotVisible(NetworkEducationModal.container);

//Tap send Icon
await TestHelpers.delay(2000);
await TabBarComponent.tapActions();
await TestHelpers.delay(2000);
await WalletActionsModal.tapSendButton();

await SendView.inputAddress(VALID_ADDRESS);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
"@metamask/design-tokens": "^2.0.0",
"@metamask/eth-sig-util": "^7.0.2",
"@metamask/etherscan-link": "^2.0.0",
"@metamask/gas-fee-controller": "^13.0.0",
"@metamask/gas-fee-controller": "^15.1.2",
"@metamask/key-tree": "^9.0.0",
"@metamask/keyring-api": "^4.0.0",
"@metamask/keyring-controller": "^16.0.0",
Expand Down
17 changes: 0 additions & 17 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4136,23 +4136,6 @@
is-hex-prefixed "1.0.0"
strip-hex-prefix "1.0.0"

"@metamask/gas-fee-controller@^13.0.0":
version "13.0.2"
resolved "https://registry.yarnpkg.com/@metamask/gas-fee-controller/-/gas-fee-controller-13.0.2.tgz#d8c0efeab91492f9ce96f7eca834bb9bd17fbb62"
integrity sha512-uhjxUToLbON0kUSZUvkDbavM/oGbNHhMV3U76Qf2GCevENiYgZhKNBnUn+KOL1Us8YD2yho2D7G28L+KZa00Wg==
dependencies:
"@metamask/base-controller" "^4.1.1"
"@metamask/controller-utils" "^8.0.4"
"@metamask/eth-query" "^4.0.0"
"@metamask/ethjs-unit" "^0.3.0"
"@metamask/network-controller" "^17.2.1"
"@metamask/polling-controller" "^5.0.1"
"@metamask/utils" "^8.3.0"
"@types/bn.js" "^5.1.5"
"@types/uuid" "^8.3.0"
bn.js "^5.2.1"
uuid "^8.3.2"

"@metamask/gas-fee-controller@^15.1.2":
version "15.1.2"
resolved "https://registry.yarnpkg.com/@metamask/gas-fee-controller/-/gas-fee-controller-15.1.2.tgz#cb8d85906efa1ff42a159ec023287ff31a63e45b"
Expand Down

0 comments on commit 31fbfa4

Please sign in to comment.