From 8b0fb64c0243fa06d97a394dfb4500f97ef05f7b Mon Sep 17 00:00:00 2001 From: sedited Date: Mon, 2 Mar 2026 09:42:18 +0100 Subject: [PATCH] validation: Move validation signal events to task runner Currently arguments passed through the validation interface are copied three times. Once on capture in the event, again when the event itself is copied into the task runner lambda, and when the various arguments used by the logging statement are copied into the lambda. This change avoids the variables captured by the event being copied again. Next to avoiding needless copies, this is done in preparation of the following two commits, which seek to clarify the ownership semantics of the blocks passed through the validation interface. Co-authored-by: stickies-v --- src/validationinterface.cpp | 78 +++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index c7be6abc3a1..3c142f8db28 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -156,33 +156,39 @@ void ValidationSignals::SyncWithValidationInterfaceQueue() // Use a macro instead of a function for conditional logging to prevent // evaluating arguments when logging is not enabled. -// -// NOTE: The lambda captures all local variables by value. -#define ENQUEUE_AND_LOG_EVENT(event, fmt, name, ...) \ - do { \ - auto local_name = (name); \ - LOG_EVENT("Enqueuing " fmt, local_name, __VA_ARGS__); \ - m_internals->m_task_runner->insert([=] { \ - LOG_EVENT(fmt, local_name, __VA_ARGS__); \ - event(); \ - }); \ +#define ENQUEUE_AND_LOG_EVENT(event, log_msg) \ + do { \ + static_assert(std::is_rvalue_reference_v, \ + "event must be passed as an rvalue"); \ + static_assert(std::is_rvalue_reference_v, \ + "log_msg must be passed as an rvalue"); \ + auto enqueue_log_msg = (log_msg); \ + LOG_EVENT("Enqueuing %s", enqueue_log_msg); \ + m_internals->m_task_runner->insert([local_log_msg = std::move(enqueue_log_msg), local_event = (event)] { \ + LOG_EVENT("%s", local_log_msg); \ + local_event(); \ + }); \ } while (0) +#define LOG_MSG(fmt, ...) \ + (ShouldLog(BCLog::VALIDATION, BCLog::Level::Debug) ? tfm::format((fmt), __VA_ARGS__) : std::string{}) + #define LOG_EVENT(fmt, ...) \ - LogDebug(BCLog::VALIDATION, fmt "\n", __VA_ARGS__) + LogDebug(BCLog::VALIDATION, fmt, __VA_ARGS__) void ValidationSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { // Dependencies exist that require UpdatedBlockTip events to be delivered in the order in which // the chain actually updates. One way to ensure this is for the caller to invoke this signal // in the same critical section where the chain is updated - auto event = [pindexNew, pindexFork, fInitialDownload, this] { - m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); }); - }; - ENQUEUE_AND_LOG_EVENT(event, "%s: new block hash=%s fork block hash=%s (in IBD=%s)", __func__, + auto log_msg = LOG_MSG("%s: new block hash=%s fork block hash=%s (in IBD=%s)", __func__, pindexNew->GetBlockHash().ToString(), pindexFork ? pindexFork->GetBlockHash().ToString() : "null", fInitialDownload); + auto event = [pindexNew, pindexFork, fInitialDownload, this] { + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); }); + }; + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::ActiveTipChange(const CBlockIndex& new_tip, bool is_ibd) @@ -193,61 +199,67 @@ void ValidationSignals::ActiveTipChange(const CBlockIndex& new_tip, bool is_ibd) void ValidationSignals::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) { + auto log_msg = LOG_MSG("%s: txid=%s wtxid=%s", __func__, + tx.info.m_tx->GetHash().ToString(), + tx.info.m_tx->GetWitnessHash().ToString()); auto event = [tx, mempool_sequence, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(tx, mempool_sequence); }); }; - ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__, - tx.info.m_tx->GetHash().ToString(), - tx.info.m_tx->GetWitnessHash().ToString()); + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) { - auto event = [tx, reason, mempool_sequence, this] { - m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason, mempool_sequence); }); - }; - ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s reason=%s", __func__, + auto log_msg = LOG_MSG("%s: txid=%s wtxid=%s reason=%s", __func__, tx->GetHash().ToString(), tx->GetWitnessHash().ToString(), RemovalReasonToString(reason)); + auto event = [tx, reason, mempool_sequence, this] { + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason, mempool_sequence); }); + }; + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::BlockConnected(const ChainstateRole& role, const std::shared_ptr& pblock, const CBlockIndex* pindex) { + auto log_msg = LOG_MSG("%s: block hash=%s block height=%d", __func__, + pblock->GetHash().ToString(), + pindex->nHeight); auto event = [role, pblock, pindex, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockConnected(role, pblock, pindex); }); }; - ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__, - pblock->GetHash().ToString(), - pindex->nHeight); + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::MempoolTransactionsRemovedForBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) { + auto log_msg = LOG_MSG("%s: block height=%s txs removed=%s", __func__, + nBlockHeight, + txs_removed_for_block.size()); auto event = [txs_removed_for_block, nBlockHeight, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForBlock(txs_removed_for_block, nBlockHeight); }); }; - ENQUEUE_AND_LOG_EVENT(event, "%s: block height=%s txs removed=%s", __func__, - nBlockHeight, - txs_removed_for_block.size()); + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) { + auto log_msg = LOG_MSG("%s: block hash=%s block height=%d", __func__, + pblock->GetHash().ToString(), + pindex->nHeight); auto event = [pblock, pindex, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockDisconnected(pblock, pindex); }); }; - ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__, - pblock->GetHash().ToString(), - pindex->nHeight); + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::ChainStateFlushed(const ChainstateRole& role, const CBlockLocator& locator) { + auto log_msg = LOG_MSG("%s: block hash=%s", __func__, + locator.IsNull() ? "null" : locator.vHave.front().ToString()); auto event = [role, locator, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.ChainStateFlushed(role, locator); }); }; - ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s", __func__, - locator.IsNull() ? "null" : locator.vHave.front().ToString()); + ENQUEUE_AND_LOG_EVENT(std::move(event), std::move(log_msg)); } void ValidationSignals::BlockChecked(const std::shared_ptr& block, const BlockValidationState& state)