-
-
Notifications
You must be signed in to change notification settings - Fork 183
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
Adding a fallback for tokenList API service based on user preference #517
Conversation
… OFF token detection from API
f296190
to
e4f1ba6
Compare
const tokenList: TokenMap = {}; | ||
for (const tokenAddress in contractmap) { | ||
const { erc20, logo, ...token } = contractmap[tokenAddress]; | ||
if (erc20) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we don't want to show the ERC721 tokens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This token list will be used for the detection of ERC20 tokens. The ERC721 tokens are handles separately as collectibles
in this repo.
Co-authored-by: Alex Donesky <alex.donesky@consensys.net>
Co-authored-by: Alex Donesky <alex.donesky@consensys.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noice. non blocking q about the future of fallbacks for token list controller
@@ -1,10 +1,12 @@ | |||
import { stub } from 'sinon'; | |||
import nock from 'nock'; | |||
import contractmap from '@metamask/contract-metadata'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Q] I agree this is the best thing we can do for a fallback, but is there a more long term fallback option that doesn't require us to maintain this repo anymore? Perhaps if the user could supply their own token list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran that by @omnat and I am quoting her reply here
Yes that’s a valid point and in the long term that’s the plan to allow people to upload their own lists. But for now this fallback is better than no auto detection, even if we don’t keep updating this list
If the user does not want to use the token service API to get the token details, this fallback to fetch from the
contract-metadata
will be called. The value ofuseStaticTokenList
will determine which token list will be loaded to the TokenListcontroller state, the true value of which will call the Static token list from thecontract-metadata
, and a false value will fetch the token list from token service API. The cache will be stored only for when the token service API is used, when the user switch to the static token list the cache will be cleared.