BIP85: revert XPRV breaking changes for application 32'

scgbckbone commented at 9:43 pm on October 4, 2024: contributor

revert (only) breaking changes wrt XPRV (application 32')
no need to mention 0x00 private key prepend as it is defined in BIP-032

rever XPRV breaking changes for application 32' a3ff1eacfa

nvk commented at 9:44 pm on October 4, 2024: none

ACK

Rob1Ham commented at 9:47 pm on October 4, 2024: none

ACK - we should never break user space, if someone has an alternative implementation it can go in a different bip

changelog corrections 32fef25e53

in bip-0085.mediawiki:442 in 32fef25e53

438@@ -438,8 +439,7 @@ BIP32, BIP39
439 
440 * 1.0 (2020-07)
441 * 2.0.0 (2024-09-22)
442-    * Swap chain code and private key bytes in application 32' for consistentcy with BIP-32 (major change)
443-    * Correct derived entropy for application 128169' test vector (major change)
444+    * Correct derived entropy for application 707764' test vector (bugfix)

scgbckbone commented at 9:53 pm on October 4, 2024:

@akarve I assume this was bug in application number ? you mentioned 128169' in previous PR BUT wrong entropy was in 707764'. I see no breaking changes in hex app

akarve commented at 0:24 am on October 7, 2024:

The only vector that needs correction as of my simplified PR #1679 is Hex (128169’).

If somehow an extant implementation got the old test vector to work this correction would be a breaking change but I agree that if the correct BIP 85 algo is used then this is non-breaking :)

luisschwab commented at 10:20 pm on October 4, 2024: none

ACK

jonatack renamed this:
~~rever XPRV breaking changes for application 32'~~
BIP85: revert XPRV breaking changes for application 32'
on Oct 4, 2024

jonatack added the label Proposed BIP modification on Oct 4, 2024

in bip-0085.mediawiki:245 in 32fef25e53

243-private key<ref name="curve-order" />. Prepend an empty byte (<code>0x00</code>)
244-per BIP32 on master key serialization. Use the last (rightmost) 32 bytes as the chain code.
245+Taking 64 bytes of the HMAC digest, the first 32 bytes are the chain code,
246+and second 32 bytes are the private key for BIP32 XPRV value.
247+
248+WARNING: This is not consistent with BIP-032

jonatack commented at 10:53 pm on October 4, 2024:

a3ff1eacfa290fd559f1fa8b8d396890e568881c nit, may as well use the same convention as the rest of this BIP, not a blocker

0WARNING: This is not consistent with BIP32

in bip-0085.mediawiki:262 in 32fef25e53

258@@ -258,7 +259,7 @@ INPUT:
259 
260 OUTPUT
261 * DERIVED ENTROPY=ead0b33988a616cf6a497f1c169d9e92562604e38305ccd3fc96f2252c177682
262-* DERIVED XPRV=xprv9s21ZrQH143K4Px85utdpu6DFvY2NpHkJajPoupAznfiacH2MC9LasyW4uvqKXNxLWcjqGTbHKAhoZoMAbmRe5g9tAPA7cUUX4UVA1vFKFm
263+* DERIVED XPRV=xprv9s21ZrQH143K2srSbCSg4m4kLvPMzcWydgmKEnMmoZUurYuBuYG46c6P71UGXMzmriLzCCBvKQWBUv3vPB3m1SATMhp3uEjXHJ42jFg7myX

jonatack commented at 10:55 pm on October 4, 2024:

Verified this change indeed reverts to the previous xprv in https://github.com/bitcoin/bips/pull/1600/files#diff-eae7a61b6d2a0b6091c1ce04618f216cbb871d987b985ba712de89e04014654cL235.

in bip-0085.mediawiki:243 in 32fef25e53

238@@ -239,9 +239,10 @@ OUTPUT
239 ===XPRV===
240 Application number: 32'
241 
242-Consistent with BIP32, use the first (leftmost) 32 bytes of the derived entropy as the
243-private key<ref name="curve-order" />. Prepend an empty byte (<code>0x00</code>)
244-per BIP32 on master key serialization. Use the last (rightmost) 32 bytes as the chain code.
245+Taking 64 bytes of the HMAC digest, the first 32 bytes are the chain code,
246+and second 32 bytes are the private key for BIP32 XPRV value.

jonatack commented at 10:58 pm on October 4, 2024:

In a3ff1eacfa290fd559f1fa8b8d396890e568881c, is there a reason not to keep the previous footnote at https://github.com/bitcoin/bips/pull/1600/files#diff-eae7a61b6d2a0b6091c1ce04618f216cbb871d987b985ba712de89e04014654cL379?

0and second 32 bytes[1] are the private key for BIP32 XPRV value.

jonatack commented at 11:00 pm on October 4, 2024: member

Approach ACK

jonatack commented at 11:04 pm on October 4, 2024: member

The reference implementations section would also need to be reverted.

nvk commented at 11:07 pm on October 4, 2024: none

Approach ACK

Thank you 🙏

jonatack removed the label Proposed BIP modification on Oct 4, 2024

jonatack added the label Bug fix on Oct 4, 2024

jonatack commented at 11:34 pm on October 4, 2024: member

The reference implementations section would also need to be reverted.

I’ve done a full revert in #1674.

From #1600 (comment):

Although the BIP is still in Draft status, I think it should have been marked as proposed or final a long time ago as it does appear to be deployed by a few projects. @scgbckbone would you like to update this pull to do that instead?

scgbckbone commented at 12:40 pm on October 5, 2024: contributor

closing this - as full revert already merged #1674

thanks @jonatack

scgbckbone closed this on Oct 5, 2024

scgbckbone commented at 12:46 pm on October 5, 2024: contributor

The reference implementations section would also need to be reverted.

I’ve done a full revert in #1674.

From #1600 (comment):

Although the BIP is still in Draft status, I think it should have been marked as proposed or final a long time ago as it does appear to be deployed by a few projects.

@scgbckbone would you like to update this pull to do that instead?

created new PR instead https://github.com/bitcoin/bips/pull/1676

BIP85: revert XPRV breaking changes for application 32' #1673