mine: do not submit a solution for a parent that is no longer the tip (#69)

The mining loop tests pindexPrev != pindexBest every 256 hashes, but never
again once a solution turns up. A thread that found one just after somebody
else extended the chain submitted a block built on a stale parent -- a
block born orphaned, at a height already taken.

Every miner thread on a machine works from the same pindexPrev, so the
moment one wins, all the others are hashing a doomed template until their
next checkpoint. On a 32-core box that is thirty threads with a standing
chance of producing a guaranteed orphan.

Re-check the tip inside cs_main before submitting, and drop the solution if
it moved.

Verified on an isolated chain with three miner threads: 2790 blocks
accepted, so nothing legitimate is being discarded, and 2 solutions dropped
for a moved tip -- which is the race actually happening and being caught.

Honest about the size of it: two in 2790 is not what fills a wallet with
orphans. On the 32-core node in production, 1287 of 4117 processed blocks
arrive with a missing parent, which is a different problem and the one
worth chasing next.

1 files changed, 30 insertions, 8 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 85f89d9..b147ddd 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -3151,17 +3151,39 @@ bool BitcoinMiner(int nThreadId)
                 SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_NORMAL);
                 CRITICAL_BLOCK(cs_main)
                 {
-                    // Save key
-                    if (!AddKey(key))
+                    // Is the parent we solved for still the tip?
+                    //
+                    // The loop only tests this every 256 hashes, and never
+                    // again once a solution turns up. So a thread that found
+                    // one just after somebody else extended the chain would
+                    // submit a block built on a parent that is no longer the
+                    // tip -- a block born orphaned, at a height already taken.
+                    //
+                    // Every miner thread on this machine works from the same
+                    // pindexPrev, so the moment one of them wins, all the
+                    // others are hashing a doomed template until their next
+                    // checkpoint. On a 32-core box that is thirty threads with
+                    // a standing chance of producing a guaranteed orphan, and
+                    // it is why a healthy miner's wallet filled up with them.
+                    if (pindexPrev != pindexBest)
                     {
-                        RandomXDestroyMinerVM(rxvm);
-                        return false;
+                        LogPrint("net", "BitcoinMiner: thread %d solved for a parent that is no longer the tip, discarding\n",
+                                 nThreadId);
                     }
-                    key.MakeNewKey();
+                    else
+                    {
+                        // Save key
+                        if (!AddKey(key))
+                        {
+                            RandomXDestroyMinerVM(rxvm);
+                            return false;
+                        }
+                        key.MakeNewKey();
 
-                    // Process this block the same as if we had received it from another node
-                    if (!ProcessBlock(NULL, pblock.release()))
-                        LogPrint("net", "ERROR in BitcoinMiner, ProcessBlock, block not accepted\n");
+                        // Process this block the same as if we had received it from another node
+                        if (!ProcessBlock(NULL, pblock.release()))
+                            LogPrint("net", "ERROR in BitcoinMiner, ProcessBlock, block not accepted\n");
+                    }
                 }
                 SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_LOWEST);