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

Expand number of cryptocurrencies the Manager supports #881

Closed
brantlymillegan opened this issue Jul 29, 2020 · 17 comments
Closed

Expand number of cryptocurrencies the Manager supports #881

brantlymillegan opened this issue Jul 29, 2020 · 17 comments
Assignees

Comments

@brantlymillegan
Copy link
Contributor

No description provided.

@brantlymillegan
Copy link
Contributor Author

brantlymillegan commented Jul 29, 2020

There are a number of top cryptocurrencies we don't support in our Manager that we need to support, such as (in order):

  • ADA
  • BSV
  • XTZ
  • XMR
  • VET
  • NEO
  • ERD
  • ALGO
  • QTUM
  • HBAR
  • ICX
  • ZIL

@makoto
Copy link
Member

makoto commented Aug 12, 2020

Just checked cointypes at https://github.com/satoshilabs/slips/blob/master/slip-0044.md

cointypes

ADA = 1815
BSV = 236
XTZ = 1729
XMR = 128
VET = 818
NEO = 888
ERD = 508
ALGO = 283
QTUM = 2301
HBAR (XHB) = 3030
ICX = 74
ZIL = 313
FLOW = 539
NEAR = 397

@makoto
Copy link
Member

makoto commented Aug 12, 2020

And some notes on format

@brantlymillegan
Copy link
Contributor Author

Yes, but we should still support it

@brantlymillegan
Copy link
Contributor Author

brantlymillegan commented Aug 17, 2020

Additional ones:

  • FIL
  • AVA
  • FLOW
  • ZEC

@makoto makoto self-assigned this Aug 18, 2020
@brantlymillegan
Copy link
Contributor Author

I just got off a call with Gemini, and they are interested in various kinds of integration with ENS. One thing they pointed out is that we support in our Manager all the cryptocurrencies they do except for ZEC. To show good will to Gemini, @makoto could you prioritize trying to get ZEC address support in the Manager ASAP?

@makoto
Copy link
Member

makoto commented Aug 19, 2020

@brantlymillegan can you check with Gemini if they are okay with just storing transparent address, not shielded address?

There is outstanding PR on ZCASH but @Arachnid commented the need to support shielded address.

ensdomains/address-encoder#10

I spoke to Trust wallet team who already support zcash but they also only support transparent address.

@brantlymillegan
Copy link
Contributor Author

brantlymillegan commented Aug 19, 2020

@brantlymillegan can you check with Gemini if they are okay with just storing transparent address, not shielded address?

There is outstanding PR on ZCASH but @Arachnid commented the need to support shielded address.

ensdomains/address-encoder#10

I spoke to Trust wallet team who already support zcash but they also only support transparent address.

But don't we want to support both transparent and shielded addresses? Is it a lot harder to support both? And if we support transparent addresses right now, can we easily add support for shielded addresses later? @Arachnid in that thread you linked talks about the need for it to be backwards compatible. Was that PR changed to be backwards compatible so that we could add shielded addresses later?

@makoto
Copy link
Member

makoto commented Aug 19, 2020

I don't know zcash well but not sure if you want to store shielded address (which suppose to be secret) into a publicly accessible place. I also don't know how hard to implement support for the shielded address tbh.

@makoto
Copy link
Member

makoto commented Aug 19, 2020

@Arachnid In terms of supporting transparent address, the PR I showed above is a bit out of date (people did some optimisation refactoring), so I tried to implement myself as follows but the testing isn't passing. Am I wrong to understand that 0xb8 and 0xbd are the version number? What should I put instead if it's wrong?

Screenshot 2020-08-19 at 19 14 20

Screenshot 2020-08-19 at 19 12 57

@brantlymillegan
Copy link
Contributor Author

brantlymillegan commented Aug 19, 2020

@makoto I just talked to @Arachnid . He said it'd be great if we could add support for both at the same time. But if we can't do that and we only add support for transparent addresses, that's fine as long as the encoding makes it clear people can distinguish between them. To determine that we probably just need to talk to a Zcash dev (otherwise it could take us a long time to try to figure out on our own).

@makoto
Copy link
Member

makoto commented Aug 24, 2020

@Arachnid could you review ensdomains/address-encoder#45 . Also we should pay bounty to the guy who originally submitted ensdomains/address-encoder#10 as it's mostly his code

@makoto
Copy link
Member

makoto commented Aug 24, 2020

@Arachnid this one is also good to go for Tezos ensdomains/address-encoder#46 . In terms of your earlier comment about the pr17, I commented here.

NOTE: I noticed that TrustWallet encode/decode differently so I reported to the team.

@makoto
Copy link
Member

makoto commented Aug 25, 2020

@Arachnid can you also either merge or close ensdomains/address-encoder#35 ? This could potentially impact other outstanding PRs

@makoto
Copy link
Member

makoto commented Aug 25, 2020

Also added DOT as it was super easy extension to KSM ensdomains/address-encoder#47

@Arachnid
Copy link
Member

Arachnid commented Sep 1, 2020

@makoto Thanks! I left some comments on your PRs.

@brantlymillegan
Copy link
Contributor Author

@makoto I've set up bounties for a bunch of these for the Apollo Gitcoin hackathon we were invited to, and I'm getting a lot of responses. So please check the outstanding PRs tab before doing the work to add a new cryptocurrency address

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants