From 4ea2048180b85dbf80b8c89e0cbd700eff4c1170 Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Tue, 17 Apr 2018 17:05:08 -0400 Subject: [PATCH 01/10] Add Unit Test for SingleThreadedSchedulerClient Ensures ordering of callbacks within a SingleThreadedSchedulerClient with respect to each other --- src/test/scheduler_tests.cpp | 44 +++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index ce46cbf5887f..3f3b0a660718 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(manythreads) size_t nTasks = microTasks.getQueueInfo(first, last); BOOST_CHECK(nTasks == 0); - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 100; ++i) { boost::chrono::system_clock::time_point t = now + boost::chrono::microseconds(randomMsec(rng)); boost::chrono::system_clock::time_point tReschedule = now + boost::chrono::microseconds(500 + randomMsec(rng)); int whichCounter = zeroToNine(rng); @@ -108,4 +108,46 @@ BOOST_AUTO_TEST_CASE(manythreads) BOOST_CHECK_EQUAL(counterSum, 200); } +BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) +{ + CScheduler scheduler; + + // each queue should be well ordered with respect to itself but not other queues + SingleThreadedSchedulerClient queue1(&scheduler); + SingleThreadedSchedulerClient queue2(&scheduler); + + // create more threads than queues + // if the queues only permit execution of one task at once then + // the extra threads should effectively be doing nothing + // if they don't we'll get out of order behaviour + boost::thread_group threads; + for (int i = 0; i < 5; ++i) { + threads.create_thread(boost::bind(&CScheduler::serviceQueue, &scheduler)); + } + + // these are not atomic, if SinglethreadedSchedulerClient prevents + // parallel execution at the queue level no synchronization should be required here + int counter1 = 0; + int counter2 = 0; + + // just simply count up on each queue - if execution is properly ordered then + // the callbacks should run in exactly the order in which they were enqueued + for (int i = 0; i < 100; ++i) { + queue1.AddToProcessQueue([i, &counter1]() { + BOOST_CHECK_EQUAL(i, counter1++); + }); + + queue2.AddToProcessQueue([i, &counter2]() { + BOOST_CHECK_EQUAL(i, counter2++); + }); + } + + // finish up + scheduler.stop(true); + threads.join_all(); + + BOOST_CHECK_EQUAL(counter1, 100); + BOOST_CHECK_EQUAL(counter2, 100); +} + BOOST_AUTO_TEST_SUITE_END() From cb9bb251ea288d2ec29ff965d13dd2747142b9fd Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Wed, 16 May 2018 11:10:31 -0400 Subject: [PATCH 02/10] Update documentation for SingleThreadedSchedulerClient() to specify the memory model --- src/scheduler.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/scheduler.h b/src/scheduler.h index 781bd52ea56f..6661c032c82f 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -87,9 +87,13 @@ class CScheduler /** * Class used by CScheduler clients which may schedule multiple jobs - * which are required to be run serially. Does not require such jobs - * to be executed on the same thread, but no two jobs will be executed - * at the same time. + * which are required to be run serially. Jobs may not be run on the + * same thread, but no two jobs will be executed + * at the same time and memory will be release-acquire consistent + * (the scheduler will internally do an acquire before invoking a callback + * as well as a release at the end). In practice this means that a callback + * B() will be able to observe all of the effects of callback A() which executed + * before it. */ class SingleThreadedSchedulerClient { private: @@ -104,6 +108,13 @@ class SingleThreadedSchedulerClient { public: explicit SingleThreadedSchedulerClient(CScheduler *pschedulerIn) : m_pscheduler(pschedulerIn) {} + + /** + * Add a callback to be executed. Callbacks are executed serially + * and memory is release-acquire consistent between callback executions. + * Practially, this means that callbacks can behave as if they are executed + * in order by a single thread. + */ void AddToProcessQueue(std::function func); // Processes all remaining queue members on the calling thread, blocking until queue is empty From f9d2ab3f3f0cd471f7f0a13bb15ffabc6f129d87 Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Wed, 16 May 2018 16:20:37 -0400 Subject: [PATCH 03/10] Update ValidationInterface() documentation to explicitly specify threading and memory model --- src/validationinterface.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/validationinterface.h b/src/validationinterface.h index 27bfd9d4ce17..008ec204b8f6 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -54,6 +54,21 @@ void CallFunctionInValidationInterfaceQueue(std::function func); */ void SyncWithValidationInterfaceQueue(); +/** + * Implement this to subscribe to events generated in validation + * + * Each CValidationInterface() subscriber will receive event callbacks + * in the order in which the events were generated by validation. + * Furthermore, each ValidationInterface() subscriber may assume that + * callbacks effectively run in a single thread with single-threaded + * memory consistency. That is, for a given ValidationInterface() + * instantiation, each callback will complete before the next one is + * invoked. This means, for example when a block is connected that the + * UpdatedBlockTip() callback may depend on an operation performed in + * the BlockConnected() callback without worrying about explicit + * synchronization. No ordering should be assumed across + * ValidationInterface() subscribers. + */ class CValidationInterface { public: virtual ~CValidationInterface() = default; From ef24337204712d08c932363bce9e1e32d9ee5e51 Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Sun, 15 Apr 2018 11:22:28 -0400 Subject: [PATCH 04/10] Hold cs_main while calling UpdatedBlockTip() and ui.NotifyBlockTip Ensures that callbacks are invoked in the order in which the chain is updated Resolves #12978 --- src/validation.cpp | 15 ++++++++------- src/validationinterface.cpp | 4 ++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index c9e6380fa884..fb5889f2c63b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2264,16 +2264,17 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb assert(trace.pblock && trace.pindex); GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); } - } - // Notify external listeners about the new tip. - GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); + // Notify external listeners about the new tip. + // Enqueue while holding cs_main to ensure that UpdatedBlockTip is called in the order in which blocks are connected + GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); - // Always notify the UI if a new block tip was connected - if (pindexFork != pindexNewTip) { - // Notify the UI - uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); + // Always notify the UI if a new block tip was connected + if (pindexFork != pindexNewTip) { + uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); + } } + // When we reach this point, we switched to a new tip (stored in pindexNewTip). } while (pindexMostWork != chainActive.Tip()); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index ab2b92527642..76a5ff27be39 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -146,6 +146,10 @@ void CMainSignals::MempoolEntryRemoved(CTransactionRef ptx, MemPoolRemovalReason } void CMainSignals::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) { + // Dependencies exist that require UpdatedBlockTip events to be delivered in the order in which + // the chain actually updates. One way to ensure this is for the caller to invoke this signal + // in the same critical section where the chain is updated + m_internals->m_schedulerClient.AddToProcessQueue([pindexNew, pindexFork, fInitialDownload, this] { m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); }); From 8640be1088b6307898af26c40e987028ee109827 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 6 Feb 2018 13:38:54 -0500 Subject: [PATCH 05/10] Fix fast-shutdown crash if genesis block was not loaded If the ShutdownRequested() check at the top of ActivateBestChain() returns false during initial genesis block load we will fail an assertion in UTXO DB flush as the best block hash IsNull(). To work around this, we move the check until after one round of ActivateBestChainStep(), ensuring the genesis block gets connected. --- src/validation.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index fb5889f2c63b..ef9a3e728d69 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2276,6 +2276,12 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb } // When we reach this point, we switched to a new tip (stored in pindexNewTip). + // We check shutdown only after giving ActivateBestChainStep a chance to run once so that we + // never shutdown before connecting the genesis block during LoadChainTip(). Previously this + // caused an assert() failure during shutdown in such cases as the UTXO DB flushing checks + // that the best block hash is non-null. + if (ShutdownRequested()) + break; } while (pindexMostWork != chainActive.Tip()); CheckBlockIndex(); From 8d15cf58ca0fd3f97fd6bcaf671b2b7e9f62caa9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 1 Apr 2021 17:46:22 -0300 Subject: [PATCH 06/10] Optimize ActivateBestChain for long chains --- src/validation.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index ef9a3e728d69..3ec2b5773ccc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2138,12 +2138,11 @@ static void PruneBlockIndexCandidates() * Try to make some progress towards making pindexMostWork the active block. * pblock is either NULL or a pointer to a CBlock corresponding to pindexMostWork. */ -static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool fAlreadyChecked, ConnectTrace& connectTrace) +static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool fAlreadyChecked, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); if (pblock == NULL) fAlreadyChecked = false; - bool fInvalidFound = false; const CBlockIndex* pindexOldTip = chainActive.Tip(); const CBlockIndex* pindexFork = chainActive.FindFork(pindexMostWork); @@ -2246,16 +2245,23 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked CBlockIndex *pindexOldTip = chainActive.Tip(); - pindexMostWork = FindMostWorkChain(); + if (pindexMostWork == nullptr) { + pindexMostWork = FindMostWorkChain(); + } // Whether we have anything to do at all. - if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip()) + if (pindexMostWork == nullptr || pindexMostWork == chainActive.Tip()) return true; + bool fInvalidFound = false; std::shared_ptr nullBlockPtr; - if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fAlreadyChecked, connectTrace)) + if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fAlreadyChecked, fInvalidFound, connectTrace)) return false; + if (fInvalidFound) { + // Wipe cache, we may need another branch now. + pindexMostWork = nullptr; + } pindexNewTip = chainActive.Tip(); pindexFork = chainActive.FindFork(pindexOldTip); fInitialDownload = IsInitialBlockDownload(); From 198f43516a8581ce16843b0bf8c5675fc75088d6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 9 Oct 2017 11:19:10 -0400 Subject: [PATCH 07/10] Do not unlock cs_main in ABC unless we've actually made progress. Technically, some internal datastructures may be in an inconsistent state if we do this, though there are no known bugs there. Still, for future safety, its much better to only unlock cs_main if we've made progress (not just tried a reorg which may make progress). --- src/validation.cpp | 64 ++++++++++++++++++++++----------------- src/validationinterface.h | 6 +++- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 3ec2b5773ccc..8bc8b7d96f6c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2238,45 +2238,53 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb SyncWithValidationInterfaceQueue(); } - const CBlockIndex *pindexFork; - bool fInitialDownload; { LOCK(cs_main); - ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked + CBlockIndex* starting_tip = chainActive.Tip(); + bool blocks_connected = false; + do { + // We absolutely may not unlock cs_main until we've made forward progress + // (with the exception of shutdown due to hardware issues, low disk space, etc). + ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked + + if (pindexMostWork == nullptr) { + pindexMostWork = FindMostWorkChain(); + } - CBlockIndex *pindexOldTip = chainActive.Tip(); - if (pindexMostWork == nullptr) { - pindexMostWork = FindMostWorkChain(); - } + // Whether we have anything to do at all. + if (pindexMostWork == nullptr || pindexMostWork == chainActive.Tip()) { + break; + } - // Whether we have anything to do at all. - if (pindexMostWork == nullptr || pindexMostWork == chainActive.Tip()) - return true; + bool fInvalidFound = false; + std::shared_ptr nullBlockPtr; + if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fAlreadyChecked, fInvalidFound, connectTrace)) + return false; + blocks_connected = true; - bool fInvalidFound = false; - std::shared_ptr nullBlockPtr; - if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fAlreadyChecked, fInvalidFound, connectTrace)) - return false; + if (fInvalidFound) { + // Wipe cache, we may need another branch now. + pindexMostWork = nullptr; + } + pindexNewTip = chainActive.Tip(); - if (fInvalidFound) { - // Wipe cache, we may need another branch now. - pindexMostWork = nullptr; - } - pindexNewTip = chainActive.Tip(); - pindexFork = chainActive.FindFork(pindexOldTip); - fInitialDownload = IsInitialBlockDownload(); + for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { + assert(trace.pblock && trace.pindex); + GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); + } + } while (!chainActive.Tip() || (starting_tip && CBlockIndexWorkComparator()(chainActive.Tip(), starting_tip))); + if (!blocks_connected) return true; - for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { - assert(trace.pblock && trace.pindex); - GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); - } + const CBlockIndex* pindexFork = chainActive.FindFork(starting_tip); + bool fInitialDownload = IsInitialBlockDownload(); // Notify external listeners about the new tip. // Enqueue while holding cs_main to ensure that UpdatedBlockTip is called in the order in which blocks are connected - GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); - - // Always notify the UI if a new block tip was connected if (pindexFork != pindexNewTip) { + // Notify ValidationInterface subscribers + GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); + + // Always notify the UI if a new block tip was connected uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); } } diff --git a/src/validationinterface.h b/src/validationinterface.h index 008ec204b8f6..91ec2ee5b46c 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -74,7 +74,11 @@ class CValidationInterface { virtual ~CValidationInterface() = default; protected: /** - * Notifies listeners of updated block chain tip + * Notifies listeners when the block chain tip advances. + * + * When multiple blocks are connected at once, UpdatedBlockTip will be called on the final tip + * but may not be called on every intermediate tip. If the latter behavior is desired, + * subscribe to BlockConnected() instead. * * Called on a background thread. */ From a51a75529ad2dfcb1b47a346ba7656baa367bcfb Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Thu, 1 Apr 2021 18:05:10 -0300 Subject: [PATCH 08/10] Fix concurrency-related bugs in ActivateBestChain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If multiple threads are invoking ActivateBestChain, it was possible to have them working towards different tips, and we could arrive at a less work tip than we should.  Fix this by introducing a ChainState lock which must be held for the entire duration of ActivateBestChain to enforce exclusion in ABC. --- src/validation.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 8bc8b7d96f6c..417abbbf7018 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -147,11 +147,17 @@ struct CBlockIndexWorkComparator { CBlockIndex* pindexBestInvalid; /** - * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and - * as good as our current tip or better. Entries may be failed, though. - */ + * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and + * as good as our current tip or better. Entries may be failed, though. + */ std::set setBlockIndexCandidates; +/** + * the ChainState Mutex + * A lock that must be held when modifying this ChainState - held in ActivateBestChain() + */ +Mutex m_cs_chainstate; + /** All pairs A->B, where A (or one if its ancestors) misses transactions, but B has transactions. */ std::multimap mapBlocksUnlinked; @@ -2226,6 +2232,12 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb // sanely for performance or correctness! AssertLockNotHeld(cs_main); + // ABC maintains a fair degree of expensive-to-calculate internal state + // because this function periodically releases cs_main so that it does not lock up other threads for too long + // during large connects - and to allow for e.g. the callback queue to drain + // we use m_cs_chainstate to enforce mutual exclusion so that only one caller may execute this function at a time + LOCK(m_cs_chainstate); + CBlockIndex* pindexNewTip = nullptr; CBlockIndex* pindexMostWork = nullptr; do { From f68251d9316f319c84fb92d5d659bbd3d88607c0 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 2 Apr 2021 12:51:15 -0300 Subject: [PATCH 09/10] Validation: rename one of the two instances using "bad-prevblk" to its correct description of "prevblk-not-found" --- src/validation.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 417abbbf7018..59ff2cc0d127 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2949,8 +2949,10 @@ bool GetPrevIndex(const CBlock& block, CBlockIndex** pindexPrevRet, CValidationS pindexPrev = nullptr; if (block.GetHash() != Params().GetConsensus().hashGenesisBlock) { BlockMap::iterator mi = mapBlockIndex.find(block.hashPrevBlock); - if (mi == mapBlockIndex.end()) - return state.DoS(0, error("%s : prev block %s not found", __func__, block.hashPrevBlock.GetHex()), 0, "bad-prevblk"); + if (mi == mapBlockIndex.end()) { + return state.DoS(0, error("%s : prev block %s not found", __func__, block.hashPrevBlock.GetHex()), 0, + "prevblk-not-found"); + } pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { //If this "invalid" block is an exact match from the checkpoints, then reconsider it From 50dbec59277e25f3e24f447773c12b60f360427a Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 2 Apr 2021 12:49:49 -0300 Subject: [PATCH 10/10] Add unit tests for signals generated by ProcessNewBlock() After a recent bug discovered in callback ordering in MainSignals, this test checks invariants in ordering of BlockConnected / BlockDisconnected / UpdatedChainTip signals Adaptation of btc@dd435ad40267f5c50ff17533c696f9302829a6a6 --- src/Makefile.test.include | 3 +- src/blockassembler.h | 4 +- src/test/CMakeLists.txt | 1 + src/test/test_pivx.cpp | 6 + src/test/test_pivx.h | 3 + src/test/validation_block_tests.cpp | 178 ++++++++++++++++++++++++++++ 6 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 src/test/validation_block_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 7f0c51f29148..d4f6a77eb346 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -101,7 +101,8 @@ BITCOIN_TESTS =\ test/univalue_tests.cpp \ test/util_tests.cpp \ test/sha256compress_tests.cpp \ - test/upgrades_tests.cpp + test/upgrades_tests.cpp \ + test/validation_block_tests.cpp SAPLING_TESTS =\ test/librust/libsapling_utils_tests.cpp \ diff --git a/src/blockassembler.h b/src/blockassembler.h index eaf183dec6a0..753b10048201 100644 --- a/src/blockassembler.h +++ b/src/blockassembler.h @@ -66,8 +66,8 @@ class BlockAssembler BlockAssembler(const CChainParams& chainparams, const bool defaultPrintPriority); /** Construct a new block template with coinbase to scriptPubKeyIn */ std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn, - CWallet* pwallet, - bool fProofOfStake, + CWallet* pwallet = nullptr, + bool fProofOfStake = false, std::vector* availableCoins = nullptr); private: diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 98d670fbc5e8..b80d39141f2a 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -133,6 +133,7 @@ set(BITCOIN_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/validation_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/sha256compress_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/upgrades_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/validation_block_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/librust/sapling_rpc_wallet_tests.cpp ${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_tests.cpp ${CMAKE_SOURCE_DIR}/src/wallet/test/crypto_tests.cpp diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 2d4b2c3b5423..adad49eb9192 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -32,6 +32,12 @@ FastRandomContext insecure_rand_ctx(insecure_rand_seed); extern bool fPrintToConsole; extern void noui_connect(); +std::ostream& operator<<(std::ostream& os, const uint256& num) +{ + os << num.ToString(); + return os; +} + BasicTestingSetup::BasicTestingSetup() { RandomInit(); diff --git a/src/test/test_pivx.h b/src/test/test_pivx.h index d22da8ecd1bd..251c9ffb929d 100644 --- a/src/test/test_pivx.h +++ b/src/test/test_pivx.h @@ -117,4 +117,7 @@ struct TestMemPoolEntryHelper TestMemPoolEntryHelper &SigOps(unsigned int _sigops) { sigOpCount = _sigops; return *this; } }; +// define an implicit conversion here so that uint256 may be used directly in BOOST_CHECK_* +std::ostream& operator<<(std::ostream& os, const uint256& num); + #endif diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp new file mode 100644 index 000000000000..dba3c04f1b63 --- /dev/null +++ b/src/test/validation_block_tests.cpp @@ -0,0 +1,178 @@ +// Copyright (c) 2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include + +#include "blockassembler.h" +#include "chainparams.h" +#include "consensus/merkle.h" +#include "consensus/validation.h" +#include "pow.h" +#include "random.h" +#include "test/test_pivx.h" +#include "validation.h" +#include "validationinterface.h" + +struct RegtestingSetup : public TestingSetup { + RegtestingSetup() : TestingSetup() { + SelectParams(CBaseChainParams::REGTEST); + } +}; + +BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegtestingSetup) + +struct TestSubscriber : public CValidationInterface { + uint256 m_expected_tip; + + TestSubscriber(uint256 tip) : m_expected_tip(std::move(tip)) {} + + void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) + { + BOOST_CHECK_EQUAL(m_expected_tip, pindexNew->GetBlockHash()); + } + + void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex, const std::vector& txnConflicted) + { + BOOST_CHECK_EQUAL(m_expected_tip, block->hashPrevBlock); + BOOST_CHECK_EQUAL(m_expected_tip, pindex->pprev->GetBlockHash()); + + m_expected_tip = block->GetHash(); + } + + void BlockDisconnected(const std::shared_ptr &block, const uint256& blockHash, int nBlockHeight, int64_t blockTime) + { + BOOST_CHECK_EQUAL(m_expected_tip, block->GetHash()); + + m_expected_tip = block->hashPrevBlock; + } +}; + +std::shared_ptr Block(const uint256& prev_hash) +{ + static int i = 0; + static uint64_t time = Params().GenesisBlock().nTime; + + CScript pubKey; + pubKey << i++ << OP_TRUE; + + auto ptemplate = BlockAssembler(Params(), false).CreateNewBlock(pubKey); + auto pblock = std::make_shared(ptemplate->block); + pblock->hashPrevBlock = prev_hash; + pblock->nTime = ++time; + + CMutableTransaction txCoinbase(*pblock->vtx[0]); + txCoinbase.vout.resize(1); + pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); + + return pblock; +} + +std::shared_ptr FinalizeBlock(std::shared_ptr pblock) +{ + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); + + while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits)) { + ++(pblock->nNonce); + } + + return pblock; +} + +// construct a valid block +const std::shared_ptr GoodBlock(const uint256& prev_hash) +{ + return FinalizeBlock(Block(prev_hash)); +} + +// construct an invalid block (but with a valid header) +const std::shared_ptr BadBlock(const uint256& prev_hash) +{ + auto pblock = Block(prev_hash); + + CMutableTransaction coinbase_spend; + coinbase_spend.vin.emplace_back(CTxIn(COutPoint(pblock->vtx[0]->GetHash(), 0), CScript(), 0)); + coinbase_spend.vout.emplace_back(pblock->vtx[0]->vout[0]); + + CTransactionRef tx = MakeTransactionRef(coinbase_spend); + pblock->vtx.emplace_back(tx); + + auto ret = FinalizeBlock(pblock); + return ret; +} + +void BuildChain(const uint256& root, int height, const unsigned int invalid_rate, const unsigned int branch_rate, const unsigned int max_size, std::vector>& blocks) +{ + if (height <= 0 || blocks.size() >= max_size) return; + + bool gen_invalid = GetRand(100) < invalid_rate; + bool gen_fork = GetRand(100) < branch_rate; + + const std::shared_ptr pblock = gen_invalid ? BadBlock(root) : GoodBlock(root); + blocks.emplace_back(pblock); + if (!gen_invalid) { + BuildChain(pblock->GetHash(), height - 1, invalid_rate, branch_rate, max_size, blocks); + } + + if (gen_fork) { + blocks.emplace_back(GoodBlock(root)); + BuildChain(blocks.back()->GetHash(), height - 1, invalid_rate, branch_rate, max_size, blocks); + } +} + +BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) +{ + // build a large-ish chain that's likely to have some forks + std::vector> blocks; + while (blocks.size() < 50) { + blocks.clear(); + BuildChain(Params().GenesisBlock().GetHash(), 100, 15, 10, 500, blocks); + } + + CValidationState state; + // Connect the genesis block and drain any outstanding events + BOOST_CHECK_MESSAGE(ProcessNewBlock(state, nullptr, std::make_shared(Params().GenesisBlock()), nullptr), "Error: genesis not connected"); + SyncWithValidationInterfaceQueue(); + + // subscribe to events (this subscriber will validate event ordering) + const CBlockIndex* initial_tip = WITH_LOCK(cs_main, return chainActive.Tip()); + TestSubscriber sub(initial_tip->GetBlockHash()); + RegisterValidationInterface(&sub); + + // create a bunch of threads that repeatedly process a block generated above at random + // this will create parallelism and randomness inside validation - the ValidationInterface + // will subscribe to events generated during block validation and assert on ordering invariance + boost::thread_group threads; + for (int i = 0; i < 10; i++) { + threads.create_thread([&blocks]() { + CValidationState state; + for (int i = 0; i < 1000; i++) { + auto block = blocks[GetRand(blocks.size() - 1)]; + ProcessNewBlock(state, nullptr, block, nullptr); + } + + // to make sure that eventually we process the full chain - do it here + for (const auto& block : blocks) { + if (block->vtx.size() == 1) { + bool processed = ProcessNewBlock(state, nullptr, block, nullptr); + // Future to do: "prevblk-not-found" here is the only valid reason to not check processed flag. + if (state.GetRejectReason() == "duplicate" || + state.GetRejectReason() == "prevblk-not-found" || + state.GetRejectReason() == "bad-prevblk") continue; + BOOST_ASSERT_MSG(processed, ("Error: " + state.GetRejectReason()).c_str()); + } + } + }); + } + + threads.join_all(); + while (GetMainSignals().CallbacksPending() > 0) { + MilliSleep(100); + } + + UnregisterValidationInterface(&sub); + + BOOST_CHECK_EQUAL(sub.m_expected_tip, WITH_LOCK(cs_main, return chainActive.Tip()->GetBlockHash())); +} + +BOOST_AUTO_TEST_SUITE_END()