Replace CTxInUndo with Coin

The earlier CTxInUndo class now holds the same information as the Coin
class. Instead of duplicating functionality, replace CTxInUndo with a
serialization adapter for Coin.

Author: sipa

src/test/coins_tests.cpp

@@ -17,7 +17,7 @@
 
 #include <boost/test/unit_test.hpp>
 
-int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out);
+int ApplyTxInUndo(const Coin& undo, CCoinsViewCache& view, const COutPoint& out);
 void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txundo, int nHeight);
 
 namespace
@@ -371,7 +371,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
             // restore inputs
             if (!tx.IsCoinBase()) {
                 const COutPoint &out = tx.vin[0].prevout;
-                const CTxInUndo &undoin = undo.vprevout[0];
+                const Coin &undoin = undo.vprevout[0];
                 ApplyTxInUndo(undoin, *(stack.back()), out);
             }
             // Store as a candidate for reconnection

src/undo.h

@@ -7,61 +7,90 @@
 #define BITCOIN_UNDO_H
 
 #include "compressor.h" 
+#include "consensus/consensus.h"
 #include "primitives/transaction.h"
 #include "serialize.h"
 
 /** Undo information for a CTxIn
  *
  *  Contains the prevout's CTxOut being spent, and its metadata as well
- *  (coinbase or not, height). Earlier versions also stored the transaction
- *  version.
+ *  (coinbase or not, height). The serialization contains a dummy value of
+ *  zero. This is be compatible with older versions which expect to see
+ *  the transaction version there.
  */
-class CTxInUndo
+class TxInUndoSerializer
 {
-public:
-    CTxOut txout;         // the txout data before being spent
-    bool fCoinBase;       // if the outpoint was the last unspent: whether it belonged to a coinbase
-    unsigned int nHeight; // if the outpoint was the last unspent: its height
-
-    CTxInUndo() : txout(), fCoinBase(false), nHeight(0) {}
-    CTxInUndo(const CTxOut &txoutIn, bool fCoinBaseIn = false, unsigned int nHeightIn = 0) : txout(txoutIn), fCoinBase(fCoinBaseIn), nHeight(nHeightIn) { }
+    const Coin* txout;
 
+public:
     template<typename Stream>
     void Serialize(Stream &s) const {
-        ::Serialize(s, VARINT(nHeight*2+(fCoinBase ? 1 : 0)));
-        if (nHeight > 0) {
-            int nVersionDummy = 0;
-            ::Serialize(s, VARINT(nVersionDummy));
+        ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0)));
+        if (txout->nHeight > 0) {
+            // Required to maintain compatibility with older undo format.
+            ::Serialize(s, (unsigned char)0);
         }
-        ::Serialize(s, CTxOutCompressor(REF(txout)));
+        ::Serialize(s, CTxOutCompressor(REF(txout->out)));
     }
 
+    TxInUndoSerializer(const Coin* coin) : txout(coin) {}
+};
+
+class TxInUndoDeserializer
+{
+    Coin* txout;
+
+public:
     template<typename Stream>
     void Unserialize(Stream &s) {
         unsigned int nCode = 0;
         ::Unserialize(s, VARINT(nCode));
-        nHeight = nCode / 2;
-        fCoinBase = nCode & 1;
-        if (nHeight > 0) {
+        txout->nHeight = nCode / 2;
+        txout->fCoinBase = nCode & 1;
+        if (txout->nHeight > 0) {
+            // Old versions stored the version number for the last spend of
+            // a transaction's outputs. Non-final spends were indicated with
+            // height = 0.
             int nVersionDummy;
             ::Unserialize(s, VARINT(nVersionDummy));
         }
-        ::Unserialize(s, REF(CTxOutCompressor(REF(txout))));
+        ::Unserialize(s, REF(CTxOutCompressor(REF(txout->out))));
     }
+
+    TxInUndoDeserializer(Coin* coin) : txout(coin) {}
 };
 
+static const size_t MAX_INPUTS_PER_BLOCK = MAX_BLOCK_BASE_SIZE / ::GetSerializeSize(CTxIn(), SER_NETWORK, PROTOCOL_VERSION);
+
 /** Undo information for a CTransaction */
 class CTxUndo
 {
 public:
     // undo information for all txins
-    std::vector<CTxInUndo> vprevout;
+    std::vector<Coin> vprevout;
 
-    ADD_SERIALIZE_METHODS;
+    template <typename Stream>
+    void Serialize(Stream& s) const {
+        // TODO: avoid reimplementing vector serializer
+        uint64_t count = vprevout.size();
+        ::Serialize(s, COMPACTSIZE(REF(count)));
+        for (const auto& prevout : vprevout) {
+            ::Serialize(s, REF(TxInUndoSerializer(&prevout)));
+        }
+    }
 
-    template <typename Stream, typename Operation>
-    inline void SerializationOp(Stream& s, Operation ser_action) {
-        READWRITE(vprevout);
+    template <typename Stream>
+    void Unserialize(Stream& s) {
+        // TODO: avoid reimplementing vector deserializer
+        uint64_t count = 0;
+        ::Unserialize(s, COMPACTSIZE(count));
+        if (count > MAX_INPUTS_PER_BLOCK) {
+            throw std::ios_base::failure("Too many input undo records");
+        }
+        vprevout.resize(count);
+        for (auto& prevout : vprevout) {
+            ::Unserialize(s, REF(TxInUndoDeserializer(&prevout)));
+        }
     }
 };
 

src/validation.cpp

@@ -1080,11 +1080,9 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
             if (nPos >= coins->vout.size() || coins->vout[nPos].IsNull())
                 assert(false);
             // mark an outpoint spent, and construct undo information
-            txundo.vprevout.push_back(CTxInUndo(coins->vout[nPos]));
-            coins->Spend(nPos);
-            CTxInUndo& undo = txundo.vprevout.back();
-            undo.nHeight = coins->nHeight;
-            undo.fCoinBase = coins->fCoinBase;
+            txundo.vprevout.emplace_back(coins->vout[nPos], coins->nHeight, coins->fCoinBase);
+            bool ret = coins->Spend(nPos);
+            assert(ret);
         }
     }
     // add outputs
@@ -1252,13 +1250,13 @@ enum DisconnectResult
 };
 
 /**
- * Apply the undo operation of a CTxInUndo to the given chain state.
- * @param undo The undo object.
+ * Restore the UTXO in a Coin at a given COutPoint
+ * @param undo The Coin to be restored.
  * @param view The coins view to which to apply the changes.
  * @param out The out point that corresponds to the tx input.
  * @return A DisconnectResult as an int
  */
-int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out)
+int ApplyTxInUndo(const Coin& undo, CCoinsViewCache& view, const COutPoint& out)
 {
     bool fClean = true;
 
@@ -1279,7 +1277,7 @@ int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint&
     if (coins->IsAvailable(out.n)) fClean = false; // overwriting existing output
     if (coins->vout.size() < out.n+1)
         coins->vout.resize(out.n+1);
-    coins->vout[out.n] = undo.txout;
+    coins->vout[out.n] = undo.out;
 
     return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN;
 }
@@ -1335,7 +1333,7 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex*
             }
             for (unsigned int j = tx.vin.size(); j-- > 0;) {
                 const COutPoint &out = tx.vin[j].prevout;
-                const CTxInUndo &undo = txundo.vprevout[j];
+                const Coin &undo = txundo.vprevout[j];
                 int res = ApplyTxInUndo(undo, view, out);
                 if (res == DISCONNECT_FAILED) return DISCONNECT_FAILED;
                 fClean = fClean && res != DISCONNECT_UNCLEAN;