Thanks, Mike!

   "PaymentRequest messages larger than 50,000 bytes should be rejected by
> the merchant's server, to mitigate denial-of-service attacks."
>
> Do you mean "users wallet" here?
>

Yes, fixed.



> You could note in the motivation section two more motivations:
> 1) That the protocol can be a foundation on which other features are built
>

I don't like putting "this is what we think will happen in the future"
types of statements in specifications, so I'm inclined to leave that out.


> 2) That it is required to assist hardware wallets when there is a virus on
> the system
>

Added:

"Resistance from man-in-the-middle attacks that replace a merchant's
bitcoin address with an attacker's address before a transaction is
authorized with a hardware wallet."

Perhaps note in the BIP that the merchant should not assume the
> merchant_data field is trustworthy - malicious buyers could rewrite it as
> they see fit. Point out that a good way to use this is to serialize server
> state, signed by a merchant-only key, in the same way one might use an HTTP
> cookie.
>

Added:

"Note that malicious clients may modify the merchant_data, so should be
authenticated in some way (for example, signed with a merchant-only key)."


>    "PaymentDetails.payment_url must be secure against man-in-the-middle
> attacks that might alter Payment.refund_to (if using HTTP, it must be
> TLS-protected).
>
> This says "must", but what should a client do here if the payment URL is
> not HTTPS? I suggest weakening this to "should", as sometimes TLS is
> redundant (e.g. if you're sending to a Tor hidden service).
>

done.


> The PaymentACK message contains a copy of Payment, but the BIP doesn't say
> what to do with it. I assume this means a client is free to ignore it and
> rely on TCP state to figure out the payment/ack connection instead? It may
> be worth noting that explicitly.
>

Added:

"payment | Copy of the Payment message that triggered this PaymentACK.
Clients may ignore this if they implement another way of associating
Payments with PaymentACKs."


>
> In the certificates section, you could observe that "validation" means
> "verification that it correctly chains to a trusted root authority, where
> trusted roots may be obtained from the operating system. If there is no
> operating system, the Mozilla root store is recommended".
>

Modified that section to say:

"...followed by additional certificates, with each subsequent certificate
being the one used to certify the previous one, up to a trusted root
authority. The recipient must verify the certificate chain according to
[RFC5280] and reject the PaymentRequest if any validation failure occurs.

Trusted root certificates may be obtained from the operating system; if
validation is done on a device without an operating system, the Mozilla
root store<http://www.mozilla.org/projects/security/certs/included/index.html>
is
recommended."

-- 
--
Gavin Andresen