Don't assert if we were beaten to the block

CaveSpectre11 · web-flow · commit 019d26ac4cdc · 2019-06-10T12:19:55.000-04:00
A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet.  This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block.  

This situation would result in the wallet being asserted; however this is a little overkill.  rather than asserting if the tip has changed, it is better to throw the block away.

This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key.  The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks.

It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further.
diff --git a/src/main.cpp b/src/main.cpp
@@ -5176,7 +5176,11 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, CBlock* pblock, CDis
 bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* const pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot)
 {
     AssertLockHeld(cs_main);
-    assert(pindexPrev && pindexPrev == chainActive.Tip());
+    assert(pindexPrev);
+    if (pindexPrev != chainActive.Tip()) {
+        LogPrintf("TestBlockValidity(): No longer working on chain tip\n");
+        return false;
+    }
 
     CCoinsViewCache viewNew(pcoinsTip);
     CBlockIndex indexDummy(block);
