Correct the headers field description in getblockchaininfo.
"The current number of headers we have validated" is wrong - the field reports the height of the best known header.
I'm open to suggestions regarding the exact wording.
Sample output:
{
"chain": "main",
"blocks": 933527,
"headers": 933527,
...
}
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34393.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process. A summary of reviews will appear here.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
I think the change makes sense.
The current help doc says; "The current number of headers we have validated", which I think it implies; It is a count of headers that have been validated.
So a reader would reasonably interpret this as: “Total number of headers validated by the node so far” which sounds like a running total.
After checking the implementation:
obj.pushKV("headers", chainman.m_best_header ? chainman.m_best_header->nHeight : -1);
header returns nHeight (which is the height of the best known header chain tip) this is a single height value — not a count, through m_best_header->nHeight which in its doc states "Best header we've seen so far for which the block is not known to be invalid (used, among others, for getheaders queries' starting points)". My interpretation is that m_best_header is a pointer to the tip of the highest cumulative-work valid header chain known to the node.
Open to feedback from other reviewers if my understanding is inaccurate.
I think the PR title and commit message could be improved, because the goal of the PR is to correct the documentation. The title should reflect that the headers field description is inaccurate, rather than suggesting an off-by-one error (which implies a value is wrong by exactly +1 or −1).
As for the wording I think it makes sense. But let’s see what other reviewers think about the change.
@musaHaruna, thank you.
I think the PR title and commit message could be improved ... rather than suggesting an off-by-one error (which implies a value is wrong by exactly +1 or −1).
The number of headers (or blocks) in a chain is height + 1. Since the field reports height, it reports a value that is exactly 1 lower than the number it claims to report.