Skip to content

Commit

Permalink
Merge #6436: refactor: Introduce LogAcceptDebug() which is `LogAcce…
Browse files Browse the repository at this point in the history
…ptCategory()` with `Debug` level

dfe86b4 fix: follow-up fixes (UdjinM6)
a254a7b refactor: sping LogAcceptCategory and LogAcceptDebug (Konstantin Akimov)
82238e6 refactor: Set log level in `LogAcceptCategory()` to `Debug` by default (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  #6399 introduced severity-based logging via b046e09. We use `LogAcceptCategory()` in quite a few places and it's always `BCLog::Level::Debug` so  log level is kind of redundant for us and just makes it harder to read the code.

  ## What was done?
  ~Set log level in `LogAcceptCategory()` to `BCLog::Level::Debug` by default~. Introduce `LogAcceptDebug()` which is `LogAcceptCategory()` with `Debug` level. Simplify corresponding Dash-specific code.

  ## How Has This Been Tested?

  ## Breaking Changes
  n/a

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  kwvg:
    utACK dfe86b4

Tree-SHA512: 167da533af088c4a3bfca22abd223a2314848ec79af10f117368f6d94a4a7faa3b009477a7af455ff890f8001622494c1e3a05112f9a7204321cc237278bf387
  • Loading branch information
PastaPastaPasta committed Dec 6, 2024
2 parents 1f58adc + dfe86b4 commit 65800cb
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 21 deletions.
6 changes: 3 additions & 3 deletions src/llmq/commitment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ bool CFinalCommitment::Verify(CDeterministicMNManager& dmnman, gsl::not_null<con
return false;
}
auto members = utils::GetAllQuorumMembers(llmqType, dmnman, pQuorumBaseBlockIndex);
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
std::stringstream ss2;
for (const auto i: irange::range(llmq_params.size)) {
Expand All @@ -106,7 +106,7 @@ bool CFinalCommitment::Verify(CDeterministicMNManager& dmnman, gsl::not_null<con
// sigs are only checked when the block is processed
if (checkSigs) {
uint256 commitmentHash = BuildCommitmentHash(llmq_params.type, quorumHash, validMembers, quorumPublicKey, quorumVvecHash);
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss3;
for (const auto &mn: members) {
ss3 << mn->proTxHash.ToString().substr(0, 4) << " | ";
Expand Down Expand Up @@ -181,7 +181,7 @@ bool CheckLLMQCommitment(CDeterministicMNManager& dmnman, const ChainstateManage
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-qc-commitment-type");
}

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
for (const auto i: irange::range(llmq_params_opt->size)) {
ss << "v[" << i << "]=" << qcTx.commitment.validMembers[i];
Expand Down
4 changes: 2 additions & 2 deletions src/llmq/dkgsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex)

CDKGLogger logger(*this, __func__, __LINE__);

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug) && IsQuorumRotationEnabled(params, m_quorum_base_block_index)) {
if (LogAcceptDebug(BCLog::LLMQ) && IsQuorumRotationEnabled(params, m_quorum_base_block_index)) {
int cycleQuorumBaseHeight = m_quorum_base_block_index->nHeight - quorumIndex;
const CBlockIndex* pCycleQuorumBaseBlockIndex = m_quorum_base_block_index->GetAncestor(cycleQuorumBaseHeight);
std::stringstream ss;
Expand All @@ -138,7 +138,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex)
if (!myProTxHash.IsNull()) {
dkgDebugManager.InitLocalSessionStatus(params, quorumIndex, m_quorum_base_block_index->GetBlockHash(), m_quorum_base_block_index->nHeight);
relayMembers = utils::GetQuorumRelayMembers(params, m_dmnman, m_quorum_base_block_index, myProTxHash, true);
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
for (const auto& r : relayMembers) {
ss << r.ToString().substr(0, 4) << " | ";
Expand Down
4 changes: 2 additions & 2 deletions src/llmq/instantsend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& re
return;
}

if (LogAcceptCategory(BCLog::INSTANTSEND, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::INSTANTSEND)) {
for (const auto& in : tx->vin) {
auto id = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in.prevout));
if (id == recoveredSig.getId()) {
Expand Down Expand Up @@ -1469,7 +1469,7 @@ void CInstantSendManager::ProcessPendingRetryLockTxs()

// CheckCanLock is already called by ProcessTx, so we should avoid calling it twice. But we also shouldn't spam
// the logs when retrying TXs that are not ready yet.
if (LogAcceptCategory(BCLog::INSTANTSEND, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::INSTANTSEND)) {
if (!CheckCanLock(*tx, false, Params().GetConsensus())) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing_shares.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ void CSigSharesManager::Cleanup()
const auto& oneSigShare = m->begin()->second;

std::string strMissingMembers;
if (LogAcceptCategory(BCLog::LLMQ_SIGS, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ_SIGS)) {
if (const auto quorumIt = quorums.find(std::make_pair(oneSigShare.getLlmqType(), oneSigShare.getQuorumHash())); quorumIt != quorums.end()) {
const auto& quorum = quorumIt->second;
for (const auto i : irange::range(quorum->members.size())) {
Expand Down
12 changes: 6 additions & 6 deletions src/llmq/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
//TODO Check if it is triggered from outside (P2P, block validation). Throwing an exception is probably a wiser choice
//assert (!newQuarterMembers.empty());

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
for (const size_t i : irange::range(nQuorums)) {
std::stringstream ss;

Expand Down Expand Up @@ -249,7 +249,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
std::move(previousQuarters.quarterHMinusC[i].begin(), previousQuarters.quarterHMinusC[i].end(), std::back_inserter(quorumMembers[i]));
std::move(newQuarterMembers[i].begin(), newQuarterMembers[i].end(), std::back_inserter(quorumMembers[i]));

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
ss << " [";
for (const auto &m: quorumMembers[i]) {
Expand Down Expand Up @@ -397,7 +397,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(cons
sortedCombinedMnsList.push_back(std::move(m));
}

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
ss << " [";
for (const auto &m: sortedCombinedMnsList) {
Expand Down Expand Up @@ -517,7 +517,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
std::move(sortedMnsUsedAtH.begin(), sortedMnsUsedAtH.end(), std::back_inserter(sortedCombinedMns));
}

if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::stringstream ss;
ss << " [";
for (const auto &m: sortedCombinedMns) {
Expand Down Expand Up @@ -789,7 +789,7 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman&
}
if (!connections.empty()) {
if (!connman.HasMasternodeQuorumNodes(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash()) &&
LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
LogAcceptDebug(BCLog::LLMQ)) {
std::string debugMsg = strprintf("%s -- adding masternodes quorum connections for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString());
for (const auto& c : connections) {
auto dmn = tip_mn_list.GetValidMN(c);
Expand Down Expand Up @@ -836,7 +836,7 @@ void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, CConnman
}

if (!probeConnections.empty()) {
if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::LLMQ)) {
std::string debugMsg = strprintf("%s -- adding masternodes probes for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString());
for (const auto& c : probeConnections) {
auto dmn = tip_mn_list.GetValidMN(c);
Expand Down
6 changes: 6 additions & 0 deletions src/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,12 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
return LogInstance().WillLogCategoryLevel(category, level);
}

/** Return true if log accepts specified category, at the debug level. */
static inline bool LogAcceptDebug(BCLog::LogFlags category)
{
return LogAcceptCategory(category, BCLog::Level::Debug);
}

/** Return true if str parses as a log category and set the flag */
bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str);

Expand Down
2 changes: 1 addition & 1 deletion src/qt/coincontroldialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ void CoinControlDialog::updateView()

// CoinJoin rounds
int nRounds = model->getRealOutpointCoinJoinRounds(output);
if (nRounds >= 0 || LogAcceptCategory(BCLog::COINJOIN, BCLog::Level::Debug)) {
if (nRounds >= 0 || LogAcceptDebug(BCLog::COINJOIN)) {
itemOutput->setText(COLUMN_COINJOIN_ROUNDS, QString::number(nRounds));
} else {
itemOutput->setText(COLUMN_COINJOIN_ROUNDS, tr("n/a"));
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3432,7 +3432,7 @@ std::vector<CompactTallyItem> CWallet::SelectCoinsGroupedByAddresses(bool fSkipD
}

// debug
if (LogAcceptCategory(BCLog::SELECTCOINS, BCLog::Level::Debug)) {
if (LogAcceptDebug(BCLog::SELECTCOINS)) {
std::string strMessage = "SelectCoinsGroupedByAddresses - vecTallyRet:\n";
for (const auto& item : vecTallyRet)
strMessage += strprintf(" %s %f\n", EncodeDestination(item.txdest), float(item.nAmount)/COIN);
Expand Down
10 changes: 5 additions & 5 deletions src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ extern const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS;

// Use a macro instead of a function for conditional logging to prevent
// evaluating arguments when logging for the category is not enabled.
#define WalletCJLogPrint(wallet, ...) \
do { \
if (LogAcceptCategory(BCLog::COINJOIN, BCLog::Level::Debug)) { \
wallet->WalletLogPrintf(__VA_ARGS__); \
} \
#define WalletCJLogPrint(wallet, ...) \
do { \
if (LogAcceptDebug(BCLog::COINJOIN)) { \
wallet->WalletLogPrintf(__VA_ARGS__); \
} \
} while (0)

/** A wrapper to reserve an address from a wallet
Expand Down

0 comments on commit 65800cb

Please sign in to comment.