BIP0070 limits and error handling #54

rnicoll commented at 4:12 PM on April 26, 2014: contributor

Changed example URL to be standard example domain. Added file size limits for PaymentRequest and PaymentACK. Added requirement to handle repeated sending of Payment message (i.e. to handle network failure).

Replaced example domain name with example.com. 22646636dd

Added file size limits for Payment and PaymentACK messages. 694314d296

Added note about handling multiple copies of a Payment message, to ensure resend is safe

in case of a transport layer failure.

d8bd74baf8

in bip-0070.mediawiki:None in d8bd74baf8 outdated

 155 | @@ -156,6 +156,11 @@ If the customer authorizes payment, then the Bitcoin client:
 156 |  # If PaymentDetails.payment_url is specified, POST a Payment message to that URL. The Payment message is serialized and sent as the body of the POST request.
 157 |  
 158 |  Errors communicating with the payment_url server should be communicated to the user.
 159 | +The merchant's server should handle receiving multiple copies of the same Payment
 160 | +message in response to a single PaymentRequest. This is required to ensure that in

leofidus commented at 11:18 PM on April 26, 2014:

I think this sentence is a bit too weak. "Handling it" is the equivalent of undefined behaviour in the C specification: it's perfectly on spec to crash or to format the hard drive. I would prefer at least "must/should handle receiving multiple copies ... gracefully".

in bip-0070.mediawiki:None in d8bd74baf8 outdated

 155 | @@ -156,6 +156,11 @@ If the customer authorizes payment, then the Bitcoin client:
 156 |  # If PaymentDetails.payment_url is specified, POST a Payment message to that URL. The Payment message is serialized and sent as the body of the POST request.
 157 |  
 158 |  Errors communicating with the payment_url server should be communicated to the user.
 159 | +The merchant's server should handle receiving multiple copies of the same Payment
 160 | +message in response to a single PaymentRequest. This is required to ensure that in
 161 | +case of a transport level failure during transmission, recovery is possible by
 162 | +re-sending the Payment message. The endpoint URL must remain valid for at least
 163 | +the same period of time as the original PaymentRequest.

leofidus commented at 11:21 PM on April 26, 2014:

Nitpick: why a must here if the previous one was a should? No point in keeping the url valid if multiple copies are not being handled; and what would "valid url" even mean in that case?

leofidus commented at 11:26 PM on April 26, 2014: none

ACK

Expanded and clarified wording on recovering from error states when sending payment to the merchant's server. 89050ce146

rnicoll commented at 11:45 PM on April 26, 2014: contributor

@leofidus I've tightened up wording to hopefully make intent clearer. I've also moved the URL validity section up to PaymentDetails, where the payment_url is actually specified, as it probably makes more sense there.

Loosened URL validity period on payment requests. 9d8e002071

in bip-0070.mediawiki:None in 89050ce146 outdated

  98 | @@ -99,6 +99,12 @@ about the merchant and a digital signature.
  99 |  | merchant_data || Arbitrary data that may be used by the merchant to identify the PaymentRequest. May be omitted if the merchant does not need to associate Payments with PaymentRequest or if they associate each PaymentRequest with a separate payment address.
 100 |  |}
 101 |  
 102 | +The payment_url specified in the PaymentDetails must remain valid at least until the PaymentDetails

gavinandresen commented at 4:27 PM on April 27, 2014:

Okey dokey-- "good luck with that". A specification cannot force a company to stay in business.

How about "should remain valid..."

in bip-0070.mediawiki:None in 89050ce146 outdated

 184 | @@ -172,6 +185,9 @@ determine whether or not the transactions satisfy conditions of
 185 |  payment. If and only if they do, if should broadcast the
 186 |  transaction(s) on the Bitcoin p2p network.
 187 |  
 188 | +Payment messages larger than 50,000 bytes should be rejected by

gavinandresen commented at 4:29 PM on April 27, 2014:

I'd weaken that to "may be rejected." The merchant might be perfectly happy to handle 1MB Payment messages.

leofidus commented at 4:48 PM on April 27, 2014:

Maybe "The server must process payment messages of a length of up to 50,000 bytes, larger messages may be rejected" is closer to the intended meaning?

rnicoll commented at 4:54 PM on April 27, 2014:

The concern would be that people tend to test against implementation not specification, so if they happen to test with a server that accepts 1MB files, and another is much stricter, problems could ensue. See the Content-Type header in HTTP as an example; in theory a client can indicate character set of a request body using a parameter on the Content-Type, however as early versions of IE didn't send the character set, many web applications were developed without taking this into account. By the time there was a push to use the character set parameter, it was generally considered that it would break too many implementations. This lead to this particularly terrible workaround: http://web.archive.org/web/20060427012511/http://ppewww.ph.gla.ac.uk/~flavell/charset/form-i18n.html

sipa commented at 5:00 PM on April 27, 2014:

Or more closely related to Bitcoin, see the mess that resulted from OpenSSL accepting more than just strictly standard signature and public key encodings.

There is a difference of course, in that here we can offer a specification that is stricter in what it is required to accept than implementations, but I still don't like it.

Clear specifications, that include rejecting input that is non conformant help set clear expectations.

in bip-0070.mediawiki:None in 89050ce146 outdated

 204 | @@ -189,6 +205,11 @@ Payment message:
 205 |  | memo || UTF-8 encoded note that should be displayed to the customer giving the status of the transaction (e.g. "Payment of 1 BTC for eleven tribbles accepted for processing.")
 206 |  |}
 207 |  
 208 | +PaymentACK messages larger than 60,000 bytes should be rejected by

gavinandresen commented at 4:32 PM on April 27, 2014:

"may" -- give clients and merchants the option to do more ("Here's an MPEG of the drone being dispatched to deliver your product!"). And I'd vote against justifications for every constant in the spec (commit messages are a better place for that, I think).

gavinandresen referenced this in commit 2baef89ae9 on May 20, 2014

gavinandresen merged this on May 20, 2014

gavinandresen closed this on May 20, 2014