Skip to content

Conversation

@ethicnology
Copy link
Contributor

@ethicnology ethicnology commented Aug 20, 2025

Replace Base64 by Base85 encoder in the Pwd Base85 section, and use consistent capitalization.

Copy link
Member

@jonatack jonatack left a 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.

@akarve
Copy link
Contributor

akarve commented Aug 20, 2025

@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?

@akarve
Copy link
Contributor

akarve commented Aug 20, 2025

@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?

@jonatack
Copy link
Member

jonatack commented Aug 20, 2025

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.

@akarve
Copy link
Contributor

akarve commented Aug 20, 2025

Hi @akarve, yes, see #1600 (comment).

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:

Furthermore, the BIPs process currently has no way to distinguish BIP versions and the way that has been implemented here is confusing.

@jonatack
Copy link
Member

Furthermore, the BIPs process currently has no way to distinguish BIP versions and the way that has been implemented here is confusing.

Could take a look at BIP3 and propose any improvements there you have in mind.

@scgbckbone
Copy link
Contributor

NACK

it's always 64 bytes as it uses whole hmac-sha512 digest

@ethicnology
Copy link
Contributor Author

ethicnology commented Aug 21, 2025

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 ethicnology changed the title BIP85: remove hardcoded byte count since the length is variable and corrects Base64/Base85 encoding references BIP85: replace Base64 by Base85 encoding in PWD BASE85 section Aug 21, 2025
@scgbckbone
Copy link
Contributor

@ethicnology could you also unify capitalization on: base64 -> Base64 & base85 -> Base85

@scgbckbone
Copy link
Contributor

ACK 3e9befd

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3e9befd

@jonatack
Copy link
Member

cc @akarve for feedback or sign-off

@akarve
Copy link
Contributor

akarve commented Aug 21, 2025

Approved

@jonatack jonatack changed the title BIP85: replace Base64 by Base85 encoding in PWD BASE85 section BIP85: replace Base64 by Base85 in PWD BASE85 section Aug 21, 2025
@jonatack jonatack merged commit 3f2a403 into bitcoin:master Aug 21, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants