Change extended public and private prefixes to match torba #245

Closed
opened 2018-12-06 22:35:50 +01:00 by tzarebczan · 6 comments
tzarebczan commented 2018-12-06 22:35:50 +01:00 (Migrated from github.com)

See original discussion in https://github.com/lbryio/torba/issues/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
See original discussion in https://github.com/lbryio/torba/issues/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 ```
alyssaoc commented 2018-12-12 19:19:13 +01:00 (Migrated from github.com)

This will become relevant with the upstream merge

This will become relevant with the upstream merge
kaykurokawa commented 2019-01-07 18:09:56 +01:00 (Migrated from github.com)

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.

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 commented 2019-03-22 15:44:36 +01:00 (Migrated from github.com)

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?

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 commented 2019-03-22 17:29:10 +01:00 (Migrated from github.com)

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.

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 commented 2019-03-22 19:07:27 +01:00 (Migrated from github.com)

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.

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 commented 2019-05-08 16:42:31 +02:00 (Migrated from github.com)

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

This is done correctly in the now-current master. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbrycrd#245
No description provided.