getblocktemplate: longpolling support #1355

1. 
   luke-jr commented at 5:15 pm on May 18, 2012: member

   Built on top of #936, this adds support for getblocktemplate longpolling to bitcoind.
2. 
   jgarzik commented at 4:23 am on July 4, 2012: contributor

   ACK on longpolling support in general. Long polling turns out to be a useful way to avoid http callbacks, with all the authentication that that entails.

   1. it is ugly and fragile to unlock, cv, then relock. Disappointing and would be nice if there were a better solution (note: that is not a NAK)

   2. does not pay attention to fShutdown

   3. “BIP22 compliance” smells more like self-promotion than a critically required bitcoind feature

3. 
   jgarzik commented at 4:10 pm on August 1, 2012: contributor

   ACK longpolling support

   Change appears mostly ACK-worthy. I worry about adding a new lock deep inside SetBestChain though.

4. 
   BitcoinPullTester commented at 7:33 am on August 10, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f1d42a05fe27d14f258ec5e1b15774dad583d458 for binaries and test log.
5. 
   BitcoinPullTester commented at 11:11 am on August 13, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/650ea32bbd60ac149809333131bd887537afa477 for binaries and test log.
6. 
   luke-jr commented at 5:50 pm on August 13, 2012: member

   Rebased
7. 
   jgarzik commented at 4:11 pm on September 4, 2012: contributor

   Re-rebase requested, now that BIP 22 is merged
8. 
   luke-jr commented at 8:33 pm on September 4, 2012: member

   done
9. 
   jgarzik commented at 1:08 am on September 5, 2012: contributor

   1. It accesses hashBestChain outside of locks.

   2. pointless wrapping inside do..while(0) block

   3. BIP 22 just says “longpollid” is a unique identifier. This code treats it as a block hash, not a job id. Thus, this change seems to hardcode unspoken assumptions about the longpollid.

   4. The code does not seem to notice TCP disconnections. Surely you do not want the thread to continue waiting for a new block, if the TCP connection is gone?

10. 
    luke-jr commented at 1:58 am on September 5, 2012: member

    1. Where?

    2. That exists so it can be break’d out of. I suppose it could probably work just as well with yet another nested if, though.

    3. “longpollid” is unique per long poll event; bitcoind only has such events when a new block is found, so the previous block hash is a fair fit. Clarified BIP 22 on the nature of “longpollid”’s uniqueness.

    4. Good idea, but I’m not sure how practical it is to do portably. Any suggestions?

11. 
    BitcoinPullTester commented at 8:35 pm on September 7, 2012: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/01e0e197ebb96cb14971c672b5704306e3ad0f1f for binaries and test log.
12. 
    luke-jr commented at 6:46 pm on November 14, 2012: member

    Rebased without the while(0). I still don’t see any hashBestChain accessing outside of locks, after looking over it again. With regard to TCP disconnects, I did look into this, but it seems not worth the effort considering:

    1. boost has no way to detect the socket being closed without reading
    2. it would violate the current layer abstraction we have in the RPC implementation
    3. while this is a problem for pools (eg, pushpool) with unreliable network clients, bitcoind’s RPC is only guaranteed to be usable from localhost, where it’s unlikely to occur
    4. a few stale sockets/threads that go away every new block shouldn’t harm the daemon much

    Thoughts?

13. getblocktemplate: longpolling support 419e8c2b82
14. 
    jgarzik commented at 5:02 pm on May 30, 2013: contributor

    No code objections.

    The main question remaining on this, our oldest pullreq: do we want/need it?

15. 
    jeremysawicki commented at 8:48 am on June 4, 2013: none

    I tested this a while ago and did not find it suitable for deployment as is.

    1. The long polling only returns when a new block is found. Ideally it should also return periodically to update the set of transactions. (Do we really want to encourage mining without including a reasonably up-to-date set of transactions?)
    2. It doesn’t handle application shutdown, so an open long polling request can prevent shutdown.
16. luke-jr closed this on Jul 16, 2013

    ---

17. 
    luke-jr commented at 0:09 am on July 17, 2013: member

    Err, no I didn’t :(
18. suprnurd referenced this in commit 17cf8dc6d1 on Dec 5, 2017
19. DrahtBot locked this on Sep 8, 2021

Contributors
luke-jr jgarzik BitcoinPullTester jeremysawicki