From 13c51f20f619b9001bb6caf225b8cd6c5c2fbb31 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 8 Jul 2012 19:04:05 +0200 Subject: [PATCH] Direct CCoins references To prevent excessive copying of CCoins in and out of the CCoinsView implementations, introduce a GetCoins() function in CCoinsViewCache with returns a direct reference. The block validation and connection logic is updated to require caching CCoinsViews, and exploits the GetCoins() function heavily. --- src/main.cpp | 102 +++++++++++++++++---------------- src/main.h | 23 +++++--- src/test/transaction_tests.cpp | 3 - 3 files changed, 67 insertions(+), 61 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a4f90dda7..42f70c023 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -170,6 +170,7 @@ CBlockIndex *CCoinsView::GetBestBlock() { return NULL; } bool CCoinsView::SetBestBlock(CBlockIndex *pindex) { return false; } bool CCoinsView::BatchWrite(const std::map &mapCoins, CBlockIndex *pindex) { return false; } + CCoinsViewBacked::CCoinsViewBacked(CCoinsView &viewIn) : base(&viewIn) { } bool CCoinsViewBacked::GetCoins(uint256 txid, CCoins &coins) { return base->GetCoins(txid, coins); } bool CCoinsViewBacked::SetCoins(uint256 txid, const CCoins &coins) { return base->SetCoins(txid, coins); } @@ -193,13 +194,30 @@ bool CCoinsViewCache::GetCoins(uint256 txid, CCoins &coins) { return false; } +std::map::iterator CCoinsViewCache::FetchCoins(uint256 txid) { + std::map::iterator it = cacheCoins.find(txid); + if (it != cacheCoins.end()) + return it; + CCoins tmp; + if (!base->GetCoins(txid,tmp)) + return it; + std::pair::iterator,bool> ret = cacheCoins.insert(std::make_pair(txid, tmp)); + return ret.first; +} + +CCoins &CCoinsViewCache::GetCoins(uint256 txid) { + std::map::iterator it = FetchCoins(txid); + assert(it != cacheCoins.end()); + return it->second; +} + bool CCoinsViewCache::SetCoins(uint256 txid, const CCoins &coins) { cacheCoins[txid] = coins; return true; } bool CCoinsViewCache::HaveCoins(uint256 txid) { - return cacheCoins.count(txid) || base->HaveCoins(txid); + return FetchCoins(txid) != cacheCoins.end(); } CBlockIndex *CCoinsViewCache::GetBestBlock() { @@ -369,7 +387,7 @@ bool CTransaction::IsStandard() const // expensive-to-check-upon-redemption script like: // DUP CHECKSIG DROP ... repeated 100 times... OP_1 // -bool CTransaction::AreInputsStandard(CCoinsView& mapInputs) const +bool CTransaction::AreInputsStandard(CCoinsViewCache& mapInputs) const { if (IsCoinBase()) return true; // Coinbases don't use vin normally @@ -683,6 +701,9 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, } } + if (!tx.HaveInputs(view)) + return error("CTxMemPool::accept() : inputs already spent"); + // Check for non-standard pay-to-script-hash in inputs if (!tx.AreInputsStandard(view) && !fTestNet) return error("CTxMemPool::accept() : nonstandard transaction input"); @@ -1154,23 +1175,14 @@ void CBlock::UpdateTime(const CBlockIndex* pindexPrev) -CTxOut CTransaction::GetOutputFor(const CTxIn& input, CCoinsView& view) +const CTxOut &CTransaction::GetOutputFor(const CTxIn& input, CCoinsViewCache& view) { - CCoins coins; - if (!view.GetCoins(input.prevout.hash, coins)) - throw std::runtime_error("CTransaction::GetOutputFor() : prevout.hash not found"); - - if (input.prevout.n >= coins.vout.size()) - throw std::runtime_error("CTransaction::GetOutputFor() : prevout.n out of range or already spent"); - - const CTxOut &out = coins.vout[input.prevout.n]; - if (out.IsNull()) - throw std::runtime_error("CTransaction::GetOutputFor() : already spent"); - - return out; + const CCoins &coins = view.GetCoins(input.prevout.hash); + assert(coins.IsAvailable(input.prevout.n)); + return coins.vout[input.prevout.n]; } -int64 CTransaction::GetValueIn(CCoinsView& inputs) const +int64 CTransaction::GetValueIn(CCoinsViewCache& inputs) const { if (IsCoinBase()) return 0; @@ -1182,7 +1194,7 @@ int64 CTransaction::GetValueIn(CCoinsView& inputs) const return nResult; } -unsigned int CTransaction::GetP2SHSigOpCount(CCoinsView& inputs) const +unsigned int CTransaction::GetP2SHSigOpCount(CCoinsViewCache& inputs) const { if (IsCoinBase()) return 0; @@ -1190,27 +1202,23 @@ unsigned int CTransaction::GetP2SHSigOpCount(CCoinsView& inputs) const unsigned int nSigOps = 0; for (unsigned int i = 0; i < vin.size(); i++) { - CTxOut prevout = GetOutputFor(vin[i], inputs); + const CTxOut &prevout = GetOutputFor(vin[i], inputs); if (prevout.scriptPubKey.IsPayToScriptHash()) nSigOps += prevout.scriptPubKey.GetSigOpCount(vin[i].scriptSig); } return nSigOps; } -bool CTransaction::UpdateCoins(CCoinsView &inputs, CTxUndo &txundo, int nHeight, const uint256 &txhash) const +bool CTransaction::UpdateCoins(CCoinsViewCache &inputs, CTxUndo &txundo, int nHeight, const uint256 &txhash) const { // mark inputs spent if (!IsCoinBase()) { BOOST_FOREACH(const CTxIn &txin, vin) { - CCoins coins; - if (!inputs.GetCoins(txin.prevout.hash, coins)) - return error("UpdateCoins() : cannot find prevtx"); + CCoins &coins = inputs.GetCoins(txin.prevout.hash); CTxInUndo undo; if (!coins.Spend(txin.prevout, undo)) return error("UpdateCoins() : cannot spend input"); txundo.vprevout.push_back(undo); - if (!inputs.SetCoins(txin.prevout.hash, coins)) - return error("UpdateCoins() : cannot update input"); } } @@ -1221,7 +1229,7 @@ bool CTransaction::UpdateCoins(CCoinsView &inputs, CTxUndo &txundo, int nHeight, return true; } -bool CTransaction::HaveInputs(CCoinsView &inputs) const +bool CTransaction::HaveInputs(CCoinsViewCache &inputs) const { if (!IsCoinBase()) { // first check whether information about the prevout hash is available @@ -1234,8 +1242,7 @@ bool CTransaction::HaveInputs(CCoinsView &inputs) const // then check whether the actual outputs are available for (unsigned int i = 0; i < vin.size(); i++) { const COutPoint &prevout = vin[i].prevout; - CCoins coins; - inputs.GetCoins(prevout.hash, coins); + const CCoins &coins = inputs.GetCoins(prevout.hash); if (!coins.IsAvailable(prevout.n)) return false; } @@ -1243,28 +1250,25 @@ bool CTransaction::HaveInputs(CCoinsView &inputs) const return true; } -bool CTransaction::CheckInputs(CCoinsView &inputs, enum CheckSig_mode csmode, bool fStrictPayToScriptHash, bool fStrictEncodings) const +bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmode, bool fStrictPayToScriptHash, bool fStrictEncodings) const { if (!IsCoinBase()) { + // This doesn't trigger the DoS code on purpose; if it did, it would make it easier + // for an attacker to attempt to split the network. + if (!HaveInputs(inputs)) + return error("CheckInputs() : %s inputs unavailable", GetHash().ToString().substr(0,10).c_str()); + + CBlockIndex *pindexBlock = inputs.GetBestBlock(); int64 nValueIn = 0; int64 nFees = 0; for (unsigned int i = 0; i < vin.size(); i++) { const COutPoint &prevout = vin[i].prevout; - CCoins coins; - if (!inputs.GetCoins(prevout.hash, coins)) - return error("CheckInputs() : cannot find prevout tx"); - - // Check for conflicts (double-spend) - // This doesn't trigger the DoS code on purpose; if it did, it would make it easier - // for an attacker to attempt to split the network. - if (!coins.IsAvailable(prevout.n)) - return error("CheckInputs() : %s prev tx already used", GetHash().ToString().substr(0,10).c_str()); + const CCoins &coins = inputs.GetCoins(prevout.hash); // If prev is coinbase, check that it's matured if (coins.IsCoinBase()) { - CBlockIndex *pindexBlock = inputs.GetBestBlock(); if (pindexBlock->nHeight - coins.nHeight < COINBASE_MATURITY) return error("CheckInputs() : tried to spend coinbase at depth %d", pindexBlock->nHeight - coins.nHeight); } @@ -1298,8 +1302,7 @@ bool CTransaction::CheckInputs(CCoinsView &inputs, enum CheckSig_mode csmode, bo (csmode == CS_AFTER_CHECKPOINT && inputs.GetBestBlock()->nHeight >= Checkpoints::GetTotalBlocksEstimate())) { for (unsigned int i = 0; i < vin.size(); i++) { const COutPoint &prevout = vin[i].prevout; - CCoins coins; - inputs.GetCoins(prevout.hash, coins); + const CCoins &coins = inputs.GetCoins(prevout.hash); // Verify signature if (!VerifySignature(coins, *this, i, fStrictPayToScriptHash, fStrictEncodings, 0)) { @@ -1367,7 +1370,7 @@ bool CTransaction::ClientCheckInputs() const -bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsView &view) +bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view) { assert(pindex == view.GetBestBlock()); @@ -1391,17 +1394,16 @@ bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsView &view) uint256 hash = tx.GetHash(); // check that all outputs are available - CCoins outs; - if (!view.GetCoins(hash, outs)) + if (!view.HaveCoins(hash)) return error("DisconnectBlock() : outputs still spent? database corrupted"); + CCoins &outs = view.GetCoins(hash); CCoins outsBlock = CCoins(tx, pindex->nHeight); if (outs != outsBlock) return error("DisconnectBlock() : added transaction mismatch? database corrupted"); // remove outputs - if (!view.SetCoins(hash, CCoins())) - return error("DisconnectBlock() : cannot delete coin outputs"); + outs = CCoins(); // restore inputs if (i > 0) { // not coinbases @@ -1441,7 +1443,7 @@ bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsView &view) bool FindUndoPos(CChainDB &chaindb, int nFile, CDiskBlockPos &pos, unsigned int nAddSize); -bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsView &view, bool fJustCheck) +bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJustCheck) { // Check it again in case a previous version let a bad block in if (!CheckBlock(!fJustCheck, !fJustCheck)) @@ -1467,8 +1469,7 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsView &view, bool fJustCheck if (fEnforceBIP30) { for (unsigned int i=0; i= MAX_BLOCK_SIGOPS) continue; + if (!tx.CheckInputs(viewTemp, CS_ALWAYS, true, false)) + continue; + CTxUndo txundo; uint256 hash = tx.GetHash(); if (!tx.UpdateCoins(viewTemp, txundo, pindexPrev->nHeight+1, hash)) diff --git a/src/main.h b/src/main.h index 3ee0108f4..1150ca4c0 100644 --- a/src/main.h +++ b/src/main.h @@ -88,6 +88,7 @@ class CDiskBlockPos; class CCoins; class CTxUndo; class CCoinsView; +class CCoinsViewCache; void RegisterWallet(CWallet* pwalletIn); void UnregisterWallet(CWallet* pwalletIn); @@ -480,7 +481,7 @@ public: @return True if all inputs (scriptSigs) use only standard transaction forms @see CTransaction::FetchInputs */ - bool AreInputsStandard(CCoinsView& mapInputs) const; + bool AreInputsStandard(CCoinsViewCache& mapInputs) const; /** Count ECDSA signature operations the old-fashioned (pre-0.6) way @return number of sigops this transaction's outputs will produce when spent @@ -494,7 +495,7 @@ public: @return maximum number of sigops required to validate this transaction's inputs @see CTransaction::FetchInputs */ - unsigned int GetP2SHSigOpCount(CCoinsView& mapInputs) const; + unsigned int GetP2SHSigOpCount(CCoinsViewCache& mapInputs) const; /** Amount of bitcoins spent by this transaction. @return sum of all outputs (note: does not include fees) @@ -519,7 +520,7 @@ public: @return Sum of value of all inputs (scriptSigs) @see CTransaction::FetchInputs */ - int64 GetValueIn(CCoinsView& mapInputs) const; + int64 GetValueIn(CCoinsViewCache& mapInputs) const; static bool AllowFree(double dPriority) { @@ -570,14 +571,14 @@ public: bool ClientCheckInputs() const; // Check whether all prevouts of this transaction are present in the UTXO set represented by view - bool HaveInputs(CCoinsView &view) const; + bool HaveInputs(CCoinsViewCache &view) const; // Check whether all inputs of this transaction are valid (no double spends, scripts & sigs, amounts) // This does not modify the UTXO set - bool CheckInputs(CCoinsView &view, enum CheckSig_mode csmode, bool fStrictPayToScriptHash=true, bool fStrictEncodings=true) const; + bool CheckInputs(CCoinsViewCache &view, enum CheckSig_mode csmode, bool fStrictPayToScriptHash=true, bool fStrictEncodings=true) const; // Apply the effects of this transaction on the UTXO set represented by view - bool UpdateCoins(CCoinsView &view, CTxUndo &txundo, int nHeight, const uint256 &txhash) const; + bool UpdateCoins(CCoinsViewCache &view, CTxUndo &txundo, int nHeight, const uint256 &txhash) const; // Context-independent validity checks bool CheckTransaction() const; @@ -586,7 +587,7 @@ public: bool AcceptToMemoryPool(bool fCheckInputs=true, bool* pfMissingInputs=NULL); protected: - static CTxOut GetOutputFor(const CTxIn& input, CCoinsView& mapInputs); + static const CTxOut &GetOutputFor(const CTxIn& input, CCoinsViewCache& mapInputs); }; /** wrapper for CTxOut that provides a more compact serialization */ @@ -1225,10 +1226,10 @@ public: // Undo the effects of this block (with given index) on the UTXO set represented by coins - bool DisconnectBlock(CBlockIndex *pindex, CCoinsView &coins); + bool DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &coins); // Apply the effects of this block (with given index) on the UTXO set represented by coins - bool ConnectBlock(CBlockIndex *pindex, CCoinsView &coins, bool fJustCheck=false); + bool ConnectBlock(CBlockIndex *pindex, CCoinsViewCache &coins, bool fJustCheck=false); // Read a block from disk bool ReadFromDisk(const CBlockIndex* pindex, bool fReadTransactions=true); @@ -1759,11 +1760,15 @@ public: bool GetCoins(uint256 txid, CCoins &coins); bool SetCoins(uint256 txid, const CCoins &coins); bool HaveCoins(uint256 txid); + CCoins &GetCoins(uint256 txid); CBlockIndex *GetBestBlock(); bool SetBestBlock(CBlockIndex *pindex); bool BatchWrite(const std::map &mapCoins, CBlockIndex *pindex); bool Flush(); unsigned int GetCacheSize(); + +private: + std::map::iterator FetchCoins(uint256 txid); }; /** CCoinsView that brings transactions from a memorypool into view. diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 17b925c34..4385b9ba3 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -256,9 +256,6 @@ BOOST_AUTO_TEST_CASE(test_GetThrow) t1.vout.resize(2); t1.vout[0].nValue = 90*CENT; t1.vout[0].scriptPubKey << OP_1; - - BOOST_CHECK_THROW(t1.AreInputsStandard(coinsDummy), runtime_error); - BOOST_CHECK_THROW(t1.GetValueIn(coinsDummy), runtime_error); } BOOST_AUTO_TEST_SUITE_END()