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

Create script to update tokens list from ethereum-lists/tokens #1247

Merged
merged 6 commits into from Apr 13, 2018

Conversation

wbobeirne
Copy link
Contributor

@wbobeirne wbobeirne commented Mar 4, 2018

Triage temporary solution for #969.

Description

Right now we've just been keeping up to date with MyCryptoHQ/mycrypto.com for token lists changes, but the future plan is to use ethereum-lists/tokens as our source of tokens. The problem is its format is kind of spread out and not easy to import directly to our repo. This PR adds a script that fetches its output from IPFS, imports and formats the tokens to match our format, and stages the tokens for commit. This won't stop us from manually updating, but should make the process much easier.

One concern of this change is that our token list has ballooned from ~90 to 513 with this. This'll be a much larger load on our node, and probably includes a lot of junk tokens. Not to mention, some of them don't use proper symbol acronyms, and instead have rather long names. We may want to either audit ethereum-lists/tokens and clean out any useless ones, or implement a blacklist of tokens to exclude.

Also of note, I'm not sure decimals or amounts are being handled properly right now with tokens. See screenshot below. Fixed, see comment below.

Changes

  • Add a script to a new scripts/ folder that fetches tokens.
  • Updated tokens list with what was fetched from the aforementioned script.

Steps to Test

  1. Checkout eth-lists-script
  2. Run npm run update:tokens
  3. Confirm the script runs without fail
  4. Confirm no changes are made, since the token lists should be the same

Screenshots

screen shot 2018-03-04 at 5 01 05 pm

screen shot 2018-03-04 at 5 07 15 pm

^ Are these amounts correct? Is something wrong with decimals?

@wbobeirne wbobeirne added status: ready for review Open PR's that are ready for technical review. Replaced with "changes needed" or "ready to merge". type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable. labels Mar 4, 2018
@wbobeirne
Copy link
Contributor Author

Looks like CI is failing with the new tokens, which probably means we'll need to clean up ethereum-lists/tokens before we can fully rely on it.

@wbobeirne
Copy link
Contributor Author

I fixed the incorrect decimals issue, the json from IPFS had decimals as strings, not ints. Fixed that up during import:

screen shot 2018-03-04 at 5 24 24 pm

HenryNguyen5 and others added 4 commits April 12, 2018 23:51
* Update scripts to handle collisions, and use typescript

* Add comment on duplicateAddress validator

* Lock dep on ts-node

* Fix tsc errors
@dternyak dternyak merged commit 574c628 into develop Apr 13, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 33.685% when pulling e4004ee on eth-lists-script into 7da50ae on develop.

@dternyak dternyak deleted the eth-lists-script branch April 16, 2018 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready for review Open PR's that are ready for technical review. Replaced with "changes needed" or "ready to merge". type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants