-
Notifications
You must be signed in to change notification settings - Fork 5.7k
BIP21: clarify that example addresses are intentionally invalid #1861
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
cc BIP author @TheBlueMatt for feedback |
This may have been on purpose, given that it is examples and not test vectors, because in the past people have sent money to addresses published in BIPs or Bitcoin books which were no longer held by anyone or for which it was then unclear where the funds should be sent. |
I think it's ok here to have invalid addresses on purpose, but maybe that should be mentioned in text that these example addresses aren't real, I also have spent some extra time in past with tests because of this. |
@murchandamus @kristapsk good points; this has come up occasionally in Bitcoin Core as well, where the developer notes state: - Use *invalid* bech32 addresses (e.g. in the constant array `EXAMPLE_ADDRESS`) for
`RPCExamples` help documentation.
- *Rationale*: Prevent accidental transactions by users and encourage the use
of bech32 addresses by default. |
I can replace legacy addresses to encourage the use of bech32. Why should we prevent accidental transactions rather than providing valid examples ? EDIT: Using valid regtest/testnet/signet addresses could solve both issues |
I'd suggest updating this as @kristapsk suggested. |
Amended with |
rename commit from |
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.
BIP 21 is Final and was specified with use of base58 addresses as it predates segwit. While some people have more recently also applied the described scheme to native segwit addresses, these examples do not conform to the specification of this proposal. I don’t think changing the examples to bech32 addresses makes sense in this context.
Recently, BIP 321 was proposed as a replacement for BIP 21. Perhaps what you were looking for is part of BIP 321?
Initially, I came to report that the addresses in the examples were invalid, and that this could lead some developers, like myself or kristapsk, to waste time debugging and questioning our integration, when in fact the issue came from the example itself being invalid Following the creation of this issue, it appears that the invalid addresses weren’t random but intentionally used to prevent accidental transactions. I suggested replacing the invalid addresses with ones from test networks like testnet, signet, or regtest, in order to keep the example valid while still avoiding accidental transactions. However, it doesn't seems to be convincing, so I've amended my commit with an invalid bech32 address from If the BIP is final and we shouldn't change anything then we can close my PR |
Hey @ethicnology, I touched base with @TheBlueMatt. The addresses were indeed invalid on purpose. So, I would also suggest what @kristapsk proposed: to add a line that informs readers that the addresses are invalid on purpose. If you would like to take a stab at that, please edit this branch, otherwise, please feel free to close it and probably @jonatack, myself, or someone else will open a PR to amend BIP 21 to add the warning. |
@murchandamus done in fad325e |
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.
LGTM, thanks!
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 fad325e
Updated the PR title and description.
Clarify to readers that the addresses used in the examples are intentionally invalid to prevent accidental transactions.