wallet: use LogTrace for walletdb log messages at trace level #30355

1. 
   ajtowns commented at 7:46 am on June 28, 2024: contributor

   Wallet sqlite logging is enabled by -debug=walletdb -loglevel=walletdb:trace however the actual log messages are sent at BCLog::Level::Info. Switch to the trace level to make this consistent. This adds [walletdb:trace] to the log output, eg:

   0[httpworker.3] [wallet/sqlite.cpp:55] [TraceSqlCallback] [/tmp/bitcoin_func_test_4fsnatpg/node0/regtest/wallets/boring/wallet.dat] SQLite Statement: BEGIN EXCLUSIVE TRANSACTION
   

   becomes

   0[httpworker.0] [wallet/sqlite.cpp:55] [TraceSqlCallback] [walletdb:trace] [/tmp/bitcoin_func_test_9lcwth4z/node0/regtest/wallets/boring/wallet.dat] SQLite Statement: BEGIN EXCLUSIVE TRANSACTION
   

2. wallet: use LogTrace for walletdb log messages at trace level 46819f5df6
3. 
   DrahtBot commented at 7:46 am on June 28, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ryanofsky, maflcko, furszy, luke-jr

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)

   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.

4. DrahtBot added the label Wallet on Jun 28, 2024
5. 
   maflcko commented at 9:00 am on June 28, 2024: member

   ACK 46819f5df6de2a860c3ec87d55527b617a3b128f

   Makes sense to use the severity level and category that the user asked for, instead of a different one.

   Could also change it to [warn] when the user request could not be fulfilled?

   https://github.com/bitcoin/bitcoin/blob/46819f5df6de2a860c3ec87d55527b617a3b128f/src/wallet/sqlite.cpp#L270

6. ryanofsky approved
7. 
   ryanofsky commented at 10:30 pm on June 28, 2024: contributor

   Code review ACK 46819f5df6de2a860c3ec87d55527b617a3b128f. Nice catch!
8. bitcoin deleted a comment on Jun 28, 2024
9. maflcko added this to the milestone 28.0 on Jul 3, 2024
10. maflcko removed this from the milestone 28.0 on Jul 4, 2024
11. 
    maflcko commented at 7:25 am on July 4, 2024: member

    Could also change it to [warn] when the user request could not be fulfilled? @ajtowns Did you see this? Seems rfm either way, but maybe address this or reply, so that maintainers don’t hold it back on this nit?

12. 
    ajtowns commented at 7:49 am on July 5, 2024: contributor

    Could also change it to [warn] when the user request could not be fulfilled?

    @ajtowns Did you see this? Seems rfm either way, but maybe address this or reply, so that maintainers don’t hold it back on this nit?

    Think it would make more sense to change that along with bumping other LogPrint{f,}() calls (eg, “WARNING SQLite is configured to not wait for data to be flushed…”, ). Also whether that should be [warn] seems up in the air at the moment: with #30347 or #30361 it might make more sense to have that be [error] rather than [warn]; or with #30364 it would presumably be [alert] instead.

13. 
    furszy commented at 12:38 pm on July 5, 2024: member

    ACK 46819f5df6de2a860c3ec87d55527b617a3b128f
14. luke-jr approved
15. 
    luke-jr commented at 5:08 pm on July 6, 2024: member

    utACK 46819f5df6de2a860c3ec87d55527b617a3b128f
16. ryanofsky merged this on Jul 8, 2024
17. ryanofsky closed this on Jul 8, 2024

    ---

Contributors
ajtowns DrahtBot maflcko ryanofsky furszy luke-jr

Labels
Wallet