Built on top of #936, this adds support for getblocktemplate longpolling to bitcoind.
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-
luke-jr commented at 5:15 PM on May 18, 2012: member
-
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.
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)
does not pay attention to fShutdown
"BIP22 compliance" smells more like self-promotion than a critically required bitcoind feature
-
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.
-
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.
-
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.
-
luke-jr commented at 5:50 PM on August 13, 2012: member
Rebased
-
jgarzik commented at 4:11 PM on September 4, 2012: contributor
Re-rebase requested, now that BIP 22 is merged
-
luke-jr commented at 8:33 PM on September 4, 2012: member
done
-
jgarzik commented at 1:08 AM on September 5, 2012: contributor
It accesses hashBestChain outside of locks.
pointless wrapping inside do..while(0) block
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.
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?
-
luke-jr commented at 1:58 AM on September 5, 2012: member
Where?
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.
"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.
Good idea, but I'm not sure how practical it is to do portably. Any suggestions?
-
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.
-
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:
- boost has no way to detect the socket being closed without reading
- it would violate the current layer abstraction we have in the RPC implementation
- 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
- a few stale sockets/threads that go away every new block shouldn't harm the daemon much
Thoughts?
-
getblocktemplate: longpolling support 419e8c2b82
-
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?
-
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.
- 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?)
- It doesn't handle application shutdown, so an open long polling request can prevent shutdown.
- luke-jr closed this on Jul 16, 2013
-
luke-jr commented at 12:09 AM on July 17, 2013: member
Err, no I didn't :(
- suprnurd referenced this in commit 17cf8dc6d1 on Dec 5, 2017
- DrahtBot locked this on Sep 8, 2021