Replace invalid example address with bip20 address#119
Replace invalid example address with bip20 address#119evoskuil wants to merge 4 commits intobitcoin:masterfrom
Conversation
|
Paging @TheBlueMatt as author. My two cents: There's nothing about address validation in the document. Performing the validation in the document should work fine with a silly example address. It may even be good to use a bad address, for the same reason that RFCs generally use unroutable IP ranges for examples, namely that people don't accidentally end up using it and (say) flooding someone or in this case sending them coins. |
|
The document specifies that the value in question be a bitcoin address. While the example is not normative it should at least be representative. It is not a bitcoin address and therefore might as well be anything, making the example fairly pointless, if not counterproductive. |
|
I dont care either way (the address in question was deliberately bad - it was the one on the wikipedia Bitcoin page, hence the last char was replaced with a W). |
|
I ran across this issue in using the BIP-21 examples as test vectors and fodder for bx uri-decode documentation. I've done this in a good number of cases and this is the first time I've encountered an invalid example. There are quite a few valid addresses out there that people can accidentally spend money to. If the intent is to provide a guard against accidental spend then I would at least warn people that the address is invalid so that they don't waste time chasing non-existent bugs. BTW, the comment on my commit should have said that the replacement address was from the BIP-20 example (vs. BIP-21). |
evoskuil
changed the title
|
FWIW, from time to time, a user approaches me with a bug: Bitcoin Wallet does not parse certain bitcoin URIs. It turned out they were trying to use the malformed URIs from this document. |
|
If you're changing it to a valid address, I'd recomment using a testnet address. Accidentally sending coins is a lot less serious there. |
|
Maybe change the label to something else too, if it isn't going to be my address. :p |
|
There doesn't seem to be author consensus that changing this is an improvement, so I'm closing this pull. |
At cost of a few extra bytes between peers, this avoids the whole "oops, we were on a chain fork" problem, and simplifies generation of temporary channel-ids (just pick a random one). Now we move the announcement_signature exchange to at least 6 confirms, which makes re-xmit tricky; I resolved that by insisting on reconnect that we send if we haven't received, and reply to the first one. The term "channel shortid" wasn't used anywhere, so I removed it; it's now a gossip-only thing anyway. One subtle change: pkt_error on unknown channels is now "MUST ignore"; this section was reworked anyway, and we'll want this if the bitcoin#120 goes through, where one side might have forgotten unformed channels). Closes: bitcoin#114 Suggested-by: Olaoluwa Osuntokun <laolu32@gmail.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> * FIXUP! Two bytes for funding-output-index. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> * FIXUP! Channel-id rework, temp ids, 32 bits only. Re-add the idea of temporary channel ids: far simpler since they're now big enough we can just fill with noise. Remove the alignment issues by combining txid and outnum using XOR; we could reduce to 128 bit if we really wanted to, but we don't. Error handling is now simple again, but while editing I changed the behaviour for unknown channels to MUST ignore (this is important for Change the 8-byte gossip channel id to `short-channel-id`. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> * FIXUP! Minor text tweaks from Pierre-Marie and Christian Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
5 participants
175tWpb8K1S7NmH4Zx6rewF9WQrcZv245Whas an invalid checksum so these example URIs fail on validating parsers