Unit test + tweaks for https://github.com/bitcoin/bitcoin/issues/17890

unit_test.diff

diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index f2c862011..ebb9b3aca 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -211,12 +211,64 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
     UnregisterValidationInterface(&sub);
 
     LOCK(cs_main);
     BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
 }
 
+BOOST_AUTO_TEST_CASE(download_blocks_out_of_order)
+{
+    const int numberOfExtraBlocks = 10;
+    std::vector<std::shared_ptr<const CBlock>> extraBlocks;
+
+    BuildChain(Params().GenesisBlock().GetHash(), numberOfExtraBlocks, 0, 0, numberOfExtraBlocks, extraBlocks);
+
+    assert(extraBlocks.size() == numberOfExtraBlocks);
+
+    bool ignored;
+    BlockValidationState state;
+    std::vector<CBlockHeader> headers;
+    std::transform(extraBlocks.begin(), extraBlocks.end(), std::back_inserter(headers), [](std::shared_ptr<const CBlock> b) { return b->GetBlockHeader(); });
+
+    // Process all the headers so we understand the toplogy of the chain
+    BOOST_CHECK(ProcessNewBlockHeaders(headers, state, Params()));
+
+    // Connect the genesis block and drain any outstanding events
+    BOOST_CHECK(ProcessNewBlock(Params(), std::make_shared<CBlock>(Params().GenesisBlock()), true, &ignored));
+    SyncWithValidationInterfaceQueue();
+
+    // subscribe to events (this subscriber will validate event ordering)
+    const CBlockIndex* initial_tip = nullptr;
+    {
+        LOCK(cs_main);
+        initial_tip = ::ChainActive().Tip();
+    }
+    TestSubscriber sub(initial_tip->GetBlockHash());
+    RegisterValidationInterface(&sub);
+
+    //std::vector<size_t> order{0, 1, 2, 3,  4, 5, 6, 7,  8, 9, 10};
+    std::vector<size_t> order{0, 1, 2, 3,  4, 5, 6, 8,  7, 9, 10};
+    //std::vector<size_t> order{0, 1, 2, 3,  4, 5, 6, 7,  9, 8, 10};
+    //std::vector<size_t> order{0, 1, 2, 3,  4, 7, 8, 9,  10, 5, 6};
+    //std::vector<size_t> order{0, 2, 3, 4,  5, 6, 7, 8,  9, 1, 10};
+
+    for (size_t i = 1; i <= numberOfExtraBlocks; ++i) {
+        // Block at height H is in extraBlocks[H - 1].
+        bool ok = ProcessNewBlock(Params(), extraBlocks[order[i] - 1], true, &ignored);
+        assert(ok);
+    }
+
+    while (GetMainSignals().CallbacksPending() > 0) {
+        UninterruptibleSleep(std::chrono::milliseconds{100});
+    }
+
+    UnregisterValidationInterface(&sub);
+
+    LOCK(cs_main);
+    BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
+}
+
 /**
  * Test that mempool updates happen atomically with reorgs.
  *
  * This prevents RPC clients, among others, from retrieving immediately-out-of-date mempool data
  * during large reorgs.
  *
diff --git a/src/validation.h b/src/validation.h
index a5335edc4..33260ea15 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -67,17 +67,17 @@ static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101;
 static const unsigned int EXTRA_DESCENDANT_TX_SIZE_LIMIT = 10000;
 /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
 static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336;
 /** Maximum kilobytes for transactions to store for processing during reorg */
 static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = 20000;
 /** The maximum size of a blk?????.dat file (since 0.8) */
-static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
+static const unsigned int MAX_BLOCKFILE_SIZE = 1200;
 /** The pre-allocation chunk size for blk?????.dat files (since 0.8) */
-static const unsigned int BLOCKFILE_CHUNK_SIZE = 0x1000000; // 16 MiB
+static const unsigned int BLOCKFILE_CHUNK_SIZE = 600;
 /** The pre-allocation chunk size for rev?????.dat files (since 0.8) */
-static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
+static const unsigned int UNDOFILE_CHUNK_SIZE = 130;
 
 /** Maximum number of dedicated script-checking threads allowed */
 static const int MAX_SCRIPTCHECK_THREADS = 15;
 /** -par default (number of script-checking threads, 0 = auto) */
 static const int DEFAULT_SCRIPTCHECK_THREADS = 0;
 /** Number of blocks that can be requested at any given time from a single peer. */

x-debug_prints.diff

--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1762,34 +1762,36 @@ DisconnectResult CChainState::DisconnectBlock(const CBlock& block, const CBlockI
     return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN;
 }
 
 static void FlushUndoFile(int undo_file, bool finalize = false)
 {
     FlatFilePos pos(undo_file, vinfoBlockFile[undo_file].nUndoSize);
+    LogPrintf("BBB FlushUndoFile(): rev%05u pos=%u, finalize=%d\n", pos.nFile, pos.nPos, finalize);
     if (!UndoFileSeq().Flush(pos, finalize)) {
         AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.");
     }
 }
 
 static void FlushBlockFile(bool fFinalize = false)
 {
     LOCK(cs_LastBlockFile);
     FlatFilePos pos(nLastBlockFile, vinfoBlockFile[nLastBlockFile].nSize);
+    LogPrintf("BBB FlushBlockFile(): blk%05u pos=%u, finalize=%d\n", pos.nFile, pos.nPos, fFinalize);
     if (!BlockFileSeq().Flush(pos, fFinalize)) {
         AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");
     }
 }
 
-static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize);
+static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize, int nHeight);
 
 static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex* pindex, const CChainParams& chainparams)
 {
     // Write undo information to disk
     if (pindex->GetUndoPos().IsNull()) {
         FlatFilePos _pos;
-        if (!FindUndoPos(state, pindex->nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40))
+        if (!FindUndoPos(state, pindex->nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40, pindex->nHeight))
             return error("ConnectBlock(): FindUndoPos failed");
         if (!UndoWriteToDisk(blockundo, _pos, pindex->pprev->GetBlockHash(), chainparams.MessageStart()))
             return AbortNode(state, "Failed to write undo data");
 
         const unsigned thisHeight = static_cast<unsigned>(pindex->nHeight);
 
@@ -1809,12 +1811,15 @@ static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationSt
         const bool isLastEntry = thisHeight == vinfoBlockFile[_pos.nFile].nHeightLast;
 
         if (wroteToOldFile && isLastEntry) {
             FlushUndoFile(_pos.nFile, true);
         }
 
+        LogPrintf("BBB WriteUndoDataForBlock(): _pos.nFile=%d, nHighestUndoFile=%d, height=%d, adjacentHeights=%d, isLastEntry=%d\n",
+                _pos.nFile, nHighestUndoFile, pindex->nHeight, adjacentHeights, isLastEntry);
+
         if (movedToSubsequent) {
             nHighestUndoFile = _pos.nFile;
         }
 
         // update nUndoPos in block index
         pindex->nUndoPos = _pos.nPos;
@@ -3287,12 +3292,15 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n
             }
         }
         pos.nFile = nFile;
         pos.nPos = vinfoBlockFile[nFile].nSize;
     }
 
+    LogPrintf("BBB FindBlockPos(): height=%u, belongs to blk%05u[%u..%u]\n",
+            nHeight, pos.nFile, pos.nPos, pos.nPos + nAddSize - 1);
+
     if ((int)nFile != nLastBlockFile) {
         if (!fKnown) {
             LogPrintf("Leaving block file %i: %s\n", nLastBlockFile, vinfoBlockFile[nLastBlockFile].ToString());
         }
         FlushBlockFile(!fKnown);
         nLastBlockFile = nFile;
@@ -3316,22 +3324,25 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n
     }
 
     setDirtyFileInfo.insert(nFile);
     return true;
 }
 
-static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize)
+static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize, int nHeight)
 {
     pos.nFile = nFile;
 
     LOCK(cs_LastBlockFile);
 
     pos.nPos = vinfoBlockFile[nFile].nUndoSize;
     vinfoBlockFile[nFile].nUndoSize += nAddSize;
     setDirtyFileInfo.insert(nFile);
 
+    LogPrintf("BBB FindUndoPos(): height=%d, belongs to: rev%05u[%u..%u]\n",
+            nHeight, nFile, pos.nPos, pos.nPos + nAddSize - 1);
+
     bool out_of_space;
     size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space);
     if (out_of_space) {
         return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!").translated, CClientUIInterface::MSG_NOPREFIX);
     }
     if (bytes_allocated != 0 && fPruneMode) {
@@ -3744,12 +3755,13 @@ bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, BlockValid
     }
     return true;
 }
 
 /** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */
 static FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, const CChainParams& chainparams, const FlatFilePos* dbp) {
+    LogPrintf("BBB SaveBlockToDisk(): height=%d\n", nHeight);
     unsigned int nBlockSize = ::GetSerializeSize(block, CLIENT_VERSION);
     FlatFilePos blockPos;
     if (dbp != nullptr)
         blockPos = *dbp;
     if (!FindBlockPos(blockPos, nBlockSize+8, nHeight, block.GetBlockTime(), dbp != nullptr)) {
         error("%s: FindBlockPos failed", __func__);
@@ -3838,12 +3850,13 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block
         ReceivedBlockTransactions(block, pindex, blockPos, chainparams.GetConsensus());
     } catch (const std::runtime_error& e) {
         return AbortNode(state, std::string("System error: ") + e.what());
     }
 
     FlushStateToDisk(chainparams, state, FlushStateMode::NONE);
+    //FlushStateToDisk(chainparams, state, FlushStateMode::ALWAYS);
 
     CheckBlockIndex(chainparams.GetConsensus());
 
     return true;
 }