Skip to content

Commit

Permalink
refactor: Update controller packages to core v49 (#7125)
Browse files Browse the repository at this point in the history
<!--
Thanks for your contribution!

Please ensure that any applicable requirements below are satisfied
before submitting this pull request. This will help ensure a quick and
efficient review cycle.
-->

**Development & PR Process**
1. Follow MetaMask [Mobile Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/coding_guidelines/CODING_GUIDELINES.md)
2. Add `release-xx` label to identify the PR slated for a upcoming
release (will be used in release discussion)
3. Add `needs-dev-review` label when work is completed
4. Add the appropiate QA label when dev review is completed
    - `needs-qa`: PR requires manual QA.
- `No QA/E2E only`: PR does not require any manual QA effort. Prior to
merging, ensure that you have successful end-to-end test runs in
Bitrise.
- `Spot check on release build`: PR does not require feature QA but
needs non-automated verification. In the description section, provide
test scenarios. Add screenshots, and or recordings of what was tested.
5. Add `QA Passed` label when QA has signed off (Only required if the PR
was labeled with `needs-qa`)
6. Add your team's label, i.e. label starting with `team-` (or
`external-contributor` label if your not a MetaMask employee)

**Description**

The controller packages have been updated to match those present in the
core monorepo release v49. See this changelog for details:
https://github.com/MetaMask/core/releases/tag/v49.0.0

The main breaking change is that the `NetworkController` state property
`network` has been split into `networkId` and `networkStatus`.

**Screenshots/Recordings**

https://recordit.co/GXEdxk9uNp

Wasn't sure exactly what to manually test, but I confirmed that balance
updates and incoming transactions seem to be working well, as well as
network switching.

**Issue**

Resolves MetaMask/mobile-planning#1226

**Checklist**

* [x] There is a related GitHub issue
* [x] Tests are included if applicable
* [x] Any added code is fully documented
  • Loading branch information
Gudahtt authored Sep 26, 2023
1 parent 572b094 commit 3635e72
Show file tree
Hide file tree
Showing 18 changed files with 544 additions and 270 deletions.
2 changes: 0 additions & 2 deletions app/components/UI/ApproveTransactionReview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ import Routes from '../../../constants/navigation/Routes';
import createStyles from './styles';
import {
selectChainId,
selectNetwork,
selectNetworkConfigurations,
selectProviderType,
selectTicker,
Expand Down Expand Up @@ -1226,7 +1225,6 @@ const mapStateToProps = (state) => ({
providerRpcTarget: selectRpcTarget(state),
primaryCurrency: state.settings.primaryCurrency,
activeTabUrl: getActiveTabUrl(state),
networkId: selectNetwork(state),
chainId: selectChainId(state),
tokenList: selectTokenList(state),
isNativeTokenBuySupported: isNetworkBuyNativeTokenSupported(
Expand Down
4 changes: 2 additions & 2 deletions app/components/Views/Asset/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from '../../../reducers/swaps';
import {
selectChainId,
selectNetwork,
selectNetworkId,
selectNetworkConfigurations,
selectRpcTarget,
} from '../../../selectors/networkController';
Expand Down Expand Up @@ -587,7 +587,7 @@ const mapStateToProps = (state) => ({
identities: selectIdentities(state),
chainId: selectChainId(state),
tokens: selectTokens(state),
networkId: selectNetwork(state),
networkId: selectNetworkId(state),
transactions: state.engine.backgroundState.TransactionController.transactions,
rpcTarget: selectRpcTarget(state),
networkConfigurations: selectNetworkConfigurations(state),
Expand Down
6 changes: 3 additions & 3 deletions app/components/Views/TransactionsView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { addAccountTimeFlagFilter } from '../../../util/transactions';
import { toLowerCaseEquals } from '../../../util/general';
import {
selectChainId,
selectNetwork,
selectNetworkId,
selectProviderType,
} from '../../../selectors/networkController';
import {
Expand Down Expand Up @@ -55,11 +55,11 @@ const TransactionsView = ({
const [submittedTxs, setSubmittedTxs] = useState([]);
const [confirmedTxs, setConfirmedTxs] = useState([]);
const [loading, setLoading] = useState();
const networkId = useSelector(selectNetwork);
const networkId = useSelector(selectNetworkId);

const filterTransactions = useCallback(
(networkId) => {
if (networkId === 'loading') return;
if (networkId === null) return;

let accountAddedTimeInsertPointFound = false;
const addedAccountTime = identities[selectedAddress]?.importTime;
Expand Down
20 changes: 10 additions & 10 deletions app/core/BackgroundBridge/BackgroundBridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import WalletConnectPort from './WalletConnectPort';
import Port from './Port';
import {
selectChainId,
selectNetwork,
selectNetworkId,
selectProviderConfig,
selectLegacyNetwork,
} from '../../selectors/networkController';
import { store } from '../../store';

Expand Down Expand Up @@ -70,7 +71,7 @@ export class BackgroundBridge extends EventEmitter {
this.engine = null;

this.chainIdSent = selectChainId(store.getState());
this.networkVersionSent = selectNetwork(store.getState());
this.networkVersionSent = selectNetworkId(store.getState());

// This will only be used for WalletConnect for now
this.addressSent =
Expand Down Expand Up @@ -99,7 +100,7 @@ export class BackgroundBridge extends EventEmitter {

if (this.isRemoteConn) {
const memState = this.getState();
const publicState = this.getProviderNetworkState(memState);
const publicState = this.getProviderNetworkState();
const selectedAddress = memState.selectedAddress;
this.notifyChainChanged(publicState);
this.notifySelectedAddressChanged(selectedAddress);
Expand Down Expand Up @@ -154,7 +155,7 @@ export class BackgroundBridge extends EventEmitter {
});
}

getProviderNetworkState({ network }) {
getProviderNetworkState() {
const providerConfig = selectProviderConfig(store.getState());
const networkType = providerConfig.type;

Expand All @@ -173,7 +174,7 @@ export class BackgroundBridge extends EventEmitter {
}

const result = {
networkVersion: network,
networkVersion: selectLegacyNetwork(store.getState()),
chainId,
};
return result;
Expand All @@ -200,7 +201,7 @@ export class BackgroundBridge extends EventEmitter {
if (!memState) {
memState = this.getState();
}
const publicState = this.getProviderNetworkState(memState);
const publicState = this.getProviderNetworkState();

// Check if update already sent
if (
Expand All @@ -227,10 +228,9 @@ export class BackgroundBridge extends EventEmitter {
}

getProviderState() {
const memState = this.getState();
return {
isUnlocked: this.isUnlocked(),
...this.getProviderNetworkState(memState),
...this.getProviderNetworkState(),
};
}

Expand Down Expand Up @@ -340,11 +340,11 @@ export class BackgroundBridge extends EventEmitter {
*/
getState() {
const vault = Engine.context.KeyringController.state.vault;
const { network, selectedAddress } = Engine.datamodel.flatState;
const { selectedAddress } = Engine.datamodel.flatState;
return {
isInitialized: !!vault,
isUnlocked: true,
network,
network: selectLegacyNetwork(store.getState()),
selectedAddress,
};
}
Expand Down
10 changes: 6 additions & 4 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
NetworkControllerActions,
NetworkControllerEvents,
NetworkState,
NetworkStatus,
} from '@metamask/network-controller';
import {
PhishingController,
Expand Down Expand Up @@ -535,9 +536,9 @@ class Engine {
},
),
new TransactionController({
blockTracker:
networkController.getProviderAndBlockTracker().blockTracker,
getNetworkState: () => networkController.state,
getProvider: () =>
networkController.getProviderAndBlockTracker().provider,
getSelectedAddress: () => preferencesController.state.selectedAddress,
incomingTransactions: {
apiKey: process.env.MM_ETHERSCAN_KEY,
Expand All @@ -554,6 +555,7 @@ class Engine {
AppConstants.NETWORK_STATE_CHANGE_EVENT,
listener,
),
provider: networkController.getProviderAndBlockTracker().provider,
}),
new SwapsController(
{
Expand Down Expand Up @@ -721,9 +723,9 @@ class Engine {

this.controllerMessenger.subscribe(
AppConstants.NETWORK_STATE_CHANGE_EVENT,
(state: { network: string; providerConfig: { chainId: any } }) => {
(state: NetworkState) => {
if (
state.network !== 'loading' &&
state.networkStatus === NetworkStatus.Available &&
state.providerConfig.chainId !== currentChainId
) {
// We should add a state or event emitter saying the provider changed
Expand Down
31 changes: 28 additions & 3 deletions app/selectors/networkController.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { createSelector } from 'reselect';
import { RootState } from '../reducers';
import { ProviderConfig, NetworkState } from '@metamask/network-controller';
import {
ProviderConfig,
NetworkState,
NetworkStatus,
} from '@metamask/network-controller';

const selectNetworkControllerState = (state: RootState) =>
state?.engine?.backgroundState?.NetworkController;
Expand Down Expand Up @@ -32,13 +36,34 @@ export const selectRpcTarget = createSelector(
selectProviderConfig,
(providerConfig: ProviderConfig) => providerConfig.rpcTarget,
);
export const selectNetwork = createSelector(
export const selectNetworkId = createSelector(
selectNetworkControllerState,
(networkControllerState: NetworkState) => networkControllerState?.network,
(networkControllerState: NetworkState) => networkControllerState?.networkId,
);

export const selectNetworkStatus = createSelector(
selectNetworkControllerState,
(networkControllerState: NetworkState) =>
networkControllerState?.networkStatus,
);

export const selectNetworkConfigurations = createSelector(
selectNetworkControllerState,
(networkControllerState: NetworkState) =>
networkControllerState.networkConfigurations,
);

/**
* Derive a value matching the now-removed property `network` that had been
* included in the NetworkController state. It's set to "loading" if the
* network is loading, but otherwise is set to the network ID.
*
* @deprecated Use networkStatus and networkId instead
*/
export const selectLegacyNetwork = createSelector(
selectNetworkControllerState,
(networkControllerState: NetworkState) => {
const { networkId, networkStatus } = networkControllerState;
return networkStatus !== NetworkStatus.Available ? 'loading' : networkId;
},
);
48 changes: 48 additions & 0 deletions app/store/migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { regex } from '../../app/util/regex';

// Generated using this script: https://gist.github.com/Gudahtt/7a8a9e452bd2efdc5ceecd93610a25d3
import ambiguousNetworks from './migration-data/amibiguous-networks.json';
import { NetworkStatus } from '@metamask/network-controller';

export const migrations = {
// Needed after https://github.com/MetaMask/controllers/pull/152
Expand Down Expand Up @@ -695,6 +696,53 @@ export const migrations = {

return state;
},
/**
* Migrate NetworkController state, splitting old `network` property into
* `networkId` and `networkStatus`. This is required to update to v8 of the
* NetworkController package.
*
* @see {@link https://github.com/MetaMask/core/blob/main/packages/network-controller/CHANGELOG.md#800}
*
* Note: the type is wrong here because it conflicts with `redux-persist`
* types, due to a bug in that package.
* See: https://github.com/rt2zz/redux-persist/issues/1065
* TODO: Use `unknown` as the state type, and silence or work around the
* redux-persist bug somehow.
*
* @param {any} state - Redux state.
* @returns Migrated Redux state.
*/
24: (state) => {
const networkControllerState =
state.engine.backgroundState.NetworkController;

if (!isObject(networkControllerState)) {
captureException(
new Error(
`Migration 24: Invalid network controller state: '${typeof networkControllerState}'`,
),
);
return state;
} else if (typeof networkControllerState.network !== 'string') {
captureException(
new Error(
`Migration 24: Invalid network state: '${typeof networkControllerState.network}'`,
),
);
return state;
}

if (networkControllerState.network === 'loading') {
networkControllerState.networkId = null;
networkControllerState.networkStatus = NetworkStatus.Unknown;
} else {
networkControllerState.networkId = networkControllerState.network;
networkControllerState.networkStatus = NetworkStatus.Available;
}
delete networkControllerState.network;

return state;
},
// If you are implementing a migration it will break the migration tests,
// please write a unit for your specific migration version
};
Expand Down
97 changes: 97 additions & 0 deletions app/store/migrations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -802,4 +802,101 @@ describe('Redux Persist Migrations', () => {
);
});
});

describe('#24', () => {
const invalidBackgroundStates = [
{
state: merge({}, initialRootState, {
engine: {
backgroundState: {
NetworkController: null,
},
},
}),
errorMessage:
"Migration 24: Invalid network controller state: 'object'",
scenario: 'network controller state is invalid',
},
{
state: merge({}, initialRootState, {
engine: {
backgroundState: {
NetworkController: { network: null },
},
},
}),
errorMessage: "Migration 24: Invalid network state: 'object'",
scenario: 'network state is invalid',
},
];

for (const { errorMessage, scenario, state } of invalidBackgroundStates) {
it(`should capture exception if ${scenario}`, () => {
const migration = migrations[24];
const newState = migration(cloneDeep(state));

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

it('should migrate loading network state', () => {
const state = {
engine: {
backgroundState: merge({}, initialBackgroundState, {
NetworkController: {
network: 'loading',
},
}),
},
};

const migration = migrations[24];
const newState = migration(state);

expect(newState.engine.backgroundState.NetworkController).toStrictEqual({
networkConfigurations: {},
networkDetails: {
isEIP1559Compatible: false,
},
networkId: null,
networkStatus: 'unknown',
providerConfig: {
chainId: '1',
type: 'mainnet',
},
});
});

it('should migrate non-loading network state', () => {
const state = {
engine: {
backgroundState: merge({}, initialBackgroundState, {
NetworkController: {
network: '1',
},
}),
},
};

const migration = migrations[24];
const newState = migration(state);

expect(newState.engine.backgroundState.NetworkController).toStrictEqual({
networkConfigurations: {},
networkDetails: {
isEIP1559Compatible: false,
},
networkId: '1',
networkStatus: 'available',
providerConfig: {
chainId: '1',
type: 'mainnet',
},
});
});
});
});
1 change: 0 additions & 1 deletion app/util/networks/handleNetworkSwitch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ function setupGetStateMock() {
engine: {
backgroundState: {
NetworkController: {
network: 'loading',
isCustomNetwork: false,
networkConfigurations: {
networkId1: {
Expand Down
Loading

0 comments on commit 3635e72

Please sign in to comment.