Assets controller #18
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
======================================
Coverage 100% 100%
======================================
Files 14 15 +1
Lines 626 770 +144
Branches 73 99 +26
======================================
+ Hits 626 770 +144
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This is a very strong pull request, I have very little feedback. Really well done. I created MetaMask/website#134 to track stuff around eth-contract-metadata. Feel free to land this whenever you want and I can cut a new release (or show you how to do that as well.)
src/AssetsController.ts
Outdated
| * | ||
| * @param address - Hex address of the collectible contract | ||
| * @param tokenId - The NFT identifier | ||
| * @returns - Current collectible list |
There was a problem hiding this comment.
Nit: This may be more accurate if it says "Promise resolving to the current collectible list".
src/AssetsController.ts
Outdated
| * @param tokenId - The NFT identifier | ||
| * @returns - Current collectible name and image | ||
| */ | ||
| async requestNFTCustomInformation(address: string, tokenId: number): Promise<CollectibleCustomInformation> { |
There was a problem hiding this comment.
Do you think this method should be public? Will it ever be called externally? Same question for fetchCollectibleBasicInformation. If these methods will only ever be used internally to this controller, I say we make them private to keep the external API as small as possible.
There was a problem hiding this comment.
totally agree, I'll move it
|
Thanks @bitpshr. I'll wait for you in order to coordinate the integration to the app as well. |
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
======================================
Coverage 100% 100%
======================================
Files 14 15 +1
Lines 626 760 +134
Branches 73 99 +26
======================================
+ Hits 626 760 +134
Continue to review full report at Codecov.
|
v1.0.1 contains two changes that should have been considered semver-major: - Type-interface incompatability with previous version (MetaMask/json-rpc-engine#139) - Introduced dependency `@metamask/json-rpc-engine` indicates a minimum supported Node.js version of 16. This prevents the module from installing on some package manager configurations, like default yarn classic. This will be re-released as v2.0.0.
Escalate errors on RPC responses
* assets controller tokens * Update PreferencesController no more tokens responsability * Update AssetsController handling collectibles * Update tests according to new AssetsController * Fix some lines on AssetsController * AssetsController using preferences selected address and assets getter * AssetsController tokens object with selected address as keys * AssetsController test tokens per account * AssetsController tokens by selected address respective test * AssetsController update new tokens when adding token * AssetsController add and remove collectibles per account * AssetsController assets per account and network * AssetsCOntroller minor documentation additions * AssetsController clean up * AssetsController clean up * AssetsController test update for private methods
* assets controller tokens * Update PreferencesController no more tokens responsability * Update AssetsController handling collectibles * Update tests according to new AssetsController * Fix some lines on AssetsController * AssetsController using preferences selected address and assets getter * AssetsController tokens object with selected address as keys * AssetsController test tokens per account * AssetsController tokens by selected address respective test * AssetsController update new tokens when adding token * AssetsController add and remove collectibles per account * AssetsController assets per account and network * AssetsCOntroller minor documentation additions * AssetsController clean up * AssetsController clean up * AssetsController test update for private methods
This PR adds AssetsController in order to support assets on it. First two assets supported are tokens (ERC20) and collectibles (ERC721).
Related #17
BLOCKER: eth-contract-metadata is not the official, we should decide what to do with collectibles metadata before merge.