[rpc] Fix pruneheight help description in getblockchaininfo #11366

pull esotericnonsense wants to merge 1 commits into bitcoin:master from esotericnonsense:2017-09-getblockchaininfo-docs changing 1 files +9 −9

esotericnonsense commented at 7:47 AM on September 19, 2017: contributor

This is a bit nitpicky, but the field isn't included in the JSON at all unless fPruneMode is set. Alternatively we could return 0 (and remove 'pruned', which would then be pointless, but someone might be relying on that).
fanquake added the label RPC/REST/ZMQ on Sep 19, 2017
practicalswift commented at 2:42 PM on September 19, 2017: contributor

utACK 9da936232743a2235dade998be7f01ce0cf1b709. Clarity is good.
jnewbery commented at 5:58 PM on September 19, 2017: member

utACK 9da936232743a2235dade998be7f01ce0cf1b709

Also consider in this PR moving the if (fPruneMode) code block up in the function so that it's immediately below the pruned line. That seems more logical and the return object would be ordered the same as the help text.

in src/rpc/blockchain.cpp:1139 in 9da9362327 outdated

1135 | @@ -1136,7 +1136,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
1136 |              "  \"verificationprogress\": xxxx, (numeric) estimate of verification progress [0..1]\n"
1137 |              "  \"chainwork\": \"xxxx\"     (string) total amount of work in active chain, in hexadecimal\n"
1138 |              "  \"pruned\": xx,             (boolean) if the blocks are subject to pruning\n"
1139 | -            "  \"pruneheight\": xxxxxx,    (numeric) lowest-height complete block stored\n"
1140 | +            "  \"pruneheight\": xxxxxx,    (numeric) lowest-height complete block stored (if pruning is enabled)\n"

promag commented at 12:03 AM on September 20, 2017:

Update to (only present if pruning is enabled). See this example for reference.

promag commented at 12:03 AM on September 20, 2017: member

Nit, I would reword commit and PR to something along [rpc] Fix pruneheight help description in getblockchaininfo.

Otherwise utACK 9da9362.
[rpc] Fix pruneheight help description in getblockchaininfo 887dcc47de
esotericnonsense force-pushed on Sep 20, 2017
esotericnonsense renamed this:
~~Trivial: RPC/getblockchaininfo: pruneheight is only present in pruned mode~~
[rpc] Fix pruneheight help description in getblockchaininfo
on Sep 20, 2017
esotericnonsense commented at 2:41 PM on September 20, 2017: contributor

Done and done, tested latest commit, appears in correct order.

in src/rpc/blockchain.cpp:1183 in 887dcc47de

1179 | @@ -1180,6 +1180,14 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
1180 |      obj.push_back(Pair("verificationprogress",  GuessVerificationProgress(Params().TxData(), chainActive.Tip())));
1181 |      obj.push_back(Pair("chainwork",             chainActive.Tip()->nChainWork.GetHex()));
1182 |      obj.push_back(Pair("pruned",                fPruneMode));
1183 | +    if (fPruneMode)

promag commented at 2:43 PM on September 20, 2017:

Since this is touched, update as per developer notes? { in this line.

esotericnonsense commented at 3:07 PM on September 20, 2017:

Done in #11367

in src/rpc/blockchain.cpp:1186 in 887dcc47de

1179 | @@ -1180,6 +1180,14 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
1180 |      obj.push_back(Pair("verificationprogress",  GuessVerificationProgress(Params().TxData(), chainActive.Tip())));
1181 |      obj.push_back(Pair("chainwork",             chainActive.Tip()->nChainWork.GetHex()));
1182 |      obj.push_back(Pair("pruned",                fPruneMode));
1183 | +    if (fPruneMode)
1184 | +    {
1185 | +        CBlockIndex *block = chainActive.Tip();
1186 | +        while (block && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA))

promag commented at 2:44 PM on September 20, 2017:

Add { }.

promag commented at 2:45 PM on September 20, 2017:

Remove first condition block and add assert(block); above.

esotericnonsense commented at 2:49 PM on September 20, 2017: contributor

Suggest merging this with #11367 to avoid PR dependencies?
esotericnonsense commented at 3:05 PM on September 20, 2017: contributor

Changes are now included in #11367.
esotericnonsense closed this on Sep 20, 2017
laanwj referenced this in commit 2d85899303 on Sep 25, 2017
esotericnonsense deleted the branch on Sep 30, 2017
PastaPastaPasta referenced this in commit 2d99b73ca1 on Dec 22, 2019
PastaPastaPasta referenced this in commit f376b46853 on Jan 2, 2020
PastaPastaPasta referenced this in commit 2329b8ebd2 on Jan 4, 2020
PastaPastaPasta referenced this in commit 1f29f1c09d on Jan 12, 2020
PastaPastaPasta referenced this in commit 15562d8680 on Jan 12, 2020
PastaPastaPasta referenced this in commit 81461b7245 on Jan 12, 2020
PastaPastaPasta referenced this in commit 4b6d804d64 on Jan 12, 2020
ckti referenced this in commit 2ec6cd7dfb on Mar 28, 2021
gades referenced this in commit 553a447c5c on Jun 26, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11370 [test] Add getblockchaininfo functional test