Payment Protocol support

[mackuba]

https://code.google.com/p/bitcoinj/wiki/PaymentProtocol

TODO:

• [x] confirm and send payment requests through the existing send window
• [x] proper error handling
• [x] loading spinner while the request is being fetched
• [x] verify network type
• [x] verify request dates
• [x] verify request signature/identity
• [x] show additional memos
• [x] show ACK info
• [x] handle local payment protocol files
• [x] handle bitcoin URLs pointing to payment protocol resources
• [x] handle QR codes encoding request URLs directly
• [x] disable QR code button if dialog is locked
• [x] green padlock icon

[mikehearn]

It'd be a good idea to upgrade to 0.11.1 before doing that. There have been a bunch of fixes you'd want in that release and more queuing up.

[mackuba]

I'm gonna need some design advice...

I'm adding the memos/notes now. There are three kinds of memos in the payment protocol, one for each of the messages, all optional:

• the initial payment request from the recipient can include a note to the payer, probably explaining what they're paying for (e.g. a list of items from the shopping cart)
• the payment message from the payer can include a note from the payer to the recipient (the BIP doesn't specify what this is meant for, and there's no guarantee anyone will actually read it...)
• the ACK message back from the recipient can include a note confirming the order

I've added the first one for now, as a "details" field that can appear below the recipient (it's hidden if it's empty):

Now I'm wondering what to do with the other two. My current ideas:

• the second note is probably not that important, so we could just skip it, so that we don't add even more noise to this dialog (or is it required for a wallet to provide this field? @mikehearn)
• I'm thinking that for payments, we could make the dialog persist after confirmation instead of disappearing instantly as with normal sends, to make it more obvious that the payment has succeeded, and then replace the bottom part (with cancel/send) with some kind of happy-green bar with a checkmark that says "Payment successful" and optionally includes the ACK message

Other unrelated changes:

• the window title changes from "Send Bitcoin" to "Pay with Bitcoin" for payment requests
• I'm planning to hide the avatar and replace it with some kind of "green padlock" icon that will show up if the payment request is signed and verified, like in a web browser

Comments?

[mikehearn]

The PaymentACK memo is meant for people sending money to each other casually, tipping, donating etc really. It's probably not useful or might be actively unhelpful for payment processors and webshops. I think we might need a BIP70 extension to mark that PaymentACK memo's would actually get read. For now, hide it - other wallets don't support uploading a message either. It's a feature that seems half baked.

All the other design points look good to me.

[mackuba]

@mikehearn if a bitcoin: URL through which you get the payment request also includes label/message fields, is it ok to use the label as a fallback for the receiver's name (if the request is not verified) or the message as a fallback memo, or should these be completely ignored if r= is set?

[mikehearn]

Hm, good question. I think most wallets ignore the rest of the URI if the r= parameter is there. This feels like another place where the relevant BIP could be tightened up. I'm not sure what the best behaviour is.

[mackuba]

Whew, error handling is complex... and that's just the first part, now the error handling of payment sending...

[mikehearn]

Yeah, error handling is always a big chunk of code.

Sometimes I wonder if bitcoinj should provide pre-translated strings for common exceptions to save people the burden of doing this kind of thing over and over. Not sure. That looks like a good set of error messages so if you get translations for them I might steal them :)

[mackuba]

Sometimes I wonder if bitcoinj should provide pre-translated strings for common exceptions to save people the burden of doing this kind of thing over and over.

I don't think this is something that should be done at the level of the library, people might want to use different language depending on what the target audience of their wallet is... ("java.net.UnknownHostException: Invalid hostname" vs. "Have you tried to turn it off and on again?" :).

[mikehearn]

I guess most wallet apps will want the latter vs the former. Or, exception.getMessage vs getLocalisedMessage.

Basically, what I want is that wallet authors can concentrate on stuff that differentiates themselves vs just boring plumbing. And I'd like to see a thousand wallets because that's awesomely decentralised, even if they're based on the same core code (they can easily fork it). So reducing expensive and redundant work seems valuable.

[mackuba]

@mikehearn - two questions:

1) What should the wallet do if there is no payment_url?

2) I'm wondering about the case when I have a bad internet connection, I submit a payment to a merchant, and they receive the transaction, but the connection fails and the ACK doesn't get back to me. From their point of view, the payment was made, they broadcast the transaction and process my order. From my point of view, the payment wasn't made, since my wallet doesn't know if they've received my Payment message and it just told me "Payment could not be completed".

Is this something I should worry about? Do you think this can cause problems? (If the merchant broadcasts the transaction, my wallet should receive it in a moment anyway, right? Though it's possible that in the meantime I will try to send it again (creating a new transaction), but then hopefully the merchant will check some kind of order ID in the payment data and respond with an error because the order was already paid...)

[mikehearn]

1) Just broadcast the tx as normal. Payment is ignored. PaymentSession should handle this for you I thought? If not then we should fix it!

2) The wallet will see the transaction next time it connects to the internet. Though I note you can get this kind of razor-thin edge case with credit cards too (actually you get much worse problems like submitting orders twice and such).

[mackuba]

PaymentSession should handle this for you I thought? If not then we should fix it!

I don't think it does, or at least I don't know how to do it... There's sendPayment, but it calls getPayment and if it returns null it just gives up.

[mackuba]

@mikehearn so if session.sendPayment returns null, I should just take the Wallet.SendRequest that I tried to pass there and call wallet.sendCoins(peerGroup, request) like in the normal send?

[mikehearn]

Yeah. Not a very good API. We need to integrate PaymentSession and wallet more.

[mackuba]

Ok, added ticket here: https://code.google.com/p/bitcoinj/issues/detail?id=557

[mikehearn]

Woo! All boxes ticked!

[mackuba]

https://github.com/hivewallet/hive-osx/releases/tag/2014051401

[mikehearn]

It failed. I tried at the BitGive foundation:

http://bitgivefoundation.org/donate-now/

but got "Payment request file could not be loaded."

[mackuba]

Works for me, or at least it opens:

Try again or check the logs.

[mikehearn]

What version of OS X are you testing with and how did you install Java?

The problem is that the base Apple 1.6 JVM is broken:

waterford:Hive mike$ ls -alh /Library/Java/JavaVirtualMachines/1.6.0_39-b04-443.jdk/Contents/Home/lib/security/ total 24 drwxr-xr-x 10 root wheel 340B 28 Feb 2013 ./ drwxr-xr-x 42 root wheel 1.4K 28 Feb 2013 ../ -rw-r--r-- 1 root wheel 2.4K 28 Feb 2013 US_export_policy.jar lrwxr-xr-x 1 root wheel 79B 28 Feb 2013 blacklist -> /System/Library/Java/Support/Deploy.bundle/Contents/Home/lib/security/blacklist lrwxr-xr-x 1 root wheel 81B 28 Feb 2013 cacerts -> /System/Library/Java/Support/CoreDeploy.bundle/Contents/Home/lib/security/cacerts -rw-r--r-- 1 root wheel 3.4K 28 Feb 2013 java.policy -rw-r--r-- 1 root wheel 11K 28 Feb 2013 java.security -rw-r--r-- 1 root wheel 2.4K 28 Feb 2013 local_policy.jar -rw-r--r-- 1 root wheel 347B 28 Feb 2013 sunpkcs11-macosx.cfg lrwxr-xr-x 1 root wheel 87B 28 Feb 2013 trusted.libraries -> /System/Library/Java/Support/Deploy.bundle/Contents/Home/lib/security/trusted.libraries waterford:Hive mike$ waterford:Hive mike$ cat /System/Library/Java/Support/CoreDeploy.bundle/Contents/Home/lib/security/cacerts cat: /System/Library/Java/Support/CoreDeploy.bundle/Contents/Home/lib/security/cacerts: No such file or directory

so when it tries to download something via SSL:

DEBUG: Payment request load error: Error Domain=BitcoinKit Code=0 "The operation couldn’t be completed. javax.net.ssl.SSLException: java.lang.RuntimeException: Un expected error: java.security.InvalidAlgorithmParameterException: the trustAnchors parameter must be non-empty" UserInfo=0x7fb4886a1190 {NSLocalizedFailureReason= javax.net.ssl.SSLException: java.lang.RuntimeException: Unexpected error: java.security.InvalidAlgorithmParameterException: the trustAnchors parameter must be non -empty, stackTrace=at com.sun.net.ssl.internal.ssl.Alerts.getSSLException(Alerts.java:190) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1762) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1723) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.handleException(SSLSocketImpl.java:1706) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1237) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1214) at sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:434) at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:166) at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1172) at sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:234) at com.google.bitcoin.protocols.payments.PaymentSession$1.call(PaymentSession.java:192) at com.google.bitcoin.protocols.payments.PaymentSession$1.call(PaymentSession.java:186)

Given that bitcoinj already ships its own root store, we could probably work around this Apple bug quite easily on the bitcoinj side by just overriding the default trust store for HTTPS connections.

[mikehearn]

Ugh. OK, so I fixed it by installing the last update from Apple. Apparently that fixes Mavericks. However, now I see something even weirder.

Hive still cannot download via SSL:

DEBUG: Payment request load error: Error Domain=BitcoinKit Code=0 "The operation couldn’t be completed. javax.net.ssl.SSLKeyException: RSA premaster secret error" UserInfo=0x608000271a40 {NSLocalizedFailureReason=javax.net.ssl.SSLKeyException: RSA premaster secret error, stackTrace=at com.sun.net.ssl.internal.ssl.RSAClientKeyExchange.(RSAClientKeyExchange.java:99) at com.sun.net.ssl.internal.ssl.ClientHandshaker.serverHelloDone(ClientHandshaker.java:747) at com.sun.net.ssl.internal.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:241) at com.sun.net.ssl.internal.ssl.Handshaker.processLoop(Handshaker.java:593) at com.sun.net.ssl.internal.ssl.Handshaker.process_record(Handshaker.java:529) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:943) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1188) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1215) at com.sun.net.ssl.internal.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1199) [snip]

However, using exactly the same JVM from the command line, WalletTool can fetch it just peachy!? I wonder what the difference is.

The above exception looks like it wraps another one. Normally you'd get several stack traces from such a throw, but in the logs I see only one. You might want to use Throwables.getRootCause(e) on the exception before passing it back to BitcoinKit to ensure maximum detail. Otherwise it's hard to debug what the real problem is here.

Anyway. It seems Apple managed to screw up their JVM pretty badly in this respect, I see lots of complaints that the Mavericks "upgrade" broke things on stackoverflow etc. So the question is what to do about it. We can:

1) Use the shipped cacerts file to patch up the Apple JVM. 2) Don't use bitcoinj/Java to download the payment request file/upload the ack and do it all in Objective-C. This would be inconvenient so I'd rather figure out what's going wrong. Plus Apple's SSL stack is obviously not tested and had a nasty security hole recently. 3) Wait for me to get Hive working with Avian (one of my post-conference tasks)

[mackuba]

What version of OS X are you testing with and how did you install Java?

1.9.2, I don't remember how I got Java... I've got 1.6.0_65, but apparently I have nothing in /Library/Java/JavaVirtualMachines, my Java is in /System/Library/Frameworks/JavaVM.framework...

/System/Library/Java/Support/CoreDeploy.bundle/Contents/Home/lib/security/cacerts file exists.

Let's see if anyone else has this problem, it's possible that it's just your installation that's somehow broken and it will work for everyone else...

However, using exactly the same JVM from the command line, WalletTool can fetch it just peachy!? I wonder what the difference is.

Can you check what java.ext.dirs is set to on your machine? We're passing an option to the VM to exclude all of the directories that aren't in /System to avoid collisions with some custom software, that might potentially cause problems. (Actually, if you have everything in /Library/Java instead of /System, then it probably does.)

The above exception looks like it wraps another one. Normally you'd get several stack traces from such a throw, but in the logs I see only one. You might want to use Throwables.getRootCause(e) on the exception before passing it back to BitcoinKit to ensure maximum detail. Otherwise it's hard to debug what the real problem is here.

I'll take a look.

Don't use bitcoinj/Java to download the payment request file/upload the ack and do it all in Objective-C. This would be inconvenient so I'd rather figure out what's going wrong.

I could do that for downloading the request without a lot of effort, but sending the payment would be harder since I'd need to rewrite a large part of what PaymentSession does there...

[mikehearn]

Ah yes. It's probably the extdirs thing. I'll take a look at Avian soon.

[mackuba]

Just tell me what those extdirs are on your computer, it's possible that I can just tweak that filter.

[mikehearn]

With the system Java 6 I see:

/Library/Java/Extensions:/System/Library/Java/Extensions:/System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/lib/ext

(with wallet tool). Looks pretty normal. I don't think I have anything weird in those directories:

waterford:target mike$ ls /System/Library/Java/Extensions/ AppleScriptEngine.jar j3dcore.jar libJ3D.jnilib mlibwrapper_jai.jar MRJToolkit.jar j3dutils.jar libJ3DAudio.jnilib vecmath.jar QTJava.zip jai_codec.jar libJ3DUtils.jnilib dns_sd.jar jai_core.jar libQTJNative.jnilib j3daudio.jar libAppleScriptEngine.jnilib libmlib_jai.jnilib waterford:target mike$ ls /Library/Java/Extensions/ waterford:target mike$ ls /System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/lib/ext apple_provider.jar dnsns.jar localedata.jar sunjce_provider.jar sunpkcs11.jar waterford:target mike$

[mackuba]

Hmm, so nothing from /Library/Java in the extdirs except /Library/Java/Extensions? In that case that shouldn't affect anything... Do you have anything in /Library/Java/Extensions?

[mikehearn]

Nope it's empty. Not sure why it's dying, but I suspect one of the JARs in these directories is required for TLS to work - god knows why it's considered an "extension" but I see various crypto libs in the file listing above. So if extdirs is cleared perhaps it can't find the crypto libs anymore. From reading the code around that stack trace, I see dynamic loading of crypto code inside the try block, so that could be the case. Is there a way to stop Hive clearing the extdirs?

[mackuba]

But only the /Library extensions are excluded, the /System ones should still be loaded...

Relevant code: https://github.com/hivewallet/BitcoinKit/blob/master/BitcoinJKit/java/bitcoinkit/src/main/java/com/hivewallet/bitcoinkit/ExtensionPathsReader.java https://github.com/hivewallet/BitcoinKit/blob/master/BitcoinJKit/HIBitcoinManager.m#L504-L517 https://github.com/hivewallet/BitcoinKit/blob/master/BitcoinJKit/HIBitcoinManager.m#L583-L617

[mackuba]

The above exception looks like it wraps another one. Normally you'd get several stack traces from such a throw, but in the logs I see only one. You might want to use Throwables.getRootCause(e) on the exception before passing it back to BitcoinKit to ensure maximum detail. Otherwise it's hard to debug what the real problem is here.

Fixed here: https://github.com/hivewallet/BitcoinKit/commit/87f25526223fe921d4183d31428a7a094653a502

In this case, the root exception was:

java.security.NoSuchAlgorithmException: SunTlsRsaPremasterSecret KeyGenerator not available
at javax.crypto.KeyGenerator.<init>(DashoA13*..)
at javax.crypto.KeyGenerator.getInstance(DashoA13*..)
at com.sun.net.ssl.internal.ssl.JsseJce.getKeyGenerator(JsseJce.java:223)
...

I managed to reproduce this by setting java.ext.dirs to an empty string, so it definitely has something to do with that.

Can you tell me:

• what you get when you call: java -cp Hive.app/Contents/Frameworks/BitcoinJKit.framework/Resources/boot.jar ExtensionPathsReader
• if you don't see an error like "Can't read Java extension directory list" in the Hive log (~/Library/Logs/Hive) when loading the payment request?

[mikehearn]

I see:

/System/Library/Java/Extensions

and I don't see any messages with the words "list" in them in any Hive log file.

[mackuba]

Hm, this is weird. If you look into ExtensionPathsReader.java, you can see that it just prints java.ext.dirs and filters out those that don't start with "/System". Which means that if your java.ext.dirs is /Library/Java/Extensions:/System/Library/Java/Extensions:/System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/lib/ext, then ExtensionPathsReader should return /System/Library/Java/Extensions:/System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/lib/ext, but it returns only the first one.

Where is your java binary located? This method calls /usr/bin/java. Is it possible that you have more than one java binary and they use different configurations?

[mikehearn]

Oh you're right, sorry, my java defaults to 1.8.0. I ran it with the one in /Library/Java/Home

/System/Library/Java/Extensions:/System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/lib/ext

[mackuba]

So /usr/bin/java is 1.8 and 1.6 is in /Library/Java/Home/bin/java?

[mikehearn]

Apparently! Not sure how it works. Probably /usr/bin/java is some kind of wrapper.

[mackuba]

Try this one: https://github.com/hivewallet/hive-osx/releases/tag/2014052101 - I've changed it to use Java from Library instead of /usr/bin.

[mikehearn]

Yep now it works.

[mackuba]

@mikehearn could you check if this build works for you? https://github.com/hivewallet/hive-osx/releases/tag/2014052701

I had to change the path again from /Library/Java/Home/bin/java to /System/Library/Frameworks/JavaVM.framework/Versions/Current/Commands/java, because the former doesn't work for some users. (BTW, these two are two different binaries on my computer, even though they print exactly the same version...)

[mikehearn]

Doesn't work. Nothing is printed in the console. The mess Apple has made of Java on MacOS is amazing:

waterford:assets mike$ /System/Library/Frameworks/JavaVM.framework/Versions/CurrentJDK/Commands/java -version java version "1.6.0_65" Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462-11M4609) Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode) waterford:assets mike$ /System/Library/Frameworks/JavaVM.framework/Versions/Current/Commands/java -version java version "1.8.0" Java(TM) SE Runtime Environment (build 1.8.0-b132) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b70, mixed mode)

How hard is it to just throw the apple Java 1.6 framework into the Hive app bundle entirely? I know Avian is a lot smaller and I'm making progress with that, but the system JVMs seem to move around or be very unreliable.