rpc: minize getTipHash() calls in gbt

Set tip at the start of the function and only update it for a long poll. Additionally have getTipHash return an optional, so the caller can explicitly check that a tip exists.
bitcoin · Jun 18, 2024 · dda0b08 · dda0b08
1 parent 7b4d324
commit dda0b08
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 9 deletions.
diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_INTERFACES_MINING_H
 #define BITCOIN_INTERFACES_MINING_H
 
+#include <optional>
 #include <uint256.h>
 
 namespace node {
@@ -29,8 +30,8 @@ class Mining
     //! If this chain is exclusively used for testing
     virtual bool isTestChain() = 0;
 
-    //! Returns the hash for the tip of this chain, 0 if none
-    virtual uint256 getTipHash() = 0;
+    //! Returns the hash for the tip of this chain
+    virtual std::optional<uint256> getTipHash() = 0;
 
    /**
      * Construct a new block template

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -847,11 +847,11 @@ class MinerImpl : public Mining
         return chainman().GetParams().IsTestChain();
     }
 
-    uint256 getTipHash() override
+    std::optional<uint256> getTipHash() override
     {
         LOCK(::cs_main);
         CBlockIndex* tip{chainman().ActiveChain().Tip()};
-        if (!tip) return uint256{0};
+        if (!tip) return {};
         return tip->GetBlockHash();
     }
 

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -667,6 +667,9 @@ static RPCHelpMan getblocktemplate()
     ChainstateManager& chainman = EnsureChainman(node);
     Mining& miner = EnsureMining(node);
     LOCK(cs_main);
+    std::optional<uint256> maybe_tip{miner.getTipHash()};
+    CHECK_NONFATAL(maybe_tip);
+    uint256 tip{maybe_tip.value()};
 
     std::string strMode = "template";
     UniValue lpval = NullUniValue;
@@ -706,7 +709,7 @@ static RPCHelpMan getblocktemplate()
             }
 
             // testBlockValidity only supports blocks built on the current Tip
-            if (block.hashPrevBlock != miner.getTipHash()) {
+            if (block.hashPrevBlock != tip) {
                 return "inconclusive-not-best-prevblk";
             }
             BlockValidationState state;
@@ -757,7 +760,7 @@ static RPCHelpMan getblocktemplate()
         else
         {
             // NOTE: Spec does not specify behaviour for non-string longpollid, but this makes testing easier
-            hashWatchedChain = miner.getTipHash();
+            hashWatchedChain = tip;
             nTransactionsUpdatedLastLP = nTransactionsUpdatedLast;
         }
 
@@ -781,6 +784,10 @@ static RPCHelpMan getblocktemplate()
         }
         ENTER_CRITICAL_SECTION(cs_main);
 
+        std::optional<uint256> maybe_tip{miner.getTipHash()};
+        CHECK_NONFATAL(maybe_tip);
+        tip = maybe_tip.value();
+
         if (!IsRPCRunning())
             throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down");
         // TODO: Maybe recheck connections/IBD and (if something wrong) send an expires-immediately template to stop miners?
@@ -802,15 +809,15 @@ static RPCHelpMan getblocktemplate()
     static CBlockIndex* pindexPrev;
     static int64_t time_start;
     static std::unique_ptr<CBlockTemplate> pblocktemplate;
-    if (!pindexPrev || pindexPrev->GetBlockHash() != miner.getTipHash() ||
+    if (!pindexPrev || pindexPrev->GetBlockHash() != tip ||
         (miner.getTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5))
     {
         // Clear pindexPrev so future calls make a new block, despite any failures from here on
         pindexPrev = nullptr;
 
         // Store the pindexBest used before createNewBlock, to avoid races
         nTransactionsUpdatedLast = miner.getTransactionsUpdated();
-        CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(miner.getTipHash());
+        CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip);
         time_start = GetTime();
 
         // Create new block
@@ -943,7 +950,7 @@ static RPCHelpMan getblocktemplate()
     result.pushKV("transactions", std::move(transactions));
     result.pushKV("coinbaseaux", std::move(aux));
     result.pushKV("coinbasevalue", (int64_t)pblock->vtx[0]->vout[0].nValue);
-    result.pushKV("longpollid", miner.getTipHash().GetHex() + ToString(nTransactionsUpdatedLast));
+    result.pushKV("longpollid", tip.GetHex() + ToString(nTransactionsUpdatedLast));
     result.pushKV("target", hashTarget.GetHex());
     result.pushKV("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1);
     result.pushKV("mutable", std::move(aMutable));