diff --git a/src/index/base.cpp b/src/index/base.cpp index 6f9860415f3..810358394a1 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -113,7 +113,7 @@ bool BaseIndex::Init() // Child init const CBlockIndex* start_block = m_best_block_index.load(); - if (!CustomInit(start_block ? std::make_optional(interfaces::BlockKey{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) { + if (!CustomInit(start_block ? std::make_optional(interfaces::BlockRef{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) { return false; } diff --git a/src/index/base.h b/src/index/base.h index beb1575ab21..fbd9069a515 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -107,7 +108,7 @@ protected: void ChainStateFlushed(ChainstateRole role, const CBlockLocator& locator) override; /// Initialize internal state from the database and block index. - [[nodiscard]] virtual bool CustomInit(const std::optional& block) { return true; } + [[nodiscard]] virtual bool CustomInit(const std::optional& block) { return true; } /// Write update index entries for a newly connected block. [[nodiscard]] virtual bool CustomAppend(const interfaces::BlockInfo& block) { return true; } @@ -118,7 +119,7 @@ protected: /// Rewind index to an earlier chain tip during a chain reorg. The tip must /// be an ancestor of the current best block. - [[nodiscard]] virtual bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) { return true; } + [[nodiscard]] virtual bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) { return true; } virtual DB& GetDB() const = 0; diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 26de7eee32f..a808cc90850 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -112,7 +112,7 @@ BlockFilterIndex::BlockFilterIndex(std::unique_ptr chain, Blo m_filter_fileseq = std::make_unique(std::move(path), "fltr", FLTR_FILE_CHUNK_SIZE); } -bool BlockFilterIndex::CustomInit(const std::optional& block) +bool BlockFilterIndex::CustomInit(const std::optional& block) { if (!m_db->Read(DB_FILTER_POS, m_next_filter_pos)) { // Check that the cause of the read failure is that the key does not exist. Any other errors @@ -316,7 +316,7 @@ bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, c return true; } -bool BlockFilterIndex::CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) +bool BlockFilterIndex::CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) { CDBBatch batch(*m_db); std::unique_ptr db_it(m_db->NewIterator()); diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index cdb9563fb8e..ccb4845ef5e 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -52,13 +52,13 @@ private: std::optional ReadFilterHeader(int height, const uint256& expected_block_hash); protected: - bool CustomInit(const std::optional& block) override; + bool CustomInit(const std::optional& block) override; bool CustomCommit(CDBBatch& batch) override; bool CustomAppend(const interfaces::BlockInfo& block) override; - bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) override; + bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) override; BaseIndex::DB& GetDB() const LIFETIMEBOUND override { return *m_db; } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index dff8e50a4e5..c950a18f3f8 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -265,7 +265,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) return true; } -bool CoinStatsIndex::CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) +bool CoinStatsIndex::CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) { CDBBatch batch(*m_db); std::unique_ptr db_it(m_db->NewIterator()); @@ -304,7 +304,7 @@ bool CoinStatsIndex::CustomRewind(const interfaces::BlockKey& current_tip, const return true; } -static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockKey& block, DBVal& result) +static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockRef& block, DBVal& result) { // First check if the result is stored under the height index and the value // there matches the block hash. This should be the case if the block is on @@ -350,7 +350,7 @@ std::optional CoinStatsIndex::LookUpStats(const CBlockIndex& block_ return stats; } -bool CoinStatsIndex::CustomInit(const std::optional& block) +bool CoinStatsIndex::CustomInit(const std::optional& block) { if (!m_db->Read(DB_MUHASH, m_muhash)) { // Check that the cause of the read failure is that the key does not diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index d6322bfa7cf..885b9e0a860 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -43,13 +43,13 @@ private: bool AllowPrune() const override { return true; } protected: - bool CustomInit(const std::optional& block) override; + bool CustomInit(const std::optional& block) override; bool CustomCommit(CDBBatch& batch) override; bool CustomAppend(const interfaces::BlockInfo& block) override; - bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) override; + bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) override; BaseIndex::DB& GetDB() const override { return *m_db; } diff --git a/src/init.cpp b/src/init.cpp index 8fbdb880bc9..2a301e114ee 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -284,7 +284,7 @@ void Shutdown(NodeContext& node) StopHTTPRPC(); StopREST(); - StopRPC(); + StopRPC(&node); StopHTTPServer(); for (const auto& client : node.chain_clients) { client->flush(); @@ -429,20 +429,6 @@ 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"); -} - void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) { SetupHelpOptions(argsman); @@ -723,8 +709,6 @@ static void StartupNotify(const ArgsManager& args) static bool AppInitServers(NodeContext& node) { const ArgsManager& args = *Assert(node.args); - RPCServer::OnStarted(&OnRPCStarted); - RPCServer::OnStopped(&OnRPCStopped); if (!InitHTTPServer(*Assert(node.shutdown))) { return false; } @@ -2017,11 +2001,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/interfaces/chain.h b/src/interfaces/chain.h index 7bc2d11b607..4e858d1f898 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -41,12 +41,6 @@ namespace interfaces { class Handler; class Wallet; -//! Hash/height pair to help track and identify blocks. -struct BlockKey { - uint256 hash; - int height = -1; -}; - //! Helper for findBlock to selectively return pieces of block data. If block is //! found, data will be returned by setting specified output variables. If block //! is not found, output variables will keep their previous values. diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index c73465bd2aa..421c5ba762f 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -6,11 +6,13 @@ #define BITCOIN_INTERFACES_MINING_H #include // for CAmount +#include // for BlockRef #include // for BlockCreateOptions #include // for CBlock, CBlockHeader #include // for CTransactionRef #include // for int64_t #include // for uint256 +#include // for MillisecondsDouble #include // for unique_ptr, shared_ptr #include // for optional @@ -55,10 +57,21 @@ public: //! Returns whether IBD is still in progress. virtual bool isInitialBlockDownload() = 0; - //! Returns the hash for the tip of this chain - virtual std::optional getTipHash() = 0; + //! Returns the hash and height for the tip of this chain + virtual std::optional getTip() = 0; /** + * Waits for the tip to change + * + * @param[in] current_tip block hash of the current chain tip. Function waits + * for the chain tip to change if this matches, otherwise + * it returns right away. + * @param[in] timeout how long to wait for a new tip + * @returns Hash and height of the current chain tip after this call. + */ + virtual BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0; + + /** * Construct a new block template * * @param[in] script_pub_key the coinbase output diff --git a/src/interfaces/types.h b/src/interfaces/types.h new file mode 100644 index 00000000000..e5edd301a71 --- /dev/null +++ b/src/interfaces/types.h @@ -0,0 +1,20 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_INTERFACES_TYPES_H +#define BITCOIN_INTERFACES_TYPES_H + +#include + +namespace interfaces { + +//! Hash/height pair to help track and identify blocks. +struct BlockRef { + uint256 hash; + int height = -1; +}; + +} // namespace interfaces + +#endif // BITCOIN_INTERFACES_TYPES_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 0e418c58bb9..1fedcd6cfa8 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -67,6 +69,7 @@ #include +using interfaces::BlockRef; using interfaces::BlockTemplate; using interfaces::BlockTip; using interfaces::Chain; @@ -137,7 +140,7 @@ public: // Stop RPC for clean shutdown if any of waitfor* commands is executed. if (args().GetBoolArg("-server", false)) { InterruptRPC(); - StopRPC(); + StopRPC(m_context); } } bool shutdownRequested() override { return ShutdownRequested(*Assert(m_context)); }; @@ -925,12 +928,33 @@ public: return chainman().IsInitialBlockDownload(); } - std::optional getTipHash() override + std::optional getTip() override { LOCK(::cs_main); CBlockIndex* tip{chainman().ActiveChain().Tip()}; if (!tip) return {}; - return tip->GetBlockHash(); + return BlockRef{tip->GetBlockHash(), tip->nHeight}; + } + + BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override + { + // Interrupt check interval + const MillisecondsDouble tick{1000}; + auto now{std::chrono::steady_clock::now()}; + auto deadline = now + timeout; + // std::chrono does not check against overflow + if (deadline < now) deadline = std::chrono::steady_clock::time_point::max(); + { + WAIT_LOCK(notifications().m_tip_block_mutex, lock); + while ((notifications().m_tip_block == uint256() || notifications().m_tip_block == current_tip) && !chainman().m_interrupt) { + now = std::chrono::steady_clock::now(); + if (now >= deadline) break; + notifications().m_tip_block_cv.wait_until(lock, std::min(deadline, now + tick)); + } + } + // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. + LOCK(::cs_main); + return BlockRef{chainman().ActiveChain().Tip()->GetBlockHash(), chainman().ActiveChain().Tip()->nHeight}; } bool processNewBlock(const std::shared_ptr& block, bool* new_block) override @@ -965,6 +989,7 @@ public: NodeContext* context() override { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } + KernelNotifications& notifications() { return *Assert(m_node.notifications); } NodeContext& m_node; }; } // namespace diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 9894052a3a0..40d45c8c2b8 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -50,6 +50,12 @@ namespace node { kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state, CBlockIndex& index) { + { + LOCK(m_tip_block_mutex); + m_tip_block = index.GetBlockHash(); + m_tip_block_cv.notify_all(); + } + uiInterface.NotifyBlockTip(state, &index); if (m_stop_at_height && index.nHeight >= m_stop_at_height) { if (!m_shutdown()) { diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index e37f4d4e1e4..9ff980ea56d 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -7,6 +7,10 @@ #include +#include +#include +#include + #include #include @@ -34,7 +38,7 @@ public: KernelNotifications(util::SignalInterrupt& shutdown, std::atomic& exit_status, node::Warnings& warnings) : m_shutdown(shutdown), m_exit_status{exit_status}, m_warnings{warnings} {} - [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override; + [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override EXCLUSIVE_LOCKS_REQUIRED(!m_tip_block_mutex); void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) override; @@ -52,6 +56,12 @@ public: int m_stop_at_height{DEFAULT_STOPATHEIGHT}; //! Useful for tests, can be set to false to avoid shutdown on fatal error. bool m_shutdown_on_fatal_error{true}; + + Mutex m_tip_block_mutex; + std::condition_variable m_tip_block_cv; + //! The block for which the last blockTip notification was received for. + uint256 m_tip_block; + private: util::SignalInterrupt& m_shutdown; std::atomic& m_exit_status; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a9cea44779d..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,21 +254,12 @@ 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", - "\nWaits for a specific new block and returns useful info about it.\n" - "\nReturns the current block on timeout or exit.\n", + "\nWaits for any new block and returns useful info about it.\n" + "\nReturns the current block on timeout or exit.\n" + "\nMake sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)", { {"timeout", RPCArg::Type::NUM, RPCArg::Default{0}, "Time in milliseconds to wait for a response. 0 indicates no timeout."}, }, @@ -295,17 +278,16 @@ static RPCHelpMan waitfornewblock() int timeout = 0; if (!request.params[0].isNull()) 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); @@ -318,7 +300,8 @@ static RPCHelpMan waitforblock() { return RPCHelpMan{"waitforblock", "\nWaits for a specific new block and returns useful info about it.\n" - "\nReturns the current block on timeout or exit.\n", + "\nReturns the current block on timeout or exit.\n" + "\nMake sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)", { {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "Block hash to wait for."}, {"timeout", RPCArg::Type::NUM, RPCArg::Default{0}, "Time in milliseconds to wait for a response. 0 indicates no timeout."}, @@ -341,15 +324,22 @@ static RPCHelpMan waitforblock() if (!request.params[1].isNull()) 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); @@ -365,7 +355,8 @@ static RPCHelpMan waitforblockheight() return RPCHelpMan{"waitforblockheight", "\nWaits for (at least) block height and returns the height and hash\n" "of the current tip.\n" - "\nReturns the current block on timeout or exit.\n", + "\nReturns the current block on timeout or exit.\n" + "\nMake sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)", { {"height", RPCArg::Type::NUM, RPCArg::Optional::NO, "Block height to wait for."}, {"timeout", RPCArg::Type::NUM, RPCArg::Default{0}, "Time in milliseconds to wait for a response. 0 indicates no timeout."}, @@ -388,16 +379,25 @@ static RPCHelpMan waitforblockheight() if (!request.params[1].isNull()) 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); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b986c26e7a3..6270496b1bc 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -661,7 +661,7 @@ static RPCHelpMan getblocktemplate() ChainstateManager& chainman = EnsureChainman(node); Mining& miner = EnsureMining(node); LOCK(cs_main); - uint256 tip{CHECK_NONFATAL(miner.getTipHash()).value()}; + uint256 tip{CHECK_NONFATAL(miner.getTip()).value().hash}; std::string strMode = "template"; UniValue lpval = NullUniValue; @@ -738,7 +738,6 @@ static RPCHelpMan getblocktemplate() { // Wait to respond until either the best block changes, OR a minute has passed and there are more transactions uint256 hashWatchedChain; - std::chrono::steady_clock::time_point checktxtime; unsigned int nTransactionsUpdatedLastLP; if (lpval.isStr()) @@ -759,24 +758,19 @@ static RPCHelpMan getblocktemplate() // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { - checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); - - WAIT_LOCK(g_best_block_mutex, lock); - while (g_best_block == hashWatchedChain && IsRPCRunning()) - { - if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) - { - // Timeout: Check transactions for update - // without holding the mempool lock to avoid deadlocks - if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP) - break; - checktxtime += std::chrono::seconds(10); - } + MillisecondsDouble checktxtime{std::chrono::minutes(1)}; + while (tip == hashWatchedChain && IsRPCRunning()) { + tip = miner.waitTipChanged(hashWatchedChain, checktxtime).hash; + // Timeout: Check transactions for update + // without holding the mempool lock to avoid deadlocks + if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP) + break; + checktxtime = std::chrono::seconds(10); } } ENTER_CRITICAL_SECTION(cs_main); - tip = CHECK_NONFATAL(miner.getTipHash()).value(); + tip = CHECK_NONFATAL(miner.getTip()).value().hash; if (!IsRPCRunning()) throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down"); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 2c07a2ff087..086f609ac3e 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -18,8 +19,7 @@ #include #include #include - -#include +#include #include #include @@ -69,22 +69,6 @@ struct RPCCommandExecution } }; -static struct CRPCSignals -{ - boost::signals2::signal Started; - boost::signals2::signal Stopped; -} g_rpcSignals; - -void RPCServer::OnStarted(std::function slot) -{ - g_rpcSignals.Started.connect(slot); -} - -void RPCServer::OnStopped(std::function slot) -{ - g_rpcSignals.Stopped.connect(slot); -} - std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& helpreq) const { std::string strRet; @@ -297,7 +281,6 @@ void StartRPC() { LogDebug(BCLog::RPC, "Starting RPC\n"); g_rpc_running = true; - g_rpcSignals.Started(); } void InterruptRPC() @@ -311,16 +294,19 @@ void InterruptRPC() }); } -void StopRPC() +void StopRPC(const std::any& context) { static std::once_flag g_rpc_stop_flag; // This function could be called twice if the GUI has been started with -server=1. assert(!g_rpc_running); - std::call_once(g_rpc_stop_flag, []() { + std::call_once(g_rpc_stop_flag, [&]() { LogDebug(BCLog::RPC, "Stopping RPC\n"); WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); DeleteAuthCookie(); - g_rpcSignals.Stopped(); + node::NodeContext& node = EnsureAnyNodeContext(context); + // The notifications interface doesn't exist between initialization step 4a and 7. + if (node.notifications) node.notifications->m_tip_block_cv.notify_all(); + LogDebug(BCLog::RPC, "RPC stopped.\n"); }); } diff --git a/src/rpc/server.h b/src/rpc/server.h index 56e8a63088b..a8896d4247a 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -18,12 +19,6 @@ class CRPCCommand; -namespace RPCServer -{ - void OnStarted(std::function slot); - void OnStopped(std::function slot); -} - /** Query whether RPC is running */ bool IsRPCRunning(); @@ -178,7 +173,7 @@ extern CRPCTable tableRPC; void StartRPC(); void InterruptRPC(); -void StopRPC(); +void StopRPC(const std::any& context); UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors); #endif // BITCOIN_RPC_SERVER_H diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 30c5982b17d..702af6578dc 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -4,6 +4,7 @@ // #include #include +#include #include #include #include @@ -69,14 +70,14 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) { ChainstateManager& chainman = *Assert(m_node.chainman); - uint256 curr_tip = ::g_best_block; + uint256 curr_tip = m_node.notifications->m_tip_block; // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can // be found. mineBlocks(10); // After adding some blocks to the tip, best block should have changed. - BOOST_CHECK(::g_best_block != curr_tip); + BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip); // Grab block 1 from disk; we'll add it to the background chain later. std::shared_ptr pblockone = std::make_shared(); @@ -91,15 +92,15 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive())); - curr_tip = ::g_best_block; + curr_tip = m_node.notifications->m_tip_block; // Mine a new block on top of the activated snapshot chainstate. mineBlocks(1); // Defined in TestChain100Setup. // After adding some blocks to the snapshot tip, best block should have changed. - BOOST_CHECK(::g_best_block != curr_tip); + BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip); - curr_tip = ::g_best_block; + curr_tip = m_node.notifications->m_tip_block; BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); @@ -138,7 +139,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // g_best_block should be unchanged after adding a block to the background // validation chain. BOOST_CHECK(block_added); - BOOST_CHECK_EQUAL(curr_tip, ::g_best_block); + BOOST_CHECK_EQUAL(curr_tip, m_node.notifications->m_tip_block); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 38fcda55af6..b47b1d09ae1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -108,10 +108,6 @@ const std::vector CHECKLEVEL_DOC { * */ static constexpr int PRUNE_LOCK_BUFFER{10}; -GlobalMutex g_best_block_mutex; -std::condition_variable g_best_block_cv; -uint256 g_best_block; - const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locator) const { AssertLockHeld(cs_main); @@ -2988,12 +2984,6 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) m_mempool->AddTransactionsUpdated(1); } - { - LOCK(g_best_block_mutex); - g_best_block = pindexNew->GetBlockHash(); - g_best_block_cv.notify_all(); - } - std::vector warning_messages; if (!m_chainman.IsInitialBlockDownload()) { const CBlockIndex* pindex = pindexNew; diff --git a/src/validation.h b/src/validation.h index 6bc8a14de95..f6aeea3faaf 100644 --- a/src/validation.h +++ b/src/validation.h @@ -85,11 +85,6 @@ enum class SynchronizationState { POST_INIT }; -extern GlobalMutex g_best_block_mutex; -extern std::condition_variable g_best_block_cv; -/** Used to notify getblocktemplate RPC of new tips. */ -extern uint256 g_best_block; - /** Documentation for argument 'checklevel'. */ extern const std::vector CHECKLEVEL_DOC; diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 91e9975ad54..f02e6914ef5 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -90,6 +90,7 @@ class BlockchainTest(BitcoinTestFramework): self._test_getdifficulty() self._test_getnetworkhashps() self._test_stopatheight() + self._test_waitforblock() # also tests waitfornewblock self._test_waitforblockheight() self._test_getblock() self._test_getdeploymentinfo() @@ -507,6 +508,38 @@ class BlockchainTest(BitcoinTestFramework): self.start_node(0) assert_equal(self.nodes[0].getblockcount(), HEIGHT + 7) + def _test_waitforblock(self): + self.log.info("Test waitforblock and waitfornewblock") + node = self.nodes[0] + + current_height = node.getblock(node.getbestblockhash())['height'] + current_hash = node.getblock(node.getbestblockhash())['hash'] + + self.log.debug("Roll the chain back a few blocks and then reconsider it") + rollback_height = current_height - 100 + rollback_hash = node.getblockhash(rollback_height) + rollback_header = node.getblockheader(rollback_hash) + + node.invalidateblock(rollback_hash) + assert_equal(node.getblockcount(), rollback_height - 1) + + self.log.debug("waitforblock should return the same block after its timeout") + assert_equal(node.waitforblock(blockhash=current_hash, timeout=1)['hash'], rollback_header['previousblockhash']) + + node.reconsiderblock(rollback_hash) + # The chain has probably already been restored by the time reconsiderblock returns, + # but poll anyway. + self.wait_until(lambda: node.waitforblock(blockhash=current_hash, timeout=100)['hash'] == current_hash) + + # roll back again + node.invalidateblock(rollback_hash) + assert_equal(node.getblockcount(), rollback_height - 1) + + node.reconsiderblock(rollback_hash) + # The chain has probably already been restored by the time reconsiderblock returns, + # but poll anyway. + self.wait_until(lambda: node.waitfornewblock(timeout=100)['hash'] == current_hash) + def _test_waitforblockheight(self): self.log.info("Test waitforblockheight") node = self.nodes[0]