novitski / bitcoinj

Automatically exported from code.google.com/p/bitcoinj
Apache License 2.0
0 stars 0 forks source link

Payment Protocol parser should verify that it uses the same network as the wallet #551

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There's no point showing the user the payment request if it's on a different 
network than the wallet, since they won't be able to send it. Right now 
PaymentSession doesn't verify this either on init or when sending, resulting in 
the coins being sent to a completely unexpected address (?!). If you use this 
tool https://bitcoincore.org/~gavin/createpaymentrequest.php to generate a 
payment request for testnet (with an "m..." address belonging to you), and then 
open it in a wallet running on mainnet, it will happily accept the payment 
request and send it to some seemingly random (and most probably non-existing) 
"1...." address...

I'm assuming this is PaymentSession's responsibility, since there's no getter 
through which I could access the params field to check it myself.

Original issue reported on code.google.com by jakub.su...@gmail.com on 30 Apr 2014 at 7:49

GoogleCodeExporter commented 9 years ago
Good catch. Resolved:

https://github.com/bitcoinj/bitcoinj/commit/c8ffc1eaee4f44d6665ddc87d28a91ea92ba
4858

Original comment by mh.in.en...@gmail.com on 30 Apr 2014 at 8:20

GoogleCodeExporter commented 9 years ago
FYI: I was planning a third part of the payment protocol refactoring that will 
take care of this (and more) issue of the current implementation.

Original comment by andreas....@gmail.com on 30 Apr 2014 at 9:13

GoogleCodeExporter commented 9 years ago
And what do you want to change? Does it really need more refactoring? I know 
you don't like using this class although I'm sure there's a way to make it fit 
on Android, like just by putting the whole binary payment request into the 
intent or something.

Original comment by mh.in.en...@gmail.com on 30 Apr 2014 at 9:30

GoogleCodeExporter commented 9 years ago
The idea of the last refactoring is pulling out three parse methods and three 
create methods (for request/payment/ack) into the PaymentProtocol class. If you 
look at the de.schildbach.wallet.util.PaymentProtocol class you'll get the 
picture.

Original comment by andreas....@gmail.com on 1 May 2014 at 6:16

GoogleCodeExporter commented 9 years ago
OK, sounds good.

Original comment by mh.in.en...@gmail.com on 1 May 2014 at 6:36