[Trivial] Correct waitforblockheight example help text

fanquake commented at 12:39 pm on November 12, 2016: member

Two commits, both trivial. The first fixes a typo in the waitforblockheight help text. The second commit is meant to address #7430. Adds the meaning of “best block”, but I don’t think we need to clarify why someone would want to be notified in the help output.

fanquake added the label RPC/REST/ZMQ on Nov 12, 2016

in src/rpc/blockchain.cpp: in 123d158a4d outdated

193@@ -194,7 +194,7 @@ UniValue waitfornewblock(const JSONRPCRequest& request)
194 {
195     if (request.fHelp || request.params.size() > 1)
196         throw runtime_error(
197-            "waitfornewblock\n"
198+            "waitfornewblock <timeout>\n"

paveljanik commented at 12:46 pm on November 12, 2016:

Optionals are marked with (), IIRC.

paveljanik commented at 12:47 pm on November 12, 2016:

Outside of the scope of this PR, but as we are here: timeout is int, in ms. And the default is false!?

MarcoFalke commented at 3:39 pm on November 12, 2016:

False means no timeout…

MarcoFalke commented at 3:41 pm on November 12, 2016:

So maybe default=0=no timeout

jnewbery commented at 4:06 pm on November 22, 2016:

Perhaps:

"1. timeout (int, optional, default=0) Time in milliseconds to wait for a response. 0 indicates no timeout.\n"

Should be fixed in waitfornewblock(), waitforblock() and waitforblockheight() RPCs.

in src/rpc/blockchain.cpp: in 123d158a4d outdated

273@@ -274,7 +274,7 @@ UniValue waitforblockheight(const JSONRPCRequest& request)
274 {
275     if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
276         throw runtime_error(
277-            "waitforblock\n"
278+            "waitforblockheight <blockheight> <timeout>\n"
279             "\nWaits for (at least) block height and returns the height and hash\n"
280             "\nof the current tip.\n"

jnewbery commented at 3:45 pm on November 22, 2016:

Since you’re fixing the comment, we should lose the \n at the start of line 279 so there’s only one newline. Otherwise the sentence gets broken up:

0waitforblockheight <blockheight> <timeout>
1
2Waits for (at least) block height and returns the height and hash
3
4of the current tip.
5...

jnewbery commented at 4:08 pm on November 22, 2016: member

tested ACK https://github.com/bitcoin/bitcoin/pull/9144/commits/9e7447e5a4f7363331e7ea21f700aac2d36ce70f. I’ve added a couple of nits to the review.

NACK https://github.com/bitcoin/bitcoin/pull/9144/commits/123d158a4d01a04873ebfa150008188d31db7ccf. I don’t think we should include definitions best block in any of the self-documenting help since it’s difficult to succinctly summarise, it should be a well-understood term, and we shouldn’t add the same definition in many places.

In fact, it’s incorrectly defined in the getbestblockhash RPC as “Returns the hash of the best (tip) block in the longest blockchain.” That’s wrong, since the best block is the tip of the chain with the most work, not the tip of the longest chain.

It’d be far better to make sure that best block is correctly defined at https://bitcoin.org/en/developer-documentation.

fanquake commented at 6:10 am on November 23, 2016: member

Will update to address nits and drop 123d158.

fanquake force-pushed on Nov 23, 2016

MarcoFalke commented at 11:48 am on November 23, 2016: member

utACK 3be423d94143368d4d28cacfaffacab9f685f184

jnewbery commented at 10:15 am on November 24, 2016: member

tested ACK https://github.com/bitcoin/bitcoin/pull/9144/commits/3be423d94143368d4d28cacfaffacab9f685f184

in src/rpc/blockchain.cpp: in 3be423d941 outdated

199             "\nWaits for a specific new block and returns useful info about it.\n"
200             "\nReturns the current block on timeout or exit.\n"
201             "\nArguments:\n"
202-            "1. timeout (milliseconds) (int, optional, default=false)\n"
203+            "1. timeout (int, optional, default=0) Time in milliseconds to wait for a response. 0 indicates no timeout.\n"
204             "\nResult::\n"

paveljanik commented at 11:04 am on November 24, 2016:

Please remove : here and completely in this file.

fanquake commented at 11:52 am on November 24, 2016:

Done.

paveljanik commented at 11:07 am on November 24, 2016: contributor

utACK https://github.com/bitcoin/bitcoin/pull/9144/commits/3be423d94143368d4d28cacfaffacab9f685f184

Correct help output for waitfor RPC commands e3c4f7e182

fanquake force-pushed on Nov 24, 2016

paveljanik commented at 11:57 am on November 24, 2016: contributor

utACK e3c4f7e

MarcoFalke commented at 12:14 pm on November 24, 2016: member

utACK e3c4f7e18246f0872c816358d4013d229261bbcf

MarcoFalke merged this on Nov 24, 2016

MarcoFalke closed this on Nov 24, 2016

MarcoFalke referenced this in commit c98f6b3d93 on Nov 24, 2016

fanquake deleted the branch on Nov 24, 2016

codablock referenced this in commit 79c34fbc6d on Jan 16, 2018

codablock referenced this in commit 7bce8fb441 on Jan 16, 2018

codablock referenced this in commit a71702e3e2 on Jan 17, 2018

andvgal referenced this in commit 078e73ae6f on Jan 6, 2019

CryptoCentric referenced this in commit 55a3743696 on Feb 25, 2019

MarcoFalke locked this on Sep 8, 2021

[Trivial] Correct waitforblockheight example help text #9144