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

Discussion: naming scheme #186

Open
ligi opened this Issue Oct 8, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@ligi
Collaborator

ligi commented Oct 8, 2017

currently the naming uses a 3 letter indicator for the network.Like this:

tokens-etc.json  tokens-kov.json  tokens-rop.json
tokens-eth.json  tokens-rin.json  tokens-rsk.json

I would suggest to use the chain-Id. So you need to have one less mapping and it is more future proof.
like this:

tokens-rin.json -> tokens-4.json
tokens-eth.json -> tokens-1.json

@409H

This comment has been minimized.

Contributor

409H commented Oct 8, 2017

Hi Ligi, I'm unaware of the chain ids - is this a de-facto numbering system (ie: 4 is always rinkeby) & if so, can you link me to the official documentation?

Also, the extra mapping would be done on the consumer sides, not ethereum-lists, so we'd need to discuss more on management (ie: remembering the chain_id values for each testnet) when accepting PRs and such.

@409H 409H added the discussion label Oct 8, 2017

@ligi

This comment has been minimized.

Collaborator

ligi commented Oct 9, 2017

To be honest I struggle to find the "official documentation" on chainId currently - but this is the case - each chain has a Id ..
You see it e.g. mentioned in ethereum/EIPs#155
We can also do the mapping in the scripts - but the mapping should IMHO not be done on the consumers

@409H

This comment has been minimized.

Contributor

409H commented Oct 9, 2017

To be honest I struggle to find the "official documentation" on chainId currently - but this is the case - each chain has a Id

I understand - all good here 👍 Would be great to find any official documentation (EIP, code comments, whatever) regarding this so we can reference it in a CONTRIBUTING.md document sometime.

We can also do the mapping in the scripts

My understanding for this repo is for the .json files to be consumed by an application, and it's just a repo to hold data dumps. Which scripts are you referencing here to do the mapping?

@ligi

This comment has been minimized.

Collaborator

ligi commented Oct 9, 2017

This script which is currently checking validity:
https://github.com/MyEtherWallet/ethereum-lists/blob/master/src/main/kotlin/ethereum/Main.kt

and generates minimized versions of the json
could also do the mapping.

@409H

This comment has been minimized.

Contributor

409H commented Oct 9, 2017

Ah, okay, totally missed that.

I'm neutral to this change, but if we do it, we'd need to document the id mapping in CONTRIBUTING.md :)

@tayvano

This comment has been minimized.

Contributor

tayvano commented Oct 9, 2017

I think this is a good plan. However, for my own sanity, can we somehow put the chain name at the end (which isn't used for validation)?

Eg tokens_1-rop or something.

The other thing is that there are chain IDs of 2 and maybe 3 numbers. So we should probably just do tokens_001 to be safe.

I can also check on how this is operating on v4.

@ligi

This comment has been minimized.

Collaborator

ligi commented Oct 9, 2017

perhaps it is better in the validation/minimizing step - ideally it is just the number - so you can directly load - without using a wildcard or use the mapping - just "tokens-${chainid}.json"
I will prepare for this a PR. PS: there is another one open: https://github.com/MyEtherWallet/ethereum-lists/pulls

@ligi

This comment has been minimized.

Collaborator

ligi commented Oct 26, 2017

can you link me to the official documentation?

https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment