Reimplement click-to-pay. Support OSX. #2299

pull gavinandresen wants to merge 1 commits into bitcoin:master from gavinandresen:localsocketuri changing 7 files +272 −204

gavinandresen commented at 12:09 AM on February 12, 2013: contributor

Switch to using Qt's QLocalServer/QLocalSocket to handle bitcoin payment links (bitcoin:... URIs)

Reason for switch: the boost::interprocess mechanism seemed flaky, and doesn't mesh as well with "The Qt Way". The new code is shorter and, I think, clearer.

I made a couple of extraneous changes (alphabetized some long lists of #includes to make it easier for me to see what new includes I needed to add).

I also added OSX support.

Submitting now for early review, should not be pulled until after 0.8.

Test plan:

On all supported platforms: Click on a link when Bitcoin-Qt is not running. Click on a link when Bitcoin-Qt is already running. Un-gracefully shutdown (kill) Bitcoin-Qt (so local socket file is left behind), then re-run.
gmaxwell commented at 12:19 AM on February 12, 2013: contributor

Suggested additional test: Click multiple links in rapid succession, does it break, can the user accidentally confirm the wrong payment?
in src/qt/bitcoingui.cpp:None in 178c7c94c0 outdated
```
 954 | @@ -949,3 +955,51 @@ void BitcoinGUI::toggleHidden()
 955 |  {
 956 |      showNormalIfMinimized(true);
 957 |  }
 958 | +
 959 | +void BitcoinGUI::handleCommandLineURIs()
```
laanwj commented at 6:01 AM on February 12, 2013:

ACK on the code changes. My only comment is that this server code shouldn't be in bitcoingui.cpp, I think it would be better to create a new class for it in qtipcserver.cpp which is neigh-empty now.

Diapolo commented at 6:39 AM on February 12, 2013:

ACK on wrong place, perhaps we should remove qtipcserver.cpp and creatre a new qturihandler.cpp (or sth. like that)?
laanwj commented at 6:59 AM on February 12, 2013: member

This needs some testing, but it is absolutely the right way forward.
in src/qt/qtipcserver.cpp:None in 178c7c94c0 outdated
```
 121 | -            break;
 122 | -    }
 123 | +    // Append a simple hash of the datadir
 124 | +    // Note that GetDataDir(true) returns a different path
 125 | +    // for -testnet versus main net
 126 | +    QString ddir(GetDataDir(true).string().c_str());
```
Diapolo commented at 7:03 AM on February 12, 2013:

If we are talking about the Qt way ^^, why not do:

<pre> QString name("BitcoinQt"); QString ddir(QString::fromStdString(GetDataDir(true).string())); name.append(QString::number(qHash(ddir))); </pre>
gavinandresen commented at 2:25 PM on February 12, 2013: contributor

I'll make the server its own class. And I'll use QString::number (I missed that method reading the QString docs).

I should have this working on the Mac, too soon.

Reimplement click-to-pay links. Add OSX support.

Switch to using Qt's QLocalServer/QLocalSocket to handle bitcoin
payment links (bitcoin:... URIs)

Reason for switch: the boost::interprocess mechanism seemed flaky,
and doesn't mesh as well with "The Qt Way"

qtipcserver.cpp/h is replaced by paymentserver.cpp/h

Click-to-pay now also works on OSX, with a custom Info.plist
that registers Bitcoin-Qt as a handler for bitcoin: URLs and
an event listener on the main QApplication that handles
QFileOpenEvents (Qt translates 'url clicked' AppleEvents into
QFileOpenEvents automagically).

8269a0953e

BitcoinPullTester commented at 9:24 PM on February 12, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/8269a0953ee9ccbdc422433fc37184e60f94b178 for binaries and test log.
gavinandresen commented at 12:04 AM on February 14, 2013: contributor

Updated / rebased. I'm happy with this code now.
laanwj commented at 7:47 AM on February 24, 2013: member

ACK
laanwj referenced this in commit 7ea7c94cf9 on Feb 24, 2013
laanwj merged this on Feb 24, 2013
laanwj closed this on Feb 24, 2013
laudney referenced this in commit 991460b8eb on Mar 19, 2014
DrahtBot locked this on Sep 8, 2021

Contributors