[Qt] paymentserver: start netManager in uiReady() #3187

Diapolo commented at 10:34 AM on October 30, 2013: none

remove explicit init of netManager as this is done in the constructor anyway
move initNetManager() call to uiReady(), which removes an assert() and allows us to use message() in initNetManager() (currently unused but could be necessary because of proxy related messages)
make initNetManager() private
update paymentservertests.cpp

laanwj commented at 7:51 AM on October 31, 2013: member

I think we should aim to get initNetManager() up as soon as possible, if possible as soon as PaymentServer is created, not postpone it further.

As I see it: The sooner it can process incoming URIs and payment requests and save them up in savedPaymentRequests, the sooner it will no longer launch a new GUI when a new URI is clicked. But I may be wrong...

Diapolo commented at 4:45 PM on October 31, 2013: none

AFAIK, I think you are wrong here. URIs are pre-processed and stored in savedPaymentRequests via ipcSendCommandLine(). Which is done early in init. initNetManager() on the other hand isn't even used for sending saved requests to a local TCP server (nor is a proxy used for this), but for fetching payment requests from remote servers. So I see no reason to not do it like in this pull. No cons, but a few pros (saved assert(), possibility to use message()).

laanwj commented at 4:55 PM on October 31, 2013: member

I know it is used for receiving requests from remote processes. However, let's say that the user does not have a bitcoin client running then clicks on two bitcoin: URIs in a short timespan. What happens? The first click will start bitcoin-qt adn queue the first URI. However, the second click will start a second bitcoin-qt, which will continue because there is no server listening yet, and then fails due to the data directory lock. The second URI is probably lost. By starting the payment server as soon as possible, the second click would be queued in the first process and simply be processed when possible when the first instance reaches uiReady.

Diapolo commented at 5:20 PM on October 31, 2013: none

It all depends on QTimer::singleShot(100, paymentServer, SLOT(uiReady())); in bitcoin.cpp anyway?

Diapolo commented at 2:18 PM on November 4, 2013: none

I still think this is valuable ;).

laanwj commented at 3:24 PM on November 6, 2013: member

Agreed; initNetManager does something else than I thought it does

Nit: you can make initNetManager private after this

Diapolo commented at 6:54 AM on November 8, 2013: none

Updated:

make initNetManager() private

Diapolo commented at 7:09 AM on November 8, 2013: none

Updated:

update paymentservertests.cpp

[Qt] paymentserver: start netManager in uiReady()

- remove explicit init of netManager as this is done in the constructor
  anyway
- move initNetManager() call to uiReady(), which removes an assert() and
  allows us to use message() in initNetManager() (currently unused but
  could be necessary because of proxy related messages)
- make initNetManager() private
- update paymentservertests.cpp

7634e0d8de

BitcoinPullTester commented at 1:50 PM on November 11, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7634e0d8de4aac80a3a84dd2438f24f98670d44d for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj referenced this in commit a6aa179699 on Nov 11, 2013

laanwj merged this on Nov 11, 2013

laanwj closed this on Nov 11, 2013

Diapolo deleted the branch on Nov 11, 2013

Bushstar referenced this in commit ff1869d3f3 on Apr 8, 2020

DrahtBot locked this on Sep 8, 2021