Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Consolidate token list controller data #527

Merged
merged 30 commits into from
Aug 6, 2021

Conversation

Cal-L
Copy link
Contributor

@Cal-L Cal-L commented Jul 19, 2021

Issue

  • TokenListController should consolidate all tokens regardless of static or dynamic into one data type: TokenListToken
  • For images, TokenListController returns a file name from static tokens and URLs from dynamic tokens.
  • FUTURE PROPOSAL: TokenListController should split the images field into two separate properties iconPath for static tokens and iconUrl for dynamic tokens.

Resolves/Changes

  • Map static tokens to return TokenListToken type. The idea was already there but I mapped it more, added address, occurrences, and aggregators.
  • Export TokenListToken type from TokenListController for use in the apps
  • Update TokenListController tests

…rn same token type regardless of static or dynamic from token list controller.
…r tests. Add util test for converting logo to URL.
@Cal-L Cal-L requested a review from a team as a code owner July 19, 2021 02:37
src/assets/TokenListController.test.ts Outdated Show resolved Hide resolved
src/assets/TokenListController.ts Outdated Show resolved Hide resolved
@Cal-L Cal-L force-pushed the improvement/consolidate-token-list-controller-data branch from a17df03 to 6ef6fbe Compare July 29, 2021 22:50
package.json Outdated Show resolved Hide resolved
@rickycodes rickycodes force-pushed the improvement/consolidate-token-list-controller-data branch from f62e3b7 to f2fb9e8 Compare July 30, 2021 14:51
rickycodes
rickycodes previously approved these changes Jul 30, 2021
Copy link
Contributor

@rickycodes rickycodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on small nit re: pinning dep, but otherwise LGTM

package.json Outdated Show resolved Hide resolved
src/apis/token-service.ts Show resolved Hide resolved
@rickycodes rickycodes force-pushed the improvement/consolidate-token-list-controller-data branch from 2227f55 to b90f0e2 Compare July 30, 2021 16:34
src/assets/TokenListController.ts Show resolved Hide resolved
src/assets/TokenListController.ts Outdated Show resolved Hide resolved
src/assets/TokenListController.ts Outdated Show resolved Hide resolved
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed two minor issues, but this looks good once the extraneous stuff is removed!

src/assets/TokenListController.ts Outdated Show resolved Hide resolved
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Cal-L Cal-L merged commit 176418f into main Aug 6, 2021
@Cal-L Cal-L deleted the improvement/consolidate-token-list-controller-data branch August 6, 2021 02:02
rickycodes added a commit that referenced this pull request Aug 10, 2021
…to dynamic-speed-up

* 'dynamic-speed-up' of github.com:MetaMask/controllers:
  Consolidate token list controller data (#527)
  14.1.0 (#553)
  Feature: Add selector subscriptions (#551)
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Add utils function to convert contract metadata logo to iconUrl. Return same token type regardless of static or dynamic from token list controller.

* Provide ContractMap type for legacy tokens. Update TokenListController tests. Add util test for converting logo to URL.

* Provide root path for TokenListController. NOTE: Static tokens will return with ABSOLUTE file paths. Client side will need to be aware of that.

* Export Token as TokenListToken from TokenListController to prevent conflict with TokenRateController.

* Clean up TokenListController. Update static logo path.

* Clean up TokenListController. Fix TokenListController tests.

* Fix github actions linting

* Fix more github actions linting

* Fix even more github actions linting

* Fix last github actions linting

* Allow for more time for token list controller API calls before timeout

* Fix linting error

* Add back newline to EOF package.json

* Update contract metadata version

* Up token list timeout to 10s

* Update split logic

* Add getFileExt utility and test

* Add back caret to contract-metadata

* Update description for fetchFromCache function

* Remove iconPath

* Remove unused enum & comment

Co-authored-by: Ricky Miller <ricky.miller@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Add utils function to convert contract metadata logo to iconUrl. Return same token type regardless of static or dynamic from token list controller.

* Provide ContractMap type for legacy tokens. Update TokenListController tests. Add util test for converting logo to URL.

* Provide root path for TokenListController. NOTE: Static tokens will return with ABSOLUTE file paths. Client side will need to be aware of that.

* Export Token as TokenListToken from TokenListController to prevent conflict with TokenRateController.

* Clean up TokenListController. Update static logo path.

* Clean up TokenListController. Fix TokenListController tests.

* Fix github actions linting

* Fix more github actions linting

* Fix even more github actions linting

* Fix last github actions linting

* Allow for more time for token list controller API calls before timeout

* Fix linting error

* Add back newline to EOF package.json

* Update contract metadata version

* Up token list timeout to 10s

* Update split logic

* Add getFileExt utility and test

* Add back caret to contract-metadata

* Update description for fetchFromCache function

* Remove iconPath

* Remove unused enum & comment

Co-authored-by: Ricky Miller <ricky.miller@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants