diff --git a/doc/README.md b/doc/README.md index 9d607ca762b4..041aa96785bf 100644 --- a/doc/README.md +++ b/doc/README.md @@ -64,6 +64,7 @@ The PIVX repo's [root README](/README.md) contains relevant information on the d ### Miscellaneous - [Assets Attribution](assets-attribution.md) - [Files](files.md) +- [Reduce Memory](reduce-memory.md) - [Tor Support](tor.md) - [Init Scripts (systemd/upstart/openrc)](init.md) diff --git a/doc/REST-interface.md b/doc/REST-interface.md index f4fb8ee57cfe..a9eed451e637 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -79,6 +79,7 @@ $ curl localhost:18332/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76 Returns various information about the TX mempool. Only supports JSON as output format. +* loaded : (boolean) if the mempool is fully loaded * size : (numeric) the number of transactions in the TX mempool * bytes : (numeric) size of the TX mempool in bytes diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md new file mode 100644 index 000000000000..dd137aef3cda --- /dev/null +++ b/doc/reduce-memory.md @@ -0,0 +1,45 @@ +# Reduce Memory + +There are a few parameters that can be dialed down to reduce the memory usage of `pivxd`. This can be useful on embedded systems or small VPSes. + +## In-memory caches + +The size of some in-memory caches can be reduced. As caches trade off memory usage for performance, reducing these will usually have a negative effect on performance. + +- `-dbcache=` - the UTXO database cache size, this defaults to `450`. The unit is MiB (1024). + - The minimum value for `-dbcache` is 4. + - A lower `-dbcache` makes initial sync time much longer. After the initial sync, the effect is less pronounced for most use-cases, unless fast validation of blocks is important, such as for mining. + +## Memory pool + +- In PIVX Core there is a memory pool limiter which can be configured with `-maxmempool=`, where `` is the size in MB (1000). The default value is `300`. + - The minimum value for `-maxmempool` is 5. + - A lower maximum mempool size means that transactions will be evicted sooner. This will affect any uses of `pivxd` that process unconfirmed transactions. + +- Unused memory allocated to the mempool (default: 300MB) is shared with the UTXO cache, so when trying to reduce memory usage you should limit the mempool, with the `-maxmempool` command line argument. + +## Number of peers + +- `-maxconnections=` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming + connections are enabled, otherwise the number of connections will never be more than `8`. + +## Thread configuration + +For each thread a thread stack needs to be allocated. By default on Linux, +threads take up 8MiB for the thread stack on a 64-bit system, and 4MiB in a +32-bit system. + +- `-par=` - the number of script verification threads, defaults to the number of cores in the system minus one. +- `-rpcthreads=` - the number of threads used for processing RPC requests, defaults to `4`. + +## Linux specific + +By default, since glibc `2.10`, the C library will create up to two heap arenas per core. This is known to cause excessive memory usage in some scenarios. To avoid this make a script that sets `MALLOC_ARENA_MAX` before starting pivxd: + +```bash +#!/usr/bin/env bash +export MALLOC_ARENA_MAX=1 +pivxd +``` + +The behavior was introduced to increase CPU locality of allocated memory and performance with concurrent allocation, so this setting could in theory reduce performance. However, in PIVX Core very little parallel allocation happens, so the impact is expected to be small or absent. 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.cpp b/src/blockassembler.cpp index e4fab31464c8..7e8da0663db5 100644 --- a/src/blockassembler.cpp +++ b/src/blockassembler.cpp @@ -226,7 +226,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc if (chainActive.Tip() != pindexPrev) return nullptr; // new block came in, move on CValidationState state; - if (!TestBlockValidity(state, *pblock, pindexPrev, false, false)) { + if (!TestBlockValidity(state, *pblock, pindexPrev, false, false, false)) { throw std::runtime_error( strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state))); } 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/blocksignature.cpp b/src/blocksignature.cpp index dedf764f83ff..df698702803d 100644 --- a/src/blocksignature.cpp +++ b/src/blocksignature.cpp @@ -40,7 +40,7 @@ bool SignBlock(CBlock& block, const CKeyStore& keystore) return SignBlockWithKey(block, key); } -bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH) +bool CheckBlockSignature(const CBlock& block) { if (block.IsProofOfWork()) return block.vchBlockSig.empty(); @@ -64,13 +64,6 @@ bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH) if (!Solver(txout.scriptPubKey, whichType, vSolutions)) return false; - if (!enableP2PKH) { - // Before v5 activation, P2PKH was always failing. - if (whichType == TX_PUBKEYHASH) { - return false; - } - } - if (whichType == TX_PUBKEY) { valtype& vchPubKey = vSolutions[0]; pubkey = CPubKey(vchPubKey); diff --git a/src/blocksignature.h b/src/blocksignature.h index feed6af20d5a..ad399b85dbc0 100644 --- a/src/blocksignature.h +++ b/src/blocksignature.h @@ -11,6 +11,6 @@ bool SignBlockWithKey(CBlock& block, const CKey& key); bool SignBlock(CBlock& block, const CKeyStore& keystore); -bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH); +bool CheckBlockSignature(const CBlock& block); #endif //PIVX_BLOCKSIGNATURE_H diff --git a/src/guiinterface.h b/src/guiinterface.h index 03307315948e..2d2053681028 100644 --- a/src/guiinterface.h +++ b/src/guiinterface.h @@ -100,9 +100,6 @@ class CClientUIInterface /** New block has been accepted */ boost::signals2::signal NotifyBlockTip; - /** New block has been accepted and is over a certain size */ - boost::signals2::signal NotifyBlockSize; - /** Banlist did change. */ boost::signals2::signal BannedListChanged; }; diff --git a/src/init.cpp b/src/init.cpp index e47aa2aa0b02..d7b4cbbcf53a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -248,8 +248,8 @@ void PrepareShutdown() DumpBudgets(g_budgetman); DumpMasternodePayments(); UnregisterNodeSignals(GetNodeSignals()); - if (g_is_mempool_loaded && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - DumpMempool(); + if (::mempool.IsLoaded() && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + DumpMempool(::mempool); } if (fFeeEstimatesInitialized) { @@ -422,7 +422,6 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-version", _("Print version and exit")); strUsage += HelpMessageOpt("-alertnotify=", _("Execute command when a relevant alert is received or we see a really long fork (%s in cmd is replaced by message)")); strUsage += HelpMessageOpt("-blocknotify=", _("Execute command when the best block changes (%s in cmd is replaced by block hash)")); - strUsage += HelpMessageOpt("-blocksizenotify=", _("Execute command when the best block changes and its size is over (%s in cmd is replaced by block hash, %d with the block size)")); strUsage += HelpMessageOpt("-checkblocks=", strprintf(_("How many blocks to check at startup (default: %u, 0 = all)"), DEFAULT_CHECKBLOCKS)); strUsage += HelpMessageOpt("-conf=", strprintf(_("Specify configuration file (default: %s)"), PIVX_CONF_FILENAME)); if (mode == HMM_BITCOIND) { @@ -622,16 +621,6 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex } } -static void BlockSizeNotifyCallback(int size, const uint256& hashNewTip) -{ - std::string strCmd = gArgs.GetArg("-blocksizenotify", ""); - - boost::replace_all(strCmd, "%s", hashNewTip.GetHex()); - boost::replace_all(strCmd, "%d", std::to_string(size)); - std::thread t(runCommand, strCmd); - t.detach(); // thread runs free -} - //////////////////////////////////////////////////// static bool fHaveGenesis = false; @@ -728,9 +717,9 @@ void ThreadImport(const std::vector& vImportFiles) } if (gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - LoadMempool(); + LoadMempool(::mempool); } - g_is_mempool_loaded = !fRequestShutdown; + ::mempool.SetIsLoaded(!ShutdownRequested()); } /** Sanity checks @@ -1786,9 +1775,6 @@ bool AppInitMain() if (gArgs.IsArgSet("-blocknotify")) uiInterface.NotifyBlockTip.connect(BlockNotifyCallback); - if (gArgs.IsArgSet("-blocksizenotify")) - uiInterface.NotifyBlockSize.connect(BlockSizeNotifyCallback); - // scan for better chains in the block chain database, that are not yet connected in the active best chain CValidationState state; if (!ActivateBestChain(state)) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 907cca1cab74..49c898edf2b6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1599,16 +1599,17 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR CInv inv(MSG_BLOCK, hashBlock); LogPrint(BCLog::NET, "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id); - //sometimes we will be sent their most recent block and its not the one we want, in that case tell where we are + // sometimes we will be sent their most recent block and its not the one we want, in that case tell where we are if (!mapBlockIndex.count(pblock->hashPrevBlock)) { + CBlockLocator locator = WITH_LOCK(cs_main, return chainActive.GetLocator();); if (find(pfrom->vBlockRequested.begin(), pfrom->vBlockRequested.end(), hashBlock) != pfrom->vBlockRequested.end()) { - //we already asked for this block, so lets work backwards and ask for the previous block - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(), pblock->hashPrevBlock)); - pfrom->vBlockRequested.push_back(pblock->hashPrevBlock); + // we already asked for this block, so lets work backwards and ask for the previous block + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, pblock->hashPrevBlock)); + pfrom->vBlockRequested.emplace_back(pblock->hashPrevBlock); } else { - //ask to sync to this block - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(), hashBlock)); - pfrom->vBlockRequested.push_back(hashBlock); + // ask to sync to this block + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKS, locator, hashBlock)); + pfrom->vBlockRequested.emplace_back(hashBlock); } } else { pfrom->AddInventoryKnown(inv); diff --git a/src/primitives/block.h b/src/primitives/block.h index ed86e531daf8..aee2abe93c92 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -94,7 +94,7 @@ class CBlock : public CBlockHeader std::vector vchBlockSig; // memory only - mutable bool fChecked; + mutable bool fChecked{false}; CBlock() { diff --git a/src/qt/pivx/balancebubble.cpp b/src/qt/pivx/balancebubble.cpp index 631d21d94acc..7ecbacc8a559 100644 --- a/src/qt/pivx/balancebubble.cpp +++ b/src/qt/pivx/balancebubble.cpp @@ -44,12 +44,12 @@ void BalanceBubble::showEvent(QShowEvent *event) { QGraphicsOpacityEffect *eff = new QGraphicsOpacityEffect(this); this->setGraphicsEffect(eff); - QPropertyAnimation *a = new QPropertyAnimation(eff,"opacity"); - a->setDuration(400); - a->setStartValue(0.1); - a->setEndValue(1); - a->setEasingCurve(QEasingCurve::InBack); - a->start(QPropertyAnimation::DeleteWhenStopped); + QPropertyAnimation *anim = new QPropertyAnimation(eff,"opacity"); + anim->setDuration(400); + anim->setStartValue(0); + anim->setEndValue(1); + anim->setEasingCurve(QEasingCurve::Linear); + anim->start(QPropertyAnimation::DeleteWhenStopped); if (!hideTimer) hideTimer = new QTimer(this); connect(hideTimer, &QTimer::timeout, this, &BalanceBubble::hideTimeout); @@ -77,4 +77,4 @@ void BalanceBubble::hideTimeout() BalanceBubble::~BalanceBubble() { delete ui; -} \ No newline at end of file +} diff --git a/src/qt/pivx/sendcustomfeedialog.cpp b/src/qt/pivx/sendcustomfeedialog.cpp index 21179611b9dd..9712b8d224df 100644 --- a/src/qt/pivx/sendcustomfeedialog.cpp +++ b/src/qt/pivx/sendcustomfeedialog.cpp @@ -123,9 +123,17 @@ void SendCustomFeeDialog::accept() // Check insane fee const CAmount insaneFee = ::minRelayTxFee.GetFeePerK() * 10000; if (customFee >= insaneFee) { + ui->lineEditCustomFee->setText(BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), insaneFee - CWallet::GetRequiredFee(1000))); inform(tr("Fee too high. Must be below: %1").arg( BitcoinUnits::formatWithUnit(walletModel->getOptionsModel()->getDisplayUnit(), insaneFee))); } else if (customFee < CWallet::GetRequiredFee(1000)) { + CAmount nFee = 0; + if (walletModel->hasWalletCustomFee()) { + walletModel->getWalletCustomFee(nFee); + } else { + nFee = CWallet::GetRequiredFee(1000); + } + ui->lineEditCustomFee->setText(BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), nFee)); inform(tr("Fee too low. Must be at least: %1").arg( BitcoinUnits::formatWithUnit(walletModel->getOptionsModel()->getDisplayUnit(), CWallet::GetRequiredFee(1000)))); } else { diff --git a/src/qt/pivx/sendmultirow.cpp b/src/qt/pivx/sendmultirow.cpp index a4ed7d309356..431bcc6ed88b 100644 --- a/src/qt/pivx/sendmultirow.cpp +++ b/src/qt/pivx/sendmultirow.cpp @@ -149,6 +149,10 @@ bool SendMultiRow::addressChanged(const QString& str, bool fOnlyValidate) updateStyle(ui->lineEditAddress); return valid; } + + setCssProperty(ui->lineEditAddress, "edit-primary-multi-book"); + updateStyle(ui->lineEditAddress); + return false; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index eb900d1dffb9..c3a37a41896d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -14,6 +14,7 @@ #include "kernel.h" #include "masternodeman.h" #include "policy/feerate.h" +#include "policy/policy.h" #include "rpc/server.h" #include "sync.h" #include "txdb.h" @@ -1149,9 +1150,13 @@ UniValue getchaintips(const JSONRPCRequest& request) UniValue mempoolInfoToJSON() { UniValue ret(UniValue::VOBJ); + ret.pushKV("loaded", mempool.IsLoaded()); ret.pushKV("size", (int64_t) mempool.size()); ret.pushKV("bytes", (int64_t) mempool.GetTotalTxSize()); ret.pushKV("usage", (int64_t) mempool.DynamicMemoryUsage()); + size_t maxmempool = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; + ret.pushKV("mempoolminfee", ValueFromAmount(std::max(mempool.GetMinFee(maxmempool), ::minRelayTxFee).GetFeePerK())); + ret.pushKV("minrelaytxfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())); return ret; } @@ -1165,9 +1170,13 @@ UniValue getmempoolinfo(const JSONRPCRequest& request) "\nResult:\n" "{\n" + " \"loaded\": true|false (boolean) True if the mempool is fully loaded\n" " \"size\": xxxxx (numeric) Current tx count\n" " \"bytes\": xxxxx (numeric) Sum of all tx sizes\n" " \"usage\": xxxxx (numeric) Total memory usage for the mempool\n" + " \"maxmempool\": xxxxx, (numeric) Maximum memory usage for the mempool\n" + " \"mempoolminfee\": xxxxx (numeric) Minimum fee rate in " + CURRENCY_UNIT + "/kB for tx to be accepted. Is the maximum of minrelaytxfee and minimum mempool fee\n" + " \"minrelaytxfee\": xxxxx (numeric) Current minimum relay fee for transactions\n" "}\n" "\nExamples:\n" + 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 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/main_tests.cpp b/src/test/main_tests.cpp index 00d6ef62ab57..6e5a9e97f8f0 100644 --- a/src/test/main_tests.cpp +++ b/src/test/main_tests.cpp @@ -72,14 +72,9 @@ CBlock CreateDummyBlockWithSignature(CKey stakingKey, BlockSignatureType type, b return block; } -bool TestBlockSignaturePreEnforcementV5(const CBlock& block) +bool TestBlockSignature(const CBlock& block) { - return CheckBlockSignature(block, false); -} - -bool TestBlockSignaturePostEnforcementV5(const CBlock& block) -{ - return CheckBlockSignature(block, true); + return CheckBlockSignature(block); } BOOST_AUTO_TEST_CASE(block_signature_test) @@ -89,27 +84,19 @@ BOOST_AUTO_TEST_CASE(block_signature_test) stakingKey.MakeNewKey(true); bool useInputP2PK = i % 2 == 0; - // Test P2PK block signature pre enforcement. + // Test P2PK block signature CBlock block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PK, useInputP2PK); - BOOST_CHECK(TestBlockSignaturePreEnforcementV5(block)); - - // Test P2PK block signature post enforcement - block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PK, useInputP2PK); - BOOST_CHECK(TestBlockSignaturePostEnforcementV5(block)); - - // Test P2PKH block signature pre enforcement ---> must fail. - block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PKH, useInputP2PK); - BOOST_CHECK(!TestBlockSignaturePreEnforcementV5(block)); + BOOST_CHECK(TestBlockSignature(block)); - // Test P2PKH block signature post enforcement + // Test P2PKH block signature block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PKH, useInputP2PK); if (useInputP2PK) { // If it's using a P2PK scriptsig as input and a P2PKH output // The block doesn't contain the public key to verify the sig anywhere. // Must fail. - BOOST_CHECK(!TestBlockSignaturePostEnforcementV5(block)); + BOOST_CHECK(!TestBlockSignature(block)); } else { - BOOST_CHECK(TestBlockSignaturePostEnforcementV5(block)); + BOOST_CHECK(TestBlockSignature(block)); } } } 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() 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() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 66f9b16f7baf..91747ba11f4e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1213,3 +1213,15 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends } +bool CTxMemPool::IsLoaded() const +{ + LOCK(cs); + return m_is_loaded; +} + +void CTxMemPool::SetIsLoaded(bool loaded) +{ + LOCK(cs); + m_is_loaded = loaded; +} + diff --git a/src/txmempool.h b/src/txmempool.h index 635606d2aa97..9a3218ea3f55 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -419,6 +419,8 @@ class CTxMemPool std::map mapSaplingNullifiers; void checkNullifiers() const; + bool m_is_loaded GUARDED_BY(cs){false}; + public: static const int ROLLING_FEE_HALFLIFE = 60 * 60 * 12; // public only for testing @@ -618,7 +620,13 @@ class CTxMemPool /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ int Expire(int64_t time); - unsigned long size() + /** @returns true if the mempool is fully loaded */ + bool IsLoaded() const; + + /** Sets the current loaded state */ + void SetIsLoaded(bool loaded); + + unsigned long size() const { LOCK(cs); return mapTx.size(); diff --git a/src/validation.cpp b/src/validation.cpp index 71cd45954f02..ee6b90377b8a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -110,7 +110,6 @@ int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; CFeeRate minRelayTxFee = CFeeRate(10000); CTxMemPool mempool(::minRelayTxFee); -std::atomic_bool g_is_mempool_loaded{false}; std::map mapRejectedBlocks; @@ -148,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; @@ -1367,11 +1372,11 @@ static int64_t nTimeTotal = 0; /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ -static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck = false, bool fAlreadyChecked = false) +static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck = false) { AssertLockHeld(cs_main); // Check it again in case a previous version let a bad block in - if (!fAlreadyChecked && !CheckBlock(block, state, !fJustCheck, !fJustCheck)) { + if (!CheckBlock(block, state, !fJustCheck, !fJustCheck, !fJustCheck)) { if (state.CorruptionPossible()) { // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware @@ -1992,13 +1997,10 @@ class ConnectTrace { * * The block is added to connectTrace if connection succeeds. */ -bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr& pblock, bool fAlreadyChecked, ConnectTrace& connectTrace) +bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace) { assert(pindexNew->pprev == chainActive.Tip()); - if (pblock == NULL) - fAlreadyChecked = false; - // Read block from disk. int64_t nTime1 = GetTimeMicros(); std::shared_ptr pthisBlock; @@ -2019,7 +2021,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st LogPrint(BCLog::BENCH, " - Load block from disk: %.2fms [%.2fs]\n", (nTime2 - nTime1) * 0.001, nTimeReadFromDisk * 0.000001); { CCoinsViewCache view(pcoinsTip); - bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, false, fAlreadyChecked); + bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, false); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { if (state.IsInvalid()) @@ -2139,12 +2141,9 @@ 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& 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); @@ -2175,7 +2174,7 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo // Connect new blocks. for (CBlockIndex* pindexConnect : reverse_iterate(vpindexToConnect)) { - if (!ConnectTip(state, pindexConnect, (pindexConnect == pindexMostWork) ? pblock : std::shared_ptr(), fAlreadyChecked, connectTrace)) { + if (!ConnectTip(state, pindexConnect, (pindexConnect == pindexMostWork) ? pblock : std::shared_ptr(), connectTrace)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) @@ -2220,7 +2219,7 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo * that is already loaded (to avoid loading it again from disk). */ -bool ActivateBestChain(CValidationState& state, std::shared_ptr pblock, bool fAlreadyChecked) +bool ActivateBestChain(CValidationState& state, std::shared_ptr pblock) { // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling @@ -2228,6 +2227,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 { @@ -2240,57 +2245,64 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb SyncWithValidationInterfaceQueue(); } - const CBlockIndex *pindexFork; - bool fInitialDownload; - while (true) { - TRY_LOCK(cs_main, lockMain); - if (!lockMain) { - MilliSleep(50); - continue; - } - ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked - - CBlockIndex *pindexOldTip = chainActive.Tip(); - pindexMostWork = FindMostWorkChain(); - - // Whether we have anything to do at all. - if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip()) - return true; - - std::shared_ptr nullBlockPtr; - if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fAlreadyChecked, connectTrace)) - return false; + { + LOCK(cs_main); + 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(); + } - pindexNewTip = chainActive.Tip(); - pindexFork = chainActive.FindFork(pindexOldTip); - fInitialDownload = IsInitialBlockDownload(); + // Whether we have anything to do at all. + if (pindexMostWork == nullptr || pindexMostWork == chainActive.Tip()) { + break; + } - for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { - assert(trace.pblock && trace.pindex); - GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); - } + bool fInvalidFound = false; + std::shared_ptr nullBlockPtr; + if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) + return false; + blocks_connected = true; - break; - } + if (fInvalidFound) { + // Wipe cache, we may need another branch now. + pindexMostWork = nullptr; + } + pindexNewTip = chainActive.Tip(); - // Notify external listeners about the new tip. - GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); + 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; - // Always notify the UI if a new block tip was connected - if (pindexFork != pindexNewTip) { + const CBlockIndex* pindexFork = chainActive.FindFork(starting_tip); + bool fInitialDownload = IsInitialBlockDownload(); - // Notify the UI - uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); + // 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 + if (pindexFork != pindexNewTip) { + // Notify ValidationInterface subscribers + GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); - unsigned size = 0; - if (pblock) - size = GetSerializeSize(*pblock, SER_NETWORK, PROTOCOL_VERSION); - // If the size is over 1 MB notify external listeners, and it is within the last 5 minutes - if (size > MAX_BLOCK_SIZE_LEGACY && pblock->GetBlockTime() > GetAdjustedTime() - 300) { - uiInterface.NotifyBlockSize(static_cast(size), pindexNewTip->GetBlockHash()); + // Always notify the UI if a new block tip was connected + uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); } } + // 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(); @@ -2759,6 +2771,12 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo return state.DoS(100, error("%s : out-of-bounds SigOpCount", __func__), REJECT_INVALID, "bad-blk-sigops", true); + // Check PoS signature. + if (fCheckSig && !CheckBlockSignature(block)) { + return state.DoS(100, error("%s : bad proof-of-stake block signature", __func__), + REJECT_INVALID, "bad-PoS-sig", true); + } + if (fCheckPOW && fCheckMerkleRoot && fCheckSig) block.fChecked = true; @@ -2932,8 +2950,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 @@ -2991,7 +3011,7 @@ bool AcceptBlockHeader(const CBlock& block, CValidationState& state, CBlockIndex return true; } -bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** ppindex, CDiskBlockPos* dbp, bool fAlreadyCheckedBlock) +bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** ppindex, CDiskBlockPos* dbp) { AssertLockHeld(cs_main); @@ -3024,7 +3044,7 @@ bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** ppi return true; } - if ((!fAlreadyCheckedBlock && !CheckBlock(block, state)) || !ContextualCheckBlock(block, state, pindex->pprev)) { + if (!CheckBlock(block, state) || !ContextualCheckBlock(block, state, pindex->pprev)) { if (state.IsInvalid() && !state.CorruptionPossible()) { pindex->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(pindex); @@ -3244,44 +3264,27 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const std::shared_pt // Preliminary checks int64_t nStartTime = GetTimeMillis(); const Consensus::Params& consensus = Params().GetConsensus(); - - // check block - bool checked = CheckBlock(*pblock, state); - - // For now, we need the tip to know whether p2pkh block signatures are accepted or not. - // After 5.0, this can be removed and replaced by the enforcement block time. - const int newHeight = chainActive.Height() + 1; - const bool enableP2PKH = consensus.NetworkUpgradeActive(newHeight, Consensus::UPGRADE_V5_0); - if (!CheckBlockSignature(*pblock, enableP2PKH)) - return error("%s : bad proof-of-stake block signature", __func__); - - if (pblock->GetHash() != consensus.hashGenesisBlock && pfrom != NULL) { - //if we get this far, check if the prev block is our prev block, if not then request sync and return false - BlockMap::iterator mi = mapBlockIndex.find(pblock->hashPrevBlock); - if (mi == mapBlockIndex.end()) { - g_connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::GETBLOCKS, chainActive.GetLocator(), UINT256_ZERO)); - return false; - } - } + int newHeight = 0; { + // CheckBlock requires cs_main lock LOCK(cs_main); - - if (!checked) { + if (!CheckBlock(*pblock, state)) { return error ("%s : CheckBlock FAILED for block %s, %s", __func__, pblock->GetHash().GetHex(), FormatStateMessage(state)); } // Store to disk CBlockIndex* pindex = nullptr; - bool ret = AcceptBlock(*pblock, state, &pindex, dbp, checked); + bool ret = AcceptBlock(*pblock, state, &pindex, dbp); if (fAccepted) *fAccepted = ret; CheckBlockIndex(); if (!ret) { return error("%s : AcceptBlock FAILED", __func__); } + newHeight = pindex->nHeight; } - if (!ActivateBestChain(state, pblock, checked)) + if (!ActivateBestChain(state, pblock)) return error("%s : ActivateBestChain failed", __func__); if (!fLiteMode) { @@ -3298,7 +3301,7 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const std::shared_pt return true; } -bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* const pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot) +bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* const pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot, bool fCheckBlockSig) { AssertLockHeld(cs_main); assert(pindexPrev); @@ -3315,7 +3318,7 @@ bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex // NOTE: CheckBlockHeader is called by CheckBlock if (!ContextualCheckBlockHeader(block, state, pindexPrev)) return error("%s: ContextualCheckBlockHeader failed: %s", __func__, FormatStateMessage(state)); - if (!CheckBlock(block, state, fCheckPOW, fCheckMerkleRoot)) + if (!CheckBlock(block, state, fCheckPOW, fCheckMerkleRoot, fCheckBlockSig)) return error("%s: CheckBlock failed: %s", __func__, FormatStateMessage(state)); if (!ContextualCheckBlock(block, state, pindexPrev)) return error("%s: ContextualCheckBlock failed: %s", __func__, FormatStateMessage(state)); @@ -4066,7 +4069,7 @@ std::string CBlockFileInfo::ToString() const static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool(void) +bool LoadMempool(CTxMemPool& pool) { int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; FILE* filestr = fopen((GetDataDir() / "mempool.dat").string().c_str(), "r"); @@ -4100,12 +4103,12 @@ bool LoadMempool(void) CAmount amountdelta = nFeeDelta; if (amountdelta) { - mempool.PrioritiseTransaction(tx->GetHash(), tx->GetHash().ToString(), prioritydummy, amountdelta); + pool.PrioritiseTransaction(tx->GetHash(), tx->GetHash().ToString(), prioritydummy, amountdelta); } CValidationState state; if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); - AcceptToMemoryPoolWithTime(mempool, state, tx, true, NULL, nTime); + AcceptToMemoryPoolWithTime(pool, state, tx, true, NULL, nTime); if (state.IsValid()) { ++count; } else { @@ -4121,7 +4124,7 @@ bool LoadMempool(void) file >> mapDeltas; for (const auto& i : mapDeltas) { - mempool.PrioritiseTransaction(i.first, i.first.ToString(), prioritydummy, i.second); + pool.PrioritiseTransaction(i.first, i.first.ToString(), prioritydummy, i.second); } } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); @@ -4132,19 +4135,22 @@ bool LoadMempool(void) return true; } -void DumpMempool(void) +bool DumpMempool(const CTxMemPool& pool) { int64_t start = GetTimeMicros(); std::map mapDeltas; std::vector vinfo; + static Mutex dump_mutex; + LOCK(dump_mutex); + { - LOCK(mempool.cs); - for (const auto &i : mempool.mapDeltas) { + LOCK(pool.cs); + for (const auto &i : pool.mapDeltas) { mapDeltas[i.first] = i.second.second; } - vinfo = mempool.infoAll(); + vinfo = pool.infoAll(); } int64_t mid = GetTimeMicros(); @@ -4152,7 +4158,7 @@ void DumpMempool(void) try { FILE* filestr = fopen((GetDataDir() / "mempool.dat.new").string().c_str(), "w"); if (!filestr) { - return; + return false; } CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); @@ -4178,7 +4184,9 @@ void DumpMempool(void) LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*0.000001, (last-mid)*0.000001); } catch (const std::exception& e) { LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + return false; } + return true; } class CMainCleanup @@ -4194,3 +4202,4 @@ class CMainCleanup mapBlockIndex.clear(); } } instance_of_cmaincleanup; + diff --git a/src/validation.h b/src/validation.h index f8d2242b8b4c..790c501aa03e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -124,7 +124,6 @@ struct BlockHasher { extern CScript COINBASE_FLAGS; extern RecursiveMutex cs_main; extern CTxMemPool mempool; -extern std::atomic_bool g_is_mempool_loaded; typedef std::unordered_map BlockMap; extern BlockMap mapBlockIndex; extern uint64_t nLastBlockTx; @@ -208,7 +207,7 @@ double ConvertBitsToDouble(unsigned int nBits); int64_t GetMasternodePayment(); /** Find the best known block, and make it the tip of the block chain */ -bool ActivateBestChain(CValidationState& state, std::shared_ptr pblock = std::shared_ptr(), bool fAlreadyChecked = false); +bool ActivateBestChain(CValidationState& state, std::shared_ptr pblock = std::shared_ptr()); CAmount GetBlockValue(int nHeight); /** Create a new block index entry for a given block hash */ @@ -333,10 +332,10 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindexPrev); /** Check a block is completely valid from start to finish (only works on top of our current best block, with cs_main held) */ -bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true); +bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true, bool fCheckBlockSig = true); /** Store block on disk. If dbp is provided, the file is known to already reside on disk */ -bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** pindex, CDiskBlockPos* dbp = NULL, bool fAlreadyCheckedBlock = false); +bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** pindex, CDiskBlockPos* dbp = NULL); bool AcceptBlockHeader(const CBlock& block, CValidationState& state, CBlockIndex** ppindex = nullptr, CBlockIndex* pindexPrev = nullptr); @@ -401,9 +400,9 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101; static const unsigned int REJECT_CONFLICT = 0x102; /** Dump the mempool to disk. */ -void DumpMempool(); +bool DumpMempool(const CTxMemPool& pool); /** Load the mempool from disk. */ -bool LoadMempool(); +bool LoadMempool(CTxMemPool& pool); #endif // BITCOIN_MAIN_H 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); }); diff --git a/src/validationinterface.h b/src/validationinterface.h index 27bfd9d4ce17..91ec2ee5b46c 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -54,12 +54,31 @@ 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; 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. */ diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6a150e2c32d9..ed615e2c5a65 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3721,12 +3721,13 @@ UniValue getwalletinfo(const JSONRPCRequest& request) " \"immature_balance\": xxxxxx, (numeric) the total immature balance of the wallet in PIV\n" " \"txcount\": xxxxxxx, (numeric) the total number of transactions in the wallet\n" " \"keypoololdest\": xxxxxx, (numeric) the timestamp (seconds since GMT epoch) of the oldest pre-generated key in the key pool\n" - " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated (only counts external keys)\n" - " \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n" - " \"keypoolsize_hd_staking\": xxxx, (numeric) how many new keys are pre-generated for staking use (used for staking contracts, only appears if the wallet is using this feature)\n" + " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated (only counts external keys)\n" + " \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n" + " \"keypoolsize_hd_staking\": xxxx, (numeric) how many new keys are pre-generated for staking use (used for staking contracts, only appears if the wallet is using this feature)\n" " \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n" " \"paytxfee\": x.xxxx (numeric) the transaction fee configuration, set in PIV/kB\n" - " \"hdseedid\": \"\" (string, optional) the Hash160 of the HD seed (only present when HD is enabled)\n" + " \"hdseedid\": \"\" (string, optional) the Hash160 of the HD seed (only present when HD is enabled)\n" + " \"last_processed_block\": xxxxx, (numeric) the last block processed block height\n" "}\n" "\nExamples:\n" + @@ -3768,6 +3769,7 @@ UniValue getwalletinfo(const JSONRPCRequest& request) if (pwalletMain->IsCrypted()) obj.pushKV("unlocked_until", nWalletUnlockTime); obj.pushKV("paytxfee", ValueFromAmount(payTxFee.GetFeePerK())); + obj.pushKV("last_processed_block", pwalletMain->GetLastBlockHeight()); return obj; } diff --git a/test/functional/example_test.py b/test/functional/example_test.py index 16bf2842365b..757a065dd2b6 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -113,7 +113,7 @@ def setup_network(self): # sync_all() should not include node2, since we're not expecting it to # sync. connect_nodes(self.nodes[0], 1) - self.sync_all([self.nodes[0:1]]) + self.sync_all(self.nodes[0:1]) # Use setup_nodes() to customize the node start behaviour (for example if # you don't want to start all nodes at the start of the test). @@ -143,7 +143,7 @@ def run_test(self): # Generating a block on one of the nodes will get us out of IBD blocks = [int(self.nodes[0].generate(1)[0], 16)] - self.sync_all([self.nodes[0:1]]) + self.sync_all(self.nodes[0:1]) # Notice above how we called an RPC by calling a method with the same # name on the node object. Notice also how we used a keyword argument diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index b13f1df94cb3..e9b419b4974d 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -191,9 +191,9 @@ def transact_and_mine(self, numblocks, mining_node): self.memutxo, Decimal("0.05"), MIN_FEE, MIN_FEE) tx_kbytes = (len(txhex) // 2) / 1000.0 self.fees_per_kb.append(float(fee)/tx_kbytes) - sync_mempools(self.nodes[0:3], wait=.1) + self.sync_mempools(self.nodes[0:3], wait=.1) mined = mining_node.getblock(mining_node.generate(1)[0],True)["tx"] - sync_blocks(self.nodes[0:3], wait=.1) + self.sync_blocks(self.nodes[0:3], wait=.1) # update which txouts are confirmed newmem = [] for utx in self.memutxo: @@ -270,7 +270,7 @@ def run_test(self): while len(self.nodes[1].getrawmempool()) > 0: self.nodes[1].generate(1) - sync_blocks(self.nodes[0:3], wait=.1) + self.sync_blocks(self.nodes[0:3], wait=.1) self.log.info("Final estimates after emptying mempools") check_estimates(self.nodes[1], self.fees_per_kb, 2) diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 234d1ad899f3..5d2333a481c8 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -10,7 +10,7 @@ """ from test_framework.test_framework import PivxTestFramework -from test_framework.util import wait_until +from test_framework.util import assert_equal import time class ReindexTest(PivxTestFramework): @@ -23,11 +23,9 @@ def reindex(self): self.nodes[0].generate(3) blockcount = self.nodes[0].getblockcount() self.stop_nodes() - time.sleep(5) extra_args = [["-reindex", "-checkblockindex=1"]] self.start_nodes(extra_args) - time.sleep(15) - wait_until(lambda: self.nodes[0].getblockcount() == blockcount) + assert_equal(self.nodes[0].getblockcount(), blockcount) # start_node is blocking on reindex self.log.info("Success") def run_test(self): diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 14f9df526527..aa2b1a7256e2 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -12,8 +12,6 @@ Decimal, ROUND_DOWN, JSONRPCException, - sync_blocks, - sync_mempools ) def satoshi_round(amount): @@ -141,7 +139,7 @@ def run_test(self): # Test reorg handling # First, the basics: self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[1].invalidateblock(self.nodes[0].getbestblockhash()) self.nodes[1].reconsiderblock(self.nodes[0].getbestblockhash()) @@ -196,12 +194,12 @@ def run_test(self): rawtx = self.nodes[0].createrawtransaction(inputs, outputs) signedtx = self.nodes[0].signrawtransaction(rawtx) txid = self.nodes[0].sendrawtransaction(signedtx['hex']) - sync_mempools(self.nodes) + self.sync_mempools() # Now try to disconnect the tip on each node... self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash()) self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) - sync_blocks(self.nodes) + self.sync_blocks() if __name__ == '__main__': MempoolPackagesTest().main() \ No newline at end of file diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 4f5eeded70ae..e81a9244d938 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -31,8 +31,14 @@ """ +from decimal import Decimal +import os + from test_framework.test_framework import PivxTestFramework -from test_framework.util import * +from test_framework.util import ( + assert_equal, + wait_until, +) class MempoolPersistTest(PivxTestFramework): def set_test_params(self): @@ -64,10 +70,11 @@ def run_test(self): self.start_node(1, extra_args=["-persistmempool=0"]) self.start_node(0) self.start_node(2) - # Give pivxd a second to reload the mempool - wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5, timeout=1) - wait_until(lambda: len(self.nodes[2].getrawmempool()) == 5, timeout=1) - # The others loaded their mempool. If node_1 loaded anything, we'd probably notice by now: + assert self.nodes[0].getmempoolinfo()["loaded"] # start_node is blocking on the mempool being loaded + assert self.nodes[2].getmempoolinfo()["loaded"] + assert_equal(len(self.nodes[0].getrawmempool()), 5) + assert_equal(len(self.nodes[2].getrawmempool()), 5) + # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now: assert_equal(len(self.nodes[1].getrawmempool()), 0) # Verify accounting of mempool transactions after restart is correct @@ -77,38 +84,39 @@ def run_test(self): self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() self.start_node(0, extra_args=["-persistmempool=0"]) - # Give bitcoind a second to reload the mempool - time.sleep(1) + assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 0) self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.") self.stop_nodes() self.start_node(0) - wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5) + assert self.nodes[0].getmempoolinfo()["loaded"] + assert_equal(len(self.nodes[0].getrawmempool()), 5) # Following code is ahead of our current repository state. Future back port. + ''' + mempooldat0 = os.path.join(self.nodes[0].datadir, 'regtest', 'mempool.dat') + mempooldat1 = os.path.join(self.nodes[1].datadir, 'regtest', 'mempool.dat') + self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") + os.remove(mempooldat0) + self.nodes[0].savemempool() + assert os.path.isfile(mempooldat0) + + self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions") + os.rename(mempooldat0, mempooldat1) + self.stop_nodes() + self.start_node(1, extra_args=[]) + assert self.nodes[0].getmempoolinfo()["loaded"] + assert_equal(len(self.nodes[1].getrawmempool()), 5) - # mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat') - # mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat') - # self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") - # os.remove(mempooldat0) - # self.nodes[0].savemempool() - # assert os.path.isfile(mempooldat0) - # - # self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions") - # os.rename(mempooldat0, mempooldat1) - # self.stop_nodes() - # self.start_node(1, extra_args=[]) - # wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5) - # - # self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") - # # to test the exception we are setting bad permissions on a tmp file called mempool.dat.new - # # which is an implementation detail that could change and break this test - # mempooldotnew1 = mempooldat1 + '.new' - # with os.fdopen(os.open(mempooldotnew1, os.O_CREAT, 0o000), 'w'): - # pass - # assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool) - # os.remove(mempooldotnew1) + self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") + # to test the exception we are creating a tmp folder called mempool.dat.new + # which is an implementation detail that could change and break this test + mempooldotnew1 = mempooldat1 + '.new' + os.mkdir(mempooldotnew1) + assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool) + os.rmdir(mempooldotnew1) + ''' if __name__ == '__main__': MempoolPersistTest().main() diff --git a/test/functional/mining_pos_coldStaking.py b/test/functional/mining_pos_coldStaking.py index edd459037e4d..f5c5eb289acb 100755 --- a/test/functional/mining_pos_coldStaking.py +++ b/test/functional/mining_pos_coldStaking.py @@ -19,8 +19,6 @@ p2p_port, bytes_to_hex_str, set_node_times, - sync_blocks, - sync_mempools, ) from decimal import Decimal @@ -102,7 +100,7 @@ def run_test(self): for peer in [0, 2]: for j in range(25): self.mocktime = self.generate_pow(peer, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # 2) node[1] sends his entire balance (50 mature rewards) to node[2] # - node[2] stakes a block - node[1] locks the change @@ -112,9 +110,9 @@ def run_test(self): assert_equal(self.nodes[1].getbalance(), 50 * 250) txid = self.nodes[1].sendtoaddress(self.nodes[2].getnewaddress(), (50 * 250 - 0.01)) assert (txid is not None) - sync_mempools(self.nodes) + self.sync_mempools() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # lock the change output (so it's not used as stake input in generate_pos) for x in self.nodes[1].listunspent(): assert (self.nodes[1].lockunspent(False, [{"txid": x['txid'], "vout": x['vout']}])) @@ -128,7 +126,7 @@ def run_test(self): self.sync_all() for i in range(6): self.mocktime = self.generate_pow(0, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getshieldbalance(), 250) # 3) nodes[0] generates a owner address @@ -193,9 +191,9 @@ def run_test(self): assert_equal(res["staker_address"], staker_address) fee = self.nodes[0].viewshieldtransaction(res["txid"])['fee'] # sync and mine 2 blocks - sync_mempools(self.nodes) + self.sync_mempools() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() self.log.info("%d Txes created." % NUM_OF_INPUTS) # check balances: self.expected_balance = NUM_OF_INPUTS * INPUT_VALUE @@ -215,9 +213,9 @@ def run_test(self): txhash = self.spendUTXOwithNode(u, 0) assert(txhash != None) self.log.info("Good. Owner was able to spend - tx: %s" % str(txhash)) - sync_mempools(self.nodes) + self.sync_mempools() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # check tx self.check_tx_in_chain(0, txhash) self.check_tx_in_chain(1, txhash) @@ -251,7 +249,7 @@ def run_test(self): self.spendUTXOwithNode, u, 1) self.log.info("Good. Cold staker was NOT able to spend (failed OP_CHECKCOLDSTAKEVERIFY)") self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # 9) check that the staker can use the coins to stake a block with internal miner. # -------------------------------------------------------------------------------- @@ -264,7 +262,7 @@ def run_test(self): self.log.info("Block %s submitted" % newblockhash) # Verify that nodes[0] accepts it - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getblockcount(), self.nodes[1].getblockcount()) assert_equal(newblockhash, self.nodes[0].getbestblockhash()) self.log.info("Great. Cold-staked block was accepted!") @@ -292,7 +290,7 @@ def run_test(self): assert_equal(new_block.hash, self.nodes[1].getbestblockhash()) # Verify that nodes[0] accepts it - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getblockcount(), self.nodes[1].getblockcount()) assert_equal(new_block.hash, self.nodes[0].getbestblockhash()) self.log.info("Great. Cold-staked block was accepted!") @@ -321,7 +319,7 @@ def run_test(self): assert("rejected" in ret) # Verify that nodes[0] rejects it - sync_blocks(self.nodes) + self.sync_blocks() assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, new_block.hash) self.log.info("Great. Malicious cold-staked block was NOT accepted!") self.checkBalances() @@ -345,7 +343,7 @@ def run_test(self): assert_equal(ret, "bad-p2cs-outs") # Verify that nodes[0] rejects it - sync_blocks(self.nodes) + self.sync_blocks() assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, new_block.hash) self.log.info("Great. Malicious cold-staked block was NOT accepted!") self.checkBalances() @@ -355,7 +353,7 @@ def run_test(self): # ---------------------------------------------------------------------------------------- self.log.info("Let's void the contracts.") self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() print("*** 13 ***") self.log.info("Cancel the stake delegation spending the delegated utxos...") delegated_utxos = getDelegatedUtxos(self.nodes[0].listunspent()) @@ -364,9 +362,9 @@ def run_test(self): txhash = self.spendUTXOsWithNode(delegated_utxos, 0) assert(txhash != None) self.log.info("Good. Owner was able to void the stake delegations - tx: %s" % str(txhash)) - sync_mempools(self.nodes) + self.sync_blocks() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # deactivate SPORK 17 and check that the owner can still spend the last utxo self.setColdStakingEnforcement(False) @@ -374,9 +372,9 @@ def run_test(self): txhash = self.spendUTXOsWithNode([final_spend], 0) assert(txhash != None) self.log.info("Good. Owner was able to void a stake delegation (with SPORK 17 disabled) - tx: %s" % str(txhash)) - sync_mempools(self.nodes) + self.sync_mempools() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # check tx self.check_tx_in_chain(0, txhash) self.check_tx_in_chain(1, txhash) @@ -403,7 +401,7 @@ def run_test(self): for peer in [0, 2]: for j in range(25): self.mocktime = self.generate_pos(peer, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() self.expected_balance = self.expected_immature_balance self.expected_immature_balance = 0 self.checkBalances() @@ -411,9 +409,9 @@ def run_test(self): txhash = self.spendUTXOsWithNode(delegated_utxos, 0) assert (txhash != None) self.log.info("Good. Owner was able to spend the cold staked coins - tx: %s" % str(txhash)) - sync_mempools(self.nodes) + self.sync_mempools() self.mocktime = self.generate_pos(2, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # check tx self.check_tx_in_chain(0, txhash) self.check_tx_in_chain(1, txhash) @@ -423,6 +421,7 @@ def run_test(self): def checkBalances(self): w_info = self.nodes[0].getwalletinfo() + assert_equal(self.nodes[0].getblockcount(), w_info['last_processed_block']) self.log.info("OWNER - Delegated %f / Cold %f [%f / %f]" % ( float(w_info["delegated_balance"]), w_info["cold_staking_balance"], float(w_info["immature_delegated_balance"]), w_info["immature_cold_staking_balance"])) @@ -430,6 +429,7 @@ def checkBalances(self): assert_equal(float(w_info["immature_delegated_balance"]), self.expected_immature_balance) assert_equal(float(w_info["cold_staking_balance"]), 0) w_info = self.nodes[1].getwalletinfo() + assert_equal(self.nodes[1].getblockcount(), w_info['last_processed_block']) self.log.info("STAKER - Delegated %f / Cold %f [%f / %f]" % ( float(w_info["delegated_balance"]), w_info["cold_staking_balance"], float(w_info["immature_delegated_balance"]), w_info["immature_cold_staking_balance"])) @@ -481,8 +481,5 @@ def add_output_to_coinstake(self, block, value, peer=1): block.re_sign_block() - - - if __name__ == '__main__': PIVX_ColdStakingTest().main() diff --git a/test/functional/mining_pos_fakestake.py b/test/functional/mining_pos_fakestake.py index 8c10db256cca..49321c8c9f13 100755 --- a/test/functional/mining_pos_fakestake.py +++ b/test/functional/mining_pos_fakestake.py @@ -50,7 +50,6 @@ from test_framework.messages import COutPoint from test_framework.test_framework import PivxTestFramework from test_framework.util import ( - sync_blocks, assert_equal, bytes_to_hex_str, set_node_times @@ -93,7 +92,7 @@ def run_test(self): self.log.info("Mining 50 blocks to reach PoS phase...") for i in range(50): self.mocktime = self.generate_pow(0, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # Check Tests 1-3 self.test_1() @@ -116,7 +115,7 @@ def test_1(self): self.log.info("Mining 5 blocks as fork depth...") for i in range(5): self.mocktime = self.generate_pow(0, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # nodes[1] spams 3 blocks with height 256 --> [REJECTED] assert_equal(self.nodes[1].getblockcount(), 255) @@ -138,7 +137,7 @@ def test_2(self): self.log.info("Mining 5 blocks to include the spends...") for i in range(5): self.mocktime = self.generate_pow(0, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() self.check_tx_in_chain(0, txid) assert_equal(self.nodes[1].getbalance(), 0) diff --git a/test/functional/mining_pos_reorg.py b/test/functional/mining_pos_reorg.py index 831b8c3c4ef9..35cb84350405 100755 --- a/test/functional/mining_pos_reorg.py +++ b/test/functional/mining_pos_reorg.py @@ -3,10 +3,8 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -from test_framework.authproxy import JSONRPCException from test_framework.test_framework import PivxTestFramework from test_framework.util import ( - sync_blocks, assert_equal, assert_raises_rpc_error, connect_nodes, @@ -49,6 +47,7 @@ def disconnect_all(self): def get_tot_balance(self, nodeid): wi = self.nodes[nodeid].getwalletinfo() + assert_equal(self.nodes[nodeid].getblockcount(), wi['last_processed_block']) return wi['balance'] + wi['immature_balance'] def check_money_supply(self, expected_piv): @@ -113,7 +112,7 @@ def findUtxoInList(txid, vout, utxo_list): # Connect with node 2 and sync self.log.info("Reconnecting node 0 and node 2") connect_nodes(self.nodes[0], 2) - sync_blocks([self.nodes[i] for i in [0, 2]]) + self.sync_blocks([self.nodes[i] for i in [0, 2]]) # verify that the stakeinput can't be spent stakeinput_tx_json = self.nodes[0].getrawtransaction(stakeinput["txid"], True) @@ -143,7 +142,7 @@ def findUtxoInList(txid, vout, utxo_list): self.log.info("Connecting and syncing nodes...") set_node_times(self.nodes, block_time_1) connect_nodes_clique(self.nodes) - sync_blocks(self.nodes) + self.sync_blocks() for i in [0, 2]: assert_equal(self.nodes[i].getbestblockhash(), new_best_hash) @@ -158,7 +157,7 @@ def findUtxoInList(txid, vout, utxo_list): stakeinput["txid"][:9], stakeinput["txid"][-4:], stakeinput["vout"])) self.nodes[0].sendrawtransaction(rawtx["hex"]) self.nodes[1].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() res, utxo = findUtxoInList(stakeinput["txid"], stakeinput["vout"], self.nodes[0].listunspent()) assert (not res or not utxo["spendable"]) diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py index a3fa5713d990..e7e77429a638 100755 --- a/test/functional/p2p_feefilter.py +++ b/test/functional/p2p_feefilter.py @@ -45,7 +45,7 @@ def run_test(self): node0 = self.nodes[0] # Get out of IBD node1.generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # Setup the p2p connections and start up the network thread. self.nodes[0].add_p2p_connection(TestNode()) @@ -69,7 +69,7 @@ def run_test(self): # Change tx fee rate to 10 sat/byte and test they are no longer received node1.settxfee(float(0.00010000)) [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)] - sync_mempools(self.nodes) # must be sure node 0 has received all txs + self.sync_mempools() # must be sure node 0 has received all txs # Send one transaction from node0 that should be received, so that we # we can sync the test on receipt (if node1's txs were relayed, they'd diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 950bf60af09c..e4de0fb59054 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -247,7 +247,7 @@ def mine_blocks(self, count): # return the list of block hashes newly mined def mine_reorg(self, length): self.nodes[0].generate(length) # make sure all invalidated blocks are node0's - sync_blocks(self.nodes, wait=0.1) + self.sync_blocks(self.nodes, wait=0.1) for x in self.p2p_connections: x.wait_for_block_announcement(int(self.nodes[0].getbestblockhash(), 16)) x.clear_last_announcement() @@ -256,7 +256,7 @@ def mine_reorg(self, length): hash_to_invalidate = self.nodes[1].getblockhash(tip_height-(length-1)) self.nodes[1].invalidateblock(hash_to_invalidate) all_hashes = self.nodes[1].generate(length+1) # Must be longer than the orig chain - sync_blocks(self.nodes, wait=0.1) + self.sync_blocks(self.nodes, wait=0.1) return [int(x, 16) for x in all_hashes] def run_test(self): diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index 17abd8fc35e3..3b873d4efed5 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -316,7 +316,7 @@ def run_test(self): # 9. Connect node1 to node0 and ensure it is able to sync connect_nodes(self.nodes[0], 1) - sync_blocks([self.nodes[0], self.nodes[1]]) + self.sync_blocks([self.nodes[0], self.nodes[1]]) self.log.info("Successfully synced nodes 1 and 0") if __name__ == '__main__': diff --git a/test/functional/rpc_getchaintips.py b/test/functional/rpc_getchaintips.py index d7fc7d52fc35..fd0646079a97 100755 --- a/test/functional/rpc_getchaintips.py +++ b/test/functional/rpc_getchaintips.py @@ -28,7 +28,8 @@ def run_test (self): self.split_network () self.nodes[0].generate(10) self.nodes[2].generate(20) - self.sync_all([self.nodes[:2], self.nodes[2:]]) + self.sync_all(self.nodes[:2]) + self.sync_all(self.nodes[2:]) tips = self.nodes[1].getchaintips () assert_equal (len (tips), 1) diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index 8e1f2352df7a..eeb8b1be245c 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -28,7 +28,7 @@ def run_test(self): self.log.info("Connect nodes to force a reorg") connect_nodes(self.nodes[0], 1) - sync_blocks(self.nodes[0:2]) + self.sync_blocks(self.nodes[0:2]) assert_equal(self.nodes[0].getblockcount(), 6) badhash = self.nodes[1].getblockhash(2) @@ -40,7 +40,7 @@ def run_test(self): self.log.info("Make sure we won't reorg to a lower work chain:") connect_nodes(self.nodes[1], 2) self.log.info("Sync node 2 to node 1 so both have 6 blocks") - sync_blocks(self.nodes[1:3]) + self.sync_blocks(self.nodes[1:3]) assert_equal(self.nodes[2].getblockcount(), 6) self.log.info("Invalidate block 5 on node 1 so its tip is now at 4") self.nodes[1].invalidateblock(self.nodes[1].getblockhash(5)) diff --git a/test/functional/sapling_fillblock.py b/test/functional/sapling_fillblock.py index 82eb0710bac5..e1b64e5bdf2f 100755 --- a/test/functional/sapling_fillblock.py +++ b/test/functional/sapling_fillblock.py @@ -11,8 +11,6 @@ assert_equal, Decimal, satoshi_round, - sync_blocks, - sync_mempools, ) import time @@ -53,7 +51,7 @@ def utxo_splitter(self, node_from, n_inputs, node_to): def check_mempool(self, miner, txids): self.log.info("Checking mempool...") - sync_mempools(self.nodes) + self.sync_mempools() mempool_info = miner.getmempoolinfo() assert_equal(mempool_info['size'], len(txids)) mempool_bytes = mempool_info['bytes'] @@ -77,7 +75,7 @@ def send_shielded(self, node, n_txes, from_address, shield_to): txids.append(node.shieldsendmany(from_address, shield_to)) if (i + 1) % 200 == 0: self.log.info("...%d Transactions created..." % (i + 1)) - sync_mempools(self.nodes) + self.sync_mempools() return txids @@ -87,7 +85,7 @@ def run_test(self): # First mine 300 blocks self.log.info("Generating 300 blocks...") miner.generate(300) - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['v5 shield']['status'], 'active') ## -- First check that the miner never produces blocks with more than 750kB of shielded txes @@ -99,7 +97,7 @@ def run_test(self): txids = self.utxo_splitter(miner, UTXOS_TO_SPLIT, alice) assert_equal(len(txids), UTXOS_TO_SPLIT) miner.generate(2) - sync_blocks(self.nodes) + self.sync_blocks() new_utxos = alice.listunspent() assert_equal(len(new_utxos), UTXOS_TO_SHIELD) diff --git a/test/functional/sapling_mempool.py b/test/functional/sapling_mempool.py index 711c3f97f50d..1e3586c64da3 100755 --- a/test/functional/sapling_mempool.py +++ b/test/functional/sapling_mempool.py @@ -7,7 +7,6 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, - sync_mempools, ) from decimal import Decimal @@ -53,7 +52,7 @@ def run_test(self): # Miner receives tx_B and accepts it in the mempool assert (txid_B in alice.getrawmempool()) - sync_mempools(self.nodes) + self.sync_mempools() assert(txid_B in miner.getrawmempool()) self.log.info("tx_B accepted in the memory pool.") @@ -87,7 +86,7 @@ def run_test(self): txid_C = alice.sendrawtransaction(txC_hex) # Miner receives tx_C and accepts it in the mempool - sync_mempools(self.nodes) + self.sync_mempools() assert(txid_C in miner.getrawmempool()) self.log.info("tx_C accepted in the memory pool.") diff --git a/test/functional/sapling_wallet.py b/test/functional/sapling_wallet.py index 0af64085b918..c9600cc342c2 100755 --- a/test/functional/sapling_wallet.py +++ b/test/functional/sapling_wallet.py @@ -11,7 +11,6 @@ connect_nodes, disconnect_nodes, satoshi_round, - sync_mempools, get_coinstake_address, wait_until, ) @@ -29,7 +28,7 @@ def set_test_params(self): self.extra_args[0].append('-sporkkey=932HEevBSujW2ud7RfB1YF91AFygbBRQj3de3LyaCRqNzKKgWXi') def check_tx_priority(self, txids): - sync_mempools(self.nodes) + self.sync_mempools() mempool = self.nodes[0].getrawmempool(True) for txid in txids: assert(Decimal(mempool[txid]['startingpriority']) == Decimal('1E+25')) diff --git a/test/functional/sapling_wallet_anchorfork.py b/test/functional/sapling_wallet_anchorfork.py index 3230015a3302..40943257ab6e 100755 --- a/test/functional/sapling_wallet_anchorfork.py +++ b/test/functional/sapling_wallet_anchorfork.py @@ -24,6 +24,7 @@ def run_test (self): self.sync_all() walletinfo = self.nodes[0].getwalletinfo() + assert_equal(self.nodes[0].getblockcount(), walletinfo['last_processed_block']) assert_equal(walletinfo['immature_balance'], 1000) assert_equal(walletinfo['balance'], 0) @@ -63,7 +64,7 @@ def run_test (self): # Partition B, node 1 mines an empty block self.nodes[1].generate(1) - sync_blocks(self.nodes[1:3]) + self.sync_blocks(self.nodes[1:3]) # Check partition assert_equal(self.nodes[1].getblockcount(), self.nodes[2].getblockcount()) @@ -77,7 +78,7 @@ def run_test (self): # Partition A, node 0 mines a block with the transaction self.nodes[0].generate(1) - self.sync_all([self.nodes[1:3]]) + self.sync_all(self.nodes[1:3]) # Partition B, node 1 mines the same shield transaction txid2 = self.nodes[1].sendrawtransaction(rawhex) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 436d68694eb4..46805f7d01f1 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -57,8 +57,6 @@ set_node_times, SPORK_ACTIVATION_TIME, SPORK_DEACTIVATION_TIME, - sync_blocks, - sync_mempools, vZC_DENOMS, wait_until, ) @@ -365,7 +363,8 @@ def split_network(self): """ disconnect_nodes(self.nodes[1], 2) disconnect_nodes(self.nodes[2], 1) - self.sync_all([self.nodes[:2], self.nodes[2:]]) + self.sync_all(self.nodes[:2]) + self.sync_all(self.nodes[2:]) def join_network(self): """ @@ -374,13 +373,52 @@ def join_network(self): connect_nodes(self.nodes[1], 2) self.sync_all() - def sync_all(self, node_groups=None): - if not node_groups: - node_groups = [self.nodes] - - for group in node_groups: - sync_blocks(group) - sync_mempools(group) + def sync_blocks(self, nodes=None, wait=1, timeout=60): + """ + Wait until everybody has the same tip. + sync_blocks needs to be called with an rpc_connections set that has least + one node already synced to the latest, stable tip, otherwise there's a + chance it might return before all nodes are stably synced. + """ + rpc_connections = nodes or self.nodes + stop_time = time.time() + timeout + while time.time() <= stop_time: + best_hash = [x.getbestblockhash() for x in rpc_connections] + if best_hash.count(best_hash[0]) == len(rpc_connections): + return + # Check that each peer has at least one connection + assert (all([len(x.getpeerinfo()) for x in rpc_connections])) + time.sleep(wait) + raise AssertionError("Block sync timed out after {}s:{}".format( + timeout, + "".join("\n {!r}".format(b) for b in best_hash), + )) + + def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True): + """ + Wait until everybody has the same transactions in their memory + pools + """ + rpc_connections = nodes or self.nodes + stop_time = time.time() + timeout + while time.time() <= stop_time: + pool = [set(r.getrawmempool()) for r in rpc_connections] + if pool.count(pool[0]) == len(rpc_connections): + if flush_scheduler: + for r in rpc_connections: + r.syncwithvalidationinterfacequeue() + return + # Check that each peer has at least one connection + assert (all([len(x.getpeerinfo()) for x in rpc_connections])) + time.sleep(wait) + raise AssertionError("Mempool sync timed out after {}s:{}".format( + timeout, + "".join("\n {!r}".format(m) for m in pool), + )) + + def sync_all(self, nodes=None): + self.sync_blocks(nodes) + self.sync_mempools(nodes) def enable_mocktime(self): """Enable mocktime for the script. @@ -551,7 +589,7 @@ def generate_pow_cache(): self.nodes[peer].generate(1) block_time += 60 # Must sync before next peer starts generating blocks - sync_blocks(self.nodes) + self.sync_blocks() # Shut them down, and clean up cache directories: self.log.info("Stopping nodes") @@ -1018,7 +1056,7 @@ def send_pings(self, mnodes): def stake_and_sync(self, node_id, num_blocks): for i in range(num_blocks): self.mocktime = self.generate_pos(node_id, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() time.sleep(1) @@ -1181,7 +1219,7 @@ def setup_2_masternodes_network(self): # First mine 250 PoW blocks for i in range(250): self.mocktime = self.generate_pow(self.minerPos, self.mocktime) - sync_blocks(self.nodes) + self.sync_blocks() # Then start staking self.stake(9) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 5436a3b40c6b..e9664b720023 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -121,8 +121,25 @@ def wait_for_rpc_connection(self): assert self.process.poll() is None, "pivxd exited with status %i during initialization" % self.process.returncode try: self.rpc = get_rpc_proxy(rpc_url(self.datadir, self.index, self.rpchost), self.index, timeout=self.rpc_timeout, coveragedir=self.coverage_dir) - while self.rpc.getblockcount() < 0: - time.sleep(1) + self.rpc.getblockcount() + wait_until(lambda: self.rpc.getmempoolinfo()['loaded']) + # Wait for the node to finish reindex, block import, and + # loading the mempool. Usually importing happens fast or + # even "immediate" when the node is started. However, there + # is no guarantee and sometimes ThreadImport might finish + # later. This is going to cause intermittent test failures, + # because generally the tests assume the node is fully + # ready after being started. + # + # For example, the node will reject block messages from p2p + # when it is still importing with the error "Unexpected + # block message received" + # + # The wait is done here to make tests as robust as possible + # and prevent racy tests and intermittent failures as much + # as possible. Some tests might not need this, but the + # overhead is trivial, and the added gurantees are worth + # the minimal performance cost. # If the call to getblockcount() succeeds then the RPC connection is up self.rpc_connected = True self.url = self.rpc.url diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 30495e47b296..795b7f5d2593 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -390,48 +390,6 @@ def connect_nodes_clique(nodes): connect_nodes(nodes[a], b) connect_nodes(nodes[b], a) -def sync_blocks(rpc_connections, *, wait=1, timeout=60): - """ - Wait until everybody has the same tip. - - sync_blocks needs to be called with an rpc_connections set that has least - one node already synced to the latest, stable tip, otherwise there's a - chance it might return before all nodes are stably synced. - """ - stop_time = time.time() + timeout - while time.time() <= stop_time: - best_hash = [x.getbestblockhash() for x in rpc_connections] - if best_hash.count(best_hash[0]) == len(rpc_connections): - return - # Check that each peer has at least one connection - assert (all([len(x.getpeerinfo()) for x in rpc_connections])) - time.sleep(wait) - raise AssertionError("Block sync timed out after {}s:{}".format( - timeout, - "".join("\n {!r}".format(b) for b in best_hash), - )) - -def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True): - """ - Wait until everybody has the same transactions in their memory - pools - """ - stop_time = time.time() + timeout - while time.time() <= stop_time: - pool = [set(r.getrawmempool()) for r in rpc_connections] - if pool.count(pool[0]) == len(rpc_connections): - if flush_scheduler: - for r in rpc_connections: - r.syncwithvalidationinterfacequeue() - return - # Check that each peer has at least one connection - assert (all([len(x.getpeerinfo()) for x in rpc_connections])) - time.sleep(wait) - raise AssertionError("Mempool sync timed out after {}s:{}".format( - timeout, - "".join("\n {!r}".format(m) for m in pool), - )) - # Transaction/Block functions ############################# diff --git a/test/functional/tiertwo_masternode_activation.py b/test/functional/tiertwo_masternode_activation.py index d04a4961b271..e1c7ab08f77f 100755 --- a/test/functional/tiertwo_masternode_activation.py +++ b/test/functional/tiertwo_masternode_activation.py @@ -9,8 +9,6 @@ connect_nodes_clique, disconnect_nodes, satoshi_round, - sync_blocks, - sync_mempools, wait_until, ) @@ -56,7 +54,7 @@ def spend_collateral(self): rawtx = self.ownerOne.createrawtransaction(inputs, outputs) signedtx = self.ownerOne.signrawtransaction(rawtx) txid = self.miner.sendrawtransaction(signedtx['hex']) - sync_mempools(self.nodes) + self.sync_mempools() self.log.info("Collateral spent in %s" % txid) self.send_pings([self.remoteTwo]) self.stake(1, [self.remoteTwo]) @@ -111,7 +109,7 @@ def run_test(self): self.advance_mocktime(30) self.log.info("spending the collateral now..") self.spend_collateral() - sync_blocks(self.nodes) + self.sync_blocks() self.log.info("checking mn status..") time.sleep(3) # wait a little bit self.wait_until_mn_vinspent(self.mnOneTxHash, 30, [self.remoteTwo]) diff --git a/test/functional/tiertwo_masternode_ping.py b/test/functional/tiertwo_masternode_ping.py index 96f43cb9c159..ab7fc3fccf7c 100755 --- a/test/functional/tiertwo_masternode_ping.py +++ b/test/functional/tiertwo_masternode_ping.py @@ -9,7 +9,6 @@ assert_greater_than, Decimal, p2p_port, - sync_blocks, ) import os @@ -38,7 +37,7 @@ def run_test(self): self.log.info("generating 141 blocks...") miner.generate(141) - sync_blocks(self.nodes) + self.sync_blocks() # Create collateral self.log.info("funding masternode controller...") @@ -46,7 +45,7 @@ def run_test(self): mnAddress = owner.getnewaddress(masternodeAlias) collateralTxId = miner.sendtoaddress(mnAddress, Decimal('10000')) miner.generate(2) - sync_blocks(self.nodes) + self.sync_blocks() time.sleep(1) collateral_rawTx = owner.getrawtransaction(collateralTxId, 1) assert_equal(owner.getbalance(), Decimal('10000')) @@ -88,14 +87,14 @@ def run_test(self): self.wait_until_mnsync_finished() self.log.info("MnSync completed in %d seconds" % (time.time() - start_time)) miner.generate(1) - sync_blocks(self.nodes) + self.sync_blocks() time.sleep(1) # Send Start message self.log.info("sending masternode broadcast...") self.controller_start_masternode(owner, masternodeAlias) miner.generate(1) - sync_blocks(self.nodes) + self.sync_blocks() time.sleep(1) # Wait until masternode is enabled everywhere (max 180 secs) @@ -106,7 +105,7 @@ def run_test(self): self.log.info("Masternode enabled in %d seconds" % (time.time() - start_time)) self.log.info("Good. Masternode enabled") miner.generate(1) - sync_blocks(self.nodes) + self.sync_blocks() time.sleep(1) last_seen = [self.get_mn_lastseen(node, collateralTxId) for node in self.nodes] diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 81b4474a21be..35a1b9e059ec 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -11,8 +11,6 @@ connect_nodes, Decimal, disconnect_nodes, - sync_blocks, - sync_mempools ) class AbandonConflictTest(PivxTestFramework): @@ -23,14 +21,14 @@ def set_test_params(self): def run_test(self): self.nodes[0].generate(5) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[1].generate(110) - sync_blocks(self.nodes) + self.sync_blocks() balance = self.nodes[0].getbalance() txA = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) txB = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) txC = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) - sync_mempools(self.nodes) + self.sync_mempools() self.nodes[1].generate(1) # Can not abandon non-wallet transaction @@ -38,7 +36,7 @@ def run_test(self): # Can not abandon confirmed transaction assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: self.nodes[0].abandontransaction(txA)) - sync_blocks(self.nodes) + self.sync_blocks() newbalance = self.nodes[0].getbalance() assert(balance - newbalance < Decimal("0.001")) #no more than fees lost balance = newbalance @@ -156,7 +154,7 @@ def run_test(self): self.nodes[1].generate(1) connect_nodes(self.nodes[0], 1) - sync_blocks(self.nodes) + self.sync_blocks() # Verify that B and C's 10 BTC outputs are available for spending again because AB1 is now conflicted newbalance = self.nodes[0].getbalance() diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 62d4ff731a3a..1d2f519364b7 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -30,11 +30,17 @@ Shutdown again, restore using importwallet, and confirm again balances are correct. """ +from decimal import Decimal +import os from random import randint import shutil from test_framework.test_framework import PivxTestFramework -from test_framework.util import * +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + connect_nodes, +) class WalletBackupTest(PivxTestFramework): def set_test_params(self): @@ -70,9 +76,9 @@ def do_one_round(self): # Have the miner (node3) mine a block. # Must sync mempools before mining. - sync_mempools(self.nodes) + self.sync_mempools() self.nodes[3].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # As above, this mirrors the original bash test. def start_three(self): @@ -97,13 +103,13 @@ def erase_three(self): def run_test(self): self.log.info("Generating initial blockchain") self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[1].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[2].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[3].generate(100) - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getbalance(), 250) assert_equal(self.nodes[1].getbalance(), 250) @@ -160,7 +166,7 @@ def run_test(self): self.log.info("Re-starting nodes") self.start_three() - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getbalance(), balance0) assert_equal(self.nodes[1].getbalance(), balance1) @@ -184,7 +190,7 @@ def run_test(self): self.nodes[1].importwallet(tmpdir + "/node1/wallet.dump") self.nodes[2].importwallet(tmpdir + "/node2/wallet.dump") - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getbalance(), balance0) assert_equal(self.nodes[1].getbalance(), balance1) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index f3c6af3edd54..e0c8634a855e 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -27,11 +27,14 @@ def setup_network(self): connect_nodes(self.nodes[0], 1) connect_nodes(self.nodes[1], 2) connect_nodes(self.nodes[0], 2) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) def get_vsize(self, txn): return self.nodes[0].decoderawtransaction(txn)['size'] + def check_wallet_processed_blocks(self, nodeid, walletinfo): + assert_equal(self.nodes[nodeid].getblockcount(), walletinfo['last_processed_block']) + def run_test(self): # Check that there's no UTXO on none of the nodes assert_equal(len(self.nodes[0].listunspent()), 0) @@ -43,24 +46,29 @@ def run_test(self): self.nodes[0].generate(1) walletinfo = self.nodes[0].getwalletinfo() + self.check_wallet_processed_blocks(0, walletinfo) assert_equal(walletinfo['immature_balance'], 250) assert_equal(walletinfo['balance'], 0) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) self.nodes[1].generate(101) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) assert_equal(self.nodes[0].getbalance(), 250) assert_equal(self.nodes[1].getbalance(), 250) assert_equal(self.nodes[2].getbalance(), 0) + walletinfo = self.nodes[0].getwalletinfo() + self.check_wallet_processed_blocks(0, walletinfo) + self.check_wallet_processed_blocks(1, self.nodes[1].getwalletinfo()) + self.check_wallet_processed_blocks(2, self.nodes[2].getwalletinfo()) + # Check that only first and second nodes have UTXOs utxos = self.nodes[0].listunspent() assert_equal(len(utxos), 1) assert_equal(len(self.nodes[1].listunspent()), 1) assert_equal(len(self.nodes[2].listunspent()), 0) - walletinfo = self.nodes[0].getwalletinfo() assert_equal(walletinfo['immature_balance'], 0) # Exercise locking of unspent outputs @@ -75,7 +83,7 @@ def run_test(self): # Send 21 PIV from 1 to 0 using sendtoaddress call. self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 21) self.nodes[1].generate(1) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) # Node0 should have two unspent outputs. # Create a couple of transactions to send them to node2, submit them through @@ -100,7 +108,7 @@ def run_test(self): # Have node1 mine a block to confirm transactions: self.nodes[1].generate(1) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) assert_equal(self.nodes[0].getbalance(), 0) node_2_expected_bal = Decimal('250') + Decimal('21') - 2 * fee_per_kbyte @@ -115,7 +123,7 @@ def run_test(self): node_2_bal -= (Decimal('10') - fee) assert_equal(self.nodes[2].getbalance(), node_2_bal) self.nodes[2].generate(1) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) node_0_bal = self.nodes[0].getbalance() assert_equal(node_0_bal, Decimal('10')) @@ -123,7 +131,7 @@ def run_test(self): txid = self.nodes[2].sendmany('', {address: 10}, 0, "") fee = self.nodes[2].gettransaction(txid)["fee"] self.nodes[2].generate(1) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) node_0_bal += Decimal('10') node_2_bal -= (Decimal('10') - fee) assert_equal(self.nodes[2].getbalance(), node_2_bal) @@ -138,7 +146,7 @@ def run_test(self): address_to_import = self.nodes[2].getnewaddress() self.nodes[0].sendtoaddress(address_to_import, 1) self.nodes[0].generate(1) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) # 2. Import address from node2 to node1 self.nodes[1].importaddress(address_to_import) @@ -162,9 +170,9 @@ def run_test(self): {"spendable": True}) # check if wallet or blochchain maintenance changes the balance - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) blocks = self.nodes[0].generate(2) - self.sync_all([self.nodes[0:3]]) + self.sync_all(self.nodes[0:3]) balance_nodes = [self.nodes[i].getbalance() for i in range(3)] block_count = self.nodes[0].getblockcount() @@ -185,6 +193,7 @@ def run_test(self): assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)]) # Exercise listsinceblock with the last two blocks + self.check_wallet_processed_blocks(0, self.nodes[0].getwalletinfo()) coinbase_tx_1 = self.nodes[0].listsinceblock(blocks[0]) assert_equal(coinbase_tx_1["lastblock"], blocks[1]) assert_equal(len(coinbase_tx_1["transactions"]), 1) diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 2db13b27b7ea..32f26a9793d0 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -61,7 +61,7 @@ def run_test(self): self.log.info("Running tests") dest_address = peer_node.getnewaddress() - test_simple_bumpfee_succeeds(rbf_node, peer_node, dest_address) + test_simple_bumpfee_succeeds(self, rbf_node, peer_node, dest_address) test_segwit_bumpfee_succeeds(rbf_node, dest_address) test_nonrbf_bumpfee_fails(peer_node, dest_address) test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address) @@ -77,16 +77,16 @@ def run_test(self): self.log.info("Success") -def test_simple_bumpfee_succeeds(rbf_node, peer_node, dest_address): +def test_simple_bumpfee_succeeds(test, rbf_node, peer_node, dest_address): rbfid = spend_one_input(rbf_node, dest_address) rbftx = rbf_node.gettransaction(rbfid) - sync_mempools((rbf_node, peer_node)) + test.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() bumped_tx = rbf_node.bumpfee(rbfid) assert_equal(bumped_tx["errors"], []) assert bumped_tx["fee"] - abs(rbftx["fee"]) > 0 # check that bumped_tx propagates, original tx was evicted and has a wallet conflict - sync_mempools((rbf_node, peer_node)) + test.sync_mempools((rbf_node, peer_node)) assert bumped_tx["txid"] in rbf_node.getrawmempool() assert bumped_tx["txid"] in peer_node.getrawmempool() assert rbfid not in rbf_node.getrawmempool() diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 3d56c223007e..86c8bd7544c4 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -20,7 +20,7 @@ """ from test_framework.test_framework import PivxTestFramework -from test_framework.util import (assert_raises_rpc_error, connect_nodes, sync_blocks, assert_equal, set_node_times) +from test_framework.util import (assert_raises_rpc_error, connect_nodes, assert_equal, set_node_times) import collections import enum @@ -137,7 +137,7 @@ def run_test(self): timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] set_node_times(self.nodes, timestamp + TIMESTAMP_WINDOW + 1) self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # For each variation of wallet key import, invoke the import RPC and # check the results from getbalance and listtransactions. @@ -163,7 +163,7 @@ def run_test(self): # Generate a block containing the new transactions. self.nodes[0].generate(1) assert_equal(self.nodes[0].getrawmempool(), []) - sync_blocks(self.nodes) + self.sync_blocks() # Check the latest results from getbalance and listtransactions. for variant in IMPORT_VARIANTS: diff --git a/test/functional/wallet_import_stakingaddress.py b/test/functional/wallet_import_stakingaddress.py index 23040c4c5222..c768ace00235 100755 --- a/test/functional/wallet_import_stakingaddress.py +++ b/test/functional/wallet_import_stakingaddress.py @@ -15,7 +15,6 @@ from test_framework.util import ( assert_equal, DecimalAmt, - sync_blocks, ) class ImportStakingTest(PivxTestFramework): @@ -45,7 +44,7 @@ def run_test(self): # mine a block and check staking balance self.nodes[0].generate(1) assert_equal(self.nodes[0].getdelegatedbalance(), DecimalAmt(10 * (i+1))) - sync_blocks(self.nodes) + self.sync_blocks() # Export keys self.log.info("Exporting keys and importing in node 1") diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index 797e8a3cc9ae..f634538a9a40 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -16,7 +16,6 @@ from test_framework.util import ( assert_equal, connect_nodes, - sync_blocks, ) class KeypoolRestoreTest(PivxTestFramework): @@ -51,7 +50,7 @@ def run_test(self): self.nodes[0].generate(1) self.nodes[0].sendtoaddress(addr_extpool, 5) self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.log.info("Restart node with wallet backup") diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index 9134bde56dd8..0cfeabd3e963 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -10,7 +10,6 @@ assert_array_result, assert_equal, assert_raises_rpc_error, - sync_blocks, ) @@ -21,7 +20,7 @@ def set_test_params(self): def run_test(self): # Generate block to get out of IBD self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() self.log.info("listreceivedbyaddress Test") diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 40f9c02c02fe..a3b95f3be2e0 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -99,7 +99,8 @@ def test_reorg(self): self.nodes[2].generate(7) self.log.info('lastblockhash=%s' % (lastblockhash)) - self.sync_all([self.nodes[:2], self.nodes[2:]]) + self.sync_all(self.nodes[:2]) + self.sync_all(self.nodes[2:]) self.join_network() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 485cab08e843..20222cc4b968 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -6,14 +6,11 @@ from decimal import Decimal from io import BytesIO -from test_framework.mininode import CTransaction, COIN +from test_framework.mininode import CTransaction from test_framework.test_framework import PivxTestFramework from test_framework.util import ( assert_array_result, - assert_equal, - bytes_to_hex_str, hex_str_to_bytes, - sync_mempools, ) def txFromHex(hexstring): diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index 6d254c784bb2..a2d5ba2bc57d 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -23,7 +23,6 @@ assert_equal, connect_nodes, disconnect_nodes, - sync_blocks, ) class ReorgsRestoreTest(PivxTestFramework): @@ -37,7 +36,7 @@ def run_test(self): # Send a tx from which to conflict outputs later txid_conflict_from = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # Disconnect node1 from others to reorg its chain later disconnect_nodes(self.nodes[0], 1) @@ -72,7 +71,7 @@ def run_test(self): # Reconnect node0 and node2 and check that conflicted_txid is effectively conflicted connect_nodes(self.nodes[0], 2) - sync_blocks([self.nodes[0], self.nodes[2]]) + self.sync_blocks([self.nodes[0], self.nodes[2]]) conflicted = self.nodes[0].gettransaction(conflicted_txid) conflicting = self.nodes[0].gettransaction(conflicting_txid) assert_equal(conflicted["confirmations"], -9) diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index 2d63fae78033..4fcbb0e13a44 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -79,7 +79,7 @@ def run_test(self): # Have node0 mine a block, if requested: if (self.options.mine_block): self.nodes[0].generate(1) - sync_blocks(self.nodes[0:2]) + self.sync_blocks(self.nodes[0:2]) tx1 = self.nodes[0].gettransaction(txid1) tx2 = self.nodes[0].gettransaction(txid2) @@ -114,7 +114,7 @@ def run_test(self): self.nodes[2].sendrawtransaction(node0_tx2["hex"]) self.nodes[2].sendrawtransaction(tx2["hex"]) self.nodes[2].generate(1) # Mine another block to make sure we sync - sync_blocks(self.nodes) + self.sync_blocks() # Re-fetch transaction info: tx1 = self.nodes[0].gettransaction(txid1) diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py index 93559581ab72..4ea0994a17ac 100755 --- a/test/functional/wallet_txn_doublespend.py +++ b/test/functional/wallet_txn_doublespend.py @@ -69,7 +69,7 @@ def run_test(self): # Have node0 mine a block: if (self.options.mine_block): self.nodes[0].generate(1) - sync_blocks(self.nodes[0:2]) + self.sync_blocks(self.nodes[0:2]) tx1 = self.nodes[0].gettransaction(txid1) tx2 = self.nodes[0].gettransaction(txid2) @@ -105,7 +105,7 @@ def run_test(self): connect_nodes(self.nodes[2], 0) connect_nodes(self.nodes[2], 1) self.nodes[2].generate(1) # Mine another block to make sure we sync - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].gettransaction(doublespend_txid)["confirmations"], 2) # Re-fetch transaction info: diff --git a/test/functional/wallet_zapwallettxes.py b/test/functional/wallet_zapwallettxes.py index 19afe606810f..4b5f085ca709 100755 --- a/test/functional/wallet_zapwallettxes.py +++ b/test/functional/wallet_zapwallettxes.py @@ -18,7 +18,6 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, - sync_mempools, ) class ZapWalletTXesTest (PivxTestFramework): @@ -44,7 +43,7 @@ def run_test(self): # This transaction will not be confirmed txid2 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 20) - sync_mempools(self.nodes, wait=.1) + self.sync_mempools(wait=.1) # Confirmed and unconfirmed transactions are now in the wallet. assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1)