search

bitpay / jsonPaymentProtocol

JSON Payment Protocol Interface
47 stars 57 forks source link

Bitpay error message uses inconsistent units #5

Closed zacleids closed 6 years ago

zacleids commented 6 years ago

While testing the json payment protocol, I purposely crafted a transaction 1000 satoshi less than the requested amount, to see the error message returned by bitpay. The error message returned was Unhandled rejection Error: The amount on the transaction (0.000252 BTC) does not match the amount requested (26200 BTC). This payment will not be accepted. The second value should be 0.000262 BTC.

unusualbob commented 6 years ago

This is server side, but will be fixed in the next major payment-protocol deploy.

zacleids commented 6 years ago

I know its server side, the error is just being printed, but I wanted to bring attention to it in case no one had caught it yet.

unusualbob commented 6 years ago

Yep, we hadn't caught it yet. I've already made a patch for it since it was just a math error, just wanted you to know it will be packaged with some larger changes so may take a bit before it's deployed. Thanks for the report :)

zacleids commented 6 years ago

Thanks, while I have you here, I wanted to ask a clarification question about the protocol. In bip70, there was a flow where the server could return unspents amounts that summed to zero, and the wallet would then ask the client to enter an amount, and send that to one or all of the unspents. Is that flow also supposed to work in the jsonPaymentProtocol, or is the outputs object always supposed to contain outputs with non-zero amounts. The specification didn't say.

unusualbob commented 6 years ago

In our current server side implementation we'll never have a case of zero outputs. However, that doesn't mean it shouldn't be supported. Right now if you already have logic that does that for BIP70 I'd say its fine to keep that for JPP. If you're writing new logic entirely then honestly its up to you if you want to include that right now as I'd consider that undocumented functionality.

zacleids commented 6 years ago

Ok, thanks :)

unusualbob commented 6 years ago

This was fixed back in April but it seems we didn't update this issue, fixed :)