Use log categories when logging events that P2P peers can trigger arbitrarily.
Rationale similar to that of PR #17762 (net: Log to net category for exceptions in ProcessMessages):
It is not good to panick end users with verbose errors (let alone writing to stderr) when any peer can generate them.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
51@@ -52,6 +52,13 @@ bool error(const char* fmt, const Args&... args)
52 return false;
53 }
54
55+template<typename... Args>
56+bool error_debug(const BCLog::LogFlags& category, const char* fmt, const Args&... args)
error_debug() looks strange to me.
bool error(…). Perhaps bool error_with_category(…) or bool error_with_debug(…)?
Now using error_with_debug_log(…) instead of error_debug(…).
Please re-review :)
3814@@ -3815,7 +3815,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<cons
3815 }
3816 if (!ret) {
3817 GetMainSignals().BlockChecked(*pblock, state);
3818- return error("%s: AcceptBlock FAILED (%s)", __func__, state.ToString());
3819+ return error_with_debug_log(BCLog::VALIDATION, "%s: AcceptBlock FAILED (%s)", __func__, state.ToString());
BCLog::VALIDATION is used for the validation interface. Is the same category applicable here?
I’d be happy to change but I’m not sure to which TBH: which BCLog category would be appropriate in these cases?
FWIW, these are the location of BCLog uses in current master:
0$ git grep -E '^ *LogPrint\(BCLog::' | tr -d " " | cut -f1 -d, | \
1 sed 's/:LogPrint(/ /g' | sort -u | awk '{ print $2 " " $1 }' | sort -u
2BCLog::ADDRMAN src/addrman.cpp
3BCLog::ADDRMAN src/addrman.h
4BCLog::BENCH src/miner.cpp
5BCLog::BENCH src/validation.cpp
6BCLog::CMPCTBLOCK src/blockencodings.cpp
7BCLog::COINDB src/txdb.cpp
8BCLog::ESTIMATEFEE src/policy/fees.cpp
9BCLog::HTTP src/httpserver.cpp
10BCLog::LEVELDB src/dbwrapper.cpp
11BCLog::LIBEVENT src/httpserver.cpp
12BCLog::MEMPOOLREJ src/net_processing.cpp
13BCLog::MEMPOOL src/net_processing.cpp
14BCLog::MEMPOOL src/txmempool.cpp
15BCLog::MEMPOOL src/validation.cpp
16BCLog::NET src/addrman.cpp
17BCLog::NET src/banman.cpp
18BCLog::NET src/netbase.cpp
19BCLog::NET src/net.cpp
20BCLog::NET src/net_processing.cpp
21BCLog::NET src/timedata.cpp
22BCLog::PROXY src/netbase.cpp
23BCLog::PRUNE src/validation.cpp
24BCLog::QT src/qt/bitcoin.cpp
25BCLog::RAND src/random.cpp
26BCLog::REINDEX src/validation.cpp
27BCLog::RPC src/httprpc.cpp
28BCLog::RPC src/init.cpp
29BCLog::RPC src/rpc/blockchain.cpp
30BCLog::RPC src/rpc/request.cpp
31BCLog::RPC src/rpc/server.cpp
32BCLog::SELECTCOINS src/wallet/coinselection.cpp
33BCLog::TOR src/torcontrol.cpp
34BCLog::VALIDATION src/validation.cpp
35BCLog::VALIDATION src/validationinterface.cpp
36BCLog::WALLETDB src/wallet/db.cpp
37BCLog::WALLETDB src/wallet/walletdb.cpp
38BCLog::ZMQ src/zmq/zmqnotificationinterface.cpp
39BCLog::ZMQ src/zmq/zmqpublishnotifier.cpp
BCLog::VALIDATION.
I think there is a tradeoff. IIRC -debug is disabled by default, disabling all debug categories. Thus, the debug log is pretty much useless to find the root cause of an issue after a crash or other misbehavior if all log statements are put into a category. Still, it makes sense to put log statments into categories to make it possible to disable or enable them specifically.
I think we should rethink what categories are enabled/disabled by default and come up with a broader guideline on debug categories.
-debug?
