Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Use new token contract's source #477

Merged

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Mar 26, 2019

This PR is almost entirely copied from an old commit from @amaurymartiny that's taken from MyCrypto 🥇

  • closes Use an up to date token currated list #267

  • For now, and for when this happens, we need to manually build the token lists, for the record it needs:
    - git clone https://github.com/ethereum-lists/tokens
    - cd tokens
    - ./gradlew run
    - upload and pin to IPFS the files in ./build/output/full/<ipfs_network_name>.json
    - change the hardcoded IPFS addresses in /fether/scripts/updateTokens/update-tokens.ts
    - run yarn update-tokens --use-hardcoded-ipfs-addresses

  • tested Fether by adding tokens that I know weren't present before, e.g MKR or ZRX

  • tested successfully on Mainnet with DAI, DADI (non 0 balances).
    To reproduce without touching real accounts simply paste the following in the console: localStorage.setItem("localforage/__paritylight::paritySignerAccounts", '[{"address":"0x77d0C0A0B7a9bEfd0ebB3b9A9E445fA9eeFfB67A","name":"Random","chainId":1}]');, this will add the account as a Signer account, and you can see it has ETH, DAI and DADI tokens. Note that it's a random account found on the internet :)

  • added a fallback image in case the token image is either not present or not reachable. It looks like this:
    image

@amaury1093
Copy link
Collaborator

Just wondering: instead of copy/pasting mycrypto's scripts here, could we use git submodules? https://git-scm.com/book/en/v2/Git-Tools-Submodules

Seems to me like a more elegant method (because there's no copy/paste duplication), but maybe I did like this in my old commit because git modules were overly complicated (for me).

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 26, 2019

You tell me as I'm far from a git wizard.
The modifications I made allow me to fetch not only foundation but also kovan and ropsten, and map these to the ipfs names (eth, kov and rop).

edit: I added comments on the PR to mention where I modified things

[key: string]: T;
}

const networks = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added this

const ipfsHtml = await httpsGet(ipfsTargetUrl);

// Get the IPFS url for the each network tokens json. Regexxing HTML hurts, but w/e
networks.forEach(async network => {
Copy link
Collaborator Author

@Tbaut Tbaut Mar 26, 2019

Choose a reason for hiding this comment

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

I've added this forEach loop to fetch the tokens for all our supported networks

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 26, 2019

After discussion, it looks like using submodules is overkill for this specific case as we'd have to clone the whole repo (there's no possibility to create a submodule out of a directory).

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 28, 2019

My last commit makes the whole thing "usable" with hardcoded addresses or fetched from the ethereum-list github repo (not working atm, I'm talking with ligi).

To test you can run yarn update-tokens --use-hardcoded-ipfs-addresses.
This basically fetches the json token files directly from hardcoded IPFS addresses that I pushed and pinned with my node. These addresses should be reachable.

edit : updated first post where everything is explained.

for the full story The code that fetches the addresses from the `ethereum-list` repo actually first retrieves an html page (1) with a list of links to the files (2) that we are interested in. Since IPFS is content addressed, the addresses of the files that I hardcoded should be the same as the files (2) contained on that html page (1). I generated the files from the `ethereum-list` repo and uploaded them to ipfs (see 1st post how). Now the reason why the script is still blocked even though I uploaded and pinned the seemingly same files is that the first IPFS end-point the html page (1) containing this list of files is unreachable!

@Tbaut Tbaut requested a review from amaury1093 March 28, 2019 15:22
Copy link
Collaborator

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

This works well for me! 💯

Also, you can remove the fether-react/scripts folder

packages/fether-react/src/Tokens/TokensList/TokensList.js Outdated Show resolved Hide resolved
packages/fether-ui/src/TokenCard/TokenCard.js Show resolved Hide resolved
packages/fether-ui/src/TokenCard/TokenCard.js Outdated Show resolved Hide resolved
scripts/updateTokens/update-tokens.ts Outdated Show resolved Hide resolved
scripts/updateTokens/update-tokens.ts Outdated Show resolved Hide resolved
scripts/updateTokens/update-tokens.ts Outdated Show resolved Hide resolved
scripts/updateTokens/index.ts Outdated Show resolved Hide resolved
@Tbaut Tbaut requested a review from amaury1093 March 28, 2019 17:13
Copy link
Collaborator

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

lgtm

packages/fether-react/src/Send/TxForm/TxForm.js Outdated Show resolved Hide resolved
@Tbaut Tbaut merged commit a1d014b into master Mar 28, 2019
@Tbaut Tbaut deleted the tbaut-use-token-list-from-git.luolix.top/ethereum-lists/tokens branch March 28, 2019 20:24
logo: tokenLogo;
}

export interface tokenLogo {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Tbaut i'm in the process i've merging these changes into the luke-124-security branch, and this doesn't let me commit without fixing a pre-commit linting error paritytech/fether/scripts/updateTokens/types/TokensJson.ts:13:9: 'tokenLogo' was used before it was define

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh weird that I haven't been noticed about that. I'd then simply advise to move the lines starting with export token interface tokenLogo toward the top of the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it only showed the linting error because I didn't realise I was using an older version of Node.js.

@Tbaut Tbaut mentioned this pull request Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use an up to date token currated list
3 participants