From 9fce0629b437441cb09c14055ee810b8ee6b7978 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Sat, 18 Jun 2016 19:38:28 +0200 Subject: [PATCH] [c++11] Use std::unique_ptr for block creation. CreateNewBlock returns a pointer for which the caller takes ownership. Use std::unique_ptr to make this explicit and simplify handling of these objects in getblocktemplate. --- src/miner.cpp | 7 ++++--- src/miner.h | 2 +- src/rpc/mining.cpp | 10 +++------- src/test/miner_tests.cpp | 15 ++++----------- src/test/test_bitcoin.cpp | 5 +++-- 5 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 989ad11a2..d38ccedf5 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -29,6 +29,7 @@ #include #include #include +#include using namespace std; @@ -102,14 +103,14 @@ void BlockAssembler::resetBlock() blockFinished = false; } -CBlockTemplate* BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) +std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) { resetBlock(); pblocktemplate.reset(new CBlockTemplate()); if(!pblocktemplate.get()) - return NULL; + return nullptr; pblock = &pblocktemplate->block; // pointer for convenience // Add dummy coinbase tx as first transaction @@ -164,7 +165,7 @@ CBlockTemplate* BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state))); } - return pblocktemplate.release(); + return std::move(pblocktemplate); } bool BlockAssembler::isStillDependent(CTxMemPool::txiter iter) diff --git a/src/miner.h b/src/miner.h index a9fea8530..54c680b5f 100644 --- a/src/miner.h +++ b/src/miner.h @@ -160,7 +160,7 @@ private: public: BlockAssembler(const CChainParams& chainparams); /** Construct a new block template with coinbase to scriptPubKeyIn */ - CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn); + std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn); private: // utility functions diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 94eeea91f..162f4a072 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -22,6 +22,7 @@ #include "utilstrencodings.h" #include "validationinterface.h" +#include #include #include @@ -508,12 +509,12 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) // Update block static CBlockIndex* pindexPrev; static int64_t nStart; - static CBlockTemplate* pblocktemplate; + static std::unique_ptr pblocktemplate; if (pindexPrev != chainActive.Tip() || (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on - pindexPrev = NULL; + pindexPrev = nullptr; // Store the pindexBest used before CreateNewBlock, to avoid races nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); @@ -521,11 +522,6 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) nStart = GetTime(); // Create new block - if(pblocktemplate) - { - delete pblocktemplate; - pblocktemplate = NULL; - } CScript scriptDummy = CScript() << OP_TRUE; pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy); if (!pblocktemplate) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index ca8d6d2e0..7b087763d 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -18,6 +18,8 @@ #include "test/test_bitcoin.h" +#include + #include BOOST_FIXTURE_TEST_SUITE(miner_tests, TestingSetup) @@ -105,7 +107,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, uint256 hashHighFeeTx = tx.GetHash(); mempool.addUnchecked(hashHighFeeTx, entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - CBlockTemplate *pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); + std::unique_ptr pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); BOOST_CHECK(pblocktemplate->block.vtx[1].GetHash() == hashParentTx); BOOST_CHECK(pblocktemplate->block.vtx[2].GetHash() == hashHighFeeTx); BOOST_CHECK(pblocktemplate->block.vtx[3].GetHash() == hashMediumFeeTx); @@ -183,7 +185,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { const CChainParams& chainparams = Params(CBaseChainParams::MAIN); CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; - CBlockTemplate *pblocktemplate; + std::unique_ptr pblocktemplate; CMutableTransaction tx,tx2; CScript script; uint256 hash; @@ -225,11 +227,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_CHECK(state.IsValid()); pblock->hashPrevBlock = pblock->GetHash(); } - delete pblocktemplate; // Just to make sure we can still make simple blocks BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; const CAmount BLOCKSUBSIDY = 50*COIN; const CAmount LOWFEE = CENT; @@ -268,7 +268,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; mempool.clear(); // block size > limit @@ -289,7 +288,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; mempool.clear(); // orphan in mempool, template creation fails @@ -313,7 +311,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(HIGHERFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; mempool.clear(); // coinbase in mempool, template creation fails @@ -371,7 +368,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) chainActive.SetTip(next); } BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; // Extend to a 210000-long block chain. while (chainActive.Tip()->nHeight < 210000) { CBlockIndex* prev = chainActive.Tip(); @@ -384,7 +380,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) chainActive.SetTip(next); } BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - delete pblocktemplate; // Delete the dummy blocks again. while (chainActive.Tip()->nHeight > nHeight) { CBlockIndex* del = chainActive.Tip(); @@ -477,7 +472,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // but relative locked txs will if inconsistently added to mempool. // For now these will still generate a valid template until BIP68 soft fork BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3); - delete pblocktemplate; // However if we advance height by 1 and time by 512, all of them should be mined for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) chainActive.Tip()->GetAncestor(chainActive.Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast @@ -486,7 +480,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5); - delete pblocktemplate; chainActive.Tip()->nHeight--; SetMockTime(0); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index c68320ba8..98b02a1f6 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -22,6 +22,8 @@ #include "test/testutil.h" +#include + #include #include #include @@ -98,7 +100,7 @@ CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector& txns, const CScript& scriptPubKey) { const CChainParams& chainparams = Params(); - CBlockTemplate *pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); + std::unique_ptr pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); CBlock& block = pblocktemplate->block; // Replace mempool-selected txns with just coinbase plus passed-in txns: @@ -115,7 +117,6 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector& ProcessNewBlock(state, chainparams, NULL, &block, true, NULL); CBlock result = block; - delete pblocktemplate; return result; }