Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update controllers that rely on provider to listen to NetworkController:networkDidChange instead of NetworkController:stateChange #3610

Merged
58 changes: 29 additions & 29 deletions packages/assets-controllers/src/AssetsContractController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ async function setupAssetContractControllers() {
const assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
messenger.subscribe('NetworkController:stateChange', listener),
onNetworkDidChange: (listener) =>
messenger.subscribe('NetworkController:networkDidChange', listener),
getNetworkClientById: (networkClientId: NetworkClientId) =>
({
...network.getNetworkClientById(networkClientId),
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('AssetsContractController', () => {
ipfsGateway: IPFS_DEFAULT_GATEWAY_URL,
provider: undefined,
});
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should update the ipfsGateWay config value when this value is changed in the preferences controller', async () => {
Expand All @@ -148,15 +148,15 @@ describe('AssetsContractController', () => {
provider: undefined,
});

messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw when provider property is accessed', async () => {
const { assetsContract, messenger } = await setupAssetContractControllers();
expect(() => console.log(assetsContract.provider)).toThrow(
'Property only used for setting',
);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting ERC-20 token balance when missing provider', async () => {
Expand All @@ -168,7 +168,7 @@ describe('AssetsContractController', () => {
TEST_ACCOUNT_PUBLIC_ADDRESS,
),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting ERC-20 token decimal when missing provider', async () => {
Expand All @@ -177,7 +177,7 @@ describe('AssetsContractController', () => {
await expect(
assetsContract.getERC20TokenDecimals(ERC20_UNI_ADDRESS),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get balance of ERC-20 token contract correctly', async () => {
Expand Down Expand Up @@ -231,7 +231,7 @@ describe('AssetsContractController', () => {
);
expect(UNIBalance.toString(16)).not.toBe('0');
expect(UNINoBalance.toString(16)).toBe('0');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 NFT tokenId correctly', async () => {
Expand Down Expand Up @@ -265,7 +265,7 @@ describe('AssetsContractController', () => {
0,
);
expect(tokenId).not.toBe(0);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting ERC-721 token standard and details when missing provider', async () => {
Expand All @@ -277,7 +277,7 @@ describe('AssetsContractController', () => {
TEST_ACCOUNT_PUBLIC_ADDRESS,
),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw contract standard error when getting ERC-20 token standard and details when provided with invalid ERC-20 address', async () => {
Expand All @@ -291,7 +291,7 @@ describe('AssetsContractController', () => {
TEST_ACCOUNT_PUBLIC_ADDRESS,
),
).rejects.toThrow(error);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 token standard and details', async () => {
Expand Down Expand Up @@ -356,7 +356,7 @@ describe('AssetsContractController', () => {
TEST_ACCOUNT_PUBLIC_ADDRESS,
);
expect(standardAndDetails.standard).toBe('ERC721');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-1155 token standard and details', async () => {
Expand Down Expand Up @@ -441,7 +441,7 @@ describe('AssetsContractController', () => {
expect(standardAndDetails.name).toBe('Apeiron Godiverse Collection');
expect(standardAndDetails.symbol).toBe('APEGC');

messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-20 token standard and details', async () => {
Expand Down Expand Up @@ -538,7 +538,7 @@ describe('AssetsContractController', () => {
TEST_ACCOUNT_PUBLIC_ADDRESS,
);
expect(standardAndDetails.standard).toBe('ERC20');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 NFT tokenURI correctly', async () => {
Expand Down Expand Up @@ -587,7 +587,7 @@ describe('AssetsContractController', () => {
'0',
);
expect(tokenId).toBe('https://api.godsunchained.com/card/0');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw an error when address given is not an ERC-721 NFT', async () => {
Expand Down Expand Up @@ -623,7 +623,7 @@ describe('AssetsContractController', () => {

const error = 'Contract does not support ERC721 metadata interface.';
await expect(result).rejects.toThrow(error);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 NFT name', async () => {
Expand Down Expand Up @@ -653,7 +653,7 @@ describe('AssetsContractController', () => {
});
const name = await assetsContract.getERC721AssetName(ERC721_GODS_ADDRESS);
expect(name).toBe('Gods Unchained');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 NFT symbol', async () => {
Expand Down Expand Up @@ -685,15 +685,15 @@ describe('AssetsContractController', () => {
ERC721_GODS_ADDRESS,
);
expect(symbol).toBe('GODS');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting ERC-721 NFT symbol when missing provider', async () => {
const { assetsContract, messenger } = await setupAssetContractControllers();
await expect(
assetsContract.getERC721AssetSymbol(ERC721_GODS_ADDRESS),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-20 token decimals', async () => {
Expand Down Expand Up @@ -725,7 +725,7 @@ describe('AssetsContractController', () => {
ERC20_SAI_ADDRESS,
);
expect(Number(decimals)).toBe(18);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-20 token name', async () => {
Expand Down Expand Up @@ -757,7 +757,7 @@ describe('AssetsContractController', () => {
const name = await assetsContract.getERC20TokenName(ERC20_DAI_ADDRESS);

expect(name).toBe('Dai Stablecoin');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get ERC-721 NFT ownership', async () => {
Expand Down Expand Up @@ -790,15 +790,15 @@ describe('AssetsContractController', () => {
'148332',
);
expect(tokenId).not.toBe('');
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting ERC-721 NFT ownership', async () => {
const { assetsContract, messenger } = await setupAssetContractControllers();
await expect(
assetsContract.getERC721OwnerOf(ERC721_GODS_ADDRESS, '148332'),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get balance of ERC-20 token in a single call on network with token detection support', async () => {
Expand Down Expand Up @@ -831,7 +831,7 @@ describe('AssetsContractController', () => {
[ERC20_SAI_ADDRESS],
);
expect(balances[ERC20_SAI_ADDRESS]).toBeDefined();
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should not have balance in a single call after switching to network without token detection support', async () => {
Expand Down Expand Up @@ -908,7 +908,7 @@ describe('AssetsContractController', () => {
[ERC20_SAI_ADDRESS],
);
expect(noBalances).toStrictEqual({});
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when transferring single ERC-1155 when missing provider', async () => {
Expand All @@ -923,7 +923,7 @@ describe('AssetsContractController', () => {
'1',
),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get the balance of a ERC-1155 NFT for a given address', async () => {
Expand Down Expand Up @@ -957,7 +957,7 @@ describe('AssetsContractController', () => {
ERC1155_ID,
);
expect(Number(balance)).toBeGreaterThan(0);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should throw missing provider error when getting the balance of a ERC-1155 NFT when missing provider', async () => {
Expand All @@ -969,7 +969,7 @@ describe('AssetsContractController', () => {
ERC1155_ID,
),
).rejects.toThrow(MISSING_PROVIDER_ERROR);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

it('should get the URI of a ERC-1155 NFT', async () => {
Expand Down Expand Up @@ -1003,6 +1003,6 @@ describe('AssetsContractController', () => {
ERC1155_ID,
);
expect(uri.toLowerCase()).toStrictEqual(expectedUri);
messenger.clearEventSubscriptions('NetworkController:stateChange');
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});
});
8 changes: 4 additions & 4 deletions packages/assets-controllers/src/AssetsContractController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class AssetsContractController extends BaseControllerV1<
* @param options - The controller options.
* @param options.chainId - The chain ID of the current network.
* @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes.
* @param options.onNetworkStateChange - Allows subscribing to network controller state changes.
* @param options.onNetworkDidChange - Allows subscribing to network controller networkDidChange events.
* @param options.getNetworkClientById - Gets the network client with the given id from the NetworkController.
* @param config - Initial options used to configure this controller.
* @param state - Initial state to set on this controller.
Expand All @@ -107,14 +107,14 @@ export class AssetsContractController extends BaseControllerV1<
{
chainId: initialChainId,
onPreferencesStateChange,
onNetworkStateChange,
onNetworkDidChange,
getNetworkClientById,
}: {
chainId: Hex;
onPreferencesStateChange: (
listener: (preferencesState: PreferencesState) => void,
) => void;
onNetworkStateChange: (
onNetworkDidChange: (
listener: (networkState: NetworkState) => void,
) => void;
getNetworkClientById: NetworkController['getNetworkClientById'];
Expand All @@ -135,7 +135,7 @@ export class AssetsContractController extends BaseControllerV1<
this.configure({ ipfsGateway });
});

onNetworkStateChange((networkState) => {
onNetworkDidChange((networkState) => {
if (this.config.chainId !== networkState.providerConfig.chainId) {
this.configure({
chainId: networkState.providerConfig.chainId,
Expand Down
10 changes: 5 additions & 5 deletions packages/assets-controllers/src/NftController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ function setupController({
) => Promise<BN>;
} = {}) {
const preferences = new PreferencesController();
const onNetworkStateChangeListeners: ((state: NetworkState) => void)[] = [];
const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = [];
const changeNetwork = (providerConfig: ProviderConfig) => {
onNetworkStateChangeListeners.forEach((listener) => {
onNetworkDidChangeListeners.forEach((listener) => {
listener({
...defaultNetworkState,
providerConfig,
Expand Down Expand Up @@ -202,8 +202,8 @@ function setupController({
const assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
onNetworkStateChangeListeners.push(listener),
onNetworkDidChange: (listener) =>
onNetworkDidChangeListeners.push(listener),
getNetworkClientById: getNetworkClientByIdSpy,
});

Expand All @@ -222,7 +222,7 @@ function setupController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
onNetworkStateChangeListeners.push(listener),
onNetworkDidChangeListeners.push(listener),
getERC721AssetName:
getERC721AssetNameStub ??
assetsContract.getERC721AssetName.bind(assetsContract),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('NftDetectionController', () => {
let assetsContract: AssetsContractController;
let clock: sinon.SinonFakeTimers;
const networkStateChangeNoop = jest.fn();
const networkDidChangeNoop = jest.fn();
const getOpenSeaApiKeyStub = jest.fn();

const messenger = new ControllerMessenger<
Expand All @@ -40,7 +41,7 @@ describe('NftDetectionController', () => {
assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: networkStateChangeNoop,
onNetworkDidChange: networkDidChangeNoop,
getNetworkClientById: jest.fn(),
});
const getNetworkClientById = jest.fn().mockImplementation(() => {
Expand Down
Loading
Loading