The final LogInfo message about "No coin database inconsistencies" was printed unconditionally, even for check levels 0-2 where no coin DB verification runs and nGoodTransactions stays 0. Split into an always-on completion line and a coin-DB result line gated on nCheckLevel >= 3 && !skipped_l3_checks.
Before (checklevel=1):
Verification: No coin database inconsistencies in last 6 blocks (0 transactions)
After (checklevel=1):
Verification: checked last 6 blocks at level 1
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | sedited |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
The bottleneck in this project has always been review and testing, not writing code. Development here is intentionally conservative and slow, and reviewer attention is the scarcest resource we have. LLMs have made this worse, anyone can now prompt them and post their output as PRs. There is an infinite amount plausible looking "improvements" for LLMs to suggest and work on.
Unless we fully trust LLMs to both write and review code, humans still have to spend time understanding the proposed changes, which incurs a non-zero cost for every opened PR.
I understand that contributing to this project can be intimidating, and using LLMs may seem tempting, but it really creates more issues for this project than it solves. The best way to help this project, is to review and test changes. You can use LLMs for this, but you shouldn't solely rely on them, or just post their output.
I'm not asking you to close this PR. I am asking you to reconsider whether it's something you genuinely think the project should pursue, independent of what your LLM suggested.
@dergoegge Thanks for the honest feedback
I did use LLM to write this PR. I did reason about it and thought it was worth fixing when I ran bitcoin-cli verifychain 1 and saw the logs mentioning "No coin database inconsistencies" using a checklevel that doens't check coins.
The redundant pindex check in point 1 is more cosmetic
Happy to close it entirely if you don't think the log fix justifies the review burden.
I agree that more reviews and testing are what Core needs. I'll spend my time on that instead, and only open a PR of my own when I'm sure it's actually worth someone's time to look at.
The redundant pindex check in point 1 is more cosmetic
If it is a simple log fix, then the chance that I can review and merge it promptly is high. If there are other cosmetic changes sprinkled in between, it takes more effort and time. Please try to keep PRs focused. In this particular case, it looks like that check is there to show the programmer that it is a valid pointer and should therefore not be removed.
@sedited Thanks, updated the PR to make it focused on the log issue
ACK 1d669637495d98241f3aa8637b5f58934eceb07f