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.

3 files changed, 67 insertions, 61 deletions

diff --git a/src/main.cpp b/src/main.cpp
index a4f90dda71..42f70c023c 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<uint256, CCoins> &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<uint256,CCoins>::iterator CCoinsViewCache::FetchCoins(uint256 txid) {
+    std::map<uint256,CCoins>::iterator it = cacheCoins.find(txid);
+    if (it != cacheCoins.end())
+        return it;
+    CCoins tmp;
+    if (!base->GetCoins(txid,tmp))
+        return it;
+    std::pair<std::map<uint256,CCoins>::iterator,bool> ret = cacheCoins.insert(std::make_pair(txid, tmp));
+    return ret.first;
+}
+
+CCoins &CCoinsViewCache::GetCoins(uint256 txid) {
+    std::map<uint256,CCoins>::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<vtx.size(); i++) {
             uint256 hash = GetTxHash(i);
-            CCoins coins;
-            if (view.GetCoins(hash, coins) && !coins.IsPruned())
+            if (view.HaveCoins(hash) && !view.GetCoins(hash).IsPruned())
                 return error("ConnectBlock() : tried to overwrite transaction");
         }
     }
@@ -3719,7 +3720,7 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
                 std::make_heap(vecPriority.begin(), vecPriority.end(), comparer);
             }
 
-            if (!tx.CheckInputs(viewTemp, CS_ALWAYS, true, false))
+            if (!tx.HaveInputs(viewTemp))
                 continue;
 
             int64 nTxFees = tx.GetValueIn(viewTemp)-tx.GetValueOut();
@@ -3728,6 +3729,9 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
             if (nBlockSigOps + nTxSigOps >= 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 3ee0108f46..1150ca4c02 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<uint256, CCoins> &mapCoins, CBlockIndex *pindex);
     bool Flush();
     unsigned int GetCacheSize();
+
+private:
+    std::map<uint256,CCoins>::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 17b925c340..4385b9ba37 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()