BIP85: Clarify spec, correct test vectors, add Portuguese language code, add dice application #1600

akarve commented at 8:55 PM on May 26, 2024: contributor

Summary of changes:

Clarify drng_reader.read is a first-class function (not an evaluation)
Clarify endianness
Clarify possibility for TPRV keys
Add new reference implementation in Python
Clarify that HD-Seed-WIF uses most significant bits
Add language code for Portuguese under BIP-32 application
Correct entropy for BASE64 test vector
Correct byte order and output for XPRV test vector
Add new champion

Unit tests for all substantive changes can be found in the reference implementation here: https://github.com/akarve/bipsea/pull/63

akarve commented at 2:27 AM on May 27, 2024: contributor

Glad to remove my implementation from reference implementations per #1580 but on review I think you will find that my implementation is more complete and unit-tested than the original. I was also unable to get the original to run at all after install.

murchandamus added the label Proposed BIP modification on May 30, 2024

murchandamus added the label Pending acceptance on May 30, 2024

murchandamus commented at 4:49 PM on May 30, 2024: contributor

Paging @ethankosakovsky

akarve commented at 2:16 AM on June 4, 2024: contributor

I will be happy to hear from @ethankosakovsky but he hasn't been active on GH since 2021. I have no idea if there's a procedure for this, nevertheless I volunteer to become a maintainer of this BIP if the author cannot be reached.

akarve renamed this:
~~BIP85: Clarify spec, correct PWD BASE64 test vector entropy~~
BIP85: Clarify spec, correct PWD BASE64 test vector, add Portuguese language code
on Jun 4, 2024

in bip-0085.mediawiki:231 in ce0f1b1af1 outdated

 227 | @@ -223,9 +228,14 @@ OUTPUT
 228 |  Application number: 32'
 229 |  
 230 |  Taking 64 bytes of the HMAC digest, the first 32 bytes are the chain code, and second 32 bytes[1] are the private key for BIP32 XPRV value. Child number, depth, and parent fingerprint are forced to zero.
 231 | +(Note that this ordering is opposite BIP32, where the chain code is the least significant 32 bytes.)

akarve commented at 2:46 AM on June 4, 2024:

I'm looking for guidance as to whether or not we change the order in this spec and update the corresponding XPRV test vector (that's what I'd recommend) or just make a note?

You can verify that BIP-32 is the opposite here. Under CKDpriv and CKDpub BIP-32 states:

"The returned chain code ci is IR."

R is for right. You will also see "most significant byte first" throughout BIP-32 indicating "big endian". BIP85 is also big endian, it states "Truncate trailing (least significant) bytes."

I have all BIP-32 vectors and the (backwards?) XPRV BIP-85 vector passing. Here you can see the tested CKDpriv implementation takes the first 32 bytes for the chain code and the also passing BIP-85 implementation takes the last 32 bytes for the chain code.

I hope I'm not missing something obvious?

murchandamus commented at 8:04 PM on June 5, 2024: contributor

Hey @akarve, I’ve tried to send an email to Ethan, let’s see what comes back. If we do not hear from Ethan, it seems to me that BIP 2 provides an option for another champion to take over stewardship of an existing BIP when the original Champion falls off the face of the earth.

akarve commented at 10:13 PM on June 22, 2024: contributor

@murchandamus Howdy. Any luck reaching Ethan?

murchandamus commented at 2:12 PM on June 26, 2024: contributor

Hey @akarve, I was unable to reach @ethankosakovsky per the email address provided in this BIP.

Sorry, there is very little precedent for this situation, so I’m still making this up as we go. We could have done this in parallel, beside trying to reach out directly, but I would like to propose the following next steps: Please post about your proposed changes on the Bitcoin Developer Mailing list, and point out that you volunteer to become the champion of BIP 85. You could also request in that email that if someone is in touch with Ethan, it would be appreciated if they could reach out to make him aware. Assuming that Ethan isn’t reactivated and nobody objects, we would then try to get some review for your changes, add you as a second BIP champion, and merge this PR.

akarve commented at 8:50 PM on June 27, 2024: contributor

@murchandamus Done. The mailing list topic is "BIP-85 Champion Unreachable? Please weigh in." in case you'd like to comment.

murchandamus commented at 6:11 PM on June 28, 2024: contributor

Yeah, I saw that. Could you please add yourself as a champion to the BIP in this PR?

akarve commented at 2:57 AM on June 29, 2024: contributor

Will do. Can you advise as to whether champions for PRs in Status: Draft should remedy inconsistencies with other BIPs, like this inconsistency with BIP-32 or leave them as is for backwards compatibility?

RandyMcMillan commented at 2:08 PM on July 3, 2024: contributor

It may be useful to be consistent with bytes or bits in the document.

Preferably use bits throughout?

akarve renamed this:
~~BIP85: Clarify spec, correct PWD BASE64 test vector, add Portuguese language code~~
BIP85: Clarify spec, correct test vectors, add Portuguese language code, add dice application
on Jul 4, 2024

akarve commented at 9:52 PM on July 4, 2024: contributor

It may be useful to be consistent with bytes or bits in the document. Preferably use bits throughout?

Yeah, I've done that wherever possible though there are some cases where say BIP-39 uses bits or where one needs an odd number of bits so I've kept those cases as-is.

I was more worried about changing the byte order for the XPRV application but went ahead with it in the name of consistency with BIP-32.

akarve commented at 11:10 PM on July 15, 2024: contributor

@murchandamus hi, we about good to go here?

lmk if i need to do anything for the failing "Diff checks" — it's objecting to changes to the header block, which are necessary in this rare case.

in bip-0085.mediawiki:5 in 0dd8012cae outdated

   1 | @@ -2,7 +2,7 @@
   2 |    BIP: 85
   3 |    Layer: Applications
   4 |    Title: Deterministic Entropy From BIP32 Keychains
   5 | -  Author: Ethan Kosakovsky <ethankosakovsky@protonmail.com>
   6 | +  Author: Aneesh Karve <dowsing.seaport0d@icloud.com>

murchandamus commented at 8:14 PM on July 17, 2024:

One of these should work:

  Author: Ethan Kosakovsky <ethankosakovsky@protonmail.com>, Aneesh Karve <dowsing.seaport0d@icloud.com>

or:

  Author: Ethan Kosakovsky <ethankosakovsky@protonmail.com>
          Aneesh Karve <dowsing.seaport0d@icloud.com>

akarve commented at 9:41 PM on July 17, 2024:

I tried the first one earlier and it made CI cranky. Let's try the second one.

in bip-0085.mediawiki:224 in 0dd8012cae outdated

 220 | @@ -207,7 +221,9 @@ OUTPUT:
 221 |  ===HD-Seed WIF===
 222 |  Application number: 2'
 223 |  
 224 | -Uses 256 bits[1] of entropy as the secret exponent to derive a private key and encode as a compressed WIF which will be used as the hdseed for Bitcoin Core wallets.
 225 | +Uses the most significant 32 bytes[1] of entropy as the secret exponent to derive

murchandamus commented at 8:19 PM on July 17, 2024:

You can define references inline as follows

Uses the most significant 32 bytes<ref name="curve-order">There is a very small chance that you'll make an invalid key that is zero or bigger than the order of the curve. If this occurs, software should hard fail (forcing users to iterate to the next index).</ref> of entropy as the secret exponent to derive

and then replace the Footnotes section at the bottom with

==Footnotes==

<references />

in bip-0085.mediawiki:242 in 0dd8012cae outdated

 237 | @@ -222,17 +238,26 @@ OUTPUT
 238 |  ===XPRV===
 239 |  Application number: 32'
 240 |  
 241 | -Taking 64 bytes of the HMAC digest, the first 32 bytes are the chain code, and second 32 bytes[1] are the private key for BIP32 XPRV value. Child number, depth, and parent fingerprint are forced to zero.
 242 | +Consistent with BIP32, use the first 32 bytes of the derived entropy as the chain code.
 243 | +Use the last 32 bytes[1] as the private key. Prepend an empty byte (<code>0x00</code>)

murchandamus commented at 8:23 PM on July 17, 2024:

Use the last 32 bytes<ref name="curve-order" /> as the private key. Prepend an empty byte (<code>0x00</code>)

murchandamus commented at 8:37 PM on July 17, 2024: contributor

Thanks for your patience.

It would be best if you add yourself as a second author rather than replace Ethan.

Can you advise as to whether champions for PRs in Status: Draft should remedy inconsistencies with other BIPs, like this inconsistency with BIP-32 or leave them as is for backwards compatibility?

Given that this BIP was drafted more than four years ago, there seems to be a good chance that it is already in use. Insofar, it seems best to remain compatible with the original specification rather than fixing inconsistencies with other BIPs.

lmk if i need to do anything for the failing "Diff checks" — it's objecting to changes to the header block, which are necessary in this rare case.

Yeah, you will need to also update the table in the repository’s README to align it with your preamble’s fields.

The changes seem fine to me, but it would be great if some people that are more invested in the content of this BIP also reviewed it.

akarve commented at 10:03 PM on July 17, 2024: contributor

Given that this BIP was drafted more than four years ago, there seems to be a good chance that it is already in use. Insofar, it seems best to remain compatible with the original specification rather than fixing inconsistencies with other BIPs.

Compared to BIP-32, WIF is relatively unused. On the chance that there are security implications to the widely used BIP-32 order I stuck with BIP-32 ordering. I can change this if the reviewers don't agree.

akarve requested review from murchandamus on Jul 17, 2024

in bip-0085.mediawiki:223 in d3ffda6e45 outdated

 218 | @@ -221,9 +219,12 @@ OUTPUT:
 219 |  ===HD-Seed WIF===
 220 |  Application number: 2'
 221 |  
 222 | -Uses the most significant 32 bytes[1] of entropy as the secret exponent to derive
 223 | -a private key and encode as a compressed WIF which will be used as the hdseed for
 224 | -Bitcoin Core wallets.
 225 | +Uses the most significant 32 bytes
 226 | +<ref name="curve-order">There is a very small chance that you'll make an invalid

murchandamus commented at 12:16 PM on July 18, 2024:

Nit: the linebreak causes a space before the reference link.

Uses the most significant 32 bytes<ref name="curve-order">
There is a very small chance that you'll make an invalid

murchandamus commented at 12:22 PM on July 18, 2024: contributor

Given that this BIP was drafted more than four years ago, there seems to be a good chance that it is already in use. Insofar, it seems best to remain compatible with the original specification rather than fixing inconsistencies with other BIPs.

Compared to BIP-32, WIF is relatively unused. On the chance that there are security implications to the widely used BIP-32 order I stuck with BIP-32 ordering. I can change this if the reviewers don't agree.

I had the impression that the scheme had not changed significantly (first 32 bytes for the chain code, last 32 bytes for the private key). Or is this about the endianness?

If it did change, especially in a backwards incompatible manner, could you please document the changes of the Specification in a Change Log as seen for example in BIP 352?

jonatack added the label PR Author action required on Jul 24, 2024

akarve commented at 9:00 PM on September 22, 2024: contributor

@murchandamus Change Log added. We should be good to go :) Thanks for your assistance in getting this over the line.

akarve requested review from murchandamus on Sep 22, 2024

jonatack removed the label Pending acceptance on Sep 23, 2024

in bip-0085.mediawiki:37 in 41d15a7d5a outdated

  33 | @@ -33,6 +34,9 @@ The terminology related to keychains used in the wild varies widely, for example
  34 |  # '''BIP39 mnemonic''' is the mnemonic phrase that is calculated from the entropy used before hashing of the mnemonic in BIP39.
  35 |  # '''BIP39 seed''' is the result of hashing the BIP39 mnemonic seed.
  36 |  
  37 | +When in doubt assume '''big endian''' byte serialization such that the leftmost

jonatack commented at 7:09 PM on September 23, 2024:

When in doubt, assume '''big endian''' byte serialization, such that the leftmost

in bip-0085.mediawiki:88 in 41d15a7d5a outdated

  84 | @@ -78,7 +85,7 @@ BIP85-DRNG-SHAKE256 is a deterministic random number generator for cryptographic
  85 |  RSA key generation is an example of a function that requires orders of magnitude more than 64 bytes of random input. Further, it is not possible to precalculate the amount of random input required until the function has completed.
  86 |  
  87 |      drng_reader = BIP85DRNG.new(bip85_entropy)
  88 | -    rsa_key = RSA.generate_key(4096, drng_reader.read())
  89 | +    rsa_key = RSA.generate_key(4096, drng_reader.read)

jonatack commented at 7:11 PM on September 23, 2024:

Why this change?

akarve commented at 11:24 PM on September 23, 2024:

Subtle point but it's a first-class function that you pass in and and not an invocation.

in bip-0085.mediawiki:364 in 41d15a7d5a outdated

 359 | +The derivation path format is: <code>m/83696968'/89101'/{sides}'/{rolls}'/{index}'</code>
 360 | +
 361 | +    2 <= sides <= 2^32 - 1
 362 | +    1 <= rolls <= 2^32 - 1
 363 | +
 364 | +Use this application to generate pin numbers or any other numeric secret.

jonatack commented at 7:15 PM on September 23, 2024:

Use this application to generate PIN numbers or any other numeric secret.

in bip-0085.mediawiki:365 in 41d15a7d5a outdated

 360 | +
 361 | +    2 <= sides <= 2^32 - 1
 362 | +    1 <= rolls <= 2^32 - 1
 363 | +
 364 | +Use this application to generate pin numbers or any other numeric secret.
 365 | +Roll values are zero indexed such that an N-sided die produces values in the range

jonatack commented at 7:15 PM on September 23, 2024:

Roll values are zero-indexed, such that an N-sided die produces values in the range

in bip-0085.mediawiki:54 in 41d15a7d5a outdated

  50 | @@ -47,10 +51,13 @@ Ultimately, all of the mnemonic/seed schemes start with some "initial entropy" t
  51 |  
  52 |  We assume a single BIP32 master root key. This specification is not concerned with how this was derived (e.g. directly or via a mnemonic scheme such as BIP39).
  53 |  
  54 | -For each application that requires its own wallet, a unique private key is derived from the BIP32 master root key using a fully hardened derivation path. The resulting private key (k) is then processed with HMAC-SHA512, where the key is "bip-entropy-from-k", and the message payload is the private key k: <code>HMAC-SHA512(key="bip-entropy-from-k", msg=k)</code>. The result produces 512 bits of entropy. Each application SHOULD use up to the required number of bits necessary for their operation truncating the rest.
  55 | +For each application that requires its own wallet, a unique private key is derived from the BIP32 master root key using a fully hardened derivation path. The resulting private key (k) is then processed with HMAC-SHA512, where the key is "bip-entropy-from-k", and the message payload is the private key k: <code>HMAC-SHA512(key="bip-entropy-from-k", msg=k)</code>. The result is 512 bits of entropy. Each application SHOULD use up to the required number of bits necessary for their operation truncating the rest.

jonatack commented at 7:20 PM on September 23, 2024:

Why this change?

akarve commented at 11:27 PM on September 23, 2024:

Will revert for simplicity.

in bip-0085.mediawiki:227 in 41d15a7d5a outdated

 223 | +Uses the most significant 32 bytes<ref name="curve-order">
 224 | +There is a very small chance that you'll make an invalid
 225 | +key that is zero or bigger than the order of the curve. If this occurs, software
 226 | +should hard fail (forcing users to iterate to the next index).</ref>
 227 | +of entropy as the secret exponent to derive a private key and encode as a compressed
 228 | +WIF which to be used as the hdseed for Bitcoin Core wallets.

jonatack commented at 7:39 PM on September 23, 2024:

Unsure which one depending on desired meaning, but needs one of (1) remove " which" or (2) s/which/, which is/ or (3) s/which/that is/

akarve commented at 11:36 PM on September 23, 2024:

Good eye, I'll just fall back to the original text (formerly mangled).

in bip-0085.mediawiki:105 in 41d15a7d5a outdated

  99 | @@ -93,14 +100,15 @@ OUTPUT
 100 |  
 101 |  ==Reference Implementation==
 102 |  
 103 | -* Python library implementation: [https://github.com/ethankosakovsky/bip85]
 104 | -* JavaScript library implementation: [https://github.com/hoganri/bip85-js]
 105 | +* Python 3.x library implementation: [https://github.com/akarve/bipsea]
 106 | +* Legacy Python library implementation: [https://github.com/ethankosakovsky/bip85]
 107 | +* Legacy JavaScript library implementation: [https://github.com/hoganri/bip85-js]

jonatack commented at 7:41 PM on September 23, 2024:

Are the two legacy implementations still compatible with this version 2.0.0 of this draft?

akarve commented at 11:27 PM on September 23, 2024:

No. I'll make a note of that and clarify the language (1.0 instead of legacy).

in bip-0085.mediawiki:20 in 41d15a7d5a outdated

  19 | -''One Key to find them,''
  20 | -''One Path to bring them all,''
  21 | -''And in cryptography bind them."''
  22 | +''One Seed to rule them all,''<br/>
  23 | +''One Key to find them,''<br/>
  24 | +''One Path to bring them all,''<br/>

jonatack commented at 7:47 PM on September 23, 2024:

For these lines, unless there's a specific reason not to

''One Path to bring them all,''<br>

jonatack commented at 7:50 PM on September 23, 2024: member

Did an editing pass. Murch might be travelling currently.

(This is a bit of a slog to review commit-by-commit, as many commits re-tweak the changes in the previous ones. I looked at each commit, then ended up looking at the overall diff.)

akarve force-pushed on Sep 24, 2024

BIP-85:

    * Add new maintainer (author unreachable)
    * Swap chain code and private key bytes in application 32' for consistentcy with BIP-32 (major change)
    * Correct derived entropy for application 128169' test vector (major change)
    * Clarify big endian serialization
    * Add the Portuguese language (9') to application 39'
    * Add dice application 89101'
    * Clarify Testnet support for XPRV application 32'
    * Minor grammar, format, clarity improvements

81cddf849c

akarve force-pushed on Sep 24, 2024

akarve requested review from jonatack on Sep 24, 2024

akarve commented at 2:49 AM on September 24, 2024: contributor

@jonatack Thank you. I turned around your feedback and rebased into a single commit for clarity.

Revert spelling error e027d0545a

Ensure numerical range renders properly with <code> 9179bec780

in bip-0085.mediawiki:105 in 9179bec780

  99 | @@ -93,14 +100,15 @@ OUTPUT
 100 |  
 101 |  ==Reference Implementation==
 102 |  
 103 | -* Python library implementation: [https://github.com/ethankosakovsky/bip85]
 104 | -* JavaScript library implementation: [https://github.com/hoganri/bip85-js]
 105 | +* 2.0 Python library implementation: [https://github.com/akarve/bipsea]
 106 | +* 1.0 Python library implementation: [https://github.com/ethankosakovsky/bip85]
 107 | +* 1.0 JavaScript library implementation: [https://github.com/hoganri/bip85-js]

jonatack commented at 2:46 PM on September 25, 2024:

A few ideas for a follow-up:

move this section to just above the change log, and/or more clearly mention that 2.0 and 1.0 refer to versions of this BIP

or

consider whether it makes sense to drop the 1.0 versions from this list

in bip-0085.mediawiki:447 in 9179bec780

 442 | +    * Correct derived entropy for application 128169' test vector (major change)
 443 | +    * Clarify big endian serialization
 444 | +    * Add the Portuguese language (9') to application 39'
 445 | +    * Add dice application 89101'
 446 | +    * Clarify Testnet support for XPRV application 32'
 447 | +    * Minor grammar, format, clarity improvements

jonatack commented at 2:49 PM on September 25, 2024:

For another pull, it may be good to standardize the various BIPs change logs on the same order (personally prefer to order by descending date, i.e. most recent first).

Edit: for what it's worth, descending date is also the recommended order in https://keepachangelog.com/en/1.1.0/

jonatack commented at 2:58 PM on September 25, 2024: member

ACK 9179bec780bae583047f84c436a4c3161570c239

jonatack merged this on Sep 25, 2024

jonatack closed this on Sep 25, 2024

jonatack removed the label PR Author action required on Sep 25, 2024

scgbckbone commented at 8:41 PM on October 4, 2024: contributor

what is this ? breaking changes after after 3 people talking for 4 months ?

I will be happy to hear from @ethankosakovsky but he hasn't been active on GH since 2021. I have no idea if there's a procedure for this, nevertheless I volunteer to become a maintainer of this BIP if the author cannot be reached.

BS #1344 (comment) (you'd just need to wait bit more, which you should if you're doing breaking changes, anyways)

nvk commented at 8:52 PM on October 4, 2024: none

NACK

achow101 commented at 9:24 PM on October 4, 2024: member

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. This should be well past the stage where breaking changes can be implemented. The breaking changes should have been discussed on the mailing list, and I think should have been in a new BIP which could supercede 85. Furthermore, the BIPs process currently has no way to distinguish BIP versions and the way that has been implemented here is confusing.

I think this PR should be reverted and the breaking changes should be first discussed on the mailing list and with the projects that have already implemented BIP 85.

I agree that the original author is unresponsive enough to justify a change of BIP champion, but I don't think that should mean the new champion can add any breaking changes that they wish.

scgbckbone commented at 9:36 PM on October 4, 2024: contributor

I agree that the original author is unresponsive enough to justify a change of BIP champion, but I don't think that should mean the new champion can add any breaking changes that they wish.

he's only unresponsive on GH, if BIP2 is followed and email sent, he'd resurface imo (as last time)

RandyMcMillan commented at 9:37 PM on October 4, 2024: contributor

NACK post merge

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

revert breaking changes PR https://github.com/bitcoin/bips/pull/1673

achow101 commented at 10:35 PM on October 4, 2024: member

he's only unresponsive on GH, if BIP2 is followed and email sent, he'd resurface imo (as last time)

#1600 (comment) and #1600 (comment) suggests that email was in fact tried, multiple times, and they were unreachable.

akarve commented at 10:50 PM on October 4, 2024: contributor

#1600 (comment) and #1600 (comment) suggests that email was in fact tried, multiple times, and they were unreachable.

Correct. Moreover an email was sent to the bitcoin dev list looking for him with no results. My understanding is that we followed BIP2 in this regard.

jonatack commented at 11:10 PM on October 4, 2024: member

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.

Will propose that in a week if not proposed by someone before.

Thank you everyone for the feedback.

jonatack commented at 11:22 PM on October 4, 2024: member

I've opened a full revert in #1674 to provide a clean slate. Any desirable changes from this pull can be proposed separately.

scgbckbone commented at 12:48 PM on October 5, 2024: contributor

@achow101 @akarve my bad, now I see that BIP2 was followed

Aldocapurro approved