Remove/ignore tx version in utxo and undo

This makes the following changes:
* In undo data and the chainstate database, the transaction nVersion
  field is removed from the data structures, always written as 0, and
  ignored when reading.
* The definition of hash_serialized in gettxoutsetinfo is changed to no
  longer incude the nVersion field. It is renamed to hash_serialized_2
  to avoid confusion. The new definition also includes transaction
  height and coinbase information, as this information was missing
  before.

This depends on having a CHashVerifier-based undo data checksum
verifier.

Apart from changing the definition of serialized_hash, downgrading
after using this patch is supported, as no release ever used the value
of nVersion field in UTXO entries.

7 files changed, 24 insertions, 43 deletions

diff --git a/src/coins.h b/src/coins.h
index 065bae56e9..12cfd6bf6b 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -84,15 +84,10 @@ public:
     //! at which height this transaction was included in the active block chain
     int nHeight;
 
-    //! version of the CTransaction; accesses to this value should probably check for nHeight as well,
-    //! as new tx version will probably only be introduced at certain heights
-    int nVersion;
-
     void FromTx(const CTransaction &tx, int nHeightIn) {
         fCoinBase = tx.IsCoinBase();
         vout = tx.vout;
         nHeight = nHeightIn;
-        nVersion = tx.nVersion;
         ClearUnspendable();
     }
 
@@ -105,11 +100,10 @@ public:
         fCoinBase = false;
         std::vector<CTxOut>().swap(vout);
         nHeight = 0;
-        nVersion = 0;
     }
 
     //! empty constructor
-    CCoins() : fCoinBase(false), vout(0), nHeight(0), nVersion(0) { }
+    CCoins() : fCoinBase(false), vout(0), nHeight(0) { }
 
     //!remove spent outputs at the end of vout
     void Cleanup() {
@@ -131,7 +125,6 @@ public:
         std::swap(to.fCoinBase, fCoinBase);
         to.vout.swap(vout);
         std::swap(to.nHeight, nHeight);
-        std::swap(to.nVersion, nVersion);
     }
 
     //! equality test
@@ -141,7 +134,6 @@ public:
              return true;
          return a.fCoinBase == b.fCoinBase &&
                 a.nHeight == b.nHeight &&
-                a.nVersion == b.nVersion &&
                 a.vout == b.vout;
     }
     friend bool operator!=(const CCoins &a, const CCoins &b) {
@@ -163,7 +155,8 @@ public:
         assert(fFirst || fSecond || nMaskCode);
         unsigned int nCode = 8*(nMaskCode - (fFirst || fSecond ? 0 : 1)) + (fCoinBase ? 1 : 0) + (fFirst ? 2 : 0) + (fSecond ? 4 : 0);
         // version
-        ::Serialize(s, VARINT(this->nVersion));
+        int nVersionDummy = 0;
+        ::Serialize(s, VARINT(nVersionDummy));
         // header code
         ::Serialize(s, VARINT(nCode));
         // spentness bitmask
@@ -187,7 +180,8 @@ public:
     void Unserialize(Stream &s) {
         unsigned int nCode = 0;
         // version
-        ::Unserialize(s, VARINT(this->nVersion));
+        int nVersionDummy;
+        ::Unserialize(s, VARINT(nVersionDummy));
         // header code
         ::Unserialize(s, VARINT(nCode));
         fCoinBase = nCode & 1;
diff --git a/src/rest.cpp b/src/rest.cpp
index 7537ed4502..9c291fe0a9 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -42,7 +42,6 @@ static const struct {
 };
 
 struct CCoin {
-    uint32_t nTxVer; // Don't call this nVersion, that name has a special meaning inside IMPLEMENT_SERIALIZE
     uint32_t nHeight;
     CTxOut out;
 
@@ -51,7 +50,8 @@ struct CCoin {
     template <typename Stream, typename Operation>
     inline void SerializationOp(Stream& s, Operation ser_action)
     {
-        READWRITE(nTxVer);
+        uint32_t nTxVerDummy = 0;
+        READWRITE(nTxVerDummy);
         READWRITE(nHeight);
         READWRITE(out);
     }
@@ -519,7 +519,6 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
                     // Safe to index into vout here because IsAvailable checked if it's off the end of the array, or if
                     // n is valid but points to an already spent output (IsNull).
                     CCoin coin;
-                    coin.nTxVer = coins.nVersion;
                     coin.nHeight = coins.nHeight;
                     coin.out = coins.vout.at(vOutPoints[i].n);
                     assert(!coin.out.IsNull());
@@ -568,7 +567,6 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
         UniValue utxos(UniValue::VARR);
         BOOST_FOREACH (const CCoin& coin, outs) {
             UniValue utxo(UniValue::VOBJ);
-            utxo.push_back(Pair("txvers", (int32_t)coin.nTxVer));
             utxo.push_back(Pair("height", (int32_t)coin.nHeight));
             utxo.push_back(Pair("value", ValueFromAmount(coin.out.nValue)));
 
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index b4b160aac9..d2f955fb32 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -781,11 +781,10 @@ struct CCoinsStats
     uint256 hashBlock;
     uint64_t nTransactions;
     uint64_t nTransactionOutputs;
-    uint64_t nSerializedSize;
     uint256 hashSerialized;
     CAmount nTotalAmount;
 
-    CCoinsStats() : nHeight(0), nTransactions(0), nTransactionOutputs(0), nSerializedSize(0), nTotalAmount(0) {}
+    CCoinsStats() : nHeight(0), nTransactions(0), nTransactionOutputs(0), nTotalAmount(0) {}
 };
 
 //! Calculate statistics about the unspent transaction output set
@@ -808,16 +807,17 @@ static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats)
         if (pcursor->GetKey(key) && pcursor->GetValue(coins)) {
             stats.nTransactions++;
             ss << key;
+            ss << VARINT(coins.nHeight * 2 + coins.fCoinBase);
             for (unsigned int i=0; i<coins.vout.size(); i++) {
                 const CTxOut &out = coins.vout[i];
                 if (!out.IsNull()) {
                     stats.nTransactionOutputs++;
                     ss << VARINT(i+1);
-                    ss << out;
+                    ss << *(const CScriptBase*)(&out.scriptPubKey);
+                    ss << VARINT(out.nValue);
                     nTotalAmount += out.nValue;
                 }
             }
-            stats.nSerializedSize += 32 + pcursor->GetValueSize();
             ss << VARINT(0);
         } else {
             return error("%s: unable to read value", __func__);
@@ -891,7 +891,6 @@ UniValue gettxoutsetinfo(const JSONRPCRequest& request)
             "  \"bestblock\": \"hex\",   (string) the best block hash hex\n"
             "  \"transactions\": n,      (numeric) The number of transactions\n"
             "  \"txouts\": n,            (numeric) The number of output transactions\n"
-            "  \"bytes_serialized\": n,  (numeric) The serialized size\n"
             "  \"hash_serialized\": \"hash\",   (string) The serialized hash\n"
             "  \"total_amount\": x.xxx          (numeric) The total amount\n"
             "}\n"
@@ -909,8 +908,7 @@ UniValue gettxoutsetinfo(const JSONRPCRequest& request)
         ret.push_back(Pair("bestblock", stats.hashBlock.GetHex()));
         ret.push_back(Pair("transactions", (int64_t)stats.nTransactions));
         ret.push_back(Pair("txouts", (int64_t)stats.nTransactionOutputs));
-        ret.push_back(Pair("bytes_serialized", (int64_t)stats.nSerializedSize));
-        ret.push_back(Pair("hash_serialized", stats.hashSerialized.GetHex()));
+        ret.push_back(Pair("hash_serialized_2", stats.hashSerialized.GetHex()));
         ret.push_back(Pair("total_amount", ValueFromAmount(stats.nTotalAmount)));
     } else {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set");
@@ -992,7 +990,6 @@ UniValue gettxout(const JSONRPCRequest& request)
     UniValue o(UniValue::VOBJ);
     ScriptPubKeyToUniv(coins.vout[n].scriptPubKey, o, true);
     ret.push_back(Pair("scriptPubKey", o));
-    ret.push_back(Pair("version", coins.nVersion));
     ret.push_back(Pair("coinbase", coins.fCoinBase));
 
     return ret;
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 1b5d8e91b5..3735f6c868 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -142,7 +142,6 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test)
                 } else {
                     updated_an_entry = true;
                 }
-                coins.nVersion = insecure_rand();
                 coins.vout.resize(1);
                 coins.vout[0].nValue = insecure_rand();
                 *entry = coins;
@@ -436,7 +435,6 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization)
     CDataStream ss1(ParseHex("0104835800816115944e077fe7c803cfa57f29b36bf87c1d358bb85e"), SER_DISK, CLIENT_VERSION);
     CCoins cc1;
     ss1 >> cc1;
-    BOOST_CHECK_EQUAL(cc1.nVersion, 1);
     BOOST_CHECK_EQUAL(cc1.fCoinBase, false);
     BOOST_CHECK_EQUAL(cc1.nHeight, 203998);
     BOOST_CHECK_EQUAL(cc1.vout.size(), 2);
@@ -449,7 +447,6 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization)
     CDataStream ss2(ParseHex("0109044086ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4eebbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa486af3b"), SER_DISK, CLIENT_VERSION);
     CCoins cc2;
     ss2 >> cc2;
-    BOOST_CHECK_EQUAL(cc2.nVersion, 1);
     BOOST_CHECK_EQUAL(cc2.fCoinBase, true);
     BOOST_CHECK_EQUAL(cc2.nHeight, 120891);
     BOOST_CHECK_EQUAL(cc2.vout.size(), 17);
@@ -468,7 +465,6 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization)
     CDataStream ss3(ParseHex("0002000600"), SER_DISK, CLIENT_VERSION);
     CCoins cc3;
     ss3 >> cc3;
-    BOOST_CHECK_EQUAL(cc3.nVersion, 0);
     BOOST_CHECK_EQUAL(cc3.fCoinBase, false);
     BOOST_CHECK_EQUAL(cc3.nHeight, 0);
     BOOST_CHECK_EQUAL(cc3.vout.size(), 1);
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 67610301d7..ef2e695ee3 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -471,7 +471,6 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) {
         threadGroup.create_thread(boost::bind(&CCheckQueue<CScriptCheck>::Thread, boost::ref(scriptcheckqueue)));
 
     CCoins coins;
-    coins.nVersion = 1;
     coins.fCoinBase = false;
     for(uint32_t i = 0; i < mtx.vin.size(); i++) {
         CTxOut txout;
diff --git a/src/undo.h b/src/undo.h
index a94e31f5cc..d21b82e2ec 100644
--- a/src/undo.h
+++ b/src/undo.h
@@ -14,7 +14,8 @@
  *
  *  Contains the prevout's CTxOut being spent, and if this was the
  *  last output of the affected transaction, its metadata as well
- *  (coinbase or not, height, transaction version)
+ *  (coinbase or not, height). Earlier versions also stored the transaction
+ *  version.
  */
 class CTxInUndo
 {
@@ -22,16 +23,17 @@ 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
-    int nVersion;         // if the outpoint was the last unspent: its version
 
-    CTxInUndo() : txout(), fCoinBase(false), nHeight(0), nVersion(0) {}
-    CTxInUndo(const CTxOut &txoutIn, bool fCoinBaseIn = false, unsigned int nHeightIn = 0, int nVersionIn = 0) : txout(txoutIn), fCoinBase(fCoinBaseIn), nHeight(nHeightIn), nVersion(nVersionIn) { }
+    CTxInUndo() : txout(), fCoinBase(false), nHeight(0) {}
+    CTxInUndo(const CTxOut &txoutIn, bool fCoinBaseIn = false, unsigned int nHeightIn = 0) : txout(txoutIn), fCoinBase(fCoinBaseIn), nHeight(nHeightIn) { }
 
     template<typename Stream>
     void Serialize(Stream &s) const {
         ::Serialize(s, VARINT(nHeight*2+(fCoinBase ? 1 : 0)));
-        if (nHeight > 0)
-            ::Serialize(s, VARINT(this->nVersion));
+        if (nHeight > 0) {
+            int nVersionDummy = 0;
+            ::Serialize(s, VARINT(nVersionDummy));
+        }
         ::Serialize(s, CTxOutCompressor(REF(txout)));
     }
 
@@ -41,8 +43,10 @@ public:
         ::Unserialize(s, VARINT(nCode));
         nHeight = nCode / 2;
         fCoinBase = nCode & 1;
-        if (nHeight > 0)
-            ::Unserialize(s, VARINT(this->nVersion));
+        if (nHeight > 0) {
+            int nVersionDummy;
+            ::Unserialize(s, VARINT(nVersionDummy));
+        }
         ::Unserialize(s, REF(CTxOutCompressor(REF(txout))));
     }
 };
diff --git a/src/validation.cpp b/src/validation.cpp
index fc7e129c05..be0c9b564e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1086,7 +1086,6 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
                 CTxInUndo& undo = txundo.vprevout.back();
                 undo.nHeight = coins->nHeight;
                 undo.fCoinBase = coins->fCoinBase;
-                undo.nVersion = coins->nVersion;
             }
         }
     }
@@ -1271,7 +1270,6 @@ int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint&
         if (!coins->IsPruned()) fClean = false; // overwriting existing transaction
         coins->fCoinBase = undo.fCoinBase;
         coins->nHeight = undo.nHeight;
-        coins->nVersion = undo.nVersion;
     } else {
         if (coins->IsPruned()) fClean = false; // adding output to missing transaction
     }
@@ -1319,11 +1317,6 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex*
         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.
-        if (outsBlock.nVersion < 0)
-            outs->nVersion = outsBlock.nVersion;
         if (*outs != outsBlock) fClean = false; // transaction mismatch
 
         // remove outputs