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):
0--- a/src/rpc/mining.cpp
1+++ b/src/rpc/mining.cpp
2@@ -739,7 +739,6 @@ static RPCHelpMan getblocktemplate()
3 {
4 // Wait to respond until either the best block changes, OR a minute has passed and there are more transactions
5 uint256 hashWatchedChain;
6- std::chrono::steady_clock::time_point checktxtime;
7 unsigned int nTransactionsUpdatedLastLP;
8
9 if (lpval.isStr())
10@@ -760,19 +759,14 @@ static RPCHelpMan getblocktemplate()
11 // Release lock while waiting
12 LEAVE_CRITICAL_SECTION(cs_main);
13 {
14- checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
15-
16- WAIT_LOCK(g_best_block_mutex, lock);
17- while (g_best_block == hashWatchedChain && IsRPCRunning())
18- {
19- if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout)
20- {
21- // Timeout: Check transactions for update
22- // without holding the mempool lock to avoid deadlocks
23- if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP)
24- break;
25- checktxtime += std::chrono::seconds(10);
26- }
27+ MillisecondsDouble checktxtime{std::chrono::minutes(1)};
28+ while (tip == hashWatchedChain && IsRPCRunning()) {
29+ tip = miner.waitTipChanged(hashWatchedChain, checktxtime).hash;
30+ // Timeout: Check transactions for update
31+ // without holding the mempool lock to avoid deadlocks
32+ if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP)
33+ break;
34+ checktxtime = std::chrono::seconds(10);
35 }
36 }
37 ENTER_CRITICAL_SECTION(cs_main);
38--- a/src/rpc/server.cpp
39+++ b/src/rpc/server.cpp
40@@ -303,7 +303,6 @@ void StopRPC(const std::any& context)
41 LogPrint(BCLog::RPC, "Stopping RPC\n");
42 WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear());
43 DeleteAuthCookie();
44- g_best_block_cv.notify_all();
45 Assert(EnsureAnyNodeContext(context).notifications)->m_tip_block_cv.notify_all();
46 LogPrint(BCLog::RPC, "RPC stopped.\n");
47 });
48--- a/src/test/validation_chainstate_tests.cpp
49+++ b/src/test/validation_chainstate_tests.cpp
50@@ -4,6 +4,7 @@
51 //
52 #include <chainparams.h>
53 #include <consensus/validation.h>
54+#include <node/kernel_notifications.h>
55 #include <random.h>
56 #include <rpc/blockchain.h>
57 #include <sync.h>
58@@ -69,14 +70,14 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
59 BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
60 {
61 ChainstateManager& chainman = *Assert(m_node.chainman);
62- uint256 curr_tip = ::g_best_block;
63+ uint256 curr_tip = m_node.notifications->m_tip_block;
64
65 // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can
66 // be found.
67 mineBlocks(10);
68
69 // After adding some blocks to the tip, best block should have changed.
70- BOOST_CHECK(::g_best_block != curr_tip);
71+ BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip);
72
73 // Grab block 1 from disk; we'll add it to the background chain later.
74 std::shared_ptr<CBlock> pblockone = std::make_shared<CBlock>();
75@@ -91,15 +92,15 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
76 // Ensure our active chain is the snapshot chainstate.
77 BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive()));
78
79- curr_tip = ::g_best_block;
80+ curr_tip = m_node.notifications->m_tip_block;
81
82 // Mine a new block on top of the activated snapshot chainstate.
83 mineBlocks(1); // Defined in TestChain100Setup.
84
85 // After adding some blocks to the snapshot tip, best block should have changed.
86- BOOST_CHECK(::g_best_block != curr_tip);
87+ BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip);
88
89- curr_tip = ::g_best_block;
90+ curr_tip = m_node.notifications->m_tip_block;
91
92 BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2);
93
94@@ -138,7 +139,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
95 // g_best_block should be unchanged after adding a block to the background
96 // validation chain.
97 BOOST_CHECK(block_added);
98- BOOST_CHECK_EQUAL(curr_tip, ::g_best_block);
99+ BOOST_CHECK_EQUAL(curr_tip, m_node.notifications->m_tip_block);
100 }
101
102 BOOST_AUTO_TEST_SUITE_END()
103--- a/src/validation.cpp
104+++ b/src/validation.cpp
105@@ -107,10 +107,6 @@ const std::vector<std::string> CHECKLEVEL_DOC {
106 * */
107 static constexpr int PRUNE_LOCK_BUFFER{10};
108
109-GlobalMutex g_best_block_mutex;
110-std::condition_variable g_best_block_cv;
111-uint256 g_best_block;
112-
113 const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locator) const
114 {
115 AssertLockHeld(cs_main);
116@@ -2987,12 +2983,6 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
117 m_mempool->AddTransactionsUpdated(1);
118 }
119
120- {
121- LOCK(g_best_block_mutex);
122- g_best_block = pindexNew->GetBlockHash();
123- g_best_block_cv.notify_all();
124- }
125-
126 std::vector<bilingual_str> warning_messages;
127 if (!m_chainman.IsInitialBlockDownload()) {
128 const CBlockIndex* pindex = pindexNew;
129--- a/src/validation.h
130+++ b/src/validation.h
131@@ -85,11 +85,6 @@ enum class SynchronizationState {
132 POST_INIT
133 };
134
135-extern GlobalMutex g_best_block_mutex;
136-extern std::condition_variable g_best_block_cv;
137-/** Used to notify getblocktemplate RPC of new tips. */
138-extern uint256 g_best_block;
139-
140 /** Documentation for argument 'checklevel'. */
141 extern const std::vector<std::string> CHECKLEVEL_DOC;
142
