Skip to content

Commit

Permalink
bump @metamask/assets-controllers to ^13.0.0 (#20916)
Browse files Browse the repository at this point in the history
## Explanation
Needed to help get us closer to phasing out a single globally selected
network.

Bumps `@metamask/assets-controllers` to ^13.0.0 and passes
networkClientId arg to newly updated `TokenController.addToken()`,
`TokenController.addTokens()`, and `TokenController.watchAsset()`.
Adopts changes in other asset-related controllers

* Fixes MetaMask/MetaMask-planning#1020
* See MetaMask/core#1676


## Screenshots/Screencaps

None. Should be no changes apparent to user.

## Manual Testing Steps

### Via Dapp API
* Visit any [coingecko token
page](https://www.coingecko.com/en/coins/ethereum-name-service)
  * On a supported built-in network (switch to mainnet)
* Click the metamask icon on the page
  * Tooltip says: "Add to MetaMask"
* Accept
* Verify the token shows up in wallet UI

### Via Wallet UI
* From wallet ui
* Import new token
* Fill in token details
* Use `0xc18360217d8f7ab5e7c516566761ea12ce7f9d72` contract address for
ENS
* Verify the token shows up in wallet UI

## Pre-merge author checklist

- [x] I've clearly explained:
  - [x] What problem this PR is solving
  - [x] How this problem was solved
  - [x] How reviewers can test my changes
- [x] Sufficient automated test coverage has been added

## Pre-merge reviewer checklist

- [ ] Manual testing (e.g. pull and build branch, run in browser, test
code being changed)
- [ ] PR is linked to the appropriate GitHub issue
- [ ] **IF** this PR fixes a bug in the release milestone, add this PR
to the release milestone

If further QA is required (e.g. new feature, complex testing steps,
large refactor), add the `Extension QA Board` label.

In this case, a QA Engineer approval will be be required.

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 3, 2023
1 parent 0139b99 commit f814c41
Show file tree
Hide file tree
Showing 18 changed files with 442 additions and 226 deletions.
11 changes: 11 additions & 0 deletions app/scripts/controllers/detect-tokens.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ describe('DetectTokensController', function () {

const tokenListMessenger = new ControllerMessenger().getRestricted({
name: 'TokenListController',
allowedEvents: ['TokenListController:stateChange'],
});
tokenListController = new TokenListController({
chainId: toHex(1),
Expand Down Expand Up @@ -249,6 +250,11 @@ describe('DetectTokensController', function () {
networkControllerMessenger,
'NetworkController:stateChange',
),
onTokenListStateChange: (listener) =>
tokenListMessenger.subscribe(
`${tokenListController.name}:stateChange`,
listener,
),
});

assetsContractController = new AssetsContractController({
Expand Down Expand Up @@ -363,6 +369,7 @@ describe('DetectTokensController', function () {
aggregators: undefined,
image: undefined,
isERC721: undefined,
name: undefined,
},
]);

Expand All @@ -384,6 +391,7 @@ describe('DetectTokensController', function () {
aggregators: undefined,
image: undefined,
isERC721: undefined,
name: undefined,
},
]);
});
Expand Down Expand Up @@ -417,6 +425,7 @@ describe('DetectTokensController', function () {
aggregators: undefined,
image: undefined,
isERC721: undefined,
name: undefined,
},
]);
const tokenAddressToAdd = erc20ContractAddresses[1];
Expand All @@ -435,6 +444,7 @@ describe('DetectTokensController', function () {
aggregators: undefined,
image: undefined,
isERC721: undefined,
name: undefined,
},
{
address: toChecksumHexAddress(tokenAddressToAdd),
Expand All @@ -443,6 +453,7 @@ describe('DetectTokensController', function () {
aggregators: undefined,
image: undefined,
isERC721: undefined,
name: undefined,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ async function watchAssetHandler(
const {
params: { options: asset, type },
origin,
networkClientId,
} = req;

const { tokenId } = asset;
Expand All @@ -56,7 +57,7 @@ async function watchAssetHandler(
);
}

await handleWatchAssetRequest(asset, type, origin);
await handleWatchAssetRequest({ asset, type, origin, networkClientId });
res.result = true;
return end();
} catch (error) {
Expand Down
24 changes: 14 additions & 10 deletions app/scripts/lib/rpc-method-middleware/handlers/watch-asset.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('watchAssetHandler', () => {
type: ERC721,
},
origin: 'testOrigin',
networkClientId: 'networkClientId1',
};

const res = {
Expand All @@ -31,11 +32,12 @@ describe('watchAssetHandler', () => {
handleWatchAssetRequest: mockHandleWatchAssetRequest,
});

expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith(
req.params.options,
req.params.type,
req.origin,
);
expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith({
asset: req.params.options,
type: req.params.type,
origin: req.origin,
networkClientId: req.networkClientId,
});
expect(res.result).toStrictEqual(true);
expect(mockEnd).toHaveBeenCalledWith();
});
Expand All @@ -51,6 +53,7 @@ describe('watchAssetHandler', () => {
type: ERC20,
},
origin: 'testOrigin',
networkClientId: 'networkClientId1',
};

const res = {
Expand All @@ -61,11 +64,12 @@ describe('watchAssetHandler', () => {
handleWatchAssetRequest: mockHandleWatchAssetRequest,
});

expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith(
req.params.options,
req.params.type,
req.origin,
);
expect(mockHandleWatchAssetRequest).toHaveBeenCalledWith({
asset: req.params.options,
type: req.params.type,
origin: req.origin,
networkClientId: req.networkClientId,
});
expect(res.result).toStrictEqual(true);
expect(mockEnd).toHaveBeenCalledWith();
});
Expand Down
34 changes: 32 additions & 2 deletions app/scripts/metamask-controller.actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,41 @@ describe('MetaMaskController', function () {
);

const [token1, token2] = await Promise.all([
metamaskController.getApi().addToken(address, symbol, decimals),
metamaskController.getApi().addToken(address, symbol, decimals),
metamaskController.getApi().addToken({ address, symbol, decimals }),
metamaskController.getApi().addToken({ address, symbol, decimals }),
]);
assert.deepEqual(token1, token2);
});

it('networkClientId is used when provided', async function () {
const supportsInterfaceStub = sinon
.stub()
.returns(Promise.resolve(false));
sinon
.stub(metamaskController.tokensController, '_createEthersContract')
.callsFake(() =>
Promise.resolve({ supportsInterface: supportsInterfaceStub }),
);
sinon
.stub(metamaskController.tokensController, 'getNetworkClientById')
.callsFake(() => ({
configuration: {
chainId: '0xa',
},
}));

await metamaskController.getApi().addToken({
address,
symbol,
decimals,
networkClientId: 'networkClientId1',
});
assert.strictEqual(
metamaskController.tokensController.getNetworkClientById.getCall(0)
.args[0],
'networkClientId1',
);
});
});

describe('#removePermissionsFor', function () {
Expand Down
71 changes: 45 additions & 26 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -453,25 +453,6 @@ export default class MetamaskController extends EventEmitter {
networkConfigurations: this.networkController.state.networkConfigurations,
});

const tokensControllerMessenger = this.controllerMessenger.getRestricted({
name: 'TokensController',
allowedActions: ['ApprovalController:addRequest'],
allowedEvents: ['NetworkController:stateChange'],
});
this.tokensController = new TokensController({
messenger: tokensControllerMessenger,
chainId: this.networkController.state.providerConfig.chainId,
onPreferencesStateChange: this.preferencesController.store.subscribe.bind(
this.preferencesController.store,
),
onNetworkStateChange: networkControllerMessenger.subscribe.bind(
networkControllerMessenger,
'NetworkController:stateChange',
),
config: { provider: this.provider },
state: initState.TokensController,
});

this.assetsContractController = new AssetsContractController(
{
chainId: this.networkController.state.providerConfig.chainId,
Expand All @@ -493,13 +474,46 @@ export default class MetamaskController extends EventEmitter {
return cb(networkState);
},
),
getNetworkClientById: this.networkController.getNetworkClientById.bind(
this.networkController,
),
},
{
provider: this.provider,
},
initState.AssetsContractController,
);

const tokensControllerMessenger = this.controllerMessenger.getRestricted({
name: 'TokensController',
allowedActions: ['ApprovalController:addRequest'],
allowedEvents: ['NetworkController:stateChange'],
});
this.tokensController = new TokensController({
messenger: tokensControllerMessenger,
chainId: this.networkController.state.providerConfig.chainId,
onPreferencesStateChange: this.preferencesController.store.subscribe.bind(
this.preferencesController.store,
),
onNetworkStateChange: networkControllerMessenger.subscribe.bind(
networkControllerMessenger,
'NetworkController:stateChange',
),
onTokenListStateChange: (listener) =>
this.controllerMessenger.subscribe(
`${this.tokenListController.name}:stateChange`,
listener,
),
getNetworkClientById: this.networkController.getNetworkClientById.bind(
this.networkController,
),
getERC20TokenName: this.assetsContractController.getERC20TokenName.bind(
this.assetsContractController,
),
config: { provider: this.provider },
state: initState.TokensController,
});

const nftControllerMessenger = this.controllerMessenger.getRestricted({
name: 'NftController',
allowedActions: [`${this.approvalController.name}:addRequest`],
Expand Down Expand Up @@ -724,17 +738,18 @@ export default class MetamaskController extends EventEmitter {
this.tokenRatesController = new TokenRatesController(
{
chainId: this.networkController.state.providerConfig.chainId,
ticker: this.networkController.state.providerConfig.ticker,
selectedAddress: this.preferencesController.getSelectedAddress(),
onTokensStateChange: (listener) =>
this.tokensController.subscribe(listener),
onCurrencyRateStateChange: (listener) =>
this.controllerMessenger.subscribe(
`${this.currencyRateController.name}:stateChange`,
listener,
),
onNetworkStateChange: networkControllerMessenger.subscribe.bind(
networkControllerMessenger,
'NetworkController:stateChange',
),
onPreferencesStateChange:
this.preferencesController.store.subscribe.bind(
this.preferencesController.store,
),
},
{
disabled:
Expand Down Expand Up @@ -3862,10 +3877,14 @@ export default class MetamaskController extends EventEmitter {
});
}

handleWatchAssetRequest = (asset, type, origin) => {
handleWatchAssetRequest = ({ asset, type, origin, networkClientId }) => {
switch (type) {
case ERC20:
return this.tokensController.watchAsset(asset, type);
return this.tokensController.watchAsset({
asset,
type,
networkClientId,
});
case ERC721:
case ERC1155:
return this.nftController.watchNft(asset, type, origin);
Expand Down
Loading

0 comments on commit f814c41

Please sign in to comment.