[Mining] IncrementExtraNonce without cs_main

Removes the `cs_main` lock usage from the nonce setup
in the miners.

The chain tip is only used for getting the Height of the tip,
so instead we can just retrieve the height directly. This requires
a change to the signature of `IncrementExtraNonce`.

The other item cs_main was protecting is the static `prevHeaderHash`;
this is only used to reset `nExtraNonce`. However, `nExtraNonce` is
thread-local, so the thread nonces will increment indefinitely, with the
occasional 0 thrown in. This is therefore unnecessary--and rarely may
result in a race:
- Thread 1: creates block template on block N-1
- Thread 2: accepts new block N
- Thread 3: creates block template on block N, resets local nonce
- Thread 1: resets local nonce
- Thread 4: creates block template on block N, resets local nonce
Thread 1 is doing useless work on the old block, this happens.
Threads 3 and 4 are doing the same work since they have the same nonce.

(If I understand right, and the nonces are used here mainly to give
the threads distinct works sets, I believe this means actually
incrementing `nExtraNonce` is unnecessary, since we take `cs_nonce` to
increment `nNonce_base` in order to give each thread a unique nonce.
We would need to rename `IncrementExtraNonce` to describe
what it's actually doing then.)

Removes one extraneous copy of the nonce into a thread local variable
(there were previously two, and I suspect the extra was already being
optimized away).

Author: Zannick

src/miner.cpp

@@ -814,19 +814,13 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
     }
 }
 
-void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce)
+void IncrementExtraNonce(CBlock* pblock, unsigned int nHeight, unsigned int& nExtraNonce)
 {
     // Update nExtraNonce
-    static uint256 hashPrevBlock;
-    if (hashPrevBlock != pblock->hashPrevBlock)
-    {
-        nExtraNonce = 0;
-        hashPrevBlock = pblock->hashPrevBlock;
-    }
     ++nExtraNonce;
-    unsigned int nHeight = pindexPrev->nHeight+1; // Height first in coinbase required for block.version=2
     CMutableTransaction txCoinbase(*pblock->vtx[0]);
-    txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS;
+    // Height first in coinbase required for block.version=2
+    txCoinbase.vin[0].scriptSig = (CScript() << (nHeight+1) << CScriptNum(nExtraNonce)) + COINBASE_FLAGS;
     assert(txCoinbase.vin[0].scriptSig.size() <= 100);
 
     pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase));
@@ -964,24 +958,19 @@ void BitcoinMiner(std::shared_ptr<CReserveScript> coinbaseScript, bool fProofOfS
         }
 
         CBlock *pblock = &pblocktemplate->block;
-        int32_t nNonceLocal;
 
         if (!fProofOfStake)
         {
             {
                 LOCK(cs_nonce);
-                nNonceLocal = nNonce_base++;
+                nExtraNonce = nNonce_base++;
                 if (!nTimeStart)
                     nTimeStart = GetTime();
             }
 
             pblock->nNonce = 0;
             pblock->nNonce64 = rand();
-            {
-                LOCK(cs_main);
-                nExtraNonce = nNonceLocal;
-                IncrementExtraNonce(pblock, chainActive.Tip(), nExtraNonce);
-            }
+            IncrementExtraNonce(pblock, chainActive.Height(), nExtraNonce);
 
             int nTries = 0;
             if (pblock->IsProgPow() && pblock->nTime >= Params().PowUpdateTimestamp()) {
@@ -1073,21 +1062,16 @@ void BitcoinRandomXMiner(std::shared_ptr<CReserveScript> coinbaseScript, int vm_
             continue;
 
         CBlock *pblock = &pblocktemplate->block;
-        int32_t nNonceLocal;
 
         {
             LOCK(cs_nonce);
-            nNonceLocal = nNonce_base++;
+            nExtraNonce = nNonce_base++;
             if (!nTimeStart)
                 nTimeStart = GetTime();
         }
 
         pblock->nNonce = startNonce;
-        {
-            LOCK(cs_main);
-            nExtraNonce = nNonceLocal;
-            IncrementExtraNonce(pblock, chainActive.Tip(), nExtraNonce);
-        }
+        IncrementExtraNonce(pblock, chainActive.Height(), nExtraNonce);
 
         int nTries = 0;
         if (pblock->IsRandomX() && pblock->nTime >= Params().PowUpdateTimestamp()) {

src/miner.h

@@ -247,7 +247,7 @@ bool GenerateActive();
 void setGenerate(bool fGenerate);
 
 /** Modify the extranonce in a block */
-void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce);
+void IncrementExtraNonce(CBlock* pblock, unsigned int nHeight, unsigned int& nExtraNonce);
 int64_t UpdateTime(CBlock* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 void GenerateBitcoins(bool fGenerate, int nThreads, std::shared_ptr<CReserveScript> coinbaseScript);
 void ThreadStakeMiner();

src/rpc/mining.cpp

@@ -220,10 +220,7 @@ UniValue generateBlocks(std::shared_ptr<CReserveScript> coinbaseScript, int nGen
         if (!pblocktemplate.get())
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
         CBlock *pblock = &pblocktemplate->block;
-        {
-            LOCK(cs_main);
-            IncrementExtraNonce(pblock, chainActive.Tip(), nExtraNonce);
-        }
+        IncrementExtraNonce(pblock, chainActive.Height(), nExtraNonce);
 
         // This will check if the key block needs to change and will take down the cache and vm, and spin up the new ones
         CheckIfValidationKeyShouldChangeAndUpdate(GetKeyBlock(pblock->nHeight));

src/test/test_veil.cpp

@@ -155,11 +155,8 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
     for (const CMutableTransaction& tx : txns)
         block.vtx.push_back(MakeTransactionRef(tx));
     // IncrementExtraNonce creates a valid coinbase and merkleRoot
-    {
-        LOCK(cs_main);
-        unsigned int extraNonce = 0;
-        IncrementExtraNonce(&block, chainActive.Tip(), extraNonce);
-    }
+    unsigned int extraNonce = 0;
+    IncrementExtraNonce(&block, chainActive.Height(), extraNonce);
 
     while (!CheckProofOfWork(block.GetX16RTPoWHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;