Skip to content

Commit

Permalink
feat: multichain currency rate polling (#12268)
Browse files Browse the repository at this point in the history
## **Description**

This PR uses the `CurrencyRateController` to poll native currency prices
across chains. This will keep prices updated in state across all
networks.

Because we're polling across all networks, it's no longer necessary to
trigger updates when switching chains, so these places have been
removed. Once we start showing tokens across all networks, "switching
chains" will become a less relevant action.

## **Related issues**


## **Manual testing steps**

1. Verify fiat prices are correct for the native currency on a network
2. Switch to a network with a different native currency, 
3. Verify fiat prices are correct for that network

## **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 Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
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.
  • Loading branch information
bergeron authored Nov 14, 2024
1 parent 4fa0c15 commit 2792684
Show file tree
Hide file tree
Showing 16 changed files with 111 additions and 68 deletions.
4 changes: 1 addition & 3 deletions app/components/UI/NetworkModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,10 @@ const NetworkModals = (props: NetworkProps) => {
};

const switchNetwork = async () => {
const { NetworkController, CurrencyRateController } = Engine.context;
const { NetworkController } = Engine.context;
const url = new URLPARSE(rpcUrl);
const existingNetwork = networkConfigurationByChainId[chainId];

CurrencyRateController.updateExchangeRate([ticker]);

if (!isPrivateConnection(url.hostname)) {
url.set('protocol', 'https:');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,18 +303,6 @@ describe('NetworkSwitcher View', () => {
],
]
`);
expect(
(Engine.context.CurrencyRateController.updateExchangeRate as jest.Mock)
.mock.calls,
).toMatchInlineSnapshot(`
[
[
[
"POL",
],
],
]
`);
});

it('renders correctly with errors', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,22 +153,20 @@ function NetworkSwitcher() {

const switchNetwork = useCallback(
(networkConfiguration) => {
const { CurrencyRateController, NetworkController } = Engine.context;
const { NetworkController } = Engine.context;
const config = Object.values(networkConfigurations).find(
({ chainId }) => chainId === networkConfiguration.chainId,
);

if (config) {
const {
nativeCurrency: ticker,
rpcEndpoints,
defaultRpcEndpointIndex,
} = config;

const { networkClientId } =
rpcEndpoints?.[defaultRpcEndpointIndex] ?? {};

CurrencyRateController.updateExchangeRate([ticker]);
NetworkController.setActiveNetwork(networkClientId);
navigateToGetStarted();
}
Expand Down
16 changes: 0 additions & 16 deletions app/components/Views/NetworkSelector/NetworkSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ import Networks, {
} from '../../../util/networks';
import {
LINEA_MAINNET,
LINEA_SEPOLIA,
MAINNET,
SEPOLIA,
} from '../../../constants/network';
import Button from '../../../component-library/components/Buttons/Button/Button';
import {
Expand All @@ -65,7 +63,6 @@ import createStyles from './NetworkSelector.styles';
import {
BUILT_IN_NETWORKS,
InfuraNetworkType,
TESTNET_TICKER_SYMBOLS,
} from '@metamask/controller-utils';
import InfoModal from '../../../../app/components/UI/Swaps/components/InfoModal';
import hideKeyFromUrl from '../../../util/hideKeyFromUrl';
Expand Down Expand Up @@ -241,7 +238,6 @@ const NetworkSelector = () => {

const onSetRpcTarget = async (networkConfiguration: NetworkConfiguration) => {
const {
CurrencyRateController,
NetworkController,
SelectedNetworkController,
} = Engine.context;
Expand All @@ -254,7 +250,6 @@ const NetworkSelector = () => {
const {
name: nickname,
chainId,
nativeCurrency: ticker,
rpcEndpoints,
defaultRpcEndpointIndex,
} = networkConfiguration;
Expand All @@ -268,8 +263,6 @@ const NetworkSelector = () => {
networkConfigurationId,
);
} else {
CurrencyRateController.updateExchangeRate([ticker]);

const { networkClientId } = rpcEndpoints[defaultRpcEndpointIndex];

await NetworkController.setActiveNetwork(networkClientId);
Expand Down Expand Up @@ -370,21 +363,13 @@ const NetworkSelector = () => {
});
const {
NetworkController,
CurrencyRateController,
AccountTrackerController,
SelectedNetworkController,
} = Engine.context;

if (domainIsConnectedDapp && process.env.MULTICHAIN_V1) {
SelectedNetworkController.setNetworkClientIdForDomain(origin, type);
} else {
let ticker = type;
if (type === LINEA_SEPOLIA) {
ticker = TESTNET_TICKER_SYMBOLS.LINEA_SEPOLIA as InfuraNetworkType;
}
if (type === SEPOLIA) {
ticker = TESTNET_TICKER_SYMBOLS.SEPOLIA as InfuraNetworkType;
}

const networkConfiguration =
networkConfigurations[BUILT_IN_NETWORKS[type].chainId];
Expand All @@ -394,7 +379,6 @@ const NetworkSelector = () => {
networkConfiguration.defaultRpcEndpointIndex
].networkClientId ?? type;

CurrencyRateController.updateExchangeRate([ticker]);
NetworkController.setActiveNetwork(clientId);
closeRpcModal();
AccountTrackerController.refresh();
Expand Down
9 changes: 6 additions & 3 deletions app/components/Views/Root/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useAppTheme, ThemeContext } from '../../../util/theme';
import { ToastContextWrapper } from '../../../component-library/components/Toast';
import { SafeAreaProvider } from 'react-native-safe-area-context';
import { isTest } from '../../../util/test/utils';
import { AssetPollingProvider } from '../../hooks/AssetPolling/AssetPollingProvider';

/**
* Top level of the component hierarchy
Expand Down Expand Up @@ -85,9 +86,11 @@ const ConnectedRoot = () => {
<SafeAreaProvider>
<ThemeContext.Provider value={theme}>
<ToastContextWrapper>
<ErrorBoundary view="Root">
<App />
</ErrorBoundary>
<AssetPollingProvider>
<ErrorBoundary view="Root">
<App />
</ErrorBoundary>
</AssetPollingProvider>
</ToastContextWrapper>
</ThemeContext.Provider>
</SafeAreaProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -822,14 +822,13 @@ export class NetworkSettings extends PureComponent {
shouldNetworkSwitchPopToWallet,
navigation,
}) => {
const { NetworkController, CurrencyRateController } = Engine.context;
const { NetworkController } = Engine.context;

const url = new URL(rpcUrl);
if (!isPrivateConnection(url.hostname)) {
url.set('protocol', 'https:');
}

CurrencyRateController.updateExchangeRate([ticker]);
const existingNetwork = this.props.networkConfigurations[chainId];

const indexRpc = rpcUrls.findIndex(({ url }) => url === rpcUrl);
Expand Down Expand Up @@ -1530,15 +1529,14 @@ export class NetworkSettings extends PureComponent {
};

switchToMainnet = () => {
const { NetworkController, CurrencyRateController } = Engine.context;
const { NetworkController } = Engine.context;
const { networkConfigurations } = this.props;

const { networkClientId } =
networkConfigurations?.rpcEndpoints?.[
networkConfigurations.defaultRpcEndpointIndex
] ?? {};

CurrencyRateController.updateExchangeRate(NetworksTicker.mainnet);
NetworkController.setActiveNetwork(networkClientId);

setTimeout(async () => {
Expand Down
3 changes: 1 addition & 2 deletions app/components/Views/Settings/NetworksSettings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,8 @@ class NetworksSettings extends PureComponent {
};

switchToMainnet = () => {
const { NetworkController, CurrencyRateController } = Engine.context;
const { NetworkController } = Engine.context;

CurrencyRateController.updateExchangeRate(NetworksTicker.mainnet);
NetworkController.setProviderType(MAINNET);

setTimeout(async () => {
Expand Down
11 changes: 11 additions & 0 deletions app/components/hooks/AssetPolling/AssetPollingProvider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React, { ReactNode } from 'react';
import useCurrencyRatePolling from './useCurrencyRatePolling';

// This provider is a step towards making controller polling fully UI based.
// Eventually, individual UI components will call the use*Polling hooks to
// poll and return particular data. This polls globally in the meantime.
export const AssetPollingProvider = ({ children }: { children: ReactNode }) => {
useCurrencyRatePolling();

return <>{children}</>;
};
42 changes: 42 additions & 0 deletions app/components/hooks/AssetPolling/useCurrencyRatePolling.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import useCurrencyRatePolling from './useCurrencyRatePolling';
import { renderHookWithProvider } from '../../../util/test/renderWithProvider';
import Engine from '../../../core/Engine';

jest.mock('../../../core/Engine', () => ({
context: {
CurrencyRateController: {
startPolling: jest.fn(),
stopPollingByPollingToken: jest.fn(),
},
},
}));

describe('useCurrencyRatePolling', () => {

it('Should poll by the native currencies in network state', async () => {

const state = {
engine: {
backgroundState: {
NetworkController: {
networkConfigurationsByChainId: {
'0x1': {
nativeCurrency: 'ETH',
},
'0x89': {
nativeCurrency: 'POL',
},
},
},
},
},
};

renderHookWithProvider(() => useCurrencyRatePolling(), {state});

expect(
jest.mocked(Engine.context.CurrencyRateController.startPolling)
).toHaveBeenCalledWith({nativeCurrencies: ['ETH', 'POL']});

});
});
39 changes: 39 additions & 0 deletions app/components/hooks/AssetPolling/useCurrencyRatePolling.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useSelector } from 'react-redux';
import usePolling from '../usePolling';
import { selectNetworkConfigurations } from '../../../selectors/networkController';
import Engine from '../../../core/Engine';
import { selectConversionRate, selectCurrencyRates } from '../../../selectors/currencyRateController';

// Polls native currency prices across networks.
const useCurrencyRatePolling = () => {

// Selectors to determine polling input
const networkConfigurations = useSelector(selectNetworkConfigurations);

// Selectors returning state updated by the polling
const conversionRate = useSelector(selectConversionRate);
const currencyRates = useSelector(selectCurrencyRates);

const nativeCurrencies = [
...new Set(
Object.values(networkConfigurations).map((n) => n.nativeCurrency),
),
];

const { CurrencyRateController } = Engine.context;

usePolling({
startPolling:
CurrencyRateController.startPolling.bind(CurrencyRateController),
stopPollingByPollingToken:
CurrencyRateController.stopPollingByPollingToken.bind(CurrencyRateController),
input: [{nativeCurrencies}],
});

return {
conversionRate,
currencyRates,
};
};

export default useCurrencyRatePolling;
10 changes: 1 addition & 9 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -708,15 +708,7 @@ export class Engine {
}),
state: initialState.CurrencyRateController,
});
const currentNetworkConfig =
networkController.getNetworkConfigurationByNetworkClientId(
networkController?.state.selectedNetworkClientId,
);
currencyRateController.startPolling({
nativeCurrencies: currentNetworkConfig?.nativeCurrency
? [currentNetworkConfig?.nativeCurrency]
: [],
});

const gasFeeController = new GasFeeController({
// @ts-expect-error TODO: Resolve mismatch between base-controller versions.
messenger: this.controllerMessenger.getRestricted({
Expand Down
2 changes: 0 additions & 2 deletions app/core/RPCMethods/lib/ethereum-chain-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ export async function switchToNetwork({
isAddNetworkFlow = false,
}) {
const {
CurrencyRateController,
NetworkController,
PermissionController,
SelectedNetworkController,
Expand Down Expand Up @@ -300,7 +299,6 @@ export async function switchToNetwork({
networkConfigurationId || networkConfiguration.networkType,
);
} else {
CurrencyRateController.updateExchangeRate(requestData.ticker);
NetworkController.setActiveNetwork(
networkConfigurationId || networkConfiguration.networkType,
);
Expand Down
2 changes: 0 additions & 2 deletions app/core/RPCMethods/wallet_addEthereumChain.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,6 @@ describe('RPC Method - wallet_addEthereumChain', () => {
}),
);
expect(spyOnSetActiveNetwork).toHaveBeenCalledTimes(1);
expect(spyOnUpdateExchangeRate).toHaveBeenCalledTimes(1);
});

it('should not add a networkConfiguration that has a chainId that already exists in wallet state, and should switch to the existing network', async () => {
Expand Down Expand Up @@ -468,7 +467,6 @@ describe('RPC Method - wallet_addEthereumChain', () => {

expect(spyOnAddNetwork).not.toHaveBeenCalled();
expect(spyOnSetActiveNetwork).toHaveBeenCalledTimes(1);
expect(spyOnUpdateExchangeRate).toHaveBeenCalledTimes(1);
});

describe('MM_CHAIN_PERMISSIONS is enabled', () => {
Expand Down
7 changes: 7 additions & 0 deletions app/selectors/currencyRateController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@ export const selectCurrentCurrency = createSelector(
(currencyRateControllerState: CurrencyRateState) =>
currencyRateControllerState?.currentCurrency,
);

export const selectCurrencyRates = createSelector(
selectCurrencyRateControllerState,
(
currencyRateControllerState: CurrencyRateState,
) => currencyRateControllerState?.currencyRates,
);
7 changes: 0 additions & 7 deletions app/util/networks/handleNetworkSwitch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,6 @@ describe('useHandleNetworkSwitch', () => {

const nickname = handleNetworkSwitch('1338');

expect(
mockEngine.context.CurrencyRateController.updateExchangeRate,
).toBeCalledWith(['TEST']);
expect(
mockEngine.context.NetworkController.setActiveNetwork,
).toBeCalledWith('networkId1');
Expand All @@ -153,10 +150,6 @@ describe('useHandleNetworkSwitch', () => {

const networkType = handleNetworkSwitch('11155111');

// TODO: This is a bug, it should be set to SepoliaETH
expect(
mockEngine.context.CurrencyRateController.updateExchangeRate,
).toBeCalledWith(['ETH']);
expect(
mockEngine.context.NetworkController.setProviderType,
).not.toBeCalledWith();
Expand Down
Loading

0 comments on commit 2792684

Please sign in to comment.