#30343 wallet, logging: Replace WalletLogPrintf() with LogInfo()
https://github.com/bitcoin/bitcoin/pull/30343 · · +534/-193 in 17 files, 14 commits · labels: Wallet, Needs rebase · draft
Goal
- Standardize wallet logging on modern severity macros while preserving wallet name prefixes
- Ensure source location logging reports the actual call site rather than a wrapper
Replaces the wallet-specific WalletLogPrintf() function with modern LogInfo(), LogWarning(), and LogError() macros across the wallet subsystem. It introduces a WalletLogContext class inheriting from util::log::Context to preserve wallet name prefixes in log messages while allowing -logsourcelocations to output actual call sites.
Problem: Wallet messages previously used a dedicated WalletLogPrintf wrapper to prefix wallet names, which prevented wallet code from using level-based logging macros and caused -logsourcelocations to report the wrapper location rather than where the log was triggered.
Category: Wallet (#53 of 84)
P3 · cleanup
- P3 because it eliminates legacy custom logging in favor of project-wide macros
- Fixes accurate call site reporting for wallet messages under -logsourcelocations
Eliminates legacy WalletLogPrintf in favor of project-wide logging macros. It preserves wallet names in output while fixing -logsourcelocations call sites for wallet developers and operators.
Membership: Replaces WalletLogPrintf across wallet components and adds WalletLogContext in src/wallet/logging.h.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 0
Reviewability: Stale: Needs rebase
The PR has merge conflicts against master and is stacked on top of unmerged logging PRs #29256 and #34778.
Author status: Active; regularly rebasing on top of prerequisite logging PRs and master.
Resolved concerns:
- Addressed lifetime safety of temporary log source objects raised by adyshimony
- Refined log severity levels across wallet loading and transaction processing following feedback from hodlinator and maflcko
Agreement: Strong
- Strong support across reviewers
- Tested and approved after temporary object lifetime questions were addressed (adyshimony)
- Reviewed with suggestions on naming and severity levels incorporated (hodlinator, maflcko)
Strong support; tested ACK from adyshimony and detailed review from hodlinator addressed
Reviewers have engaged with the approach and verified behavior. adyshimony tested and ACKed the branch after lifetime edge cases were discussed, and feedback from hodlinator on method naming and severity levels was incorporated.
- adyshimony tested and posted: 'ACK on 7fc95e7cbb3e'
- hodlinator reviewed and suggested adopting Log() accessors and refining severity levels
- maflcko noted documented guidelines for LogError versus LogWarning
Review verdicts (DrahtBot): 0 (+1)
- Stale ACK: adyshimony
Dependencies
Depends on: #29256, #34778
Enables:
- Accurate source location reporting in wallet logs when using -logsourcelocations
Based on (shares commits with): #29256, #34778
Files
228 lines under test/bench/ci.
- src/util/log.h +185/-62
- src/test/logging_tests.cpp +207/-21
- src/wallet/wallet.cpp +39/-35
- src/logging.cpp +18/-24
- src/wallet/walletdb.cpp +21/-15
- src/wallet/logging.h +29/-0
- src/wallet/wallet.h +9/-10
- src/wallet/scriptpubkeyman.h +5/-9
- src/wallet/spend.cpp +5/-3
- src/wallet/scriptpubkeyman.cpp +4/-3
- src/logging.h +3/-3
- src/validation.cpp +3/-2
- doc/developer-notes.md +1/-2
- src/wallet/receive.cpp +2/-1
- .github/workflows/ci.yml +1/-1
- src/wallet/dump.cpp +1/-1
- src/wallet/wallettool.cpp +1/-1
Card
This PR integrates the wallet subsystem with Bitcoin Core's modern level-based logging macros (LogInfo, LogWarning, LogError), eliminating the custom WalletLogPrintf helper. By introducing a WalletLogContext that prepends wallet identifiers automatically, it preserves multi-wallet log disambiguation while fixing -logsourcelocations to print real call sites. The change is worthwhile internal cleanup that improves debugging ergonomics for wallet maintainers and users. It has received strong review including a tested ACK from adyshimony, but is currently stale due to merge conflicts and its base dependencies on #29256 and #34778.