Skip to content
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

BIP38: remove broken links #1445

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

MarnixCroes
Copy link
Contributor

No description provided.


===Prefix===
It is proposed that the resulting Base58Check-encoded string start with a '6'. The number '6' is intended to represent, from the perspective of the user, "a private key that needs something else to be usable" - an umbrella definition that could be understood in the future to include keys participating in multisig transactions, and was chosen with deference to the existing prefix '5' most commonly observed in [[Wallet Import Format]] which denotes an unencrypted private key.
It is proposed that the resulting Base58Check-encoded string start with a '6'. The number '6' is intended to represent, from the perspective of the user, "a private key that needs something else to be usable" - an umbrella definition that could be understood in the future to include keys participating in multisig transactions, and was chosen with deference to the existing prefix '5' most commonly observed in [[https://en.bitcoin.it/wiki/Wallet_import_format|WIF]] which denotes an unencrypted private key.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest sticking to the previous literal, so that this doesn't fall out of being just administrivia.

Choose a reason for hiding this comment

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

Ok

@luke-jr
Copy link
Member

luke-jr commented Jun 29, 2023

@voisine

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

While the original links seem to have been all dead, adding targets to the link seems like a potential meaningful change in what the author may have wanted to express. I do not think that this should be merged without endorsement of the original BIP authors, even if that means that the links remain dead.

I do not consider the dead links a major issue, since the terms "secp256k1", "Base58Check", and "Wallet Import Format" are all easy research targets.

I recommend closing this PR after 2024-05-15 unless it has been endorsed by the BIP author (paging @voisine) by that date or further discussion indicates another outcome.

Copy link
Contributor

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

Tend to NACK, as elsewhere in this repository these terms are without a link, or in a couple of cases to a different link. It may make more sense to drop the links.

@murchandamus murchandamus added the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label May 8, 2024
@MarnixCroes
Copy link
Contributor Author

removed the dead links, as suggested here #1445 (review)
otherwise, feel free to close the PR

@jonatack jonatack changed the title BIP38: fix broken links BIP38: remove broken links May 22, 2024
Copy link
Contributor

@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, removes broken non-essential links with no change in meaning of the BIP.

@jonatack jonatack merged commit e2f7481 into bitcoin:master May 22, 2024
3 checks passed
@MarnixCroes MarnixCroes deleted the bip38-fix-links branch May 22, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified Proposed BIP modification
Projects
None yet
6 participants