-
Notifications
You must be signed in to change notification settings - Fork 5.8k
BIP85: replace Base64 by Base85 in PWD BASE85 section #1939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jonatack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @akarve @scgbckbone @nvk @ethankosakovsky for feedback, this BIP is deployed by a few projects and is in final status (#1676), so no potentially breaking changes may be introduced.
|
@ethicnology in this spec the derived entropy, e.g. on both of the applications you commented on, is of fixed length. in what context would this be variable? there is trimming for the password length but that is explicitly post entropy. if you want truly variable password length and dictionaries (allows length of 1) maybe the new dice app is what you want? |
|
@jonatack this particular change i'm asking for clarification on; so far i don't see the need. as for backward compatibility we now have a semver log. is it your opinion that even if the community adopts a major semver change that it needs to backward compatible or that said major semver change belongs in a clean BIP? |
Hi @akarve, yes, see #1600 (comment). I agree that breaking changes (if any) should first be discussed on the mailing list and with the projects that have already implemented the BIP, particularly for a BIP with final status, then probably go into a new BIP that replaces the previous one. Semver would be for non-breaking changes. |
ok this is a topic for another day. i’ll just point out that the situation leading to the above reasoning has since been addressed:
|
Could take a look at BIP3 and propose any improvements there you have in mind. |
|
NACK it's always 64 bytes as it uses whole hmac-sha512 digest |
I understand, I was confused with the password length. I've amended my commit to replace only the Base64 by Base85 in the PWD BASE85 section |
|
@ethicnology could you also unify capitalization on: |
|
ACK 3e9befd |
jonatack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3e9befd
|
cc @akarve for feedback or sign-off |
|
Approved |
Replace
Base64byBase85encoder in the Pwd Base85 section, and use consistent capitalization.