← Watch feed
validation: Move validation signal events to task runner
Public commit record
73/100 · Adequate
What the developer wrote
Authored by sedited
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 <stickies-v@protonmail.com>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code cleanup in Bitcoin Core's validation notification system. It reduces unnecessary copying of data when block and transaction events are passed between internal components, and it changes how debug log messages are formatted. There is no direct security vulnerability being fixed here; it is preparation for later commits that clarify object ownership.
Recommended action
No immediate security action required. Treat as a normal refactoring commit. Review the two follow-up commits referenced in the message to assess whether they introduce or fix any ownership-related safety issues.
Security signals we found
01
Refactoring only: no boundary checks, memory allocations, or cryptographic operations changed
02
No validation of untrusted input added or removed
03
No bug pattern such as use-after-free, double-free, or out-of-bounds access is addressed
04
Commit message frames change as preparation for ownership-semantics clarification in follow-up commits
Evidence from the diff
The change refactors the ENQUEUE_AND_LOG_EVENT macro in src/validationinterface.cpp so that the event lambda and its log message are passed as rvalue references and moved into the task runner lambda, rather than being captured by value twice. It also introduces a LOG_MSG macro that pre-formats the debug string only when validation debug logging is enabled. The commit explicitly states this is a performance/ownership cleanup in preparation for subsequent commits, not a security fix.
Changed components
src/validationinterface.cppValidationSignals event enqueueing macrosInspect captured patch +45 / −33
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index c7be6abc..3c142f8d 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<decltype((event))>, \
+ "event must be passed as an rvalue"); \
+ static_assert(std::is_rvalue_reference_v<decltype((log_msg))>, \
+ "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<const CBlock>& 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<RemovedMempoolTransactionInfo>& 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<const CBlock>& 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<const CBlock>& block, const BlockValidationState& state)
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.