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

pull akarve wants to merge 3 commits into bitcoin:master from akarve:master changing 2 files +87 −15
  1. 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

  2. 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.
  3. murchandamus added the label Proposed BIP modification on May 30, 2024
  4. murchandamus added the label Pending acceptance on May 30, 2024
  5. murchandamus commented at 4:49 pm on May 30, 2024: contributor
  6. 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.
  7. 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
  8. 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?

  9. 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.
  10. akarve commented at 10:13 pm on June 22, 2024: contributor
    @murchandamus Howdy. Any luck reaching Ethan?
  11. 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.

  12. 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.
  13. 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?
  14. 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?
  15. 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?

  16. 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
  17. 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.

  18. 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.

  19. 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:

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

    or:

    0  Author: Ethan Kosakovsky <ethankosakovsky@protonmail.com>
    1          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.
  20. 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

    0Uses 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

    0==Footnotes==
    1
    2<references />
    
  21. 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:
    0Use the last 32 bytes<ref name="curve-order" /> as the private key. Prepend an empty byte (<code>0x00</code>)
    
  22. 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.

  23. 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.

  24. akarve requested review from murchandamus on Jul 17, 2024
  25. 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.

    0Uses the most significant 32 bytes<ref name="curve-order">
    1There is a very small chance that you'll make an invalid
    
  26. 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?

  27. jonatack added the label PR Author action required on Jul 24, 2024
  28. 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.
  29. akarve requested review from murchandamus on Sep 22, 2024
  30. jonatack removed the label Pending acceptance on Sep 23, 2024
  31. 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:
    0When in doubt, assume '''big endian''' byte serialization, such that the leftmost
    
  32. 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.
  33. 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:
    0Use this application to generate PIN numbers or any other numeric secret.
    
  34. 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:
    0Roll values are zero-indexed, such that an N-sided die produces values in the range
    
  35. 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.
  36. 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).
  37. 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).
  38. 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

    0''One Path to bring them all,''<br>
    
  39. 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.)

  40. akarve force-pushed on Sep 24, 2024
  41. akarve force-pushed on Sep 24, 2024
  42. akarve force-pushed on Sep 24, 2024
  43. 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
  44. akarve force-pushed on Sep 24, 2024
  45. akarve requested review from jonatack on Sep 24, 2024
  46. 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.
  47. Revert spelling error e027d0545a
  48. Ensure numerical range renders properly with <code> 9179bec780
  49. 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
  50. 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/

  51. jonatack commented at 2:58 pm on September 25, 2024: member
    ACK 9179bec780bae583047f84c436a4c3161570c239
  52. jonatack merged this on Sep 25, 2024
  53. jonatack closed this on Sep 25, 2024

  54. jonatack removed the label PR Author action required on Sep 25, 2024
  55. 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)

  56. nvk commented at 8:52 pm on October 4, 2024: none
    NACK
  57. 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.

  58. 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)

  59. RandyMcMillan commented at 9:37 pm on October 4, 2024: contributor
    NACK post merge
  60. scgbckbone commented at 9:43 pm on October 4, 2024: contributor
    revert breaking changes PR https://github.com/bitcoin/bips/pull/1673
  61. 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.

  62. 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.

  63. 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.

  64. 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.
  65. scgbckbone commented at 12:48 pm on October 5, 2024: contributor
    @achow101 @akarve my bad, now I see that BIP2 was followed

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-21 09:10 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me