bitcoin
BIP85: Clarify spec, correct PWD BASE64 test vector, add Portuguese language code
Summary of changes:
- Clarify drng_reader.read is a first-class function (not an evaluation)
- 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 DERIVED ENTROPY for PWD BASE64 test vector. See the following relevant test cases:
Unit tests and complete implementation that led to these suggestions is in akarve/bipsea.
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.
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.
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.
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.
@murchandamus Done. The mailing list topic is "BIP-85 Champion Unreachable? Please weigh in." in case you'd like to comment.
Yeah, I saw that. Could you please add yourself as a champion to the BIP in this PR?
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?
It may be useful to be consistent with bytes or bits in the document.
Preferably use bits throughout?
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.
@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.
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.
@murchandamus Change Log added. We should be good to go :) Thanks for your assistance in getting this over the line.
@jonatack Thank you. I turned around your feedback and rebased into a single commit for clarity.
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 https://github.com/bitcoin/bips/pull/1344#issuecomment-1420909645 (you'd just need to wait bit more, which you should if you're doing breaking changes, anyways)
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.
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)
he's only unresponsive on GH, if BIP2 is followed and email sent, he'd resurface imo (as last time)
https://github.com/bitcoin/bips/pull/1600#issuecomment-2150861263 and https://github.com/bitcoin/bips/pull/1600#issuecomment-2191817502 suggests that email was in fact tried, multiple times, and they were unreachable.
#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.
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.
I've opened a full revert in #1674 to provide a clean slate. Any desirable changes from this pull can be proposed separately.