From e66dbde6d14cb5f253b3bf8850a98f4fda2d9f49 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:04 -0700 Subject: [PATCH 01/29] Add SizeEstimate to CDBBatch This allows estimating the in-memory size of a LevelDB batch. --- src/dbwrapper.h | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/dbwrapper.h b/src/dbwrapper.h index b13e98b7a..442f37b42 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -55,11 +55,19 @@ private: CDataStream ssKey; CDataStream ssValue; + size_t size_estimate; + public: /** * @param[in] _parent CDBWrapper that this batch is to be submitted to */ - CDBBatch(const CDBWrapper &_parent) : parent(_parent), ssKey(SER_DISK, CLIENT_VERSION), ssValue(SER_DISK, CLIENT_VERSION) { }; + CDBBatch(const CDBWrapper &_parent) : parent(_parent), ssKey(SER_DISK, CLIENT_VERSION), ssValue(SER_DISK, CLIENT_VERSION), size_estimate(0) { }; + + void Clear() + { + batch.Clear(); + size_estimate = 0; + } template void Write(const K& key, const V& value) @@ -74,6 +82,14 @@ public: leveldb::Slice slValue(ssValue.data(), ssValue.size()); batch.Put(slKey, slValue); + // LevelDB serializes writes as: + // - byte: header + // - varint: key length (1 byte up to 127B, 2 bytes up to 16383B, ...) + // - byte[]: key + // - varint: value length + // - byte[]: value + // The formula below assumes the key and value are both less than 16k. + size_estimate += 3 + (slKey.size() > 127) + slKey.size() + (slValue.size() > 127) + slValue.size(); ssKey.clear(); ssValue.clear(); } @@ -86,8 +102,16 @@ public: leveldb::Slice slKey(ssKey.data(), ssKey.size()); batch.Delete(slKey); + // LevelDB serializes erases as: + // - byte: header + // - varint: key length + // - byte[]: key + // The formula below assumes the key is less than 16kB. + size_estimate += 2 + (slKey.size() > 127) + slKey.size(); ssKey.clear(); } + + size_t SizeEstimate() const { return size_estimate; } }; class CDBIterator From f54580e7e4f225bb615204daef32f72ab8688418 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:09 -0700 Subject: [PATCH 02/29] error() in disconnect for disk corruption, not inconsistency The error() function unconditionally reports an error. It should only be used for actually exception situations, and not for the type of inconsistencies that ApplyTxInUndo/DisconnectBlock can graciously deal with. This also makes a subtle semantics change: in ApplyTxInUndo, when a record with metadata is encountered (indicating it is the last spend from a tx), don't wipe the CCoins record if it wasn't empty at that point. This makes sure that UTXO operations never affect any other UTXOs (including those from the same tx). --- src/test/coins_tests.cpp | 2 +- src/validation.cpp | 70 +++++++++++++++++++--------------------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 31ed1a50b..1b5d8e91b 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -17,7 +17,7 @@ #include -bool ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out); +int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out); void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txundo, int nHeight); namespace diff --git a/src/validation.cpp b/src/validation.cpp index ac16af3ee..ed94be5c2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1248,39 +1248,6 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std } // anon namespace -/** - * Apply the undo operation of a CTxInUndo to the given chain state. - * @param undo The undo object. - * @param view The coins view to which to apply the changes. - * @param out The out point that corresponds to the tx input. - * @return True on success. - */ -bool ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out) -{ - bool fClean = true; - - CCoinsModifier coins = view.ModifyCoins(out.hash); - if (undo.nHeight != 0) { - // undo data contains height: this is the last output of the prevout tx being spent - if (!coins->IsPruned()) - fClean = fClean && error("%s: undo data overwriting existing transaction", __func__); - coins->Clear(); - coins->fCoinBase = undo.fCoinBase; - coins->nHeight = undo.nHeight; - coins->nVersion = undo.nVersion; - } else { - if (coins->IsPruned()) - fClean = fClean && error("%s: undo data adding output to missing transaction", __func__); - } - if (coins->IsAvailable(out.n)) - fClean = fClean && error("%s: undo data overwriting existing output", __func__); - if (coins->vout.size() < out.n+1) - coins->vout.resize(out.n+1); - coins->vout[out.n] = undo.txout; - - return fClean; -} - enum DisconnectResult { DISCONNECT_OK, // All good. @@ -1288,6 +1255,35 @@ enum DisconnectResult DISCONNECT_FAILED // Something else went wrong. }; +/** + * Apply the undo operation of a CTxInUndo to the given chain state. + * @param undo The undo object. + * @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) +{ + bool fClean = true; + + CCoinsModifier coins = view.ModifyCoins(out.hash); + if (undo.nHeight != 0) { + // undo data contains height: this is the last output of the prevout tx being spent + 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 + } + 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; + + return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; +} + /** Undo the effects of this block (with given index) on the UTXO set represented by coins. * When UNCLEAN or FAILED is returned, view is left in an indeterminate state. */ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view) @@ -1329,8 +1325,7 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* // 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 = fClean && error("DisconnectBlock(): added transaction mismatch? database corrupted"); + if (*outs != outsBlock) fClean = false; // transaction mismatch // remove outputs outs->Clear(); @@ -1346,8 +1341,9 @@ 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]; - if (!ApplyTxInUndo(undo, view, out)) - fClean = false; + int res = ApplyTxInUndo(undo, view, out); + if (res == DISCONNECT_FAILED) return DISCONNECT_FAILED; + fClean = fClean && res != DISCONNECT_UNCLEAN; } } } From e484652fc36ef7135cf08ad380ea7142b6cbadc0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:14 -0700 Subject: [PATCH 03/29] Introduce CHashVerifier to hash read data This is necessary later, when we drop the nVersion field from the undo data. At that point deserializing and reserializing the data won't roundtrip anymore, and thus that approach can't be used to verify checksums anymore. With this CHashVerifier approach, we can deserialize while hashing the exact serialized form that was used. This is both more efficient and more correct in that case. --- src/hash.h | 35 +++++++++++++++++++++++++++++++++++ src/validation.cpp | 9 ++++----- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/hash.h b/src/hash.h index eacb8f04f..b8de19c0f 100644 --- a/src/hash.h +++ b/src/hash.h @@ -160,6 +160,41 @@ public: } }; +/** Reads data from an underlying stream, while hashing the read data. */ +template +class CHashVerifier : public CHashWriter +{ +private: + Source* source; + +public: + CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {} + + void read(char* pch, size_t nSize) + { + source->read(pch, nSize); + this->write(pch, nSize); + } + + void ignore(size_t nSize) + { + char data[1024]; + while (nSize > 0) { + size_t now = std::min(nSize, 1024); + read(data, now); + nSize -= now; + } + } + + template + CHashVerifier& operator>>(T& obj) + { + // Unserialize from this stream + ::Unserialize(*this, obj); + return (*this); + } +}; + /** Compute the 256-bit hash of an object's serialization. */ template uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL_VERSION) diff --git a/src/validation.cpp b/src/validation.cpp index ed94be5c2..fc7e129c0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1210,8 +1210,10 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CDiskBlockPos& pos, const uin // Read block uint256 hashChecksum; + CHashVerifier verifier(&filein); // We need a CHashVerifier as reserializing may lose data try { - filein >> blockundo; + verifier << hashBlock; + verifier >> blockundo; filein >> hashChecksum; } catch (const std::exception& e) { @@ -1219,10 +1221,7 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CDiskBlockPos& pos, const uin } // Verify checksum - CHashWriter hasher(SER_GETHASH, PROTOCOL_VERSION); - hasher << hashBlock; - hasher << blockundo; - if (hashChecksum != hasher.GetHash()) + if (hashChecksum != verifier.GetHash()) return error("%s: Checksum mismatch", __func__); return true; From 7e0032290669fae5f52c256856c53038511c7db4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:16 -0700 Subject: [PATCH 04/29] Add specialization of SipHash for 256 + 32 bit data We'll need a version of SipHash for tuples of 256 bits and 32 bits data, when CCoinsViewCache switches from using txids to COutPoints as keys. --- src/hash.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ src/hash.h | 1 + src/test/hash_tests.cpp | 17 +++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/hash.cpp b/src/hash.cpp index a14a2386a..b361c90d1 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -208,3 +208,44 @@ uint64_t SipHashUint256(uint64_t k0, uint64_t k1, const uint256& val) SIPROUND; return v0 ^ v1 ^ v2 ^ v3; } + +uint64_t SipHashUint256Extra(uint64_t k0, uint64_t k1, const uint256& val, uint32_t extra) +{ + /* Specialized implementation for efficiency */ + uint64_t d = val.GetUint64(0); + + uint64_t v0 = 0x736f6d6570736575ULL ^ k0; + uint64_t v1 = 0x646f72616e646f6dULL ^ k1; + uint64_t v2 = 0x6c7967656e657261ULL ^ k0; + uint64_t v3 = 0x7465646279746573ULL ^ k1 ^ d; + + SIPROUND; + SIPROUND; + v0 ^= d; + d = val.GetUint64(1); + v3 ^= d; + SIPROUND; + SIPROUND; + v0 ^= d; + d = val.GetUint64(2); + v3 ^= d; + SIPROUND; + SIPROUND; + v0 ^= d; + d = val.GetUint64(3); + v3 ^= d; + SIPROUND; + SIPROUND; + v0 ^= d; + d = (((uint64_t)36) << 56) | extra; + v3 ^= d; + SIPROUND; + SIPROUND; + v0 ^= d; + v2 ^= 0xFF; + SIPROUND; + SIPROUND; + SIPROUND; + SIPROUND; + return v0 ^ v1 ^ v2 ^ v3; +} diff --git a/src/hash.h b/src/hash.h index b8de19c0f..b9952d39f 100644 --- a/src/hash.h +++ b/src/hash.h @@ -241,5 +241,6 @@ public: * .Finalize() */ uint64_t SipHashUint256(uint64_t k0, uint64_t k1, const uint256& val); +uint64_t SipHashUint256Extra(uint64_t k0, uint64_t k1, const uint256& val, uint32_t extra); #endif // BITCOIN_HASH_H diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp index d8de765db..bb7e47324 100644 --- a/src/test/hash_tests.cpp +++ b/src/test/hash_tests.cpp @@ -128,6 +128,23 @@ BOOST_AUTO_TEST_CASE(siphash) tx.nVersion = 1; ss << tx; BOOST_CHECK_EQUAL(SipHashUint256(1, 2, ss.GetHash()), 0x79751e980c2a0a35ULL); + + // Check consistency between CSipHasher and SipHashUint256[Extra]. + FastRandomContext ctx; + for (int i = 0; i < 16; ++i) { + uint64_t k1 = ctx.rand64(); + uint64_t k2 = ctx.rand64(); + uint256 x = GetRandHash(); + uint32_t n = ctx.rand32(); + uint8_t nb[4]; + WriteLE32(nb, n); + CSipHasher sip256(k1, k2); + sip256.Write(x.begin(), 32); + CSipHasher sip288 = sip256; + sip288.Write(nb, 4); + BOOST_CHECK_EQUAL(SipHashUint256(k1, k2, x), sip256.Finalize()); + BOOST_CHECK_EQUAL(SipHashUint256Extra(k1, k2, x, n), sip288.Finalize()); + } } BOOST_AUTO_TEST_SUITE_END() From d342424301013ec47dc146a4beb49d5c9319d80a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:18 -0700 Subject: [PATCH 05/29] 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] From c3aa0c11947dfd82702df276d39bb7f748dd83a1 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 12 May 2017 15:19:19 -0700 Subject: [PATCH 06/29] Report on-disk size in gettxoutsetinfo --- src/coins.cpp | 1 + src/coins.h | 4 ++++ src/dbwrapper.h | 17 ++++++++++++++++- src/rpc/blockchain.cpp | 4 ++++ src/txdb.cpp | 5 +++++ src/txdb.h | 2 ++ test/functional/blockchain.py | 3 +++ 7 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/coins.cpp b/src/coins.cpp index b2e33abf3..02f424fad 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -55,6 +55,7 @@ uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); } CCoinsViewCursor *CCoinsViewBacked::Cursor() const { return base->Cursor(); } +size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} diff --git a/src/coins.h b/src/coins.h index 12cfd6bf6..47ef90114 100644 --- a/src/coins.h +++ b/src/coins.h @@ -319,6 +319,9 @@ public: //! As we use CCoinsViews polymorphically, have a virtual destructor virtual ~CCoinsView() {} + + //! Estimate database size (0 if not implemented) + virtual size_t EstimateSize() const { return 0; } }; @@ -336,6 +339,7 @@ public: void SetBackend(CCoinsView &viewIn); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); CCoinsViewCursor *Cursor() const; + size_t EstimateSize() const override; }; diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 442f37b42..24ef71bfb 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -305,7 +305,22 @@ public: * Return true if the database managed by this class contains no entries. */ bool IsEmpty(); + + template + size_t EstimateSize(const K& key_begin, const K& key_end) const + { + CDataStream ssKey1(SER_DISK, CLIENT_VERSION), ssKey2(SER_DISK, CLIENT_VERSION); + ssKey1.reserve(DBWRAPPER_PREALLOC_KEY_SIZE); + ssKey2.reserve(DBWRAPPER_PREALLOC_KEY_SIZE); + ssKey1 << key_begin; + ssKey2 << key_end; + leveldb::Slice slKey1(ssKey1.data(), ssKey1.size()); + leveldb::Slice slKey2(ssKey2.data(), ssKey2.size()); + uint64_t size = 0; + leveldb::Range range(slKey1, slKey2); + pdb->GetApproximateSizes(&range, 1, &size); + return size; + } }; #endif // BITCOIN_DBWRAPPER_H - diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d2f955fb3..e59b9b86b 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -782,6 +782,7 @@ struct CCoinsStats uint64_t nTransactions; uint64_t nTransactionOutputs; uint256 hashSerialized; + uint64_t nDiskSize; CAmount nTotalAmount; CCoinsStats() : nHeight(0), nTransactions(0), nTransactionOutputs(0), nTotalAmount(0) {} @@ -826,6 +827,7 @@ static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) } stats.hashSerialized = ss.GetHash(); stats.nTotalAmount = nTotalAmount; + stats.nDiskSize = view->EstimateSize(); return true; } @@ -892,6 +894,7 @@ UniValue gettxoutsetinfo(const JSONRPCRequest& request) " \"transactions\": n, (numeric) The number of transactions\n" " \"txouts\": n, (numeric) The number of output transactions\n" " \"hash_serialized\": \"hash\", (string) The serialized hash\n" + " \"disk_size\": n, (numeric) The estimated size of the chainstate on disk\n" " \"total_amount\": x.xxx (numeric) The total amount\n" "}\n" "\nExamples:\n" @@ -909,6 +912,7 @@ UniValue gettxoutsetinfo(const JSONRPCRequest& request) ret.push_back(Pair("transactions", (int64_t)stats.nTransactions)); ret.push_back(Pair("txouts", (int64_t)stats.nTransactionOutputs)); ret.push_back(Pair("hash_serialized_2", stats.hashSerialized.GetHex())); + ret.push_back(Pair("disk_size", stats.nDiskSize)); ret.push_back(Pair("total_amount", ValueFromAmount(stats.nTotalAmount))); } else { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); diff --git a/src/txdb.cpp b/src/txdb.cpp index 76aab2398..f139384a2 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -67,6 +67,11 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return db.WriteBatch(batch); } +size_t CCoinsViewDB::EstimateSize() const +{ + return db.EstimateSize(DB_COINS, (char)(DB_COINS+1)); +} + CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(GetDataDir() / "blocks" / "index", nCacheSize, fMemory, fWipe) { } diff --git a/src/txdb.h b/src/txdb.h index 117e7201f..df164cb82 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -78,6 +78,8 @@ public: uint256 GetBestBlock() const; bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); CCoinsViewCursor *Cursor() const; + + size_t EstimateSize() const override; }; /** Specialization of CCoinsViewCursor to iterate over a CCoinsViewDB */ diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py index b0faea9b3..4bfd3ee67 100755 --- a/test/functional/blockchain.py +++ b/test/functional/blockchain.py @@ -50,6 +50,9 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['height'], 200) assert_equal(res['txouts'], 200) assert_equal(res['bestblock'], node.getblockhash(200)) + size = res['disk_size'] + assert size > 6400 + assert size < 64000 assert_equal(len(res['bestblock']), 64) assert_equal(len(res['hash_serialized_2']), 64) From 7d991b55dbf0b0f6e21c0680ee3ebd09df09012f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:19 -0700 Subject: [PATCH 07/29] Store/allow tx metadata in all undo records Previously, transaction metadata (height, coinbase or not, and before the previous commit also nVersion) was only stored for undo records that correspond to the last output of a transaction being spent. This only saves 2 bytes per undo record. Change this to storing this information for every undo record, and stop complaining for having it in non-last output spends. This means that undo dat written with this patch won't be readable by older versions anymore. --- src/undo.h | 3 +-- src/validation.cpp | 17 ++++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/undo.h b/src/undo.h index d21b82e2e..37a1c89f4 100644 --- a/src/undo.h +++ b/src/undo.h @@ -12,8 +12,7 @@ /** Undo information for a CTxIn * - * Contains the prevout's CTxOut being spent, and if this was the - * last output of the affected transaction, its metadata as well + * Contains the prevout's CTxOut being spent, and its metadata as well * (coinbase or not, height). Earlier versions also stored the transaction * version. */ diff --git a/src/validation.cpp b/src/validation.cpp index be0c9b564..fccef1719 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1082,11 +1082,9 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund // mark an outpoint spent, and construct undo information txundo.vprevout.push_back(CTxInUndo(coins->vout[nPos])); coins->Spend(nPos); - if (coins->vout.size() == 0) { - CTxInUndo& undo = txundo.vprevout.back(); - undo.nHeight = coins->nHeight; - undo.fCoinBase = coins->fCoinBase; - } + CTxInUndo& undo = txundo.vprevout.back(); + undo.nHeight = coins->nHeight; + undo.fCoinBase = coins->fCoinBase; } } // add outputs @@ -1266,11 +1264,16 @@ int ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& CCoinsModifier coins = view.ModifyCoins(out.hash); if (undo.nHeight != 0) { - // undo data contains height: this is the last output of the prevout tx being spent - if (!coins->IsPruned()) fClean = false; // overwriting existing transaction + if (!coins->IsPruned()) { + if (coins->fCoinBase != undo.fCoinBase || (uint32_t)coins->nHeight != undo.nHeight) fClean = false; // metadata mismatch + } + // restore height/coinbase tx metadata from undo data coins->fCoinBase = undo.fCoinBase; coins->nHeight = undo.nHeight; } else { + // Undo data does not contain height/coinbase. This should never happen + // for newly created undo entries. Previously, this data was only saved + // for the last spend of a transaction's outputs, so check IsPruned(). if (coins->IsPruned()) fClean = false; // adding output to missing transaction } if (coins->IsAvailable(out.n)) fClean = false; // overwriting existing output From 422634e2f5ac1ff74cd358144cecbac63007adc4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:23 -0700 Subject: [PATCH 08/29] Introduce Coin, a single unspent output --- src/coins.h | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/coins.h b/src/coins.h index 47ef90114..69c24ab45 100644 --- a/src/coins.h +++ b/src/coins.h @@ -20,6 +20,68 @@ #include #include +/** + * A UTXO entry. + * + * Serialized format: + * - VARINT((coinbase ? 1 : 0) | (height << 1)) + * - the non-spent CTxOut (via CTxOutCompressor) + */ +class Coin +{ +public: + //! whether the containing transaction was a coinbase + bool fCoinBase; + + //! unspent transaction output + CTxOut out; + + //! at which height the containing transaction was included in the active block chain + uint32_t nHeight; + + //! construct a Coin from a CTxOut and height/coinbase properties. + Coin(CTxOut&& outIn, int nHeightIn, bool fCoinBaseIn) : fCoinBase(fCoinBaseIn), out(std::move(outIn)), nHeight(nHeightIn) {} + Coin(const CTxOut& outIn, int nHeightIn, bool fCoinBaseIn) : fCoinBase(fCoinBaseIn), out(outIn), nHeight(nHeightIn) {} + + void Clear() { + out.SetNull(); + fCoinBase = false; + nHeight = 0; + } + + //! empty constructor + Coin() : fCoinBase(false), nHeight(0) { } + + bool IsCoinBase() const { + return fCoinBase; + } + + template + void Serialize(Stream &s) const { + assert(!IsPruned()); + uint32_t code = nHeight * 2 + fCoinBase; + ::Serialize(s, VARINT(code)); + ::Serialize(s, CTxOutCompressor(REF(out))); + } + + template + void Unserialize(Stream &s) { + uint32_t code = 0; + ::Unserialize(s, VARINT(code)); + nHeight = code >> 1; + fCoinBase = code & 1; + ::Unserialize(s, REF(CTxOutCompressor(out))); + } + + bool IsPruned() const { + return out.IsNull(); + } + + size_t DynamicMemoryUsage() const { + return memusage::DynamicUsage(out.scriptPubKey); + } +}; + /** * Pruned version of CTransaction: only retains metadata and unspent transaction outputs * From cb2c7fdac2dc74368ed24ae4717ed72178956b92 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:25 -0700 Subject: [PATCH 09/29] 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. --- src/test/coins_tests.cpp | 4 +-- src/undo.h | 77 +++++++++++++++++++++++++++------------- src/validation.cpp | 18 +++++----- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 3735f6c86..e86c71994 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -17,7 +17,7 @@ #include -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 diff --git a/src/undo.h b/src/undo.h index 37a1c89f4..3749d5d7a 100644 --- a/src/undo.h +++ b/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 { + const Coin* txout; + 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) { } - template 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 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 vprevout; + std::vector vprevout; - ADD_SERIALIZE_METHODS; + template + 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 - inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(vprevout); + template + 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))); + } } }; diff --git a/src/validation.cpp b/src/validation.cpp index fccef1719..dad0e1a3e 100644 --- a/src/validation.cpp +++ b/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; From bd83111a0fcfdb97204a0180bcf861d3b53bb6c2 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:27 -0700 Subject: [PATCH 10/29] Optimization: Coin&& to ApplyTxInUndo This avoids a prevector copy in ApplyTxInUndo. --- src/test/coins_tests.cpp | 6 +++--- src/validation.cpp | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index e86c71994..9c758ee9a 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -17,7 +17,7 @@ #include -int ApplyTxInUndo(const Coin& undo, CCoinsViewCache& view, const COutPoint& out); +int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out); void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txundo, int nHeight); namespace @@ -371,8 +371,8 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // restore inputs if (!tx.IsCoinBase()) { const COutPoint &out = tx.vin[0].prevout; - const Coin &undoin = undo.vprevout[0]; - ApplyTxInUndo(undoin, *(stack.back()), out); + Coin coin = undo.vprevout[0]; + ApplyTxInUndo(std::move(coin), *(stack.back()), out); } // Store as a candidate for reconnection disconnectedids.insert(undohash); diff --git a/src/validation.cpp b/src/validation.cpp index dad0e1a3e..d6cc59b48 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1256,7 +1256,7 @@ enum DisconnectResult * @param out The out point that corresponds to the tx input. * @return A DisconnectResult as an int */ -int ApplyTxInUndo(const Coin& undo, CCoinsViewCache& view, const COutPoint& out) +int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out) { bool fClean = true; @@ -1277,7 +1277,7 @@ int ApplyTxInUndo(const Coin& undo, CCoinsViewCache& view, const COutPoint& out) 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.out; + coins->vout[out.n] = std::move(undo.out); return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } @@ -1326,18 +1326,18 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* // restore inputs if (i > 0) { // not coinbases - const CTxUndo &txundo = blockUndo.vtxundo[i-1]; + CTxUndo &txundo = blockUndo.vtxundo[i-1]; if (txundo.vprevout.size() != tx.vin.size()) { error("DisconnectBlock(): transaction and undo data inconsistent"); return DISCONNECT_FAILED; } for (unsigned int j = tx.vin.size(); j-- > 0;) { const COutPoint &out = tx.vin[j].prevout; - const Coin &undo = txundo.vprevout[j]; - int res = ApplyTxInUndo(undo, view, out); + int res = ApplyTxInUndo(std::move(txundo.vprevout[j]), view, out); if (res == DISCONNECT_FAILED) return DISCONNECT_FAILED; fClean = fClean && res != DISCONNECT_UNCLEAN; } + // At this point, all of txundo.vprevout should have been moved out. } } From 000391132608343c66488d62625c714814959bc9 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:29 -0700 Subject: [PATCH 11/29] Introduce new per-txout CCoinsViewCache functions The new functions are: * CCoinsViewCache::AddCoin: Add a single COutPoint/Coin pair. * CCoinsViewCache::SpendCoin: Remove a single COutPoint. * AddCoins: utility function that invokes CCoinsViewCache::AddCoin for each output in a CTransaction. * AccessByTxid: utility function that searches for any output with a given txid. * CCoinsViewCache::AccessCoin: retrieve the Coin for a COutPoint. * CCoinsViewCache::HaveCoins: check whether a non-empty Coin exists for a given COutPoint. The AddCoin and SpendCoin methods will eventually replace ModifyCoins and ModifyNewCoins, AddCoins will replace CCoins::FromTx, and the new AccessCoins and HaveCoins functions will replace their per-txid counterparts. Note that AccessCoin for now returns a copy of the Coin object. In a later commit it will be change to returning a const reference (which keeps working in all call sites). --- src/coins.cpp | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++- src/coins.h | 33 +++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 02f424fad..3ac46d080 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -4,6 +4,7 @@ #include "coins.h" +#include "consensus/consensus.h" #include "memusage.h" #include "random.h" @@ -70,7 +71,7 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; } -CCoinsMap::const_iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { +CCoinsMap::iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { CCoinsMap::iterator it = cacheCoins.find(txid); if (it != cacheCoins.end()) return it; @@ -153,6 +154,58 @@ CCoinsModifier CCoinsViewCache::ModifyNewCoins(const uint256 &txid, bool coinbas return CCoinsModifier(*this, ret.first, 0); } +void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possible_overwrite) { + assert(!coin.IsPruned()); + if (coin.out.scriptPubKey.IsUnspendable()) return; + CCoinsMap::iterator it; + bool inserted; + std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint.hash), std::tuple<>()); + bool fresh = false; + if (!inserted) { + cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + } + if (!possible_overwrite) { + if (it->second.coins.IsAvailable(outpoint.n)) { + throw std::logic_error("Adding new coin that replaces non-pruned entry"); + } + fresh = it->second.coins.IsPruned() && !(it->second.flags & CCoinsCacheEntry::DIRTY); + } + if (it->second.coins.vout.size() <= outpoint.n) { + it->second.coins.vout.resize(outpoint.n + 1); + } + it->second.coins.vout[outpoint.n] = std::move(coin.out); + it->second.coins.nHeight = coin.nHeight; + it->second.coins.fCoinBase = coin.fCoinBase; + it->second.flags |= CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0); + cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); +} + +void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { + bool fCoinbase = tx.IsCoinBase(); + const uint256& txid = tx.GetHash(); + for (size_t i = 0; i < tx.vout.size(); ++i) { + // Pass fCoinbase as the possible_overwrite flag to AddCoin, in order to correctly + // deal with the pre-BIP30 occurrances of duplicate coinbase transactions. + cache.AddCoin(COutPoint(txid, i), Coin(tx.vout[i], nHeight, fCoinbase), fCoinbase); + } +} + +void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { + CCoinsMap::iterator it = FetchCoins(outpoint.hash); + if (it == cacheCoins.end()) return; + cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + if (moveout && it->second.coins.IsAvailable(outpoint.n)) { + *moveout = Coin(it->second.coins.vout[outpoint.n], it->second.coins.nHeight, it->second.coins.fCoinBase); + } + it->second.coins.Spend(outpoint.n); // Ignore return value: SpendCoin has no effect if no UTXO found. + if (it->second.coins.IsPruned() && it->second.flags & CCoinsCacheEntry::FRESH) { + cacheCoins.erase(it); + } else { + cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); + it->second.flags |= CCoinsCacheEntry::DIRTY; + } +} + const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { CCoinsMap::const_iterator it = FetchCoins(txid); if (it == cacheCoins.end()) { @@ -162,6 +215,18 @@ const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { } } +static const Coin coinEmpty; + +const Coin CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { + CCoinsMap::const_iterator it = FetchCoins(outpoint.hash); + if (it == cacheCoins.end() || !it->second.coins.IsAvailable(outpoint.n)) { + return coinEmpty; + } else { + return Coin(it->second.coins.vout[outpoint.n], it->second.coins.nHeight, it->second.coins.fCoinBase); + } +} + + bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { CCoinsMap::const_iterator it = FetchCoins(txid); // We're using vtx.empty() instead of IsPruned here for performance reasons, @@ -171,6 +236,11 @@ bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { return (it != cacheCoins.end() && !it->second.coins.vout.empty()); } +bool CCoinsViewCache::HaveCoins(const COutPoint &outpoint) const { + CCoinsMap::const_iterator it = FetchCoins(outpoint.hash); + return (it != cacheCoins.end() && it->second.coins.IsAvailable(outpoint.n)); +} + bool CCoinsViewCache::HaveCoinsInCache(const uint256 &txid) const { CCoinsMap::const_iterator it = cacheCoins.find(txid); return it != cacheCoins.end(); @@ -318,3 +388,16 @@ CCoinsModifier::~CCoinsModifier() CCoinsViewCursor::~CCoinsViewCursor() { } + +static const size_t MAX_OUTPUTS_PER_BLOCK = MAX_BLOCK_BASE_SIZE / ::GetSerializeSize(CTxOut(), SER_NETWORK, PROTOCOL_VERSION); // TODO: merge with similar definition in undo.h. + +const Coin AccessByTxid(const CCoinsViewCache& view, const uint256& txid) +{ + COutPoint iter(txid, 0); + while (iter.n < MAX_OUTPUTS_PER_BLOCK) { + const Coin& alternate = view.AccessCoin(iter); + if (!alternate.IsPruned()) return alternate; + ++iter.n; + } + return coinEmpty; +} diff --git a/src/coins.h b/src/coins.h index 69c24ab45..5879530f9 100644 --- a/src/coins.h +++ b/src/coins.h @@ -452,6 +452,7 @@ public: // Standard CCoinsView methods bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; + bool HaveCoins(const COutPoint &outpoint) const; uint256 GetBestBlock() const; void SetBestBlock(const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); @@ -470,6 +471,14 @@ public: */ const CCoins* AccessCoins(const uint256 &txid) const; + /** + * Return a copy of a Coin in the cache, or a pruned one if not found. This is + * more efficient than GetCoins. Modifications to other cache entries are + * allowed while accessing the returned pointer. + * TODO: return a reference to a Coin after changing CCoinsViewCache storage. + */ + const Coin AccessCoin(const COutPoint &output) const; + /** * Return a modifiable reference to a CCoins. If no entry with the given * txid exists, a new one is created. Simultaneous modifications are not @@ -488,6 +497,19 @@ public: */ CCoinsModifier ModifyNewCoins(const uint256 &txid, bool coinbase); + /** + * Add a coin. Set potential_overwrite to true if a non-pruned version may + * already exist. + */ + void AddCoin(const COutPoint& outpoint, Coin&& coin, bool potential_overwrite); + + /** + * Spend a coin. Pass moveto in order to get the deleted data. + * If no unspent output exists for the passed outpoint, this call + * has no effect. + */ + void SpendCoin(const COutPoint &outpoint, Coin* moveto = nullptr); + /** * Push the modifications applied to this cache to its base. * Failure to call this method before destruction will cause the changes to be forgotten. @@ -525,7 +547,7 @@ public: friend class CCoinsModifier; private: - CCoinsMap::const_iterator FetchCoins(const uint256 &txid) const; + CCoinsMap::iterator FetchCoins(const uint256 &txid) const; /** * By making the copy constructor private, we prevent accidentally using it when one intends to create a cache on top of a base cache. @@ -533,4 +555,13 @@ private: CCoinsViewCache(const CCoinsViewCache &); }; +//! Utility function to add all of a transaction's outputs to a cache. +// It assumes that overwrites are only possible for coinbase transactions, +// TODO: pass in a boolean to limit these possible overwrites to known +// (pre-BIP34) cases. +void AddCoins(CCoinsViewCache& cache, const CTransaction& tx, int nHeight); + +//! Utility function to find any unspent output with a given txid. +const Coin AccessByTxid(const CCoinsViewCache& cache, const uint256& txid); + #endif // BITCOIN_COINS_H From f68cdfe92b37f5a75be612b7de3c1a03245696d0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:30 -0700 Subject: [PATCH 12/29] Switch from per-tx to per-txout CCoinsViewCache methods in some places --- src/bench/ccoins_caching.cpp | 4 +- src/bitcoin-tx.cpp | 26 +++++----- src/consensus/tx_verify.cpp | 14 +++--- src/rpc/rawtransaction.cpp | 37 ++++++++------- src/test/script_P2SH_tests.cpp | 2 +- src/test/sigopcount_tests.cpp | 2 +- src/test/transaction_tests.cpp | 4 +- src/validation.cpp | 87 ++++++++++++++-------------------- 8 files changed, 83 insertions(+), 93 deletions(-) diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp index 1e8e3d462..5aab3381f 100644 --- a/src/bench/ccoins_caching.cpp +++ b/src/bench/ccoins_caching.cpp @@ -35,14 +35,14 @@ SetupDummyInputs(CBasicKeyStore& keystoreRet, CCoinsViewCache& coinsRet) dummyTransactions[0].vout[0].scriptPubKey << ToByteVector(key[0].GetPubKey()) << OP_CHECKSIG; dummyTransactions[0].vout[1].nValue = 50 * CENT; dummyTransactions[0].vout[1].scriptPubKey << ToByteVector(key[1].GetPubKey()) << OP_CHECKSIG; - coinsRet.ModifyCoins(dummyTransactions[0].GetHash())->FromTx(dummyTransactions[0], 0); + AddCoins(coinsRet, dummyTransactions[0], 0); dummyTransactions[1].vout.resize(2); dummyTransactions[1].vout[0].nValue = 21 * CENT; dummyTransactions[1].vout[0].scriptPubKey = GetScriptForDestination(key[2].GetPubKey().GetID()); dummyTransactions[1].vout[1].nValue = 22 * CENT; dummyTransactions[1].vout[1].scriptPubKey = GetScriptForDestination(key[3].GetPubKey().GetID()); - coinsRet.ModifyCoins(dummyTransactions[1].GetHash())->FromTx(dummyTransactions[1], 0); + AddCoins(coinsRet, dummyTransactions[1], 0); return dummyTransactions; } diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 45738b5df..a6a96fa02 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -556,24 +556,26 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) if (nOut < 0) throw std::runtime_error("vout must be positive"); + COutPoint out(txid, nOut); std::vector pkData(ParseHexUV(prevOut["scriptPubKey"], "scriptPubKey")); CScript scriptPubKey(pkData.begin(), pkData.end()); { - CCoinsModifier coins = view.ModifyCoins(txid); - if (coins->IsAvailable(nOut) && coins->vout[nOut].scriptPubKey != scriptPubKey) { + const Coin& coin = view.AccessCoin(out); + if (!coin.IsPruned() && coin.out.scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); - err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + "\nvs:\n"+ + err = err + ScriptToAsmStr(coin.out.scriptPubKey) + "\nvs:\n"+ ScriptToAsmStr(scriptPubKey); throw std::runtime_error(err); } - if ((unsigned int)nOut >= coins->vout.size()) - coins->vout.resize(nOut+1); - coins->vout[nOut].scriptPubKey = scriptPubKey; - coins->vout[nOut].nValue = 0; + Coin newcoin; + newcoin.out.scriptPubKey = scriptPubKey; + newcoin.out.nValue = 0; if (prevOut.exists("amount")) { - coins->vout[nOut].nValue = AmountFromValue(prevOut["amount"]); + newcoin.out.nValue = AmountFromValue(prevOut["amount"]); } + newcoin.nHeight = 1; + view.AddCoin(out, std::move(newcoin), true); } // if redeemScript given and private keys given, @@ -595,13 +597,13 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) // Sign what we can: for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { CTxIn& txin = mergedTx.vin[i]; - const CCoins* coins = view.AccessCoins(txin.prevout.hash); - if (!coins || !coins->IsAvailable(txin.prevout.n)) { + const Coin& coin = view.AccessCoin(txin.prevout); + if (coin.IsPruned()) { fComplete = false; continue; } - const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey; - const CAmount& amount = coins->vout[txin.prevout.n].nValue; + const CScript& prevPubKey = coin.out.scriptPubKey; + const CAmount& amount = coin.out.nValue; SignatureData sigdata; // Only sign SIGHASH_SINGLE if there's a corresponding output: diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 043f4cf95..9a6f3c9cd 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -213,20 +213,20 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c for (unsigned int i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; - const CCoins *coins = inputs.AccessCoins(prevout.hash); - assert(coins); + const Coin& coin = inputs.AccessCoin(prevout); + assert(!coin.IsPruned()); // If prev is coinbase, check that it's matured - if (coins->IsCoinBase()) { - if (nSpendHeight - coins->nHeight < COINBASE_MATURITY) + if (coin.IsCoinBase()) { + if (nSpendHeight - coin.nHeight < COINBASE_MATURITY) return state.Invalid(false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", - strprintf("tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight)); + strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } // Check for negative or overflow input values - nValueIn += coins->vout[prevout.n].nValue; - if (!MoneyRange(coins->vout[prevout.n].nValue) || !MoneyRange(nValueIn)) + nValueIn += coin.out.nValue; + if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 683bb2524..3f7b6adea 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -637,9 +637,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view BOOST_FOREACH(const CTxIn& txin, mergedTx.vin) { - const uint256& prevHash = txin.prevout.hash; - CCoins coins; - view.AccessCoins(prevHash); // this is certainly allowed to fail + view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail. } view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long @@ -691,24 +689,26 @@ UniValue signrawtransaction(const JSONRPCRequest& request) if (nOut < 0) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "vout must be positive"); + COutPoint out(txid, nOut); std::vector pkData(ParseHexO(prevOut, "scriptPubKey")); CScript scriptPubKey(pkData.begin(), pkData.end()); { - CCoinsModifier coins = view.ModifyCoins(txid); - if (coins->IsAvailable(nOut) && coins->vout[nOut].scriptPubKey != scriptPubKey) { + const Coin& coin = view.AccessCoin(out); + if (!coin.IsPruned() && coin.out.scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); - err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + "\nvs:\n"+ + err = err + ScriptToAsmStr(coin.out.scriptPubKey) + "\nvs:\n"+ ScriptToAsmStr(scriptPubKey); throw JSONRPCError(RPC_DESERIALIZATION_ERROR, err); } - if ((unsigned int)nOut >= coins->vout.size()) - coins->vout.resize(nOut+1); - coins->vout[nOut].scriptPubKey = scriptPubKey; - coins->vout[nOut].nValue = 0; + Coin newcoin; + newcoin.out.scriptPubKey = scriptPubKey; + newcoin.out.nValue = 0; if (prevOut.exists("amount")) { - coins->vout[nOut].nValue = AmountFromValue(find_value(prevOut, "amount")); + newcoin.out.nValue = AmountFromValue(find_value(prevOut, "amount")); } + newcoin.nHeight = 1; + view.AddCoin(out, std::move(newcoin), true); } // if redeemScript given and not using the local wallet (private keys @@ -766,13 +766,13 @@ UniValue signrawtransaction(const JSONRPCRequest& request) // Sign what we can: for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { CTxIn& txin = mergedTx.vin[i]; - const CCoins* coins = view.AccessCoins(txin.prevout.hash); - if (coins == NULL || !coins->IsAvailable(txin.prevout.n)) { + const Coin& coin = view.AccessCoin(txin.prevout); + if (coin.IsPruned()) { TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); continue; } - const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey; - const CAmount& amount = coins->vout[txin.prevout.n].nValue; + const CScript& prevPubKey = coin.out.scriptPubKey; + const CAmount& amount = coin.out.nValue; SignatureData sigdata; // Only sign SIGHASH_SINGLE if there's a corresponding output: @@ -844,9 +844,12 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) nMaxRawTxFee = 0; CCoinsViewCache &view = *pcoinsTip; - const CCoins* existingCoins = view.AccessCoins(hashTx); + bool fHaveChain = false; + for (size_t o = 0; !fHaveChain && o < tx->vout.size(); o++) { + const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); + fHaveChain = !existingCoin.IsPruned(); + } bool fHaveMempool = mempool.exists(hashTx); - bool fHaveChain = existingCoins && existingCoins->nHeight < 1000000000; if (!fHaveMempool && !fHaveChain) { // push to local node and sync with wallets CValidationState state; diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index ede68f23d..1ab0fef44 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -316,7 +316,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txFrom.vout[6].scriptPubKey = GetScriptForDestination(CScriptID(twentySigops)); txFrom.vout[6].nValue = 6000; - coins.ModifyCoins(txFrom.GetHash())->FromTx(txFrom, 0); + AddCoins(coins, txFrom, 0); CMutableTransaction txTo; txTo.vout.resize(1); diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 92781d763..4e117448f 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -102,7 +102,7 @@ void BuildTxs(CMutableTransaction& spendingTx, CCoinsViewCache& coins, CMutableT spendingTx.vout[0].nValue = 1; spendingTx.vout[0].scriptPubKey = CScript(); - coins.ModifyCoins(creationTx.GetHash())->FromTx(creationTx, 0); + AddCoins(coins, creationTx, 0); } BOOST_AUTO_TEST_CASE(GetTxSigOpCost) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index ef2e695ee..665827696 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -307,14 +307,14 @@ SetupDummyInputs(CBasicKeyStore& keystoreRet, CCoinsViewCache& coinsRet) dummyTransactions[0].vout[0].scriptPubKey << ToByteVector(key[0].GetPubKey()) << OP_CHECKSIG; dummyTransactions[0].vout[1].nValue = 50*CENT; dummyTransactions[0].vout[1].scriptPubKey << ToByteVector(key[1].GetPubKey()) << OP_CHECKSIG; - coinsRet.ModifyCoins(dummyTransactions[0].GetHash())->FromTx(dummyTransactions[0], 0); + AddCoins(coinsRet, dummyTransactions[0], 0); dummyTransactions[1].vout.resize(2); dummyTransactions[1].vout[0].nValue = 21*CENT; dummyTransactions[1].vout[0].scriptPubKey = GetScriptForDestination(key[2].GetPubKey().GetID()); dummyTransactions[1].vout[1].nValue = 22*CENT; dummyTransactions[1].vout[1].scriptPubKey = GetScriptForDestination(key[3].GetPubKey().GetID()); - coinsRet.ModifyCoins(dummyTransactions[1].GetHash())->FromTx(dummyTransactions[1], 0); + AddCoins(coinsRet, dummyTransactions[1], 0); return dummyTransactions; } diff --git a/src/validation.cpp b/src/validation.cpp index d6cc59b48..7ff7efc5e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -498,8 +498,8 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // during reorgs to ensure COINBASE_MATURITY is still met. bool fSpendsCoinbase = false; BOOST_FOREACH(const CTxIn &txin, tx.vin) { - const CCoins *coins = view.AccessCoins(txin.prevout.hash); - if (coins->IsCoinBase()) { + const Coin &coin = view.AccessCoin(txin.prevout); + if (coin.IsCoinBase()) { fSpendsCoinbase = true; break; } @@ -818,15 +818,8 @@ bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus } if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it - int nHeight = -1; - { - const CCoinsViewCache& view = *pcoinsTip; - const CCoins* coins = view.AccessCoins(hash); - if (coins) - nHeight = coins->nHeight; - } - if (nHeight > 0) - pindexSlow = chainActive[nHeight]; + const Coin& coin = AccessByTxid(*pcoinsTip, hash); + if (!coin.IsPruned()) pindexSlow = chainActive[coin.nHeight]; } if (pindexSlow) { @@ -1074,19 +1067,12 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund if (!tx.IsCoinBase()) { txundo.vprevout.reserve(tx.vin.size()); BOOST_FOREACH(const CTxIn &txin, tx.vin) { - CCoinsModifier coins = inputs.ModifyCoins(txin.prevout.hash); - unsigned nPos = txin.prevout.n; - - if (nPos >= coins->vout.size() || coins->vout[nPos].IsNull()) - assert(false); - // mark an outpoint spent, and construct undo information - txundo.vprevout.emplace_back(coins->vout[nPos], coins->nHeight, coins->fCoinBase); - bool ret = coins->Spend(nPos); - assert(ret); + txundo.vprevout.emplace_back(); + inputs.SpendCoin(txin.prevout, &txundo.vprevout.back()); } } // add outputs - inputs.ModifyNewCoins(tx.GetHash(), tx.IsCoinBase())->FromTx(tx, nHeight); + AddCoins(inputs, tx, nHeight); } void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight) @@ -1260,24 +1246,21 @@ int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out) { bool fClean = true; - CCoinsModifier coins = view.ModifyCoins(out.hash); - if (undo.nHeight != 0) { - if (!coins->IsPruned()) { - if (coins->fCoinBase != undo.fCoinBase || (uint32_t)coins->nHeight != undo.nHeight) fClean = false; // metadata mismatch + if (view.HaveCoins(out)) fClean = false; // overwriting transaction output + + if (undo.nHeight == 0) { + // Missing undo metadata (height and coinbase). Older versions included this + // information only in undo records for the last spend of a transactions' + // outputs. This implies that it must be present for some other output of the same tx. + const Coin& alternate = AccessByTxid(view, out.hash); + if (!alternate.IsPruned()) { + undo.nHeight = alternate.nHeight; + undo.fCoinBase = alternate.fCoinBase; + } else { + return DISCONNECT_FAILED; // adding output for transaction without known metadata } - // restore height/coinbase tx metadata from undo data - coins->fCoinBase = undo.fCoinBase; - coins->nHeight = undo.nHeight; - } else { - // Undo data does not contain height/coinbase. This should never happen - // for newly created undo entries. Previously, this data was only saved - // for the last spend of a transaction's outputs, so check IsPruned(). - if (coins->IsPruned()) fClean = false; // adding output to missing transaction } - 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] = std::move(undo.out); + view.AddCoin(out, std::move(undo), undo.fCoinBase); return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } @@ -1313,15 +1296,15 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* // Check that all outputs are available and match the outputs in the block itself // exactly. - { - CCoinsModifier outs = view.ModifyCoins(hash); - outs->ClearUnspendable(); - - CCoins outsBlock(tx, pindex->nHeight); - if (*outs != outsBlock) fClean = false; // transaction mismatch - - // remove outputs - outs->Clear(); + for (size_t o = 0; o < tx.vout.size(); o++) { + if (!tx.vout[o].scriptPubKey.IsUnspendable()) { + COutPoint out(hash, o); + Coin coin; + view.SpendCoin(out, &coin); + if (tx.vout[o] != coin.out) { + fClean = false; // transaction output mismatch + } + } } // restore inputs @@ -1518,10 +1501,12 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd if (fEnforceBIP30) { for (const auto& tx : block.vtx) { - const CCoins* coins = view.AccessCoins(tx->GetHash()); - if (coins && !coins->IsPruned()) - return state.DoS(100, error("ConnectBlock(): tried to overwrite transaction"), - REJECT_INVALID, "bad-txns-BIP30"); + for (size_t o = 0; o < tx->vout.size(); o++) { + if (view.HaveCoins(COutPoint(tx->GetHash(), o))) { + return state.DoS(100, error("ConnectBlock(): tried to overwrite transaction"), + REJECT_INVALID, "bad-txns-BIP30"); + } + } } } @@ -1588,7 +1573,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd // be in ConnectBlock because they require the UTXO set prevheights.resize(tx.vin.size()); for (size_t j = 0; j < tx.vin.size(); j++) { - prevheights[j] = view.AccessCoins(tx.vin[j].prevout.hash)->nHeight; + prevheights[j] = view.AccessCoin(tx.vin[j].prevout).nHeight; } if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { From c87b957a32e03c09d410abadf661f87eb813bcdb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 27 Apr 2017 10:37:33 -0400 Subject: [PATCH 13/29] Only pass things committed to by tx's witness hash to CScriptCheck This clarifies a bit more the ways in which the new script execution cache could break consensus in the future if additional data from the CCoins object were to be used as a part of script execution. After this change, any such consensus breaks should be very visible to reviewers, hopefully ensuring no such changes can be made. --- src/test/script_P2SH_tests.cpp | 3 ++- src/test/transaction_tests.cpp | 3 ++- src/validation.cpp | 12 ++++++++++-- src/validation.h | 4 ++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 1ab0fef44..0789b2e80 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -112,7 +112,8 @@ BOOST_AUTO_TEST_CASE(sign) { CScript sigSave = txTo[i].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; - bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)(); + const CTxOut& output = txFrom.vout[txTo[i].vin[0].prevout.n]; + bool sigOK = CScriptCheck(output.scriptPubKey, output.nValue, txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false, &txdata)(); if (i == j) BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); else diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 665827696..986922b2a 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -481,7 +481,8 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { for(uint32_t i = 0; i < mtx.vin.size(); i++) { std::vector vChecks; - CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); + const CTxOut& output = coins.vout[tx.vin[i].prevout.n]; + CScriptCheck check(output.scriptPubKey, output.nValue, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); vChecks.push_back(CScriptCheck()); check.swap(vChecks.back()); control.Add(vChecks); diff --git a/src/validation.cpp b/src/validation.cpp index 7ff7efc5e..43d2cf1d6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1119,8 +1119,16 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi const CCoins* coins = inputs.AccessCoins(prevout.hash); assert(coins); + // We very carefully only pass in things to CScriptCheck which + // are clearly committed to by tx' witness hash. This provides + // a sanity check that our caching is not introducing consensus + // failures through additional data in, eg, the coins being + // spent being checked as a part of CScriptCheck. + const CScript& scriptPubKey = coins->vout[prevout.n].scriptPubKey; + const CAmount amount = coins->vout[prevout.n].nValue; + // Verify signature - CScriptCheck check(*coins, tx, i, flags, cacheStore, &txdata); + CScriptCheck check(scriptPubKey, amount, tx, i, flags, cacheStore, &txdata); if (pvChecks) { pvChecks->push_back(CScriptCheck()); check.swap(pvChecks->back()); @@ -1132,7 +1140,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // arguments; if so, don't trigger DoS protection to // avoid splitting the network between upgraded and // non-upgraded nodes. - CScriptCheck check2(*coins, tx, i, + CScriptCheck check2(scriptPubKey, amount, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &txdata); if (check2()) return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); diff --git a/src/validation.h b/src/validation.h index b19c3ff4f..8931cfc4d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -404,8 +404,8 @@ private: public: CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} - CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) : - scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), + CScriptCheck(const CScript& scriptPubKeyIn, const CAmount amountIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) : + scriptPubKey(scriptPubKeyIn), amount(amountIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { } bool operator()(); From 8b3868c1b4bf89c41b26ecb3a4b7c3a2557e3868 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:32 -0700 Subject: [PATCH 14/29] Switch CScriptCheck to use Coin instead of CCoins --- src/test/transaction_tests.cpp | 15 ++++++++------- src/validation.cpp | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 986922b2a..5c7516fbf 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -470,18 +470,19 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { for (int i=0; i<20; i++) threadGroup.create_thread(boost::bind(&CCheckQueue::Thread, boost::ref(scriptcheckqueue))); - CCoins coins; - coins.fCoinBase = false; + std::vector coins; for(uint32_t i = 0; i < mtx.vin.size(); i++) { - CTxOut txout; - txout.nValue = 1000; - txout.scriptPubKey = scriptPubKey; - coins.vout.push_back(txout); + Coin coin; + coin.nHeight = 1; + coin.fCoinBase = false; + coin.out.nValue = 1000; + coin.out.scriptPubKey = scriptPubKey; + coins.emplace_back(std::move(coin)); } for(uint32_t i = 0; i < mtx.vin.size(); i++) { std::vector vChecks; - const CTxOut& output = coins.vout[tx.vin[i].prevout.n]; + const CTxOut& output = coins[tx.vin[i].prevout.n].out; CScriptCheck check(output.scriptPubKey, output.nValue, tx, i, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false, &txdata); vChecks.push_back(CScriptCheck()); check.swap(vChecks.back()); diff --git a/src/validation.cpp b/src/validation.cpp index 43d2cf1d6..b295a7f86 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1116,16 +1116,16 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi if (fScriptChecks) { for (unsigned int i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; - const CCoins* coins = inputs.AccessCoins(prevout.hash); - assert(coins); + const Coin& coin = inputs.AccessCoin(prevout); + assert(!coin.IsPruned()); // We very carefully only pass in things to CScriptCheck which // are clearly committed to by tx' witness hash. This provides // a sanity check that our caching is not introducing consensus // failures through additional data in, eg, the coins being // spent being checked as a part of CScriptCheck. - const CScript& scriptPubKey = coins->vout[prevout.n].scriptPubKey; - const CAmount amount = coins->vout[prevout.n].nValue; + const CScript& scriptPubKey = coin.out.scriptPubKey; + const CAmount amount = coin.out.nValue; // Verify signature CScriptCheck check(scriptPubKey, amount, tx, i, flags, cacheStore, &txdata); From 961e4839793f8b4ad37d29672faf1695ff6ec03a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:34 -0700 Subject: [PATCH 15/29] Switch tests from ModifyCoins to AddCoin/SpendCoin --- src/test/coins_tests.cpp | 268 +++++++++++++++++---------------------- 1 file changed, 119 insertions(+), 149 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 9c758ee9a..3ad2625ba 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -22,6 +22,15 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund namespace { +//! equality test +bool operator==(const Coin &a, const Coin &b) { + // Empty Coin objects are always equal. + if (a.IsPruned() && b.IsPruned()) return true; + return a.fCoinBase == b.fCoinBase && + a.nHeight == b.nHeight && + a.out == b.out; +} + class CCoinsViewTest : public CCoinsView { uint256 hashBestBlock_; @@ -134,8 +143,9 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) { uint256 txid = txids[insecure_rand() % txids.size()]; // txid we're going to modify in this iteration. CCoins& coins = result[txid]; - CCoinsModifier entry = stack.back()->ModifyCoins(txid); - BOOST_CHECK(coins == *entry); + const Coin& entry = stack.back()->AccessCoin(COutPoint(txid, 0)); + BOOST_CHECK((entry.IsPruned() && coins.IsPruned()) || entry == Coin(coins.vout[0], coins.nHeight, coins.fCoinBase)); + if (insecure_rand() % 5 == 0 || coins.IsPruned()) { if (coins.IsPruned()) { added_an_entry = true; @@ -144,12 +154,15 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) } coins.vout.resize(1); coins.vout[0].nValue = insecure_rand(); - *entry = coins; } else { coins.Clear(); - entry->Clear(); removed_an_entry = true; } + if (coins.IsPruned()) { + stack.back()->SpendCoin(COutPoint(txid, 0)); + } else { + stack.back()->AddCoin(COutPoint(txid, 0), Coin(coins.vout[0], coins.nHeight, coins.fCoinBase), true); + } } // Once every 1000 iterations and at the end, verify the full cache. @@ -325,8 +338,9 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // The test is designed to ensure spending a duplicate coinbase will work properly // if that ever happens and not resurrect the previously overwritten coinbase - if (duplicateids.count(prevouthash)) + if (duplicateids.count(prevouthash)) { spent_a_duplicate_coinbase = true; + } } // Update the expected result to know about the new output coins @@ -341,10 +355,8 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // Track this tx and undo info to use later alltxs.insert(std::make_pair(tx.GetHash(),std::make_tuple(tx,undo,oldcoins))); - } - - //1/20 times undo a previous transaction - else if (utxoset.size()) { + } else if (utxoset.size()) { + //1/20 times undo a previous transaction TxData &txd = FindRandomFrom(utxoset); CTransaction &tx = std::get<0>(txd); @@ -365,8 +377,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // See code in DisconnectBlock // remove outputs { - CCoinsModifier outs = stack.back()->ModifyCoins(undohash); - outs->Clear(); + stack.back()->SpendCoin(COutPoint(undohash, 0)); } // restore inputs if (!tx.IsCoinBase()) { @@ -496,6 +507,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) } const static uint256 TXID; +const static COutPoint OUTPOINT = {uint256(), 0}; const static CAmount PRUNED = -1; const static CAmount ABSENT = -2; const static CAmount FAIL = -3; @@ -577,10 +589,10 @@ public: CCoinsViewCacheTest cache{&base}; }; -void CheckAccessCoins(CAmount base_value, CAmount cache_value, CAmount expected_value, char cache_flags, char expected_flags) +void CheckAccessCoin(CAmount base_value, CAmount cache_value, CAmount expected_value, char cache_flags, char expected_flags) { SingleEntryCacheTest test(base_value, cache_value, cache_flags); - test.cache.AccessCoins(TXID); + test.cache.AccessCoin(OUTPOINT); test.cache.SelfTest(); CAmount result_value; @@ -599,39 +611,39 @@ BOOST_AUTO_TEST_CASE(ccoins_access) * Base Cache Result Cache Result * Value Value Value Flags Flags */ - CheckAccessCoins(ABSENT, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY ); - CheckAccessCoins(ABSENT, PRUNED, PRUNED, 0 , 0 ); - CheckAccessCoins(ABSENT, PRUNED, PRUNED, FRESH , FRESH ); - CheckAccessCoins(ABSENT, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckAccessCoins(ABSENT, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); - CheckAccessCoins(ABSENT, VALUE2, VALUE2, 0 , 0 ); - CheckAccessCoins(ABSENT, VALUE2, VALUE2, FRESH , FRESH ); - CheckAccessCoins(ABSENT, VALUE2, VALUE2, DIRTY , DIRTY ); - CheckAccessCoins(ABSENT, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); - CheckAccessCoins(PRUNED, ABSENT, PRUNED, NO_ENTRY , FRESH ); - CheckAccessCoins(PRUNED, PRUNED, PRUNED, 0 , 0 ); - CheckAccessCoins(PRUNED, PRUNED, PRUNED, FRESH , FRESH ); - CheckAccessCoins(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckAccessCoins(PRUNED, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); - CheckAccessCoins(PRUNED, VALUE2, VALUE2, 0 , 0 ); - CheckAccessCoins(PRUNED, VALUE2, VALUE2, FRESH , FRESH ); - CheckAccessCoins(PRUNED, VALUE2, VALUE2, DIRTY , DIRTY ); - CheckAccessCoins(PRUNED, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); - CheckAccessCoins(VALUE1, ABSENT, VALUE1, NO_ENTRY , 0 ); - CheckAccessCoins(VALUE1, PRUNED, PRUNED, 0 , 0 ); - CheckAccessCoins(VALUE1, PRUNED, PRUNED, FRESH , FRESH ); - CheckAccessCoins(VALUE1, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckAccessCoins(VALUE1, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); - CheckAccessCoins(VALUE1, VALUE2, VALUE2, 0 , 0 ); - CheckAccessCoins(VALUE1, VALUE2, VALUE2, FRESH , FRESH ); - CheckAccessCoins(VALUE1, VALUE2, VALUE2, DIRTY , DIRTY ); - CheckAccessCoins(VALUE1, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(ABSENT, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY ); + CheckAccessCoin(ABSENT, PRUNED, PRUNED, 0 , 0 ); + CheckAccessCoin(ABSENT, PRUNED, PRUNED, FRESH , FRESH ); + CheckAccessCoin(ABSENT, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckAccessCoin(ABSENT, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(ABSENT, VALUE2, VALUE2, 0 , 0 ); + CheckAccessCoin(ABSENT, VALUE2, VALUE2, FRESH , FRESH ); + CheckAccessCoin(ABSENT, VALUE2, VALUE2, DIRTY , DIRTY ); + CheckAccessCoin(ABSENT, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(PRUNED, ABSENT, PRUNED, NO_ENTRY , FRESH ); + CheckAccessCoin(PRUNED, PRUNED, PRUNED, 0 , 0 ); + CheckAccessCoin(PRUNED, PRUNED, PRUNED, FRESH , FRESH ); + CheckAccessCoin(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckAccessCoin(PRUNED, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(PRUNED, VALUE2, VALUE2, 0 , 0 ); + CheckAccessCoin(PRUNED, VALUE2, VALUE2, FRESH , FRESH ); + CheckAccessCoin(PRUNED, VALUE2, VALUE2, DIRTY , DIRTY ); + CheckAccessCoin(PRUNED, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(VALUE1, ABSENT, VALUE1, NO_ENTRY , 0 ); + CheckAccessCoin(VALUE1, PRUNED, PRUNED, 0 , 0 ); + CheckAccessCoin(VALUE1, PRUNED, PRUNED, FRESH , FRESH ); + CheckAccessCoin(VALUE1, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckAccessCoin(VALUE1, PRUNED, PRUNED, DIRTY|FRESH, DIRTY|FRESH); + CheckAccessCoin(VALUE1, VALUE2, VALUE2, 0 , 0 ); + CheckAccessCoin(VALUE1, VALUE2, VALUE2, FRESH , FRESH ); + CheckAccessCoin(VALUE1, VALUE2, VALUE2, DIRTY , DIRTY ); + CheckAccessCoin(VALUE1, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH); } -void CheckModifyCoins(CAmount base_value, CAmount cache_value, CAmount modify_value, CAmount expected_value, char cache_flags, char expected_flags) +void CheckSpendCoins(CAmount base_value, CAmount cache_value, CAmount expected_value, char cache_flags, char expected_flags) { SingleEntryCacheTest test(base_value, cache_value, cache_flags); - SetCoinsValue(modify_value, *test.cache.ModifyCoins(TXID)); + test.cache.SpendCoin(OUTPOINT); test.cache.SelfTest(); CAmount result_value; @@ -641,79 +653,55 @@ void CheckModifyCoins(CAmount base_value, CAmount cache_value, CAmount modify_va BOOST_CHECK_EQUAL(result_flags, expected_flags); }; -BOOST_AUTO_TEST_CASE(ccoins_modify) +BOOST_AUTO_TEST_CASE(ccoins_spend) { - /* Check ModifyCoin behavior, requesting a coin from a cache view layered on - * top of a base view, writing a modification to the coin, and then checking + /* Check SpendCoin behavior, requesting a coin from a cache view layered on + * top of a base view, spending, and then checking * the resulting entry in the cache after the modification. * - * Base Cache Write Result Cache Result - * Value Value Value Value Flags Flags + * Base Cache Result Cache Result + * Value Value Value Flags Flags */ - CheckModifyCoins(ABSENT, ABSENT, PRUNED, ABSENT, NO_ENTRY , NO_ENTRY ); - CheckModifyCoins(ABSENT, ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY|FRESH); - CheckModifyCoins(ABSENT, PRUNED, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(ABSENT, PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(ABSENT, PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(ABSENT, PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(ABSENT, PRUNED, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(ABSENT, PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(ABSENT, PRUNED, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(ABSENT, PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); - CheckModifyCoins(ABSENT, VALUE2, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(ABSENT, VALUE2, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(ABSENT, VALUE2, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(ABSENT, VALUE2, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(ABSENT, VALUE2, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(ABSENT, VALUE2, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(ABSENT, VALUE2, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(ABSENT, VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); - CheckModifyCoins(PRUNED, ABSENT, PRUNED, ABSENT, NO_ENTRY , NO_ENTRY ); - CheckModifyCoins(PRUNED, ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY|FRESH); - CheckModifyCoins(PRUNED, PRUNED, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(PRUNED, PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(PRUNED, PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(PRUNED, PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(PRUNED, PRUNED, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(PRUNED, PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(PRUNED, PRUNED, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(PRUNED, PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); - CheckModifyCoins(PRUNED, VALUE2, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(PRUNED, VALUE2, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(PRUNED, VALUE2, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(PRUNED, VALUE2, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(PRUNED, VALUE2, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(PRUNED, VALUE2, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(PRUNED, VALUE2, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(PRUNED, VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); - CheckModifyCoins(VALUE1, ABSENT, PRUNED, PRUNED, NO_ENTRY , DIRTY ); - CheckModifyCoins(VALUE1, ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY ); - CheckModifyCoins(VALUE1, PRUNED, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(VALUE1, PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(VALUE1, PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(VALUE1, PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(VALUE1, PRUNED, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(VALUE1, PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(VALUE1, PRUNED, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(VALUE1, PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); - CheckModifyCoins(VALUE1, VALUE2, PRUNED, PRUNED, 0 , DIRTY ); - CheckModifyCoins(VALUE1, VALUE2, PRUNED, ABSENT, FRESH , NO_ENTRY ); - CheckModifyCoins(VALUE1, VALUE2, PRUNED, PRUNED, DIRTY , DIRTY ); - CheckModifyCoins(VALUE1, VALUE2, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); - CheckModifyCoins(VALUE1, VALUE2, VALUE3, VALUE3, 0 , DIRTY ); - CheckModifyCoins(VALUE1, VALUE2, VALUE3, VALUE3, FRESH , DIRTY|FRESH); - CheckModifyCoins(VALUE1, VALUE2, VALUE3, VALUE3, DIRTY , DIRTY ); - CheckModifyCoins(VALUE1, VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH); + CheckSpendCoins(ABSENT, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY ); + CheckSpendCoins(ABSENT, PRUNED, PRUNED, 0 , DIRTY ); + CheckSpendCoins(ABSENT, PRUNED, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(ABSENT, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(ABSENT, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); + CheckSpendCoins(ABSENT, VALUE2, PRUNED, 0 , DIRTY ); + CheckSpendCoins(ABSENT, VALUE2, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(ABSENT, VALUE2, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(ABSENT, VALUE2, ABSENT, DIRTY|FRESH, NO_ENTRY ); + CheckSpendCoins(PRUNED, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY ); + CheckSpendCoins(PRUNED, PRUNED, PRUNED, 0 , DIRTY ); + CheckSpendCoins(PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); + CheckSpendCoins(PRUNED, VALUE2, PRUNED, 0 , DIRTY ); + CheckSpendCoins(PRUNED, VALUE2, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(PRUNED, VALUE2, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(PRUNED, VALUE2, ABSENT, DIRTY|FRESH, NO_ENTRY ); + CheckSpendCoins(VALUE1, ABSENT, PRUNED, NO_ENTRY , DIRTY ); + CheckSpendCoins(VALUE1, PRUNED, PRUNED, 0 , DIRTY ); + CheckSpendCoins(VALUE1, PRUNED, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(VALUE1, PRUNED, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(VALUE1, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY ); + CheckSpendCoins(VALUE1, VALUE2, PRUNED, 0 , DIRTY ); + CheckSpendCoins(VALUE1, VALUE2, ABSENT, FRESH , NO_ENTRY ); + CheckSpendCoins(VALUE1, VALUE2, PRUNED, DIRTY , DIRTY ); + CheckSpendCoins(VALUE1, VALUE2, ABSENT, DIRTY|FRESH, NO_ENTRY ); } -void CheckModifyNewCoinsBase(CAmount base_value, CAmount cache_value, CAmount modify_value, CAmount expected_value, char cache_flags, char expected_flags, bool coinbase) +void CheckAddCoinBase(CAmount base_value, CAmount cache_value, CAmount modify_value, CAmount expected_value, char cache_flags, char expected_flags, bool coinbase) { SingleEntryCacheTest test(base_value, cache_value, cache_flags); CAmount result_value; char result_flags; try { - SetCoinsValue(modify_value, *test.cache.ModifyNewCoins(TXID, coinbase)); + CTxOut output; + output.nValue = modify_value; + test.cache.AddCoin(OUTPOINT, Coin(std::move(output), 1, coinbase), coinbase); + test.cache.SelfTest(); GetCoinsMapEntry(test.cache.map(), result_value, result_flags); } catch (std::logic_error& e) { result_value = FAIL; @@ -724,64 +712,46 @@ void CheckModifyNewCoinsBase(CAmount base_value, CAmount cache_value, CAmount mo BOOST_CHECK_EQUAL(result_flags, expected_flags); } -// Simple wrapper for CheckModifyNewCoinsBase function above that loops through +// Simple wrapper for CheckAddCoinBase function above that loops through // different possible base_values, making sure each one gives the same results. -// This wrapper lets the modify_new test below be shorter and less repetitive, -// while still verifying that the CoinsViewCache::ModifyNewCoins implementation +// This wrapper lets the coins_add test below be shorter and less repetitive, +// while still verifying that the CoinsViewCache::AddCoin implementation // ignores base values. template -void CheckModifyNewCoins(Args&&... args) +void CheckAddCoin(Args&&... args) { for (CAmount base_value : {ABSENT, PRUNED, VALUE1}) - CheckModifyNewCoinsBase(base_value, std::forward(args)...); + CheckAddCoinBase(base_value, std::forward(args)...); } -BOOST_AUTO_TEST_CASE(ccoins_modify_new) +BOOST_AUTO_TEST_CASE(ccoins_add) { - /* Check ModifyNewCoin behavior, requesting a new coin from a cache view, + /* Check AddCoin behavior, requesting a new coin from a cache view, * writing a modification to the coin, and then checking the resulting * entry in the cache after the modification. Verify behavior with the - * with the ModifyNewCoin coinbase argument set to false, and to true. + * with the AddCoin potential_overwrite argument set to false, and to true. * - * Cache Write Result Cache Result Coinbase - * Value Value Value Flags Flags + * Cache Write Result Cache Result potential_overwrite + * Value Value Value Flags Flags */ - CheckModifyNewCoins(ABSENT, PRUNED, ABSENT, NO_ENTRY , NO_ENTRY , false); - CheckModifyNewCoins(ABSENT, PRUNED, PRUNED, NO_ENTRY , DIRTY , true ); - CheckModifyNewCoins(ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY|FRESH, false); - CheckModifyNewCoins(ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY , true ); - CheckModifyNewCoins(PRUNED, PRUNED, ABSENT, 0 , NO_ENTRY , false); - CheckModifyNewCoins(PRUNED, PRUNED, PRUNED, 0 , DIRTY , true ); - CheckModifyNewCoins(PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY , false); - CheckModifyNewCoins(PRUNED, PRUNED, ABSENT, FRESH , NO_ENTRY , true ); - CheckModifyNewCoins(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY , false); - CheckModifyNewCoins(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY , true ); - CheckModifyNewCoins(PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY , false); - CheckModifyNewCoins(PRUNED, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY , true ); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, 0 , DIRTY|FRESH, false); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, 0 , DIRTY , true ); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH, false); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH, true ); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, DIRTY , DIRTY , false); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, DIRTY , DIRTY , true ); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, false); - CheckModifyNewCoins(PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, true ); - CheckModifyNewCoins(VALUE2, PRUNED, FAIL , 0 , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, PRUNED, PRUNED, 0 , DIRTY , true ); - CheckModifyNewCoins(VALUE2, PRUNED, FAIL , FRESH , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, PRUNED, ABSENT, FRESH , NO_ENTRY , true ); - CheckModifyNewCoins(VALUE2, PRUNED, FAIL , DIRTY , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, PRUNED, PRUNED, DIRTY , DIRTY , true ); - CheckModifyNewCoins(VALUE2, PRUNED, FAIL , DIRTY|FRESH, NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, PRUNED, ABSENT, DIRTY|FRESH, NO_ENTRY , true ); - CheckModifyNewCoins(VALUE2, VALUE3, FAIL , 0 , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, VALUE3, VALUE3, 0 , DIRTY , true ); - CheckModifyNewCoins(VALUE2, VALUE3, FAIL , FRESH , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, VALUE3, VALUE3, FRESH , DIRTY|FRESH, true ); - CheckModifyNewCoins(VALUE2, VALUE3, FAIL , DIRTY , NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, VALUE3, VALUE3, DIRTY , DIRTY , true ); - CheckModifyNewCoins(VALUE2, VALUE3, FAIL , DIRTY|FRESH, NO_ENTRY , false); - CheckModifyNewCoins(VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, true ); + CheckAddCoin(ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY|FRESH, false); + CheckAddCoin(ABSENT, VALUE3, VALUE3, NO_ENTRY , DIRTY , true ); + CheckAddCoin(PRUNED, VALUE3, VALUE3, 0 , DIRTY|FRESH, false); + CheckAddCoin(PRUNED, VALUE3, VALUE3, 0 , DIRTY , true ); + CheckAddCoin(PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH, false); + CheckAddCoin(PRUNED, VALUE3, VALUE3, FRESH , DIRTY|FRESH, true ); + CheckAddCoin(PRUNED, VALUE3, VALUE3, DIRTY , DIRTY , false); + CheckAddCoin(PRUNED, VALUE3, VALUE3, DIRTY , DIRTY , true ); + CheckAddCoin(PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, false); + CheckAddCoin(PRUNED, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, true ); + CheckAddCoin(VALUE2, VALUE3, FAIL , 0 , NO_ENTRY , false); + CheckAddCoin(VALUE2, VALUE3, VALUE3, 0 , DIRTY , true ); + CheckAddCoin(VALUE2, VALUE3, FAIL , FRESH , NO_ENTRY , false); + CheckAddCoin(VALUE2, VALUE3, VALUE3, FRESH , DIRTY|FRESH, true ); + CheckAddCoin(VALUE2, VALUE3, FAIL , DIRTY , NO_ENTRY , false); + CheckAddCoin(VALUE2, VALUE3, VALUE3, DIRTY , DIRTY , true ); + CheckAddCoin(VALUE2, VALUE3, FAIL , DIRTY|FRESH, NO_ENTRY , false); + CheckAddCoin(VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH, true ); } void CheckWriteCoins(CAmount parent_value, CAmount child_value, CAmount expected_value, char parent_flags, char child_flags, char expected_flags) From 05293f3cb75ad08ca23cba8e795e27d4d5e4d690 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:36 -0700 Subject: [PATCH 16/29] Remove ModifyCoins/ModifyNewCoins --- src/coins.cpp | 83 +-------------------------------------------------- src/coins.h | 47 ----------------------------- 2 files changed, 1 insertion(+), 129 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 3ac46d080..8b2f14837 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -60,12 +60,7 @@ size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} -CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), hasModifier(false), cachedCoinsUsage(0) { } - -CCoinsViewCache::~CCoinsViewCache() -{ - assert(!hasModifier); -} +CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), cachedCoinsUsage(0) { } size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; @@ -98,62 +93,6 @@ bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { return false; } -CCoinsModifier CCoinsViewCache::ModifyCoins(const uint256 &txid) { - assert(!hasModifier); - std::pair ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())); - size_t cachedCoinUsage = 0; - if (ret.second) { - if (!base->GetCoins(txid, ret.first->second.coins)) { - // The parent view does not have this entry; mark it as fresh. - ret.first->second.coins.Clear(); - ret.first->second.flags = CCoinsCacheEntry::FRESH; - } else if (ret.first->second.coins.IsPruned()) { - // The parent view only has a pruned entry for this; mark it as fresh. - ret.first->second.flags = CCoinsCacheEntry::FRESH; - } - } else { - cachedCoinUsage = ret.first->second.coins.DynamicMemoryUsage(); - } - // Assume that whenever ModifyCoins is called, the entry will be modified. - ret.first->second.flags |= CCoinsCacheEntry::DIRTY; - return CCoinsModifier(*this, ret.first, cachedCoinUsage); -} - -/* ModifyNewCoins allows for faster coin modification when creating the new - * outputs from a transaction. It assumes that BIP 30 (no duplicate txids) - * applies and has already been tested for (or the test is not required due to - * BIP 34, height in coinbase). If we can assume BIP 30 then we know that any - * non-coinbase transaction we are adding to the UTXO must not already exist in - * the utxo unless it is fully spent. Thus we can check only if it exists DIRTY - * at the current level of the cache, in which case it is not safe to mark it - * FRESH (b/c then its spentness still needs to flushed). If it's not dirty and - * doesn't exist or is pruned in the current cache, we know it either doesn't - * exist or is pruned in parent caches, which is the definition of FRESH. The - * exception to this is the two historical violations of BIP 30 in the chain, - * both of which were coinbases. We do not mark these fresh so we we can ensure - * that they will still be properly overwritten when spent. - */ -CCoinsModifier CCoinsViewCache::ModifyNewCoins(const uint256 &txid, bool coinbase) { - assert(!hasModifier); - std::pair ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())); - if (!coinbase) { - // New coins must not already exist. - if (!ret.first->second.coins.IsPruned()) - throw std::logic_error("ModifyNewCoins should not find pre-existing coins on a non-coinbase unless they are pruned!"); - - if (!(ret.first->second.flags & CCoinsCacheEntry::DIRTY)) { - // If the coin is known to be pruned (have no unspent outputs) in - // the current view and the cache entry is not dirty, we know the - // coin also must be pruned in the parent view as well, so it is safe - // to mark this fresh. - ret.first->second.flags |= CCoinsCacheEntry::FRESH; - } - } - ret.first->second.coins.Clear(); - ret.first->second.flags |= CCoinsCacheEntry::DIRTY; - return CCoinsModifier(*this, ret.first, 0); -} - void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possible_overwrite) { assert(!coin.IsPruned()); if (coin.out.scriptPubKey.IsUnspendable()) return; @@ -257,7 +196,6 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { } bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { - assert(!hasModifier); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). CCoinsMap::iterator itUs = cacheCoins.find(it->first); @@ -366,25 +304,6 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } -CCoinsModifier::CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_, size_t usage) : cache(cache_), it(it_), cachedCoinUsage(usage) { - assert(!cache.hasModifier); - cache.hasModifier = true; -} - -CCoinsModifier::~CCoinsModifier() -{ - assert(cache.hasModifier); - cache.hasModifier = false; - it->second.coins.Cleanup(); - cache.cachedCoinsUsage -= cachedCoinUsage; // Subtract the old usage - if ((it->second.flags & CCoinsCacheEntry::FRESH) && it->second.coins.IsPruned()) { - cache.cacheCoins.erase(it); - } else { - // If the coin still exists after the modification, add the new usage - cache.cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); - } -} - CCoinsViewCursor::~CCoinsViewCursor() { } diff --git a/src/coins.h b/src/coins.h index 5879530f9..fcf7c47a5 100644 --- a/src/coins.h +++ b/src/coins.h @@ -405,36 +405,10 @@ public: }; -class CCoinsViewCache; - -/** - * A reference to a mutable cache entry. Encapsulating it allows us to run - * cleanup code after the modification is finished, and keeping track of - * concurrent modifications. - */ -class CCoinsModifier -{ -private: - CCoinsViewCache& cache; - CCoinsMap::iterator it; - size_t cachedCoinUsage; // Cached memory usage of the CCoins object before modification - CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_, size_t usage); - -public: - CCoins* operator->() { return &it->second.coins; } - CCoins& operator*() { return it->second.coins; } - ~CCoinsModifier(); - friend class CCoinsViewCache; -}; - /** CCoinsView that adds a memory cache for transactions to another CCoinsView */ class CCoinsViewCache : public CCoinsViewBacked { protected: - /* Whether this cache has an active modifier. */ - bool hasModifier; - - /** * Make mutable so that we can "fill the cache" even from Get-methods * declared as "const". @@ -447,7 +421,6 @@ protected: public: CCoinsViewCache(CCoinsView *baseIn); - ~CCoinsViewCache(); // Standard CCoinsView methods bool GetCoins(const uint256 &txid, CCoins &coins) const; @@ -479,24 +452,6 @@ public: */ const Coin AccessCoin(const COutPoint &output) const; - /** - * Return a modifiable reference to a CCoins. If no entry with the given - * txid exists, a new one is created. Simultaneous modifications are not - * allowed. - */ - CCoinsModifier ModifyCoins(const uint256 &txid); - - /** - * Return a modifiable reference to a CCoins. Assumes that no entry with the given - * txid exists and creates a new one. This saves a database access in the case where - * the coins were to be wiped out by FromTx anyway. This should not be called with - * the 2 historical coinbase duplicate pairs because the new coins are marked fresh, and - * in the event the duplicate coinbase was spent before a flush, the now pruned coins - * would not properly overwrite the first coinbase of the pair. Simultaneous modifications - * are not allowed. - */ - CCoinsModifier ModifyNewCoins(const uint256 &txid, bool coinbase); - /** * Add a coin. Set potential_overwrite to true if a non-pruned version may * already exist. @@ -544,8 +499,6 @@ public: const CTxOut &GetOutputFor(const CTxIn& input) const; - friend class CCoinsModifier; - private: CCoinsMap::iterator FetchCoins(const uint256 &txid) const; From 13870b56fcd0bfacedce3ae42a3de3d5e9dc7bc1 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:37 -0700 Subject: [PATCH 17/29] Replace CCoins-based CTxMemPool::pruneSpent with isSpent --- src/rest.cpp | 3 +-- src/rpc/blockchain.cpp | 3 +-- src/txmempool.cpp | 11 ++--------- src/txmempool.h | 2 +- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index 9c291fe0a..4a7101009 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -513,8 +513,7 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) uint256 hash = vOutPoints[i].hash; bool hit = false; if (view.GetCoins(hash, coins)) { - mempool.pruneSpent(hash, coins); - if (coins.IsAvailable(vOutPoints[i].n)) { + if (coins.IsAvailable(vOutPoints[i].n) && !mempool.isSpent(vOutPoints[i])) { hit = true; // 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). diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e59b9b86b..94e42a864 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -973,9 +973,8 @@ UniValue gettxout(const JSONRPCRequest& request) if (fMempool) { LOCK(mempool.cs); CCoinsViewMemPool view(pcoinsTip, mempool); - if (!view.GetCoins(hash, coins)) + if (!view.GetCoins(hash, coins) || mempool.isSpent(COutPoint(hash, n))) // TODO: this should be done by the CCoinsViewMemPool return NullUniValue; - mempool.pruneSpent(hash, coins); // TODO: this should be done by the CCoinsViewMemPool } else { if (!pcoinsTip->GetCoins(hash, coins)) return NullUniValue; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 33df0536d..51b93e92b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -343,17 +343,10 @@ CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) : nCheckFrequency = 0; } -void CTxMemPool::pruneSpent(const uint256 &hashTx, CCoins &coins) +bool CTxMemPool::isSpent(const COutPoint& outpoint) { LOCK(cs); - - auto it = mapNextTx.lower_bound(COutPoint(hashTx, 0)); - - // iterate over all COutPoints in mapNextTx whose hash equals the provided hashTx - while (it != mapNextTx.end() && it->first->hash == hashTx) { - coins.Spend(it->first->n); // and remove those outputs from coins - it++; - } + return mapNextTx.count(outpoint); } unsigned int CTxMemPool::GetTransactionsUpdated() const diff --git a/src/txmempool.h b/src/txmempool.h index a91eb5be5..6547f64f7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -509,7 +509,7 @@ public: void _clear(); //lock free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); void queryHashes(std::vector& vtxid); - void pruneSpent(const uint256& hash, CCoins &coins); + bool isSpent(const COutPoint& outpoint); unsigned int GetTransactionsUpdated() const; void AddTransactionsUpdated(unsigned int n); /** From 4ec0d9e794e3f338e1ebb8b644ae890d2c2da2ee Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 26 Apr 2017 16:39:58 -0700 Subject: [PATCH 18/29] Refactor GetUTXOStats in preparation for per-COutPoint iteration --- src/rpc/blockchain.cpp | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 94e42a864..3e9b1a1b1 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -788,6 +788,22 @@ struct CCoinsStats CCoinsStats() : nHeight(0), nTransactions(0), nTransactionOutputs(0), nTotalAmount(0) {} }; +static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, const std::map& outputs) +{ + assert(!outputs.empty()); + ss << hash; + ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase); + stats.nTransactions++; + for (const auto output : outputs) { + ss << VARINT(output.first + 1); + ss << *(const CScriptBase*)(&output.second.out.scriptPubKey); + ss << VARINT(output.second.out.nValue); + stats.nTransactionOutputs++; + stats.nTotalAmount += output.second.out.nValue; + } + ss << VARINT(0); +} + //! Calculate statistics about the unspent transaction output set static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) { @@ -800,33 +816,25 @@ static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) stats.nHeight = mapBlockIndex.find(stats.hashBlock)->second->nHeight; } ss << stats.hashBlock; - CAmount nTotalAmount = 0; while (pcursor->Valid()) { boost::this_thread::interruption_point(); uint256 key; CCoins coins; if (pcursor->GetKey(key) && pcursor->GetValue(coins)) { - stats.nTransactions++; - ss << key; - ss << VARINT(coins.nHeight * 2 + coins.fCoinBase); + std::map outputs; for (unsigned int i=0; iNext(); } stats.hashSerialized = ss.GetHash(); - stats.nTotalAmount = nTotalAmount; stats.nDiskSize = view->EstimateSize(); return true; } From 50830796889ecaa458871f1db878c255dd2554cb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:39 -0700 Subject: [PATCH 19/29] Switch CCoinsView and chainstate db from per-txid to per-txout This patch makes several related changes: * Changes the CCoinsView virtual methods (GetCoins, HaveCoins, ...) to be COutPoint/Coin-based rather than txid/CCoins-based. * Changes the chainstate db to a new incompatible format that is also COutPoint/Coin based. * Implements reconstruction code for hash_serialized_2. * Adapts the coins_tests unit tests (thanks to Russell Yanofsky). A side effect of the new CCoinsView model is that we can no longer use the (unreliable) test for transaction outputs in the UTXO set to determine whether we already have a particular transaction. --- src/coins.cpp | 112 ++++++--------- src/coins.h | 74 +++++----- src/init.cpp | 4 +- src/net_processing.cpp | 5 +- src/qt/transactiondesc.cpp | 7 +- src/rest.cpp | 20 +-- src/rpc/blockchain.cpp | 47 ++++--- src/rpc/rawtransaction.cpp | 7 +- src/test/coins_tests.cpp | 237 ++++++++++++++------------------ src/test/test_bitcoin_fuzzy.cpp | 4 +- src/txdb.cpp | 66 ++++++--- src/txdb.h | 16 +-- src/txmempool.cpp | 40 +++--- src/txmempool.h | 33 ++++- src/validation.cpp | 64 ++++----- 15 files changed, 363 insertions(+), 373 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 8b2f14837..38b8df171 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -42,41 +42,40 @@ bool CCoins::Spend(uint32_t nPos) return true; } -bool CCoinsView::GetCoins(const uint256 &txid, CCoins &coins) const { return false; } -bool CCoinsView::HaveCoins(const uint256 &txid) const { return false; } +bool CCoinsView::GetCoins(const COutPoint &outpoint, Coin &coin) const { return false; } +bool CCoinsView::HaveCoins(const COutPoint &outpoint) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; } CCoinsViewCursor *CCoinsView::Cursor() const { return 0; } CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } -bool CCoinsViewBacked::GetCoins(const uint256 &txid, CCoins &coins) const { return base->GetCoins(txid, coins); } -bool CCoinsViewBacked::HaveCoins(const uint256 &txid) const { return base->HaveCoins(txid); } +bool CCoinsViewBacked::GetCoins(const COutPoint &outpoint, Coin &coin) const { return base->GetCoins(outpoint, coin); } +bool CCoinsViewBacked::HaveCoins(const COutPoint &outpoint) const { return base->HaveCoins(outpoint); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); } CCoinsViewCursor *CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } -SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} +SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} -CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), cachedCoinsUsage(0) { } +CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), cachedCoinsUsage(0) {} size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; } -CCoinsMap::iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { - CCoinsMap::iterator it = cacheCoins.find(txid); +CCoinsMap::iterator CCoinsViewCache::FetchCoins(const COutPoint &outpoint) const { + CCoinsMap::iterator it = cacheCoins.find(outpoint); if (it != cacheCoins.end()) return it; - CCoins tmp; - if (!base->GetCoins(txid, tmp)) + Coin tmp; + if (!base->GetCoins(outpoint, tmp)) return cacheCoins.end(); - CCoinsMap::iterator ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())).first; - tmp.swap(ret->second.coins); + CCoinsMap::iterator ret = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::forward_as_tuple(std::move(tmp))).first; if (ret->second.coins.IsPruned()) { - // The parent only has an empty entry for this txid; we can consider our + // The parent only has an empty entry for this outpoint; we can consider our // version as fresh. ret->second.flags = CCoinsCacheEntry::FRESH; } @@ -84,10 +83,10 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { return ret; } -bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { - CCoinsMap::const_iterator it = FetchCoins(txid); +bool CCoinsViewCache::GetCoins(const COutPoint &outpoint, Coin &coin) const { + CCoinsMap::const_iterator it = FetchCoins(outpoint); if (it != cacheCoins.end()) { - coins = it->second.coins; + coin = it->second.coins; return true; } return false; @@ -98,23 +97,18 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi if (coin.out.scriptPubKey.IsUnspendable()) return; CCoinsMap::iterator it; bool inserted; - std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint.hash), std::tuple<>()); + std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>()); bool fresh = false; if (!inserted) { cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); } if (!possible_overwrite) { - if (it->second.coins.IsAvailable(outpoint.n)) { + if (!it->second.coins.IsPruned()) { throw std::logic_error("Adding new coin that replaces non-pruned entry"); } - fresh = it->second.coins.IsPruned() && !(it->second.flags & CCoinsCacheEntry::DIRTY); + fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY); } - if (it->second.coins.vout.size() <= outpoint.n) { - it->second.coins.vout.resize(outpoint.n + 1); - } - it->second.coins.vout[outpoint.n] = std::move(coin.out); - it->second.coins.nHeight = coin.nHeight; - it->second.coins.fCoinBase = coin.fCoinBase; + it->second.coins = std::move(coin); it->second.flags |= CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0); cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); } @@ -130,58 +124,38 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { } void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { - CCoinsMap::iterator it = FetchCoins(outpoint.hash); + CCoinsMap::iterator it = FetchCoins(outpoint); if (it == cacheCoins.end()) return; cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); - if (moveout && it->second.coins.IsAvailable(outpoint.n)) { - *moveout = Coin(it->second.coins.vout[outpoint.n], it->second.coins.nHeight, it->second.coins.fCoinBase); + if (moveout) { + *moveout = std::move(it->second.coins); } - it->second.coins.Spend(outpoint.n); // Ignore return value: SpendCoin has no effect if no UTXO found. - if (it->second.coins.IsPruned() && it->second.flags & CCoinsCacheEntry::FRESH) { + if (it->second.flags & CCoinsCacheEntry::FRESH) { cacheCoins.erase(it); } else { - cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); it->second.flags |= CCoinsCacheEntry::DIRTY; - } -} - -const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { - CCoinsMap::const_iterator it = FetchCoins(txid); - if (it == cacheCoins.end()) { - return NULL; - } else { - return &it->second.coins; + it->second.coins.Clear(); } } static const Coin coinEmpty; -const Coin CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { - CCoinsMap::const_iterator it = FetchCoins(outpoint.hash); - if (it == cacheCoins.end() || !it->second.coins.IsAvailable(outpoint.n)) { +const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { + CCoinsMap::const_iterator it = FetchCoins(outpoint); + if (it == cacheCoins.end()) { return coinEmpty; } else { - return Coin(it->second.coins.vout[outpoint.n], it->second.coins.nHeight, it->second.coins.fCoinBase); + return it->second.coins; } } - -bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { - CCoinsMap::const_iterator it = FetchCoins(txid); - // We're using vtx.empty() instead of IsPruned here for performance reasons, - // as we only care about the case where a transaction was replaced entirely - // in a reorganization (which wipes vout entirely, as opposed to spending - // which just cleans individual outputs). - return (it != cacheCoins.end() && !it->second.coins.vout.empty()); -} - bool CCoinsViewCache::HaveCoins(const COutPoint &outpoint) const { - CCoinsMap::const_iterator it = FetchCoins(outpoint.hash); - return (it != cacheCoins.end() && it->second.coins.IsAvailable(outpoint.n)); + CCoinsMap::const_iterator it = FetchCoins(outpoint); + return (it != cacheCoins.end() && !it->second.coins.IsPruned()); } -bool CCoinsViewCache::HaveCoinsInCache(const uint256 &txid) const { - CCoinsMap::const_iterator it = cacheCoins.find(txid); +bool CCoinsViewCache::HaveCoinsInCache(const COutPoint &outpoint) const { + CCoinsMap::const_iterator it = cacheCoins.find(outpoint); return it != cacheCoins.end(); } @@ -206,7 +180,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn // Otherwise we will need to create it in the parent // and move the data up and mark it as dirty CCoinsCacheEntry& entry = cacheCoins[it->first]; - entry.coins.swap(it->second.coins); + entry.coins = std::move(it->second.coins); cachedCoinsUsage += entry.coins.DynamicMemoryUsage(); entry.flags = CCoinsCacheEntry::DIRTY; // We can mark it FRESH in the parent if it was FRESH in the child @@ -233,7 +207,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn } else { // A normal modification. cachedCoinsUsage -= itUs->second.coins.DynamicMemoryUsage(); - itUs->second.coins.swap(it->second.coins); + itUs->second.coins = std::move(it->second.coins); cachedCoinsUsage += itUs->second.coins.DynamicMemoryUsage(); itUs->second.flags |= CCoinsCacheEntry::DIRTY; // NOTE: It is possible the child has a FRESH flag here in @@ -258,7 +232,7 @@ bool CCoinsViewCache::Flush() { return fOk; } -void CCoinsViewCache::Uncache(const uint256& hash) +void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); if (it != cacheCoins.end() && it->second.flags == 0) { @@ -273,9 +247,9 @@ unsigned int CCoinsViewCache::GetCacheSize() const { const CTxOut &CCoinsViewCache::GetOutputFor(const CTxIn& input) const { - const CCoins* coins = AccessCoins(input.prevout.hash); - assert(coins && coins->IsAvailable(input.prevout.n)); - return coins->vout[input.prevout.n]; + const Coin& coin = AccessCoin(input.prevout); + assert(!coin.IsPruned()); + return coin.out; } CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const @@ -294,9 +268,7 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const { if (!tx.IsCoinBase()) { for (unsigned int i = 0; i < tx.vin.size(); i++) { - const COutPoint &prevout = tx.vin[i].prevout; - const CCoins* coins = AccessCoins(prevout.hash); - if (!coins || !coins->IsAvailable(prevout.n)) { + if (!HaveCoins(tx.vin[i].prevout)) { return false; } } @@ -304,13 +276,9 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } -CCoinsViewCursor::~CCoinsViewCursor() -{ -} - static const size_t MAX_OUTPUTS_PER_BLOCK = MAX_BLOCK_BASE_SIZE / ::GetSerializeSize(CTxOut(), SER_NETWORK, PROTOCOL_VERSION); // TODO: merge with similar definition in undo.h. -const Coin AccessByTxid(const CCoinsViewCache& view, const uint256& txid) +const Coin& AccessByTxid(const CCoinsViewCache& view, const uint256& txid) { COutPoint iter(txid, 0); while (iter.n < MAX_OUTPUTS_PER_BLOCK) { diff --git a/src/coins.h b/src/coins.h index fcf7c47a5..ae85a199c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -299,28 +299,28 @@ public: } }; -class SaltedTxidHasher +class SaltedOutpointHasher { private: /** Salt */ const uint64_t k0, k1; public: - SaltedTxidHasher(); + SaltedOutpointHasher(); /** * This *must* return size_t. With Boost 1.46 on 32-bit systems the * unordered_map will behave unpredictably if the custom hasher returns a * uint64_t, resulting in failures when syncing the chain (#4634). */ - size_t operator()(const uint256& txid) const { - return SipHashUint256(k0, k1, txid); + size_t operator()(const COutPoint& id) const { + return SipHashUint256Extra(k0, k1, id.hash, id.n); } }; struct CCoinsCacheEntry { - CCoins coins; // The actual cached data. + Coin coins; // The actual cached data. unsigned char flags; enum Flags { @@ -333,20 +333,21 @@ struct CCoinsCacheEntry */ }; - CCoinsCacheEntry() : coins(), flags(0) {} + CCoinsCacheEntry() : flags(0) {} + explicit CCoinsCacheEntry(Coin&& coin_) : coins(std::move(coin_)), flags(0) {} }; -typedef std::unordered_map CCoinsMap; +typedef std::unordered_map CCoinsMap; /** Cursor for iterating over CoinsView state */ class CCoinsViewCursor { public: CCoinsViewCursor(const uint256 &hashBlockIn): hashBlock(hashBlockIn) {} - virtual ~CCoinsViewCursor(); + virtual ~CCoinsViewCursor() {} - virtual bool GetKey(uint256 &key) const = 0; - virtual bool GetValue(CCoins &coins) const = 0; + virtual bool GetKey(COutPoint &key) const = 0; + virtual bool GetValue(Coin &coin) const = 0; virtual unsigned int GetValueSize() const = 0; virtual bool Valid() const = 0; @@ -362,17 +363,17 @@ private: class CCoinsView { public: - //! Retrieve the CCoins (unspent transaction outputs) for a given txid - virtual bool GetCoins(const uint256 &txid, CCoins &coins) const; + //! Retrieve the Coin (unspent transaction output) for a given outpoint. + virtual bool GetCoins(const COutPoint &outpoint, Coin &coin) const; - //! Just check whether we have data for a given txid. - //! This may (but cannot always) return true for fully spent transactions - virtual bool HaveCoins(const uint256 &txid) const; + //! Just check whether we have data for a given outpoint. + //! This may (but cannot always) return true for spent outputs. + virtual bool HaveCoins(const COutPoint &outpoint) const; //! Retrieve the block hash whose state this CCoinsView currently represents virtual uint256 GetBestBlock() const; - //! Do a bulk modification (multiple CCoins changes + BestBlock change). + //! Do a bulk modification (multiple Coin changes + BestBlock change). //! The passed mapCoins can be modified. virtual bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); @@ -395,12 +396,12 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); - bool GetCoins(const uint256 &txid, CCoins &coins) const; - bool HaveCoins(const uint256 &txid) const; - uint256 GetBestBlock() const; + bool GetCoins(const COutPoint &outpoint, Coin &coin) const override; + bool HaveCoins(const COutPoint &outpoint) const override; + uint256 GetBestBlock() const override; void SetBackend(CCoinsView &viewIn); - bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); - CCoinsViewCursor *Cursor() const; + bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; + CCoinsViewCursor *Cursor() const override; size_t EstimateSize() const override; }; @@ -416,41 +417,32 @@ protected: mutable uint256 hashBlock; mutable CCoinsMap cacheCoins; - /* Cached dynamic memory usage for the inner CCoins objects. */ + /* Cached dynamic memory usage for the inner Coin objects. */ mutable size_t cachedCoinsUsage; public: CCoinsViewCache(CCoinsView *baseIn); // Standard CCoinsView methods - bool GetCoins(const uint256 &txid, CCoins &coins) const; - bool HaveCoins(const uint256 &txid) const; + bool GetCoins(const COutPoint &outpoint, Coin &coin) const; bool HaveCoins(const COutPoint &outpoint) const; uint256 GetBestBlock() const; void SetBestBlock(const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); /** - * Check if we have the given tx already loaded in this cache. + * Check if we have the given utxo already loaded in this cache. * The semantics are the same as HaveCoins(), but no calls to * the backing CCoinsView are made. */ - bool HaveCoinsInCache(const uint256 &txid) const; + bool HaveCoinsInCache(const COutPoint &outpoint) const; /** - * Return a pointer to CCoins in the cache, or NULL if not found. This is + * Return a reference to Coin in the cache, or a pruned one if not found. This is * more efficient than GetCoins. Modifications to other cache entries are * allowed while accessing the returned pointer. */ - const CCoins* AccessCoins(const uint256 &txid) const; - - /** - * Return a copy of a Coin in the cache, or a pruned one if not found. This is - * more efficient than GetCoins. Modifications to other cache entries are - * allowed while accessing the returned pointer. - * TODO: return a reference to a Coin after changing CCoinsViewCache storage. - */ - const Coin AccessCoin(const COutPoint &output) const; + const Coin& AccessCoin(const COutPoint &output) const; /** * Add a coin. Set potential_overwrite to true if a non-pruned version may @@ -473,12 +465,12 @@ public: bool Flush(); /** - * Removes the transaction with the given hash from the cache, if it is + * Removes the UTXO with the given outpoint from the cache, if it is * not modified. */ - void Uncache(const uint256 &txid); + void Uncache(const COutPoint &outpoint); - //! Calculate the size of the cache (in number of transactions) + //! Calculate the size of the cache (in number of transaction outputs) unsigned int GetCacheSize() const; //! Calculate the size of the cache (in bytes) @@ -500,7 +492,7 @@ public: const CTxOut &GetOutputFor(const CTxIn& input) const; private: - CCoinsMap::iterator FetchCoins(const uint256 &txid) const; + CCoinsMap::iterator FetchCoins(const COutPoint &outpoint) const; /** * By making the copy constructor private, we prevent accidentally using it when one intends to create a cache on top of a base cache. @@ -515,6 +507,6 @@ private: void AddCoins(CCoinsViewCache& cache, const CTransaction& tx, int nHeight); //! Utility function to find any unspent output with a given txid. -const Coin AccessByTxid(const CCoinsViewCache& cache, const uint256& txid); +const Coin& AccessByTxid(const CCoinsViewCache& cache, const uint256& txid); #endif // BITCOIN_COINS_H diff --git a/src/init.cpp b/src/init.cpp index 3bbdb16c3..4420b6f65 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -146,9 +146,9 @@ class CCoinsViewErrorCatcher : public CCoinsViewBacked { public: CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {} - bool GetCoins(const uint256 &txid, CCoins &coins) const { + bool GetCoins(const COutPoint &outpoint, Coin &coin) const override { try { - return CCoinsViewBacked::GetCoins(txid, coins); + return CCoinsViewBacked::GetCoins(outpoint, coin); } catch(const std::runtime_error& e) { uiInterface.ThreadSafeMessageBox(_("Error reading from database, shutting down."), "", CClientUIInterface::MSG_ERROR); LogPrintf("Error reading from database: %s\n", e.what()); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3102c2ef9..1c99d047e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -911,12 +911,11 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) recentRejects->reset(); } - // Use pcoinsTip->HaveCoinsInCache as a quick approximation to exclude - // requesting or processing some txs which have already been included in a block return recentRejects->contains(inv.hash) || mempool.exists(inv.hash) || mapOrphanTransactions.count(inv.hash) || - pcoinsTip->HaveCoinsInCache(inv.hash); + pcoinsTip->HaveCoinsInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 + pcoinsTip->HaveCoinsInCache(COutPoint(inv.hash, 1)); } case MSG_BLOCK: case MSG_WITNESS_BLOCK: diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index d81188895..78317c112 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -293,13 +293,12 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco { COutPoint prevout = txin.prevout; - CCoins prev; - if(pcoinsTip->GetCoins(prevout.hash, prev)) + Coin prev; + if(pcoinsTip->GetCoins(prevout, prev)) { - if (prevout.n < prev.vout.size()) { strHTML += "
  • "; - const CTxOut &vout = prev.vout[prevout.n]; + const CTxOut &vout = prev.out; CTxDestination address; if (ExtractDestination(vout.scriptPubKey, address)) { diff --git a/src/rest.cpp b/src/rest.cpp index 4a7101009..16c8c4f52 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -47,6 +47,9 @@ struct CCoin { ADD_SERIALIZE_METHODS; + CCoin() : nHeight(0) {} + CCoin(Coin&& in) : nHeight(in.nHeight), out(std::move(in.out)) {} + template inline void SerializationOp(Stream& s, Operation ser_action) { @@ -509,20 +512,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) view.SetBackend(viewMempool); // switch cache backend to db+mempool in case user likes to query mempool for (size_t i = 0; i < vOutPoints.size(); i++) { - CCoins coins; - uint256 hash = vOutPoints[i].hash; bool hit = false; - if (view.GetCoins(hash, coins)) { - if (coins.IsAvailable(vOutPoints[i].n) && !mempool.isSpent(vOutPoints[i])) { - hit = true; - // 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.nHeight = coins.nHeight; - coin.out = coins.vout.at(vOutPoints[i].n); - assert(!coin.out.IsNull()); - outs.push_back(coin); - } + Coin coin; + if (view.GetCoins(vOutPoints[i], coin) && !mempool.isSpent(vOutPoints[i])) { + hit = true; + outs.emplace_back(std::move(coin)); } hits.push_back(hit); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 3e9b1a1b1..77f1dcb21 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -816,24 +816,27 @@ static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) stats.nHeight = mapBlockIndex.find(stats.hashBlock)->second->nHeight; } ss << stats.hashBlock; + uint256 prevkey; + std::map outputs; while (pcursor->Valid()) { boost::this_thread::interruption_point(); - uint256 key; - CCoins coins; - if (pcursor->GetKey(key) && pcursor->GetValue(coins)) { - std::map outputs; - for (unsigned int i=0; iGetKey(key) && pcursor->GetValue(coin)) { + if (!outputs.empty() && key.hash != prevkey) { + ApplyStats(stats, ss, prevkey, outputs); + outputs.clear(); } - ApplyStats(stats, ss, key, outputs); + prevkey = key.hash; + outputs[key.n] = std::move(coin); } else { return error("%s: unable to read value", __func__); } pcursor->Next(); } + if (!outputs.empty()) { + ApplyStats(stats, ss, prevkey, outputs); + } stats.hashSerialized = ss.GetHash(); stats.nDiskSize = view->EstimateSize(); return true; @@ -973,35 +976,37 @@ UniValue gettxout(const JSONRPCRequest& request) std::string strHash = request.params[0].get_str(); uint256 hash(uint256S(strHash)); int n = request.params[1].get_int(); + COutPoint out(hash, n); bool fMempool = true; if (request.params.size() > 2) fMempool = request.params[2].get_bool(); - CCoins coins; + Coin coin; if (fMempool) { LOCK(mempool.cs); CCoinsViewMemPool view(pcoinsTip, mempool); - if (!view.GetCoins(hash, coins) || mempool.isSpent(COutPoint(hash, n))) // TODO: this should be done by the CCoinsViewMemPool + if (!view.GetCoins(out, coin) || mempool.isSpent(out)) { // TODO: filtering spent coins should be done by the CCoinsViewMemPool return NullUniValue; + } } else { - if (!pcoinsTip->GetCoins(hash, coins)) + if (!pcoinsTip->GetCoins(out, coin)) { return NullUniValue; + } } - if (n<0 || (unsigned int)n>=coins.vout.size() || coins.vout[n].IsNull()) - return NullUniValue; BlockMap::iterator it = mapBlockIndex.find(pcoinsTip->GetBestBlock()); CBlockIndex *pindex = it->second; ret.push_back(Pair("bestblock", pindex->GetBlockHash().GetHex())); - if ((unsigned int)coins.nHeight == MEMPOOL_HEIGHT) + if (coin.nHeight == MEMPOOL_HEIGHT) { ret.push_back(Pair("confirmations", 0)); - else - ret.push_back(Pair("confirmations", pindex->nHeight - coins.nHeight + 1)); - ret.push_back(Pair("value", ValueFromAmount(coins.vout[n].nValue))); + } else { + ret.push_back(Pair("confirmations", (int64_t)(pindex->nHeight - coin.nHeight + 1))); + } + ret.push_back(Pair("value", ValueFromAmount(coin.out.nValue))); UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(coins.vout[n].scriptPubKey, o, true); + ScriptPubKeyToUniv(coin.out.scriptPubKey, o, true); ret.push_back(Pair("scriptPubKey", o)); - ret.push_back(Pair("coinbase", coins.fCoinBase)); + ret.push_back(Pair("coinbase", (bool)coin.fCoinBase)); return ret; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3f7b6adea..8ecbf9ede 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -219,9 +219,10 @@ UniValue gettxoutproof(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); pblockindex = mapBlockIndex[hashBlock]; } else { - CCoins coins; - if (pcoinsTip->GetCoins(oneTxid, coins) && coins.nHeight > 0 && coins.nHeight <= chainActive.Height()) - pblockindex = chainActive[coins.nHeight]; + const Coin& coin = AccessByTxid(*pcoinsTip, oneTxid); + if (!coin.IsPruned() && coin.nHeight > 0 && coin.nHeight <= chainActive.Height()) { + pblockindex = chainActive[coin.nHeight]; + } } if (pblockindex == NULL) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 3ad2625ba..8d519c644 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -34,27 +34,27 @@ bool operator==(const Coin &a, const Coin &b) { class CCoinsViewTest : public CCoinsView { uint256 hashBestBlock_; - std::map map_; + std::map map_; public: - bool GetCoins(const uint256& txid, CCoins& coins) const + bool GetCoins(const COutPoint& outpoint, Coin& coin) const { - std::map::const_iterator it = map_.find(txid); + std::map::const_iterator it = map_.find(outpoint); if (it == map_.end()) { return false; } - coins = it->second; - if (coins.IsPruned() && insecure_rand() % 2 == 0) { + coin = it->second; + if (coin.IsPruned() && insecure_rand() % 2 == 0) { // Randomly return false in case of an empty entry. return false; } return true; } - bool HaveCoins(const uint256& txid) const + bool HaveCoins(const COutPoint& outpoint) const { - CCoins coins; - return GetCoins(txid, coins); + Coin coin; + return GetCoins(outpoint, coin); } uint256 GetBestBlock() const { return hashBestBlock_; } @@ -106,7 +106,7 @@ static const unsigned int NUM_SIMULATION_ITERATIONS = 40000; // This is a large randomized insert/remove simulation test on a variable-size // stack of caches on top of CCoinsViewTest. // -// It will randomly create/update/delete CCoins entries to a tip of caches, with +// It will randomly create/update/delete Coin entries to a tip of caches, with // txids picked from a limited list of random 256-bit hashes. Occasionally, a // new tip is added to the stack of caches, or the tip is flushed and removed. // @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) bool missed_an_entry = false; // A simple map to track what we expect the cache stack to represent. - std::map result; + std::map result; // The cache stack. CCoinsViewTest base; // A CCoinsViewTest at the bottom. @@ -142,39 +142,38 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) // Do a random modification. { uint256 txid = txids[insecure_rand() % txids.size()]; // txid we're going to modify in this iteration. - CCoins& coins = result[txid]; + Coin& coin = result[COutPoint(txid, 0)]; const Coin& entry = stack.back()->AccessCoin(COutPoint(txid, 0)); - BOOST_CHECK((entry.IsPruned() && coins.IsPruned()) || entry == Coin(coins.vout[0], coins.nHeight, coins.fCoinBase)); + BOOST_CHECK(coin == entry); - if (insecure_rand() % 5 == 0 || coins.IsPruned()) { - if (coins.IsPruned()) { + if (insecure_rand() % 5 == 0 || coin.IsPruned()) { + if (coin.IsPruned()) { added_an_entry = true; } else { updated_an_entry = true; } - coins.vout.resize(1); - coins.vout[0].nValue = insecure_rand(); + coin.out.nValue = insecure_rand(); + coin.nHeight = 1; } else { - coins.Clear(); + coin.Clear(); removed_an_entry = true; } - if (coins.IsPruned()) { + if (coin.IsPruned()) { stack.back()->SpendCoin(COutPoint(txid, 0)); } else { - stack.back()->AddCoin(COutPoint(txid, 0), Coin(coins.vout[0], coins.nHeight, coins.fCoinBase), true); + stack.back()->AddCoin(COutPoint(txid, 0), Coin(coin), true); } } // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { - for (std::map::iterator it = result.begin(); it != result.end(); it++) { - const CCoins* coins = stack.back()->AccessCoins(it->first); - if (coins) { - BOOST_CHECK(*coins == it->second); - found_an_entry = true; - } else { - BOOST_CHECK(it->second.IsPruned()); + for (auto it = result.begin(); it != result.end(); it++) { + const Coin& coin = stack.back()->AccessCoin(it->first); + BOOST_CHECK(coin == it->second); + if (coin.IsPruned()) { missed_an_entry = true; + } else { + found_an_entry = true; } } BOOST_FOREACH(const CCoinsViewCacheTest *test, stack) { @@ -229,19 +228,19 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) BOOST_CHECK(missed_an_entry); } -typedef std::tuple TxData; // Store of all necessary tx and undo data for next test -std::map alltxs; +typedef std::map> UtxoData; +UtxoData utxoData; -TxData &FindRandomFrom(const std::set &txidset) { - assert(txidset.size()); - std::set::iterator txIt = txidset.lower_bound(GetRandHash()); - if (txIt == txidset.end()) { - txIt = txidset.begin(); +UtxoData::iterator FindRandomFrom(const std::set &utxoSet) { + assert(utxoSet.size()); + auto utxoSetIt = utxoSet.lower_bound(COutPoint(GetRandHash(), 0)); + if (utxoSetIt == utxoSet.end()) { + utxoSetIt = utxoSet.begin(); } - std::map::iterator txdit = alltxs.find(*txIt); - assert(txdit != alltxs.end()); - return txdit->second; + auto utxoDataIt = utxoData.find(*utxoSetIt); + assert(utxoDataIt != utxoData.end()); + return utxoDataIt; } @@ -254,7 +253,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) { bool spent_a_duplicate_coinbase = false; // A simple map to track what we expect the cache stack to represent. - std::map result; + std::map result; // The cache stack. CCoinsViewTest base; // A CCoinsViewTest at the bottom. @@ -262,10 +261,10 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) stack.push_back(new CCoinsViewCacheTest(&base)); // Start with one cache. // Track the txids we've used in various sets - std::set coinbaseids; - std::set disconnectedids; - std::set duplicateids; - std::set utxoset; + std::set coinbaseids; + std::set disconnectedids; + std::set duplicateids; + std::set utxoset; for (unsigned int i = 0; i < NUM_SIMULATION_ITERATIONS; i++) { uint32_t randiter = insecure_rand(); @@ -277,22 +276,22 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) tx.vout.resize(1); tx.vout[0].nValue = i; //Keep txs unique unless intended to duplicate unsigned int height = insecure_rand(); - CCoins oldcoins; + Coin oldcoins; // 2/20 times create a new coinbase if (randiter % 20 < 2 || coinbaseids.size() < 10) { // 1/10 of those times create a duplicate coinbase if (insecure_rand() % 10 == 0 && coinbaseids.size()) { - TxData &txd = FindRandomFrom(coinbaseids); + auto utxod = FindRandomFrom(coinbaseids); // Reuse the exact same coinbase - tx = std::get<0>(txd); + tx = std::get<0>(utxod->second); // shouldn't be available for reconnection if its been duplicated - disconnectedids.erase(tx.GetHash()); + disconnectedids.erase(utxod->first); - duplicateids.insert(tx.GetHash()); + duplicateids.insert(utxod->first); } else { - coinbaseids.insert(tx.GetHash()); + coinbaseids.insert(COutPoint(tx.GetHash(), 0)); } assert(CTransaction(tx).IsCoinBase()); } @@ -300,85 +299,82 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // 17/20 times reconnect previous or add a regular tx else { - uint256 prevouthash; + COutPoint prevout; // 1/20 times reconnect a previously disconnected tx if (randiter % 20 == 2 && disconnectedids.size()) { - TxData &txd = FindRandomFrom(disconnectedids); - tx = std::get<0>(txd); - prevouthash = tx.vin[0].prevout.hash; - if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevouthash)) { - disconnectedids.erase(tx.GetHash()); + auto utxod = FindRandomFrom(disconnectedids); + tx = std::get<0>(utxod->second); + prevout = tx.vin[0].prevout; + if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevout)) { + disconnectedids.erase(utxod->first); continue; } // If this tx is already IN the UTXO, then it must be a coinbase, and it must be a duplicate - if (utxoset.count(tx.GetHash())) { + if (utxoset.count(utxod->first)) { assert(CTransaction(tx).IsCoinBase()); - assert(duplicateids.count(tx.GetHash())); + assert(duplicateids.count(utxod->first)); } - disconnectedids.erase(tx.GetHash()); + disconnectedids.erase(utxod->first); } // 16/20 times create a regular tx else { - TxData &txd = FindRandomFrom(utxoset); - prevouthash = std::get<0>(txd).GetHash(); + auto utxod = FindRandomFrom(utxoset); + prevout = utxod->first; // Construct the tx to spend the coins of prevouthash - tx.vin[0].prevout.hash = prevouthash; - tx.vin[0].prevout.n = 0; + tx.vin[0].prevout = prevout; assert(!CTransaction(tx).IsCoinBase()); } // In this simple test coins only have two states, spent or unspent, save the unspent state to restore - oldcoins = result[prevouthash]; + oldcoins = result[prevout]; // Update the expected result of prevouthash to know these coins are spent - result[prevouthash].Clear(); + result[prevout].Clear(); - utxoset.erase(prevouthash); + utxoset.erase(prevout); // The test is designed to ensure spending a duplicate coinbase will work properly // if that ever happens and not resurrect the previously overwritten coinbase - if (duplicateids.count(prevouthash)) { + if (duplicateids.count(prevout)) { spent_a_duplicate_coinbase = true; } } // Update the expected result to know about the new output coins - result[tx.GetHash()].FromTx(tx, height); + assert(tx.vout.size() == 1); + const COutPoint outpoint(tx.GetHash(), 0); + result[outpoint] = Coin(tx.vout[0], height, CTransaction(tx).IsCoinBase()); // Call UpdateCoins on the top cache CTxUndo undo; UpdateCoins(tx, *(stack.back()), undo, height); // Update the utxo set for future spends - utxoset.insert(tx.GetHash()); + utxoset.insert(outpoint); // Track this tx and undo info to use later - alltxs.insert(std::make_pair(tx.GetHash(),std::make_tuple(tx,undo,oldcoins))); + utxoData.emplace(outpoint, std::make_tuple(tx,undo,oldcoins)); } else if (utxoset.size()) { //1/20 times undo a previous transaction - TxData &txd = FindRandomFrom(utxoset); + auto utxod = FindRandomFrom(utxoset); - CTransaction &tx = std::get<0>(txd); - CTxUndo &undo = std::get<1>(txd); - CCoins &origcoins = std::get<2>(txd); - - uint256 undohash = tx.GetHash(); + CTransaction &tx = std::get<0>(utxod->second); + CTxUndo &undo = std::get<1>(utxod->second); + Coin &origcoins = std::get<2>(utxod->second); // Update the expected result // Remove new outputs - result[undohash].Clear(); + result[utxod->first].Clear(); // If not coinbase restore prevout if (!tx.IsCoinBase()) { - result[tx.vin[0].prevout.hash] = origcoins; + result[tx.vin[0].prevout] = origcoins; } // Disconnect the tx from the current UTXO // See code in DisconnectBlock // remove outputs - { - stack.back()->SpendCoin(COutPoint(undohash, 0)); - } + stack.back()->SpendCoin(utxod->first); // restore inputs if (!tx.IsCoinBase()) { const COutPoint &out = tx.vin[0].prevout; @@ -386,23 +382,19 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) ApplyTxInUndo(std::move(coin), *(stack.back()), out); } // Store as a candidate for reconnection - disconnectedids.insert(undohash); + disconnectedids.insert(utxod->first); // Update the utxoset - utxoset.erase(undohash); + utxoset.erase(utxod->first); if (!tx.IsCoinBase()) - utxoset.insert(tx.vin[0].prevout.hash); + utxoset.insert(tx.vin[0].prevout); } // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { - for (std::map::iterator it = result.begin(); it != result.end(); it++) { - const CCoins* coins = stack.back()->AccessCoins(it->first); - if (coins) { - BOOST_CHECK(*coins == it->second); - } else { - BOOST_CHECK(it->second.IsPruned()); - } + for (auto it = result.begin(); it != result.end(); it++) { + const Coin& coin = stack.back()->AccessCoin(it->first); + BOOST_CHECK(coin == it->second); } } @@ -443,50 +435,36 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) BOOST_AUTO_TEST_CASE(ccoins_serialization) { // Good example - CDataStream ss1(ParseHex("0104835800816115944e077fe7c803cfa57f29b36bf87c1d358bb85e"), SER_DISK, CLIENT_VERSION); - CCoins cc1; + CDataStream ss1(ParseHex("97f23c835800816115944e077fe7c803cfa57f29b36bf87c1d35"), SER_DISK, CLIENT_VERSION); + Coin cc1; ss1 >> cc1; BOOST_CHECK_EQUAL(cc1.fCoinBase, false); BOOST_CHECK_EQUAL(cc1.nHeight, 203998); - BOOST_CHECK_EQUAL(cc1.vout.size(), 2); - BOOST_CHECK_EQUAL(cc1.IsAvailable(0), false); - BOOST_CHECK_EQUAL(cc1.IsAvailable(1), true); - BOOST_CHECK_EQUAL(cc1.vout[1].nValue, 60000000000ULL); - BOOST_CHECK_EQUAL(HexStr(cc1.vout[1].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("816115944e077fe7c803cfa57f29b36bf87c1d35")))))); + BOOST_CHECK_EQUAL(cc1.out.nValue, 60000000000ULL); + BOOST_CHECK_EQUAL(HexStr(cc1.out.scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("816115944e077fe7c803cfa57f29b36bf87c1d35")))))); // Good example - CDataStream ss2(ParseHex("0109044086ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4eebbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa486af3b"), SER_DISK, CLIENT_VERSION); - CCoins cc2; + CDataStream ss2(ParseHex("8ddf77bbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa4"), SER_DISK, CLIENT_VERSION); + Coin cc2; ss2 >> cc2; BOOST_CHECK_EQUAL(cc2.fCoinBase, true); BOOST_CHECK_EQUAL(cc2.nHeight, 120891); - BOOST_CHECK_EQUAL(cc2.vout.size(), 17); - for (int i = 0; i < 17; i++) { - BOOST_CHECK_EQUAL(cc2.IsAvailable(i), i == 4 || i == 16); - } - BOOST_CHECK_EQUAL(cc2.vout[4].nValue, 234925952); - BOOST_CHECK_EQUAL(HexStr(cc2.vout[4].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("61b01caab50f1b8e9c50a5057eb43c2d9563a4ee")))))); - BOOST_CHECK_EQUAL(cc2.vout[16].nValue, 110397); - BOOST_CHECK_EQUAL(HexStr(cc2.vout[16].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4")))))); + BOOST_CHECK_EQUAL(cc2.out.nValue, 110397); + BOOST_CHECK_EQUAL(HexStr(cc2.out.scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4")))))); // Smallest possible example - CDataStream ssx(SER_DISK, CLIENT_VERSION); - BOOST_CHECK_EQUAL(HexStr(ssx.begin(), ssx.end()), ""); - - CDataStream ss3(ParseHex("0002000600"), SER_DISK, CLIENT_VERSION); - CCoins cc3; + CDataStream ss3(ParseHex("000006"), SER_DISK, CLIENT_VERSION); + Coin cc3; ss3 >> cc3; BOOST_CHECK_EQUAL(cc3.fCoinBase, false); BOOST_CHECK_EQUAL(cc3.nHeight, 0); - BOOST_CHECK_EQUAL(cc3.vout.size(), 1); - BOOST_CHECK_EQUAL(cc3.IsAvailable(0), true); - BOOST_CHECK_EQUAL(cc3.vout[0].nValue, 0); - BOOST_CHECK_EQUAL(cc3.vout[0].scriptPubKey.size(), 0); + BOOST_CHECK_EQUAL(cc3.out.nValue, 0); + BOOST_CHECK_EQUAL(cc3.out.scriptPubKey.size(), 0); // scriptPubKey that ends beyond the end of the stream - CDataStream ss4(ParseHex("0002000800"), SER_DISK, CLIENT_VERSION); + CDataStream ss4(ParseHex("000007"), SER_DISK, CLIENT_VERSION); try { - CCoins cc4; + Coin cc4; ss4 >> cc4; BOOST_CHECK_MESSAGE(false, "We should have thrown"); } catch (const std::ios_base::failure& e) { @@ -497,17 +475,16 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) uint64_t x = 3000000000ULL; tmp << VARINT(x); BOOST_CHECK_EQUAL(HexStr(tmp.begin(), tmp.end()), "8a95c0bb00"); - CDataStream ss5(ParseHex("0002008a95c0bb0000"), SER_DISK, CLIENT_VERSION); + CDataStream ss5(ParseHex("00008a95c0bb00"), SER_DISK, CLIENT_VERSION); try { - CCoins cc5; + Coin cc5; ss5 >> cc5; BOOST_CHECK_MESSAGE(false, "We should have thrown"); } catch (const std::ios_base::failure& e) { } } -const static uint256 TXID; -const static COutPoint OUTPOINT = {uint256(), 0}; +const static COutPoint OUTPOINT; const static CAmount PRUNED = -1; const static CAmount ABSENT = -2; const static CAmount FAIL = -3; @@ -522,15 +499,15 @@ const static auto FLAGS = {char(0), FRESH, DIRTY, char(DIRTY | FRESH)}; const static auto CLEAN_FLAGS = {char(0), FRESH}; const static auto ABSENT_FLAGS = {NO_ENTRY}; -void SetCoinsValue(CAmount value, CCoins& coins) +void SetCoinsValue(CAmount value, Coin& coin) { assert(value != ABSENT); - coins.Clear(); - assert(coins.IsPruned()); + coin.Clear(); + assert(coin.IsPruned()); if (value != PRUNED) { - coins.vout.emplace_back(); - coins.vout.back().nValue = value; - assert(!coins.IsPruned()); + coin.out.nValue = value; + coin.nHeight = 1; + assert(!coin.IsPruned()); } } @@ -544,24 +521,22 @@ size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags) CCoinsCacheEntry entry; entry.flags = flags; SetCoinsValue(value, entry.coins); - auto inserted = map.emplace(TXID, std::move(entry)); + auto inserted = map.emplace(OUTPOINT, std::move(entry)); assert(inserted.second); return inserted.first->second.coins.DynamicMemoryUsage(); } void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags) { - auto it = map.find(TXID); + auto it = map.find(OUTPOINT); if (it == map.end()) { value = ABSENT; flags = NO_ENTRY; } else { if (it->second.coins.IsPruned()) { - assert(it->second.coins.vout.size() == 0); value = PRUNED; } else { - assert(it->second.coins.vout.size() == 1); - value = it->second.coins.vout[0].nValue; + value = it->second.coins.out.nValue; } flags = it->second.flags; assert(flags != NO_ENTRY); diff --git a/src/test/test_bitcoin_fuzzy.cpp b/src/test/test_bitcoin_fuzzy.cpp index e11e46bb0..de1425160 100644 --- a/src/test/test_bitcoin_fuzzy.cpp +++ b/src/test/test_bitcoin_fuzzy.cpp @@ -168,8 +168,8 @@ int do_fuzz() { try { - CCoins block; - ds >> block; + Coin coin; + ds >> coin; } catch (const std::ios_base::failure& e) {return 0;} break; } diff --git a/src/txdb.cpp b/src/txdb.cpp index f139384a2..19c900250 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -14,6 +14,7 @@ #include +static const char DB_COIN = 'C'; static const char DB_COINS = 'c'; static const char DB_BLOCK_FILES = 'f'; static const char DB_TXINDEX = 't'; @@ -24,17 +25,40 @@ static const char DB_FLAG = 'F'; static const char DB_REINDEX_FLAG = 'R'; static const char DB_LAST_BLOCK = 'l'; +namespace { + +struct CoinsEntry { + COutPoint* outpoint; + char key; + CoinsEntry(const COutPoint* ptr) : outpoint(const_cast(ptr)), key(DB_COIN) {} + + template + void Serialize(Stream &s) const { + s << key; + s << outpoint->hash; + s << VARINT(outpoint->n); + } + + template + void Unserialize(Stream& s) { + s >> key; + s >> outpoint->hash; + s >> VARINT(outpoint->n); + } +}; + +} CCoinsViewDB::CCoinsViewDB(size_t nCacheSize, bool fMemory, bool fWipe) : db(GetDataDir() / "chainstate", nCacheSize, fMemory, fWipe, true) { } -bool CCoinsViewDB::GetCoins(const uint256 &txid, CCoins &coins) const { - return db.Read(std::make_pair(DB_COINS, txid), coins); +bool CCoinsViewDB::GetCoins(const COutPoint &outpoint, Coin &coin) const { + return db.Read(CoinsEntry(&outpoint), coin); } -bool CCoinsViewDB::HaveCoins(const uint256 &txid) const { - return db.Exists(std::make_pair(DB_COINS, txid)); +bool CCoinsViewDB::HaveCoins(const COutPoint &outpoint) const { + return db.Exists(CoinsEntry(&outpoint)); } uint256 CCoinsViewDB::GetBestBlock() const { @@ -50,10 +74,11 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { size_t changed = 0; for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { + CoinsEntry entry(&it->first); if (it->second.coins.IsPruned()) - batch.Erase(std::make_pair(DB_COINS, it->first)); + batch.Erase(entry); else - batch.Write(std::make_pair(DB_COINS, it->first), it->second.coins); + batch.Write(entry, it->second.coins); changed++; } count++; @@ -63,13 +88,14 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { if (!hashBlock.IsNull()) batch.Write(DB_BEST_BLOCK, hashBlock); - LogPrint(BCLog::COINDB, "Committing %u changed transactions (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count); - return db.WriteBatch(batch); + bool ret = db.WriteBatch(batch); + LogPrint(BCLog::COINDB, "Committed %u changed transaction outputs (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count); + return ret; } size_t CCoinsViewDB::EstimateSize() const { - return db.EstimateSize(DB_COINS, (char)(DB_COINS+1)); + return db.EstimateSize(DB_COIN, (char)(DB_COIN+1)); } CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(GetDataDir() / "blocks" / "index", nCacheSize, fMemory, fWipe) { @@ -101,29 +127,31 @@ CCoinsViewCursor *CCoinsViewDB::Cursor() const /* It seems that there are no "const iterators" for LevelDB. Since we only need read operations on it, use a const-cast to get around that restriction. */ - i->pcursor->Seek(DB_COINS); + i->pcursor->Seek(DB_COIN); // Cache key of first record if (i->pcursor->Valid()) { - i->pcursor->GetKey(i->keyTmp); + CoinsEntry entry(&i->keyTmp.second); + i->pcursor->GetKey(entry); + i->keyTmp.first = entry.key; } else { i->keyTmp.first = 0; // Make sure Valid() and GetKey() return false } return i; } -bool CCoinsViewDBCursor::GetKey(uint256 &key) const +bool CCoinsViewDBCursor::GetKey(COutPoint &key) const { // Return cached key - if (keyTmp.first == DB_COINS) { + if (keyTmp.first == DB_COIN) { key = keyTmp.second; return true; } return false; } -bool CCoinsViewDBCursor::GetValue(CCoins &coins) const +bool CCoinsViewDBCursor::GetValue(Coin &coin) const { - return pcursor->GetValue(coins); + return pcursor->GetValue(coin); } unsigned int CCoinsViewDBCursor::GetValueSize() const @@ -133,14 +161,18 @@ unsigned int CCoinsViewDBCursor::GetValueSize() const bool CCoinsViewDBCursor::Valid() const { - return keyTmp.first == DB_COINS; + return keyTmp.first == DB_COIN; } void CCoinsViewDBCursor::Next() { pcursor->Next(); - if (!pcursor->Valid() || !pcursor->GetKey(keyTmp)) + CoinsEntry entry(&keyTmp.second); + if (!pcursor->Valid() || !pcursor->GetKey(entry)) { keyTmp.first = 0; // Invalidate cached key after last record so that Valid() and GetKey() return false + } else { + keyTmp.first = entry.key; + } } bool CBlockTreeDB::WriteBatchSync(const std::vector >& fileInfo, int nLastFile, const std::vector& blockinfo) { diff --git a/src/txdb.h b/src/txdb.h index df164cb82..73011fe0f 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -73,11 +73,11 @@ protected: public: CCoinsViewDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); - bool GetCoins(const uint256 &txid, CCoins &coins) const; - bool HaveCoins(const uint256 &txid) const; - uint256 GetBestBlock() const; - bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); - CCoinsViewCursor *Cursor() const; + bool GetCoins(const COutPoint &outpoint, Coin &coin) const override; + bool HaveCoins(const COutPoint &outpoint) const override; + uint256 GetBestBlock() const override; + bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; + CCoinsViewCursor *Cursor() const override; size_t EstimateSize() const override; }; @@ -88,8 +88,8 @@ class CCoinsViewDBCursor: public CCoinsViewCursor public: ~CCoinsViewDBCursor() {} - bool GetKey(uint256 &key) const; - bool GetValue(CCoins &coins) const; + bool GetKey(COutPoint &key) const; + bool GetValue(Coin &coin) const; unsigned int GetValueSize() const; bool Valid() const; @@ -99,7 +99,7 @@ private: CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256 &hashBlockIn): CCoinsViewCursor(hashBlockIn), pcursor(pcursorIn) {} std::unique_ptr pcursor; - std::pair keyTmp; + std::pair keyTmp; friend class CCoinsViewDB; }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 51b93e92b..648b40a7a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -524,9 +524,9 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) continue; - const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); - if (nCheckFrequency != 0) assert(coins); - if (!coins || (coins->IsCoinBase() && ((signed long)nMemPoolHeight) - coins->nHeight < COINBASE_MATURITY)) { + const Coin &coin = pcoins->AccessCoin(txin.prevout); + if (nCheckFrequency != 0) assert(!coin.IsPruned()); + if (coin.IsPruned() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) { txToRemove.insert(it); break; } @@ -654,8 +654,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const parentSigOpCost += it2->GetSigOpCost(); } } else { - const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash); - assert(coins && coins->IsAvailable(txin.prevout.n)); + assert(pcoins->HaveCoins(txin.prevout)); } // Check whether its inputs are marked in mapNextTx. auto it3 = mapNextTx.find(txin.prevout); @@ -891,20 +890,24 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetCoins(const uint256 &txid, CCoins &coins) const { +bool CCoinsViewMemPool::GetCoins(const COutPoint &outpoint, Coin &coin) const { // If an entry in the mempool exists, always return that one, as it's guaranteed to never // conflict with the underlying cache, and it cannot have pruned entries (as it contains full) // transactions. First checking the underlying cache risks returning a pruned entry instead. - CTransactionRef ptx = mempool.get(txid); + CTransactionRef ptx = mempool.get(outpoint.hash); if (ptx) { - coins = CCoins(*ptx, MEMPOOL_HEIGHT); - return true; + if (outpoint.n < ptx->vout.size()) { + coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); + return true; + } else { + return false; + } } - return (base->GetCoins(txid, coins) && !coins.IsPruned()); + return (base->GetCoins(outpoint, coin) && !coin.IsPruned()); } -bool CCoinsViewMemPool::HaveCoins(const uint256 &txid) const { - return mempool.exists(txid) || base->HaveCoins(txid); +bool CCoinsViewMemPool::HaveCoins(const COutPoint &outpoint) const { + return mempool.exists(outpoint) || base->HaveCoins(outpoint); } size_t CTxMemPool::DynamicMemoryUsage() const { @@ -1015,7 +1018,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } } -void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining) { +void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining) { LOCK(cs); unsigned nTxnRemoved = 0; @@ -1046,11 +1049,10 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRe if (pvNoSpendsRemaining) { BOOST_FOREACH(const CTransaction& tx, txn) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { - if (exists(txin.prevout.hash)) - continue; - auto iter = mapNextTx.lower_bound(COutPoint(txin.prevout.hash, 0)); - if (iter == mapNextTx.end() || iter->first->hash != txin.prevout.hash) - pvNoSpendsRemaining->push_back(txin.prevout.hash); + if (exists(txin.prevout.hash)) continue; + if (!mapNextTx.count(txin.prevout)) { + pvNoSpendsRemaining->push_back(txin.prevout); + } } } } @@ -1067,3 +1069,5 @@ bool CTxMemPool::TransactionWithinChainLimit(const uint256& txid, size_t chainLi return it == mapTx.end() || (it->GetCountWithAncestors() < chainLimit && it->GetCountWithDescendants() < chainLimit); } + +SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} diff --git a/src/txmempool.h b/src/txmempool.h index 6547f64f7..c86859118 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -30,8 +30,8 @@ class CAutoFile; class CBlockIndex; -/** Fake height value used in CCoins to signify they are only in the memory pool (since 0.8) */ -static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; +/** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ +static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; struct LockPoints { @@ -321,6 +321,20 @@ enum class MemPoolRemovalReason { REPLACED //! Removed for replacement }; +class SaltedTxidHasher +{ +private: + /** Salt */ + const uint64_t k0, k1; + +public: + SaltedTxidHasher(); + + size_t operator()(const uint256& txid) const { + return SipHashUint256(k0, k1, txid); + } +}; + /** * CTxMemPool stores valid-according-to-the-current-best-chain transactions * that may be included in the next block. @@ -570,10 +584,10 @@ public: CFeeRate GetMinFee(size_t sizelimit) const; /** Remove transactions from the mempool until its dynamic size is <= sizelimit. - * pvNoSpendsRemaining, if set, will be populated with the list of transactions + * pvNoSpendsRemaining, if set, will be populated with the list of outpoints * which are not in mempool which no longer have any spends in this mempool. */ - void TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining=NULL); + void TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining=NULL); /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ int Expire(int64_t time); @@ -599,6 +613,13 @@ public: return (mapTx.count(hash) != 0); } + bool exists(const COutPoint& outpoint) const + { + LOCK(cs); + auto it = mapTx.find(outpoint.hash); + return (it != mapTx.end() && outpoint.n < it->GetTx().vout.size()); + } + CTransactionRef get(const uint256& hash) const; TxMempoolInfo info(const uint256& hash) const; std::vector infoAll() const; @@ -658,8 +679,8 @@ protected: public: CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); - bool GetCoins(const uint256 &txid, CCoins &coins) const; - bool HaveCoins(const uint256 &txid) const; + bool GetCoins(const COutPoint &outpoint, Coin &coin) const; + bool HaveCoins(const COutPoint &outpoint) const; }; #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/validation.cpp b/src/validation.cpp index b295a7f86..cf4dab420 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -268,15 +268,15 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool prevheights.resize(tx.vin.size()); for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { const CTxIn& txin = tx.vin[txinIndex]; - CCoins coins; - if (!viewMemPool.GetCoins(txin.prevout.hash, coins)) { + Coin coin; + if (!viewMemPool.GetCoins(txin.prevout, coin)) { return error("%s: Missing input", __func__); } - if (coins.nHeight == MEMPOOL_HEIGHT) { + if (coin.nHeight == MEMPOOL_HEIGHT) { // Assume all mempool transaction confirm in the next block prevheights[txinIndex] = tip->nHeight + 1; } else { - prevheights[txinIndex] = coins.nHeight; + prevheights[txinIndex] = coin.nHeight; } } lockPair = CalculateSequenceLocks(tx, flags, &prevheights, index); @@ -315,9 +315,9 @@ void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); } - std::vector vNoSpendsRemaining; + std::vector vNoSpendsRemaining; pool.TrimToSize(limit, &vNoSpendsRemaining); - BOOST_FOREACH(const uint256& removed, vNoSpendsRemaining) + BOOST_FOREACH(const COutPoint& removed, vNoSpendsRemaining) pcoinsTip->Uncache(removed); } @@ -344,7 +344,7 @@ static bool IsCurrentForFeeEstimation() bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, - bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& vHashTxnToUncache) + bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& vHashTxnToUncache) { const CTransaction& tx = *ptx; const uint256 hash = tx.GetHash(); @@ -437,29 +437,29 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C view.SetBackend(viewMemPool); // do we already have it? - bool fHadTxInCache = pcoinsTip->HaveCoinsInCache(hash); - if (view.HaveCoins(hash)) { - if (!fHadTxInCache) - vHashTxnToUncache.push_back(hash); - return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-known"); - } - - // do all inputs exist? - // Note that this does not check for the presence of actual outputs (see the next check for that), - // and only helps with filling in pfMissingInputs (to determine missing vs spent). - BOOST_FOREACH(const CTxIn txin, tx.vin) { - if (!pcoinsTip->HaveCoinsInCache(txin.prevout.hash)) - vHashTxnToUncache.push_back(txin.prevout.hash); - if (!view.HaveCoins(txin.prevout.hash)) { - if (pfMissingInputs) - *pfMissingInputs = true; - return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() + for (size_t out = 0; out < tx.vout.size(); out++) { + COutPoint outpoint(hash, out); + bool fHadTxInCache = pcoinsTip->HaveCoinsInCache(outpoint); + if (view.HaveCoins(outpoint)) { + if (!fHadTxInCache) { + vHashTxnToUncache.push_back(outpoint); + } + return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-known"); } } - // are the actual inputs available? - if (!view.HaveInputs(tx)) - return state.Invalid(false, REJECT_DUPLICATE, "bad-txns-inputs-spent"); + // do all inputs exist? + BOOST_FOREACH(const CTxIn txin, tx.vin) { + if (!pcoinsTip->HaveCoinsInCache(txin.prevout)) { + vHashTxnToUncache.push_back(txin.prevout); + } + if (!view.HaveCoins(txin.prevout)) { + if (pfMissingInputs) { + *pfMissingInputs = true; + } + return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() + } + } // Bring the best block into scope view.GetBestBlock(); @@ -763,10 +763,10 @@ bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, bool fOverrideMempoolLimit, const CAmount nAbsurdFee) { - std::vector vHashTxToUncache; + std::vector vHashTxToUncache; bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee, vHashTxToUncache); if (!res) { - BOOST_FOREACH(const uint256& hashTx, vHashTxToUncache) + BOOST_FOREACH(const COutPoint& hashTx, vHashTxToUncache) pcoinsTip->Uncache(hashTx); } // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits @@ -1762,12 +1762,12 @@ bool static FlushStateToDisk(CValidationState &state, FlushStateMode mode, int n } // Flush best chain related state. This can only be done if the blocks / block index write was also done. if (fDoFullFlush) { - // Typical CCoins structures on disk are around 128 bytes in size. + // Typical Coin structures on disk are around 48 bytes in size. // Pushing a new one to the database can cause it to be written // twice (once in the log, and once in the tables). This is already // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. - if (!CheckDiskSpace(128 * 2 * 2 * pcoinsTip->GetCacheSize())) + if (!CheckDiskSpace(48 * 2 * 2 * pcoinsTip->GetCacheSize())) return state.Error("out of disk space"); // Flush the chainstate (which may refer to block index entries). if (!pcoinsTip->Flush()) @@ -1848,7 +1848,7 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { } } } - LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utx)", __func__, + LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)", __func__, chainActive.Tip()->GetBlockHash().ToString(), chainActive.Height(), chainActive.Tip()->nVersion, log(chainActive.Tip()->nChainWork.getdouble())/log(2.0), (unsigned long)chainActive.Tip()->nChainTx, DateTimeStrFormat("%Y-%m-%d %H:%M:%S", chainActive.Tip()->GetBlockTime()), From ce23efaa5c2e8e50744a896424b01052db34a3d6 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 21 May 2017 12:01:29 -0700 Subject: [PATCH 20/29] Extend coins_tests --- src/test/coins_tests.cpp | 56 ++++++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 8d519c644..f3f6e9f5f 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -87,9 +87,12 @@ public: { // Manually recompute the dynamic usage of the whole data, and compare it. size_t ret = memusage::DynamicUsage(cacheCoins); + size_t count = 0; for (CCoinsMap::iterator it = cacheCoins.begin(); it != cacheCoins.end(); it++) { ret += it->second.coins.DynamicMemoryUsage(); + ++count; } + BOOST_CHECK_EQUAL(GetCacheSize(), count); BOOST_CHECK_EQUAL(DynamicMemoryUsage(), ret); } @@ -118,10 +121,12 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) bool removed_all_caches = false; bool reached_4_caches = false; bool added_an_entry = false; + bool added_an_unspendable_entry = false; bool removed_an_entry = false; bool updated_an_entry = false; bool found_an_entry = false; bool missed_an_entry = false; + bool uncached_an_entry = false; // A simple map to track what we expect the cache stack to represent. std::map result; @@ -143,36 +148,49 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) { uint256 txid = txids[insecure_rand() % txids.size()]; // txid we're going to modify in this iteration. Coin& coin = result[COutPoint(txid, 0)]; - const Coin& entry = stack.back()->AccessCoin(COutPoint(txid, 0)); + const Coin& entry = (insecure_rand() % 500 == 0) ? AccessByTxid(*stack.back(), txid) : stack.back()->AccessCoin(COutPoint(txid, 0)); BOOST_CHECK(coin == entry); if (insecure_rand() % 5 == 0 || coin.IsPruned()) { - if (coin.IsPruned()) { - added_an_entry = true; + Coin newcoin; + newcoin.out.nValue = insecure_rand(); + newcoin.nHeight = 1; + if (insecure_rand() % 16 == 0 && coin.IsPruned()) { + newcoin.out.scriptPubKey.assign(1 + (insecure_rand() & 0x3F), OP_RETURN); + BOOST_CHECK(newcoin.out.scriptPubKey.IsUnspendable()); + added_an_unspendable_entry = true; } else { - updated_an_entry = true; + newcoin.out.scriptPubKey.assign(insecure_rand() & 0x3F, 0); // Random sizes so we can test memory usage accounting + (coin.IsPruned() ? added_an_entry : updated_an_entry) = true; + coin = newcoin; } - coin.out.nValue = insecure_rand(); - coin.nHeight = 1; + stack.back()->AddCoin(COutPoint(txid, 0), std::move(newcoin), !coin.IsPruned() || insecure_rand() & 1); } else { - coin.Clear(); removed_an_entry = true; - } - if (coin.IsPruned()) { + coin.Clear(); stack.back()->SpendCoin(COutPoint(txid, 0)); - } else { - stack.back()->AddCoin(COutPoint(txid, 0), Coin(coin), true); } } + // One every 10 iterations, remove a random entry from the cache + if (insecure_rand() % 10) { + COutPoint out(txids[insecure_rand() % txids.size()], 0); + int cacheid = insecure_rand() % stack.size(); + stack[cacheid]->Uncache(out); + uncached_an_entry |= !stack[cacheid]->HaveCoinsInCache(out); + } + // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { for (auto it = result.begin(); it != result.end(); it++) { + bool have = stack.back()->HaveCoins(it->first); const Coin& coin = stack.back()->AccessCoin(it->first); + BOOST_CHECK(have == !coin.IsPruned()); BOOST_CHECK(coin == it->second); if (coin.IsPruned()) { missed_an_entry = true; } else { + BOOST_CHECK(stack.back()->HaveCoinsInCache(it->first)); found_an_entry = true; } } @@ -222,10 +240,12 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) BOOST_CHECK(removed_all_caches); BOOST_CHECK(reached_4_caches); BOOST_CHECK(added_an_entry); + BOOST_CHECK(added_an_unspendable_entry); BOOST_CHECK(removed_an_entry); BOOST_CHECK(updated_an_entry); BOOST_CHECK(found_an_entry); BOOST_CHECK(missed_an_entry); + BOOST_CHECK(uncached_an_entry); } // Store of all necessary tx and undo data for next test @@ -275,6 +295,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) tx.vin.resize(1); tx.vout.resize(1); tx.vout[0].nValue = i; //Keep txs unique unless intended to duplicate + tx.vout[0].scriptPubKey.assign(insecure_rand() & 0x3F, 0); // Random sizes so we can test memory usage accounting unsigned int height = insecure_rand(); Coin oldcoins; @@ -393,11 +414,24 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { for (auto it = result.begin(); it != result.end(); it++) { + bool have = stack.back()->HaveCoins(it->first); const Coin& coin = stack.back()->AccessCoin(it->first); + BOOST_CHECK(have == !coin.IsPruned()); BOOST_CHECK(coin == it->second); } } + // One every 10 iterations, remove a random entry from the cache + if (utxoset.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(utxoset)->first); + } + if (disconnectedids.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(disconnectedids)->first); + } + if (duplicateids.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(duplicateids)->first); + } + if (insecure_rand() % 100 == 0) { // Every 100 iterations, flush an intermediate cache if (stack.size() > 1 && insecure_rand() % 2 == 0) { From 97072d6685564dd50aab4c145b1758ccc10863b3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:41 -0700 Subject: [PATCH 21/29] Remove unused CCoins methods --- src/coins.cpp | 32 ---------- src/coins.h | 167 -------------------------------------------------- 2 files changed, 199 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 38b8df171..09fd2b13d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -10,38 +10,6 @@ #include -/** - * calculate number of bytes for the bitmask, and its number of non-zero bytes - * each bit in the bitmask represents the availability of one output, but the - * availabilities of the first two outputs are encoded separately - */ -void CCoins::CalcMaskSize(unsigned int &nBytes, unsigned int &nNonzeroBytes) const { - unsigned int nLastUsedByte = 0; - for (unsigned int b = 0; 2+b*8 < vout.size(); b++) { - bool fZero = true; - for (unsigned int i = 0; i < 8 && 2+b*8+i < vout.size(); i++) { - if (!vout[2+b*8+i].IsNull()) { - fZero = false; - continue; - } - } - if (!fZero) { - nLastUsedByte = b + 1; - nNonzeroBytes++; - } - } - nBytes += nLastUsedByte; -} - -bool CCoins::Spend(uint32_t nPos) -{ - if (nPos >= vout.size() || vout[nPos].IsNull()) - return false; - vout[nPos].SetNull(); - Cleanup(); - return true; -} - bool CCoinsView::GetCoins(const COutPoint &outpoint, Coin &coin) const { return false; } bool CCoinsView::HaveCoins(const COutPoint &outpoint) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } diff --git a/src/coins.h b/src/coins.h index ae85a199c..bd2f2caf2 100644 --- a/src/coins.h +++ b/src/coins.h @@ -82,58 +82,6 @@ public: } }; -/** - * Pruned version of CTransaction: only retains metadata and unspent transaction outputs - * - * Serialized format: - * - VARINT(nVersion) - * - VARINT(nCode) - * - unspentness bitvector, for vout[2] and further; least significant byte first - * - the non-spent CTxOuts (via CTxOutCompressor) - * - VARINT(nHeight) - * - * The nCode value consists of: - * - bit 0: IsCoinBase() - * - bit 1: vout[0] is not spent - * - bit 2: vout[1] is not spent - * - The higher bits encode N, the number of non-zero bytes in the following bitvector. - * - In case both bit 1 and bit 2 are unset, they encode N-1, as there must be at - * least one non-spent output). - * - * Example: 0104835800816115944e077fe7c803cfa57f29b36bf87c1d358bb85e - * <><><--------------------------------------------><----> - * | \ | / - * version code vout[1] height - * - * - version = 1 - * - code = 4 (vout[1] is not spent, and 0 non-zero bytes of bitvector follow) - * - unspentness bitvector: as 0 non-zero bytes follow, it has length 0 - * - vout[1]: 835800816115944e077fe7c803cfa57f29b36bf87c1d35 - * * 8358: compact amount representation for 60000000000 (600 BTC) - * * 00: special txout type pay-to-pubkey-hash - * * 816115944e077fe7c803cfa57f29b36bf87c1d35: address uint160 - * - height = 203998 - * - * - * Example: 0109044086ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4eebbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa486af3b - * <><><--><--------------------------------------------------><----------------------------------------------><----> - * / \ \ | | / - * version code unspentness vout[4] vout[16] height - * - * - version = 1 - * - code = 9 (coinbase, neither vout[0] or vout[1] are unspent, - * 2 (1, +1 because both bit 1 and bit 2 are unset) non-zero bitvector bytes follow) - * - unspentness bitvector: bits 2 (0x04) and 14 (0x4000) are set, so vout[2+2] and vout[14+2] are unspent - * - vout[4]: 86ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4ee - * * 86ef97d579: compact amount representation for 234925952 (2.35 BTC) - * * 00: special txout type pay-to-pubkey-hash - * * 61b01caab50f1b8e9c50a5057eb43c2d9563a4ee: address uint160 - * - vout[16]: bbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa4 - * * bbd123: compact amount representation for 110397 (0.001 BTC) - * * 00: special txout type pay-to-pubkey-hash - * * 8c988f1a4a4de2161e0f50aac7f17e7f9555caa4: address uint160 - * - height = 120891 - */ class CCoins { public: @@ -146,98 +94,9 @@ public: //! at which height this transaction was included in the active block chain int nHeight; - void FromTx(const CTransaction &tx, int nHeightIn) { - fCoinBase = tx.IsCoinBase(); - vout = tx.vout; - nHeight = nHeightIn; - ClearUnspendable(); - } - - //! construct a CCoins from a CTransaction, at a given height - CCoins(const CTransaction &tx, int nHeightIn) { - FromTx(tx, nHeightIn); - } - - void Clear() { - fCoinBase = false; - std::vector().swap(vout); - nHeight = 0; - } - //! empty constructor CCoins() : fCoinBase(false), vout(0), nHeight(0) { } - //!remove spent outputs at the end of vout - void Cleanup() { - while (vout.size() > 0 && vout.back().IsNull()) - vout.pop_back(); - if (vout.empty()) - std::vector().swap(vout); - } - - void ClearUnspendable() { - BOOST_FOREACH(CTxOut &txout, vout) { - if (txout.scriptPubKey.IsUnspendable()) - txout.SetNull(); - } - Cleanup(); - } - - void swap(CCoins &to) { - std::swap(to.fCoinBase, fCoinBase); - to.vout.swap(vout); - std::swap(to.nHeight, nHeight); - } - - //! equality test - friend bool operator==(const CCoins &a, const CCoins &b) { - // Empty CCoins objects are always equal. - if (a.IsPruned() && b.IsPruned()) - return true; - return a.fCoinBase == b.fCoinBase && - a.nHeight == b.nHeight && - a.vout == b.vout; - } - friend bool operator!=(const CCoins &a, const CCoins &b) { - return !(a == b); - } - - void CalcMaskSize(unsigned int &nBytes, unsigned int &nNonzeroBytes) const; - - bool IsCoinBase() const { - return fCoinBase; - } - - template - void Serialize(Stream &s) const { - unsigned int nMaskSize = 0, nMaskCode = 0; - CalcMaskSize(nMaskSize, nMaskCode); - bool fFirst = vout.size() > 0 && !vout[0].IsNull(); - bool fSecond = vout.size() > 1 && !vout[1].IsNull(); - assert(fFirst || fSecond || nMaskCode); - unsigned int nCode = 8*(nMaskCode - (fFirst || fSecond ? 0 : 1)) + (fCoinBase ? 1 : 0) + (fFirst ? 2 : 0) + (fSecond ? 4 : 0); - // version - int nVersionDummy = 0; - ::Serialize(s, VARINT(nVersionDummy)); - // header code - ::Serialize(s, VARINT(nCode)); - // spentness bitmask - for (unsigned int b = 0; b void Unserialize(Stream &s) { unsigned int nCode = 0; @@ -270,32 +129,6 @@ public: } // coinbase height ::Unserialize(s, VARINT(nHeight)); - Cleanup(); - } - - //! mark a vout spent - bool Spend(uint32_t nPos); - - //! check whether a particular output is still available - bool IsAvailable(unsigned int nPos) const { - return (nPos < vout.size() && !vout[nPos].IsNull()); - } - - //! check whether the entire CCoins is spent - //! note that only !IsPruned() CCoins can be serialized - bool IsPruned() const { - BOOST_FOREACH(const CTxOut &out, vout) - if (!out.IsNull()) - return false; - return true; - } - - size_t DynamicMemoryUsage() const { - size_t ret = memusage::DynamicUsage(vout); - BOOST_FOREACH(const CTxOut &out, vout) { - ret += RecursiveDynamicUsage(out.scriptPubKey); - } - return ret; } }; From 41aa5b79a3d79f8afe4c556b4f14fb1dc0cc3f9f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:42 -0700 Subject: [PATCH 22/29] Pack Coin more tightly --- src/coins.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coins.h b/src/coins.h index bd2f2caf2..7f4dffc7e 100644 --- a/src/coins.h +++ b/src/coins.h @@ -30,18 +30,18 @@ class Coin { public: - //! whether the containing transaction was a coinbase - bool fCoinBase; - //! unspent transaction output CTxOut out; - //! at which height the containing transaction was included in the active block chain - uint32_t nHeight; + //! whether containing transaction was a coinbase + unsigned int fCoinBase : 1; - //! construct a Coin from a CTxOut and height/coinbase properties. - Coin(CTxOut&& outIn, int nHeightIn, bool fCoinBaseIn) : fCoinBase(fCoinBaseIn), out(std::move(outIn)), nHeight(nHeightIn) {} - Coin(const CTxOut& outIn, int nHeightIn, bool fCoinBaseIn) : fCoinBase(fCoinBaseIn), out(outIn), nHeight(nHeightIn) {} + //! at which height this containing transaction was included in the active block chain + uint32_t nHeight : 31; + + //! construct a Coin from a CTxOut and height/coinbase information. + Coin(CTxOut&& outIn, int nHeightIn, bool fCoinBaseIn) : out(std::move(outIn)), fCoinBase(fCoinBaseIn), nHeight(nHeightIn) {} + Coin(const CTxOut& outIn, int nHeightIn, bool fCoinBaseIn) : out(outIn), fCoinBase(fCoinBaseIn),nHeight(nHeightIn) {} void Clear() { out.SetNull(); From b2af357f39c7d17ab6ddb2938531155bf90126ec Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:44 -0700 Subject: [PATCH 23/29] Reduce reserved memory space for flushing As the maximum amount of data that can be pulled into the cache due to a block validation is much lower now (at most one CCoin entry per input and per output), reduce the conservative estimate used to determine flushing time. --- src/txdb.h | 4 +--- src/validation.cpp | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/txdb.h b/src/txdb.h index 73011fe0f..189094737 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -22,9 +22,7 @@ class uint256; //! Compensate for extra memory peak (x1.5-x1.9) at flush time. static constexpr int DB_PEAK_USAGE_FACTOR = 2; //! No need to periodic flush if at least this much space still available. -static constexpr int MAX_BLOCK_COINSDB_USAGE = 200 * DB_PEAK_USAGE_FACTOR; -//! Always periodic flush if less than this much space still available. -static constexpr int MIN_BLOCK_COINSDB_USAGE = 50 * DB_PEAK_USAGE_FACTOR; +static constexpr int MAX_BLOCK_COINSDB_USAGE = 10 * DB_PEAK_USAGE_FACTOR; //! -dbcache default (MiB) static const int64_t nDefaultDbCache = 450; //! max. -dbcache (MiB) diff --git a/src/validation.cpp b/src/validation.cpp index cf4dab420..005eaafff 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1719,9 +1719,8 @@ bool static FlushStateToDisk(CValidationState &state, FlushStateMode mode, int n int64_t nMempoolSizeMax = GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; int64_t cacheSize = pcoinsTip->DynamicMemoryUsage() * DB_PEAK_USAGE_FACTOR; int64_t nTotalSpace = nCoinCacheUsage + std::max(nMempoolSizeMax - nMempoolUsage, 0); - // The cache is large and we're within 10% and 200 MiB or 50% and 50MiB of the limit, but we have time now (not in the middle of a block processing). - bool fCacheLarge = mode == FLUSH_STATE_PERIODIC && cacheSize > std::min(std::max(nTotalSpace / 2, nTotalSpace - MIN_BLOCK_COINSDB_USAGE * 1024 * 1024), - std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024)); + // The cache is large and we're within 10% and 10 MiB of the limit, but we have time now (not in the middle of a block processing). + bool fCacheLarge = mode == FLUSH_STATE_PERIODIC && cacheSize > std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024); // The cache is over the limit, we have to write now. bool fCacheCritical = mode == FLUSH_STATE_IF_NEEDED && cacheSize > nTotalSpace; // It's been a while since we wrote the block index to disk. Do this frequently, so we don't need to redownload after a crash. From 8b25d2c0ce64d7f8577a0c2e601e505c9f1140bf Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:46 -0700 Subject: [PATCH 24/29] Upgrade from per-tx database to per-txout --- src/coins.h | 1 + src/init.cpp | 6 ++++++ src/txdb.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/txdb.h | 2 ++ 4 files changed, 54 insertions(+) diff --git a/src/coins.h b/src/coins.h index 7f4dffc7e..5ffb810aa 100644 --- a/src/coins.h +++ b/src/coins.h @@ -82,6 +82,7 @@ public: } }; +//! Legacy class to deserialize pre-pertxout database entries without reindex. class CCoins { public: diff --git a/src/init.cpp b/src/init.cpp index 4420b6f65..8ce384795 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1455,6 +1455,12 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) //If we're reindexing in prune mode, wipe away unusable block files and all undo data files if (fPruneMode) CleanupBlockRevFiles(); + } else { + // If necessary, upgrade from older database format. + if (!pcoinsdbview->Upgrade()) { + strLoadError = _("Error upgrading chainstate database"); + break; + } } if (!LoadBlockIndex(chainparams)) { diff --git a/src/txdb.cpp b/src/txdb.cpp index 19c900250..f4a6fad85 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -252,3 +252,48 @@ bool CBlockTreeDB::LoadBlockIndexGuts(std::function pcursor(db.NewIterator()); + pcursor->Seek(std::make_pair(DB_COINS, uint256())); + if (!pcursor->Valid()) { + return true; + } + + LogPrintf("Upgrading database...\n"); + size_t batch_size = 1 << 24; + CDBBatch batch(db); + while (pcursor->Valid()) { + boost::this_thread::interruption_point(); + std::pair key; + if (pcursor->GetKey(key) && key.first == DB_COINS) { + CCoins old_coins; + if (!pcursor->GetValue(old_coins)) { + return error("%s: cannot parse CCoins record", __func__); + } + COutPoint outpoint(key.second, 0); + for (size_t i = 0; i < old_coins.vout.size(); ++i) { + if (!old_coins.vout[i].IsNull() && !old_coins.vout[i].scriptPubKey.IsUnspendable()) { + Coin newcoin(std::move(old_coins.vout[i]), old_coins.nHeight, old_coins.fCoinBase); + outpoint.n = i; + CoinEntry entry(&outpoint); + batch.Write(entry, newcoin); + } + } + batch.Erase(key); + if (batch.SizeEstimate() > batch_size) { + db.WriteBatch(batch); + batch.Clear(); + } + pcursor->Next(); + } else { + break; + } + } + db.WriteBatch(batch); + return true; +} diff --git a/src/txdb.h b/src/txdb.h index 189094737..51dc355bb 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -77,6 +77,8 @@ public: bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; CCoinsViewCursor *Cursor() const override; + //! Attempt to update from an older database format. Returns whether an error occurred. + bool Upgrade(); size_t EstimateSize() const override; }; From 580b023092a28f444068b53792eb542f9d5e6892 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:48 -0700 Subject: [PATCH 25/29] [MOVEONLY] Move old CCoins class to txdb.cpp It's only used for upgrading from the old database anymore. --- src/coins.h | 51 ------------------------------------------------ src/txdb.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 51 deletions(-) diff --git a/src/coins.h b/src/coins.h index 5ffb810aa..43da9a3c6 100644 --- a/src/coins.h +++ b/src/coins.h @@ -82,57 +82,6 @@ public: } }; -//! Legacy class to deserialize pre-pertxout database entries without reindex. -class CCoins -{ -public: - //! whether transaction is a coinbase - bool fCoinBase; - - //! unspent transaction outputs; spent outputs are .IsNull(); spent outputs at the end of the array are dropped - std::vector vout; - - //! at which height this transaction was included in the active block chain - int nHeight; - - //! empty constructor - CCoins() : fCoinBase(false), vout(0), nHeight(0) { } - - template - void Unserialize(Stream &s) { - unsigned int nCode = 0; - // version - int nVersionDummy; - ::Unserialize(s, VARINT(nVersionDummy)); - // header code - ::Unserialize(s, VARINT(nCode)); - fCoinBase = nCode & 1; - std::vector vAvail(2, false); - vAvail[0] = (nCode & 2) != 0; - vAvail[1] = (nCode & 4) != 0; - unsigned int nMaskCode = (nCode / 8) + ((nCode & 6) != 0 ? 0 : 1); - // spentness bitmask - while (nMaskCode > 0) { - unsigned char chAvail = 0; - ::Unserialize(s, chAvail); - for (unsigned int p = 0; p < 8; p++) { - bool f = (chAvail & (1 << p)) != 0; - vAvail.push_back(f); - } - if (chAvail != 0) - nMaskCode--; - } - // txouts themself - vout.assign(vAvail.size(), CTxOut()); - for (unsigned int i = 0; i < vAvail.size(); i++) { - if (vAvail[i]) - ::Unserialize(s, REF(CTxOutCompressor(vout[i]))); - } - // coinbase height - ::Unserialize(s, VARINT(nHeight)); - } -}; - class SaltedOutpointHasher { private: diff --git a/src/txdb.cpp b/src/txdb.cpp index f4a6fad85..205b1b0a8 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -253,6 +253,61 @@ bool CBlockTreeDB::LoadBlockIndexGuts(std::function vout; + + //! at which height this transaction was included in the active block chain + int nHeight; + + //! empty constructor + CCoins() : fCoinBase(false), vout(0), nHeight(0) { } + + template + void Unserialize(Stream &s) { + unsigned int nCode = 0; + // version + int nVersionDummy; + ::Unserialize(s, VARINT(nVersionDummy)); + // header code + ::Unserialize(s, VARINT(nCode)); + fCoinBase = nCode & 1; + std::vector vAvail(2, false); + vAvail[0] = (nCode & 2) != 0; + vAvail[1] = (nCode & 4) != 0; + unsigned int nMaskCode = (nCode / 8) + ((nCode & 6) != 0 ? 0 : 1); + // spentness bitmask + while (nMaskCode > 0) { + unsigned char chAvail = 0; + ::Unserialize(s, chAvail); + for (unsigned int p = 0; p < 8; p++) { + bool f = (chAvail & (1 << p)) != 0; + vAvail.push_back(f); + } + if (chAvail != 0) + nMaskCode--; + } + // txouts themself + vout.assign(vAvail.size(), CTxOut()); + for (unsigned int i = 0; i < vAvail.size(); i++) { + if (vAvail[i]) + ::Unserialize(s, REF(CTxOutCompressor(vout[i]))); + } + // coinbase height + ::Unserialize(s, VARINT(nHeight)); + } +}; + +} + /** Upgrade the database from older formats. * * Currently implemented: from the per-tx utxo model (0.8..0.14.x) to per-txout. From 119e552f7ccd49c0137a3c6b4f94018a84d69620 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 26 Apr 2017 16:09:27 -0700 Subject: [PATCH 26/29] Merge CCoinsViewCache's GetOutputFor and AccessCoin They're doing the same thing now. --- src/coins.cpp | 9 +-------- src/coins.h | 2 -- src/consensus/tx_verify.cpp | 4 ++-- src/policy/policy.cpp | 4 ++-- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 09fd2b13d..59bcdce9c 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -213,13 +213,6 @@ unsigned int CCoinsViewCache::GetCacheSize() const { return cacheCoins.size(); } -const CTxOut &CCoinsViewCache::GetOutputFor(const CTxIn& input) const -{ - const Coin& coin = AccessCoin(input.prevout); - assert(!coin.IsPruned()); - return coin.out; -} - CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const { if (tx.IsCoinBase()) @@ -227,7 +220,7 @@ CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const CAmount nResult = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) - nResult += GetOutputFor(tx.vin[i]).nValue; + nResult += AccessCoin(tx.vin[i].prevout).out.nValue; return nResult; } diff --git a/src/coins.h b/src/coins.h index 43da9a3c6..1203c3fad 100644 --- a/src/coins.h +++ b/src/coins.h @@ -272,8 +272,6 @@ public: //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view bool HaveInputs(const CTransaction& tx) const; - const CTxOut &GetOutputFor(const CTxIn& input) const; - private: CCoinsMap::iterator FetchCoins(const COutPoint &outpoint) const; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 9a6f3c9cd..1195a1561 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -126,7 +126,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in unsigned int nSigOps = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) { - const CTxOut &prevout = inputs.GetOutputFor(tx.vin[i]); + const CTxOut &prevout = inputs.AccessCoin(tx.vin[i].prevout).out; if (prevout.scriptPubKey.IsPayToScriptHash()) nSigOps += prevout.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig); } @@ -146,7 +146,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i for (unsigned int i = 0; i < tx.vin.size(); i++) { - const CTxOut &prevout = inputs.GetOutputFor(tx.vin[i]); + const CTxOut &prevout = inputs.AccessCoin(tx.vin[i].prevout).out; nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, &tx.vin[i].scriptWitness, flags); } return nSigOps; diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index f4fffd657..14d58e744 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -165,7 +165,7 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) for (unsigned int i = 0; i < tx.vin.size(); i++) { - const CTxOut& prev = mapInputs.GetOutputFor(tx.vin[i]); + const CTxOut& prev = mapInputs.AccessCoin(tx.vin[i].prevout).out; std::vector > vSolutions; txnouttype whichType; @@ -204,7 +204,7 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) if (tx.vin[i].scriptWitness.IsNull()) continue; - const CTxOut &prev = mapInputs.GetOutputFor(tx.vin[i]); + const CTxOut &prev = mapInputs.AccessCoin(tx.vin[i].prevout).out; // get the scriptPubKey corresponding to this input: CScript prevScript = prev.scriptPubKey; From 73de2c1ff345ac38c098d7b1bef03176f3ea1f16 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 4 May 2017 00:15:36 -0700 Subject: [PATCH 27/29] Rename CCoinsCacheEntry::coins to coin --- src/coins.cpp | 44 ++++++++++++++++++++-------------------- src/coins.h | 4 ++-- src/test/coins_tests.cpp | 14 ++++++------- src/txdb.cpp | 4 ++-- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 59bcdce9c..a6b08b4fe 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -42,19 +42,19 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoins(const COutPoint &outpoint) const if (!base->GetCoins(outpoint, tmp)) return cacheCoins.end(); CCoinsMap::iterator ret = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::forward_as_tuple(std::move(tmp))).first; - if (ret->second.coins.IsPruned()) { + if (ret->second.coin.IsPruned()) { // The parent only has an empty entry for this outpoint; we can consider our // version as fresh. ret->second.flags = CCoinsCacheEntry::FRESH; } - cachedCoinsUsage += ret->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); return ret; } bool CCoinsViewCache::GetCoins(const COutPoint &outpoint, Coin &coin) const { CCoinsMap::const_iterator it = FetchCoins(outpoint); if (it != cacheCoins.end()) { - coin = it->second.coins; + coin = it->second.coin; return true; } return false; @@ -68,17 +68,17 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>()); bool fresh = false; if (!inserted) { - cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); } if (!possible_overwrite) { - if (!it->second.coins.IsPruned()) { + if (!it->second.coin.IsPruned()) { throw std::logic_error("Adding new coin that replaces non-pruned entry"); } fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY); } - it->second.coins = std::move(coin); + it->second.coin = std::move(coin); it->second.flags |= CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0); - cachedCoinsUsage += it->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage += it->second.coin.DynamicMemoryUsage(); } void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { @@ -94,15 +94,15 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { CCoinsMap::iterator it = FetchCoins(outpoint); if (it == cacheCoins.end()) return; - cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); if (moveout) { - *moveout = std::move(it->second.coins); + *moveout = std::move(it->second.coin); } if (it->second.flags & CCoinsCacheEntry::FRESH) { cacheCoins.erase(it); } else { it->second.flags |= CCoinsCacheEntry::DIRTY; - it->second.coins.Clear(); + it->second.coin.Clear(); } } @@ -113,13 +113,13 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { if (it == cacheCoins.end()) { return coinEmpty; } else { - return it->second.coins; + return it->second.coin; } } bool CCoinsViewCache::HaveCoins(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = FetchCoins(outpoint); - return (it != cacheCoins.end() && !it->second.coins.IsPruned()); + return (it != cacheCoins.end() && !it->second.coin.IsPruned()); } bool CCoinsViewCache::HaveCoinsInCache(const COutPoint &outpoint) const { @@ -144,12 +144,12 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn if (itUs == cacheCoins.end()) { // The parent cache does not have an entry, while the child does // We can ignore it if it's both FRESH and pruned in the child - if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coins.IsPruned())) { + if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsPruned())) { // Otherwise we will need to create it in the parent // and move the data up and mark it as dirty CCoinsCacheEntry& entry = cacheCoins[it->first]; - entry.coins = std::move(it->second.coins); - cachedCoinsUsage += entry.coins.DynamicMemoryUsage(); + entry.coin = std::move(it->second.coin); + cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); entry.flags = CCoinsCacheEntry::DIRTY; // We can mark it FRESH in the parent if it was FRESH in the child // Otherwise it might have just been flushed from the parent's cache @@ -162,21 +162,21 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn // parent cache entry has unspent outputs. If this ever happens, // it means the FRESH flag was misapplied and there is a logic // error in the calling code. - if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coins.IsPruned()) + if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsPruned()) throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs"); // Found the entry in the parent cache - if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coins.IsPruned()) { + if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsPruned()) { // The grandparent does not have an entry, and the child is // modified and being pruned. This means we can just delete // it from the parent. - cachedCoinsUsage -= itUs->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); cacheCoins.erase(itUs); } else { // A normal modification. - cachedCoinsUsage -= itUs->second.coins.DynamicMemoryUsage(); - itUs->second.coins = std::move(it->second.coins); - cachedCoinsUsage += itUs->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); + itUs->second.coin = std::move(it->second.coin); + cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); itUs->second.flags |= CCoinsCacheEntry::DIRTY; // NOTE: It is possible the child has a FRESH flag here in // the event the entry we found in the parent is pruned. But @@ -204,7 +204,7 @@ void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); if (it != cacheCoins.end() && it->second.flags == 0) { - cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); cacheCoins.erase(it); } } diff --git a/src/coins.h b/src/coins.h index 1203c3fad..294abb678 100644 --- a/src/coins.h +++ b/src/coins.h @@ -103,7 +103,7 @@ public: struct CCoinsCacheEntry { - Coin coins; // The actual cached data. + Coin coin; // The actual cached data. unsigned char flags; enum Flags { @@ -117,7 +117,7 @@ struct CCoinsCacheEntry }; CCoinsCacheEntry() : flags(0) {} - explicit CCoinsCacheEntry(Coin&& coin_) : coins(std::move(coin_)), flags(0) {} + explicit CCoinsCacheEntry(Coin&& coin_) : coin(std::move(coin_)), flags(0) {} }; typedef std::unordered_map CCoinsMap; diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index f3f6e9f5f..53e01636c 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -64,8 +64,8 @@ public: for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Same optimization used in CCoinsViewDB is to only write dirty entries. - map_[it->first] = it->second.coins; - if (it->second.coins.IsPruned() && insecure_rand() % 3 == 0) { + map_[it->first] = it->second.coin; + if (it->second.coin.IsPruned() && insecure_rand() % 3 == 0) { // Randomly delete empty entries on write. map_.erase(it->first); } @@ -89,7 +89,7 @@ public: size_t ret = memusage::DynamicUsage(cacheCoins); size_t count = 0; for (CCoinsMap::iterator it = cacheCoins.begin(); it != cacheCoins.end(); it++) { - ret += it->second.coins.DynamicMemoryUsage(); + ret += it->second.coin.DynamicMemoryUsage(); ++count; } BOOST_CHECK_EQUAL(GetCacheSize(), count); @@ -554,10 +554,10 @@ size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags) assert(flags != NO_ENTRY); CCoinsCacheEntry entry; entry.flags = flags; - SetCoinsValue(value, entry.coins); + SetCoinsValue(value, entry.coin); auto inserted = map.emplace(OUTPOINT, std::move(entry)); assert(inserted.second); - return inserted.first->second.coins.DynamicMemoryUsage(); + return inserted.first->second.coin.DynamicMemoryUsage(); } void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags) @@ -567,10 +567,10 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags) value = ABSENT; flags = NO_ENTRY; } else { - if (it->second.coins.IsPruned()) { + if (it->second.coin.IsPruned()) { value = PRUNED; } else { - value = it->second.coins.out.nValue; + value = it->second.coin.out.nValue; } flags = it->second.flags; assert(flags != NO_ENTRY); diff --git a/src/txdb.cpp b/src/txdb.cpp index 205b1b0a8..45a7d69a3 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -75,10 +75,10 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { CoinsEntry entry(&it->first); - if (it->second.coins.IsPruned()) + if (it->second.coin.IsPruned()) batch.Erase(entry); else - batch.Write(entry, it->second.coins); + batch.Write(entry, it->second.coin); changed++; } count++; From a5e02bc7f8a1af27fcafd892d8da651e8f1ab156 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 21 May 2017 12:30:38 -0700 Subject: [PATCH 28/29] Increase travis unit test timeout --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 97bb475e4..a9b246536 100644 --- a/.travis.yml +++ b/.travis.yml @@ -69,7 +69,7 @@ script: - ./configure --cache-file=../config.cache $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || ( cat config.log && false) - make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && make $GOAL V=1 ; false ) - export LD_LIBRARY_PATH=$TRAVIS_BUILD_DIR/depends/$HOST/lib - - if [ "$RUN_TESTS" = "true" ]; then make $MAKEJOBS check VERBOSE=1; fi + - if [ "$RUN_TESTS" = "true" ]; then travis_wait 30 make $MAKEJOBS check VERBOSE=1; fi - if [ "$TRAVIS_EVENT_TYPE" = "cron" ]; then extended="--extended --exclude pruning"; fi - if [ "$RUN_TESTS" = "true" ]; then test/functional/test_runner.py --coverage --quiet ${extended}; fi after_script: From 589827975f9f241e2f23eb674a7383592bff1cad Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 30 May 2017 17:58:54 -0700 Subject: [PATCH 29/29] scripted-diff: various renames for per-utxo consistency Thanks to John Newberry for pointing these out. -BEGIN VERIFY SCRIPT- sed -i 's/\/GetCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/HaveCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/HaveCoinInCache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/IsSpent/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/FetchCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/CoinEntry/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/coins_to_uncache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/coins_to_uncache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/had_coin_in_cache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/coinbase_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/disconnected_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/duplicate_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\/old_coin/g' src/test/coins_tests.cpp sed -i 's/\/orig_coin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h -END VERIFY SCRIPT- --- src/bitcoin-tx.cpp | 4 +- src/coins.cpp | 44 +++++++++--------- src/coins.h | 24 +++++----- src/consensus/tx_verify.cpp | 2 +- src/init.cpp | 4 +- src/net_processing.cpp | 4 +- src/qt/transactiondesc.cpp | 2 +- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 4 +- src/rpc/rawtransaction.cpp | 8 ++-- src/test/coins_tests.cpp | 90 ++++++++++++++++++------------------- src/txdb.cpp | 20 ++++----- src/txdb.h | 4 +- src/txmempool.cpp | 14 +++--- src/txmempool.h | 4 +- src/validation.cpp | 34 +++++++------- 16 files changed, 132 insertions(+), 132 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index a6a96fa02..cf280f485 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -562,7 +562,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) { const Coin& coin = view.AccessCoin(out); - if (!coin.IsPruned() && coin.out.scriptPubKey != scriptPubKey) { + if (!coin.IsSpent() && coin.out.scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); err = err + ScriptToAsmStr(coin.out.scriptPubKey) + "\nvs:\n"+ ScriptToAsmStr(scriptPubKey); @@ -598,7 +598,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { CTxIn& txin = mergedTx.vin[i]; const Coin& coin = view.AccessCoin(txin.prevout); - if (coin.IsPruned()) { + if (coin.IsSpent()) { fComplete = false; continue; } diff --git a/src/coins.cpp b/src/coins.cpp index a6b08b4fe..5b7c56267 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -10,16 +10,16 @@ #include -bool CCoinsView::GetCoins(const COutPoint &outpoint, Coin &coin) const { return false; } -bool CCoinsView::HaveCoins(const COutPoint &outpoint) const { return false; } +bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; } +bool CCoinsView::HaveCoin(const COutPoint &outpoint) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; } CCoinsViewCursor *CCoinsView::Cursor() const { return 0; } CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } -bool CCoinsViewBacked::GetCoins(const COutPoint &outpoint, Coin &coin) const { return base->GetCoins(outpoint, coin); } -bool CCoinsViewBacked::HaveCoins(const COutPoint &outpoint) const { return base->HaveCoins(outpoint); } +bool CCoinsViewBacked::GetCoin(const COutPoint &outpoint, Coin &coin) const { return base->GetCoin(outpoint, coin); } +bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base->HaveCoin(outpoint); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); } @@ -34,15 +34,15 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; } -CCoinsMap::iterator CCoinsViewCache::FetchCoins(const COutPoint &outpoint) const { +CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const { CCoinsMap::iterator it = cacheCoins.find(outpoint); if (it != cacheCoins.end()) return it; Coin tmp; - if (!base->GetCoins(outpoint, tmp)) + if (!base->GetCoin(outpoint, tmp)) return cacheCoins.end(); CCoinsMap::iterator ret = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::forward_as_tuple(std::move(tmp))).first; - if (ret->second.coin.IsPruned()) { + if (ret->second.coin.IsSpent()) { // The parent only has an empty entry for this outpoint; we can consider our // version as fresh. ret->second.flags = CCoinsCacheEntry::FRESH; @@ -51,8 +51,8 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoins(const COutPoint &outpoint) const return ret; } -bool CCoinsViewCache::GetCoins(const COutPoint &outpoint, Coin &coin) const { - CCoinsMap::const_iterator it = FetchCoins(outpoint); +bool CCoinsViewCache::GetCoin(const COutPoint &outpoint, Coin &coin) const { + CCoinsMap::const_iterator it = FetchCoin(outpoint); if (it != cacheCoins.end()) { coin = it->second.coin; return true; @@ -61,7 +61,7 @@ bool CCoinsViewCache::GetCoins(const COutPoint &outpoint, Coin &coin) const { } void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possible_overwrite) { - assert(!coin.IsPruned()); + assert(!coin.IsSpent()); if (coin.out.scriptPubKey.IsUnspendable()) return; CCoinsMap::iterator it; bool inserted; @@ -71,7 +71,7 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); } if (!possible_overwrite) { - if (!it->second.coin.IsPruned()) { + if (!it->second.coin.IsSpent()) { throw std::logic_error("Adding new coin that replaces non-pruned entry"); } fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY); @@ -92,7 +92,7 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { } void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { - CCoinsMap::iterator it = FetchCoins(outpoint); + CCoinsMap::iterator it = FetchCoin(outpoint); if (it == cacheCoins.end()) return; cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); if (moveout) { @@ -109,7 +109,7 @@ void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { static const Coin coinEmpty; const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { - CCoinsMap::const_iterator it = FetchCoins(outpoint); + CCoinsMap::const_iterator it = FetchCoin(outpoint); if (it == cacheCoins.end()) { return coinEmpty; } else { @@ -117,12 +117,12 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { } } -bool CCoinsViewCache::HaveCoins(const COutPoint &outpoint) const { - CCoinsMap::const_iterator it = FetchCoins(outpoint); - return (it != cacheCoins.end() && !it->second.coin.IsPruned()); +bool CCoinsViewCache::HaveCoin(const COutPoint &outpoint) const { + CCoinsMap::const_iterator it = FetchCoin(outpoint); + return (it != cacheCoins.end() && !it->second.coin.IsSpent()); } -bool CCoinsViewCache::HaveCoinsInCache(const COutPoint &outpoint) const { +bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const { CCoinsMap::const_iterator it = cacheCoins.find(outpoint); return it != cacheCoins.end(); } @@ -144,7 +144,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn if (itUs == cacheCoins.end()) { // The parent cache does not have an entry, while the child does // We can ignore it if it's both FRESH and pruned in the child - if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsPruned())) { + if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) { // Otherwise we will need to create it in the parent // and move the data up and mark it as dirty CCoinsCacheEntry& entry = cacheCoins[it->first]; @@ -162,11 +162,11 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn // parent cache entry has unspent outputs. If this ever happens, // it means the FRESH flag was misapplied and there is a logic // error in the calling code. - if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsPruned()) + if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs"); // Found the entry in the parent cache - if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsPruned()) { + if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) { // The grandparent does not have an entry, and the child is // modified and being pruned. This means we can just delete // it from the parent. @@ -229,7 +229,7 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const { if (!tx.IsCoinBase()) { for (unsigned int i = 0; i < tx.vin.size(); i++) { - if (!HaveCoins(tx.vin[i].prevout)) { + if (!HaveCoin(tx.vin[i].prevout)) { return false; } } @@ -244,7 +244,7 @@ const Coin& AccessByTxid(const CCoinsViewCache& view, const uint256& txid) COutPoint iter(txid, 0); while (iter.n < MAX_OUTPUTS_PER_BLOCK) { const Coin& alternate = view.AccessCoin(iter); - if (!alternate.IsPruned()) return alternate; + if (!alternate.IsSpent()) return alternate; ++iter.n; } return coinEmpty; diff --git a/src/coins.h b/src/coins.h index 294abb678..476db8f37 100644 --- a/src/coins.h +++ b/src/coins.h @@ -58,7 +58,7 @@ public: template void Serialize(Stream &s) const { - assert(!IsPruned()); + assert(!IsSpent()); uint32_t code = nHeight * 2 + fCoinBase; ::Serialize(s, VARINT(code)); ::Serialize(s, CTxOutCompressor(REF(out))); @@ -73,7 +73,7 @@ public: ::Unserialize(s, REF(CTxOutCompressor(out))); } - bool IsPruned() const { + bool IsSpent() const { return out.IsNull(); } @@ -147,11 +147,11 @@ class CCoinsView { public: //! Retrieve the Coin (unspent transaction output) for a given outpoint. - virtual bool GetCoins(const COutPoint &outpoint, Coin &coin) const; + virtual bool GetCoin(const COutPoint &outpoint, Coin &coin) const; //! Just check whether we have data for a given outpoint. //! This may (but cannot always) return true for spent outputs. - virtual bool HaveCoins(const COutPoint &outpoint) const; + virtual bool HaveCoin(const COutPoint &outpoint) const; //! Retrieve the block hash whose state this CCoinsView currently represents virtual uint256 GetBestBlock() const; @@ -179,8 +179,8 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); - bool GetCoins(const COutPoint &outpoint, Coin &coin) const override; - bool HaveCoins(const COutPoint &outpoint) const override; + bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; void SetBackend(CCoinsView &viewIn); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; @@ -207,22 +207,22 @@ public: CCoinsViewCache(CCoinsView *baseIn); // Standard CCoinsView methods - bool GetCoins(const COutPoint &outpoint, Coin &coin) const; - bool HaveCoins(const COutPoint &outpoint) const; + bool GetCoin(const COutPoint &outpoint, Coin &coin) const; + bool HaveCoin(const COutPoint &outpoint) const; uint256 GetBestBlock() const; void SetBestBlock(const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); /** * Check if we have the given utxo already loaded in this cache. - * The semantics are the same as HaveCoins(), but no calls to + * The semantics are the same as HaveCoin(), but no calls to * the backing CCoinsView are made. */ - bool HaveCoinsInCache(const COutPoint &outpoint) const; + bool HaveCoinInCache(const COutPoint &outpoint) const; /** * Return a reference to Coin in the cache, or a pruned one if not found. This is - * more efficient than GetCoins. Modifications to other cache entries are + * more efficient than GetCoin. Modifications to other cache entries are * allowed while accessing the returned pointer. */ const Coin& AccessCoin(const COutPoint &output) const; @@ -273,7 +273,7 @@ public: bool HaveInputs(const CTransaction& tx) const; private: - CCoinsMap::iterator FetchCoins(const COutPoint &outpoint) const; + CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; /** * By making the copy constructor private, we prevent accidentally using it when one intends to create a cache on top of a base cache. diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 1195a1561..bf68f8754 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -214,7 +214,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c { const COutPoint &prevout = tx.vin[i].prevout; const Coin& coin = inputs.AccessCoin(prevout); - assert(!coin.IsPruned()); + assert(!coin.IsSpent()); // If prev is coinbase, check that it's matured if (coin.IsCoinBase()) { diff --git a/src/init.cpp b/src/init.cpp index 8ce384795..0c1705394 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -146,9 +146,9 @@ class CCoinsViewErrorCatcher : public CCoinsViewBacked { public: CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {} - bool GetCoins(const COutPoint &outpoint, Coin &coin) const override { + bool GetCoin(const COutPoint &outpoint, Coin &coin) const override { try { - return CCoinsViewBacked::GetCoins(outpoint, coin); + return CCoinsViewBacked::GetCoin(outpoint, coin); } catch(const std::runtime_error& e) { uiInterface.ThreadSafeMessageBox(_("Error reading from database, shutting down."), "", CClientUIInterface::MSG_ERROR); LogPrintf("Error reading from database: %s\n", e.what()); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1c99d047e..971db3427 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -914,8 +914,8 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) return recentRejects->contains(inv.hash) || mempool.exists(inv.hash) || mapOrphanTransactions.count(inv.hash) || - pcoinsTip->HaveCoinsInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 - pcoinsTip->HaveCoinsInCache(COutPoint(inv.hash, 1)); + pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 + pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 1)); } case MSG_BLOCK: case MSG_WITNESS_BLOCK: diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 78317c112..233fc0877 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -294,7 +294,7 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco COutPoint prevout = txin.prevout; Coin prev; - if(pcoinsTip->GetCoins(prevout, prev)) + if(pcoinsTip->GetCoin(prevout, prev)) { { strHTML += "
  • "; diff --git a/src/rest.cpp b/src/rest.cpp index 16c8c4f52..b08d7153b 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -514,7 +514,7 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) for (size_t i = 0; i < vOutPoints.size(); i++) { bool hit = false; Coin coin; - if (view.GetCoins(vOutPoints[i], coin) && !mempool.isSpent(vOutPoints[i])) { + if (view.GetCoin(vOutPoints[i], coin) && !mempool.isSpent(vOutPoints[i])) { hit = true; outs.emplace_back(std::move(coin)); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 77f1dcb21..6440cfdad 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -985,11 +985,11 @@ UniValue gettxout(const JSONRPCRequest& request) if (fMempool) { LOCK(mempool.cs); CCoinsViewMemPool view(pcoinsTip, mempool); - if (!view.GetCoins(out, coin) || mempool.isSpent(out)) { // TODO: filtering spent coins should be done by the CCoinsViewMemPool + if (!view.GetCoin(out, coin) || mempool.isSpent(out)) { // TODO: filtering spent coins should be done by the CCoinsViewMemPool return NullUniValue; } } else { - if (!pcoinsTip->GetCoins(out, coin)) { + if (!pcoinsTip->GetCoin(out, coin)) { return NullUniValue; } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8ecbf9ede..e27c2a77c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -220,7 +220,7 @@ UniValue gettxoutproof(const JSONRPCRequest& request) pblockindex = mapBlockIndex[hashBlock]; } else { const Coin& coin = AccessByTxid(*pcoinsTip, oneTxid); - if (!coin.IsPruned() && coin.nHeight > 0 && coin.nHeight <= chainActive.Height()) { + if (!coin.IsSpent() && coin.nHeight > 0 && coin.nHeight <= chainActive.Height()) { pblockindex = chainActive[coin.nHeight]; } } @@ -696,7 +696,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) { const Coin& coin = view.AccessCoin(out); - if (!coin.IsPruned() && coin.out.scriptPubKey != scriptPubKey) { + if (!coin.IsSpent() && coin.out.scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); err = err + ScriptToAsmStr(coin.out.scriptPubKey) + "\nvs:\n"+ ScriptToAsmStr(scriptPubKey); @@ -768,7 +768,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { CTxIn& txin = mergedTx.vin[i]; const Coin& coin = view.AccessCoin(txin.prevout); - if (coin.IsPruned()) { + if (coin.IsSpent()) { TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); continue; } @@ -848,7 +848,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) bool fHaveChain = false; for (size_t o = 0; !fHaveChain && o < tx->vout.size(); o++) { const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); - fHaveChain = !existingCoin.IsPruned(); + fHaveChain = !existingCoin.IsSpent(); } bool fHaveMempool = mempool.exists(hashTx); if (!fHaveMempool && !fHaveChain) { diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 53e01636c..4a4e55169 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -25,7 +25,7 @@ namespace //! equality test bool operator==(const Coin &a, const Coin &b) { // Empty Coin objects are always equal. - if (a.IsPruned() && b.IsPruned()) return true; + if (a.IsSpent() && b.IsSpent()) return true; return a.fCoinBase == b.fCoinBase && a.nHeight == b.nHeight && a.out == b.out; @@ -37,24 +37,24 @@ class CCoinsViewTest : public CCoinsView std::map map_; public: - bool GetCoins(const COutPoint& outpoint, Coin& coin) const + bool GetCoin(const COutPoint& outpoint, Coin& coin) const { std::map::const_iterator it = map_.find(outpoint); if (it == map_.end()) { return false; } coin = it->second; - if (coin.IsPruned() && insecure_rand() % 2 == 0) { + if (coin.IsSpent() && insecure_rand() % 2 == 0) { // Randomly return false in case of an empty entry. return false; } return true; } - bool HaveCoins(const COutPoint& outpoint) const + bool HaveCoin(const COutPoint& outpoint) const { Coin coin; - return GetCoins(outpoint, coin); + return GetCoin(outpoint, coin); } uint256 GetBestBlock() const { return hashBestBlock_; } @@ -65,7 +65,7 @@ public: if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Same optimization used in CCoinsViewDB is to only write dirty entries. map_[it->first] = it->second.coin; - if (it->second.coin.IsPruned() && insecure_rand() % 3 == 0) { + if (it->second.coin.IsSpent() && insecure_rand() % 3 == 0) { // Randomly delete empty entries on write. map_.erase(it->first); } @@ -151,20 +151,20 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) const Coin& entry = (insecure_rand() % 500 == 0) ? AccessByTxid(*stack.back(), txid) : stack.back()->AccessCoin(COutPoint(txid, 0)); BOOST_CHECK(coin == entry); - if (insecure_rand() % 5 == 0 || coin.IsPruned()) { + if (insecure_rand() % 5 == 0 || coin.IsSpent()) { Coin newcoin; newcoin.out.nValue = insecure_rand(); newcoin.nHeight = 1; - if (insecure_rand() % 16 == 0 && coin.IsPruned()) { + if (insecure_rand() % 16 == 0 && coin.IsSpent()) { newcoin.out.scriptPubKey.assign(1 + (insecure_rand() & 0x3F), OP_RETURN); BOOST_CHECK(newcoin.out.scriptPubKey.IsUnspendable()); added_an_unspendable_entry = true; } else { newcoin.out.scriptPubKey.assign(insecure_rand() & 0x3F, 0); // Random sizes so we can test memory usage accounting - (coin.IsPruned() ? added_an_entry : updated_an_entry) = true; + (coin.IsSpent() ? added_an_entry : updated_an_entry) = true; coin = newcoin; } - stack.back()->AddCoin(COutPoint(txid, 0), std::move(newcoin), !coin.IsPruned() || insecure_rand() & 1); + stack.back()->AddCoin(COutPoint(txid, 0), std::move(newcoin), !coin.IsSpent() || insecure_rand() & 1); } else { removed_an_entry = true; coin.Clear(); @@ -177,20 +177,20 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) COutPoint out(txids[insecure_rand() % txids.size()], 0); int cacheid = insecure_rand() % stack.size(); stack[cacheid]->Uncache(out); - uncached_an_entry |= !stack[cacheid]->HaveCoinsInCache(out); + uncached_an_entry |= !stack[cacheid]->HaveCoinInCache(out); } // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { for (auto it = result.begin(); it != result.end(); it++) { - bool have = stack.back()->HaveCoins(it->first); + bool have = stack.back()->HaveCoin(it->first); const Coin& coin = stack.back()->AccessCoin(it->first); - BOOST_CHECK(have == !coin.IsPruned()); + BOOST_CHECK(have == !coin.IsSpent()); BOOST_CHECK(coin == it->second); - if (coin.IsPruned()) { + if (coin.IsSpent()) { missed_an_entry = true; } else { - BOOST_CHECK(stack.back()->HaveCoinsInCache(it->first)); + BOOST_CHECK(stack.back()->HaveCoinInCache(it->first)); found_an_entry = true; } } @@ -281,9 +281,9 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) stack.push_back(new CCoinsViewCacheTest(&base)); // Start with one cache. // Track the txids we've used in various sets - std::set coinbaseids; - std::set disconnectedids; - std::set duplicateids; + std::set coinbase_coins; + std::set disconnected_coins; + std::set duplicate_coins; std::set utxoset; for (unsigned int i = 0; i < NUM_SIMULATION_ITERATIONS; i++) { @@ -297,22 +297,22 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) tx.vout[0].nValue = i; //Keep txs unique unless intended to duplicate tx.vout[0].scriptPubKey.assign(insecure_rand() & 0x3F, 0); // Random sizes so we can test memory usage accounting unsigned int height = insecure_rand(); - Coin oldcoins; + Coin old_coin; // 2/20 times create a new coinbase - if (randiter % 20 < 2 || coinbaseids.size() < 10) { + if (randiter % 20 < 2 || coinbase_coins.size() < 10) { // 1/10 of those times create a duplicate coinbase - if (insecure_rand() % 10 == 0 && coinbaseids.size()) { - auto utxod = FindRandomFrom(coinbaseids); + if (insecure_rand() % 10 == 0 && coinbase_coins.size()) { + auto utxod = FindRandomFrom(coinbase_coins); // Reuse the exact same coinbase tx = std::get<0>(utxod->second); // shouldn't be available for reconnection if its been duplicated - disconnectedids.erase(utxod->first); + disconnected_coins.erase(utxod->first); - duplicateids.insert(utxod->first); + duplicate_coins.insert(utxod->first); } else { - coinbaseids.insert(COutPoint(tx.GetHash(), 0)); + coinbase_coins.insert(COutPoint(tx.GetHash(), 0)); } assert(CTransaction(tx).IsCoinBase()); } @@ -322,21 +322,21 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) COutPoint prevout; // 1/20 times reconnect a previously disconnected tx - if (randiter % 20 == 2 && disconnectedids.size()) { - auto utxod = FindRandomFrom(disconnectedids); + if (randiter % 20 == 2 && disconnected_coins.size()) { + auto utxod = FindRandomFrom(disconnected_coins); tx = std::get<0>(utxod->second); prevout = tx.vin[0].prevout; if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevout)) { - disconnectedids.erase(utxod->first); + disconnected_coins.erase(utxod->first); continue; } // If this tx is already IN the UTXO, then it must be a coinbase, and it must be a duplicate if (utxoset.count(utxod->first)) { assert(CTransaction(tx).IsCoinBase()); - assert(duplicateids.count(utxod->first)); + assert(duplicate_coins.count(utxod->first)); } - disconnectedids.erase(utxod->first); + disconnected_coins.erase(utxod->first); } // 16/20 times create a regular tx @@ -349,7 +349,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) assert(!CTransaction(tx).IsCoinBase()); } // In this simple test coins only have two states, spent or unspent, save the unspent state to restore - oldcoins = result[prevout]; + old_coin = result[prevout]; // Update the expected result of prevouthash to know these coins are spent result[prevout].Clear(); @@ -357,7 +357,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // The test is designed to ensure spending a duplicate coinbase will work properly // if that ever happens and not resurrect the previously overwritten coinbase - if (duplicateids.count(prevout)) { + if (duplicate_coins.count(prevout)) { spent_a_duplicate_coinbase = true; } @@ -375,21 +375,21 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) utxoset.insert(outpoint); // Track this tx and undo info to use later - utxoData.emplace(outpoint, std::make_tuple(tx,undo,oldcoins)); + utxoData.emplace(outpoint, std::make_tuple(tx,undo,old_coin)); } else if (utxoset.size()) { //1/20 times undo a previous transaction auto utxod = FindRandomFrom(utxoset); CTransaction &tx = std::get<0>(utxod->second); CTxUndo &undo = std::get<1>(utxod->second); - Coin &origcoins = std::get<2>(utxod->second); + Coin &orig_coin = std::get<2>(utxod->second); // Update the expected result // Remove new outputs result[utxod->first].Clear(); // If not coinbase restore prevout if (!tx.IsCoinBase()) { - result[tx.vin[0].prevout] = origcoins; + result[tx.vin[0].prevout] = orig_coin; } // Disconnect the tx from the current UTXO @@ -403,7 +403,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) ApplyTxInUndo(std::move(coin), *(stack.back()), out); } // Store as a candidate for reconnection - disconnectedids.insert(utxod->first); + disconnected_coins.insert(utxod->first); // Update the utxoset utxoset.erase(utxod->first); @@ -414,9 +414,9 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // Once every 1000 iterations and at the end, verify the full cache. if (insecure_rand() % 1000 == 1 || i == NUM_SIMULATION_ITERATIONS - 1) { for (auto it = result.begin(); it != result.end(); it++) { - bool have = stack.back()->HaveCoins(it->first); + bool have = stack.back()->HaveCoin(it->first); const Coin& coin = stack.back()->AccessCoin(it->first); - BOOST_CHECK(have == !coin.IsPruned()); + BOOST_CHECK(have == !coin.IsSpent()); BOOST_CHECK(coin == it->second); } } @@ -425,11 +425,11 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) if (utxoset.size() > 1 && insecure_rand() % 30) { stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(utxoset)->first); } - if (disconnectedids.size() > 1 && insecure_rand() % 30) { - stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(disconnectedids)->first); + if (disconnected_coins.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(disconnected_coins)->first); } - if (duplicateids.size() > 1 && insecure_rand() % 30) { - stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(duplicateids)->first); + if (duplicate_coins.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache(FindRandomFrom(duplicate_coins)->first); } if (insecure_rand() % 100 == 0) { @@ -537,11 +537,11 @@ void SetCoinsValue(CAmount value, Coin& coin) { assert(value != ABSENT); coin.Clear(); - assert(coin.IsPruned()); + assert(coin.IsSpent()); if (value != PRUNED) { coin.out.nValue = value; coin.nHeight = 1; - assert(!coin.IsPruned()); + assert(!coin.IsSpent()); } } @@ -567,7 +567,7 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags) value = ABSENT; flags = NO_ENTRY; } else { - if (it->second.coin.IsPruned()) { + if (it->second.coin.IsSpent()) { value = PRUNED; } else { value = it->second.coin.out.nValue; diff --git a/src/txdb.cpp b/src/txdb.cpp index 45a7d69a3..c8f509029 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -27,10 +27,10 @@ static const char DB_LAST_BLOCK = 'l'; namespace { -struct CoinsEntry { +struct CoinEntry { COutPoint* outpoint; char key; - CoinsEntry(const COutPoint* ptr) : outpoint(const_cast(ptr)), key(DB_COIN) {} + CoinEntry(const COutPoint* ptr) : outpoint(const_cast(ptr)), key(DB_COIN) {} template void Serialize(Stream &s) const { @@ -53,12 +53,12 @@ CCoinsViewDB::CCoinsViewDB(size_t nCacheSize, bool fMemory, bool fWipe) : db(Get { } -bool CCoinsViewDB::GetCoins(const COutPoint &outpoint, Coin &coin) const { - return db.Read(CoinsEntry(&outpoint), coin); +bool CCoinsViewDB::GetCoin(const COutPoint &outpoint, Coin &coin) const { + return db.Read(CoinEntry(&outpoint), coin); } -bool CCoinsViewDB::HaveCoins(const COutPoint &outpoint) const { - return db.Exists(CoinsEntry(&outpoint)); +bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { + return db.Exists(CoinEntry(&outpoint)); } uint256 CCoinsViewDB::GetBestBlock() const { @@ -74,8 +74,8 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { size_t changed = 0; for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { - CoinsEntry entry(&it->first); - if (it->second.coin.IsPruned()) + CoinEntry entry(&it->first); + if (it->second.coin.IsSpent()) batch.Erase(entry); else batch.Write(entry, it->second.coin); @@ -130,7 +130,7 @@ CCoinsViewCursor *CCoinsViewDB::Cursor() const i->pcursor->Seek(DB_COIN); // Cache key of first record if (i->pcursor->Valid()) { - CoinsEntry entry(&i->keyTmp.second); + CoinEntry entry(&i->keyTmp.second); i->pcursor->GetKey(entry); i->keyTmp.first = entry.key; } else { @@ -167,7 +167,7 @@ bool CCoinsViewDBCursor::Valid() const void CCoinsViewDBCursor::Next() { pcursor->Next(); - CoinsEntry entry(&keyTmp.second); + CoinEntry entry(&keyTmp.second); if (!pcursor->Valid() || !pcursor->GetKey(entry)) { keyTmp.first = 0; // Invalidate cached key after last record so that Valid() and GetKey() return false } else { diff --git a/src/txdb.h b/src/txdb.h index 51dc355bb..974dd4ebe 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -71,8 +71,8 @@ protected: public: CCoinsViewDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); - bool GetCoins(const COutPoint &outpoint, Coin &coin) const override; - bool HaveCoins(const COutPoint &outpoint) const override; + bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; CCoinsViewCursor *Cursor() const override; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 648b40a7a..81d213dca 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -525,8 +525,8 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem if (it2 != mapTx.end()) continue; const Coin &coin = pcoins->AccessCoin(txin.prevout); - if (nCheckFrequency != 0) assert(!coin.IsPruned()); - if (coin.IsPruned() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) { + if (nCheckFrequency != 0) assert(!coin.IsSpent()); + if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) { txToRemove.insert(it); break; } @@ -654,7 +654,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const parentSigOpCost += it2->GetSigOpCost(); } } else { - assert(pcoins->HaveCoins(txin.prevout)); + assert(pcoins->HaveCoin(txin.prevout)); } // Check whether its inputs are marked in mapNextTx. auto it3 = mapNextTx.find(txin.prevout); @@ -890,7 +890,7 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetCoins(const COutPoint &outpoint, Coin &coin) const { +bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { // If an entry in the mempool exists, always return that one, as it's guaranteed to never // conflict with the underlying cache, and it cannot have pruned entries (as it contains full) // transactions. First checking the underlying cache risks returning a pruned entry instead. @@ -903,11 +903,11 @@ bool CCoinsViewMemPool::GetCoins(const COutPoint &outpoint, Coin &coin) const { return false; } } - return (base->GetCoins(outpoint, coin) && !coin.IsPruned()); + return (base->GetCoin(outpoint, coin) && !coin.IsSpent()); } -bool CCoinsViewMemPool::HaveCoins(const COutPoint &outpoint) const { - return mempool.exists(outpoint) || base->HaveCoins(outpoint); +bool CCoinsViewMemPool::HaveCoin(const COutPoint &outpoint) const { + return mempool.exists(outpoint) || base->HaveCoin(outpoint); } size_t CTxMemPool::DynamicMemoryUsage() const { diff --git a/src/txmempool.h b/src/txmempool.h index c86859118..8cbc4dc8c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -679,8 +679,8 @@ protected: public: CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); - bool GetCoins(const COutPoint &outpoint, Coin &coin) const; - bool HaveCoins(const COutPoint &outpoint) const; + bool GetCoin(const COutPoint &outpoint, Coin &coin) const; + bool HaveCoin(const COutPoint &outpoint) const; }; #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/validation.cpp b/src/validation.cpp index 005eaafff..842abf5fb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -269,7 +269,7 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { const CTxIn& txin = tx.vin[txinIndex]; Coin coin; - if (!viewMemPool.GetCoins(txin.prevout, coin)) { + if (!viewMemPool.GetCoin(txin.prevout, coin)) { return error("%s: Missing input", __func__); } if (coin.nHeight == MEMPOOL_HEIGHT) { @@ -344,7 +344,7 @@ static bool IsCurrentForFeeEstimation() bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, - bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& vHashTxnToUncache) + bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& coins_to_uncache) { const CTransaction& tx = *ptx; const uint256 hash = tx.GetHash(); @@ -439,10 +439,10 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // do we already have it? for (size_t out = 0; out < tx.vout.size(); out++) { COutPoint outpoint(hash, out); - bool fHadTxInCache = pcoinsTip->HaveCoinsInCache(outpoint); - if (view.HaveCoins(outpoint)) { - if (!fHadTxInCache) { - vHashTxnToUncache.push_back(outpoint); + bool had_coin_in_cache = pcoinsTip->HaveCoinInCache(outpoint); + if (view.HaveCoin(outpoint)) { + if (!had_coin_in_cache) { + coins_to_uncache.push_back(outpoint); } return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-known"); } @@ -450,10 +450,10 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // do all inputs exist? BOOST_FOREACH(const CTxIn txin, tx.vin) { - if (!pcoinsTip->HaveCoinsInCache(txin.prevout)) { - vHashTxnToUncache.push_back(txin.prevout); + if (!pcoinsTip->HaveCoinInCache(txin.prevout)) { + coins_to_uncache.push_back(txin.prevout); } - if (!view.HaveCoins(txin.prevout)) { + if (!view.HaveCoin(txin.prevout)) { if (pfMissingInputs) { *pfMissingInputs = true; } @@ -763,10 +763,10 @@ bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, bool fOverrideMempoolLimit, const CAmount nAbsurdFee) { - std::vector vHashTxToUncache; - bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee, vHashTxToUncache); + std::vector coins_to_uncache; + bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee, coins_to_uncache); if (!res) { - BOOST_FOREACH(const COutPoint& hashTx, vHashTxToUncache) + BOOST_FOREACH(const COutPoint& hashTx, coins_to_uncache) pcoinsTip->Uncache(hashTx); } // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits @@ -819,7 +819,7 @@ bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it const Coin& coin = AccessByTxid(*pcoinsTip, hash); - if (!coin.IsPruned()) pindexSlow = chainActive[coin.nHeight]; + if (!coin.IsSpent()) pindexSlow = chainActive[coin.nHeight]; } if (pindexSlow) { @@ -1117,7 +1117,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi for (unsigned int i = 0; i < tx.vin.size(); i++) { const COutPoint &prevout = tx.vin[i].prevout; const Coin& coin = inputs.AccessCoin(prevout); - assert(!coin.IsPruned()); + assert(!coin.IsSpent()); // We very carefully only pass in things to CScriptCheck which // are clearly committed to by tx' witness hash. This provides @@ -1254,14 +1254,14 @@ int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out) { bool fClean = true; - if (view.HaveCoins(out)) fClean = false; // overwriting transaction output + if (view.HaveCoin(out)) fClean = false; // overwriting transaction output if (undo.nHeight == 0) { // Missing undo metadata (height and coinbase). Older versions included this // information only in undo records for the last spend of a transactions' // outputs. This implies that it must be present for some other output of the same tx. const Coin& alternate = AccessByTxid(view, out.hash); - if (!alternate.IsPruned()) { + if (!alternate.IsSpent()) { undo.nHeight = alternate.nHeight; undo.fCoinBase = alternate.fCoinBase; } else { @@ -1510,7 +1510,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd if (fEnforceBIP30) { for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { - if (view.HaveCoins(COutPoint(tx->GetHash(), o))) { + if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { return state.DoS(100, error("ConnectBlock(): tried to overwrite transaction"), REJECT_INVALID, "bad-txns-BIP30"); }