kernel, logging: Deliver each context's log output to its own logging connection #30342

pull ryanofsky wants to merge 26 commits into bitcoin:master from ryanofsky:pr/gklog changing 79 files +1569 −838

ryanofsky commented at 5:52 PM on June 26, 2024: contributor
Problem: An application that opens two chainstates (say mainnet and signet, or a test creating several contexts) gets log lines from both in one interleaved stream, with no way to tell which context each came from. #33847 lets an application set a logging connection on each context, but kernel code still logs through one global logger, so every connection receives every context's output.

Solution: Pass logger objects to the kernel classes that log (ChainstateManager, Chainstate, BlockManager, CoinsViews, CCoinsViewDB, CDBWrapper, FlatFileSeq and others) and log through them instead of the global logger. In libbitcoinkernel, each logging connection is the logger for the contexts it is set on, so a context's log output reaches only its own connection, and the kernel library no longer has any global logging state. In libbitcoinkernel only, messages from code that isn't given a log context, such as some low-level utility code, are dropped. There is no change in behavior for bitcoind and other executables not using the kernel API: code without a log context still logs to the application's logger, and log output is unchanged.

(This PR is an alternative to #30338 which solves the same problem, but makes logging calls more verbose and changes them across the entire codebase instead of just in kernel code.)



This is based on #34778 + #34374 + #29256 + #33847. The non-base commits are:
DrahtBot commented at 5:52 PM on June 26, 2024: contributor


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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/30342.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

Concept ACK sedited

Approach NACK stickies-v

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36395 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36395.svg"> (script: clarify implicit signature-cache input requirements by l0rinc)
- #36353 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36353.svg"> (kernel: don't reuse cached CheckBlock results across params by FlashWayne)
- #36326 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36326.svg"> (kernel: Use typed errors for fatal and flush error notifications by arejula27)
- #36244 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36244.svg"> (validation, net: Process blocks asynchronously and reduce cs_main contention by w0xlt)
- #36183 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36183.svg"> (span: diagnose dangling views from MakeByteSpan/MakeUCharSpan by kevkevinpal)
- #36167 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36167.svg"> ([RFC] Enable -Wunused by fanquake)
- #36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
- #36074 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36074.svg"> (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
- #36000 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36000.svg"> (validation: prefetch blocks while connecting by l0rinc)
- #35938 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35938.svg"> (blockstorage: fail instead of storing a null XOR key over a lost one by kwsantiago)
- #35911 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35911.svg"> (Warn on and add missing [[noreturn]] by fanquake)
- #35906 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35906.svg"> (First steps towards a stateless, side-effect free validation library by purpleKarrot)
- #35788 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35788.svg"> (bitcoin-util: Add evalscript subcommand by ajtowns)
- #35744 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35744.svg"> (coins: prevent DB resize from invalidating cursors by l0rinc)
- #35731 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35731.svg"> (Indexes: Harden the flush-error notification invariant by arejula27)
- #35714 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35714.svg"> (validation: stop writes after flush failure by l0rinc)
- #35676 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35676.svg"> (util: Abort in CheckDiskSpace/FlatFileSeq::Open on rare exceptions by maflcko)
- #35646 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35646.svg"> (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
- #35641 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35641.svg"> (kernel: Add script evaluation tracer by sedited)
- #35570 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35570.svg"> (refactor: Change some validation.cpp methods to return BlockValidationState by optout21)
- #35569 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35569.svg"> (Encapsulation for CTransaction by purpleKarrot)
- #35557 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35557.svg"> (kernel, validation: Add btck_chainstate_manager_set_clock_time by ryanofsky)
- #35524 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35524.svg"> (validation: Avoid rewriting the genesis block during index recovery by winterrdog)
- #35322 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35322.svg"> (logging: streamline Logger state and drop redundant methods by ryanofsky)
- #35307 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35307.svg"> (blockstorage: keep snapshot base in normal blockfile range by shuv-amp)
- #35187 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35187.svg"> (kernel: Block validation without a complete UTXO set by sedited)
- #35071 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35071.svg"> (Reindex: save progress to continue after interruption by pinheadmz)
- #35003 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35003.svg"> (validation: improve block data I/O error handling in P2P paths by furszy)
- #34864 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34864.svg"> (coins: tighten cache entry state invariants by l0rinc)
- #34844 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34844.svg"> (util: Add util::NotNull<SmartPtrType> by maflcko)
- #34775 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34775.svg"> (kernel: make logging callback global by stickies-v)
- #34729 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34729.svg"> (Reduce log noise by ajtowns)
- #34374 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34374.svg"> (kernel: use struct-based logging and simplify logging interface by stickies-v)
- #34254 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34254.svg"> (validation: Prevent duplicate logging and looping in invalid block handling by mzumsande)
- #34132 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34132.svg"> (coins, dbwrapper: remove error catcher, make point-read failures fatal by l0rinc)
- #33847 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33847.svg"> (kernel: Attach logging connections to contexts, per-connection log levels by ryanofsky)
- #33646 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33646.svg"> (log: check fclose() results and report safely in logging.cpp by cedwies)
- #33324 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33324.svg"> (blocks: add resumable reobfuscation for existing block files by l0rinc)
- #29843 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/29843.svg"> (policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only) by ajtowns)
- #29491 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/29491.svg"> ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
- #28690 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/28690.svg"> (build: Introduce internal kernel library by sedited)
- #24230 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/24230.svg"> (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
- #19461 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19461.svg"> (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
- #19460 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19460.svg"> (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
- #10102 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/10102.svg"> (Multiprocess bitcoin by ryanofsky)
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.



LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
- FlatFileSeq(m_logger, data_dir, "a", 100) in src/test/flatfile_tests.cpp (flatfile_allocate)
- FlatFileSeq(m_logger, data_dir, "a", 100) in src/test/flatfile_tests.cpp (flatfile_flush)
2026-10-02 00:43:02
sedited commented at 8:21 PM on June 26, 2024: contributor

Concept ACK.
ryanofsky force-pushed on Jun 26, 2024
DrahtBot added the label CI failed on Jun 26, 2024
DrahtBot commented at 10:35 PM on June 26, 2024: contributor



🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/26722091796
ryanofsky commented at 10:36 PM on June 26, 2024: contributor

Updated 45423355735770f01d2bac738cc8b0b41415e921 -> db1b9f7696af80036de739408cd9eae5d06481ec (pr/gklog.1 -> pr/gklog.2, compare) moving code to an earlier commit to fix a "test-each-commit" test failure https://github.com/bitcoin/bitcoin/actions/runs/9684398780/job/26722073921?pr=30342, and cleaning up code to fix a clang-tidy warning https://cirrus-ci.com/task/5893151045451776
DrahtBot removed the label CI failed on Jun 27, 2024
DrahtBot added the label Needs rebase on Jul 2, 2024
ryanofsky referenced this in commit ecadc8f6e5 on Jul 3, 2024
ryanofsky force-pushed on Jul 9, 2024
ryanofsky commented at 10:23 PM on July 9, 2024: contributor

Rebased db1b9f7696af80036de739408cd9eae5d06481ec -> 42d0e06d1a4343c2bf3257f26cf18b79072c6b0d (pr/gklog.2 -> pr/gklog.3, compare) to fix conflict with #30141. Also added a commit to let kernel applications direct low-level util output to custom log instances. Updated 42d0e06d1a4343c2bf3257f26cf18b79072c6b0d -> fcaed3faf0f12b3b322e65df1e81d455bbee7db5 (pr/gklog.3 -> pr/gklog.4, compare) to fix CI errors Updated fcaed3faf0f12b3b322e65df1e81d455bbee7db5 -> f005677f762754ae09417c2a418e27d28ef3c7e1 (pr/gklog.4 -> pr/gklog.5, compare) to fix more CI errors Rebased f005677f762754ae09417c2a418e27d28ef3c7e1 -> dcd055ecf9c20b84930c2ce45d922427cf0112ea (pr/gklog.5 -> pr/gklog.6, compare) due to conflicts with #30425 and #30407 Rebased dcd055ecf9c20b84930c2ce45d922427cf0112ea -> 53d7933727ebe0670dceb88ec9f1020bd7b0b280 (pr/gklog.6 -> pr/gklog.7, compare) due to conflict with #28052 Rebased 53d7933727ebe0670dceb88ec9f1020bd7b0b280 -> 10c275342dca365a6678da5fc016ca9d46d9dfca (pr/gklog.7 -> pr/gklog.8, compare) due to conflict with #30485 Rebased 10c275342dca365a6678da5fc016ca9d46d9dfca -> 493dd26b75d0e35422b3230be3c72a0f96eb5e8a (pr/gklog.8 -> pr/gklog.9, compare) due to various conflicts Rebased 493dd26b75d0e35422b3230be3c72a0f96eb5e8a -> e971028fa5e570acf585073494acd65f4831ab1c (pr/gklog.9 -> pr/gklog.10, compare) due to conflicts with #31393, #31490, #30965



Rebased e971028fa5e570acf585073494acd65f4831ab1c -> 282db88a9b1789f0d78928c87c7204402ac11ac3 (pr/gklog.10 -> pr/gklog.11, compare)



Rebased 282db88a9b1789f0d78928c87c7204402ac11ac3 -> ec6d771d3b34ec979e373f025c4f064ae44df860 (pr/gklog.11 -> pr/gklog.12, compare)



Rebased ec6d771d3b34ec979e373f025c4f064ae44df860 -> b1b72969c9764636721a7518a2f4453d0eb3845d (pr/gklog.12 -> pr/gklog.13, compare)



Rebased b1b72969c9764636721a7518a2f4453d0eb3845d -> d0e2944bb18e44c599312b980900c324ae698464 (pr/gklog.13 -> pr/gklog.14, compare) on updated base PR to fix CI errors (https://github.com/bitcoin/bitcoin/actions/runs/18514131135?pr=30342)



Rebased d0e2944bb18e44c599312b980900c324ae698464 -> 8482a49ab89012c3cd99acd43e1f698311426c3a (pr/gklog.14 -> pr/gklog.15, compare)



Rebased 8482a49ab89012c3cd99acd43e1f698311426c3a -> baaaa41b93cba064d12d641899968ee897eb0671 (pr/gklog.15 -> pr/gklog.16, compare) to fix fuzz test errors https://github.com/bitcoin/bitcoin/actions/runs/19249628369/job/55031630303?pr=30342, also making updates to bitcoinkernel.h documentation



Rebased baaaa41b93cba064d12d641899968ee897eb0671 -> b9dc75a2b5d6bfadce65cd2837a3042c888a8afd (pr/gklog.16 -> pr/gklog.17, compare)



Rebased b9dc75a2b5d6bfadce65cd2837a3042c888a8afd -> 97be3ef0b6239a9e674be3d8c70f99ec5fe5b8eb (pr/gklog.17 -> pr/gklog.18, compare)



Rebased 97be3ef0b6239a9e674be3d8c70f99ec5fe5b8eb -> db29e8fb43fa8011eb5c68f376ea74b4f087deda (pr/gklog.18 -> pr/gklog.19, compare) on top of #29256 pr/bclog.34 and #33847 pr/klog.4



Updated db29e8fb43fa8011eb5c68f376ea74b4f087deda -> d6c23d4219b30dc8e045f4c6b90a7ad493faf383 (pr/gklog.19 -> pr/gklog.20, compare) to fix kernel test MSAN error https://github.com/bitcoin/bitcoin/actions/runs/21115418786/job/60720235987?pr=30342



Rebased d6c23d4219b30dc8e045f4c6b90a7ad493faf383 -> 75e1e958f5e8a27491723aa88c69bf3cd8de7509 (pr/gklog.20 -> pr/gklog.21, compare) on top of #29256 pr/bclog.34 and #33847 pr/klog.5



Rebased 75e1e958f5e8a27491723aa88c69bf3cd8de7509 -> cc0cfe20c8ac56ffe3a47d9ee3f109fdf0c09c36 (pr/gklog.21 -> pr/gklog.22, compare) due to conflicts with #34253 and #33680



Updated cc0cfe20c8ac56ffe3a47d9ee3f109fdf0c09c36 -> c2e3362b0050483e5a95c9aaccb9d3afe6328c1e (pr/gklog.22 -> pr/gklog.23, compare) to fix CI errors https://github.com/bitcoin/bitcoin/actions/runs/21643951237



Updated c2e3362b0050483e5a95c9aaccb9d3afe6328c1e -> 7b78335f66b08d039f20dc31fb326883ab5ad3c8 (pr/gklog.23 -> pr/gklog.24, compare) to fix CI errors https://github.com/bitcoin/bitcoin/actions/runs/21645374775



Rebased 7b78335f66b08d039f20dc31fb326883ab5ad3c8 -> aa4d26979f0b1c24adeeec489b36180c87ca484c (pr/gklog.24 -> pr/gklog.25, compare) on top of #29256 pr/bclog.36 and #33847 pr/klog.5



Updated aa4d26979f0b1c24adeeec489b36180c87ca484c -> 5126eaf1e369f1579d2307125aa8383786f9b3f8 (pr/gklog.25 -> pr/gklog.26, compare) to fix IWYU and each-commit errors https://github.com/bitcoin/bitcoin/actions/runs/21830259022/job/62995439943?pr=30342



Updated 5126eaf1e369f1579d2307125aa8383786f9b3f8 -> eeb84002a84226614e5d3acb9013c15ba5ce71ff (pr/gklog.26 -> pr/gklog.27, compare) to fix IWYU errors https://github.com/bitcoin/bitcoin/actions/runs/21833389551/job/62998114427?pr=30342



Rebased eeb84002a84226614e5d3acb9013c15ba5ce71ff -> d137676ffd09da75f0418f7787fab8e249824e6f (pr/gklog.27 -> pr/gklog.28, compare) to fix conflicts with #32950, #34165, #33512 on top of #29256 pr/bclog.37



Updated d137676ffd09da75f0418f7787fab8e249824e6f -> ea7842f5b68c2986982c31f2482d47d2bc89c382 (pr/gklog.28 -> pr/gklog.29, compare) to fix silent conflicts with threadpool tests #34562 causing windows build errors and fuzz test failures in ci https://github.com/bitcoin/bitcoin/actions/runs/22559603524/job/65396727322?pr=30342



Updated ea7842f5b68c2986982c31f2482d47d2bc89c382 -> b6b95a62447c589a0078474fb8f6bc4b2e7e8a51 (pr/gklog.29 -> pr/gklog.30, compare) to fix threadpool fuzz test cleanup https://github.com/bitcoin/bitcoin/actions/runs/22584916974/job/65427016231?pr=30342



Rebased b6b95a62447c589a0078474fb8f6bc4b2e7e8a51 -> f048e807c6b4327474466c749fe48cb43dd651a0 (pr/gklog.30 -> pr/gklog.31, compare) on top of #29256 pr/bclog.39



Updated f048e807c6b4327474466c749fe48cb43dd651a0 -> 71e2a726cb812f40869c8e9dd09257ffbdb4ab05 (pr/gklog.31 -> pr/gklog.32, compare) to fix IWYU error in index/base.cpp https://github.com/bitcoin/bitcoin/actions/runs/22861803182/job/66317637780?pr=30342. Not really sure the IWYU error makes sense but implemented the suggestion



Rebased 71e2a726cb812f40869c8e9dd09257ffbdb4ab05 -> 74c694e7a55ed3d77bd70eef3b66fed7c5edb222 (pr/gklog.32 -> pr/gklog.33, compare) on top of #29256 pr/bclog.40



Squashed 74c694e7a55ed3d77bd70eef3b66fed7c5edb222 -> f3a87a203f8c6916786916200368701132f419d1 (pr/gklog.33 -> pr/gklog.34, compare) to fix compile error in internal commit https://github.com/bitcoin/bitcoin/actions/runs/23868674296/job/69594579042?pr=30342



Rebased f3a87a203f8c6916786916200368701132f419d1 -> 41fb7ccf129bee40b19e5968a7ee8b39c6893544 (pr/gklog.34 -> pr/gklog.35, compare) on top of #29256 pr/bclog.41



Updated 41fb7ccf129bee40b19e5968a7ee8b39c6893544 -> 6e7439bf9c5d0db4332ad71949ffad5cf0c91329 (pr/gklog.35 -> pr/gklog.36, compare) to fix ci error https://github.com/bitcoin/bitcoin/actions/runs/25750656813/job/75625762044 to fix test errors in test_kernel from a missing logger in btck_check_block_context_free in certain test orders



Rebased 6e7439bf9c5d0db4332ad71949ffad5cf0c91329 -> eeaa0f75f4fdb23b26cb0218c5b85d0c6203f978 (pr/gklog.36 -> pr/gklog.37, compare) on top of #29256 pr/bclog.42



Updated eeaa0f75f4fdb23b26cb0218c5b85d0c6203f978 -> 8ae06e76b2daacec338a590c5c9cf5216ca0d7df (pr/gklog.37 -> pr/gklog.38, compare) to fix CI: iwyu in sigcache.h, duplicate include in bitcoind.cpp, fuzzer null g_setup in dbwrapper.cpp https://github.com/bitcoin/bitcoin/actions/runs/26643336975/job/78525287457



Rebased 8ae06e76b2daacec338a590c5c9cf5216ca0d7df -> d70eae19a8b6024723f33f29907a3afc1a0a9ca1 (pr/gklog.38 -> pr/gklog.39, compare) on top of #29256 pr/bclog.42 and #33847 pr/klog.5 due to conflicts with #34806, #34865, #34866, and #34887



Rebased d70eae19a8b6024723f33f29907a3afc1a0a9ca1 -> b3a16a1ce6af8d32f628c9e95d1a2520e6fc34f0 (pr/gklog.39 -> pr/gklog.40, compare) on top of #29256 pr/bclog.42 and #33847 pr/klog.5 due to conflicts with #35465 and #35521



Squashed b3a16a1ce6af8d32f628c9e95d1a2520e6fc34f0 -> 459562de2121f94d55d0a6bfe86ed11a6c2b59af (pr/gklog.40 -> pr/gklog.41, compare) to fix test ancestor commits: pass m_logger to CCoinsViewDB in coins_db_leveldb_layout test https://github.com/bitcoin/bitcoin/actions/runs/28108478624/job/83229088477; and to fix fuzzer crash: stop dbwrapper_concurrent_reads thread pool before logger is destroyed https://github.com/bitcoin/bitcoin/actions/runs/28108478624/job/83229088738



Rebased 459562de212c27108832bb4d2822d7eda9a8b98e -> 2e611706fce63745a9bf08db4e4ca1631279e00e (pr/gklog.41 -> pr/gklog.42, compare) due to conflicts with #35568 and #35295



Updated 2e611706fce63745a9bf08db4e4ca1631279e00e -> 372179265729d0b0418bb9ec4dd6b05e812ebc0e (pr/gklog.42 -> pr/gklog.43, compare) to fix CI: duplicate BOOST_FIXTURE_TEST_SUITE in coinsviewoverlay_tests.cpp broke cmake configure https://github.com/bitcoin/bitcoin/actions/runs/29337611210/job/87100747364; and missing log context in CoinsViews::InitCache triggered LOG_REQUIRE_CONTEXT assertion in MSVC fuzz build https://github.com/bitcoin/bitcoin/actions/runs/29337611210/job/87100747283



Updated 372179265729d0b0418bb9ec4dd6b05e812ebc0e -> 532547978dac2d144fb92019b8b2007abb0e95ed (pr/gklog.43 -> pr/gklog.44, compare) to fix CI compile errors caused by silent conflicts with #35295 https://github.com/bitcoin/bitcoin/actions/runs/29343064668/job/87119598397



Updated 532547978dac2d144fb92019b8b2007abb0e95ed -> 9884477bcdc8abce1b640c66615c0ae44e0f9400 (pr/gklog.44 -> pr/gklog.45, compare) to fix CI: stop thread pool in coins_view fuzz targets before logger destroyed (https://github.com/bitcoin/bitcoin/actions/runs/29347701435/job/87135632047); store util::log::Context by value in CBitcoinLevelDBLogger to fix dangling ref SIGSEGV in feature_init.py (https://github.com/bitcoin/bitcoin/actions/runs/29347701435/job/87135631969)



Updated 9884477bcdc8abce1b640c66615c0ae44e0f9400 -> aa9bf0282d833956a41758a474282a1f765530a5 (pr/gklog.45 -> pr/gklog.46, compare) to fix CI: thread-safety analysis missing EXCLUSIVE_LOCKS_REQUIRED(!g_thread_pool_mutex) annotation in cleanup_coins_view() https://github.com/bitcoin/bitcoin/actions/runs/29352995762/job/87153614524



Updated aa9bf0282d833956a41758a474282a1f765530a5 -> d62ba526841ccb30912fe7ca7eac6265fd1cba43 (pr/gklog.46 -> pr/gklog.47, compare) to fix fuzzer crash: stop coinscache_sim thread pool before logger is destroyed https://github.com/bitcoin/bitcoin/actions/runs/29355819465/job/87163129249



Rebased d62ba526841ccb30912fe7ca7eac6265fd1cba43 -> 9cf188afa4742a181f5ac6a892a47e8be66ddc42 (pr/gklog.47 -> pr/gklog.48, compare) due to conflict with #35723



Updated 9cf188afa4742a181f5ac6a892a47e8be66ddc42 -> 846df5f6f474f727f13075ede6e4ce385e5e0bea (pr/gklog.48 -> pr/gklog.49, compare) to fix CI: undeclared g_thread_pool_mutex in cleanup functions (silent conflict with #35723 in push-48 rebase) https://github.com/bitcoin/bitcoin/actions/runs/29372633900/job/87219192227



Rebased 846df5f6f474f727f13075ede6e4ce385e5e0bea -> 84748bd36dff8cb77154623eac4c3f3647720072 (pr/gklog.49 -> pr/gklog.50, compare) due to conflict with #35496



Rebased 84748bd36dff8cb77154623eac4c3f3647720072 -> dbe85f39723568b54b02ee9fbce8585a865a6e11 (pr/gklog.50 -> pr/gklog.51, compare) due to conflicts with #35965



Rebased dbe85f39723568b54b02ee9fbce8585a865a6e11 -> ed27abafecce71a81e57a428e3c718e1cbc6a0c0 (pr/gklog.51 -> pr/gklog.52, compare) on top of #29256 pr/bclog.42 and #33847 pr/klog.7 due to conflicts with #36207, #35675, and #36364



Updated ed27abafecce71a81e57a428e3c718e1cbc6a0c0 -> 2f6341bf981004a9bea27f65e59859f7aab64319 (pr/gklog.52 -> pr/gklog.53, compare) to fix CI: declare LOG_REQUIRE_CONTEXT inline constexpr to avoid GCC -Wunused-const-variable errors, a warning recently enabled in master by #36275 https://github.com/bitcoin/bitcoin/actions/runs/36932568610/job/110605215926; fix IWYU include issues https://github.com/bitcoin/bitcoin/actions/runs/36932568610/job/110605215873. Also drop the TODO commit; the remaining log statements without a log context will be handled in a followup.
DrahtBot removed the label Needs rebase on Jul 10, 2024
ryanofsky force-pushed on Jul 10, 2024
ryanofsky force-pushed on Jul 10, 2024
DrahtBot added the label CI failed on Jul 10, 2024
DrahtBot commented at 6:22 PM on July 10, 2024: contributor



🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/27284943547
DrahtBot removed the label CI failed on Jul 10, 2024
DrahtBot added the label Needs rebase on Jul 16, 2024
ryanofsky force-pushed on Jul 18, 2024
DrahtBot removed the label Needs rebase on Jul 18, 2024
DrahtBot added the label CI failed on Jul 22, 2024
DrahtBot commented at 11:07 AM on July 22, 2024: contributor


🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/27590643116

<details><summary>Hints</summary>

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label CI failed on Jul 22, 2024
DrahtBot added the label Needs rebase on Jul 26, 2024
ryanofsky force-pushed on Aug 7, 2024
ryanofsky force-pushed on Aug 7, 2024
DrahtBot removed the label Needs rebase on Aug 7, 2024
DrahtBot added the label CI failed on Aug 13, 2024
DrahtBot commented at 7:50 PM on August 13, 2024: contributor


🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/28438476134

<details><summary>Hints</summary>

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot added the label Needs rebase on Aug 21, 2024
ryanofsky force-pushed on Dec 9, 2024
DrahtBot removed the label Needs rebase on Dec 9, 2024
DrahtBot removed the label CI failed on Dec 9, 2024
DrahtBot added the label CI failed on Dec 11, 2024
DrahtBot removed the label CI failed on Dec 17, 2024
DrahtBot added the label Needs rebase on Dec 18, 2024
ryanofsky force-pushed on Mar 12, 2025
DrahtBot removed the label Needs rebase on Mar 12, 2025
DrahtBot added the label Needs rebase on Mar 14, 2025
ryanofsky referenced this in commit 6d0745d22b on Apr 3, 2025
ryanofsky force-pushed on Apr 3, 2025
DrahtBot removed the label Needs rebase on Apr 3, 2025
ryanofsky referenced this in commit 2be41ef85f on Apr 3, 2025
ryanofsky referenced this in commit eaac991552 on Apr 4, 2025
ryanofsky force-pushed on Apr 4, 2025
DrahtBot added the label CI failed on Apr 4, 2025
DrahtBot commented at 3:37 AM on April 4, 2025: contributor


🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/39937617956

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label CI failed on Apr 4, 2025
DrahtBot added the label Needs rebase on Apr 8, 2025
ryanofsky referenced this in commit 8e3ee9d7e7 on Oct 15, 2025
ryanofsky referenced this in commit 9e7dabc35d on Oct 15, 2025
ryanofsky force-pushed on Oct 15, 2025
DrahtBot removed the label Needs rebase on Oct 15, 2025
ryanofsky referenced this in commit a5700c7911 on Oct 16, 2025
ryanofsky referenced this in commit e4ca362858 on Oct 16, 2025
ryanofsky force-pushed on Oct 16, 2025
DrahtBot added the label Needs rebase on Oct 29, 2025
fanquake referenced this in commit 4da01123df on Nov 4, 2025
ryanofsky referenced this in commit af46e97d08 on Nov 10, 2025
ryanofsky referenced this in commit d0bd114e97 on Nov 10, 2025
ryanofsky referenced this in commit 9883750ab0 on Nov 10, 2025
ryanofsky referenced this in commit 6bbf29c5f8 on Nov 10, 2025
ryanofsky force-pushed on Nov 10, 2025
DrahtBot removed the label Needs rebase on Nov 11, 2025
ryanofsky referenced this in commit 0926479924 on Nov 11, 2025
ryanofsky force-pushed on Nov 11, 2025
DrahtBot added the label Needs rebase on Nov 25, 2025
ryanofsky referenced this in commit 45af98eb5a on Dec 12, 2025
ryanofsky referenced this in commit 168128bb5e on Dec 12, 2025
ryanofsky referenced this in commit 7c49fcfc21 on Dec 12, 2025
ryanofsky referenced this in commit 2d0bc18b9f on Dec 12, 2025
ryanofsky force-pushed on Dec 12, 2025
DrahtBot removed the label Needs rebase on Dec 12, 2025
DrahtBot added the label CI failed on Dec 12, 2025
DrahtBot commented at 7:39 PM on December 12, 2025: contributor


🚧 At least one of the CI tasks failed. Task ASan + LSan + UBSan + integer: https://github.com/bitcoin/bitcoin/actions/runs/20166570929/job/57895220709 LLM reason (✨ experimental): CTest failure: test_kernel failed (exit code 8) causing the CI to fail.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot added the label Needs rebase on Dec 14, 2025
ryanofsky referenced this in commit fdcf06d4bb on Dec 16, 2025
ryanofsky referenced this in commit 90476d0bb5 on Dec 16, 2025
ryanofsky force-pushed on Dec 16, 2025
DrahtBot removed the label Needs rebase on Dec 16, 2025
DrahtBot added the label Needs rebase on Dec 16, 2025
ryanofsky referenced this in commit 5d43d4a39c on Jan 18, 2026
ryanofsky referenced this in commit ac3938a2f4 on Jan 18, 2026
ryanofsky referenced this in commit 05d22deffd on Jan 18, 2026
ryanofsky referenced this in commit c8f6a474f9 on Jan 18, 2026
ryanofsky referenced this in commit 480f08b3bd on Jan 18, 2026
ryanofsky force-pushed on Jan 18, 2026
DrahtBot removed the label Needs rebase on Jan 18, 2026
ryanofsky force-pushed on Jan 18, 2026
DrahtBot removed the label CI failed on Jan 18, 2026
DrahtBot added the label Needs rebase on Jan 19, 2026
ryanofsky referenced this in commit cde4ae89a0 on Jan 22, 2026
ryanofsky referenced this in commit 6d370c720a on Jan 22, 2026
ryanofsky force-pushed on Jan 22, 2026
DrahtBot removed the label Needs rebase on Jan 23, 2026
DrahtBot added the label Needs rebase on Jan 29, 2026
ryanofsky force-pushed on Feb 3, 2026
ryanofsky force-pushed on Feb 3, 2026
DrahtBot added the label CI failed on Feb 3, 2026
DrahtBot commented at 7:53 PM on February 3, 2026: contributor


🚧 At least one of the CI tasks failed. Task macOS native: https://github.com/bitcoin/bitcoin/actions/runs/21643951237/job/62390928227 LLM reason (✨ experimental): CI failure caused by the Bitcoin Kernel Test Suite aborting (SIGABRT) in the test_kernel tests.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label Needs rebase on Feb 3, 2026
ryanofsky force-pushed on Feb 3, 2026
DrahtBot removed the label CI failed on Feb 3, 2026
DrahtBot added the label Needs rebase on Feb 8, 2026
ryanofsky referenced this in commit 3a73b718e4 on Feb 9, 2026
ryanofsky referenced this in commit 2819b6d067 on Feb 9, 2026
ryanofsky referenced this in commit 8714e10178 on Feb 9, 2026
ryanofsky force-pushed on Feb 9, 2026
DrahtBot removed the label Needs rebase on Feb 9, 2026
ryanofsky force-pushed on Feb 9, 2026
DrahtBot added the label CI failed on Feb 9, 2026
DrahtBot commented at 4:49 PM on February 9, 2026: contributor


🚧 At least one of the CI tasks failed. Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/21830259022/job/62995439943 LLM reason (✨ experimental): CI failure caused by an IWYU (Include-What-You-Use) check failing.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
ryanofsky force-pushed on Feb 9, 2026
DrahtBot removed the label CI failed on Feb 9, 2026
DrahtBot added the label Needs rebase on Feb 18, 2026
ryanofsky referenced this in commit 5f5ac95167 on Mar 2, 2026
ryanofsky referenced this in commit ee3fb8cf39 on Mar 2, 2026
ryanofsky force-pushed on Mar 2, 2026
DrahtBot added the label CI failed on Mar 2, 2026
DrahtBot removed the label Needs rebase on Mar 2, 2026
ryanofsky force-pushed on Mar 2, 2026
ryanofsky force-pushed on Mar 3, 2026
DrahtBot removed the label CI failed on Mar 3, 2026
ryanofsky referenced this in commit 8517c14b77 on Mar 4, 2026
stickies-v commented at 12:58 AM on March 6, 2026: contributor
I've finally started reviewing this PR in more detail, after my recent related work on kernel logging. I'm currently leaning Concept ACK, Approach NACK.

Concept ACK because:
- a large part of the bitcoinkernel interface is contextualized, e.g. btck_ChainstateManager is initialized with a btck_Context, so having contextualized logging seems like a natural fit
- bitcoinkernel is a library, we should be minimally opinionated on how it's used, so long as it's safe. If users have good use cases for e.g. multiple ChainstateManager's, being able to separate log streams would be good
Approach NACK because I think the below downsides are bigger than the above upsides:
- logging interface becomes more cumbersome, i.e. caller has to figure out if a logger is in scope and if it should be used. It is easy to misuse (e.g. forget to log to context when it exists, which seems hard to enforce.)
- non-trivial code change / review cost, lots of merge conflicts, ...
I think this change might become more desirable if/when we have more demand for this feature (i.e. more actual use cases for contextualized logging) or when it becomes a less invasive code change to do so (e.g. if/when kernel is a separate codebase, or when it becomes feasible to use an interface like m_log.info(...)).

I've asked the kernel working group for use cases and motivations for contextualized logging, and so far I've not heard much (but I hope people will chime in here rather than me speaking on their behalf). If you have any, perhaps opening a tracking issue for that might be useful so we can better decide when this feature becomes worth it given the complexity? I'm definitely not against the idea itself, I just pragmatically think it's not worth the current cost.

Type	Reviewers
Concept ACK	sedited
Approach NACK	stickies-v

ryanofsky commented at 3:39 PM on March 6, 2026: contributor

Thanks for the review!

logging interface becomes more cumbersome, i.e. caller has to figure out if a logger is in scope and if it should be used. It is easy to misuse (e.g. forget to log to context when it exists, which seems hard to enforce.)

I think it's straightforward to enforce if there are places where we want to require log contexts. For example if we want to enforce it in kernel files it could look like:

--- a/src/util/log.h
+++ b/src/util/log.h
@@ -156,11 +156,19 @@ void Log(Level level, bool should_ratelimit, SourceLocation&& source_loc, Contex
 #define FirstArg_Impl(arg, ...) arg
 #define FirstArg_(args) FirstArg_Impl args
 
+constexpr bool LOG_REQUIRE_CONTEXT = false;
+
+template <typename T>
+concept LogContext = requires {
+    requires std::remove_reference_t<T>::log_context;
+};
+
 //! Internal helper to conditionally log. Only evaluates arguments when needed.
 // Allow __func__ to be used in any context without warnings:
 // NOLINTBEGIN(bugprone-lambda-function-name)
 #define LogPrint_(level, should_ratelimit, take_category, ...)                                     \
     do {                                                                                           \
+        static_assert(!LOG_REQUIRE_CONTEXT || LogContext<decltype(FirstArg_((__VA_ARGS__)))>, "Log call must pass a log context as first argument"); \
         auto&& _context{util::log::detail::GetContext<take_category>(FirstArg_((__VA_ARGS__)))};   \
         if (util::log::ShouldLog(_context.logger, _context.category, (level))) {                   \
             util::log::detail::Log((level), (should_ratelimit), SourceLocation{__func__},          \
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -76,6 +76,8 @@
 #include <tuple>
 #include <utility>
 
+#define LOG_REQUIRE_CONTEXT true
+
 using kernel::CCoinsStats;
 using kernel::ChainstateRole;
 using kernel::CoinStatsHashType;

</details>

Also note that without enforcement, the consequence of a missing context argument is just the log message going to the global logging stream, which already happens now.

non-trivial code change / review cost, lots of merge conflicts, ...

I'd be interested to know more about what part of the code change seem difficult to review. The main change is adding a new parameter to a bunch of functions, which seems like the archetype of a change that is trivial to verify. I guess there are a lot of test updates too. Anyway, would like to know more about what specific difficulties you see here.

DrahtBot added the label Needs rebase on Mar 6, 2026
stickies-v commented at 6:03 AM on March 8, 2026: contributor

I think it's straightforward to enforce if there are places where we want to require log contexts. For example if we want to enforce it in kernel files it could look like:

Nice, I hadn't thought of a per-file approach, that seems elegant.

I'd be interested to know more about what part of the code change seem difficult to review.

I don't think (and didn't say) that this PR is difficult to review. But I think "non-trivial code change / review cost, lots of merge conflicts, ..." is a reasonable statement for a cumulative diff of this size and 26 PR merge conflicts.

Note: I do think this is an elegant implementation, and I am not necessarily opposed to the changes, I think it would be nice if kernel eventually has contextualized logging. I just don't think it's currently important as it seems users (incl myself) don't actually need this. Just making kernel logging explicitly global (~which I planned to open a PR for earlier, but I've been focused on v31 review for the past weeks~ opened #34775) seems like the more pragmatic change to me now. It addresses the interface weirdness with a much smaller change, and keeps the logging interface simpler.
ryanofsky referenced this in commit a71c30c23d on Mar 9, 2026
ryanofsky referenced this in commit 502820b5a5 on Mar 9, 2026
ryanofsky force-pushed on Mar 9, 2026
DrahtBot removed the label Needs rebase on Mar 9, 2026
DrahtBot added the label CI failed on Mar 9, 2026
ryanofsky force-pushed on Mar 9, 2026
DrahtBot removed the label CI failed on Mar 9, 2026
DrahtBot added the label Needs rebase on Mar 11, 2026
ryanofsky referenced this in commit 1a8958da29 on Apr 1, 2026
ryanofsky referenced this in commit 9a9f683c1b on Apr 1, 2026
ryanofsky force-pushed on Apr 1, 2026
DrahtBot removed the label Needs rebase on Apr 1, 2026
ryanofsky force-pushed on Apr 1, 2026
DrahtBot added the label CI failed on Apr 1, 2026
DrahtBot commented at 8:57 PM on April 1, 2026: contributor


🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/23868674296/job/69594579042 LLM reason (✨ experimental): CI failed because the fuzz tests build failed while compiling src/test/fuzz/mini_miner.cpp (gmake error in fuzz target).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label CI failed on Apr 1, 2026
DrahtBot added the label Needs rebase on Apr 7, 2026
ryanofsky referenced this in commit 0a582a83e8 on May 7, 2026
ryanofsky force-pushed on May 12, 2026
DrahtBot added the label CI failed on May 12, 2026
DrahtBot commented at 6:21 PM on May 12, 2026: contributor


🚧 At least one of the CI tasks failed. Task macOS native: https://github.com/bitcoin/bitcoin/actions/runs/25750656813/job/75625762044 LLM reason (✨ experimental): CI failed because the test_kernel suite aborted with fatal error/SIGABRT in btck_check_block_context_free (Bitcoin Kernel Test Suite: 1 failure).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label Needs rebase on May 12, 2026
ryanofsky force-pushed on May 13, 2026
DrahtBot removed the label CI failed on May 13, 2026
enirox001 referenced this in commit b3e82e80d0 on May 15, 2026
DrahtBot added the label Needs rebase on May 21, 2026

log test: verify log argument evaluation semantics

Test that LogInfo/LogWarning/LogError always evaluate their arguments
even when logging is disabled.

ajtowns pointed out this behavior was important and could affect non-logging
code if changed in
https://github.com/bitcoin/bitcoin/pull/34374#discussion_r2734793117

Co-authored-by: l0rinc <pap.lorinc@gmail.com>

963ed23bc8

log test: add some test coverage on LogAcceptCategory
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
3ea9b7cb10

log test: Add test for all accepted logging arguments

Add new logging test to call macros with all allowed combinations of macro
arguments.

The new test replaces a less comprehensive test that doesn't cover log
statements without format arguments. It's also moved to the top of the test
suite because it's a good illustration of what typical log prints look like,
what logging calls are allowed and disallowed, and what the resulting log
output looks like.

9c509ca53c

log refactor: Ensure categories are not logged at info and higher levels

Previously this used to be possible through the LogPrintLevel call but now that
call is removed, this change is just an internal refactoring and has no visible
effect except in tests.

98f3bd6242

log refactor: log macro rewrite

Rewrite log macros to fix a number of issues: unnecessary strprintf
calls during fuzzing, confusing error messages when macros are called
with the wrong arguments, duplicated code with unexplained differences
and undocumented assumptions.

Since this is a rewrite, reading the new code in log.h first should be clearer
than starting from the diff.

Specific benefits of the new implementation are:

- Functionality is implemented once in a single `LOG_EMIT` macro instead of
  multiple times in different macros with diverging code paths and unexplained
  differences.

- Unnecessary `strprintf` calls are now skipped when logging is disabled (in
  bitcoind when `-noprinttoconsole -nodebuglogfile` options are used, and in
  tests and kernel applications when DisableLogging is called). This change
  should not affect bitcoind noticeably, but could speed up fuzz tests calling
  functions which log.

- Clearer error messages: If you pass a category to a macro which does not
  accept it, or forget to pass a category to a macro which requires it, you
  will see a direct message telling you to add or remove the category instead
  of expanded macro syntax errors.

- Previously it was possible to call `detail_LogWithSrcLoc` with
  inconsistent or invalid arguments, for example bypassing ratelimiting
  without it being explicit. New `LOG_EMIT` macro is compile-time safe
  and enforces the exact same restrictions as other macros.

- Previously "always evaluate arguments" behavior at Info and higher levels
  looked accidental and was undocumented
  (https://github.com/bitcoin/bitcoin/pull/34374#discussion_r2734793117). Now
  the behavior is documented and explicit.

- `NO_RATE_LIMIT` special case used to bypass rate limiting is dropped.
  The `LOG_EMIT` macro can control rate limiting (and other options if
  they are added in the future) while still using the same ratelimiting
  defaults as other macros.

Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-Authored-By: stickies-v <stickies-v@protonmail.com>

31f9929f3d

log refactor: Drop Entry::should_ratelimit field

Drop Entry::should_ratelimit field in favor of Options::ratelimit field.

Dropping the Entry::should_ratelimit field makes buffered log messages
in m_msgs_before_open smaller, and avoids potential confusion because
the field is stored after rate limiting has been applied, so just gets
ignored. Conceptually it also makes sense to treat the ratelimit option
as an input to the logger rather than as an attribute of the message
being logged.

f0929e1b78

Merge branch 'pr/relog' into pr/bclog 1ac1be8b15

log refactor: Allow log macros to accept context arguments

Allow LogDebug(), LogTrace(), LogInfo(), LogWarning(), and LogError() macros to
accept context arguments to provide more information in log messages and more
control over logging to callers.

This functionality is used in followup PRs:

- https://github.com/bitcoin/bitcoin/pull/30342 - To let libbitcoinkernel send
  output to specfic `BCLog::Logger` instances instead of a global instance, so
  output can be disambiguated and applications can have more control over
  logging.

- https://github.com/bitcoin/bitcoin/pull/30343 - To replace custom
  `WalletLogPrintf` calls with standard logging calls that automatically include
  wallet names and don't log everything at info level.

This commit does not change behavior of current log prints or require them to
be updated. It includes tests and documentation covering the new functionality.

Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-Authored-By: stickies-v <stickies-v@protonmail.com>

aa37de26b0

doc: Add documentation about log levels and macros 1eedcb9db5

log refactor: Add support for custom log contexts

Custom log contexts allow overridding log formatting and adding metadata, such
as request ids or wallet names to log messages, while still using standard
macros for logging. This is used to replace WalletLogPrintf() functions with
LogInfo() calls in followup PR #30343.

b5d3925d27

ryanofsky force-pushed on May 29, 2026
DrahtBot removed the label Needs rebase on May 29, 2026
DrahtBot added the label CI failed on May 29, 2026
DrahtBot commented at 5:56 PM on May 29, 2026: contributor


🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26643336975/job/78525287276 LLM reason (✨ experimental): CI failed because lint-includes.py detected a duplicate #include <logging.h> in src/bitcoind.cpp during include-lint checks.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
ryanofsky force-pushed on Jun 1, 2026
DrahtBot removed the label CI failed on Jun 2, 2026
DrahtBot added the label Needs rebase on Jun 3, 2026
ryanofsky force-pushed on Jun 8, 2026
DrahtBot removed the label Needs rebase on Jun 8, 2026
DrahtBot added the label Needs rebase on Jun 11, 2026
ryanofsky force-pushed on Jun 24, 2026
DrahtBot removed the label Needs rebase on Jun 24, 2026
DrahtBot added the label CI failed on Jun 24, 2026
DrahtBot commented at 5:53 PM on June 24, 2026: contributor


🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/28108478624/job/83229088477 LLM reason (✨ experimental): CI failed due to a C++ build error: test_bitcoin could not compile coins_tests.cpp because CCoinsViewDB was constructed with the wrong number/types of arguments (no matching constructor).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
ryanofsky force-pushed on Jun 24, 2026
DrahtBot removed the label CI failed on Jun 24, 2026
DrahtBot added the label Needs rebase on Jul 8, 2026
ryanofsky force-pushed on Jul 14, 2026
DrahtBot added the label CI failed on Jul 14, 2026
DrahtBot commented at 2:00 PM on July 14, 2026: contributor


🚧 At least one of the CI tasks failed. Task No wallet: https://github.com/bitcoin/bitcoin/actions/runs/29337611210/job/87100747364 LLM reason (✨ experimental): CI failed during CMake configuration because Clang 17 rejected the warning flag -Wunique-object-duplication (treated as an error via -Werror).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label Needs rebase on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
DrahtBot removed the label CI failed on Jul 14, 2026
DrahtBot added the label Needs rebase on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
ryanofsky force-pushed on Jul 14, 2026
DrahtBot added the label CI failed on Jul 14, 2026
DrahtBot commented at 10:57 PM on July 14, 2026: contributor


🚧 At least one of the CI tasks failed. Task NetBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/29372633900/job/87219192123 LLM reason (✨ experimental): CI failed because the fuzz test build for coins_view.cpp did not compile (g_thread_pool_mutex undeclared / thread-safety attribute errors, treated as -Werror).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label Needs rebase on Jul 14, 2026
DrahtBot removed the label CI failed on Jul 15, 2026
DrahtBot added the label Needs rebase on Aug 11, 2026
ryanofsky force-pushed on Aug 15, 2026
DrahtBot removed the label Needs rebase on Aug 15, 2026
DrahtBot added the label Needs rebase on Aug 19, 2026
ryanofsky force-pushed on Aug 20, 2026
DrahtBot removed the label Needs rebase on Aug 20, 2026
DrahtBot added the label Needs rebase on Sep 15, 2026
ryanofsky referenced this in commit 8e379446ec on Sep 25, 2026

kernel: add missing log levels and categories

There are log levels and categories emitted by kernel code that
are not yet part of the C API. Once entries are delivered as
structs in a future commit, every emitted value must map to a
constant, so add the missing ones first in this preparatory commit.

The LOCK constant is unconditional so the C API is the same in every
build.

No behaviour change. Part of a series of commits to make kernel logging
struct-based instead of string-based.

694eaad2eb

kernel: expose btck_LogEntry

Introduce the btck_LogEntry struct the logging callback will use in
a future commit (instead of the current string argument). The struct
is not yet used.

The level and category definitions move up so the callback typedef can
reference btck_LogEntry; review with --color-moved.

No behaviour change. Part of a series of commits to make kernel logging
struct-based instead of string-based.

29be57feda

kernel: add log level and category name helpers to wrapper

Convenience functions for consumers that want to format levels and
categories.

No behaviour change. Part of a series of commits to make kernel logging
struct-based instead of string-based.

0fe21007b1

kernel: remove API to format and disable logging

Both functions lose their purpose once the kernel logger is no longer
coupled to the node logger: formatting becomes the consumer's job, and
nothing is buffered before a connection exists, so there is nothing to
disable. Remove them early to simplify the next commits.

Behaviour change until the backend switch later in this series:
- strings are delivered in BCLog::Logger's default format
- the 1MB pre-connection buffer cannot be turned off.

Part of a series of commits to make kernel logging struct-based instead
of string-based.

9015a4071e

kernel: add KernelLogger

Add the kernel's own logging backend, unused until the next commit
switches the C API over to it.

Callbacks run while the logger's mutex is held: invocations are
serialized, and unregistering waits for an in-flight callback.

Callbacks are typed with a local LogCallback alias carrying the
struct-based signature, because btck_LogCallback still delivers a
string at this point. The next commit changes the typedef and replaces
the alias.

No behaviour change. Part of a series of commits to make kernel logging
struct-based instead of string-based.

bf1f4cb09d

kernel: make logging struct-based and simplify configuration

Update btck_LogCallback to use struct-based logging and drop logging.cpp
from the kernel build.

Since consumers can now easily filter by category, simplify the logging
configuration to just btck_logging_set_min_level.

Behaviour changes for consumers:
- Filtering is now purely levels-based, and consumers can choose any
  level they like. Debug and Trace logging applies to all categories,
  further filtering should be done client-side.
  Note: evaluation remains unchanged: Debug and Trace statements are
  only evaluated when they are requested, Info and above is always
  evaluated.
- Entries logged before the first connection are dropped, not buffered.
- Trailing newlines are stripped, callback exceptions are swallowed,
  and connection create and destroy no longer take cs_main.
- Log messages are no longer escaped, this can be done client-side.

KernelLogger moves into the anonymous namespace now that every member
is used. It stayed outside in the previous commit to avoid unused
member warnings.

Node logging is unchanged. Final commit of the series to make kernel
logging struct-based instead of string-based.

ad66992952

Merge branch 'pr/kslog' into pr/klog f1fda5fcdc

kernel: Set log level per connection and attach connections to contexts

Change btck_logging_set_min_level to take a btck_LoggingConnection and apply
only to that connection, and add btck_context_options_set_logger to attach a
connection to the contexts created with the options. A new connection receives
nothing until a context using it exists. The connection must outlive the
context options and contexts using it, and destroying it while a context still
uses it aborts.

Previously every connection received all kernel log output at one
process-wide level, and connections weren't passed to any other kernel
function. Code using the API had no reason to keep a connection where it
creates contexts, so separating log output per context later would have
required restructuring callers rather than adding a parameter.

Routing each context's entries only to its own connection is left for #30342.
Until then a connection used by a context may also receive entries from other
kernel operations, which the header documents. Because callers already attach
connections where log output comes from, that follow-up narrows what
connections receive without changing the API.

This change was written with Claude Opus 5.5 (1M context).

6c338d2ac9

Merge branch 'pr/bclog' into pr/gklog dec507c9cb
Merge branch 'pr/klog' into pr/gklog d4c9b9d16b

kernel: Update log hooks for context-aware logging API

Define the kernel's log hooks with the util::log::hooks::ShouldLog and
util::log::hooks::Log signatures introduced by the log macro rewrite,
replacing the util::log::ShouldDebugLog, ShouldTraceLog, and Log
functions #34374 defines. Both hooks now receive the logger and level.
All kernel log output still goes to the single KernelLogger, so the
logger argument is ignored.

This change was written with Claude Opus 5.5 (1M context).

c94e677a25

refactor: Pass Logger instances to kernel objects

Pass Logger instances to BlockManager, CCoinsViewDB, CDBWrapper,
ChainstateManager, and CoinsViews instances so libbitcoinkernel applications
and test code have the option to control where log output goes instead of
having all output sent to the global logger.

This commit just passes the logger objects without using them. The next commit
updates log print statements to use the new objects.

2a7dcb3f4c

logging: Add LOG_REQUIRE_CONTEXT option

Add option to require context objects to be passed to log macros in a file or
block of code, so new logging calls can't forget to specify them and
accidentally log to the global logging stream.

e2791f624d

refactor: Log kernel output to local log instances

This is a mechanical change updating kernel code that currently uses the global
log instance to log to local instances instead.

dec6a159b6

logging: Drop global Logger instance

Change LogInstance() to return a BCLog::Logger created by the application,
instead of allocating and leaking a global one. bitcoind, the other
executables, and test and fuzz setup code now create a Logger before the
first LogInstance() call, so their behavior is unchanged.

Code that is passed a logger explicitly, like the kernel objects that take
one since the previous commits, no longer depends on a global instance
existing. The kernel library needs no change here because it stopped using
LogInstance() in #34374.

This change was written with Claude Opus 5.5 (1M context).

253400ef68

kernel: Deliver log entries only to the connection of their context

Deliver each kernel log entry only to the logging connection of the context
it comes from, instead of to every connection that is set on any context.
Previously, while a connection was attached to at least one context, it also
received entries from other contexts and from code with no log context, as
the header documented. That caveat is now removed.

Each btck_LoggingConnection is now the util::log::Logger that its contexts'
kernel objects log to, so routing just follows the logger passed to
ChainstateManager and the objects it creates. A context without a logging
connection passes a connection with no callback, which discards entries.
Entries logged without a log context, from code that doesn't run on behalf of
a context, are dropped. The process-wide KernelLogger and its callback list
are gone, so the kernel library no longer has global logging state.

C API functions that log now log to the context they operate on. Log
messages that only repeated a failure already reported by a null return
("Block decode failed.", "Block header decode failed.", "Genesis block has
no previous.") are removed, as are "Logger connected." and "Logger
disconnecting.", which could only reach the connection itself.

Messages from low-level code that isn't passed a log context are now
dropped: util/fs_helpers.cpp and AutoFile I/O errors, random.cpp, and
DEBUG_LOCKORDER diagnostics in sync.cpp. A few messages from code that could
be given a context are also still dropped (signet.cpp and coinstats.cpp,
and the chain parameter and SHA256 implementation messages logged during
construction) and could be converted later.

This change was written with Claude Opus 5.5 (1M context).

2f6341bf98

ryanofsky force-pushed on Oct 1, 2026
ryanofsky renamed this:
~~kernel, logging: Pass Logger instances to kernel objects~~
kernel, logging: Deliver each context's log output to its own logging connection
on Oct 1, 2026
DrahtBot added the label CI failed on Oct 1, 2026
DrahtBot commented at 11:32 PM on October 1, 2026: contributor


🚧 At least one of the CI tasks failed. Task previous releases: https://github.com/bitcoin/bitcoin/actions/runs/36932568610/job/110605215754 LLM reason (✨ experimental): Build failed due to a -Werror=unused-const-variable error: LOG_REQUIRE_CONTEXT is defined but not used in src/util/log.h.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
DrahtBot removed the label Needs rebase on Oct 1, 2026
ryanofsky force-pushed on Oct 2, 2026
DrahtBot removed the label CI failed on Oct 2, 2026
DrahtBot added the label Needs rebase on Oct 5, 2026
DrahtBot commented at 10:11 PM on October 5, 2026: contributor



🐙 This pull request conflicts with the target branch and needs rebase.
nervana21 referenced this in commit 71bcef978d on Oct 6, 2026
nervana21 referenced this in commit 647467c498 on Oct 6, 2026

Contributors

Labels

Linked (view graph)

#27587 Bitcoin Kernel Library Project Tracking #29256 log, refactor: Allow log macros to accept context arguments #30338 RFC: Instanced logs #30595 kernel: Introduce C header API #33847 kernel: Attach logging connections to contexts, per-connection log levels #34062 RFC: separate kernel logging infrastructure #34374 kernel: use struct-based logging and simplify logging interface #34778 logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usability

kernel, logging: Deliver each context's log output to its own logging connection #30342

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)