Full URL Support in bitcoin-qt

TheBlueMatt commented at 3:53 pm on October 26, 2011: member

This adds support for opening bitcoin-qt to handle a URL instead of just drag-and-drop URL support (which has been available in bitcoin-qt since before merge). Again it does not support OSX as you have to add OSX-specific URL handling and I don’t have access to any OSX boxes to code that. This does add hooks to add Bitcoin as the default bitcoin: opener for Win32 in the nsis installer, also the bitcoin-qt packages in the bitcoin/bitcoin ppa on launchpad install gnome hooks for the same.

Details: If bitcoin is already running, the launch of the second process will use boost/interprocess/ipc/message_queue to send the URL to the running process which will handle it. Otherwise the new process will launch to the send coins window. Because of this, if bitcoin crashes, links will have to be clicked 3 times before bitcoin will open.

in src/qt/bitcoin.cpp: in 310cf9f211 outdated

130@@ -112,6 +131,25 @@ std::string _(const char* psz)
131 
132 int main(int argc, char *argv[])
133 {
134+    // Do this early as we don't want to boter initializing if we are just calling IPC

gavinandresen commented at 4:21 pm on October 26, 2011:

typo nit: boter –> bother

laanwj commented at 4:27 pm on October 26, 2011: member

Great, an direct URL handler would certainly be useful

Some comments:

qtipcserver: I think it would be preferable to implement this entire feature in src/qt, without touching the core
ThreadSafeHandleURL: Does it need to block? Otherwise, you could shorten the function by leaving out the DirectConnection/BlockedQueuedConnection and simply use QueuedConnection to send signals asynchronously
Security: any browser<->bitcoin UI binding scares me a bit. Can javascript applications DDOS bitcoin by sending masses of URLs? (or sneak in an extra payment at unexpected times). Maybe we need to add a dialog box for confirmation.

in src/qt/bitcoin.cpp: in 310cf9f211 outdated

136+    {
137+        if (strlen(argv[i]) > 7 && strncasecmp(argv[i], "bitcoin:", 8) == 0)
138+        {
139+            const char *strURL = argv[i];
140+            try {
141+                boost::interprocess::message_queue mq(boost::interprocess::open_only, "BitcoinURL");

gavinandresen commented at 4:39 pm on October 26, 2011:

This is the bitcoin-is-already-running case? And the code at line 222 is the bitcoin-wasn’t-running-so-we’ll-start-it case?

TheBlueMatt commented at 4:50 pm on October 26, 2011:

Correct.

TheBlueMatt commented at 4:50 pm on October 26, 2011: member

qtipcserver: Yea, I thought about doing that but I didnt feel like shuffling even more code around instead of just porting what I already had…Ill move the files to qt/ at least. ThreadSafeHandleURL: I would prefer it did, seems like it would be less of a DDoS target if it limited the number of queued objects to at least what boost will allow (2 in this case) and then limit to what Qt will be able to handle JS DDos: probably a good idea to test and see how high one can get Bitcoin’s CPU usage to go (also limit the number of queued payment boxes in Bitcoin’s ui?). In terms of adding yet another dialog box, I dont think its necessary, Bitcoin will already confirm before sending.

laanwj commented at 4:56 pm on October 26, 2011: member

I’d prefer to have a dialog box before sending it to the running bitcoin process… I don’t like it queuing payment boxes at all without the user’s permission (Limiting the number of those would fight the symptoms, not the problem). Not all users may check the final send dialog very well before sending, so a nefarious background javascript could add a payment just before send (by somehow detecting user activity).

That’s why I went with drag/drop initially. There is (afaik) no way for a web application to fake that.

TheBlueMatt commented at 5:31 pm on October 26, 2011: member

Limiting the number of send boxes in the Bitcoin window was attempting to solve the DDoS issue by preventing a website from loading infinite send boxes. Having a popup before sending the send command to the running bitcoin process means you can now DDoS users by launching new processes all over the place, which is much worse.

The limiting of the number of send boxes also solves (IMO) the background payment send. If the user did have 2 send boxes and all of a sudden 3 appear right after they hit send (and before they click “Yes, Im sure”) then they won’t click “Yes, Im sure”)

laanwj commented at 5:39 pm on October 26, 2011: member

That’s not my point. DDoS was only one possibility but not the most dangerous one, I would be really careful. I agree with limiting the number of send boxes, but that’s not enough IMO to prevent issues with this.

Maybe a dialog box is also not the best solution. But I really don’t want an external processes, especially not an untrusted one in the browser, to have access to the list of send instructions without explicit manual user action.

TheBlueMatt commented at 5:56 pm on October 26, 2011: member

DoS is a completely separate issue from trust issues here. DoS: yea, should be tested a bit more just to see what kind of load you can generate from a browser.

Trust: I really fail to see the problem here. If a user clicks on a link (or one is otherwise opened), they still have to see the send screen AND click “Yes, Im sure” before a send can happen, I really don’t think adding more is going to do anything nor do I think there is a problem.

laanwj commented at 6:03 pm on October 26, 2011: member

The problem is that Javascript can easily simulate link clicks. If it was limited to explicit clicks by the user then I wouldn’t be having issues with it.

Imagine that someone hacks a forum and adds a little script that randomly adds send commands in the background. Sure, most users might notice it in time and cancel it, but there will always be a few users that accept (for example, those that are new to the program). As money is involved here, there is just too much incentive to pull those kind of tricks. People will be trying to hack this from all angles.

TheBlueMatt commented at 6:26 pm on October 26, 2011: member

Fixed the DoS issue (now only one link/sec can be opened). Also moved the files to qt/ and didn’t touch init.cpp

TheBlueMatt commented at 6:28 pm on October 26, 2011: member

If the confirm dialog window is open, this shouldn’t allow new sends to be added to the window, or the send (currently it does window, can’t test send atm). Also, if a new send is added, the window should be automatically scrolled to the bottom (the new send). With those in place I really see no problem here, if users want to be stupid, there is nothing we can do to stop them.

TheBlueMatt commented at 5:03 am on December 7, 2011: member

In an attempt at making this slightly more useful, I added the latest commit to automatically focus on new SendCoinEntrys and scroll to them, however because the scrolling is called before the qt thread is free to recalculate the new frame size, it scrolls to n-1 SendCoinEntrys not the last one. I’m assuming there is a simple way to do this, but I dont feel like spending a ton of time digging through qt docs to figure it out when I’m assuming @laanwj probably knows the solution, so how would one scroll to the end directly after adding a new SendCoinEntry?

laanwj commented at 5:55 pm on December 7, 2011: member

I haven’t tested it, but this German post seems to address the problem, by adding qApp->processEvents(): between adding the widget and scrolling;

http://qtforum.de/forum/viewtopic.php?p=30842#30842

In this case it’d be

0QCoreApplication::instance()->processEvents();

TheBlueMatt commented at 9:54 pm on December 7, 2011: member

OK, I think Ive addressed every issue with this, except for the question of whether to popup before adding the recipient to the transaction or not until the send confirm dialog. I am of the opinion that there shouldnt be an extra popup before adding as it will annoy users if you can create a desktop popup once per second (or any other time frame) from js. I would instead just say that since the sendcoinsentry scrolls to the bottom when new recipients are added, it can be safely left up to the user to check the send (especially since the list of recipients is repeated in the confirmation dialog anyway).

luke-jr commented at 3:19 pm on December 23, 2011: member

This needs rebasing.

TheBlueMatt commented at 4:27 am on December 24, 2011: member

rebased

gavinandresen commented at 2:10 am on December 29, 2011: contributor

Has anybody tested this besides Matt?

luke-jr commented at 2:55 am on December 29, 2011: member

I’ve tested this in combination with non-BTC unit support, via CLI (ie, bitcoin-qt bitcoin:foo).

Automatically refocus on new SendCoinsEntrys and scroll to them. 9a93c4c024

Add support for opening bitcoin: URIs directly. 7d145a0f59

justmoon commented at 12:04 pm on January 6, 2012: contributor

Built this and tested a couple cases from the wiki page on the command line:

0moon@clymene:/atlas/prj/bitcoin$ ./bitcoin-qt bitcoin:1NS17iag9jJgTHD1VXjvLCEnZuQ3rJED9L
1moon@clymene:/atlas/prj/bitcoin$ ./bitcoin-qt "bitcoin:1NS17iag9jJgTHD1VXjvLCEnZuQ3rJED9L?amount=20.3&label=Luke-Jr"
2moon@clymene:/atlas/prj/bitcoin$ ./bitcoin-qt "bitcoin:1NS17iag9jJgTHD1VXjvLCEnZuQ3rJED9L?amount=1"
3moon@clymene:/atlas/prj/bitcoin$ ./bitcoin-qt "bitcoin:1NS17iag9jJgTHD1VXjvLCEnZuQ3rJED9L?amount=.0000001"

Worked perfectly. :)

TheBlueMatt commented at 5:20 pm on January 6, 2012: member

Gmaxwell suggested that the send coins confirmation dialog delay several seconds before it allows the user to click OK, similar to the way firefox does plugin installation. I agree, but judging from my cursory googling, its something that would have to be written (ie its not a simple parameter or hasnt been written before that we can copy/paste), so I feel it falls outside the scope of this patch.

laanwj referenced this in commit 70f55355e2 on Jan 26, 2012

laanwj merged this on Jan 26, 2012

laanwj closed this on Jan 26, 2012

coblee referenced this in commit f5d0b8db5c on Jul 17, 2012

nomnombtc referenced this in commit cd28b0011d on May 18, 2017

dexX7 referenced this in commit d945a386cc on May 29, 2018

kallewoof referenced this in commit 2412d38015 on Oct 4, 2019

Losangelosgenetics referenced this in commit 75aae2ac41 on Mar 12, 2020

KolbyML referenced this in commit bc86927231 on Sep 4, 2020

rajarshimaitra referenced this in commit d627f47f65 on Aug 5, 2021

DrahtBot locked this on Sep 8, 2021

Full URL Support in bitcoin-qt #593