From 1d9f1cb4bd6b119e1d56cbdd7f6ce4d4521fffa3 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Mon, 28 Jul 2025 13:57:52 +0100 Subject: [PATCH] kernel: improve BlockChecked ownership semantics Subscribers to the BlockChecked validation interface event may need access to the block outside of the callback scope. Currently, this is only possible by copying the block, which makes exposing this validation interface event publicly either cumbersome or with significant copy overhead. By using shared_ptr, we make the shared ownership explicit and allow users to safely use the block outside of the callback scope. --- src/bitcoin-chainstate.cpp | 4 ++-- src/net_processing.cpp | 6 +++--- src/rpc/mining.cpp | 6 +++--- src/test/util/mining.cpp | 5 +++-- src/test/validationinterface_tests.cpp | 11 +++++------ src/validation.cpp | 4 ++-- src/validationinterface.cpp | 6 ++++-- src/validationinterface.h | 4 ++-- 8 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 47ddfaad8d..4ac6a6f41a 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -200,9 +200,9 @@ int main(int argc, char* argv[]) explicit submitblock_StateCatcher(const uint256& hashIn) : hash(hashIn), found(false), state() {} protected: - void BlockChecked(const CBlock& block, const BlockValidationState& stateIn) override + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& stateIn) override { - if (block.GetHash() != hash) + if (block->GetHash() != hash) return; found = true; state = stateIn; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0c4a89c44c..e5c2a86d42 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -513,7 +513,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void BlockChecked(const CBlock& block, const BlockValidationState& state) override + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) override EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); @@ -2103,11 +2103,11 @@ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlock * Handle invalid block rejection and consequent peer discouragement, maintain which * peers announce compact blocks. */ -void PeerManagerImpl::BlockChecked(const CBlock& block, const BlockValidationState& state) +void PeerManagerImpl::BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) { LOCK(cs_main); - const uint256 hash(block.GetHash()); + const uint256 hash(block->GetHash()); std::map>::iterator it = mapBlockSource.find(hash); // If the block failed validation, we know where it came from and we're still connected diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f11305f16b..8b45a8c129 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1030,9 +1030,9 @@ public: explicit submitblock_StateCatcher(const uint256 &hashIn) : hash(hashIn), state() {} protected: - void BlockChecked(const CBlock& block, const BlockValidationState& stateIn) override { - if (block.GetHash() != hash) - return; + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& stateIn) override + { + if (block->GetHash() != hash) return; found = true; state = stateIn; } diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 1cd8f8ee71..0dacf71b68 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -18,6 +18,7 @@ #include #include +#include using node::BlockAssembler; using node::NodeContext; @@ -83,9 +84,9 @@ struct BlockValidationStateCatcher : public CValidationInterface { m_state{} {} protected: - void BlockChecked(const CBlock& block, const BlockValidationState& state) override + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) override { - if (block.GetHash() != m_hash) return; + if (block->GetHash() != m_hash) return; m_state = state; } }; diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp index a46cfc3029..8a24b28245 100644 --- a/src/test/validationinterface_tests.cpp +++ b/src/test/validationinterface_tests.cpp @@ -12,11 +12,12 @@ #include #include +#include BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, ChainTestingSetup) struct TestSubscriberNoop final : public CValidationInterface { - void BlockChecked(const CBlock&, const BlockValidationState&) override {} + void BlockChecked(const std::shared_ptr&, const BlockValidationState&) override {} }; BOOST_AUTO_TEST_CASE(unregister_validation_interface_race) @@ -25,10 +26,9 @@ BOOST_AUTO_TEST_CASE(unregister_validation_interface_race) // Start thread to generate notifications std::thread gen{[&] { - const CBlock block_dummy; BlockValidationState state_dummy; while (generate) { - m_node.validation_signals->BlockChecked(block_dummy, state_dummy); + m_node.validation_signals->BlockChecked(std::make_shared(), state_dummy); } }}; @@ -60,15 +60,14 @@ public: { if (m_on_destroy) m_on_destroy(); } - void BlockChecked(const CBlock& block, const BlockValidationState& state) override + void BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) override { if (m_on_call) m_on_call(); } void Call() { - CBlock block; BlockValidationState state; - m_signals.BlockChecked(block, state); + m_signals.BlockChecked(std::make_shared(), state); } std::function m_on_call; std::function m_on_destroy; diff --git a/src/validation.cpp b/src/validation.cpp index e339fa774f..bd2963b496 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3178,7 +3178,7 @@ bool Chainstate::ConnectTip( CCoinsViewCache view(&CoinsTip()); bool rv = ConnectBlock(*block_to_connect, state, pindexNew, view); if (m_chainman.m_options.signals) { - m_chainman.m_options.signals->BlockChecked(*block_to_connect, state); + m_chainman.m_options.signals->BlockChecked(block_to_connect, state); } if (!rv) { if (state.IsInvalid()) @@ -4554,7 +4554,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& blo } if (!ret) { if (m_options.signals) { - m_options.signals->BlockChecked(*block, state); + m_options.signals->BlockChecked(block, state); } LogError("%s: AcceptBlock FAILED (%s)\n", __func__, state.ToString()); return false; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index f1ac9305ff..313f730ea0 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -245,9 +246,10 @@ void ValidationSignals::ChainStateFlushed(ChainstateRole role, const CBlockLocat locator.IsNull() ? "null" : locator.vHave.front().ToString()); } -void ValidationSignals::BlockChecked(const CBlock& block, const BlockValidationState& state) { +void ValidationSignals::BlockChecked(const std::shared_ptr& block, const BlockValidationState& state) +{ LOG_EVENT("%s: block hash=%s state=%s", __func__, - block.GetHash().ToString(), state.ToString()); + block->GetHash().ToString(), state.ToString()); m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockChecked(block, state); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 5a50876809..e0a88ad0f2 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -150,7 +150,7 @@ protected: * is guaranteed to be the current best block at the time the * callback was generated (not necessarily now). */ - virtual void BlockChecked(const CBlock&, const BlockValidationState&) {} + virtual void BlockChecked(const std::shared_ptr&, const BlockValidationState&) {} /** * Notifies listeners that a block which builds directly on our current tip * has been received and connected to the headers tree, though not validated yet. @@ -224,7 +224,7 @@ public: void BlockConnected(ChainstateRole, const std::shared_ptr &, const CBlockIndex *pindex); void BlockDisconnected(const std::shared_ptr &, const CBlockIndex* pindex); void ChainStateFlushed(ChainstateRole, const CBlockLocator &); - void BlockChecked(const CBlock&, const BlockValidationState&); + void BlockChecked(const std::shared_ptr&, const BlockValidationState&); void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr&); };