-
Notifications
You must be signed in to change notification settings - Fork 127
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
add bip44 derivation paths #604
Conversation
Yes, it's better to add the derivation path for ERC20 tokens the same as for platform coins, because there is still a possibility to activate the ERC20 token with the legacy
|
Done. I've also updated a few tickers which did not have a protocol suffix (ref: #498) Will handle Trezor name info in separate PR. This info is not available in a convenient form - the list at https://trezor.io/coins is incomplete - for example QTUM is not shown (and is not in source code of page) until explicitly searched for. I'll contact Satoshi Labs to see if they can provide a complete list in csv or json format. |
if tickers are changed, they need to be changed in https://github.com/KomodoPlatform/coins/tree/master/api_ids too |
I've updated the API json files, we can change logic in a different PR |
api_ids/coinpaprika_ids.json
Outdated
@@ -204,7 +204,7 @@ | |||
"FIRO-BEP20": "firo-firo", | |||
"FIRO": "firo-firo", | |||
"FJC-BEP20": "fjc-fujicoin", | |||
"FJC": "fjc-fujicoin", | |||
"FJCB-BEP20": "fjc-fujicoin", |
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.
that is the wrong coin, FJC is still FJC, the native coin, FJCB (next line) is now FJCB-BEP20, the BEP20 token
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.
LGTM! One question - README.md
contains 7. Trezor coin name (optional)
topic, but trezor_coin
field hasn't been added in this PR. Do you plan to add the field at the next PR?
README.md
Outdated
|
||
## 6. Derivation Path | ||
|
||
The [BIP44 derivation path](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki) os now required to ensure Heirarchical deterministic wallet functionality. The best source for this data is via [Satoshi Labs SLP-044](https://github.com/satoshilabs/slips/blob/master/slip-0044.md) |
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 path os now required
the typo?
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.
fixed, thanks
Will handle Trezor name info in separate PR. This info is not available in a convenient form - the list at https://trezor.io/coins is incomplete - for example QTUM is not shown (and is not in source code of page) until explicitly searched for. I've contacted Satoshi Labs to see if they can provide a complete list in csv or json format and am awaiting a response. If no reply soon, I'll add what I can from the default page link. |
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.
lgtm
Sourced from https://github.com/satoshilabs/slips/blob/master/slip-0044.md
Need to merge this before merging KomodoPlatform/developer-docs#420 (comment)
@sergeyboyko0791 For coins like
XXX-PLG20
should the MATIC derivation path be added (and same for other EVMs)?And re: #512 (comment) what will happen with coins that lack this info?