Reduce unnecessary default logging #23235

pull ajtowns wants to merge 4 commits into bitcoin:master from ajtowns:202110-acceptblockheader-silence changing 5 files +15 −11
  1. ajtowns commented at 3:52 AM on October 9, 2021: contributor

    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: contributor

    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:

    $ git grep 'LogPrintf(' -- "*.cpp" "*.h" | wc -l
493
  6. DrahtBot commented at 6:55 PM on October 9, 2021: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    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. DrahtBot cross-referenced this on Oct 10, 2021 from issue validation: log CChainState::CheckBlockIndex() consistency checks by jonatack
  8. MarcoFalke commented at 9:41 AM on October 11, 2021: member 
                                         File "/tmp/cirrus-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_misc.py", line 60, in run_test
                                       assert_equal(len(node.logging()), 26)
                                     File "/tmp/cirrus-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 50, in assert_equal
                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
                                   AssertionError: not(27 == 26)
  9. MarcoFalke removed the label Validation on Oct 11, 2021
  10. MarcoFalke removed the label Block storage on Oct 11, 2021
  11. MarcoFalke added the label Utils/log/libs on Oct 11, 2021
  12. sophia0898 approved
  13. validation: include block hash when reporting prev block not found errors 1d7d835ec3
  14. validation: move header validation error logging to VALIDATION debug category da94ebc2fa
  15. blockstorage: use debug log category 31b2b802b5
  16. validation: put coins cache write log into bench debug log b5950dd59c
  17. ajtowns force-pushed on Oct 11, 2021
  18. 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.

  19. promag commented at 11:50 AM on October 12, 2021: member

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20.

  20. practicalswift commented at 8:15 AM on October 13, 2021: contributor

    cr ACK b5950dd59ca3e144721a5f15568a65be43bd2f20

  22. laanwj commented at 12:24 PM on October 13, 2021: member

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20

  23. meshcollider commented at 5:19 AM on October 14, 2021: contributor

    Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20

  24. meshcollider merged this on Oct 14, 2021
  25. meshcollider closed this on Oct 14, 2021
  26. sidhujag referenced this in commit 2eecdc5983 on Oct 14, 2021
  27. Fabcien referenced this in commit baa8f6fd97 on Sep 23, 2022
  28. Fabcien referenced this in commit 5e24b193bd on Sep 23, 2022
  29. Fabcien referenced this in commit a589c68bad on Sep 23, 2022
  30. Fabcien referenced this in commit 85f4c47e49 on Sep 23, 2022
  31. Sjors cross-referenced this on Oct 3, 2022 from issue wallet: fast rescan with BIP157 block filters for descriptor wallets by theStack
  32. bitcoin locked this on Oct 30, 2022
Contributors
ajtownspracticalswiftDrahtBotMarcoFalkesophia0898promagEmpactlaanwjmeshcollider
Labels
Utils/log/libs
Linked (view graph)
#17421 ERROR AcceptBlockHeader log messages are kind of noisy#21559 Add mitigation for disk fill via logging attacks#25957 wallet: fast rescan with BIP157 block filters for descriptor wallets
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:53 UTC