If we are going to add a new m_tip_block variable to replace g_best_block seems like it would be good to make replacement complete and eliminate g_best_block. I think following could work (compiles but untested):
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -739,7 +739,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())
@@ -760,19 +759,14 @@ 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);
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -303,7 +303,6 @@ void StopRPC(const std::any& context)
LogPrint(BCLog::RPC, "Stopping RPC\n");
WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear());
DeleteAuthCookie();
- g_best_block_cv.notify_all();
Assert(EnsureAnyNodeContext(context).notifications)->m_tip_block_cv.notify_all();
LogPrint(BCLog::RPC, "RPC stopped.\n");
});
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -4,6 +4,7 @@
//
#include <chainparams.h>
#include <consensus/validation.h>
+#include <node/kernel_notifications.h>
#include <random.h>
#include <rpc/blockchain.h>
#include <sync.h>
@@ -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<CBlock> pblockone = std::make_shared<CBlock>();
@@ -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()
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -107,10 +107,6 @@ const std::vector<std::string> 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);
@@ -2987,12 +2983,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<bilingual_str> warning_messages;
if (!m_chainman.IsInitialBlockDownload()) {
const CBlockIndex* pindex = pindexNew;
--- 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<std::string> CHECKLEVEL_DOC;
