From 9a3bbe8fc57d88919acd4eadbc96124711f17ec2 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 25 Jan 2021 11:51:38 -0800 Subject: [PATCH 1/5] [test] Introduce a unit test helper to create a valid mempool transaction. --- src/test/util/setup_common.cpp | 49 ++++++++++++++++++++++++++++++++++ src/test/util/setup_common.h | 17 ++++++++++++ 2 files changed, 66 insertions(+) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 790329004c6..03ab3f523de 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -260,6 +260,55 @@ CBlock TestChain100Setup::CreateAndProcessBlock(const std::vectorGetHash(), input_vout); + CTxIn input(outpoint_to_spend); + mempool_txn.vin.push_back(input); + + // Create an output + CTxOut output(output_amount, output_destination); + mempool_txn.vout.push_back(output); + + // Sign the transaction + // - Add the signing key to a keystore + FillableSigningProvider keystore; + keystore.AddKey(input_signing_key); + // - Populate a CoinsViewCache with the unspent output + CCoinsView coins_view; + CCoinsViewCache coins_cache(&coins_view); + AddCoins(coins_cache, *input_transaction.get(), input_height); + // - Use GetCoin to properly populate utxo_to_spend, + Coin utxo_to_spend; + assert(coins_cache.GetCoin(outpoint_to_spend, utxo_to_spend)); + // - Then add it to a map to pass in to SignTransaction + std::map input_coins; + input_coins.insert({outpoint_to_spend, utxo_to_spend}); + // - Default signature hashing type + int nHashType = SIGHASH_ALL; + std::map input_errors; + assert(SignTransaction(mempool_txn, &keystore, input_coins, nHashType, input_errors)); + + // Add transaction to the mempool + { + LOCK(cs_main); + const MempoolAcceptResult result = AcceptToMemoryPool(*m_node.mempool.get(), MakeTransactionRef(mempool_txn), /* bypass_limits */ false); + assert(result.m_result_type == MempoolAcceptResult::ResultType::VALID); + } + + return mempool_txn; +} + TestChain100Setup::~TestChain100Setup() { gArgs.ForceSetArg("-segwitheight", "0"); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 4be4763f350..33f24e7c448 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -123,6 +123,23 @@ struct TestChain100Setup : public RegTestingSetup { //! Mine a series of new blocks on the active chain. void mineBlocks(int num_blocks); + /** + * Create a transaction and submit to the mempool. + * + * @param input_transaction The transaction to spend + * @param input_vout The vout to spend from the input_transaction + * @param input_height The height of the block that included the input_transaction + * @param input_signing_key The key to spend the input_transaction + * @param output_destination Where to send the output + * @param output_amount How much to send + */ + CMutableTransaction CreateValidMempoolTransaction(CTransactionRef input_transaction, + int input_vout, + int input_height, + CKey input_signing_key, + CScript output_destination, + CAmount output_amount = CAmount(1 * COIN)); + ~TestChain100Setup(); bool m_deterministic; From a2d908e1daa1d1be74568bd7d1d04b724da7d79c Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 2 Feb 2021 15:07:06 -0800 Subject: [PATCH 2/5] [test] Throw error instead of segfaulting in failure scenario If the miner code is faulty and does not include any transactions in a block, the code segfaults when it tries to access block transactions. Instead, add a check that safely aborts the process. --- src/test/miner_tests.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index e9672736362..9c06283a583 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -123,6 +123,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co m_node.mempool->addUnchecked(entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); std::unique_ptr pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 4); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx); BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx); BOOST_CHECK(pblocktemplate->block.vtx[3]->GetHash() == hashMediumFeeTx); @@ -157,6 +158,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co hashLowFeeTx = tx.GetHash(); m_node.mempool->addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6); BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx); BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx); @@ -191,6 +193,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee m_node.mempool->addUnchecked(entry.Fee(10000).FromTx(tx)); pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 9); BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2); } From df6a5fc1dff3b1b7c2f2b67aad1ff17cac99f7b6 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 10 Feb 2021 18:30:51 -0800 Subject: [PATCH 3/5] [util] Change GetMockTime to return chrono type instead of int --- src/logging.cpp | 6 +++--- src/util/time.cpp | 4 ++-- src/util/time.h | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 4ddcf1d9308..529bb11d5bd 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -203,9 +203,9 @@ std::string BCLog::Logger::LogTimestampStr(const std::string& str) strStamped.pop_back(); strStamped += strprintf(".%06dZ", nTimeMicros%1000000); } - int64_t mocktime = GetMockTime(); - if (mocktime) { - strStamped += " (mocktime: " + FormatISO8601DateTime(mocktime) + ")"; + std::chrono::seconds mocktime = GetMockTime(); + if (mocktime > 0s) { + strStamped += " (mocktime: " + FormatISO8601DateTime(count_seconds(mocktime)) + ")"; } strStamped += ' ' + str; } else diff --git a/src/util/time.cpp b/src/util/time.cpp index 295806c54af..2589ec12a04 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -53,9 +53,9 @@ void SetMockTime(int64_t nMockTimeIn) nMockTime.store(nMockTimeIn, std::memory_order_relaxed); } -int64_t GetMockTime() +std::chrono::seconds GetMockTime() { - return nMockTime.load(std::memory_order_relaxed); + return std::chrono::seconds(nMockTime.load(std::memory_order_relaxed)); } int64_t GetTimeMillis() diff --git a/src/util/time.h b/src/util/time.h index 03b75b5be56..38edc71de19 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -45,8 +45,9 @@ int64_t GetSystemTimeInSeconds(); // Like GetTime(), but not mockable /** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */ void SetMockTime(int64_t nMockTimeIn); + /** For testing */ -int64_t GetMockTime(); +std::chrono::seconds GetMockTime(); /** Return system time (or mocked time, if set) */ template From 47a7a1687d276bfa8769dee4bb78e8725f67a50e Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Fri, 12 Feb 2021 10:23:45 -0800 Subject: [PATCH 4/5] [util] Introduce a SetMockTime that takes chrono time --- src/util/time.cpp | 5 +++++ src/util/time.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/util/time.cpp b/src/util/time.cpp index 2589ec12a04..7b0eb8fd8e4 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -53,6 +53,11 @@ void SetMockTime(int64_t nMockTimeIn) nMockTime.store(nMockTimeIn, std::memory_order_relaxed); } +void SetMockTime(std::chrono::seconds mock_time_in) +{ + nMockTime.store(mock_time_in.count(), std::memory_order_relaxed); +} + std::chrono::seconds GetMockTime() { return std::chrono::seconds(nMockTime.load(std::memory_order_relaxed)); diff --git a/src/util/time.h b/src/util/time.h index 38edc71de19..52679a26473 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -46,6 +46,9 @@ int64_t GetSystemTimeInSeconds(); // Like GetTime(), but not mockable /** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */ void SetMockTime(int64_t nMockTimeIn); +/** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */ +void SetMockTime(std::chrono::seconds mock_time_in); + /** For testing */ std::chrono::seconds GetMockTime(); From 1363b6c27dbd2614fd555d148ea624ed8b95f14e Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 3 Feb 2021 18:35:23 -0800 Subject: [PATCH 5/5] [doc / util] Use comments to clarify time unit for int64_t type. --- src/util/time.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/time.h b/src/util/time.h index 52679a26473..c4f930781a1 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -43,7 +43,12 @@ int64_t GetTimeMicros(); /** Returns the system time (not mockable) */ int64_t GetSystemTimeInSeconds(); // Like GetTime(), but not mockable -/** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */ +/** + * DEPRECATED + * Use SetMockTime with chrono type + * + * @param[in] nMockTimeIn Time in seconds. + */ void SetMockTime(int64_t nMockTimeIn); /** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */