Skip to content

Commit

Permalink
chore: bump asset controllers to 39 + polling API (#28025)
Browse files Browse the repository at this point in the history
## **Description**

Bumps the asset controllers to 39. In this version, the polling API has
changed from `startPollingByNetworkClientId` to a more flexible
`startPolling` that can accept any input. The `usePolling` hook is
modified to accommodate this.


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28025?quickstart=1)

## **Related issues**


## **Manual testing steps**

Should be no noticeable changes. Currency rates should continue to be
fetched. Try switching between chains with different native currencies
like mainnet, polygon, bnb and verify the native token has fiat prices.

## **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
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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-extension/blob/develop/.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: MetaMask Bot <metamaskbot@users.noreply.github.com>
  • Loading branch information
bergeron and metamaskbot authored Oct 29, 2024
1 parent 54b110a commit acb79ec
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 97 deletions.
7 changes: 3 additions & 4 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3995,10 +3995,9 @@ export default class MetamaskController extends EventEmitter {
),

// CurrencyRateController
currencyRateStartPollingByNetworkClientId:
currencyRateController.startPollingByNetworkClientId.bind(
currencyRateController,
),
currencyRateStartPolling: currencyRateController.startPolling.bind(
currencyRateController,
),
currencyRateStopPollingByPollingToken:
currencyRateController.stopPollingByPollingToken.bind(
currencyRateController,
Expand Down
8 changes: 1 addition & 7 deletions lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -674,13 +674,13 @@
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
"@metamask/assets-controllers>@metamask/polling-controller": true,
"@metamask/assets-controllers>@metamask/rpc-errors": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/eth-query": true,
"@metamask/metamask-eth-abis": true,
"@metamask/name-controller>async-mutex": true,
"@metamask/rpc-errors": true,
"@metamask/utils": true,
"bn.js": true,
"cockatiel": true,
Expand All @@ -702,12 +702,6 @@
"uuid": true
}
},
"@metamask/assets-controllers>@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
"@metamask/utils": true
}
},
"@metamask/base-controller": {
"globals": {
"setTimeout": true
Expand Down
8 changes: 1 addition & 7 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -674,13 +674,13 @@
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
"@metamask/assets-controllers>@metamask/polling-controller": true,
"@metamask/assets-controllers>@metamask/rpc-errors": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/eth-query": true,
"@metamask/metamask-eth-abis": true,
"@metamask/name-controller>async-mutex": true,
"@metamask/rpc-errors": true,
"@metamask/utils": true,
"bn.js": true,
"cockatiel": true,
Expand All @@ -702,12 +702,6 @@
"uuid": true
}
},
"@metamask/assets-controllers>@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
"@metamask/utils": true
}
},
"@metamask/base-controller": {
"globals": {
"setTimeout": true
Expand Down
8 changes: 1 addition & 7 deletions lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -674,13 +674,13 @@
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
"@metamask/assets-controllers>@metamask/polling-controller": true,
"@metamask/assets-controllers>@metamask/rpc-errors": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/eth-query": true,
"@metamask/metamask-eth-abis": true,
"@metamask/name-controller>async-mutex": true,
"@metamask/rpc-errors": true,
"@metamask/utils": true,
"bn.js": true,
"cockatiel": true,
Expand All @@ -702,12 +702,6 @@
"uuid": true
}
},
"@metamask/assets-controllers>@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
"@metamask/utils": true
}
},
"@metamask/base-controller": {
"globals": {
"setTimeout": true
Expand Down
8 changes: 1 addition & 7 deletions lavamoat/browserify/mmi/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -766,13 +766,13 @@
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
"@metamask/assets-controllers>@metamask/polling-controller": true,
"@metamask/assets-controllers>@metamask/rpc-errors": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/eth-query": true,
"@metamask/metamask-eth-abis": true,
"@metamask/name-controller>async-mutex": true,
"@metamask/rpc-errors": true,
"@metamask/utils": true,
"bn.js": true,
"cockatiel": true,
Expand All @@ -794,12 +794,6 @@
"uuid": true
}
},
"@metamask/assets-controllers>@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
"@metamask/utils": true
}
},
"@metamask/base-controller": {
"globals": {
"setTimeout": true
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,12 @@
"@metamask/address-book-controller": "^6.0.0",
"@metamask/announcement-controller": "^7.0.0",
"@metamask/approval-controller": "^7.0.0",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A38.3.0#~/.yarn/patches/@metamask-assets-controllers-npm-38.3.0-57b3d695bb.patch",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A39.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-39.0.0-57b3d695bb.patch",
"@metamask/base-controller": "^7.0.0",
"@metamask/bitcoin-wallet-snap": "^0.8.2",
"@metamask/browser-passworder": "^4.3.0",
"@metamask/contract-metadata": "^2.5.0",
"@metamask/controller-utils": "^11.2.0",
"@metamask/controller-utils": "^11.4.0",
"@metamask/design-tokens": "^4.0.0",
"@metamask/ens-controller": "^13.0.0",
"@metamask/ens-resolver-snap": "^0.1.2",
Expand Down
5 changes: 3 additions & 2 deletions ui/hooks/useCurrencyRatePolling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ const useCurrencyRatePolling = (networkClientId?: string) => {
const selectedNetworkClientId = useSelector(getSelectedNetworkClientId);

usePolling({
startPollingByNetworkClientId: currencyRateStartPollingByNetworkClientId,
startPolling: (input) =>
currencyRateStartPollingByNetworkClientId(input.networkClientId),
stopPollingByPollingToken: currencyRateStopPollingByPollingToken,
networkClientId: networkClientId ?? selectedNetworkClientId,
input: { networkClientId: networkClientId ?? selectedNetworkClientId },
enabled: useCurrencyRateCheck && completedOnboarding,
});
};
Expand Down
5 changes: 3 additions & 2 deletions ui/hooks/useGasFeeEstimates.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,10 @@ export function useGasFeeEstimates(_networkClientId) {
}, [networkClientId]);

usePolling({
startPollingByNetworkClientId: gasFeeStartPollingByNetworkClientId,
startPolling: (input) =>
gasFeeStartPollingByNetworkClientId(input.networkClientId),
stopPollingByPollingToken: gasFeeStopPollingByPollingToken,
networkClientId,
input: { networkClientId },
});

return {
Expand Down
9 changes: 4 additions & 5 deletions ui/hooks/useGasFeeEstimates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
getIsNetworkBusyByChainId,
} from '../ducks/metamask/metamask';
import {
gasFeeStartPollingByNetworkClientId,
gasFeeStopPollingByPollingToken,
getNetworkConfigurationByNetworkClientId,
} from '../store/actions';
Expand Down Expand Up @@ -115,9 +114,9 @@ describe('useGasFeeEstimates', () => {
renderHook(() => useGasFeeEstimates());
});
expect(usePolling).toHaveBeenCalledWith({
startPollingByNetworkClientId: gasFeeStartPollingByNetworkClientId,
startPolling: expect.any(Function),
stopPollingByPollingToken: gasFeeStopPollingByPollingToken,
networkClientId: 'selectedNetworkClientId',
input: { networkClientId: 'selectedNetworkClientId' },
});
});

Expand All @@ -127,9 +126,9 @@ describe('useGasFeeEstimates', () => {
renderHook(() => useGasFeeEstimates('networkClientId1'));
});
expect(usePolling).toHaveBeenCalledWith({
startPollingByNetworkClientId: gasFeeStartPollingByNetworkClientId,
startPolling: expect.any(Function),
stopPollingByPollingToken: gasFeeStopPollingByPollingToken,
networkClientId: 'networkClientId1',
input: { networkClientId: 'networkClientId1' },
});
});

Expand Down
16 changes: 6 additions & 10 deletions ui/hooks/usePolling.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,29 @@ import usePolling from './usePolling';

describe('usePolling', () => {
// eslint-disable-next-line jest/no-done-callback
it('calls startPollingByNetworkClientId and callback option args with polling token when component instantiating the hook mounts', (done) => {
it('calls startPolling and calls back with polling token when component instantiating the hook mounts', (done) => {
const mockStart = jest.fn().mockImplementation(() => {
return Promise.resolve('pollingToken');
});
const mockStop = jest.fn();
const networkClientId = 'mainnet';
const options = {};
const mockState = {
metamask: {},
};

renderHookWithProvider(() => {
usePolling({
callback: (pollingToken) => {
expect(mockStart).toHaveBeenCalledWith(networkClientId, options);
expect(mockStart).toHaveBeenCalledWith({ networkClientId });
expect(pollingToken).toBeDefined();
done();
return (_pollingToken) => {
// noop
};
},
startPollingByNetworkClientId: mockStart,
startPolling: mockStart,
stopPollingByPollingToken: mockStop,
networkClientId,
options,
input: { networkClientId },
});
}, mockState);
});
Expand All @@ -39,7 +37,6 @@ describe('usePolling', () => {
});
const mockStop = jest.fn();
const networkClientId = 'mainnet';
const options = {};
const mockState = {
metamask: {},
};
Expand All @@ -54,10 +51,9 @@ describe('usePolling', () => {
done();
};
},
startPollingByNetworkClientId: mockStart,
startPolling: mockStart,
stopPollingByPollingToken: mockStop,
networkClientId,
options,
input: { networkClientId },
}),
mockState,
);
Expand Down
30 changes: 8 additions & 22 deletions ui/hooks/usePolling.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,16 @@
import { useEffect, useRef } from 'react';

type UsePollingOptions = {
type UsePollingOptions<PollingInput> = {
callback?: (pollingToken: string) => (pollingToken: string) => void;
startPollingByNetworkClientId: (
networkClientId: string,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
options: any,
) => Promise<string>;
startPolling: (input: PollingInput) => Promise<string>;
stopPollingByPollingToken: (pollingToken: string) => void;
networkClientId: string;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
options?: any;
input: PollingInput;
enabled?: boolean;
};

const usePolling = (usePollingOptions: UsePollingOptions) => {
const usePolling = <PollingInput>(
usePollingOptions: UsePollingOptions<PollingInput>,
) => {
const pollTokenRef = useRef<null | string>(null);
const cleanupRef = useRef<null | ((pollingToken: string) => void)>(null);
let isMounted = false;
Expand All @@ -38,10 +32,7 @@ const usePolling = (usePollingOptions: UsePollingOptions) => {

// Start polling when the component mounts
usePollingOptions
.startPollingByNetworkClientId(
usePollingOptions.networkClientId,
usePollingOptions.options,
)
.startPolling(usePollingOptions.input)
.then((pollToken) => {
pollTokenRef.current = pollToken;
cleanupRef.current = usePollingOptions.callback?.(pollToken) || null;
Expand All @@ -56,12 +47,7 @@ const usePolling = (usePollingOptions: UsePollingOptions) => {
cleanup();
};
}, [
usePollingOptions.networkClientId,
usePollingOptions.options &&
JSON.stringify(
usePollingOptions.options,
Object.keys(usePollingOptions.options).sort(),
),
usePollingOptions.input && JSON.stringify(usePollingOptions.input),
usePollingOptions.enabled,
]);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,10 @@ const ConfirmTransaction = () => {
const prevTransactionId = usePrevious(transactionId);

usePolling({
startPollingByNetworkClientId: gasFeeStartPollingByNetworkClientId,
startPolling: (input) =>
gasFeeStartPollingByNetworkClientId(input.networkClientId),
stopPollingByPollingToken: gasFeeStopPollingByPollingToken,
networkClientId: transaction.networkClientId ?? networkClientId,
input: { networkClientId: transaction.networkClientId ?? networkClientId },
});

useEffect(() => {
Expand Down
4 changes: 2 additions & 2 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4524,8 +4524,8 @@ export async function currencyRateStartPollingByNetworkClientId(
networkClientId: string,
): Promise<string> {
const pollingToken = await submitRequestToBackground(
'currencyRateStartPollingByNetworkClientId',
[networkClientId],
'currencyRateStartPolling',
[{ networkClientId }],
);
await addPollingTokenToAppState(pollingToken);
return pollingToken;
Expand Down
Loading

0 comments on commit acb79ec

Please sign in to comment.