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

Change extended public and private prefixes to match torba #245

Closed
tzarebczan opened this issue Dec 6, 2018 · 6 comments
Closed

Change extended public and private prefixes to match torba #245

tzarebczan opened this issue Dec 6, 2018 · 6 comments
Assignees
Labels
type: improvement Existing (or partially existing) functionality needs to be changed

Comments

@tzarebczan
Copy link
Contributor

See original discussion in lbryio/torba#45. Since Torba matched lbryum and most users are on lbryum wallets, and lbrycrd doesn't use hd wallets yet, it makes sense to change this on LBRYcrd to match the light wallet.

@kaykurokawa agreed via chat:

kaykurokawa [24 hours ago]
but yes we can change the EXT_PUBLIC / EXT_PRIVATE header to match whatever torba is, they are currently not used by lbrycrd , we use SECRET and if torba's not using SECRET they should match lbrycrd
@tzarebczan tzarebczan added type: improvement Existing (or partially existing) functionality needs to be changed needs: priority Priority level needs to be defined labels Dec 6, 2018
@alyssaoc
Copy link

This will become relevant with the upstream merge

@alyssaoc alyssaoc removed the needs: priority Priority level needs to be defined label Dec 12, 2018
@kaykurokawa
Copy link

kaykurokawa commented Jan 7, 2019

For the Core upstream merge, please make sure that base58Prefixes[EXT_PUBLIC_KEY] and base58Prefixes[EXT_SECRET_KEY] in chainparams.cpp remains the same as what is used in Bitcoin, and not what is currently used in lbrycrd.

Torba uses the same prefixes as Bitcoin for mainnet,testnet, and regtest. See: https://github.com/lbryio/torba/blob/master/torba/coin/bitcoinsegwit.py (and here is Bitcoin chainparam: https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L132 )

I think fixing this in the current code base (without upstream merge) is unnecessary as those variables in chainparams is never used. However, the upstream merge will pull in features that will use those variables so please close this issue once we have the upstream PR in place and the above specified change is confirmed.

@tzarebczan
Copy link
Contributor Author

It's not only the extended keys that are different, but the pubkey and script prefixes are as well: https://github.com/lbryio/torba/blob/master/torba/coin/bitcoinsegwit.py#L65 . What are the ramifications of changing those on LBRYcrd?

@tzarebczan
Copy link
Contributor Author

Nevermind, Kay had linked the wrong torba params - here they are on lbrynet: https://github.com/lbryio/lbry/blob/master/lbrynet/extras/wallet/ledger.py#L35

So the prefixes do match lbrycrd, just the extended ones don't.

@tzarebczan
Copy link
Contributor Author

tzarebczan commented Mar 22, 2019

We should also ensure that the BIP44 constant for LBRY is used: https://github.com/satoshilabs/slips/blob/master/slip-0044.md

Edit: Nevermind, this is only required for multicurrency wallets.

@BrannonKing
Copy link
Member

This is done correctly in the now-current master. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement Existing (or partially existing) functionality needs to be changed
Projects
None yet
Development

No branches or pull requests

5 participants