getblocktemplate: longpolling support #1355

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:gmp_longpoll changing 3 files +47 −0
  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. luke-jr cross-referenced this on May 18, 2012 from issue BIP22: getblocktemplate by luke-jr
  3. luke-jr cross-referenced this on Jun 8, 2012 from issue JSON-RPC: add filter{clearall,clear,add}, a block and tx notification system by jgarzik
  4. 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

  5. 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.

  6. 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.

  7. luke-jr cross-referenced this on Aug 13, 2012 from issue next 2012-08-13 by luke-jr
  8. 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.

  9. luke-jr commented at 5:50 PM on August 13, 2012: member

    Rebased

  10. jgarzik commented at 4:11 PM on September 4, 2012: contributor

    Re-rebase requested, now that BIP 22 is merged

  11. luke-jr commented at 8:33 PM on September 4, 2012: member

    done

  12. 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?

  13. 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?

  14. 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.

  15. 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?

  16. getblocktemplate: longpolling support 419e8c2b82
  17. 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?

  18. 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.
  19. luke-jr closed this on Jul 16, 2013

  20. luke-jr commented at 12:09 AM on July 17, 2013: member

    Err, no I didn't :(

  21. luke-jr cross-referenced this on Jul 17, 2013 from issue getblocktemplate: longpolling support by luke-jr
  22. suprnurd referenced this in commit 17cf8dc6d1 on Dec 5, 2017
  23. bitcoin locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:56 UTC