rpc: help metadata fixes#35513
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35513. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Please squash your commits |
54568d5 to
08dff58
Compare
08dff58 to
aadb3b5
Compare
| "4. \"valid-fork\" This branch is not part of the active chain, but is fully validated\n" | ||
| "5. \"active\" This is the tip of the active main chain, which is certainly valid"}, | ||
| "5. \"active\" This is the tip of the active main chain, which is certainly valid\n" | ||
| "6. \"unknown\" Status could not be determined"}, |
There was a problem hiding this comment.
Can you explain how to trigger this? This is uncovered code according to https://maflcko.github.io/b-c-cov/total.coverage/src/rpc/blockchain.cpp.gcov.html
There was a problem hiding this comment.
Agree, there is no known trigger.
I only added it to show what the implementation could theoretically return
There was a problem hiding this comment.
I don't think documenting something unreachable helps "align the docs" and this can be droped.
There was a problem hiding this comment.
Possibly this could be a NONFATAL_UNREACHABLE, but I haven't looked closely.
| {RPCResult::Type::NUM, "connections", /*optional=*/true, "the total number of connections"}, | ||
| {RPCResult::Type::NUM, "connections_in", /*optional=*/true, "the number of inbound connections"}, | ||
| {RPCResult::Type::NUM, "connections_out", /*optional=*/true, "the number of outbound connections"}, | ||
| {RPCResult::Type::BOOL, "networkactive", /*optional=*/true, "whether p2p networking is enabled"}, |
There was a problem hiding this comment.
I think this is wrong/dead code.
The correct fix would be to just require connman. See EnsureAny in src/rpc/server_util.h
There was a problem hiding this comment.
In aadb3b5
Agree, we should require connman (and peerman and mempool) using the helper(s).
aadb3b5 to
ac6a44b
Compare
Align RPC help documentation with actual implementation.