From f8001c08e403880653af187a82627fc171e38c21 Mon Sep 17 00:00:00 2001 From: Amaury SECHET Date: Thu, 11 May 2017 19:32:46 +0200 Subject: [PATCH] Split the block undoing in two parts and add unit tests. The first read the undo data from disk and the second does the undo. This ensure we don't depend on the state of the filesystem to undo and makes is easier to unit test. The patch also includes a basic unit test for undo. --- src/Makefile.test.include | 1 + src/test/undo_tests.cpp | 105 ++++++++++++++++++++++++++++++++++++++ src/undo.h | 16 ++++++ src/validation.cpp | 63 +++++++++++++---------- src/validation.h | 6 ++- 5 files changed, 163 insertions(+), 28 deletions(-) create mode 100644 src/test/undo_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 2e315957be..d00c566c64 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -122,6 +122,7 @@ BITCOIN_TESTS =\ test/txvalidationcache_tests.cpp \ test/versionbits_tests.cpp \ test/uint256_tests.cpp \ + test/undo_tests.cpp \ test/univalue_tests.cpp \ test/util_tests.cpp diff --git a/src/test/undo_tests.cpp b/src/test/undo_tests.cpp new file mode 100644 index 0000000000..8df726da28 --- /dev/null +++ b/src/test/undo_tests.cpp @@ -0,0 +1,105 @@ +// Copyright (c) 2017 Amaury SÉCHET +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "undo.h" +#include "chainparams.h" +#include "consensus/validation.h" +#include "validation.h" + +#include "test/test_bitcoin.h" + +#include + +BOOST_FIXTURE_TEST_SUITE(undo_tests, BasicTestingSetup) + +static void UpdateUTXOSet(const CBlock &block, CCoinsViewCache &view, + CBlockUndo &blockundo, + const CChainParams &chainparams, uint32_t nHeight) { + CValidationState state; + + auto &coinbaseTx = *block.vtx[0]; + UpdateCoins(coinbaseTx, view, nHeight); + + for (size_t i = 1; i < block.vtx.size(); i++) { + auto &tx = *block.vtx[1]; + + blockundo.vtxundo.push_back(CTxUndo()); + UpdateCoins(tx, view, blockundo.vtxundo.back(), nHeight); + } + + view.SetBestBlock(block.GetHash()); +} + +static void UndoBlock(const CBlock &block, CCoinsViewCache &view, + const CBlockUndo &blockundo, + const CChainParams &chainparams, uint32_t nHeight) { + CValidationState state; + + CBlockIndex pindex; + pindex.nHeight = nHeight; + + ApplyBlockUndo(block, state, &pindex, view, blockundo); +} + +static bool HashSpendableCoin(const CCoinsViewCache &view, uint256 txid) { + const CCoins *coins = view.AccessCoins(txid); + return coins != nullptr && !coins->IsPruned(); +} + +BOOST_AUTO_TEST_CASE(connect_utxo_extblock) { + SelectParams(CBaseChainParams::MAIN); + const CChainParams &chainparams = Params(); + + CBlock block; + CMutableTransaction tx; + + CCoinsView coinsDummy; + CCoinsViewCache view(&coinsDummy); + + block.hashPrevBlock = GetRandHash(); + view.SetBestBlock(block.hashPrevBlock); + + // Create a block with coinbase and resolution transaction. + tx.vin.resize(1); + tx.vin[0].scriptSig.resize(10); + tx.vout.resize(1); + tx.vout[0].nValue = 42; + auto coinbaseTx = CTransaction(tx); + + block.vtx.resize(2); + block.vtx[0] = MakeTransactionRef(tx); + + tx.vout[0].scriptPubKey = CScript() << OP_TRUE; + tx.vin[0].prevout.hash = GetRandHash(); + tx.vin[0].prevout.n = 0; + tx.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; + tx.vin[0].scriptSig.resize(0); + tx.nVersion = 2; + + auto prevTx0 = CTransaction(tx); + view.ModifyNewCoins(prevTx0.GetId(), prevTx0.IsCoinBase()) + ->FromTx(prevTx0, 100); + + tx.vin[0].prevout.hash = prevTx0.GetId(); + auto tx0 = CTransaction(tx); + block.vtx[1] = MakeTransactionRef(tx0); + + // Now update the UTXO set. + CBlockUndo blockundo; + UpdateUTXOSet(block, view, blockundo, chainparams, 123456); + + BOOST_CHECK(view.GetBestBlock() == block.GetHash()); + BOOST_CHECK(HashSpendableCoin(view, coinbaseTx.GetId())); + BOOST_CHECK(HashSpendableCoin(view, tx0.GetId())); + BOOST_CHECK(!HashSpendableCoin(view, prevTx0.GetId())); + + UndoBlock(block, view, blockundo, chainparams, 123456); + + BOOST_CHECK(view.GetBestBlock() == block.hashPrevBlock); + BOOST_CHECK(!HashSpendableCoin(view, coinbaseTx.GetId())); + BOOST_CHECK(!HashSpendableCoin(view, tx0.GetId())); + BOOST_CHECK(HashSpendableCoin(view, prevTx0.GetId())); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/undo.h b/src/undo.h index c04353f80b..94c6832ac5 100644 --- a/src/undo.h +++ b/src/undo.h @@ -1,5 +1,6 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2017 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -10,6 +11,11 @@ #include "primitives/transaction.h" #include "serialize.h" +class CBlock; +class CBlockIndex; +class CCoinsViewCache; +class CValidationState; + /** Undo information for a CTxIn * * Contains the prevout's CTxOut being spent, and if this was the @@ -77,4 +83,14 @@ class CBlockUndo { } }; +/** Apply the undo operation of a CTxInUndo to the given chain state. */ +bool ApplyTxInUndo(const CTxInUndo &undo, CCoinsViewCache &view, + const COutPoint &out); + +/** Undo a block from the block and the undoblock data. + * See DisconnectBlock for more details. */ +bool ApplyBlockUndo(const CBlock &block, CValidationState &state, + const CBlockIndex *pindex, CCoinsViewCache &coins, + const CBlockUndo &blockUndo, bool *pfClean = nullptr); + #endif // BITCOIN_UNDO_H diff --git a/src/validation.cpp b/src/validation.cpp index 50121eea70..8bae3604c5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1,5 +1,6 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2017 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -1221,7 +1222,7 @@ void UpdateCoins(const CTransaction &tx, CCoinsViewCache &inputs, // mark inputs spent if (!tx.IsCoinBase()) { txundo.vprevout.reserve(tx.vin.size()); - BOOST_FOREACH (const CTxIn &txin, tx.vin) { + for (const CTxIn &txin : tx.vin) { CCoinsModifier coins = inputs.ModifyCoins(txin.prevout.hash); unsigned nPos = txin.prevout.n; @@ -1506,61 +1507,69 @@ bool DisconnectBlock(const CBlock &block, CValidationState &state, if (pfClean) *pfClean = false; - bool fClean = true; - CBlockUndo blockUndo; CDiskBlockPos pos = pindex->GetUndoPos(); if (pos.IsNull()) return error("DisconnectBlock(): no undo data available"); if (!UndoReadFromDisk(blockUndo, pos, pindex->pprev->GetBlockHash())) return error("DisconnectBlock(): failure reading undo data"); + return ApplyBlockUndo(block, state, pindex, view, blockUndo, pfClean); +} + +bool ApplyBlockUndo(const CBlock &block, CValidationState &state, + const CBlockIndex *pindex, CCoinsViewCache &view, + const CBlockUndo &blockUndo, bool *pfClean) { + if (pfClean) *pfClean = false; + bool fClean = true; + if (blockUndo.vtxundo.size() + 1 != block.vtx.size()) return error("DisconnectBlock(): block and undo data inconsistent"); - // undo transactions in reverse order - for (int i = block.vtx.size() - 1; i >= 0; i--) { + // Undo transactions in reverse order. + size_t i = block.vtx.size(); + while (i-- > 0) { const CTransaction &tx = *(block.vtx[i]); uint256 txid = tx.GetId(); // Check that all outputs are available and match the outputs in the - // block itself - // exactly. + // block itself exactly. { CCoinsModifier outs = view.ModifyCoins(txid); outs->ClearUnspendable(); CCoins outsBlock(tx, pindex->nHeight); - // The CCoins serialization does not serialize negative numbers. - // No network rules currently depend on the version here, so an - // inconsistency is harmless - // but it must be corrected before txout nversion ever influences a - // network rule. + // The CCoins serialization does not serialize negative numbers. No + // network rules currently depend on the version here, so an + // inconsistency is harmless but it must be corrected before txout + // nversion ever influences a network rule. if (outsBlock.nVersion < 0) outs->nVersion = outsBlock.nVersion; if (*outs != outsBlock) fClean = fClean && error("DisconnectBlock(): added transaction " "mismatch? database corrupted"); - // remove outputs + // Remove outputs. outs->Clear(); } - // restore inputs - if (i > 0) { // not coinbases - const CTxUndo &txundo = blockUndo.vtxundo[i - 1]; - if (txundo.vprevout.size() != tx.vin.size()) - return error("DisconnectBlock(): transaction and undo data " - "inconsistent"); - for (unsigned int j = tx.vin.size(); j-- > 0;) { - const COutPoint &out = tx.vin[j].prevout; - const CTxInUndo &undo = txundo.vprevout[j]; - if (!ApplyTxInUndo(undo, view, out)) fClean = false; - } + // Restore inputs. + if (i < 1) { + // Skip the coinbase. + continue; } - } - // move best block pointer to prevout block - view.SetBestBlock(pindex->pprev->GetBlockHash()); + const CTxUndo &txundo = blockUndo.vtxundo[i - 1]; + if (txundo.vprevout.size() != tx.vin.size()) + return error("DisconnectBlock(): transaction and undo data " + "inconsistent"); + for (unsigned int j = tx.vin.size(); j-- > 0;) { + const COutPoint &out = tx.vin[j].prevout; + const CTxInUndo &undo = txundo.vprevout[j]; + if (!ApplyTxInUndo(undo, view, out)) fClean = false; + } + } + // Move best block pointer to previous block. + view.SetBestBlock(block.hashPrevBlock); if (pfClean) { *pfClean = fClean; return true; diff --git a/src/validation.h b/src/validation.h index 41d165710b..f40f6194e6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1,5 +1,6 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2017 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -40,6 +41,7 @@ class CInv; class CConnman; class CScriptCheck; class CTxMemPool; +class CTxUndo; class CValidationInterface; class CValidationState; struct ChainTxData; @@ -436,6 +438,8 @@ bool CheckInputs(const CTransaction &tx, CValidationState &state, /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction &tx, CCoinsViewCache &inputs, int nHeight); +void UpdateCoins(const CTransaction &tx, CCoinsViewCache &inputs, + CTxUndo &txundo, int nHeight); /** Transaction validation functions */ @@ -590,7 +594,7 @@ bool ConnectBlock(const CBlock &block, CValidationState &state, * any case, coins may be modified. */ bool DisconnectBlock(const CBlock &block, CValidationState &state, const CBlockIndex *pindex, CCoinsViewCache &coins, - bool *pfClean = NULL); + bool *pfClean = nullptr); /** Check a block is completely valid from start to finish (only works on top of * our current best block, with cs_main held) */