Reduce unnecessary default logging #23235

1. 
   ajtowns commented at 3:52 am on October 9, 2021: member

   Moves the following log messages into debug log categories:

   • “AcceptBlockHeader: …” to validation
   • “Prune: deleted blk/rev” to new blockstorage log category
   • “Leaving block file” moves from validation to blockstorage
   • “write coins cache to disk” to bench

   Also adds the hash of the block to the log message when AcceptBlockHeader is rejecting because of problems with the prev block.

2. 
   ajtowns commented at 3:58 am on October 9, 2021: member

   Inspired by an irc complaint the other day about a ContextualCheckBlockHeader log message about a bad-diffbits failure for block 000000000000000000639be19a0123a1c99d9fef89f0b8ac055a77f4ef86ae3b which is BCH block at height 478577 from Aug 2017.

   This might resolve issue #17421

3. DrahtBot added the label Block storage on Oct 9, 2021
4. DrahtBot added the label Validation on Oct 9, 2021
5. 
   practicalswift commented at 11:37 am on October 9, 2021: contributor

   Very Strong Concept ACK

   We still do too much unconditional logging IMO: it is spammy (low signal-to-noise) and potentially dangerous (see disk fill attack scenarios described in #21559).

   Thanks a lot for improving the situation! :)

   I’m willing to review any PR that improves the logging situation by thoughtfully moving unconditional logging (LogPrintf(…)) to conditional logging (LogPrint(CATEGORY, …): feel free to ping me in to such PRs for speedy review! :)

   FWIW:

   0$ git grep 'LogPrintf(' -- "*.cpp" "*.h" | wc -l
   1493
   

6. 
   DrahtBot commented at 6:55 pm on October 9, 2021: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #22956 (validation: log CChainState::CheckBlockIndex() consistency checks by jonatack)

   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.

7. 
   MarcoFalke commented at 9:41 am on October 11, 2021: member

   0                                     File "/tmp/cirrus-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_misc.py", line 60, in run_test
   1                                       assert_equal(len(node.logging()), 26)
   2                                     File "/tmp/cirrus-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 50, in assert_equal
   3                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
   4                                   AssertionError: not(27 == 26)
   

8. MarcoFalke removed the label Validation on Oct 11, 2021
9. MarcoFalke removed the label Block storage on Oct 11, 2021
10. MarcoFalke added the label Utils/log/libs on Oct 11, 2021
11. sophia0898 approved
12. validation: include block hash when reporting prev block not found errors 1d7d835ec3
13. validation: move header validation error logging to VALIDATION debug category da94ebc2fa
14. blockstorage: use debug log category 31b2b802b5
15. validation: put coins cache write log into bench debug log b5950dd59c
16. ajtowns force-pushed on Oct 11, 2021
17. in src/validation.cpp:3208 in da94ebc2fa outdated

    3204@@ -3205,7 +3205,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS
    3205             if (ppindex)
    3206                 *ppindex = pindex;
    3207             if (pindex->nStatus & BLOCK_FAILED_MASK) {
    3208-                LogPrintf("ERROR: %s: block %s is marked invalid\n", __func__, hash.ToString());
    

    ---

    ---

    

    promag commented at 11:50 am on October 12, 2021:

    da94ebc2facd75c6105a7bd31765c6d2b37fc73b

    nit, not sure if this change (commit) merits a release note.

18. 
    promag commented at 11:50 am on October 12, 2021: member

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20.
19. 
    practicalswift commented at 8:15 am on October 13, 2021: contributor

    cr ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
20. 
    Empact commented at 11:48 am on October 13, 2021: member

    Code review ACK https://github.com/bitcoin/bitcoin/pull/23235/commits/b5950dd59ca3e144721a5f15568a65be43bd2f20
21. 
    laanwj commented at 12:24 pm on October 13, 2021: member

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
22. 
    meshcollider commented at 5:19 am on October 14, 2021: contributor

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
23. meshcollider merged this on Oct 14, 2021
24. meshcollider closed this on Oct 14, 2021

    ---

25. sidhujag referenced this in commit 2eecdc5983 on Oct 14, 2021
26. Fabcien referenced this in commit baa8f6fd97 on Sep 23, 2022
27. Fabcien referenced this in commit 5e24b193bd on Sep 23, 2022
28. Fabcien referenced this in commit a589c68bad on Sep 23, 2022
29. Fabcien referenced this in commit 85f4c47e49 on Sep 23, 2022
30. DrahtBot locked this on Oct 30, 2022

Contributors
ajtowns practicalswift DrahtBot MarcoFalke sophia0898 promag Empact laanwj meshcollider

Labels
Utils/log/libs