From 6f563ffb714db410666ec518e76e1806581367c8 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Wed, 29 May 2024 12:57:20 -0700 Subject: [PATCH 1/5] Release 157.0.0 (#4337) Releasing new version of assets-controllers --------- Co-authored-by: Elliot Winkler --- package.json | 2 +- packages/address-book-controller/package.json | 2 +- packages/assets-controllers/CHANGELOG.md | 30 +++++++++++----- packages/assets-controllers/package.json | 4 +-- packages/controller-utils/CHANGELOG.md | 9 ++++- packages/controller-utils/package.json | 2 +- packages/ens-controller/package.json | 2 +- packages/gas-fee-controller/package.json | 2 +- packages/logging-controller/package.json | 2 +- packages/message-manager/package.json | 2 +- packages/name-controller/package.json | 2 +- packages/network-controller/package.json | 2 +- packages/permission-controller/package.json | 2 +- packages/phishing-controller/package.json | 2 +- packages/polling-controller/package.json | 2 +- packages/preferences-controller/package.json | 2 +- .../queued-request-controller/package.json | 2 +- packages/signature-controller/package.json | 2 +- packages/transaction-controller/package.json | 2 +- .../user-operation-controller/package.json | 2 +- yarn.lock | 34 +++++++++---------- 21 files changed, 66 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index 6520612e7f..52539ab94c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "156.0.0", + "version": "157.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/address-book-controller/package.json b/packages/address-book-controller/package.json index 568910fb50..c796063683 100644 --- a/packages/address-book-controller/package.json +++ b/packages/address-book-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/utils": "^8.3.0" }, "devDependencies": { diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 950f291d68..9c9332f8ec 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,15 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Uncategorized +## [30.0.0] +### Added + +- Adds a new field `marketData` to the state of `TokenRatesController` ([#4206](https://github.com/MetaMask/core/pull/4206)) +- Adds a new `RatesController` to manage prices for non-EVM blockchains ([#4242](https://github.com/MetaMask/core/pull/4242)) + +### Changed + +- **BREAKING:** Changed price and token API endpoints from `*.metafi.codefi.network` to `*.api.cx.metamask.io` ([#4301](https://github.com/MetaMask/core/pull/4301)) +- When fetching token list for Linea Mainnet, use `occurrenceFloor` parameter of 1 instead of 3, and filter tokens to those with a `lineaTeam` aggregator or more than 3 aggregators ([#4253](https://github.com/MetaMask/core/pull/4253)) - **BREAKING:** The NftController messenger must now allow the `NetworkController:getNetworkClientById` action ([#4305](https://github.com/MetaMask/core/pull/4305)) -- NftControllerMessenger now makes use of `selectedNetworkClientId` when responding to changes in NetworkController state to capture the currently selected chain rather than `providerConfig` ([#4305](https://github.com/MetaMask/core/pull/4305)) - - This should be functionally equivalent, but is being noted anyway. -- NftDetectionController now makes use of `selectedNetworkClientId` when responding to changes in NetworkController state to capture the currently selected chain rather than `providerConfig` ([#4307](https://github.com/MetaMask/core/pull/4307)) - - This should be functionally equivalent, but is being noted anyway. -- TokenRatesController now makes use of `selectedNetworkClientId` when responding to changes in NetworkController state to capture the currently selected chain rather than `providerConfig` ([#4317](https://github.com/MetaMask/core/pull/4317)) - - This should be functionally equivalent, but is being noted anyway. +- **BREAKING:** Bump dependency and peer dependency `@metamask/network-controller` to `^18.1.2` ([#4332](https://github.com/MetaMask/core/pull/4332)) +- Bump `@metamask/keyring-api` to `^6.1.1` ([#4262](https://github.com/MetaMask/core/pull/4262)) + +### Removed + +- **BREAKING:** Removed `contractExchangeRates` and `contractExchangeRatesByChainId` from the state of `TokenRatesController` ([#4206](https://github.com/MetaMask/core/pull/4206)) + +### Fixed + +- Only update NFT state when metadata actually changes ([#4143](https://github.com/MetaMask/core/pull/4143)) ## [29.0.0] @@ -783,7 +796,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use Ethers for AssetsContractController ([#845](https://github.com/MetaMask/core/pull/845)) -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@29.0.0...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@30.0.0...HEAD +[30.0.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@29.0.0...@metamask/assets-controllers@30.0.0 [29.0.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@28.0.0...@metamask/assets-controllers@29.0.0 [28.0.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@27.2.0...@metamask/assets-controllers@28.0.0 [27.2.0]: https://github.com/MetaMask/core/compare/@metamask/assets-controllers@27.1.0...@metamask/assets-controllers@27.2.0 diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index ede0136cfb..7f7021740e 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/assets-controllers", - "version": "29.0.0", + "version": "30.0.0", "description": "Controllers which manage interactions involving ERC-20, ERC-721, and ERC-1155 tokens (including NFTs)", "keywords": [ "MetaMask", @@ -51,7 +51,7 @@ "@metamask/approval-controller": "^6.0.2", "@metamask/base-controller": "^5.0.2", "@metamask/contract-metadata": "^2.4.0", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/keyring-controller": "^16.0.0", "@metamask/metamask-eth-abis": "^3.1.1", diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 2b7b5a5f49..8b33dcfa94 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.0.0] + +### Changed + +- **BREAKING:** Changed price and token API endpoints from `*.metafi.codefi.network` to `*.api.cx.metamask.io` ([#4301](https://github.com/MetaMask/core/pull/4301)) + ## [9.1.0] ### Added @@ -327,7 +333,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All changes listed after this point were applied to this package following the monorepo conversion. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@9.1.0...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@10.0.0...HEAD +[10.0.0]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@9.1.0...@metamask/controller-utils@10.0.0 [9.1.0]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@9.0.2...@metamask/controller-utils@9.1.0 [9.0.2]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@9.0.1...@metamask/controller-utils@9.0.2 [9.0.1]: https://github.com/MetaMask/core/compare/@metamask/controller-utils@9.0.0...@metamask/controller-utils@9.0.1 diff --git a/packages/controller-utils/package.json b/packages/controller-utils/package.json index 706998fe9f..f6fa0635b2 100644 --- a/packages/controller-utils/package.json +++ b/packages/controller-utils/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/controller-utils", - "version": "9.1.0", + "version": "10.0.0", "description": "Data and convenience functions shared by multiple packages", "keywords": [ "MetaMask", diff --git a/packages/ens-controller/package.json b/packages/ens-controller/package.json index 1529da5fb1..f816ba74ad 100644 --- a/packages/ens-controller/package.json +++ b/packages/ens-controller/package.json @@ -43,7 +43,7 @@ "dependencies": { "@ethersproject/providers": "^5.7.0", "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/utils": "^8.3.0", "punycode": "^2.1.1" }, diff --git a/packages/gas-fee-controller/package.json b/packages/gas-fee-controller/package.json index 461074006f..c346c34038 100644 --- a/packages/gas-fee-controller/package.json +++ b/packages/gas-fee-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/ethjs-unit": "^0.3.0", "@metamask/network-controller": "^18.1.2", diff --git a/packages/logging-controller/package.json b/packages/logging-controller/package.json index 8c0859e117..b5248c2e34 100644 --- a/packages/logging-controller/package.json +++ b/packages/logging-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "uuid": "^8.3.2" }, "devDependencies": { diff --git a/packages/message-manager/package.json b/packages/message-manager/package.json index 42f2bb1b07..1d436c727c 100644 --- a/packages/message-manager/package.json +++ b/packages/message-manager/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-sig-util": "^7.0.1", "@metamask/utils": "^8.3.0", "@types/uuid": "^8.3.0", diff --git a/packages/name-controller/package.json b/packages/name-controller/package.json index 09772263b3..d8e797a158 100644 --- a/packages/name-controller/package.json +++ b/packages/name-controller/package.json @@ -43,7 +43,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/utils": "^8.3.0", "async-mutex": "^0.2.6" }, diff --git a/packages/network-controller/package.json b/packages/network-controller/package.json index 8e828dd185..2c85ed9910 100644 --- a/packages/network-controller/package.json +++ b/packages/network-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-block-tracker": "^9.0.2", "@metamask/eth-json-rpc-infura": "^9.1.0", "@metamask/eth-json-rpc-middleware": "^12.1.1", diff --git a/packages/permission-controller/package.json b/packages/permission-controller/package.json index 99bc89ae19..4c5f91240d 100644 --- a/packages/permission-controller/package.json +++ b/packages/permission-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/json-rpc-engine": "^8.0.2", "@metamask/rpc-errors": "^6.2.1", "@metamask/utils": "^8.3.0", diff --git a/packages/phishing-controller/package.json b/packages/phishing-controller/package.json index 2c9049751d..3b76b03224 100644 --- a/packages/phishing-controller/package.json +++ b/packages/phishing-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@types/punycode": "^2.1.0", "eth-phishing-detect": "^1.2.0", "punycode": "^2.1.1" diff --git a/packages/polling-controller/package.json b/packages/polling-controller/package.json index 9b7167968c..8d8ab0916f 100644 --- a/packages/polling-controller/package.json +++ b/packages/polling-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/network-controller": "^18.1.2", "@metamask/utils": "^8.3.0", "@types/uuid": "^8.3.0", diff --git a/packages/preferences-controller/package.json b/packages/preferences-controller/package.json index 0fb705ed49..bc3c3bc87c 100644 --- a/packages/preferences-controller/package.json +++ b/packages/preferences-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0" + "@metamask/controller-utils": "^10.0.0" }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", diff --git a/packages/queued-request-controller/package.json b/packages/queued-request-controller/package.json index 57ab804a1e..4e689f6b08 100644 --- a/packages/queued-request-controller/package.json +++ b/packages/queued-request-controller/package.json @@ -42,7 +42,7 @@ }, "dependencies": { "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/json-rpc-engine": "^8.0.2", "@metamask/rpc-errors": "^6.2.1", "@metamask/swappable-obj-proxy": "^2.2.0", diff --git a/packages/signature-controller/package.json b/packages/signature-controller/package.json index 77376ecb17..8140a6d95f 100644 --- a/packages/signature-controller/package.json +++ b/packages/signature-controller/package.json @@ -43,7 +43,7 @@ "dependencies": { "@metamask/approval-controller": "^6.0.2", "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/keyring-controller": "^16.0.0", "@metamask/logging-controller": "^3.0.1", "@metamask/message-manager": "^8.0.2", diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 2689d3466e..ccd5d3d712 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -49,7 +49,7 @@ "@ethersproject/providers": "^5.7.0", "@metamask/approval-controller": "^6.0.2", "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/gas-fee-controller": "^15.1.2", "@metamask/metamask-eth-abis": "^3.1.1", diff --git a/packages/user-operation-controller/package.json b/packages/user-operation-controller/package.json index 40bb1628a8..a6d0ef0173 100644 --- a/packages/user-operation-controller/package.json +++ b/packages/user-operation-controller/package.json @@ -44,7 +44,7 @@ "dependencies": { "@metamask/approval-controller": "^6.0.2", "@metamask/base-controller": "^5.0.2", - "@metamask/controller-utils": "^9.1.0", + "@metamask/controller-utils": "^10.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/gas-fee-controller": "^15.1.2", "@metamask/keyring-controller": "^16.0.0", diff --git a/yarn.lock b/yarn.lock index a6da8e93ec..8d20e9ec1c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1657,7 +1657,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 deepmerge: ^4.2.2 @@ -1720,7 +1720,7 @@ __metadata: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 "@metamask/contract-metadata": ^2.4.0 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-provider-http": ^0.3.0 "@metamask/keyring-api": ^6.1.1 @@ -1897,7 +1897,7 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@^9.1.0, @metamask/controller-utils@workspace:packages/controller-utils": +"@metamask/controller-utils@^10.0.0, @metamask/controller-utils@workspace:packages/controller-utils": version: 0.0.0-use.local resolution: "@metamask/controller-utils@workspace:packages/controller-utils" dependencies: @@ -1999,7 +1999,7 @@ __metadata: "@ethersproject/providers": ^5.7.0 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/network-controller": ^18.1.2 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 @@ -2310,7 +2310,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-unit": ^0.3.0 "@metamask/network-controller": ^18.1.2 @@ -2459,7 +2459,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@types/jest": ^27.4.1 deepmerge: ^4.2.2 jest: ^27.5.1 @@ -2477,7 +2477,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-sig-util": ^7.0.1 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 @@ -2506,7 +2506,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 async-mutex: ^0.2.6 @@ -2526,7 +2526,7 @@ __metadata: "@json-rpc-specification/meta-schema": ^1.0.6 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-block-tracker": ^9.0.2 "@metamask/eth-json-rpc-infura": ^9.1.0 "@metamask/eth-json-rpc-middleware": ^12.1.1 @@ -2622,7 +2622,7 @@ __metadata: "@metamask/approval-controller": ^6.0.2 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/json-rpc-engine": ^8.0.2 "@metamask/rpc-errors": ^6.2.1 "@metamask/utils": ^8.3.0 @@ -2669,7 +2669,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@types/jest": ^27.4.1 "@types/punycode": ^2.1.0 deepmerge: ^4.2.2 @@ -2691,7 +2691,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/network-controller": ^18.1.2 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 @@ -2726,7 +2726,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/keyring-controller": ^16.0.0 "@types/jest": ^27.4.1 deepmerge: ^4.2.2 @@ -2788,7 +2788,7 @@ __metadata: dependencies: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/json-rpc-engine": ^8.0.2 "@metamask/network-controller": ^18.1.2 "@metamask/rpc-errors": ^6.2.1 @@ -2892,7 +2892,7 @@ __metadata: "@metamask/approval-controller": ^6.0.2 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/keyring-controller": ^16.0.0 "@metamask/logging-controller": ^3.0.1 "@metamask/message-manager": ^8.0.2 @@ -3050,7 +3050,7 @@ __metadata: "@metamask/approval-controller": ^6.0.2 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-provider-http": ^0.3.0 "@metamask/gas-fee-controller": ^15.1.2 @@ -3092,7 +3092,7 @@ __metadata: "@metamask/approval-controller": ^6.0.2 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^5.0.2 - "@metamask/controller-utils": ^9.1.0 + "@metamask/controller-utils": ^10.0.0 "@metamask/eth-query": ^4.0.0 "@metamask/gas-fee-controller": ^15.1.2 "@metamask/keyring-controller": ^16.0.0 From 8fbe6f80d15b054742b1b75b9e496f6fc8563683 Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 30 May 2024 01:14:21 +0200 Subject: [PATCH 2/5] feat: migrate TokensController to BaseControllerV2 (#4304) ## Explanation The TokensController has been migrated to BaseControllerV2. As part of this migration, the deprecated config property has been removed and has been replaced with flatten properties on the controller constructor (`chainId`, `selectedAddress` and `provider`). PS: A migration is needed when using a new release of this controller. ## References * Fixes #4075 ## Changelog ### `@metamask/assets-controllers` #### Changed - **BREAKING:** Convert TokensController to `BaseControllerV2` - The constructor parameters have changed; rather than accepting a "config" parameter, it takes`selectedAddress` and `provider` parameters. - **BREAKING:** Convert Token object in TokenBalancesController to a `type` instead of `interface` and replace the `balanceError` property with `hasBalanceError` flag. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/TokenBalancesController.test.ts | 20 +- .../src/TokenBalancesController.ts | 16 +- .../src/TokenDetectionController.test.ts | 11 +- .../src/TokenRatesController.test.ts | 4 +- .../src/TokenRatesController.ts | 14 +- .../src/TokensController.test.ts | 180 ++------ .../src/TokensController.ts | 403 ++++++++++-------- packages/assets-controllers/src/index.ts | 3 +- 8 files changed, 285 insertions(+), 366 deletions(-) diff --git a/packages/assets-controllers/src/TokenBalancesController.test.ts b/packages/assets-controllers/src/TokenBalancesController.test.ts index 01d023a8cd..1d722b421c 100644 --- a/packages/assets-controllers/src/TokenBalancesController.test.ts +++ b/packages/assets-controllers/src/TokenBalancesController.test.ts @@ -10,7 +10,10 @@ import type { } from './TokenBalancesController'; import { TokenBalancesController } from './TokenBalancesController'; import type { Token } from './TokenRatesController'; -import { getDefaultTokensState, type TokensState } from './TokensController'; +import { + getDefaultTokensState, + type TokensControllerState, +} from './TokensController'; const controllerName = 'TokenBalancesController'; @@ -192,7 +195,7 @@ describe('TokenBalancesController', () => { getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), messenger, }); - const triggerTokensStateChange = async (state: TokensState) => { + const triggerTokensStateChange = async (state: TokensControllerState) => { controllerMessenger.publish('TokensController:stateChange', state, []); }; @@ -230,7 +233,7 @@ describe('TokenBalancesController', () => { getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), messenger, }); - const triggerTokensStateChange = async (state: TokensState) => { + const triggerTokensStateChange = async (state: TokensControllerState) => { controllerMessenger.publish('TokensController:stateChange', state, []); }; @@ -303,7 +306,7 @@ describe('TokenBalancesController', () => { await controller.updateBalances(); - expect(tokens[0].balanceError).toBeNull(); + expect(tokens[0].hasBalanceError).toBe(false); expect(Object.keys(controller.state.contractBalances)).toContain(address); expect(controller.state.contractBalances[address]).not.toBe(toHex(0)); }); @@ -338,15 +341,14 @@ describe('TokenBalancesController', () => { await controller.updateBalances(); - expect(tokens[0].balanceError).toBeInstanceOf(Error); - expect(tokens[0].balanceError).toHaveProperty('message', errorMsg); + expect(tokens[0].hasBalanceError).toBe(true); expect(controller.state.contractBalances[address]).toBe(toHex(0)); getERC20BalanceOfStub.mockReturnValue(new BN(1)); await controller.updateBalances(); - expect(tokens[0].balanceError).toBeNull(); + expect(tokens[0].hasBalanceError).toBe(false); expect(Object.keys(controller.state.contractBalances)).toContain(address); expect(controller.state.contractBalances[address]).not.toBe(0); }); @@ -361,7 +363,7 @@ describe('TokenBalancesController', () => { interval: 1337, messenger, }); - const triggerTokensStateChange = async (state: TokensState) => { + const triggerTokensStateChange = async (state: TokensControllerState) => { controllerMessenger.publish('TokensController:stateChange', state, []); }; const updateBalancesSpy = jest.spyOn(controller, 'updateBalances'); @@ -390,7 +392,7 @@ describe('TokenBalancesController', () => { getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), messenger, }); - const triggerTokensStateChange = async (state: TokensState) => { + const triggerTokensStateChange = async (state: TokensControllerState) => { controllerMessenger.publish('TokensController:stateChange', state, []); }; expect(controller.state.contractBalances).toStrictEqual({}); diff --git a/packages/assets-controllers/src/TokenBalancesController.ts b/packages/assets-controllers/src/TokenBalancesController.ts index 1acc2f226c..280793ef68 100644 --- a/packages/assets-controllers/src/TokenBalancesController.ts +++ b/packages/assets-controllers/src/TokenBalancesController.ts @@ -196,20 +196,20 @@ export class TokenBalancesController extends BaseController< return; } + const { selectedAddress } = this.messagingSystem.call( + 'PreferencesController:getState', + ); + const newContractBalances: ContractBalances = {}; for (const token of this.#tokens) { const { address } = token; - const { selectedAddress } = this.messagingSystem.call( - 'PreferencesController:getState', - ); try { - newContractBalances[address] = toHex( - await this.#getERC20BalanceOf(address, selectedAddress), - ); - token.balanceError = null; + const balance = await this.#getERC20BalanceOf(address, selectedAddress); + newContractBalances[address] = toHex(balance); + token.hasBalanceError = false; } catch (error) { newContractBalances[address] = toHex(0); - token.balanceError = error; + token.hasBalanceError = true; } } diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 00dde0671b..7b040fefee 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -44,7 +44,10 @@ import { type TokenListState, type TokenListToken, } from './TokenListController'; -import type { TokensController, TokensState } from './TokensController'; +import type { + TokensController, + TokensControllerState, +} from './TokensController'; import { getDefaultTokensState } from './TokensController'; const DEFAULT_INTERVAL = 180000; @@ -1996,7 +1999,7 @@ type WithControllerCallback = ({ controller: TokenDetectionController; mockGetSelectedAccount: (address: string) => void; mockKeyringGetState: (state: KeyringControllerState) => void; - mockTokensGetState: (state: TokensState) => void; + mockTokensGetState: (state: TokensControllerState) => void; mockTokenListGetState: (state: TokenListState) => void; mockPreferencesGetState: (state: PreferencesState) => void; mockGetNetworkClientById: ( @@ -2090,7 +2093,7 @@ async function withController( 'NetworkController:getState', mockNetworkState.mockReturnValue({ ...defaultNetworkState }), ); - const mockTokensState = jest.fn(); + const mockTokensState = jest.fn(); controllerMessenger.registerActionHandler( 'TokensController:getState', mockTokensState.mockReturnValue({ ...getDefaultTokensState() }), @@ -2133,7 +2136,7 @@ async function withController( mockKeyringGetState: (state: KeyringControllerState) => { mockKeyringState.mockReturnValue(state); }, - mockTokensGetState: (state: TokensState) => { + mockTokensGetState: (state: TokensControllerState) => { mockTokensState.mockReturnValue(state); }, mockPreferencesGetState: (state: PreferencesState) => { diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index a992905e23..bf7826f2c3 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -35,7 +35,7 @@ import type { Token, TokenRatesState, } from './TokenRatesController'; -import type { TokensState } from './TokensController'; +import type { TokensControllerState } from './TokensController'; const defaultSelectedAddress = '0x0000000000000000000000000000000000000001'; const mockTokenAddress = '0x0000000000000000000000000000000000000010'; @@ -2387,7 +2387,7 @@ describe('TokenRatesController', () => { type ControllerEvents = { networkStateChange: (state: NetworkState) => void; preferencesStateChange: (state: PreferencesState) => void; - tokensStateChange: (state: TokensState) => void; + tokensStateChange: (state: TokensControllerState) => void; }; /** diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 7d07b46d62..42e65672b5 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -19,7 +19,7 @@ import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import { ZERO_ADDRESS } from './token-prices-service/codefi-v2'; -import type { TokensState } from './TokensController'; +import type { TokensControllerState } from './TokensController'; /** * @type Token @@ -30,19 +30,17 @@ import type { TokensState } from './TokensController'; * @property symbol - Symbol of the token * @property image - Image of the token, url or bit32 image */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface Token { + +export type Token = { address: string; decimals: number; symbol: string; aggregators?: string[]; image?: string; - balanceError?: unknown; + hasBalanceError?: boolean; isERC721?: boolean; name?: string; -} +}; /** * @type TokenRatesConfig @@ -218,7 +216,7 @@ export class TokenRatesController extends StaticIntervalPollingControllerV1< listener: (preferencesState: PreferencesState) => void, ) => void; onTokensStateChange: ( - listener: (tokensState: TokensState) => void, + listener: (tokensState: TokensControllerState) => void, ) => void; onNetworkStateChange: ( listener: (networkState: NetworkState) => void, diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index 43a0044ee7..311ec8c23e 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -41,7 +41,10 @@ import { ERC1155Standard } from './Standards/NftStandards/ERC1155/ERC1155Standar import { TOKEN_END_POINT_API } from './token-service'; import type { Token } from './TokenRatesController'; import { TokensController } from './TokensController'; -import type { TokensControllerMessenger } from './TokensController'; +import type { + TokensControllerMessenger, + TokensControllerState, +} from './TokensController'; jest.mock('@ethersproject/contracts'); jest.mock('uuid'); @@ -729,9 +732,7 @@ describe('TokensController', () => { const address = erc721ContractAddresses[0]; const { symbol, decimals } = contractMaps[address]; - controller.update({ - tokens: [{ address, symbol, decimals }], - }); + await controller.addToken({ address, symbol, decimals }); const result = await controller.updateTokenType(address); expect(result.isERC721).toBe(true); @@ -747,9 +748,7 @@ describe('TokensController', () => { const address = erc20ContractAddresses[0]; const { symbol, decimals } = contractMaps[address]; - controller.update({ - tokens: [{ address, symbol, decimals }], - }); + await controller.addToken({ address, symbol, decimals }); const result = await controller.updateTokenType(address); expect(result.isERC721).toBe(false); @@ -763,14 +762,10 @@ describe('TokensController', () => { ); const tokenAddress = '0xda5584cc586d07c7141aa427224a4bd58e64af7d'; - controller.update({ - tokens: [ - { - address: tokenAddress, - symbol: 'TESTNFT', - decimals: 0, - }, - ], + await controller.addToken({ + address: tokenAddress, + symbol: 'TESTNFT', + decimals: 0, }); const result = await controller.updateTokenType(tokenAddress); @@ -785,14 +780,10 @@ describe('TokensController', () => { ); const tokenAddress = '0xda5584cc586d07c7141aa427224a4bd58e64af7d'; - controller.update({ - tokens: [ - { - address: tokenAddress, - symbol: 'TESTNFT', - decimals: 0, - }, - ], + await controller.addToken({ + address: tokenAddress, + symbol: 'TESTNFT', + decimals: 0, }); const result = await controller.updateTokenType(tokenAddress); @@ -927,9 +918,7 @@ describe('TokensController', () => { await withController( { options: { - config: { - chainId, - }, + chainId, }, }, async ({ controller }) => { @@ -1158,102 +1147,6 @@ describe('TokensController', () => { }); }); - describe('_getNewAllTokensState method', () => { - it('should nest newTokens under chain ID and selected address when provided with newTokens as input', async () => { - const dummySelectedAddress = '0x1'; - const dummyTokens: Token[] = [ - { - address: '0x01', - symbol: 'barA', - decimals: 2, - aggregators: [], - image: undefined, - }, - ]; - - await withController( - { - options: { - chainId: ChainId.mainnet, - config: { - selectedAddress: dummySelectedAddress, - }, - }, - }, - ({ controller }) => { - const processedTokens = controller._getNewAllTokensState({ - newTokens: dummyTokens, - }); - - expect( - processedTokens.newAllTokens[ChainId.mainnet][dummySelectedAddress], - ).toStrictEqual(dummyTokens); - }, - ); - }); - - it('should nest detectedTokens under chain ID and selected address when provided with detectedTokens as input', async () => { - const dummySelectedAddress = '0x1'; - const dummyTokens: Token[] = [ - { - address: '0x01', - symbol: 'barA', - decimals: 2, - aggregators: [], - image: undefined, - }, - ]; - - await withController( - { - options: { - chainId: ChainId.mainnet, - config: { - selectedAddress: dummySelectedAddress, - }, - }, - }, - ({ controller }) => { - const processedTokens = controller._getNewAllTokensState({ - newDetectedTokens: dummyTokens, - }); - - expect( - processedTokens.newAllDetectedTokens[ChainId.mainnet][ - dummySelectedAddress - ], - ).toStrictEqual(dummyTokens); - }, - ); - }); - - it('should nest ignoredTokens under chain ID and selected address when provided with ignoredTokens as input', async () => { - const dummySelectedAddress = '0x1'; - const dummyIgnoredTokens = ['0x01']; - - await withController( - { - options: { - chainId: ChainId.mainnet, - config: { - selectedAddress: dummySelectedAddress, - }, - }, - }, - ({ controller }) => { - const processedTokens = controller._getNewAllTokensState({ - newIgnoredTokens: dummyIgnoredTokens, - }); - expect( - processedTokens.newAllIgnoredTokens[ChainId.mainnet][ - dummySelectedAddress - ], - ).toStrictEqual(dummyIgnoredTokens); - }, - ); - }); - }); - describe('watchAsset', () => { it('should error if passed no type', async () => { await withController(async ({ controller }) => { @@ -1887,13 +1780,16 @@ describe('TokensController', () => { .mockReturnValueOnce('67890'); const acceptedRequest = new Promise((resolve) => { - controller.subscribe((state) => { - if ( - state.allTokens?.[chainId]?.[interactingAddress].length === 2 - ) { - resolve(); - } - }); + messenger.subscribe( + 'TokensController:stateChange', + (state: TokensControllerState) => { + if ( + state.allTokens?.[chainId]?.[interactingAddress].length === 2 + ) { + resolve(); + } + }, + ); }); const anotherAsset = buildTokenWithName({ @@ -2140,9 +2036,7 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - config: { - selectedAddress, - }, + selectedAddress, }, }, async ({ controller }) => { @@ -2173,9 +2067,7 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - config: { - selectedAddress, - }, + selectedAddress, }, }, async ({ controller }) => { @@ -2207,9 +2099,7 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - config: { - selectedAddress, - }, + selectedAddress, }, }, async ({ controller }) => { @@ -2355,14 +2245,12 @@ async function withController( }); const controller = new TokensController({ chainId: ChainId.mainnet, - config: { - selectedAddress: '0x1', - // The tests assume that this is set, but they shouldn't make that - // assumption. But we have to do this due to a bug in TokensController - // where the provider can possibly be `undefined` if `networkClientId` is - // not specified. - provider: new FakeProvider(), - }, + selectedAddress: '0x1', + // The tests assume that this is set, but they shouldn't make that + // assumption. But we have to do this due to a bug in TokensController + // where the provider can possibly be `undefined` if `networkClientId` is + // not specified. + provider: new FakeProvider(), messenger: controllerMessenger, ...options, }); diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index a96de9cd6c..07e960def9 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -2,11 +2,11 @@ import { Contract } from '@ethersproject/contracts'; import { Web3Provider } from '@ethersproject/providers'; import type { AddApprovalRequest } from '@metamask/approval-controller'; import type { - BaseConfig, - BaseState, RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; import contractsMap from '@metamask/contract-metadata'; import { toChecksumHexAddress, @@ -24,14 +24,16 @@ import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, NetworkControllerNetworkDidChangeEvent, + NetworkState, Provider, } from '@metamask/network-controller'; -import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller'; +import type { + PreferencesControllerStateChangeEvent, + PreferencesState, +} from '@metamask/preferences-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { EventEmitter } from 'events'; -import type { Patch } from 'immer/dist/immer'; import { v1 as random } from 'uuid'; import { formatAggregatorNames, formatIconUrlWithProxy } from './assetsUtil'; @@ -48,21 +50,6 @@ import type { } from './TokenListController'; import type { Token } from './TokenRatesController'; -/** - * @type TokensConfig - * - * Tokens controller configuration - * @property selectedAddress - Vault selected address - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TokensConfig extends BaseConfig { - selectedAddress: string; - chainId: Hex; - provider: Provider | undefined; -} - /** * @type SuggestedAssetMeta * @@ -82,7 +69,7 @@ type SuggestedAssetMeta = { }; /** - * @type TokensState + * @type TokensControllerState * * Assets controller state * @property tokens - List of tokens associated with the active network and address pair @@ -92,7 +79,7 @@ type SuggestedAssetMeta = { * @property allIgnoredTokens - Object containing hidden/ignored tokens by network and account * @property allDetectedTokens - Object containing tokens detected with non-zero balances */ -export type TokensState = { +export type TokensControllerState = { tokens: Token[]; ignoredTokens: string[]; detectedTokens: Token[]; @@ -101,20 +88,43 @@ export type TokensState = { allDetectedTokens: { [chainId: Hex]: { [key: string]: Token[] } }; }; -/** - * The name of the {@link TokensController}. - */ +const metadata = { + tokens: { + persist: true, + anonymous: false, + }, + ignoredTokens: { + persist: true, + anonymous: false, + }, + detectedTokens: { + persist: true, + anonymous: false, + }, + allTokens: { + persist: true, + anonymous: false, + }, + allIgnoredTokens: { + persist: true, + anonymous: false, + }, + allDetectedTokens: { + persist: true, + anonymous: false, + }, +}; + const controllerName = 'TokensController'; export type TokensControllerActions = | TokensControllerGetStateAction | TokensControllerAddDetectedTokensAction; -// TODO: Once `TokensController` is upgraded to V2, rewrite this type using the `ControllerGetStateAction` type, which constrains `TokensState` as `Record`. -export type TokensControllerGetStateAction = { - type: `${typeof controllerName}:getState`; - handler: () => TokensState; -}; +export type TokensControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + TokensControllerState +>; export type TokensControllerAddDetectedTokensAction = { type: `${typeof controllerName}:addDetectedTokens`; @@ -128,11 +138,10 @@ export type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; -// TODO: Once `TokensController` is upgraded to V2, rewrite this type using the `ControllerStateChangeEvent` type, which constrains `TokensState` as `Record`. -export type TokensControllerStateChangeEvent = { - type: `${typeof controllerName}:stateChange`; - payload: [TokensState, Patch[]]; -}; +export type TokensControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + TokensControllerState +>; export type TokensControllerEvents = TokensControllerStateChangeEvent; @@ -152,7 +161,7 @@ export type TokensControllerMessenger = RestrictedControllerMessenger< AllowedEvents['type'] >; -export const getDefaultTokensState = (): TokensState => { +export const getDefaultTokensState = (): TokensControllerState => { return { tokens: [], ignoredTokens: [], @@ -166,91 +175,60 @@ export const getDefaultTokensState = (): TokensState => { /** * Controller that stores assets and exposes convenience methods */ -export class TokensController extends BaseControllerV1< - TokensConfig, - TokensState & BaseState +export class TokensController extends BaseController< + typeof controllerName, + TokensControllerState, + TokensControllerMessenger > { - private readonly mutex = new Mutex(); + readonly #mutex = new Mutex(); - private abortController: AbortController; + #chainId: Hex; - private readonly messagingSystem: TokensControllerMessenger; + #selectedAddress: string; - /** - * Fetch metadata for a token. - * - * @param tokenAddress - The address of the token. - * @returns The token metadata. - */ - private async fetchTokenMetadata( - tokenAddress: string, - ): Promise { - try { - const token = await fetchTokenMetadata( - this.config.chainId, - tokenAddress, - this.abortController.signal, - ); - return token; - } catch (error) { - if ( - error instanceof Error && - error.message.includes(TOKEN_METADATA_NO_SUPPORT_ERROR) - ) { - return undefined; - } - throw error; - } - } - - /** - * EventEmitter instance used to listen to specific EIP747 events - */ - hub = new EventEmitter(); + #provider: Provider | undefined; - /** - * Name of this controller used during composition - */ - override name = 'TokensController'; + #abortController: AbortController; /** - * Creates a TokensController instance. - * - * @param options - The controller options. + * Tokens controller options + * @param options - Constructor options. * @param options.chainId - The chain ID of the current network. - * @param options.config - Initial options used to configure this controller. + * @param options.selectedAddress - Vault selected address + * @param options.provider - Network provider. * @param options.state - Initial state to set on this controller. * @param options.messenger - The controller messenger. */ constructor({ chainId: initialChainId, - config, + selectedAddress, + provider, state, messenger, }: { chainId: Hex; - config?: Partial; - state?: Partial; + selectedAddress: string; + provider: Provider | undefined; + state?: Partial; messenger: TokensControllerMessenger; }) { - super(config, state); + super({ + name: controllerName, + metadata, + messenger, + state: { + ...getDefaultTokensState(), + ...state, + }, + }); - this.defaultConfig = { - selectedAddress: '', - chainId: initialChainId, - provider: undefined, - ...config, - }; + this.#chainId = initialChainId; - this.defaultState = { - ...getDefaultTokensState(), - ...state, - }; + this.#provider = provider; - this.initialize(); - this.abortController = new AbortController(); + this.#selectedAddress = selectedAddress; - this.messagingSystem = messenger; + this.#abortController = new AbortController(); this.messagingSystem.registerActionHandler( `${controllerName}:addDetectedTokens` as const, @@ -259,33 +237,12 @@ export class TokensController extends BaseControllerV1< this.messagingSystem.subscribe( 'PreferencesController:stateChange', - ({ selectedAddress }) => { - const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; - const { chainId } = this.config; - this.configure({ selectedAddress }); - this.update({ - tokens: allTokens[chainId]?.[selectedAddress] ?? [], - ignoredTokens: allIgnoredTokens[chainId]?.[selectedAddress] ?? [], - detectedTokens: allDetectedTokens[chainId]?.[selectedAddress] ?? [], - }); - }, + this.#onPreferenceControllerStateChange.bind(this), ); this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - ({ providerConfig }) => { - const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; - const { selectedAddress } = this.config; - const { chainId } = providerConfig; - this.abortController.abort(); - this.abortController = new AbortController(); - this.configure({ chainId }); - this.update({ - tokens: allTokens[chainId]?.[selectedAddress] || [], - ignoredTokens: allIgnoredTokens[chainId]?.[selectedAddress] || [], - detectedTokens: allDetectedTokens[chainId]?.[selectedAddress] || [], - }); - }, + this.#onNetworkDidChange.bind(this), ); this.messagingSystem.subscribe( @@ -293,12 +250,77 @@ export class TokensController extends BaseControllerV1< ({ tokenList }) => { const { tokens } = this.state; if (tokens.length && !tokens[0].name) { - this.updateTokensAttribute(tokenList, 'name'); + this.#updateTokensAttribute(tokenList, 'name'); } }, ); } + /** + * Handles the event when the network changes. + * + * @param networkState - The changed network state. + * @param networkState.providerConfig - RPC URL and network name provider settings of the currently connected network + */ + #onNetworkDidChange({ providerConfig }: NetworkState) { + const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; + const { chainId } = providerConfig; + this.#abortController.abort(); + this.#abortController = new AbortController(); + this.#chainId = chainId; + this.update((state) => { + state.tokens = allTokens[chainId]?.[this.#selectedAddress] || []; + state.ignoredTokens = + allIgnoredTokens[chainId]?.[this.#selectedAddress] || []; + state.detectedTokens = + allDetectedTokens[chainId]?.[this.#selectedAddress] || []; + }); + } + + /** + * Handles the state change of the preference controller. + * @param preferencesState - The new state of the preference controller. + * @param preferencesState.selectedAddress - The current selected address of the preference controller. + */ + #onPreferenceControllerStateChange({ selectedAddress }: PreferencesState) { + const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; + this.#selectedAddress = selectedAddress; + this.update((state) => { + state.tokens = allTokens[this.#chainId]?.[selectedAddress] ?? []; + state.ignoredTokens = + allIgnoredTokens[this.#chainId]?.[selectedAddress] ?? []; + state.detectedTokens = + allDetectedTokens[this.#chainId]?.[selectedAddress] ?? []; + }); + } + + /** + * Fetch metadata for a token. + * + * @param tokenAddress - The address of the token. + * @returns The token metadata. + */ + async #fetchTokenMetadata( + tokenAddress: string, + ): Promise { + try { + const token = await fetchTokenMetadata( + this.#chainId, + tokenAddress, + this.#abortController.signal, + ); + return token; + } catch (error) { + if ( + error instanceof Error && + error.message.includes(TOKEN_METADATA_NO_SUPPORT_ERROR) + ) { + return undefined; + } + throw error; + } + } + /** * Adds a token to the stored token list. * @@ -329,8 +351,9 @@ export class TokensController extends BaseControllerV1< interactingAddress?: string; networkClientId?: NetworkClientId; }): Promise { - const { chainId, selectedAddress } = this.config; - const releaseLock = await this.mutex.acquire(); + const chainId = this.#chainId; + const selectedAddress = this.#selectedAddress; + const releaseLock = await this.#mutex.acquire(); const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; let currentChainId = chainId; if (networkClientId) { @@ -352,12 +375,12 @@ export class TokensController extends BaseControllerV1< allDetectedTokens[currentChainId]?.[accountAddress] || []; const newTokens: Token[] = [...tokens]; const [isERC721, tokenMetadata] = await Promise.all([ - this._detectIsERC721(address, networkClientId), + this.#detectIsERC721(address, networkClientId), // TODO parameterize the token metadata fetch by networkClientId - this.fetchTokenMetadata(address), + this.#fetchTokenMetadata(address), ]); // TODO remove this once this method is fully parameterized by networkClientId - if (!networkClientId && currentChainId !== this.config.chainId) { + if (!networkClientId && currentChainId !== this.#chainId) { throw new Error( 'TokensController Error: Switched networks while adding token', ); @@ -393,7 +416,7 @@ export class TokensController extends BaseControllerV1< ); const { newAllTokens, newAllIgnoredTokens, newAllDetectedTokens } = - this._getNewAllTokensState({ + this.#getNewAllTokensState({ newTokens, newIgnoredTokens, newDetectedTokens, @@ -401,7 +424,7 @@ export class TokensController extends BaseControllerV1< interactingChainId: currentChainId, }); - let newState: Partial = { + let newState: Partial = { allTokens: newAllTokens, allIgnoredTokens: newAllIgnoredTokens, allDetectedTokens: newAllDetectedTokens, @@ -417,7 +440,9 @@ export class TokensController extends BaseControllerV1< }; } - this.update(newState); + this.update((state) => { + Object.assign(state, newState); + }); return newTokens; } finally { releaseLock(); @@ -431,7 +456,7 @@ export class TokensController extends BaseControllerV1< * @param networkClientId - Optional network client ID used to determine interacting chain ID. */ async addTokens(tokensToImport: Token[], networkClientId?: NetworkClientId) { - const releaseLock = await this.mutex.acquire(); + const releaseLock = await this.#mutex.acquire(); const { tokens, detectedTokens, ignoredTokens } = this.state; const importedTokensMap: { [key: string]: true } = {}; // Used later to dedupe imported tokens @@ -474,20 +499,20 @@ export class TokensController extends BaseControllerV1< } const { newAllTokens, newAllDetectedTokens, newAllIgnoredTokens } = - this._getNewAllTokensState({ + this.#getNewAllTokensState({ newTokens, newDetectedTokens, newIgnoredTokens, interactingChainId, }); - this.update({ - tokens: newTokens, - allTokens: newAllTokens, - detectedTokens: newDetectedTokens, - allDetectedTokens: newAllDetectedTokens, - ignoredTokens: newIgnoredTokens, - allIgnoredTokens: newAllIgnoredTokens, + this.update((state) => { + state.tokens = newTokens; + state.allTokens = newAllTokens; + state.detectedTokens = newDetectedTokens; + state.allDetectedTokens = newAllDetectedTokens; + state.ignoredTokens = newIgnoredTokens; + state.allIgnoredTokens = newAllIgnoredTokens; }); } finally { releaseLock(); @@ -518,19 +543,19 @@ export class TokensController extends BaseControllerV1< ); const { newAllIgnoredTokens, newAllDetectedTokens, newAllTokens } = - this._getNewAllTokensState({ + this.#getNewAllTokensState({ newIgnoredTokens, newDetectedTokens, newTokens, }); - this.update({ - ignoredTokens: newIgnoredTokens, - tokens: newTokens, - detectedTokens: newDetectedTokens, - allIgnoredTokens: newAllIgnoredTokens, - allDetectedTokens: newAllDetectedTokens, - allTokens: newAllTokens, + this.update((state) => { + state.ignoredTokens = newIgnoredTokens; + state.tokens = newTokens; + state.detectedTokens = newDetectedTokens; + state.allIgnoredTokens = newAllIgnoredTokens; + state.allDetectedTokens = newAllDetectedTokens; + state.allTokens = newAllTokens; }); } @@ -546,12 +571,12 @@ export class TokensController extends BaseControllerV1< incomingDetectedTokens: Token[], detectionDetails?: { selectedAddress: string; chainId: Hex }, ) { - const releaseLock = await this.mutex.acquire(); + const releaseLock = await this.#mutex.acquire(); // Get existing tokens for the chain + account - const chainId = detectionDetails?.chainId ?? this.config.chainId; + const chainId = detectionDetails?.chainId ?? this.#chainId; const accountAddress = - detectionDetails?.selectedAddress ?? this.config.selectedAddress; + detectionDetails?.selectedAddress ?? this.#selectedAddress; const { allTokens, allDetectedTokens, allIgnoredTokens } = this.state; let newTokens = [...(allTokens?.[chainId]?.[accountAddress] ?? [])]; @@ -607,7 +632,7 @@ export class TokensController extends BaseControllerV1< } }); - const { newAllTokens, newAllDetectedTokens } = this._getNewAllTokensState( + const { newAllTokens, newAllDetectedTokens } = this.#getNewAllTokensState( { newTokens, newDetectedTokens, @@ -618,18 +643,15 @@ export class TokensController extends BaseControllerV1< // We may be detecting tokens on a different chain/account pair than are currently configured. // Re-point `tokens` and `detectedTokens` to keep them referencing the current chain/account. - const { chainId: currentChain, selectedAddress: currentAddress } = - this.config; - - newTokens = newAllTokens?.[currentChain]?.[currentAddress] || []; + newTokens = newAllTokens?.[this.#chainId]?.[this.#selectedAddress] || []; newDetectedTokens = - newAllDetectedTokens?.[currentChain]?.[currentAddress] || []; + newAllDetectedTokens?.[this.#chainId]?.[this.#selectedAddress] || []; - this.update({ - tokens: newTokens, - allTokens: newAllTokens, - detectedTokens: newDetectedTokens, - allDetectedTokens: newAllDetectedTokens, + this.update((state) => { + state.tokens = newTokens; + state.allTokens = newAllTokens; + state.detectedTokens = newDetectedTokens; + state.allDetectedTokens = newAllDetectedTokens; }); } finally { releaseLock(); @@ -644,14 +666,17 @@ export class TokensController extends BaseControllerV1< * @returns The new token object with the added isERC721 field. */ async updateTokenType(tokenAddress: string) { - const isERC721 = await this._detectIsERC721(tokenAddress); - const { tokens } = this.state; + const isERC721 = await this.#detectIsERC721(tokenAddress); + const tokens = [...this.state.tokens]; const tokenIndex = tokens.findIndex((token) => { return token.address.toLowerCase() === tokenAddress.toLowerCase(); }); - tokens[tokenIndex].isERC721 = isERC721; - this.update({ tokens }); - return tokens[tokenIndex]; + const updatedToken = { ...tokens[tokenIndex], isERC721 }; + tokens[tokenIndex] = updatedToken; + this.update((state) => { + state.tokens = tokens; + }); + return updatedToken; } /** @@ -660,7 +685,7 @@ export class TokensController extends BaseControllerV1< * @param tokenList - Represents the fetched token list from service API * @param tokenAttribute - Represents the token attribute that we want to update on the token list */ - private updateTokensAttribute( + #updateTokensAttribute( tokenList: TokenListMap, tokenAttribute: keyof Token & keyof TokenListToken, ) { @@ -674,7 +699,9 @@ export class TokensController extends BaseControllerV1< : { ...token }; }); - this.update({ tokens: newTokens }); + this.update((state) => { + state.tokens = newTokens; + }); } /** @@ -685,7 +712,7 @@ export class TokensController extends BaseControllerV1< * @returns A boolean indicating whether the token address passed in supports the EIP-721 * interface. */ - async _detectIsERC721( + async #detectIsERC721( tokenAddress: string, networkClientId?: NetworkClientId, ) { @@ -698,7 +725,7 @@ export class TokensController extends BaseControllerV1< return Promise.resolve(false); } - const tokenContract = this._createEthersContract( + const tokenContract = this.#createEthersContract( tokenAddress, abiERC721, networkClientId, @@ -714,7 +741,7 @@ export class TokensController extends BaseControllerV1< } } - _getProvider(networkClientId?: NetworkClientId): Web3Provider { + #getProvider(networkClientId?: NetworkClientId): Web3Provider { return new Web3Provider( // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released networkClientId @@ -722,21 +749,21 @@ export class TokensController extends BaseControllerV1< 'NetworkController:getNetworkClientById', networkClientId, ).provider - : this.config.provider, + : this.#provider, ); } - _createEthersContract( + #createEthersContract( tokenAddress: string, abi: string, networkClientId?: NetworkClientId, ): Contract { - const web3provider = this._getProvider(networkClientId); + const web3provider = this.#getProvider(networkClientId); const tokenContract = new Contract(tokenAddress, abi, web3provider); return tokenContract; } - _generateRandomId(): string { + #generateRandomId(): string { return random(); } @@ -776,13 +803,13 @@ export class TokensController extends BaseControllerV1< // Validate contract - if (await this._detectIsERC721(asset.address, networkClientId)) { + if (await this.#detectIsERC721(asset.address, networkClientId)) { throw rpcErrors.invalidParams( `Contract ${asset.address} must match type ${type}, but was detected as ${ERC721}`, ); } - const provider = this._getProvider(networkClientId); + const provider = this.#getProvider(networkClientId); const isErc1155 = await safelyExecute(() => new ERC1155Standard(provider).contractSupportsBase1155Interface( asset.address, @@ -861,13 +888,13 @@ export class TokensController extends BaseControllerV1< const suggestedAssetMeta: SuggestedAssetMeta = { asset, - id: this._generateRandomId(), + id: this.#generateRandomId(), time: Date.now(), type, - interactingAddress: interactingAddress || this.config.selectedAddress, + interactingAddress: interactingAddress || this.#selectedAddress, }; - await this._requestApproval(suggestedAssetMeta); + await this.#requestApproval(suggestedAssetMeta); const { address, symbol, decimals, name, image } = asset; await this.addToken({ @@ -893,7 +920,7 @@ export class TokensController extends BaseControllerV1< * @param params.interactingChainId - The chainId to use to store the tokens. * @returns The updated `allTokens` and `allIgnoredTokens` state. */ - _getNewAllTokensState(params: { + #getNewAllTokensState(params: { newTokens?: Token[]; newIgnoredTokens?: string[]; newDetectedTokens?: Token[]; @@ -908,10 +935,9 @@ export class TokensController extends BaseControllerV1< interactingChainId, } = params; const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; - const { chainId, selectedAddress } = this.config; - const userAddressToAddTokens = interactingAddress ?? selectedAddress; - const chainIdToAddTokens = interactingChainId ?? chainId; + const userAddressToAddTokens = interactingAddress ?? this.#selectedAddress; + const chainIdToAddTokens = interactingChainId ?? this.#chainId; let newAllTokens = allTokens; if ( @@ -976,10 +1002,13 @@ export class TokensController extends BaseControllerV1< * Removes all tokens from the ignored list. */ clearIgnoredTokens() { - this.update({ ignoredTokens: [], allIgnoredTokens: {} }); + this.update((state) => { + state.ignoredTokens = []; + state.allIgnoredTokens = {}; + }); } - async _requestApproval(suggestedAssetMeta: SuggestedAssetMeta) { + async #requestApproval(suggestedAssetMeta: SuggestedAssetMeta) { return this.messagingSystem.call( 'ApprovalController:addRequest', { diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index b276b2ccd3..1322c58d39 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -38,8 +38,7 @@ export type { } from './TokenRatesController'; export { TokenRatesController } from './TokenRatesController'; export type { - TokensConfig, - TokensState, + TokensControllerState, TokensControllerActions, TokensControllerGetStateAction, TokensControllerAddDetectedTokensAction, From f522918bace3ab98978d2b10cb15d84293b52a7d Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 30 May 2024 10:42:28 +0200 Subject: [PATCH 3/5] feat: migrate NftDetectionController to BaseControllerV2 (#4312) ## Explanation The NftDetectionController has been migrated to BaseControllerV2. As part of this migration, the deprecated config property has been removed and replaced with messenger actions for `chainId` and `selectedAddress` retrieval.. ## References * Fixes #4074 ## Changelog ### `@metamask/assets-controllers` #### Changed - **BREAKING:** Convert NftDetectionController to `BaseControllerV2` ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/NftDetectionController.test.ts | 323 +++++++++------ .../src/NftDetectionController.ts | 391 ++++++++---------- packages/controller-utils/src/constants.ts | 4 + 3 files changed, 376 insertions(+), 342 deletions(-) diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index 5f6ca9a8ff..11b90114dd 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -1,4 +1,5 @@ -import { NFT_API_BASE_URL, ChainId, toHex } from '@metamask/controller-utils'; +import { ControllerMessenger } from '@metamask/base-controller'; +import { NFT_API_BASE_URL, ChainId } from '@metamask/controller-utils'; import { NetworkClientType, defaultState as defaultNetworkState, @@ -24,15 +25,18 @@ import { buildMockGetNetworkClientById, } from '../../network-controller/tests/helpers'; import { Source } from './constants'; -import { getDefaultNftState, type NftState } from './NftController'; +import { getDefaultNftState } from './NftController'; import { - type NftDetectionConfig, NftDetectionController, BlockaidResultType, + type AllowedActions, + type AllowedEvents, } from './NftDetectionController'; const DEFAULT_INTERVAL = 180000; +const controllerName = 'NftDetectionController' as const; + describe('NftDetectionController', () => { let clock: sinon.SinonFakeTimers; @@ -283,25 +287,14 @@ describe('NftDetectionController', () => { sinon.restore(); }); - it('should set default config', async () => { - await withController(({ controller }) => { - expect(controller.config).toStrictEqual({ - interval: DEFAULT_INTERVAL, - chainId: toHex(1), - selectedAddress: '', - disabled: true, - }); - }); - }); - it('should poll and detect NFTs on interval while on mainnet', async () => { await withController( - { config: { interval: 10 } }, + { options: { interval: 10 } }, async ({ controller, controllerEvents }) => { const mockNfts = sinon .stub(controller, 'detectNfts') .returns(Promise.resolve()); - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, }); @@ -425,7 +418,7 @@ describe('NftDetectionController', () => { ], ]); - controllerEvents.networkStateChange({ + controllerEvents.triggerNetworkStateChange({ ...defaultNetworkState, selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', }); @@ -460,13 +453,36 @@ describe('NftDetectionController', () => { ); }); - it('should detect mainnet correctly', async () => { - await withController(({ controller }) => { - controller.configure({ chainId: ChainId.mainnet }); - expect(controller.isMainnet()).toBe(true); - controller.configure({ chainId: ChainId.goerli }); - expect(controller.isMainnet()).toBe(false); - }); + it('should detect mainnet truthy', async () => { + await withController( + { + mockNetworkState: { + selectedNetworkClientId: 'mainnet', + }, + mockPreferencesState: { + selectedAddress: '', + }, + }, + ({ controller }) => { + expect(controller.isMainnet()).toBe(true); + }, + ); + }); + + it('should detect mainnet falsy', async () => { + await withController( + { + mockNetworkState: { + selectedNetworkClientId: 'goerli', + }, + mockPreferencesState: { + selectedAddress: '', + }, + }, + ({ controller }) => { + expect(controller.isMainnet()).toBe(false); + }, + ); }); it('should not autodetect while not on mainnet', async () => { @@ -486,20 +502,22 @@ describe('NftDetectionController', () => { await withController( { - config: { - interval: pollingInterval, - }, options: { + interval: pollingInterval, addNft: mockAddNft, - chainId: '0x1', disabled: false, - selectedAddress: '0x1', }, mockNetworkClientConfigurationsByNetworkClientId: { 'AAAA-AAAA-AAAA-AAAA': buildCustomNetworkClientConfiguration({ chainId: '0x123', }), }, + mockNetworkState: { + selectedNetworkClientId: 'mainnet', + }, + mockPreferencesState: { + selectedAddress: '0x1', + }, }, async ({ controller, controllerEvents }) => { await controller.start(); @@ -561,7 +579,7 @@ describe('NftDetectionController', () => { }, ); - controllerEvents.networkStateChange({ + controllerEvents.triggerNetworkStateChange({ ...defaultNetworkState, selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', }); @@ -576,11 +594,16 @@ describe('NftDetectionController', () => { it('should detect and add NFTs correctly when blockaid result is not included in response', async () => { const mockAddNft = jest.fn(); + const selectedAddress = '0x1'; await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft }, + mockPreferencesState: { + selectedAddress, + }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x1'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -616,11 +639,14 @@ describe('NftDetectionController', () => { it('should detect and add NFTs correctly when blockaid result is in response', async () => { const mockAddNft = jest.fn(); + const selectedAddress = '0x123'; await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x123'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -665,11 +691,14 @@ describe('NftDetectionController', () => { it('should detect and add NFTs and filter them correctly', async () => { const mockAddNft = jest.fn(); + const selectedAddress = '0x12345'; await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x12345'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -730,7 +759,7 @@ describe('NftDetectionController', () => { { options: { addNft: mockAddNft } }, async ({ controller, controllerEvents }) => { const selectedAddress = '0x1'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -783,11 +812,14 @@ describe('NftDetectionController', () => { ], }; }); + const selectedAddress = '0x9'; await withController( - { options: { addNft: mockAddNft, getNftState: mockGetNftState } }, + { + options: { addNft: mockAddNft, getNftState: mockGetNftState }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x9'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -808,19 +840,19 @@ describe('NftDetectionController', () => { it('should not detect and add NFTs if there is no selectedAddress', async () => { const mockAddNft = jest.fn(); + const selectedAddress = ''; // Emtpy selected address await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = ''; // Emtpy selected address - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, // auto-detect is enabled so it proceeds to check userAddress }); - // confirm that default selected address is an empty string - expect(controller.config.selectedAddress).toBe(''); - await controller.detectNfts(); expect(mockAddNft).not.toHaveBeenCalled(); @@ -832,7 +864,7 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); const mockNetworkClient: NetworkClient = { configuration: { - chainId: toHex(1), + chainId: ChainId.mainnet, rpcUrl: 'https://test.network', ticker: 'TEST', type: NetworkClientType.Custom, @@ -854,10 +886,10 @@ describe('NftDetectionController', () => { it('should not detectNfts when disabled is false and useNftDetection is true', async () => { await withController( - { config: { interval: 10 }, options: { disabled: false } }, + { options: { disabled: false, interval: 10 } }, async ({ controller, controllerEvents }) => { const mockNfts = sinon.stub(controller, 'detectNfts'); - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, }); @@ -881,11 +913,14 @@ describe('NftDetectionController', () => { it('should not detect and add NFTs if preferences controller useNftDetection is set to false', async () => { const mockAddNft = jest.fn(); + const selectedAddress = '0x9'; await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft, disabled: false }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x9'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: false, @@ -920,7 +955,7 @@ describe('NftDetectionController', () => { await withController( { options: { addNft: mockAddNft } }, async ({ controller, controllerEvents }) => { - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -941,53 +976,59 @@ describe('NftDetectionController', () => { it('should rethrow error when Nft APi server fails with error other than fetch failure', async () => { const selectedAddress = '0x4'; - await withController(async ({ controller, controllerEvents }) => { - // This mock is for the initial detect call after preferences change - nock(NFT_API_BASE_URL) - .get(`/users/${selectedAddress}/tokens`) - .query({ - continuation: '', - limit: '50', - chainIds: '1', - includeTopBid: true, - }) - .reply(200, { - tokens: [], + await withController( + { mockPreferencesState: { selectedAddress } }, + async ({ controller, controllerEvents }) => { + // This mock is for the initial detect call after preferences change + nock(NFT_API_BASE_URL) + .get(`/users/${selectedAddress}/tokens`) + .query({ + continuation: '', + limit: '50', + chainIds: '1', + includeTopBid: true, + }) + .reply(200, { + tokens: [], + }); + controllerEvents.triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + selectedAddress, + useNftDetection: true, }); - controllerEvents.preferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - useNftDetection: true, - }); - // Wait for detect call triggered by preferences state change to settle - await advanceTime({ - clock, - duration: 1, - }); - // This mock is for the call under test - nock(NFT_API_BASE_URL) - .get(`/users/${selectedAddress}/tokens`) - .query({ - continuation: '', - limit: '50', - chainIds: '1', - includeTopBid: true, - }) - .replyWithError(new Error('UNEXPECTED ERROR')); - - await expect(() => controller.detectNfts()).rejects.toThrow( - 'UNEXPECTED ERROR', - ); - }); + // Wait for detect call triggered by preferences state change to settle + await advanceTime({ + clock, + duration: 1, + }); + // This mock is for the call under test + nock(NFT_API_BASE_URL) + .get(`/users/${selectedAddress}/tokens`) + .query({ + continuation: '', + limit: '50', + chainIds: '1', + includeTopBid: true, + }) + .replyWithError(new Error('UNEXPECTED ERROR')); + + await expect(() => controller.detectNfts()).rejects.toThrow( + 'UNEXPECTED ERROR', + ); + }, + ); }); it('should rethrow error when attempt to add NFT fails', async () => { const mockAddNft = jest.fn(); + const selectedAddress = '0x1'; await withController( - { options: { addNft: mockAddNft } }, + { + options: { addNft: mockAddNft }, + mockPreferencesState: { selectedAddress }, + }, async ({ controller, controllerEvents }) => { - const selectedAddress = '0x1'; - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -1013,7 +1054,7 @@ describe('NftDetectionController', () => { // Repeated preference changes should only trigger 1 detection for (let i = 0; i < 5; i++) { - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, }); @@ -1022,7 +1063,7 @@ describe('NftDetectionController', () => { expect(detectNfts.callCount).toBe(1); // Irrelevant preference changes shouldn't trigger a detection - controllerEvents.preferencesStateChange({ + controllerEvents.triggerPreferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, securityAlertsEnabled: true, @@ -1037,9 +1078,8 @@ describe('NftDetectionController', () => { * A collection of mock external controller events. */ type ControllerEvents = { - nftsStateChange: (state: NftState) => void; - preferencesStateChange: (state: PreferencesState) => void; - networkStateChange: (state: NetworkState) => void; + triggerPreferencesStateChange: (state: PreferencesState) => void; + triggerNetworkStateChange: (state: NetworkState) => void; }; type WithControllerCallback = ({ @@ -1051,11 +1091,12 @@ type WithControllerCallback = ({ type WithControllerOptions = { options?: Partial[0]>; - config?: Partial; mockNetworkClientConfigurationsByNetworkClientId?: Record< NetworkClientId, NetworkClientConfiguration >; + mockNetworkState?: Partial; + mockPreferencesState?: Partial; }; type WithControllerArgs = @@ -1077,43 +1118,69 @@ async function withController( const [ { options = {}, - config = {}, mockNetworkClientConfigurationsByNetworkClientId = {}, + mockNetworkState = {}, + mockPreferencesState = {}, }, testFunction, ] = args.length === 2 ? args : [{}, args[0]]; - // Explicit cast used here because we know the `on____` functions are always - // set in the constructor. - const controllerEvents = {} as ControllerEvents; + const messenger = new ControllerMessenger(); + + messenger.registerActionHandler( + 'NetworkController:getState', + jest.fn().mockReturnValue({ + ...defaultNetworkState, + ...mockNetworkState, + }), + ); const getNetworkClientById = buildMockGetNetworkClientById( mockNetworkClientConfigurationsByNetworkClientId, ); + messenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientById, + ); - const controller = new NftDetectionController( - { - chainId: ChainId.mainnet, - onNftsStateChange: (listener) => { - controllerEvents.nftsStateChange = listener; - }, - onPreferencesStateChange: (listener) => { - controllerEvents.preferencesStateChange = listener; - }, - onNetworkStateChange: (listener) => { - controllerEvents.networkStateChange = listener; - }, - getOpenSeaApiKey: jest.fn(), - addNft: jest.fn(), - getNftApi: jest.fn(), - getNetworkClientById, - getNftState: getDefaultNftState, - disabled: true, - selectedAddress: '', - ...options, - }, - config, + messenger.registerActionHandler( + 'PreferencesController:getState', + jest.fn().mockReturnValue({ + ...getDefaultPreferencesState(), + ...mockPreferencesState, + }), ); + + const controllerMessenger = messenger.getRestricted({ + name: controllerName, + allowedActions: [ + 'NetworkController:getState', + 'NetworkController:getNetworkClientById', + 'PreferencesController:getState', + ], + allowedEvents: [ + 'NetworkController:stateChange', + 'PreferencesController:stateChange', + ], + }); + + const controller = new NftDetectionController({ + messenger: controllerMessenger, + disabled: true, + addNft: jest.fn(), + getNftState: getDefaultNftState, + ...options, + }); + + const controllerEvents = { + triggerPreferencesStateChange: (state: PreferencesState) => { + messenger.publish('PreferencesController:stateChange', state, []); + }, + triggerNetworkStateChange: (state: NetworkState) => { + messenger.publish('NetworkController:stateChange', state, []); + }, + }; + try { return await testFunction({ controller, diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index e47251fbd2..7b8d5171b0 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -1,19 +1,26 @@ -import type { BaseConfig, BaseState } from '@metamask/base-controller'; +import type { AddApprovalRequest } from '@metamask/approval-controller'; +import type { RestrictedControllerMessenger } from '@metamask/base-controller'; import { fetchWithErrorHandling, toChecksumHexAddress, ChainId, NFT_API_BASE_URL, + NFT_API_VERSION, + NFT_API_TIMEOUT, } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkController, - NetworkState, NetworkClient, + NetworkControllerGetNetworkClientByIdAction, + NetworkControllerStateChangeEvent, + NetworkControllerGetStateAction, } from '@metamask/network-controller'; -import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; -import type { PreferencesState } from '@metamask/preferences-controller'; -import type { Hex } from '@metamask/utils'; +import { StaticIntervalPollingController } from '@metamask/polling-controller'; +import type { + PreferencesControllerGetStateAction, + PreferencesControllerStateChangeEvent, + PreferencesState, +} from '@metamask/preferences-controller'; import { Source } from './constants'; import { @@ -24,6 +31,26 @@ import { const DEFAULT_INTERVAL = 180000; +const controllerName = 'NftDetectionController'; + +export type AllowedActions = + | AddApprovalRequest + | NetworkControllerGetStateAction + | NetworkControllerGetNetworkClientByIdAction + | PreferencesControllerGetStateAction; + +export type AllowedEvents = + | PreferencesControllerStateChangeEvent + | NetworkControllerStateChangeEvent; + +export type NftDetectionControllerMessenger = RestrictedControllerMessenger< + typeof controllerName, + AllowedActions, + AllowedEvents, + AllowedActions['type'], + AllowedEvents['type'] +>; + /** * @type ApiNft * @@ -44,10 +71,7 @@ const DEFAULT_INTERVAL = 180000; * @property creator - The NFT owner information object * @property lastSale - When this item was last sold */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface ApiNft { +export type ApiNft = { token_id: string; num_sales: number | null; background_color: string | null; @@ -63,7 +87,7 @@ export interface ApiNft { asset_contract: ApiNftContract; creator: ApiNftCreator; last_sale: ApiNftLastSale | null; -} +}; /** * @type ApiNftContract @@ -79,10 +103,7 @@ export interface ApiNft { * @property description - The NFT contract description * @property external_link - External link containing additional information */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface ApiNftContract { +export type ApiNftContract = { address: string; asset_contract_type: string | null; created_date: string | null; @@ -96,7 +117,7 @@ export interface ApiNftContract { image_url?: string | null; tokenCount?: string | null; }; -} +}; /** * @type ApiNftLastSale @@ -106,14 +127,11 @@ export interface ApiNftContract { * @property total_price - URI of NFT image associated with this owner * @property transaction - Object containing transaction_hash and block_hash */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface ApiNftLastSale { +export type ApiNftLastSale = { event_timestamp: string; total_price: string; transaction: { transaction_hash: string; block_hash: string }; -} +}; /** * @type ApiNftCreator @@ -123,31 +141,11 @@ export interface ApiNftLastSale { * @property profile_img_url - URI of NFT image associated with this owner * @property address - The owner address */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface ApiNftCreator { +export type ApiNftCreator = { user: { username: string }; profile_img_url: string; address: string; -} - -/** - * @type NftDetectionConfig - * - * NftDetection configuration - * @property interval - Polling interval used to fetch new token rates - * @property chainId - Current chain ID - * @property selectedAddress - Vault selected address - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftDetectionConfig extends BaseConfig { - interval: number; - chainId: Hex; - selectedAddress: string; -} +}; export type ReservoirResponse = { tokens: TokensResponse[]; @@ -350,162 +348,62 @@ export type Metadata = { /** * Controller that passively polls on a set interval for NFT auto detection */ -export class NftDetectionController extends StaticIntervalPollingControllerV1< - NftDetectionConfig, - BaseState +export class NftDetectionController extends StaticIntervalPollingController< + typeof controllerName, + Record, + NftDetectionControllerMessenger > { - private intervalId?: ReturnType; - - private getOwnerNftApi({ - address, - next, - }: { - address: string; - next?: string; - }) { - return `${NFT_API_BASE_URL}/users/${address}/tokens?chainIds=1&limit=50&includeTopBid=true&continuation=${ - next ?? '' - }`; - } - - private async getOwnerNfts(address: string) { - let nftApiResponse: ReservoirResponse; - let nfts: TokensResponse[] = []; - let next; - - do { - nftApiResponse = await fetchWithErrorHandling({ - url: this.getOwnerNftApi({ address, next }), - options: { - headers: { - Version: '1', - }, - }, - timeout: 15000, - }); - - if (!nftApiResponse) { - return nfts; - } - - const newNfts = nftApiResponse.tokens.filter( - (elm) => - elm.token.isSpam === false && - (elm.blockaidResult?.result_type - ? elm.blockaidResult?.result_type === BlockaidResultType.Benign - : true), - ); - - nfts = [...nfts, ...newNfts]; - } while ((next = nftApiResponse.continuation)); - - return nfts; - } - - /** - * Name of this controller used during composition - */ - override name = 'NftDetectionController'; + #intervalId?: ReturnType; - private readonly getOpenSeaApiKey: () => string | undefined; + #interval: number; - private readonly addNft: NftController['addNft']; + #disabled: boolean; - private readonly getNftApi: NftController['getNftApi']; + readonly #addNft: NftController['addNft']; - private readonly getNftState: () => NftState; - - private readonly getNetworkClientById: NetworkController['getNetworkClientById']; + readonly #getNftState: () => NftState; /** - * Creates an NftDetectionController instance. + * The controller options * * @param options - The controller options. - * @param options.chainId - The chain ID of the current network. - * @param options.onNftsStateChange - Allows subscribing to assets controller state changes. - * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. - * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. - * @param options.getOpenSeaApiKey - Gets the OpenSea API key, if one is set. + * @param options.interval - The pooling interval. + * @param options.messenger - A reference to the messaging system. + * @param options.disabled - Represents previous value of useNftDetection. Used to detect changes of useNftDetection. Default value is true. * @param options.addNft - Add an NFT. - * @param options.getNftApi - Gets the URL to fetch an NFT from OpenSea. * @param options.getNftState - Gets the current state of the Assets controller. - * @param options.disabled - Represents previous value of useNftDetection. Used to detect changes of useNftDetection. Default value is true. - * @param options.selectedAddress - Represents current selected address. - * @param options.getNetworkClientById - Gets the network client by ID, from the NetworkController. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. */ - constructor( - { - chainId: initialChainId, - getNetworkClientById, - onPreferencesStateChange, - onNetworkStateChange, - getOpenSeaApiKey, - addNft, - getNftApi, - getNftState, - disabled: initialDisabled, - selectedAddress: initialSelectedAddress, - }: { - chainId: Hex; - getNetworkClientById: NetworkController['getNetworkClientById']; - onNftsStateChange: (listener: (nftsState: NftState) => void) => void; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - getOpenSeaApiKey: () => string | undefined; - addNft: NftController['addNft']; - getNftApi: NftController['getNftApi']; - getNftState: () => NftState; - disabled: boolean; - selectedAddress: string; - }, - config?: Partial, - state?: Partial, - ) { - super(config, state); - this.defaultConfig = { - interval: DEFAULT_INTERVAL, - chainId: initialChainId, - selectedAddress: initialSelectedAddress, - disabled: initialDisabled, - }; - this.initialize(); - this.getNftState = getNftState; - this.getNetworkClientById = getNetworkClientById; - onPreferencesStateChange(({ selectedAddress, useNftDetection }) => { - const { selectedAddress: previouslySelectedAddress, disabled } = - this.config; - - if ( - selectedAddress !== previouslySelectedAddress || - !useNftDetection !== disabled - ) { - this.configure({ selectedAddress, disabled: !useNftDetection }); - if (useNftDetection) { - this.start(); - } else { - this.stop(); - } - } + constructor({ + interval = DEFAULT_INTERVAL, + messenger, + disabled = false, + addNft, + getNftState, + }: { + interval?: number; + messenger: NftDetectionControllerMessenger; + disabled: boolean; + addNft: NftController['addNft']; + getNftState: () => NftState; + }) { + super({ + name: controllerName, + messenger, + metadata: {}, + state: {}, }); + this.#interval = interval; + this.#disabled = disabled; - onNetworkStateChange(({ selectedNetworkClientId }) => { - const selectedNetworkClient = getNetworkClientById( - selectedNetworkClientId, - ); - const { chainId } = selectedNetworkClient.configuration; + this.#getNftState = getNftState; + this.#addNft = addNft; - this.configure({ chainId }); - }); - this.getOpenSeaApiKey = getOpenSeaApiKey; - this.addNft = addNft; - this.getNftApi = getNftApi; - this.setIntervalLength(this.config.interval); + this.messagingSystem.subscribe( + 'PreferencesController:stateChange', + this.#onPreferencesControllerStateChange.bind(this), + ); + + this.setIntervalLength(this.#interval); } async _executePoll( @@ -519,38 +417,36 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< * Start polling for the currency rate. */ async start() { - if (!this.isMainnet() || this.disabled) { + if (!this.isMainnet() || this.#disabled) { return; } - await this.startPolling(); + await this.#startPolling(); } /** * Stop polling for the currency rate. */ stop() { - this.stopPolling(); + this.#stopPolling(); } - private stopPolling() { - if (this.intervalId) { - clearInterval(this.intervalId); + #stopPolling() { + if (this.#intervalId) { + clearInterval(this.#intervalId); } } /** * Starts a new polling interval. * - * @param interval - An interval on which to poll. */ - private async startPolling(interval?: number): Promise { - interval && this.configure({ interval }, false, false); - this.stopPolling(); + async #startPolling(): Promise { + this.#stopPolling(); await this.detectNfts(); - this.intervalId = setInterval(async () => { + this.#intervalId = setInterval(async () => { await this.detectNfts(); - }, this.config.interval); + }, this.#interval); } /** @@ -558,11 +454,79 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< * * @returns Whether current network is mainnet. */ - isMainnet = (): boolean => this.config.chainId === ChainId.mainnet; + isMainnet(): boolean { + const { selectedNetworkClientId } = this.messagingSystem.call( + 'NetworkController:getState', + ); + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + selectedNetworkClientId, + ); + return chainId === ChainId.mainnet; + } - isMainnetByNetworkClientId = (networkClient: NetworkClient): boolean => { + isMainnetByNetworkClientId(networkClient: NetworkClient): boolean { return networkClient.configuration.chainId === ChainId.mainnet; - }; + } + + /** + * Handles the state change of the preference controller. + * @param preferencesState - The new state of the preference controller. + * @param preferencesState.useNftDetection - Boolean indicating user preference on NFT detection. + */ + #onPreferencesControllerStateChange({ useNftDetection }: PreferencesState) { + if (!useNftDetection !== this.#disabled) { + this.#disabled = !useNftDetection; + if (useNftDetection) { + this.start(); + } else { + this.stop(); + } + } + } + + #getOwnerNftApi({ address, next }: { address: string; next?: string }) { + return `${NFT_API_BASE_URL}/users/${address}/tokens?chainIds=1&limit=50&includeTopBid=true&continuation=${ + next ?? '' + }`; + } + + async #getOwnerNfts(address: string) { + let nftApiResponse: ReservoirResponse; + let nfts: TokensResponse[] = []; + let next; + + do { + nftApiResponse = await fetchWithErrorHandling({ + url: this.#getOwnerNftApi({ address, next }), + options: { + headers: { + Version: NFT_API_VERSION, + }, + }, + timeout: NFT_API_TIMEOUT, + }); + + if (!nftApiResponse) { + return nfts; + } + + const newNfts = + nftApiResponse.tokens?.filter( + (elm) => + elm.token.isSpam === false && + (elm.blockaidResult?.result_type + ? elm.blockaidResult?.result_type === BlockaidResultType.Benign + : true), + ) ?? []; + + nfts = [...nfts, ...newNfts]; + } while ((next = nftApiResponse.continuation)); + + return nfts; + } /** * Triggers asset ERC721 token auto detection on mainnet. Any newly detected NFTs are @@ -572,17 +536,16 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< * @param options.networkClientId - The network client ID to detect NFTs on. * @param options.userAddress - The address to detect NFTs for. */ - async detectNfts( - { - networkClientId, - userAddress, - }: { - networkClientId?: NetworkClientId; - userAddress: string; - } = { userAddress: this.config.selectedAddress }, - ) { + async detectNfts(options?: { + networkClientId?: NetworkClientId; + userAddress?: string; + }) { + const userAddress = + options?.userAddress ?? + this.messagingSystem.call('PreferencesController:getState') + .selectedAddress; /* istanbul ignore if */ - if (!this.isMainnet() || this.disabled) { + if (!this.isMainnet() || this.#disabled) { return; } /* istanbul ignore else */ @@ -590,7 +553,7 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< return; } - const apiNfts = await this.getOwnerNfts(userAddress); + const apiNfts = await this.#getOwnerNfts(userAddress); const addNftPromises = apiNfts.map(async (nft) => { const { tokenId: token_id, @@ -611,8 +574,8 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< let ignored; /* istanbul ignore else */ - const { ignoredNfts } = this.getNftState(); - if (ignoredNfts.length) { + const { ignoredNfts } = this.#getNftState(); + if (ignoredNfts.length > 0) { ignored = ignoredNfts.find((c) => { /* istanbul ignore next */ return ( @@ -641,11 +604,11 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< collection && { collection }, ); - await this.addNft(contract, token_id, { + await this.#addNft(contract, token_id, { nftMetadata, userAddress, source: Source.Detected, - networkClientId, + networkClientId: options?.networkClientId, }); } }); diff --git a/packages/controller-utils/src/constants.ts b/packages/controller-utils/src/constants.ts index 915b6fad59..1c4ae8d499 100644 --- a/packages/controller-utils/src/constants.ts +++ b/packages/controller-utils/src/constants.ts @@ -110,6 +110,10 @@ export const OPENSEA_PROXY_URL = export const NFT_API_BASE_URL = 'https://nft.api.cx.metamask.io'; +export const NFT_API_VERSION = '1'; + +export const NFT_API_TIMEOUT = 15000; + // Default origin for controllers export const ORIGIN_METAMASK = 'metamask'; From f1ca83d34a63563d8cff0f9f479027600e4c3927 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 30 May 2024 11:24:17 -0400 Subject: [PATCH 4/5] [composable-controller] Narrow class and messenger types by parameterizing over child controllers list (#3952) ## Explanation Currently, the allow lists of `ComposableControllerMessenger` are set to `string`, which is too permissive. Each `ComposableController` instance needs typing and allow lists that are specific to its set of input child controllers. To achieve this, the `ComposableController` class and its messenger are made polymorphic upon the `ControllerState` type, which defines the shape of the composed state. ## References - Closes #3627 ## Changelog ### [`@metamask/composable-controller`](https://github.com/MetaMask/core/pull/3952/files#diff-9745631a8a7023c408b4f9b96dc8c0eaa9a41599e8d93eb470bb268f1fcc75ff) ### Added - Adds and exports new types: ([#3952](https://github.com/MetaMask/core/pull/3952)) - `RestrictedControllerMessengerConstraint`, which is the narrowest supertype of all controller-messenger instances. - `LegacyControllerStateConstraint`, a universal supertype for the controller state object, encompassing both BaseControllerV1 and BaseControllerV2 state. - `ComposableControllerStateConstraint`, the narrowest supertype for the composable controller state object. ### Changed - **BREAKING:** The `ComposableController` class is now a generic class that expects one generic argument `ComposableControllerState` ([#3952](https://github.com/MetaMask/core/pull/3952)). - **BREAKING:** For the `ComposableController` class to be typed correctly, any of its child controllers that extend `BaseControllerV1` must have an overridden `name` property that is defined using the `as const` assertion. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/AddressBookController.ts | 2 +- .../src/AccountTrackerController.ts | 2 +- .../src/AssetsContractController.ts | 2 +- .../assets-controllers/src/NftController.ts | 2 +- .../src/TokenRatesController.ts | 2 +- packages/composable-controller/CHANGELOG.md | 12 ++ .../src/ComposableController.test.ts | 184 ++++++++++++------ .../src/ComposableController.ts | 146 ++++++++++---- packages/composable-controller/src/index.ts | 4 +- .../src/DecryptMessageManager.ts | 2 +- .../src/EncryptionPublicKeyManager.ts | 2 +- .../message-manager/src/MessageManager.ts | 2 +- .../src/PersonalMessageManager.ts | 2 +- .../src/TypedMessageManager.ts | 2 +- 14 files changed, 264 insertions(+), 102 deletions(-) diff --git a/packages/address-book-controller/src/AddressBookController.ts b/packages/address-book-controller/src/AddressBookController.ts index 1101dffc94..cd96f5355c 100644 --- a/packages/address-book-controller/src/AddressBookController.ts +++ b/packages/address-book-controller/src/AddressBookController.ts @@ -78,7 +78,7 @@ export class AddressBookController extends BaseControllerV1< /** * Name of this controller used during composition */ - override name = 'AddressBookController'; + override name = 'AddressBookController' as const; /** * Creates an AddressBookController instance. diff --git a/packages/assets-controllers/src/AccountTrackerController.ts b/packages/assets-controllers/src/AccountTrackerController.ts index 6c8479dc2b..3596790358 100644 --- a/packages/assets-controllers/src/AccountTrackerController.ts +++ b/packages/assets-controllers/src/AccountTrackerController.ts @@ -112,7 +112,7 @@ export class AccountTrackerController extends StaticIntervalPollingControllerV1< /** * Name of this controller used during composition */ - override name = 'AccountTrackerController'; + override name = 'AccountTrackerController' as const; private readonly getIdentities: () => PreferencesState['identities']; diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 71cad85390..b9dd83602a 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -107,7 +107,7 @@ export class AssetsContractController extends BaseControllerV1< /** * Name of this controller used during composition */ - override name = 'AssetsContractController'; + override name = 'AssetsContractController' as const; private readonly getNetworkClientById: NetworkController['getNetworkClientById']; diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index ff712bf8bf..90ae8560cf 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -935,7 +935,7 @@ export class NftController extends BaseControllerV1 { /** * Name of this controller used during composition */ - override name = 'NftController'; + override name = 'NftController' as const; private readonly getERC721AssetName: AssetsContractController['getERC721AssetName']; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 42e65672b5..e065d2e40b 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -172,7 +172,7 @@ export class TokenRatesController extends StaticIntervalPollingControllerV1< /** * Name of this controller used during composition */ - override name = 'TokenRatesController'; + override name = 'TokenRatesController' as const; private readonly getNetworkClientById: NetworkController['getNetworkClientById']; diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index eb07c36f28..ee846caee9 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Adds and exports new types: ([#3952](https://github.com/MetaMask/core/pull/3952)) + - `RestrictedControllerMessengerConstraint`, which is the narrowest supertype of all controller-messenger instances. + - `LegacyControllerStateConstraint`, a universal supertype for the controller state object, encompassing both BaseControllerV1 and BaseControllerV2 state. + - `ComposableControllerStateConstraint`, the narrowest supertype for the composable controller state object. + +### Changed + +- **BREAKING:** The `ComposableController` class is now a generic class that expects one generic argument `ComposableControllerState` ([#3952](https://github.com/MetaMask/core/pull/3952)). + - **BREAKING:** For the `ComposableController` class to be typed correctly, any of its child controllers that extend `BaseControllerV1` must have an overridden `name` property that is defined using the `as const` assertion. + ## [6.0.1] ### Fixed diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index ca5df665ef..6ab79f223c 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -27,9 +27,9 @@ type FooControllerEvent = { type FooMessenger = RestrictedControllerMessenger< 'FooController', never, - FooControllerEvent, + FooControllerEvent | QuzControllerEvent, never, - never + QuzControllerEvent['type'] >; const fooControllerStateMetadata = { @@ -60,6 +60,50 @@ class FooController extends BaseController< } } +type QuzControllerState = { + quz: string; +}; +type QuzControllerEvent = { + type: `QuzController:stateChange`; + payload: [QuzControllerState, Patch[]]; +}; + +type QuzMessenger = RestrictedControllerMessenger< + 'QuzController', + never, + QuzControllerEvent, + never, + never +>; + +const quzControllerStateMetadata = { + quz: { + persist: true, + anonymous: true, + }, +}; + +class QuzController extends BaseController< + 'QuzController', + QuzControllerState, + QuzMessenger +> { + constructor(messagingSystem: QuzMessenger) { + super({ + messenger: messagingSystem, + metadata: quzControllerStateMetadata, + name: 'QuzController', + state: { quz: 'quz' }, + }); + } + + updateQuz(quz: string) { + super.update((state) => { + state.quz = quz; + }); + } +} + // Mock BaseControllerV1 classes type BarControllerState = BaseState & { @@ -71,7 +115,7 @@ class BarController extends BaseControllerV1 { bar: 'bar', }; - override name = 'BarController'; + override name = 'BarController' as const; constructor() { super(); @@ -92,7 +136,7 @@ class BazController extends BaseControllerV1 { baz: 'baz', }; - override name = 'BazController'; + override name = 'BazController' as const; constructor() { super(); @@ -107,9 +151,13 @@ describe('ComposableController', () => { describe('BaseControllerV1', () => { it('should compose controller state', () => { + type ComposableControllerState = { + BarController: BarControllerState; + BazController: BazControllerState; + }; const composableMessenger = new ControllerMessenger< never, - ComposableControllerEvents + ComposableControllerEvents >().getRestricted({ name: 'ComposableController', allowedActions: [], @@ -127,9 +175,12 @@ describe('ComposableController', () => { }); it('should notify listeners of nested state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents + ComposableControllerEvents >(); const composableMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', @@ -159,39 +210,60 @@ describe('ComposableController', () => { describe('BaseControllerV2', () => { it('should compose controller state', () => { + type ComposableControllerState = { + FooController: FooControllerState; + QuzController: QuzControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent + | QuzControllerEvent >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ + const fooMessenger = controllerMessenger.getRestricted< + 'FooController', + never, + QuzControllerEvent['type'] + >({ name: 'FooController', allowedActions: [], + allowedEvents: ['QuzController:stateChange'], + }); + const quzMessenger = controllerMessenger.getRestricted({ + name: 'QuzController', + allowedActions: [], allowedEvents: [], }); - const fooController = new FooController(fooControllerMessenger); + const fooController = new FooController(fooMessenger); + const quzController = new QuzController(quzMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], - allowedEvents: ['FooController:stateChange'], - }); - const composableController = new ComposableController({ - controllers: [fooController], - messenger: composableControllerMessenger, + allowedEvents: [ + 'FooController:stateChange', + 'QuzController:stateChange', + ], }); + const composableController = + new ComposableController({ + controllers: [fooController, quzController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ FooController: { foo: 'foo' }, + QuzController: { quz: 'quz' }, }); }); it('should notify listeners of nested state change', () => { + type ComposableControllerState = { + FooController: FooControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -199,16 +271,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [fooController], messenger: composableControllerMessenger, }); @@ -231,10 +299,15 @@ describe('ComposableController', () => { describe('Mixed BaseControllerV1 and BaseControllerV2', () => { it('should compose controller state', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -242,19 +315,16 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - const composableController = new ComposableController({ - controllers: [barController, fooController], - messenger: composableControllerMessenger, - }); + const composableController = + new ComposableController({ + controllers: [barController, fooController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ BarController: { bar: 'bar' }, FooController: { foo: 'foo' }, @@ -262,10 +332,15 @@ describe('ComposableController', () => { }); it('should notify listeners of BaseControllerV1 state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -273,16 +348,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -305,10 +376,15 @@ describe('ComposableController', () => { }); it('should notify listeners of BaseControllerV2 state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -316,16 +392,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -370,10 +442,14 @@ describe('ComposableController', () => { }); it('should throw if composing a controller that does not extend from BaseController', () => { + type ComposableControllerState = { + FooController: FooControllerState; + }; const notController = new JsonRpcEngine(); const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -381,11 +457,7 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 8ee6bbe3a5..4c2ec8ad3b 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -6,6 +6,8 @@ import type { EventConstraint, RestrictedControllerMessenger, StateConstraint, + StateMetadata, + ControllerStateChangeEvent, } from '@metamask/base-controller'; import type { Patch } from 'immer'; @@ -15,7 +17,7 @@ export const controllerName = 'ComposableController'; * A universal subtype of all controller instances that extend from `BaseControllerV1`. * Any `BaseControllerV1` instance can be assigned to this type. * - * Note that this type is not the greatest subtype or narrowest supertype of all `BaseControllerV1` instances. + * Note that this type is not the widest subtype or narrowest supertype of all `BaseControllerV1` instances. * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. */ export type BaseControllerV1Instance = @@ -27,7 +29,7 @@ export type BaseControllerV1Instance = * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`). * Any `BaseController` instance can be assigned to this type. * - * Note that this type is not the greatest subtype or narrowest supertype of all `BaseController` instances. + * Note that this type is not the widest subtype or narrowest supertype of all `BaseController` instances. * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. * * For this reason, we only look for `BaseController` properties that we use in the ComposableController (name and state). @@ -41,13 +43,25 @@ export type BaseControllerInstance = { * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`) or `BaseControllerV1`. * Any `BaseController` or `BaseControllerV1` instance can be assigned to this type. * - * Note that this type is not the greatest subtype or narrowest supertype of all `BaseController` and `BaseControllerV1` instances. + * Note that this type is not the widest subtype or narrowest supertype of all `BaseController` and `BaseControllerV1` instances. * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. */ export type ControllerInstance = | BaseControllerV1Instance | BaseControllerInstance; +/** + * The narrowest supertype of all `RestrictedControllerMessenger` instances. + */ +export type RestrictedControllerMessengerConstraint = + RestrictedControllerMessenger< + string, + ActionConstraint, + EventConstraint, + string, + string + >; + /** * Determines if the given controller is an instance of `BaseControllerV1` * @param controller - Controller instance to check @@ -82,13 +96,7 @@ export function isBaseController( ): controller is BaseController< string, StateConstraint, - RestrictedControllerMessenger< - string, - ActionConstraint, - EventConstraint, - string, - string - > + RestrictedControllerMessengerConstraint > { return ( 'name' in controller && @@ -99,43 +107,108 @@ export function isBaseController( ); } -// TODO: Replace `any` with `Json` once `BaseControllerV2` migrations are completed for all controllers. -export type ComposableControllerState = { - // `any` is used here to disable the `BaseController` type constraint which expects state properties to extend `Record`. - // `ComposableController` state needs to accommodate `BaseControllerV1` state objects that may have properties wider than `Json`. +/** + * A universal supertype for the controller state object, encompassing both `BaseControllerV1` and `BaseControllerV2` state. + */ +export type LegacyControllerStateConstraint = BaseState | StateConstraint; + +/** + * A universal supertype for the composable controller state object. + * + * This type is only intended to be used for disabling the generic constraint on the `ControllerState` type argument in the `BaseController` type as a temporary solution for ensuring compatibility with BaseControllerV1 child controllers. + * Note that it is unsuitable for general use as a type constraint. + */ +// TODO: Replace with `ComposableControllerStateConstraint` once BaseControllerV2 migrations are completed for all controllers. +type LegacyComposableControllerStateConstraint = { + // `any` is used here to disable the generic constraint on the `ControllerState` type argument in the `BaseController` type, + // enabling composable controller state types with BaseControllerV1 state objects to be. // eslint-disable-next-line @typescript-eslint/no-explicit-any [name: string]: Record; }; -export type ComposableControllerStateChangeEvent = { - type: `${typeof controllerName}:stateChange`; - payload: [ComposableControllerState, Patch[]]; +/** + * The narrowest supertype for the composable controller state object. + * This is also a widest subtype of the 'LegacyComposableControllerStateConstraint' type. + */ +// TODO: Replace with `{ [name: string]: StateConstraint }` once BaseControllerV2 migrations are completed for all controllers. +export type ComposableControllerStateConstraint = { + [name: string]: LegacyControllerStateConstraint; }; -export type ComposableControllerEvents = ComposableControllerStateChangeEvent; - -type AnyControllerStateChangeEvent = { - type: `${string}:stateChange`; - payload: [ControllerInstance['state'], Patch[]]; +/** + * A controller state change event for any controller instance that extends from either `BaseControllerV1` or `BaseControllerV2`. + */ +// TODO: Replace all instances with `ControllerStateChangeEvent` once `BaseControllerV2` migrations are completed for all controllers. +type LegacyControllerStateChangeEvent< + ControllerName extends string, + ControllerState extends LegacyControllerStateConstraint, +> = { + type: `${ControllerName}:stateChange`; + payload: [ControllerState, Patch[]]; }; -type AllowedEvents = AnyControllerStateChangeEvent; +export type ComposableControllerStateChangeEvent< + ComposableControllerState extends ComposableControllerStateConstraint, +> = LegacyControllerStateChangeEvent< + typeof controllerName, + ComposableControllerState +>; + +export type ComposableControllerEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerStateChangeEvent; + +type ChildControllerStateChangeEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerState extends Record< + infer ControllerName extends string, + infer ControllerState +> + ? ControllerState extends StateConstraint + ? ControllerStateChangeEvent + : ControllerState extends Record + ? LegacyControllerStateChangeEvent + : never + : never; + +type AllowedEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ChildControllerStateChangeEvents; -export type ComposableControllerMessenger = RestrictedControllerMessenger< +export type ComposableControllerMessenger< + ComposableControllerState extends ComposableControllerStateConstraint, +> = RestrictedControllerMessenger< typeof controllerName, never, - ComposableControllerEvents | AllowedEvents, + | ComposableControllerEvents + | AllowedEvents, never, - AllowedEvents['type'] + AllowedEvents['type'] >; +type GetChildControllers< + ComposableControllerState, + ControllerName extends keyof ComposableControllerState = keyof ComposableControllerState, +> = ControllerName extends string + ? ComposableControllerState[ControllerName] extends StateConstraint + ? { name: ControllerName; state: ComposableControllerState[ControllerName] } + : BaseControllerV1< + BaseConfig & Record, + BaseState & ComposableControllerState[ControllerName] + > + : never; + /** * Controller that can be used to compose multiple controllers together. + * @template ChildControllerState - The composed state of the child controllers that are being used to instantiate the composable controller. */ -export class ComposableController extends BaseController< +export class ComposableController< + ComposableControllerState extends LegacyComposableControllerStateConstraint, + ChildControllers extends ControllerInstance = GetChildControllers, +> extends BaseController< typeof controllerName, ComposableControllerState, - ComposableControllerMessenger + ComposableControllerMessenger > { /** * Creates a ComposableController instance. @@ -149,8 +222,8 @@ export class ComposableController extends BaseController< controllers, messenger, }: { - controllers: ControllerInstance[]; - messenger: ComposableControllerMessenger; + controllers: ChildControllers[]; + messenger: ComposableControllerMessenger; }) { if (messenger === undefined) { throw new Error(`Messaging system is required`); @@ -158,18 +231,21 @@ export class ComposableController extends BaseController< super({ name: controllerName, - metadata: controllers.reduce( + metadata: controllers.reduce>( (metadata, controller) => ({ ...metadata, [controller.name]: isBaseController(controller) ? controller.metadata : { persist: true, anonymous: true }, }), - {}, + {} as never, + ), + state: controllers.reduce( + (state, controller) => { + return { ...state, [controller.name]: controller.state }; + }, + {} as never, ), - state: controllers.reduce((state, controller) => { - return { ...state, [controller.name]: controller.state }; - }, {}), messenger, }); diff --git a/packages/composable-controller/src/index.ts b/packages/composable-controller/src/index.ts index 9deb15385b..fc00056334 100644 --- a/packages/composable-controller/src/index.ts +++ b/packages/composable-controller/src/index.ts @@ -1,8 +1,10 @@ export type { - ComposableControllerState, + ComposableControllerStateConstraint, ComposableControllerStateChangeEvent, ComposableControllerEvents, ComposableControllerMessenger, + LegacyControllerStateConstraint, + RestrictedControllerMessengerConstraint, } from './ComposableController'; export { ComposableController, diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 2ef29ff738..dbfd3a4a39 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -68,7 +68,7 @@ export class DecryptMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'DecryptMessageManager'; + override name = 'DecryptMessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index cf9288ac50..ab4f40f7d1 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -65,7 +65,7 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'EncryptionPublicKeyManager'; + override name = 'EncryptionPublicKeyManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/MessageManager.ts b/packages/message-manager/src/MessageManager.ts index 2f98cf92fc..8a561cbfe6 100644 --- a/packages/message-manager/src/MessageManager.ts +++ b/packages/message-manager/src/MessageManager.ts @@ -70,7 +70,7 @@ export class MessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'MessageManager'; + override name = 'MessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/PersonalMessageManager.ts b/packages/message-manager/src/PersonalMessageManager.ts index 5f3db2aab4..f0f58e17ba 100644 --- a/packages/message-manager/src/PersonalMessageManager.ts +++ b/packages/message-manager/src/PersonalMessageManager.ts @@ -74,7 +74,7 @@ export class PersonalMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'PersonalMessageManager'; + override name = 'PersonalMessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/TypedMessageManager.ts b/packages/message-manager/src/TypedMessageManager.ts index 9193d3018e..75682dedcc 100644 --- a/packages/message-manager/src/TypedMessageManager.ts +++ b/packages/message-manager/src/TypedMessageManager.ts @@ -95,7 +95,7 @@ export class TypedMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'TypedMessageManager'; + override name = 'TypedMessageManager' as const; /** * Creates a new TypedMessage with an 'unapproved' status using the passed messageParams. From 50efa9cd5a55ae434e34cc25edb499ad836cc3a1 Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 30 May 2024 22:10:29 +0200 Subject: [PATCH 5/5] feat: migrate NftController to BaseControllerV2 (#4310) --- .../src/NftController.test.ts | 1178 ++++++++--------- .../assets-controllers/src/NftController.ts | 886 +++++++------ .../src/NftDetectionController.test.ts | 6 +- .../src/NftDetectionController.ts | 6 +- packages/assets-controllers/src/index.ts | 13 +- 5 files changed, 1074 insertions(+), 1015 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 8e2de092d7..2c5004e1c3 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1,8 +1,5 @@ import type { Network } from '@ethersproject/providers'; -import type { - ApprovalStateChange, - GetApprovalsState, -} from '@metamask/approval-controller'; +import type { ApprovalControllerMessenger } from '@metamask/approval-controller'; import { ApprovalController } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -21,7 +18,6 @@ import { import type { NetworkClientConfiguration, NetworkClientId, - NetworkState, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import { @@ -43,8 +39,16 @@ import { } from '../../network-controller/tests/helpers'; import { getFormattedIpfsUrl } from './assetsUtil'; import { Source } from './constants'; -import type { Nft, NftControllerMessenger } from './NftController'; -import { NftController } from './NftController'; +import type { + Nft, + NftControllerState, + NftControllerMessenger, +} from './NftController'; +import { + NftController, + type AllowedActions, + type AllowedEvents, +} from './NftController'; const CRYPTOPUNK_ADDRESS = '0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB'; const ERC721_KUDOSADDRESS = '0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163'; @@ -128,23 +132,13 @@ function setupController({ NetworkClientConfiguration >; } = {}) { - const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; - const changeNetwork = ({ - selectedNetworkClientId, - }: { - selectedNetworkClientId: NetworkClientId; - }) => { - onNetworkDidChangeListeners.forEach((listener) => { - listener({ - ...defaultNetworkState, - selectedNetworkClientId, - }); - }); - }; - const messenger = new ControllerMessenger< - ExtractAvailableAction | GetApprovalsState, - ExtractAvailableEvent | ApprovalStateChange + | ExtractAvailableAction + | AllowedActions + | ExtractAvailableAction, + | ExtractAvailableEvent + | AllowedEvents + | ExtractAvailableEvent >(); const getNetworkClientById = buildMockGetNetworkClientById( @@ -172,34 +166,40 @@ function setupController({ 'ApprovalController:addRequest', 'NetworkController:getNetworkClientById', ], - allowedEvents: [], + allowedEvents: [ + 'NetworkController:networkDidChange', + 'PreferencesController:stateChange', + ], }); - const preferencesStateChangeListeners: ((state: PreferencesState) => void)[] = - []; const nftController = new NftController({ chainId: ChainId.mainnet, - onPreferencesStateChange: (listener) => { - preferencesStateChangeListeners.push(listener); - }, - onNetworkStateChange: (listener) => - onNetworkDidChangeListeners.push(listener), getERC721AssetName: jest.fn(), getERC721AssetSymbol: jest.fn(), getERC721TokenURI: jest.fn(), getERC721OwnerOf: jest.fn(), getERC1155BalanceOf: jest.fn(), getERC1155TokenURI: jest.fn(), - getNetworkClientById, onNftAdded: jest.fn(), messenger: nftControllerMessenger, ...options, }); + const triggerPreferencesStateChange = (state: PreferencesState) => { - for (const listener of preferencesStateChangeListeners) { - listener(state); - } + messenger.publish('PreferencesController:stateChange', state, []); }; + + const changeNetwork = ({ + selectedNetworkClientId, + }: { + selectedNetworkClientId: NetworkClientId; + }) => { + messenger.publish('NetworkController:networkDidChange', { + ...defaultNetworkState, + selectedNetworkClientId, + }); + }; + triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -208,9 +208,9 @@ function setupController({ return { nftController, - changeNetwork, messenger, approvalController, + changeNetwork, triggerPreferencesStateChange, }; } @@ -868,11 +868,14 @@ describe('NftController', () => { }); const acceptedRequest = new Promise((resolve) => { - nftController.subscribe((state) => { - if (state.allNfts?.[SECOND_OWNER_ADDRESS]?.[GOERLI.chainId]) { - resolve(); - } - }); + messenger.subscribe( + 'NftController:stateChange', + (state: NftControllerState) => { + if (state.allNfts?.[SECOND_OWNER_ADDRESS]?.[GOERLI.chainId]) { + resolve(); + } + }, + ); }); // check that the NFT is not in state to begin with @@ -964,11 +967,14 @@ describe('NftController', () => { }); const acceptedRequest = new Promise((resolve) => { - nftController.subscribe((state) => { - if (state.allNfts?.[OWNER_ADDRESS]?.[GOERLI.chainId].length) { - resolve(); - } - }); + messenger.subscribe( + 'NftController:stateChange', + (state: NftControllerState) => { + if (state.allNfts?.[OWNER_ADDRESS]?.[GOERLI.chainId].length) { + resolve(); + } + }, + ); }); // check that the NFT is not in state to begin with @@ -1046,13 +1052,15 @@ describe('NftController', () => { describe('addNft', () => { it('should add NFT and NFT contract', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + chainId: ChainId.mainnet, + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('Name'), }, }); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1068,7 +1076,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1085,7 +1093,9 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: '0x01', logo: 'url', @@ -1135,7 +1145,7 @@ describe('NftController', () => { name: 'name', image: 'image', description: 'description', - standard: 'ERC721', + standard: ERC721, favorite: false, }, userAddress: detectedUserAddress, @@ -1146,22 +1156,29 @@ describe('NftController', () => { source: 'detected', tokenId: '2', address: '0x01', - standard: 'ERC721', + standard: ERC721, }); }); it('should add NFT by selected address', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); - const { chainId } = nftController.config; + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const mockGetERC1155TokenURI = jest.fn().mockRejectedValue(''); + + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + getERC1155TokenURI: mockGetERC1155TokenURI, + }, + }); const firstAddress = '0x123'; const secondAddress = '0x321'; - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -1180,12 +1197,14 @@ describe('NftController', () => { selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', image: 'url', name: 'name', + standard: ERC721, + tokenURI, tokenId: '1234', favorite: false, isCurrentlyOwned: true, @@ -1193,8 +1212,12 @@ describe('NftController', () => { }); it('should update NFT if image is different', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1207,7 +1230,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1230,7 +1253,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1244,8 +1267,13 @@ describe('NftController', () => { }); it('should not duplicate NFT nor NFT contract if already added', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1267,17 +1295,19 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); it('should add NFT and get information from NFT-API', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not an ERC721 contract')), @@ -1287,10 +1317,9 @@ describe('NftController', () => { }, }); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'Description', @@ -1307,8 +1336,10 @@ describe('NftController', () => { }); it('should add NFT erc721 and aggregate NFT data from both contract and NFT-API', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), getERC721TokenURI: jest @@ -1343,19 +1374,17 @@ describe('NftController', () => { description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.config; - await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, image: 'Kudos Image (directly from tokenURI)', name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -1363,18 +1392,22 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, name: 'KudosToken', symbol: 'KDO', - schemaName: 'ERC721', + schemaName: ERC721, }); }); it('should add NFT erc1155 and get NFT information from contract when NFT API call fail', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not a 721 contract')), @@ -1396,11 +1429,11 @@ describe('NftController', () => { image: 'image (directly from tokenURI)', animation_url: null, }); - const { selectedAddress, chainId } = nftController.config; + await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC1155_NFT_ADDRESS, image: 'image (directly from tokenURI)', @@ -1416,8 +1449,10 @@ describe('NftController', () => { }); it('should add NFT erc721 and get NFT information only from contract', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { @@ -1437,24 +1472,24 @@ describe('NftController', () => { name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.config; - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformationFromApi' as any) - .returns(undefined); + + nock('https://nft.api.cx.metamask.io') + .get( + '/tokens?chainIds=1&tokens=0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163%3A1203&includeTopBid=true&includeAttributes=true&includeLastSale=true', + ) + .reply(404, { error: 'Not found' }); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, image: 'Kudos Image (directly from tokenURI)', name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -1462,23 +1497,32 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, name: 'KudosToken', symbol: 'KDO', - schemaName: 'ERC721', + schemaName: ERC721, }); }); it('should add NFT by provider type', async () => { - const { nftController, changeNetwork } = setupController(); - const { selectedAddress } = nftController.config; - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + const selectedAddress = OWNER_ADDRESS; + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, changeNetwork } = setupController({ + options: { + selectedAddress, + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNft('0x01', '1234'); @@ -1496,36 +1540,32 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '1234', favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); it('should add an nft and nftContract to state when all contract information is falsy and the source is left empty (defaults to "custom")', async () => { + const tokenURI = 'https://url/'; const mockOnNftAdded = jest.fn(); + const mockGetERC721AssetSymbol = jest.fn().mockResolvedValue(''); + const mockGetERC721AssetName = jest.fn().mockResolvedValue(''); + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress, chainId } = nftController.config; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftContractInformation' as any).returns({ - asset_contract_type: null, - created_date: null, - schema_name: null, - symbol: null, - total_supply: null, - description: null, - external_link: null, - collection: { name: null, image_url: null }, - }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftInformation' as any).returns({ + nock('https://url').get('/').reply(200, { name: 'name', image: 'url', description: 'description', @@ -1535,9 +1575,10 @@ describe('NftController', () => { expect(nftController.state.allNftContracts).toStrictEqual({ [selectedAddress]: { - [chainId]: [ + [ChainId.mainnet]: [ { address: '0x01234abcdefg', + schemaName: ERC721, }, ], }, @@ -1545,13 +1586,15 @@ describe('NftController', () => { expect(nftController.state.allNfts).toStrictEqual({ [selectedAddress]: { - [chainId]: [ + [ChainId.mainnet]: [ { address: '0x01234abcdefg', description: 'description', image: 'url', name: 'name', tokenId: '1234', + standard: ERC721, + tokenURI, favorite: false, isCurrentlyOwned: true, }, @@ -1562,41 +1605,32 @@ describe('NftController', () => { expect(mockOnNftAdded).toHaveBeenCalledWith({ address: '0x01234abcdefg', tokenId: '1234', - standard: undefined, + standard: ERC721, symbol: undefined, source: Source.Custom, }); }); it('should add an nft and nftContract to state when all contract information is falsy and the source is "dapp"', async () => { + const tokenURI = 'https://url/'; const mockOnNftAdded = jest.fn(); + const mockGetERC721AssetSymbol = jest.fn().mockResolvedValue(''); + const mockGetERC721AssetName = jest.fn().mockResolvedValue(''); + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ options: { onNftAdded: mockOnNftAdded, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, }, }); - changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftContractInformation' as any).returns({ - asset_contract_type: null, - created_date: null, - schema_name: null, - symbol: null, - total_supply: null, - description: null, - external_link: null, - collection: { name: null, image_url: null }, - }); - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftInformation' as any).returns({ + nock('https://url').get('/').reply(200, { name: 'name', image: 'url', description: 'description', }); + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); await nftController.addNft('0x01234abcdefg', '1234', { userAddress: '0x123', @@ -1608,6 +1642,7 @@ describe('NftController', () => { [GOERLI.chainId]: [ { address: '0x01234abcdefg', + schemaName: ERC721, }, ], }, @@ -1623,7 +1658,9 @@ describe('NftController', () => { name: 'name', tokenId: '1234', favorite: false, + standard: ERC721, isCurrentlyOwned: true, + tokenURI, }, ], }, @@ -1632,16 +1669,18 @@ describe('NftController', () => { expect(mockOnNftAdded).toHaveBeenCalledWith({ address: '0x01234abcdefg', tokenId: '1234', - standard: undefined, + standard: ERC721, symbol: undefined, source: Source.Dapp, }); }); it('should add an nft and nftContract when there is valid contract information and source is "detected"', async () => { + const selectedAddress = OWNER_ADDRESS; const mockOnNftAdded = jest.fn(); const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, getERC721AssetName: jest .fn() @@ -1672,23 +1711,7 @@ describe('NftController', () => { }, ], }); - /* nock(OPENSEA_PROXY_URL) - .get(`/chain/ethereum/contract/${ERC721_KUDOSADDRESS}`) - .reply(200, { - address: ERC721_KUDOSADDRESS, - chain: 'ethereum', - collection: 'KDO', - contract_standard: 'erc721', - name: 'Kudos', - total_supply: 10, - }) - .get(`/collections/KDO`) - .reply(200, { - description: 'Kudos Description', - image_url: 'Kudos logo (from proxy API)', - }); */ - const { selectedAddress, chainId } = nftController.config; await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', '123', @@ -1699,11 +1722,11 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress]?.[chainId], + nftController.state.allNfts[selectedAddress]?.[ChainId.mainnet], ).toBeUndefined(); expect( - nftController.state.allNftContracts[selectedAddress]?.[chainId], + nftController.state.allNftContracts[selectedAddress]?.[ChainId.mainnet], ).toBeUndefined(); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID, { @@ -1712,14 +1735,14 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toStrictEqual([ { address: ERC721_KUDOSADDRESS, description: 'Kudos Description', image: 'Kudos image (from proxy API)', name: 'Kudos Name', - standard: 'ERC721', + standard: ERC721, tokenId: ERC721_KUDOS_TOKEN_ID, favorite: false, isCurrentlyOwned: true, @@ -1733,29 +1756,31 @@ describe('NftController', () => { ]); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toStrictEqual([ { address: ERC721_KUDOSADDRESS, logo: 'Kudos logo (from proxy API)', name: 'Kudos', totalSupply: '10', - schemaName: 'ERC721', + schemaName: ERC721, }, ]); expect(mockOnNftAdded).toHaveBeenCalledWith({ address: ERC721_KUDOSADDRESS, tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, source: Source.Detected, }); }); it('should not add an nft and nftContract when there is not valid contract information (or an issue fetching it) and source is "detected"', async () => { + const selectedAddress = OWNER_ADDRESS; const mockOnNftAdded = jest.fn(); const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, getERC721AssetName: jest .fn() @@ -1770,9 +1795,6 @@ describe('NftController', () => { `/tokens?chainIds=1&tokens=${ERC721_KUDOSADDRESS}%3A${ERC721_KUDOS_TOKEN_ID}&includeTopBid=true&includeAttributes=true&includeLastSale=true`, ) .replyWithError(new Error('Failed to fetch')); - - const { selectedAddress } = nftController.config; - await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', '123', @@ -1792,8 +1814,12 @@ describe('NftController', () => { }); it('should not add duplicate NFTs to the ignoredNfts list', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1814,13 +1840,13 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(2); expect(nftController.state.ignoredNfts).toHaveLength(0); nftController.removeAndIgnoreNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(1); @@ -1834,19 +1860,20 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(2); expect(nftController.state.ignoredNfts).toHaveLength(1); nftController.removeAndIgnoreNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(1); }); it('should add NFT with metadata hosted in IPFS', async () => { - const { nftController } = setupController({ + const selectedAddress = OWNER_ADDRESS; + const { nftController, triggerPreferencesStateChange } = setupController({ options: { getERC721AssetName: jest .fn() @@ -1865,10 +1892,11 @@ describe('NftController', () => { .mockRejectedValue(new Error('Not an ERC1155 token')), }, }); - nftController.configure({ + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + selectedAddress, ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, }); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, @@ -1876,22 +1904,24 @@ describe('NftController', () => { ); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_DEPRESSIONIST_ADDRESS, name: "Maltjik.jpg's Depressionists", symbol: 'DPNS', - schemaName: 'ERC721', + schemaName: ERC721, }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_DEPRESSIONIST_ADDRESS, tokenId: '36', image: 'image', name: 'name', description: 'description', - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -1900,6 +1930,7 @@ describe('NftController', () => { }); it('should add NFT erc721 when call to NFT API fail', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController(); nock(NFT_API_BASE_URL) .get( @@ -1907,12 +1938,10 @@ describe('NftController', () => { ) .replyWithError(new Error('Failed to fetch')); - const { selectedAddress, chainId } = nftController.config; - await nftController.addNft(ERC721_NFT_ADDRESS, ERC721_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_NFT_ADDRESS, image: null, @@ -2166,21 +2195,23 @@ describe('NftController', () => { describe('addNftVerifyOwnership', () => { it('should verify ownership by selected address and add NFT', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const firstAddress = '0x123'; const secondAddress = '0x321'; - const { chainId } = nftController.config; - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -2199,13 +2230,15 @@ describe('NftController', () => { selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', image: 'url', name: 'name', tokenId: '1234', + standard: ERC721, + tokenURI, favorite: false, isCurrentlyOwned: true, }); @@ -2214,9 +2247,7 @@ describe('NftController', () => { it('should throw an error if selected address is not owner of input NFT', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); const firstAddress = '0x123'; triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -2230,21 +2261,27 @@ describe('NftController', () => { }); it('should verify ownership by selected address and add NFT by the correct chainId when passed networkClientId', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const firstAddress = '0x123'; const secondAddress = '0x321'; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url') + .get('/') + .reply(200, { + name: 'name', + image: 'url', + description: 'description', + }) + .persist(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -2269,9 +2306,11 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '1234', favorite: false, isCurrentlyOwned: true, + tokenURI, }); expect( nftController.state.allNfts[secondAddress][GOERLI.chainId][0], @@ -2280,15 +2319,23 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); it('should verify ownership by selected address and add NFT by the correct userAddress when passed userAddress', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork, triggerPreferencesStateChange } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); // Ensure that the currently selected address is not the same as either of the userAddresses triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -2299,15 +2346,16 @@ describe('NftController', () => { const firstAddress = '0x123'; const secondAddress = '0x321'; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url') + .get('/') + .reply(200, { + name: 'name', + image: 'url', + description: 'description', + }) + .persist(); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNftVerifyOwnership('0x01', '1234', { userAddress: firstAddress, @@ -2326,7 +2374,9 @@ describe('NftController', () => { name: 'name', tokenId: '1234', favorite: false, + standard: ERC721, isCurrentlyOwned: true, + tokenURI, }); expect( nftController.state.allNfts[secondAddress][GOERLI.chainId][0], @@ -2336,16 +2386,22 @@ describe('NftController', () => { image: 'url', name: 'name', tokenId: '4321', + standard: ERC721, favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); }); describe('removeNft', () => { it('should remove NFT and NFT contract', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2357,17 +2413,17 @@ describe('NftController', () => { }); nftController.removeNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(0); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(0); }); it('should not remove NFT contract if NFT still exists', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2388,23 +2444,27 @@ describe('NftController', () => { }); nftController.removeNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); it('should remove NFT by selected address', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); - const { chainId } = nftController.config; - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); const firstAddress = '0x123'; const secondAddress = '0x321'; triggerPreferencesStateChange({ @@ -2420,16 +2480,16 @@ describe('NftController', () => { }); await nftController.addNft('0x01', '1234'); nftController.removeNft('0x01', '1234'); - expect(nftController.state.allNfts[secondAddress][chainId]).toHaveLength( - 0, - ); + expect( + nftController.state.allNfts[secondAddress][ChainId.mainnet], + ).toHaveLength(0); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x02', description: 'description', @@ -2438,18 +2498,27 @@ describe('NftController', () => { tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, + standard: ERC721, }); }); it('should remove NFT by provider type', async () => { - const { nftController, changeNetwork } = setupController(); - const { selectedAddress } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, changeNetwork } = setupController({ + options: { + selectedAddress, + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNft('0x02', '4321'); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); @@ -2471,6 +2540,8 @@ describe('NftController', () => { tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, + standard: ERC721, }); }); @@ -2534,8 +2605,12 @@ describe('NftController', () => { }); it('should be able to clear the ignoredNfts list', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x02', '1', { nftMetadata: { @@ -2547,15 +2622,15 @@ describe('NftController', () => { }, }); - expect(nftController.state.allNfts[selectedAddress][chainId]).toHaveLength( - 1, - ); + expect( + nftController.state.allNfts[selectedAddress][ChainId.mainnet], + ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(0); nftController.removeAndIgnoreNft('0x02', '1'); - expect(nftController.state.allNfts[selectedAddress][chainId]).toHaveLength( - 0, - ); + expect( + nftController.state.allNfts[selectedAddress][ChainId.mainnet], + ).toHaveLength(0); expect(nftController.state.ignoredNfts).toHaveLength(1); nftController.clearIgnoredNfts(); @@ -2691,27 +2766,25 @@ describe('NftController', () => { }); it('should add NFT with null metadata if the ipfs gateway is disabled and opensea is disabled', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const selectedAddress = OWNER_ADDRESS; + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: jest.fn().mockRejectedValue(''), + getERC1155TokenURI: jest.fn().mockResolvedValue('ipfs://*'), + }, + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), + selectedAddress, isIpfsGatewayEnabled: false, openSeaEnabled: false, }); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftURIAndStandard' as any) - .returns(['ipfs://*', ERC1155]); - - const { selectedAddress, chainId } = nftController.config; - await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC1155_NFT_ADDRESS, name: null, @@ -2728,8 +2801,13 @@ describe('NftController', () => { describe('updateNftFavoriteStatus', () => { it('should not set NFT as favorite if nft not found', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2743,7 +2821,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2753,8 +2831,13 @@ describe('NftController', () => { ); }); it('should set NFT as favorite', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2768,7 +2851,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2779,8 +2862,13 @@ describe('NftController', () => { }); it('should set NFT as favorite and then unset it', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2794,7 +2882,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2810,7 +2898,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2821,8 +2909,13 @@ describe('NftController', () => { }); it('should keep the favorite status as true after updating metadata', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2836,7 +2929,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2853,13 +2946,13 @@ describe('NftController', () => { image: 'new_image', name: 'new_name', description: 'new_description', - standard: 'ERC721', + standard: ERC721, }, }, ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ image: 'new_image', @@ -2873,13 +2966,18 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); it('should keep the favorite status as false after updating metadata', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2887,7 +2985,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2904,13 +3002,13 @@ describe('NftController', () => { image: 'new_image', name: 'new_name', description: 'new_description', - standard: 'ERC721', + standard: ERC721, }, }, ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ image: 'new_image', @@ -2924,7 +3022,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); @@ -2991,12 +3089,14 @@ describe('NftController', () => { describe('checkAndUpdateNftsOwnershipStatus', () => { describe('checkAndUpdateAllNftsOwnershipStatus', () => { it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and update the isCurrentlyOwned value to false when NFT is not still owned', async () => { - const { nftController } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3006,26 +3106,28 @@ describe('NftController', () => { favorite: false, }, }); - expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); + expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(false); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave/set the isCurrentlyOwned value to true when NFT is still owned', async () => { - const { nftController } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3037,26 +3139,28 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave the isCurrentlyOwned value as is when NFT ownership check fails', async () => { - const { nftController } = setupController(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'isNftOwner' as any) - .throws(new Error('Unable to verify ownership')); - - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + jest + .spyOn(nftController, 'isNftOwner') + .mockRejectedValue('Unable to verify ownership'); + await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3068,29 +3172,29 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and update the isCurrentlyOwned value to false when NFT is not still owned, when the currently configured selectedAddress/chainId are different from those passed', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, - selectedAddress: OWNER_ADDRESS, + selectedAddress, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3102,13 +3206,11 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3131,8 +3233,13 @@ describe('NftController', () => { describe('checkAndUpdateSingleNftOwnershipStatus', () => { it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and update its isCurrentlyOwned property in state if batch is false and isNftOwner returns false', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + const nft = { address: '0x02', tokenId: '1', @@ -3148,25 +3255,28 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, false); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(false); }); it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and return the updated NFT object without updating state if batch is true', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + const nft = { address: '0x02', tokenId: '1', @@ -3182,19 +3292,17 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); const updatedNft = await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, true); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); @@ -3202,17 +3310,17 @@ describe('NftController', () => { }); it('should check whether the passed NFT is still owned by the the selectedAddress/chainId combination passed in the accountParams argument and update its isCurrentlyOwned property in state, when the currently configured selectedAddress/chainId are different from those passed', async () => { + const firstSelectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, - selectedAddress: OWNER_ADDRESS, + selectedAddress: firstSelectedAddress, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; const nft = { address: '0x02', tokenId: '1', @@ -3228,13 +3336,11 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[firstSelectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3255,6 +3361,7 @@ describe('NftController', () => { }); it('should check whether the passed NFT is still owned by the the selectedAddress/chainId combination passed in the accountParams argument and return the updated NFT object without updating state, when the currently configured selectedAddress/chainId are different from those passed and batch is true', async () => { + const firstSelectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); @@ -3265,7 +3372,6 @@ describe('NftController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; const nft = { address: '0x02', tokenId: '1', @@ -3281,13 +3387,11 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[firstSelectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3329,39 +3433,51 @@ describe('NftController', () => { standard: 'standard', favorite: false, }; - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; it('should return null if the NFT does not exist in the state', async () => { + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + expect( nftController.findNftByAddressAndTokenId( mockNft.address, mockNft.tokenId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBeNull(); }); it('should return the NFT by the address and tokenId', () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); expect( nftController.findNftByAddressAndTokenId( mockNft.address, mockNft.tokenId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toStrictEqual({ nft: mockNft, index: 0 }); }); }); describe('updateNftByAddressAndTokenId', () => { - const { nftController } = setupController(); - + const selectedAddress = OWNER_ADDRESS; const mockTransactionId = '60d36710-b150-11ec-8a49-c377fbd05e27'; const mockNft = { address: '0x02', @@ -3384,12 +3500,17 @@ describe('NftController', () => { transactionId: mockTransactionId, }; - const { selectedAddress, chainId } = nftController.config; - it('should update the NFT if the NFT exist', async () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const { nftController } = setupController({ + options: { + selectedAddress, + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); nftController.updateNft( mockNft, @@ -3397,15 +3518,21 @@ describe('NftController', () => { transactionId: mockTransactionId, }, selectedAddress, - chainId, + ChainId.mainnet, ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual(expectedMockNft); }); it('should return undefined if the NFT does not exist', () => { + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + expect( nftController.updateNft( mockNft, @@ -3413,15 +3540,13 @@ describe('NftController', () => { transactionId: mockTransactionId, }, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBeUndefined(); }); }); describe('resetNftTransactionStatusByTransactionId', () => { - const { nftController } = setupController(); - const mockTransactionId = '60d36710-b150-11ec-8a49-c377fbd05e27'; const nonExistTransactionId = '0123'; @@ -3436,45 +3561,67 @@ describe('NftController', () => { transactionId: mockTransactionId, }; - const { selectedAddress, chainId } = nftController.config; - it('should not update any NFT state and should return false when passed a transaction id that does not match that of any NFT', async () => { + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + expect( nftController.resetNftTransactionStatusByTransactionId( nonExistTransactionId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBe(false); }); it('should set the transaction id of an NFT in state to undefined, and return true when it has successfully updated this state', async () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); expect( - nftController.state.allNfts[selectedAddress][chainId][0].transactionId, + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] + .transactionId, ).toBe(mockTransactionId); expect( nftController.resetNftTransactionStatusByTransactionId( mockTransactionId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBe(true); expect( - nftController.state.allNfts[selectedAddress][chainId][0].transactionId, + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] + .transactionId, ).toBeUndefined(); }); }); describe('updateNftMetadata', () => { it('should update Nft metadata successfully', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const tokenURI = 'https://api.pudgypenguins.io/lil/4'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + selectedAddress, + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3482,13 +3629,11 @@ describe('NftController', () => { networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://api.pudgypenguins.io').get('/lil/4').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3497,9 +3642,9 @@ describe('NftController', () => { image: null, isCurrentlyOwned: true, name: null, - standard: 'ERC721', + standard: ERC721, tokenId: '3', - tokenURI: 'https://api.pudgypenguins.io/lil/4', + tokenURI, }, ]; @@ -3517,7 +3662,7 @@ describe('NftController', () => { image: 'url pudgy', name: 'name pudgy', tokenId: '3', - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: 'https://api.pudgypenguins.io/lil/4', @@ -3525,27 +3670,36 @@ describe('NftController', () => { }); it('should not update metadata when state nft and fetched nft are the same', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.config; - const spy = jest.spyOn(nftController, 'updateNft'); + const selectedAddress = OWNER_ADDRESS; + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + selectedAddress, + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); + const updateNftSpy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { nftMetadata: { name: 'toto', description: 'description', image: 'image.png', - standard: 'ERC721', + standard: ERC721, + tokenURI, }, networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ + nock('https://url') + .get('/') + .reply(200, { name: 'toto', image: 'image.png', description: 'description', - }); + }) + .persist(); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3554,7 +3708,7 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', }, ]; @@ -3564,7 +3718,7 @@ describe('NftController', () => { networkClientId: testNetworkClientId, }); - expect(spy).toHaveBeenCalledTimes(0); + expect(updateNftSpy).toHaveBeenCalledTimes(0); expect( nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0], ).toStrictEqual({ @@ -3574,14 +3728,22 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', + tokenURI, }); }); it('should trigger update metadata when state nft and fetched nft are not the same', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const selectedAddress = OWNER_ADDRESS; + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + selectedAddress, + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3589,18 +3751,16 @@ describe('NftController', () => { name: 'toto', description: 'description', image: 'image.png', - standard: 'ERC721', + standard: ERC721, }, networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'toto', - image: 'image-updated.png', - description: 'description', - }); + nock('https://url').get('/').reply(200, { + name: 'toto', + image: 'image-updated.png', + description: 'description', + }); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3609,7 +3769,7 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', }, ]; @@ -3629,190 +3789,12 @@ describe('NftController', () => { image: 'image-updated.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', - tokenId: '3', - }); - }); - - it('should not update metadata when calls to fetch metadata fail', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.config; - const spy = jest.spyOn(nftController, 'updateNft'); - const testNetworkClientId = 'sepolia'; - await nftController.addNft('0xtest', '3', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .rejects(new Error('Error')); - const testInputNfts: Nft[] = [ - { - address: '0xtest', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '3', - }, - ]; - - await nftController.updateNftMetadata({ - nfts: testInputNfts, - networkClientId: testNetworkClientId, - }); - - expect(spy).toHaveBeenCalledTimes(0); - expect( - nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0], - ).toStrictEqual({ - address: '0xtest', - description: '', - favorite: false, - image: '', - isCurrentlyOwned: true, - name: '', - standard: 'ERC721', + standard: ERC721, tokenId: '3', + tokenURI, }); }); - it('should update metadata when some calls to fetch metadata succeed', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.config; - const spy = jest.spyOn(nftController, 'updateNft'); - const testNetworkClientId = 'sepolia'; - // Add nfts - await nftController.addNft('0xtest1', '1', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - await nftController.addNft('0xtest2', '2', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - await nftController.addNft('0xtest3', '3', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .onFirstCall() - .returns({ - name: 'name pudgy 1', - image: 'url pudgy 1', - description: 'description pudgy 2', - }) - .onSecondCall() - .returns({ - name: 'name pudgy 2', - image: 'url pudgy 2', - description: 'description pudgy 2', - }) - .onThirdCall() - .rejects(new Error('Error')); - - const testInputNfts: Nft[] = [ - { - address: '0xtest1', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '1', - }, - { - address: '0xtest2', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '2', - }, - { - address: '0xtest3', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '3', - }, - ]; - - await nftController.updateNftMetadata({ - nfts: testInputNfts, - networkClientId: testNetworkClientId, - }); - - expect(spy).toHaveBeenCalledTimes(2); - expect( - nftController.state.allNfts[selectedAddress][SEPOLIA.chainId], - ).toStrictEqual([ - { - address: '0xtest1', - description: 'description pudgy 2', - favorite: false, - image: 'url pudgy 1', - isCurrentlyOwned: true, - name: 'name pudgy 1', - standard: 'ERC721', - tokenId: '1', - }, - { - address: '0xtest2', - description: 'description pudgy 2', - favorite: false, - image: 'url pudgy 2', - isCurrentlyOwned: true, - name: 'name pudgy 2', - standard: 'ERC721', - tokenId: '2', - }, - { - address: '0xtest3', - tokenId: '3', - favorite: false, - isCurrentlyOwned: true, - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - ]); - }); - it('should not update metadata when nfts has image/name/description already', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); @@ -3825,7 +3807,7 @@ describe('NftController', () => { name: 'test name', description: 'test description', image: 'test image', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3843,8 +3825,14 @@ describe('NftController', () => { }); it('should trigger calling updateNftMetadata when preferences change - openseaEnabled', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, triggerPreferencesStateChange, changeNetwork } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -3855,7 +3843,7 @@ describe('NftController', () => { name: '', description: '', image: '', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3866,13 +3854,11 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://url').get('/').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); // trigger preference change triggerPreferencesStateChange({ @@ -3886,8 +3872,14 @@ describe('NftController', () => { }); it('should trigger calling updateNftMetadata when preferences change - ipfs enabled', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, triggerPreferencesStateChange, changeNetwork } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -3898,7 +3890,7 @@ describe('NftController', () => { name: '', description: '', image: '', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3909,13 +3901,11 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://url').get('/').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); // trigger preference change triggerPreferencesStateChange({ diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 90ae8560cf..83fdeae592 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1,11 +1,13 @@ import { isAddress } from '@ethersproject/address'; import type { AddApprovalRequest } from '@metamask/approval-controller'; import type { - BaseConfig, - BaseState, RestrictedControllerMessenger, + ControllerStateChangeEvent, +} from '@metamask/base-controller'; +import { + BaseController, + type ControllerGetStateAction, } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; import { safelyExecute, handleFetch, @@ -20,17 +22,19 @@ import { } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkController, NetworkControllerGetNetworkClientByIdAction, + NetworkControllerNetworkDidChangeEvent, NetworkState, } from '@metamask/network-controller'; -import type { PreferencesState } from '@metamask/preferences-controller'; +import type { + PreferencesControllerStateChangeEvent, + PreferencesState, +} from '@metamask/preferences-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { remove0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import BN from 'bn.js'; -import { EventEmitter } from 'events'; import { v4 as random } from 'uuid'; import type { AssetsContractController } from './AssetsContractController'; @@ -76,14 +80,12 @@ type SuggestedNftMeta = { * @property isCurrentlyOwned - Boolean indicating whether the address/chainId combination where it's currently stored currently owns this NFT * @property transactionId - Transaction Id associated with the NFT */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface Nft extends NftMetadata { - tokenId: string; - address: string; - isCurrentlyOwned?: boolean; -} +export type Nft = + | { + tokenId: string; + address: string; + isCurrentlyOwned?: boolean; + } & NftMetadata; type NftUpdate = { nft: Nft; @@ -105,10 +107,7 @@ type NftUpdate = { * @property schemaName - The schema followed by the contract, it could be `ERC721` or `ERC1155` * @property externalLink - External link containing additional information */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftContract { +export type NftContract = { name?: string; logo?: string; address: string; @@ -119,7 +118,7 @@ export interface NftContract { createdDate?: string; schemaName?: string; externalLink?: string; -} +}; /** * @type NftMetadata @@ -139,10 +138,7 @@ export interface NftContract { * @property creator - The NFT owner information object * @property standard - NFT standard name for the NFT, e.g., ERC-721 or ERC-1155 */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftMetadata { +export type NftMetadata = { name: string | null; description: string | null; image: string | null; @@ -164,94 +160,288 @@ export interface NftMetadata { attributes?: Attributes; lastSale?: LastSale; rarityRank?: string; -} - -/** - * @type NftConfig - * - * NFT controller configuration - * @property selectedAddress - Vault selected address - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftConfig extends BaseConfig { - selectedAddress: string; - chainId: Hex; - ipfsGateway: string; - openSeaEnabled: boolean; - useIPFSSubdomains: boolean; - isIpfsGatewayEnabled: boolean; -} +}; /** - * @type NftState + * @type NftControllerState * * NFT controller state * @property allNftContracts - Object containing NFT contract information * @property allNfts - Object containing NFTs per account and network * @property ignoredNfts - List of NFTs that should be ignored */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftState extends BaseState { +export type NftControllerState = { allNftContracts: { - [key: string]: { [chainId: Hex]: NftContract[] }; + [key: string]: { + [chainId: Hex]: NftContract[]; + }; + }; + allNfts: { + [key: string]: { + [chainId: Hex]: Nft[]; + }; }; - allNfts: { [key: string]: { [chainId: Hex]: Nft[] } }; ignoredNfts: Nft[]; -} +}; + +const nftControllerMetadata = { + allNftContracts: { persist: true, anonymous: false }, + allNfts: { persist: true, anonymous: false }, + ignoredNfts: { persist: true, anonymous: false }, +}; const ALL_NFTS_STATE_KEY = 'allNfts'; const ALL_NFTS_CONTRACTS_STATE_KEY = 'allNftContracts'; -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -interface NftAsset { +type NftAsset = { address: string; tokenId: string; -} +}; /** * The name of the {@link NftController}. */ const controllerName = 'NftController'; +export type NftControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + NftControllerState +>; +export type NftControllerActions = NftControllerGetStateAction; + /** * The external actions available to the {@link NftController}. */ -type AllowedActions = +export type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; +export type AllowedEvents = + | PreferencesControllerStateChangeEvent + | NetworkControllerNetworkDidChangeEvent; + +export type NftControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + NftControllerState +>; + +export type NftControllerEvents = NftControllerStateChangeEvent; + /** * The messenger of the {@link NftController}. */ export type NftControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - AllowedActions, - never, + NftControllerActions | AllowedActions, + NftControllerEvents | AllowedEvents, AllowedActions['type'], - never + AllowedEvents['type'] >; -export const getDefaultNftState = (): NftState => { - return { - allNftContracts: {}, - allNfts: {}, - ignoredNfts: [], - }; -}; +export const getDefaultNftControllerState = (): NftControllerState => ({ + allNftContracts: {}, + allNfts: {}, + ignoredNfts: [], +}); /** * Controller that stores assets and exposes convenience methods */ -export class NftController extends BaseControllerV1 { - private readonly mutex = new Mutex(); +export class NftController extends BaseController< + typeof controllerName, + NftControllerState, + NftControllerMessenger +> { + readonly #mutex = new Mutex(); + + /** + * Optional API key to use with opensea + */ + openSeaApiKey?: string; + + #selectedAddress: string; + + #chainId: Hex; + + #ipfsGateway: string; + + #openSeaEnabled: boolean; + + #useIpfsSubdomains: boolean; + + #isIpfsGatewayEnabled: boolean; + + readonly #getERC721AssetName: AssetsContractController['getERC721AssetName']; + + readonly #getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; + + readonly #getERC721TokenURI: AssetsContractController['getERC721TokenURI']; + + readonly #getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; + + readonly #getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; + + readonly #getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - private readonly messagingSystem: NftControllerMessenger; + readonly #onNftAdded?: (data: { + address: string; + symbol: string | undefined; + tokenId: string; + standard: string | null; + source: Source; + }) => void; + + /** + * Creates an NftController instance. + * + * @param options - The controller options. + * @param options.chainId - The chain ID of the current network. + * @param options.selectedAddress - The currently selected address. + * @param options.ipfsGateway - The configured IPFS gateway. + * @param options.openSeaEnabled - Controls whether the OpenSea API is used. + * @param options.useIpfsSubdomains - Controls whether IPFS subdomains are used. + * @param options.isIpfsGatewayEnabled - Controls whether IPFS is enabled or not. + * @param options.getERC721AssetName - Gets the name of the asset at the given address. + * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. + * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. + * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. + * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. + * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. + * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data + * for tracking the NFT added event. + * @param options.messenger - The controller messenger. + * @param options.state - Initial state to set on this controller. + */ + constructor({ + chainId: initialChainId, + selectedAddress = '', + ipfsGateway = IPFS_DEFAULT_GATEWAY_URL, + openSeaEnabled = false, + useIpfsSubdomains = true, + isIpfsGatewayEnabled = true, + getERC721AssetName, + getERC721AssetSymbol, + getERC721TokenURI, + getERC721OwnerOf, + getERC1155BalanceOf, + getERC1155TokenURI, + onNftAdded, + messenger, + state = {}, + }: { + chainId: Hex; + selectedAddress?: string; + ipfsGateway?: string; + openSeaEnabled?: boolean; + useIpfsSubdomains?: boolean; + isIpfsGatewayEnabled?: boolean; + getERC721AssetName: AssetsContractController['getERC721AssetName']; + getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; + getERC721TokenURI: AssetsContractController['getERC721TokenURI']; + getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; + getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; + getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; + onNftAdded?: (data: { + address: string; + symbol: string | undefined; + tokenId: string; + standard: string | null; + source: string; + }) => void; + messenger: NftControllerMessenger; + state?: Partial; + }) { + super({ + name: controllerName, + metadata: nftControllerMetadata, + messenger, + state: { + ...getDefaultNftControllerState(), + ...state, + }, + }); + + this.#selectedAddress = selectedAddress; + this.#chainId = initialChainId; + this.#ipfsGateway = ipfsGateway; + this.#openSeaEnabled = openSeaEnabled; + this.#useIpfsSubdomains = useIpfsSubdomains; + this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; + + this.#getERC721AssetName = getERC721AssetName; + this.#getERC721AssetSymbol = getERC721AssetSymbol; + this.#getERC721TokenURI = getERC721TokenURI; + this.#getERC721OwnerOf = getERC721OwnerOf; + this.#getERC1155BalanceOf = getERC1155BalanceOf; + this.#getERC1155TokenURI = getERC1155TokenURI; + this.#onNftAdded = onNftAdded; + + this.messagingSystem.subscribe( + 'PreferencesController:stateChange', + this.#onPreferencesControllerStateChange.bind(this), + ); + + this.messagingSystem.subscribe( + 'NetworkController:networkDidChange', + this.#onNetworkControllerNetworkDidChange.bind(this), + ); + } + + /** + * Handles the network change on the network controller. + * @param networkState - The new state of the preference controller. + * @param networkState.selectedNetworkClientId - The current selected network client id. + */ + #onNetworkControllerNetworkDidChange({ + selectedNetworkClientId, + }: NetworkState) { + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + selectedNetworkClientId, + ); + this.#chainId = chainId; + } + + /** + * Handles the state change of the preference controller. + * @param preferencesState - The new state of the preference controller. + * @param preferencesState.selectedAddress - The current selected address. + * @param preferencesState.ipfsGateway - The configured IPFS gateway. + * @param preferencesState.openSeaEnabled - Controls whether the OpenSea API is used. + * @param preferencesState.isIpfsGatewayEnabled - Controls whether IPFS is enabled or not. + */ + async #onPreferencesControllerStateChange({ + selectedAddress, + ipfsGateway, + openSeaEnabled, + isIpfsGatewayEnabled, + }: PreferencesState) { + this.#selectedAddress = selectedAddress; + this.#ipfsGateway = ipfsGateway; + this.#openSeaEnabled = openSeaEnabled; + this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; + + const needsUpdateNftMetadata = + (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; + + if (needsUpdateNftMetadata) { + const nfts: Nft[] = + this.state.allNfts[selectedAddress]?.[this.#chainId] ?? []; + // filter only nfts + const nftsToUpdate = nfts.filter( + (singleNft) => + !singleNft.name && !singleNft.description && !singleNft.image, + ); + if (nftsToUpdate.length !== 0) { + await this.updateNftMetadata({ + nfts: nftsToUpdate, + userAddress: selectedAddress, + }); + } + } + } getNftApi() { return `${NFT_API_BASE_URL}/tokens`; @@ -266,24 +456,27 @@ export class NftController extends BaseControllerV1 { * @param passedConfig.userAddress - the address passed through the NFT detection flow to ensure assets are stored to the correct account * @param passedConfig.chainId - the chainId passed through the NFT detection flow to ensure assets are stored to the correct account */ - private updateNestedNftState( - newCollection: Nft[] | NftContract[], - baseStateKey: 'allNfts' | 'allNftContracts', + #updateNestedNftState< + Key extends typeof ALL_NFTS_STATE_KEY | typeof ALL_NFTS_CONTRACTS_STATE_KEY, + NftCollection extends Key extends typeof ALL_NFTS_STATE_KEY + ? Nft[] + : NftContract[], + >( + newCollection: NftCollection, + baseStateKey: Key, { userAddress, chainId }: { userAddress: string; chainId: Hex }, ) { - const { [baseStateKey]: oldState } = this.state; - - const addressState = oldState[userAddress]; - const newAddressState = { - ...addressState, - ...{ [chainId]: newCollection }, - }; - const newState = { - ...oldState, - ...{ [userAddress]: newAddressState }, - }; - this.update({ - [baseStateKey]: newState, + this.update((state) => { + const oldState = state[baseStateKey]; + const addressState = oldState[userAddress] || {}; + const newAddressState = { + ...addressState, + [chainId]: newCollection, + }; + state[baseStateKey] = { + ...oldState, + [userAddress]: newAddressState, + }; }); } @@ -294,7 +487,7 @@ export class NftController extends BaseControllerV1 { * @param tokenId - The NFT identifier. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformationFromApi( + async #getNftInformationFromApi( contractAddress: string, tokenId: string, ): Promise { @@ -374,14 +567,12 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformationFromTokenURI( + async #getNftInformationFromTokenURI( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, ): Promise { - const { ipfsGateway, useIPFSSubdomains, isIpfsGatewayEnabled } = - this.config; - const result = await this.getNftURIAndStandard( + const result = await this.#getNftURIAndStandard( contractAddress, tokenId, networkClientId, @@ -391,7 +582,7 @@ export class NftController extends BaseControllerV1 { const hasIpfsTokenURI = tokenURI.startsWith('ipfs://'); - if (hasIpfsTokenURI && !isIpfsGatewayEnabled) { + if (hasIpfsTokenURI && !this.#isIpfsGatewayEnabled) { return { image: null, name: null, @@ -402,7 +593,7 @@ export class NftController extends BaseControllerV1 { }; } - const isDisplayNFTMediaToggleEnabled = this.config.openSeaEnabled; + const isDisplayNFTMediaToggleEnabled = this.#openSeaEnabled; if (!hasIpfsTokenURI && !isDisplayNFTMediaToggleEnabled) { return { image: null, @@ -415,7 +606,11 @@ export class NftController extends BaseControllerV1 { } if (hasIpfsTokenURI) { - tokenURI = getFormattedIpfsUrl(ipfsGateway, tokenURI, useIPFSSubdomains); + tokenURI = getFormattedIpfsUrl( + this.#ipfsGateway, + tokenURI, + this.#useIpfsSubdomains, + ); } try { @@ -453,14 +648,14 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving NFT uri and token standard. */ - private async getNftURIAndStandard( + async #getNftURIAndStandard( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, ): Promise<[string, string]> { // try ERC721 uri try { - const uri = await this.getERC721TokenURI( + const uri = await this.#getERC721TokenURI( contractAddress, tokenId, networkClientId, @@ -472,7 +667,7 @@ export class NftController extends BaseControllerV1 { // try ERC1155 uri try { - const tokenURI = await this.getERC1155TokenURI( + const tokenURI = await this.#getERC1155TokenURI( contractAddress, tokenId, networkClientId, @@ -507,25 +702,25 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformation( + async #getNftInformation( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, ): Promise { - const chainId = this.getCorrectChainId({ + const chainId = this.#getCorrectChainId({ networkClientId, }); const [blockchainMetadata, nftApiMetadata] = await Promise.all([ safelyExecute(() => - this.getNftInformationFromTokenURI( + this.#getNftInformationFromTokenURI( contractAddress, tokenId, networkClientId, ), ), - this.config.openSeaEnabled && chainId === '0x1' + this.#openSeaEnabled && chainId === '0x1' ? safelyExecute(() => - this.getNftInformationFromApi(contractAddress, tokenId), + this.#getNftInformationFromApi(contractAddress, tokenId), ) : undefined, ]); @@ -548,7 +743,7 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftContractInformationFromContract( + async #getNftContractInformationFromContract( contractAddress: string, networkClientId?: NetworkClientId, ): Promise< @@ -557,8 +752,8 @@ export class NftController extends BaseControllerV1 { Pick > { const [name, symbol] = await Promise.all([ - this.getERC721AssetName(contractAddress, networkClientId), - this.getERC721AssetSymbol(contractAddress, networkClientId), + this.#getERC721AssetName(contractAddress, networkClientId), + this.#getERC721AssetSymbol(contractAddress, networkClientId), ]); return { @@ -576,7 +771,7 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the NFT contract name, image and description. */ - private async getNftContractInformation( + async #getNftContractInformation( contractAddress: string, nftMetadataFromApi: NftMetadata, networkClientId?: NetworkClientId, @@ -586,7 +781,7 @@ export class NftController extends BaseControllerV1 { Pick > { const blockchainContractData = await safelyExecute(() => - this.getNftContractInformationFromContract( + this.#getNftContractInformationFromContract( contractAddress, networkClientId, ), @@ -637,9 +832,9 @@ export class NftController extends BaseControllerV1 { * @param chainId - The chainId of the network where the NFT is being added. * @param userAddress - The address of the account where the NFT is being added. * @param source - Whether the NFT was detected, added manually or suggested by a dapp. - * @returns Promise resolving to the current NFT list. + * @returns A promise resolving to `undefined`. */ - private async addIndividualNft( + async #addIndividualNft( tokenAddress: string, tokenId: string, nftMetadata: NftMetadata, @@ -647,18 +842,17 @@ export class NftController extends BaseControllerV1 { chainId: Hex, userAddress: string, source: Source, - ): Promise { - // TODO: Remove unused return - const releaseLock = await this.mutex.acquire(); + ): Promise { + const releaseLock = await this.#mutex.acquire(); try { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; + const nfts = [...(allNfts[userAddress]?.[chainId] ?? [])]; - const existingEntry: Nft | undefined = nfts.find( + const existingEntry = nfts.find( (nft) => - nft.address.toLowerCase() === tokenAddress.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId, ); @@ -667,46 +861,49 @@ export class NftController extends BaseControllerV1 { nftMetadata, existingEntry, ); - if (differentMetadata || !existingEntry.isCurrentlyOwned) { - // TODO: Switch to indexToUpdate - const indexToRemove = nfts.findIndex( - (nft) => - nft.address.toLowerCase() === tokenAddress.toLowerCase() && - nft.tokenId === tokenId, - ); - /* istanbul ignore next */ - if (indexToRemove !== -1) { - nfts.splice(indexToRemove, 1); - } - } else { - return nfts; + + if (!differentMetadata && existingEntry.isCurrentlyOwned) { + return; } - } - const newEntry: Nft = { - address: tokenAddress, - tokenId, - favorite: existingEntry?.favorite || false, - isCurrentlyOwned: true, - ...nftMetadata, - }; + const indexToUpdate = nfts.findIndex( + (nft) => + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && + nft.tokenId === tokenId, + ); - const newNfts = [...nfts, newEntry]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + if (indexToUpdate !== -1) { + nfts[indexToUpdate] = { + ...existingEntry, + ...nftMetadata, + }; + } + } else { + const newEntry: Nft = { + address: checksumHexAddress, + tokenId, + favorite: false, + isCurrentlyOwned: true, + ...nftMetadata, + }; + + nfts.push(newEntry); + } + + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { chainId, userAddress, }); - if (this.onNftAdded) { - this.onNftAdded({ - address: tokenAddress, + if (this.#onNftAdded) { + this.#onNftAdded({ + address: checksumHexAddress, symbol: nftContract.symbol, tokenId: tokenId.toString(), standard: nftMetadata.standard, source, }); } - return newNfts; } finally { releaseLock(); } @@ -723,7 +920,7 @@ export class NftController extends BaseControllerV1 { * @param options.source - Whether the NFT was detected, added manually or suggested by a dapp. * @returns Promise resolving to the current NFT contracts list. */ - private async addNftContract({ + async #addNftContract({ tokenAddress, userAddress, networkClientId, @@ -736,11 +933,11 @@ export class NftController extends BaseControllerV1 { networkClientId?: NetworkClientId; source?: Source; }): Promise { - const releaseLock = await this.mutex.acquire(); + const releaseLock = await this.#mutex.acquire(); try { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); const { allNftContracts } = this.state; - const chainId = this.getCorrectChainId({ + const chainId = this.#getCorrectChainId({ networkClientId, }); @@ -748,7 +945,8 @@ export class NftController extends BaseControllerV1 { const existingEntry = nftContracts.find( (nftContract) => - nftContract.address.toLowerCase() === tokenAddress.toLowerCase(), + nftContract.address.toLowerCase() === + checksumHexAddress.toLowerCase(), ); if (existingEntry) { return nftContracts; @@ -757,8 +955,8 @@ export class NftController extends BaseControllerV1 { // this doesn't work currently for detection if the user switches networks while the detection is processing // will be fixed once detection uses networkClientIds // get name and symbol if ERC721 then put together the metadata - const contractInformation = await this.getNftContractInformation( - tokenAddress, + const contractInformation = await this.#getNftContractInformation( + checksumHexAddress, nftMetadata, networkClientId, ); @@ -791,7 +989,7 @@ export class NftController extends BaseControllerV1 { /* istanbul ignore next */ const newEntry: NftContract = Object.assign( {}, - { address: tokenAddress }, + { address: checksumHexAddress }, description && { description }, name && { name }, image_url && { logo: image_url }, @@ -804,10 +1002,14 @@ export class NftController extends BaseControllerV1 { external_link && { externalLink: external_link }, ); const newNftContracts = [...nftContracts, newEntry]; - this.updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { - chainId, - userAddress, - }); + this.#updateNestedNftState( + newNftContracts, + ALL_NFTS_CONTRACTS_STATE_KEY, + { + chainId, + userAddress, + }, + ); return newNftContracts; } finally { @@ -824,7 +1026,7 @@ export class NftController extends BaseControllerV1 { * @param options.chainId - The chainId of the network where the NFT is being removed. * @param options.userAddress - The address of the account where the NFT is being removed. */ - private removeAndIgnoreIndividualNft( + #removeAndIgnoreIndividualNft( address: string, tokenId: string, { @@ -835,17 +1037,17 @@ export class NftController extends BaseControllerV1 { userAddress: string; }, ) { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNfts, ignoredNfts } = this.state; const newIgnoredNfts = [...ignoredNfts]; const nfts = allNfts[userAddress]?.[chainId] || []; const newNfts = nfts.filter((nft) => { if ( - nft.address.toLowerCase() === address.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId ) { const alreadyIgnored = newIgnoredNfts.find( - (c) => c.address === address && c.tokenId === tokenId, + (c) => c.address === checksumHexAddress && c.tokenId === tokenId, ); !alreadyIgnored && newIgnoredNfts.push(nft); return false; @@ -853,13 +1055,13 @@ export class NftController extends BaseControllerV1 { return true; }); - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); - this.update({ - ignoredNfts: newIgnoredNfts, + this.update((state) => { + state.ignoredNfts = newIgnoredNfts; }); } @@ -872,22 +1074,22 @@ export class NftController extends BaseControllerV1 { * @param options.chainId - The chainId of the network where the NFT is being removed. * @param options.userAddress - The address of the account where the NFT is being removed. */ - private removeIndividualNft( + #removeIndividualNft( address: string, tokenId: string, { chainId, userAddress }: { chainId: Hex; userAddress: string }, ) { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const newNfts = nfts.filter( (nft) => !( - nft.address.toLowerCase() === address.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId ), ); - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); @@ -902,19 +1104,21 @@ export class NftController extends BaseControllerV1 { * @param options.userAddress - The address of the account where the NFT is being removed. * @returns Promise resolving to the current NFT contracts list. */ - private removeNftContract( + #removeNftContract( address: string, { chainId, userAddress }: { chainId: Hex; userAddress: string }, ): NftContract[] { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNftContracts } = this.state; const nftContracts = allNftContracts[userAddress]?.[chainId] || []; const newNftContracts = nftContracts.filter( (nftContract) => - !(nftContract.address.toLowerCase() === address.toLowerCase()), + !( + nftContract.address.toLowerCase() === checksumHexAddress.toLowerCase() + ), ); - this.updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { + this.#updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { chainId, userAddress, }); @@ -922,173 +1126,7 @@ export class NftController extends BaseControllerV1 { return newNftContracts; } - /** - * EventEmitter instance used to listen to specific EIP747 events - */ - hub = new EventEmitter(); - - /** - * Optional API key to use with opensea - */ - openSeaApiKey?: string; - - /** - * Name of this controller used during composition - */ - override name = 'NftController' as const; - - private readonly getERC721AssetName: AssetsContractController['getERC721AssetName']; - - private readonly getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - - private readonly getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - - private readonly getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - - private readonly getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - - private readonly getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - - private readonly getNetworkClientById: NetworkController['getNetworkClientById']; - - private readonly onNftAdded?: (data: { - address: string; - symbol: string | undefined; - tokenId: string; - standard: string | null; - source: Source; - }) => void; - - /** - * Creates an NftController instance. - * - * @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.getERC721AssetName - Gets the name of the asset at the given address. - * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. - * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. - * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. - * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. - * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. - * @param options.getNetworkClientById - Gets the network client for the given networkClientId. - * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data - * for tracking the NFT added event. - * @param options.messenger - The controller messenger. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - */ - constructor( - { - chainId: initialChainId, - onPreferencesStateChange, - onNetworkStateChange, - getERC721AssetName, - getERC721AssetSymbol, - getERC721TokenURI, - getERC721OwnerOf, - getERC1155BalanceOf, - getERC1155TokenURI, - getNetworkClientById, - onNftAdded, - messenger, - }: { - chainId: Hex; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - getERC721AssetName: AssetsContractController['getERC721AssetName']; - getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - getNetworkClientById: NetworkController['getNetworkClientById']; - onNftAdded?: (data: { - address: string; - symbol: string | undefined; - tokenId: string; - standard: string | null; - source: string; - }) => void; - messenger: NftControllerMessenger; - }, - config?: Partial, - state?: Partial, - ) { - super(config, state); - this.defaultConfig = { - selectedAddress: '', - chainId: initialChainId, - ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - openSeaEnabled: false, - useIPFSSubdomains: true, - isIpfsGatewayEnabled: true, - }; - - this.defaultState = getDefaultNftState(); - this.initialize(); - this.getERC721AssetName = getERC721AssetName; - this.getERC721AssetSymbol = getERC721AssetSymbol; - this.getERC721TokenURI = getERC721TokenURI; - this.getERC721OwnerOf = getERC721OwnerOf; - this.getERC1155BalanceOf = getERC1155BalanceOf; - this.getERC1155TokenURI = getERC1155TokenURI; - this.getNetworkClientById = getNetworkClientById; - this.onNftAdded = onNftAdded; - this.messagingSystem = messenger; - - onPreferencesStateChange( - async ({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }) => { - this.configure({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }); - - const needsUpdateNftMetadata = - (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; - - if (needsUpdateNftMetadata) { - const { chainId } = this.config; - const nfts: Nft[] = - this.state.allNfts[selectedAddress]?.[chainId] ?? []; - // filter only nfts - const nftsToUpdate = nfts.filter( - (singleNft) => - !singleNft.name && !singleNft.description && !singleNft.image, - ); - if (nftsToUpdate.length !== 0) { - await this.updateNftMetadata({ - nfts: nftsToUpdate, - userAddress: selectedAddress, - }); - } - } - }, - ); - - onNetworkStateChange(({ selectedNetworkClientId }) => { - const selectedNetworkClient = getNetworkClientById( - selectedNetworkClientId, - ); - const { chainId } = selectedNetworkClient.configuration; - - this.configure({ chainId }); - }); - } - - private async validateWatchNft( + async #validateWatchNft( asset: NftAsset, type: NFTStandardType, userAddress: string, @@ -1143,15 +1181,21 @@ export class NftController extends BaseControllerV1 { // temporary method to get the correct chainId until we remove chainId from the config & the chainId arg from the detection logic // Just a helper method to prefer the networkClient chainId first then the chainId argument and then finally the config chainId - private getCorrectChainId({ + #getCorrectChainId({ networkClientId, }: { networkClientId?: NetworkClientId; }) { if (networkClientId) { - return this.getNetworkClientById(networkClientId).configuration.chainId; + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + return chainId; } - return this.config.chainId; + return this.#chainId; } /** @@ -1174,17 +1218,17 @@ export class NftController extends BaseControllerV1 { origin: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - await this.validateWatchNft(asset, type, userAddress); + await this.#validateWatchNft(asset, type, userAddress); - const nftMetadata = await this.getNftInformation( + const nftMetadata = await this.#getNftInformation( asset.address, asset.tokenId, networkClientId, @@ -1252,7 +1296,7 @@ export class NftController extends BaseControllerV1 { ): Promise { // Checks the ownership for ERC-721. try { - const owner = await this.getERC721OwnerOf( + const owner = await this.#getERC721OwnerOf( nftAddress, tokenId, networkClientId, @@ -1265,7 +1309,7 @@ export class NftController extends BaseControllerV1 { // Checks the ownership for ERC-1155. try { - const balance = await this.getERC1155BalanceOf( + const balance = await this.#getERC1155BalanceOf( ownerAddress, nftAddress, tokenId, @@ -1297,7 +1341,7 @@ export class NftController extends BaseControllerV1 { address: string, tokenId: string, { - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, source, }: { @@ -1305,7 +1349,7 @@ export class NftController extends BaseControllerV1 { networkClientId?: NetworkClientId; source?: Source; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { if ( @@ -1339,7 +1383,7 @@ export class NftController extends BaseControllerV1 { tokenId: string, { nftMetadata, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, source = Source.Custom, networkClientId, }: { @@ -1347,18 +1391,22 @@ export class NftController extends BaseControllerV1 { userAddress?: string; source?: Source; networkClientId?: NetworkClientId; - } = { userAddress: this.config.selectedAddress }, + } = { userAddress: this.#selectedAddress }, ) { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); nftMetadata = nftMetadata || - (await this.getNftInformation(tokenAddress, tokenId, networkClientId)); + (await this.#getNftInformation( + checksumHexAddress, + tokenId, + networkClientId, + )); - const newNftContracts = await this.addNftContract({ - tokenAddress, + const newNftContracts = await this.#addNftContract({ + tokenAddress: checksumHexAddress, userAddress, networkClientId, source, @@ -1368,13 +1416,13 @@ export class NftController extends BaseControllerV1 { // If NFT contract was not added, do not add individual NFT const nftContract = newNftContracts.find( (contract) => - contract.address.toLowerCase() === tokenAddress.toLowerCase(), + contract.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); // If NFT contract information, add individual NFT if (nftContract) { - await this.addIndividualNft( - tokenAddress, + await this.#addIndividualNft( + checksumHexAddress, tokenId, nftMetadata, nftContract, @@ -1395,14 +1443,14 @@ export class NftController extends BaseControllerV1 { */ async updateNftMetadata({ nfts, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, }: { nfts: Nft[]; userAddress?: string; networkClientId?: NetworkClientId; }) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const nftsWithChecksumAdr = nfts.map((nft) => { return { @@ -1410,9 +1458,9 @@ export class NftController extends BaseControllerV1 { address: toChecksumHexAddress(nft.address), }; }); - const nftMetadataResults = await Promise.allSettled( + const nftMetadataResults = await Promise.all( nftsWithChecksumAdr.map(async (nft) => { - const resMetadata = await this.getNftInformation( + const resMetadata = await this.#getNftInformation( nft.address, nft.tokenId, networkClientId, @@ -1423,26 +1471,22 @@ export class NftController extends BaseControllerV1 { }; }), ); - const successfulNewFetchedNfts = nftMetadataResults.filter( - (result): result is PromiseFulfilledResult => - result.status === 'fulfilled', - ); + // We want to avoid updating the state if the state and fetched nft info are the same - const nftsWithDifferentMetadata: PromiseFulfilledResult[] = []; + const nftsWithDifferentMetadata: NftUpdate[] = []; const { allNfts } = this.state; const stateNfts = allNfts[userAddress]?.[chainId] || []; - successfulNewFetchedNfts.forEach((singleNft) => { + nftMetadataResults.forEach((singleNft) => { const existingEntry: Nft | undefined = stateNfts.find( (nft) => - nft.address.toLowerCase() === - singleNft.value.nft.address.toLowerCase() && - nft.tokenId === singleNft.value.nft.tokenId, + nft.address.toLowerCase() === singleNft.nft.address.toLowerCase() && + nft.tokenId === singleNft.nft.tokenId, ); if (existingEntry) { const differentMetadata = compareNftMetadata( - singleNft.value.newMetadata, + singleNft.newMetadata, existingEntry, ); @@ -1454,12 +1498,7 @@ export class NftController extends BaseControllerV1 { if (nftsWithDifferentMetadata.length !== 0) { nftsWithDifferentMetadata.forEach((elm) => - this.updateNft( - elm.value.nft, - elm.value.newMetadata, - userAddress, - chainId, - ), + this.updateNft(elm.nft, elm.newMetadata, userAddress, chainId), ); } } @@ -1478,22 +1517,25 @@ export class NftController extends BaseControllerV1 { tokenId: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); - address = toChecksumHexAddress(address); - this.removeIndividualNft(address, tokenId, { chainId, userAddress }); + const chainId = this.#getCorrectChainId({ networkClientId }); + const checksumHexAddress = toChecksumHexAddress(address); + this.#removeIndividualNft(checksumHexAddress, tokenId, { + chainId, + userAddress, + }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const remainingNft = nfts.find( - (nft) => nft.address.toLowerCase() === address.toLowerCase(), + (nft) => nft.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); if (!remainingNft) { - this.removeNftContract(address, { chainId, userAddress }); + this.#removeNftContract(checksumHexAddress, { chainId, userAddress }); } } @@ -1511,24 +1553,24 @@ export class NftController extends BaseControllerV1 { tokenId: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); - address = toChecksumHexAddress(address); - this.removeAndIgnoreIndividualNft(address, tokenId, { + const chainId = this.#getCorrectChainId({ networkClientId }); + const checksumHexAddress = toChecksumHexAddress(address); + this.#removeAndIgnoreIndividualNft(checksumHexAddress, tokenId, { chainId, userAddress, }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const remainingNft = nfts.find( - (nft) => nft.address.toLowerCase() === address.toLowerCase(), + (nft) => nft.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); if (!remainingNft) { - this.removeNftContract(address, { chainId, userAddress }); + this.#removeNftContract(checksumHexAddress, { chainId, userAddress }); } } @@ -1536,7 +1578,9 @@ export class NftController extends BaseControllerV1 { * Removes all NFTs from the ignored list. */ clearIgnoredNfts() { - this.update({ ignoredNfts: [] }); + this.update((state) => { + state.ignoredNfts = []; + }); } /** @@ -1554,13 +1598,13 @@ export class NftController extends BaseControllerV1 { nft: Nft, batch: boolean, { - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { address, tokenId } = nft; let isOwned = nft.isCurrentlyOwned; try { @@ -1573,28 +1617,42 @@ export class NftController extends BaseControllerV1 { // we want to keep the current value of isCurrentlyOwned for this flow. } - nft.isCurrentlyOwned = isOwned; + const updatedNft = { + ...nft, + isCurrentlyOwned: isOwned, + }; if (batch) { - return nft; + return updatedNft; } // if this is not part of a batched update we update this one NFT in state const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; - const nftToUpdate = nfts.find( + const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; + const indexToUpdate = nfts.findIndex( (item) => item.tokenId === tokenId && item.address.toLowerCase() === address.toLowerCase(), ); - if (nftToUpdate) { - nftToUpdate.isCurrentlyOwned = isOwned; - this.updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { + + if (indexToUpdate !== -1) { + nfts[indexToUpdate] = updatedNft; + this.update((state) => { + state.allNfts[userAddress] = Object.assign( + {}, + state.allNfts[userAddress], + { + [chainId]: nfts, + }, + ); + }); + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); } - return nft; + + return updatedNft; } /** @@ -1607,12 +1665,12 @@ export class NftController extends BaseControllerV1 { async checkAndUpdateAllNftsOwnershipStatus( { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const updatedNfts = await Promise.all( @@ -1626,7 +1684,7 @@ export class NftController extends BaseControllerV1 { }), ); - this.updateNestedNftState(updatedNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(updatedNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); @@ -1648,17 +1706,17 @@ export class NftController extends BaseControllerV1 { favorite: boolean, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; + const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; const index: number = nfts.findIndex( (nft) => nft.address === address && nft.tokenId === tokenId, ); @@ -1675,7 +1733,7 @@ export class NftController extends BaseControllerV1 { // Update Nfts array nfts[index] = updatedNft; - this.updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { chainId, userAddress, }); @@ -1748,7 +1806,7 @@ export class NftController extends BaseControllerV1 { updatedNft, ...nfts.slice(nftInfo.index + 1), ]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { chainId, userAddress: selectedAddress, }); @@ -1787,7 +1845,7 @@ export class NftController extends BaseControllerV1 { ...nfts.slice(index + 1), ]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { chainId, userAddress: selectedAddress, }); diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index 11b90114dd..8984134a0e 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -25,7 +25,7 @@ import { buildMockGetNetworkClientById, } from '../../network-controller/tests/helpers'; import { Source } from './constants'; -import { getDefaultNftState } from './NftController'; +import { getDefaultNftControllerState } from './NftController'; import { NftDetectionController, BlockaidResultType, @@ -800,7 +800,7 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); const mockGetNftState = jest.fn().mockImplementation(() => { return { - ...getDefaultNftState(), + ...getDefaultNftControllerState(), ignoredNfts: [ // This address and token ID are always detected, as determined by // the nock mocks setup in `beforeEach` @@ -1168,7 +1168,7 @@ async function withController( messenger: controllerMessenger, disabled: true, addNft: jest.fn(), - getNftState: getDefaultNftState, + getNftState: getDefaultNftControllerState, ...options, }); diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 7b8d5171b0..25f7fd6ef4 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -25,7 +25,7 @@ import type { import { Source } from './constants'; import { type NftController, - type NftState, + type NftControllerState, type NftMetadata, } from './NftController'; @@ -361,7 +361,7 @@ export class NftDetectionController extends StaticIntervalPollingController< readonly #addNft: NftController['addNft']; - readonly #getNftState: () => NftState; + readonly #getNftState: () => NftControllerState; /** * The controller options @@ -384,7 +384,7 @@ export class NftDetectionController extends StaticIntervalPollingController< messenger: NftDetectionControllerMessenger; disabled: boolean; addNft: NftController['addNft']; - getNftState: () => NftState; + getNftState: () => NftControllerState; }) { super({ name: controllerName, diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1322c58d39..d7387825f6 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -1,7 +1,18 @@ export * from './AccountTrackerController'; export * from './AssetsContractController'; export * from './CurrencyRateController'; -export * from './NftController'; +export type { + NftControllerState, + NftControllerMessenger, + NftControllerActions, + NftControllerGetStateAction, + NftControllerEvents, + NftControllerStateChangeEvent, + Nft, + NftContract, + NftMetadata, +} from './NftController'; +export { getDefaultNftControllerState, NftController } from './NftController'; export * from './NftDetectionController'; export type { TokenBalancesControllerMessenger,