Rationale: When bitcoin-cli help verifychain is called, the user doesn’t get any documentation about the checklevel argument, leading to issues like #18995.
This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels.
-checklevel command line documentation instead of duplicating the text here (which has a chance of becoming stale if the meanings change).
static const std::vector<std::string> FOO{"level 1verifies block validity", "bar"}; and then call Join(FOO, ", ") in the command line help and Join(FOO, ".\n") in the RPC help.
7@@ -8,8 +8,11 @@
8
9 #include <memory>
10 #include <string>
11+#include <vector>
12 #include <util/system.h>
13
14+static const std::vector<std::string> CHECKLEVEL_DOC{"level 0 reads the blocks from disk", "level 1 verifies block validity", "level 2 verifies undo data", "level 3 checks disconnection of tip blocks", "level 4 tries to reconnect the blocks", "each level includes the checks of the previous levels"};
I think it would be slightly easier to read if each was on their own line (like before).
0static const std::vector<std::string> CHECKLEVEL_DOC{
1"level 0 reads the blocks from disk",
2...
3"each level includes the checks of the previous levels",
4};
Also, a better place to put this would be in validation.h (just the declaration) and then define it in validation.cpp.
init.h seem an odd place to put RPC documentation.
I’ve changed it so that each is in their own line but wasn’t quite sure how to just declare in the .h and define in .cpp without the compiler telling me I was re-declaring it.
At the moment CHECKLEVEL_DOC is declared and defined in validation.h
83@@ -83,6 +84,15 @@ static const int DEFAULT_STOPATHEIGHT = 0;
84 static const unsigned int MIN_BLOCKS_TO_KEEP = 288;
85 static const signed int DEFAULT_CHECKBLOCKS = 6;
86 static const unsigned int DEFAULT_CHECKLEVEL = 3;
87+static const std::vector<std::string> CHECKLEVEL_DOC = {
0extern const std::vector<std::string> CHECKLEVEL_DOC;
In the header (for expensive types, not integers) this should just be the declaration. The rest should go to the cpp file.
88+ "level 0 reads the blocks from disk",
89+ "level 1 verifies block validity",
90+ "level 2 verifies undo data",
91+ "level 3 checks disconnection of tip blocks",
92+ "level 4 tries to reconnect the blocks",
93+ "each level includes the checks of the previous levels"
0 "each level includes the checks of the previous levels",
Nit: Add a trailing comma for consistency and to make clang-format happy?
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
81+ "level 0 reads the blocks from disk",
82+ "level 1 verifies block validity",
83+ "level 2 verifies undo data",
84+ "level 3 checks disconnection of tip blocks",
85+ "level 4 tries to reconnect the blocks",
86+ "each level includes the checks of the previous levels,"
76@@ -77,6 +77,14 @@ static constexpr std::chrono::hours DATABASE_WRITE_INTERVAL{1};
77 static constexpr std::chrono::hours DATABASE_FLUSH_INTERVAL{24};
78 /** Maximum age of our tip for us to be considered current for fee estimation */
79 static constexpr std::chrono::hours MAX_FEE_ESTIMATION_TIP_AGE{3};
80+extern const std::vector<std::string> CHECKLEVEL_DOC = {
extern here in the .cpp file, extern in the .h file should be enough, if I’m not mistaken.
0const std::vector<std::string> CHECKLEVEL_DOC{
nit: no need for the =
CHECKLEVEL_DOC?
Is validation.h/cpp the best place to put CHECKLEVEL_DOC?
checklevel is a validation setting, so where else would you put the documentation?
150@@ -150,6 +151,8 @@ extern bool fPruneMode;
151 /** Number of MiB of block files that we're trying to stay below. */
152 extern uint64_t nPruneTarget;
153
154+extern const std::vector<std::string> CHECKLEVEL_DOC;
Is validation.h/cpp the best place to put CHECKLEVEL_DOC?
checklevel is a validation setting, so where else would you put the documentation?
Yup, bad grep on my part.
1094@@ -1095,7 +1095,8 @@ static UniValue verifychain(const JSONRPCRequest& request)
1095 RPCHelpMan{"verifychain",
1096 "\nVerifies blockchain database.\n",
1097 {
1098- {"checklevel", RPCArg::Type::NUM, /* default */ strprintf("%d, range=0-4", DEFAULT_CHECKLEVEL), "How thorough the block verification is."},
1099+ {"checklevel", RPCArg::Type::NUM, /* default */ strprintf("%d, range=0-4", DEFAULT_CHECKLEVEL),
1100+ strprintf("How thorough the block verification is:\n %s", Join(CHECKLEVEL_DOC, ".\n"))},
suggestion:
0- strprintf("How thorough the block verification is:\n %s", Join(CHECKLEVEL_DOC, ".\n"))},
1+ strprintf("How thorough the block verification is:\n - %s", Join(CHECKLEVEL_DOC, "\n- "))},
which would improve the output from
01. checklevel (numeric, optional, default=3, range=0-4) How thorough the block verification is:
1 level 0 reads the blocks from disk.
2 level 1 verifies block validity.
3 level 2 verifies undo data.
4 level 3 checks disconnection of tip blocks.
5 level 4 tries to reconnect the blocks.
6 each level includes the checks of the previous levels
72. nblocks (numeric, optional, default=6, 0=all) The number of blocks to check.
to
01. checklevel (numeric, optional, default=3, range=0-4) How thorough the block verification is:
1 - level 0 reads the blocks from disk
2 - level 1 verifies block validity
3 - level 2 verifies undo data
4 - level 3 checks disconnection of tip blocks
5 - level 4 tries to reconnect the blocks
6 - each level includes the checks of the previous levels
72. nblocks (numeric, optional, default=6, 0=all) The number of blocks to check.
Thanks for fixing this issue.
ACK 292ed3c3810c4ca613c3edc5cb711c9843628fcc modulo suggestion below, happy to re-ACK if you take the suggestion. Verified the bitcoind checklevel help is unchanged and the RPC verifychain help is augmented.
0$ bitcoind -regtest -help-debug | grep -A6 checklevel
1 -checklevel=<n>
2 How thorough the block verification of -checkblocks is: level 0 reads
3 the blocks from disk, level 1 verifies block validity, level 2
4 verifies undo data, level 3 checks disconnection of tip blocks,
5 level 4 tries to reconnect the blocks, each level includes the
6 checks of the previous levels (0-4, default: 3)
git diff 292ed3c 501e6ab shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway
ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b 🚝
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b 🚝
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUjXMgv+L0HtUWgGV37oAs6lbrPfM8FnucZRCR88E9knckuvVA/uBsRso96E7b68
8jxmnCi7Ay7EGYeFVj3PCO/NvW1xMcwuU0GOQkCnfaPVGwDPZW4bJzxJz3fmIXJFL
9aoPy8C7erguDYzG5c8Y1Hx7LepCzaqmV/+0oQhp6AthIcRj2L/TKefXmj+RnJj4q
10WhfZTc6228eDi4ymZpjsJET/veYHllV7NoIQFF8lL7YHO6jvEleFRA4iWGAMy+SB
117TKmqL8G7+Ye9WJPHobmaAgaBc+wuerwvYI7rw/kcBjm/YfmrSg+2s+kdYzDei4A
12xr1XZR+vLKYlEZrnRShpagAkIUkiS4ovlhk64YSSm5CKxum/lKp0e5rJW0n+ClVg
134eJ84l1wS2Wn+/F2f3yq+5mKZYTNl5lS6CxEUu/a0UhE3iw102YbY9P9AN0pk6iL
14n4yQoVITjwsh9DGoUkymbhpWjyBWaYkgJ08oODWRXBxwtQERoGxqDZXz1P7Y6CKq
15p+a2uevd
16=ae1b
17-----END PGP SIGNATURE-----
Timestamp of file with hash 401273e51463f483d4eab16695a4f04b81f42ac918c61babfed48abdc8e8baa6 -
Labels
Docs