Fix BIP 21 and BIP 72 ambiguities & mistakes #30
pull swansontec wants to merge 6 commits into bitcoin:master from swansontec:master changing 2 files +23 −12-
swansontec commented at 2:12 am on March 7, 2014: contributorAs mentioned in the bitcoin-development mailing list, I have discovered some ambiguities and mistakes while writing my own bip-21 parser. This pull request fixes the various issues.
-
BIP 21: Add a missing "&" rule to the ABNF grammar
The URI syntax uses an ampersand to separate query parameters, but the ABNF grammar didn't reflect this fact. Also, the type of grammar used here is "ABNF" and not "BNF".
-
BIP 21: Mention RFC 3986 and character set 0ee7ff7bed
-
BIP 21: Precisely define the valid query characters c823eeb596
-
BIP 72: Reduce the amount of escaping needed
RFC 3986 obsoletes RFC 1738, which this BIP was wrongly referencing. The new RFC requires far less escaping for query parameters.
-
schildbach commented at 7:50 am on March 7, 2014: contributorIs the ABNF grammar case sensitive? Because RFC 3986 is, afaik. This is relevant e.g. for the “bitcoin:” scheme, which could also be written as “BITCOIN:” or “BiTcOiN:”
-
in bip-0021.mediawiki: in 820736896e outdated
38 (See also [[#Simpler syntax|a simpler representation of syntax]]) 39 40 bitcoinurn = "bitcoin:" bitcoinaddress [ "?" bitcoinparams ] 41 bitcoinaddress = base58 *base58 42- bitcoinparams = *bitcoinparam 43+ bitcoinparams = bitcoinparam [ "&" bitcoinparams ]
sarchar commented at 8:08 am on March 7, 2014:I know it’s pedantic, but RFC 3986 seems to imply that the query string can be empty, even if “?” is present. Thus,
0bitcoin:1JwSSubhmg6iPtRjtyqhUYYH7bZg3Lfy1T?
is a valid URI, according to RFC 3986, but would not match this grammar.
swansontec commented at 4:51 pm on March 7, 2014:To fix this, we could add a second set of square brackets to
bitcoinurn
, like so:0bitcoinurn = "bitcoin:" bitcoinaddress [ "?" [bitcoinparams] ]
swansontec commented at 7:55 pm on March 7, 2014:Also, the RFC only defines the general syntax of a URI, and the individual schemes are all a subset of that, each with their own restrictions. Therefore, it is perfectly valid for us to make an empty query string illegal in our scheme. It might be worthwhile to see what the reference client does with URI’s like this, and just match that behaviour.
laanwj commented at 7:06 am on March 8, 2014:We accept empty query strings so justbitcoin:address?
. Also the address can be empty in case of a payment request i.e.bitcoin:?r=...
. But that’s a different BIP.swansontec commented at 4:55 pm on March 7, 2014: contributor@schildbach You are right, the scheme is case-insensitive, although the RFC strongly advises everyone to write it in lower case. On the other hand, we have to accept things like “BiTcOiN:”. I will add another patch to fix this.schildbach commented at 5:01 pm on March 7, 2014: contributorWhile you’re at it, parameter names are case insensitive as well afaik.laanwj commented at 5:04 pm on March 7, 2014: memberThe URI scheme identifier is case insensitive, however we only accept parameter names in lower-case.schildbach commented at 5:10 pm on March 7, 2014: contributor@laanwj I’m talking about RFC 3986 rather than current impls.laanwj commented at 5:20 pm on March 7, 2014: memberI don’t think RFC 3986 says anything about the interpretation/comparison of parameter names. (see for example 6.2.2.1: The other generic syntax components [apart from scheme and host] are assumed to be case-sensitive unless specifically defined otherwise by the scheme)swansontec commented at 0:55 am on March 11, 2014: contributor@schildbach Ok, I have researched this a bit more, and yes, ABNF is case-insensitive. So, this means that both the scheme (good) and query parameter names (bad) are case-insensitive. The only way to make the query parameter names case-sensitive is to encode their ASCII values, like so:
0amountparam = %x61.6d.6f.75.6e.74 "=" *digit [ "." *digit ] 1labelparam = %x6c.61.62.65.6c "=" *qchar 2messageparam = %x6d.65.73.73.61.67.65 "=" *qchar 3reqparam = %x72.65.71 "-" qchar *qchar "=" *qchar
This is ugly, but conforms to what bitcoin-qt is doing.
swansontec commented at 1:26 am on March 11, 2014: contributorNow that the grammar is free of syntax errors, it is possible to actually run it. There is an interactive ABNF tester at [http://coasttocoastresearch.com/interactiveapg]. The grammar from the BIP doesn’t define “digit”, “base58”, or “qchar”, so the following temporary definitions are still needed to make it work:
0digit = %x30-39 1base58 = %x31-39 / %x41-48 / %x4A-4E / %x50-5A / %x61-6B / %x6D-7A 2qchar = digit / %x41-5A / %x61-7A
laanwj commented at 7:33 am on March 13, 2014: memberAdding raw hex in the grammar description doesn’t make it easier to read for humans, which are the prime audience here. Maybe just add a mention that the parameter names are case sentitive.BIP 21: Clarify case-sensitivity
The "bitcoin:" URI scheme needs to be case-insensitive according to the RFC, but the query parameters should be case-sensitive according to the bitcoin-qt client implementation.
BIP 21: Allow empty addresses and query parameters
Empty addresses should be valid for compatibility with BIP 72. Empty query parameters should also be valid, since they are harmless, and clients already accept them. With these changes, bitcoin:?& would be a valid (but useless) URI.
swansontec commented at 8:01 am on March 13, 2014: contributorYes, @laanwj, I agree that this is harder to read. I have rebased the pull-request, reverting the change to the ABNF grammar and inserting a note instead.laanwj commented at 12:13 pm on March 13, 2014: memberACKswansontec commented at 3:50 pm on March 18, 2014: contributorThere haven’t been any comments in a while. Perhaps this is ready to merge?gavinandresen commented at 3:59 pm on March 18, 2014: contributorACKlaanwj referenced this in commit a3fe3270ac on Mar 18, 2014laanwj merged this on Mar 18, 2014laanwj closed this on Mar 18, 2014
real-or-random referenced this in commit 3a4f1688a6 on Aug 10, 2022guggero referenced this in commit 309042716c on Sep 29, 2022
This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 03:10 UTC
More mirrored repositories can be found on mirror.b10c.me