From dca923150e3ac10a57c23a7e29e76516d32ec10d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 12 Aug 2024 12:36:49 +0200 Subject: [PATCH] Replace RPCNotifyBlockChange with waitTipChanged() This refactoring commit uses the newly introduced waitTipChanged mining interface method to replace the RPCNotifyBlockChange mechanism. --- src/init.cpp | 9 ----- src/rpc/blockchain.cpp | 84 ++++++++++++++++++++---------------------- src/rpc/blockchain.h | 3 -- 3 files changed, 39 insertions(+), 57 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 65575f26fcc..0a013d8ccb6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -429,16 +429,12 @@ static void registerSignalHandler(int signal, void(*handler)(int)) } #endif -static boost::signals2::connection rpc_notify_block_change_connection; static void OnRPCStarted() { - rpc_notify_block_change_connection = uiInterface.NotifyBlockTip_connect(std::bind(RPCNotifyBlockChange, std::placeholders::_2)); } static void OnRPCStopped() { - rpc_notify_block_change_connection.disconnect(); - RPCNotifyBlockChange(nullptr); g_best_block_cv.notify_all(); LogDebug(BCLog::RPC, "RPC stopped.\n"); } @@ -2011,11 +2007,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // cannot yet be called. Before we make it callable, we need to make sure // that the RPC's view of the best block is valid and consistent with // ChainstateManager's active tip. - // - // If we do not do this, RPC's view of the best block will be height=0 and - // hash=0x0. This will lead to erroroneous responses for things like - // waitforblockheight. - RPCNotifyBlockChange(WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip())); SetRPCWarmupFinished(); uiInterface.InitMessage(_("Done loading").translated); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 44334b7a390..683a9dae882 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -61,21 +62,12 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; +using interfaces::Mining; using node::BlockManager; using node::NodeContext; using node::SnapshotMetadata; using util::MakeUnorderedList; -struct CUpdatedBlock -{ - uint256 hash; - int height; -}; - -static GlobalMutex cs_blockchange; -static std::condition_variable cond_blockchange; -static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange); - std::tuple, CCoinsStats, const CBlockIndex*> PrepareUTXOSnapshot( Chainstate& chainstate, @@ -262,16 +254,6 @@ static RPCHelpMan getbestblockhash() }; } -void RPCNotifyBlockChange(const CBlockIndex* pindex) -{ - if(pindex) { - LOCK(cs_blockchange); - latestblock.hash = pindex->GetBlockHash(); - latestblock.height = pindex->nHeight; - } - cond_blockchange.notify_all(); -} - static RPCHelpMan waitfornewblock() { return RPCHelpMan{"waitfornewblock", @@ -298,16 +280,14 @@ static RPCHelpMan waitfornewblock() timeout = request.params[0].getInt(); if (timeout < 0) throw JSONRPCError(RPC_MISC_ERROR, "Negative timeout"); - CUpdatedBlock block; - { - WAIT_LOCK(cs_blockchange, lock); - block = latestblock; - if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); - else - cond_blockchange.wait(lock, [&block]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); - block = latestblock; + NodeContext& node = EnsureAnyNodeContext(request.context); + Mining& miner = EnsureMining(node); + + auto block{CHECK_NONFATAL(miner.getTip()).value()}; + if (IsRPCRunning()) { + block = timeout ? miner.waitTipChanged(block.hash, std::chrono::milliseconds(timeout)) : miner.waitTipChanged(block.hash); } + UniValue ret(UniValue::VOBJ); ret.pushKV("hash", block.hash.GetHex()); ret.pushKV("height", block.height); @@ -346,14 +326,20 @@ static RPCHelpMan waitforblock() timeout = request.params[1].getInt(); if (timeout < 0) throw JSONRPCError(RPC_MISC_ERROR, "Negative timeout"); - CUpdatedBlock block; - { - WAIT_LOCK(cs_blockchange, lock); - if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning();}); - else - cond_blockchange.wait(lock, [&hash]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.hash == hash || !IsRPCRunning(); }); - block = latestblock; + NodeContext& node = EnsureAnyNodeContext(request.context); + Mining& miner = EnsureMining(node); + + auto block{CHECK_NONFATAL(miner.getTip()).value()}; + const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout}; + while (IsRPCRunning() && block.hash != hash) { + if (timeout) { + auto now{std::chrono::steady_clock::now()}; + if (now >= deadline) break; + const MillisecondsDouble remaining{deadline - now}; + block = miner.waitTipChanged(block.hash, remaining); + } else { + block = miner.waitTipChanged(block.hash); + } } UniValue ret(UniValue::VOBJ); @@ -395,15 +381,23 @@ static RPCHelpMan waitforblockheight() timeout = request.params[1].getInt(); if (timeout < 0) throw JSONRPCError(RPC_MISC_ERROR, "Negative timeout"); - CUpdatedBlock block; - { - WAIT_LOCK(cs_blockchange, lock); - if(timeout) - cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning();}); - else - cond_blockchange.wait(lock, [&height]() EXCLUSIVE_LOCKS_REQUIRED(cs_blockchange) {return latestblock.height >= height || !IsRPCRunning(); }); - block = latestblock; + NodeContext& node = EnsureAnyNodeContext(request.context); + Mining& miner = EnsureMining(node); + + auto block{CHECK_NONFATAL(miner.getTip()).value()}; + const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout}; + + while (IsRPCRunning() && block.height < height) { + if (timeout) { + auto now{std::chrono::steady_clock::now()}; + if (now >= deadline) break; + const MillisecondsDouble remaining{deadline - now}; + block = miner.waitTipChanged(block.hash, remaining); + } else { + block = miner.waitTipChanged(block.hash); + } } + UniValue ret(UniValue::VOBJ); ret.pushKV("hash", block.hash.GetHex()); ret.pushKV("height", block.height); diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index a23f5cf024e..89b9921d559 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -35,9 +35,6 @@ static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5; */ double GetDifficulty(const CBlockIndex& blockindex); -/** Callback for when block tip changed. */ -void RPCNotifyBlockChange(const CBlockIndex*); - /** Block description to JSON */ UniValue blockToJSON(node::BlockManager& blockman, const CBlock& block, const CBlockIndex& tip, const CBlockIndex& blockindex, TxVerbosity verbosity) LOCKS_EXCLUDED(cs_main);