From d342424301013ec47dc146a4beb49d5c9319d80a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:18 -0700 Subject: [PATCH] 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. --- doc/release-notes.md | 9 +++++++++ src/coins.h | 16 +++++----------- src/rest.cpp | 6 ++---- src/rpc/blockchain.cpp | 13 +++++-------- src/test/coins_tests.cpp | 4 ---- src/test/transaction_tests.cpp | 1 - src/undo.h | 20 ++++++++++++-------- src/validation.cpp | 7 ------- test/functional/blockchain.py | 7 +++---- 9 files changed, 36 insertions(+), 47 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index af792118d..075c7c391 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -36,6 +36,15 @@ Notable changes Low-level RPC changes --------------------- +- The new database model no longer stores information about transaction + versions of unspent outputs. This means that: + - The `gettxout` RPC no longer has a `version` field in the response. + - The `gettxoutsetinfo` RPC reports `hash_serialized_2` instead of `hash_serialized`, + which does not commit to the transaction versions of unspent outputs, but does + commit to the height and coinbase information. + - The `getutxos` REST path no longer reports the `txvers` field in JSON format, + and always reports 0 for transaction versions in the binary format + - Error codes have been updated to be more accurate for the following error cases: - `getblock` now returns RPC_MISC_ERROR if the block can't be found on disk (for example if the block has been pruned). Previously returned RPC_INTERNAL_ERROR. diff --git a/src/coins.h b/src/coins.h index 065bae56e..12cfd6bf6 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().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 7537ed450..9c291fe0a 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 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 b4b160aac..d2f955fb3 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; iGetValueSize(); 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 1b5d8e91b..3735f6c86 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 67610301d..ef2e695ee 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::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 a94e31f5c..d21b82e2e 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 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 fc7e129c0..be0c9b564 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 diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py index b6112c728..b0faea9b3 100755 --- a/test/functional/blockchain.py +++ b/test/functional/blockchain.py @@ -49,10 +49,9 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['transactions'], 200) assert_equal(res['height'], 200) assert_equal(res['txouts'], 200) - assert_equal(res['bytes_serialized'], 13924), assert_equal(res['bestblock'], node.getblockhash(200)) assert_equal(len(res['bestblock']), 64) - assert_equal(len(res['hash_serialized']), 64) + assert_equal(len(res['hash_serialized_2']), 64) self.log.info("Test that gettxoutsetinfo() works for blockchain with just the genesis block") b1hash = node.getblockhash(1) @@ -64,7 +63,7 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res2['height'], 0) assert_equal(res2['txouts'], 0) assert_equal(res2['bestblock'], node.getblockhash(0)) - assert_equal(len(res2['hash_serialized']), 64) + assert_equal(len(res2['hash_serialized_2']), 64) self.log.info("Test that gettxoutsetinfo() returns the same result after invalidate/reconsider block") node.reconsiderblock(b1hash) @@ -75,7 +74,7 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['height'], res3['height']) assert_equal(res['txouts'], res3['txouts']) assert_equal(res['bestblock'], res3['bestblock']) - assert_equal(res['hash_serialized'], res3['hash_serialized']) + assert_equal(res['hash_serialized_2'], res3['hash_serialized_2']) def _test_getblockheader(self): node = self.nodes[0] -- 2.11.4.GIT