From f54580e7e4f225bb615204daef32f72ab8688418 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:09 -0700 Subject: 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/validation.cpp | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) (limited to 'src/validation.cpp') diff --git a/src/validation.cpp b/src/validation.cpp index ac16af3ee..ed94be5c2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1248,46 +1248,42 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std } // anon namespace +enum DisconnectResult +{ + DISCONNECT_OK, // All good. + DISCONNECT_UNCLEAN, // Rolled back, but UTXO set was inconsistent with block. + 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 True on success. + * @return A DisconnectResult as an int */ -bool ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint& out) +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 = fClean && error("%s: undo data overwriting existing transaction", __func__); - coins->Clear(); + 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 = fClean && error("%s: undo data adding output to missing transaction", __func__); + if (coins->IsPruned()) fClean = false; // adding output to missing transaction } - if (coins->IsAvailable(out.n)) - fClean = fClean && error("%s: undo data overwriting existing output", __func__); + 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; + return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } -enum DisconnectResult -{ - DISCONNECT_OK, // All good. - DISCONNECT_UNCLEAN, // Rolled back, but UTXO set was inconsistent with block. - DISCONNECT_FAILED // Something else went wrong. -}; - /** 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; } } } -- cgit v1.2.3 From e484652fc36ef7135cf08ad380ea7142b6cbadc0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:14 -0700 Subject: 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/validation.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/validation.cpp') 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; -- cgit v1.2.3 From d342424301013ec47dc146a4beb49d5c9319d80a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:18 -0700 Subject: 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. --- src/validation.cpp | 7 ------- 1 file changed, 7 deletions(-) (limited to 'src/validation.cpp') 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 -- cgit v1.2.3 From 7d991b55dbf0b0f6e21c0680ee3ebd09df09012f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:19 -0700 Subject: 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/validation.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'src/validation.cpp') 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 -- cgit v1.2.3 From cb2c7fdac2dc74368ed24ae4717ed72178956b92 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:25 -0700 Subject: 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/validation.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'src/validation.cpp') 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; -- cgit v1.2.3 From bd83111a0fcfdb97204a0180bcf861d3b53bb6c2 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:27 -0700 Subject: Optimization: Coin&& to ApplyTxInUndo This avoids a prevector copy in ApplyTxInUndo. --- src/validation.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/validation.cpp') 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. } } -- cgit v1.2.3 From f68cdfe92b37f5a75be612b7de3c1a03245696d0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:30 -0700 Subject: Switch from per-tx to per-txout CCoinsViewCache methods in some places --- src/validation.cpp | 87 ++++++++++++++++++++++-------------------------------- 1 file changed, 36 insertions(+), 51 deletions(-) (limited to 'src/validation.cpp') 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)) { -- cgit v1.2.3 From c87b957a32e03c09d410abadf661f87eb813bcdb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 27 Apr 2017 10:37:33 -0400 Subject: 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/validation.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src/validation.cpp') 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()))); -- cgit v1.2.3 From 8b3868c1b4bf89c41b26ecb3a4b7c3a2557e3868 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:32 -0700 Subject: Switch CScriptCheck to use Coin instead of CCoins --- src/validation.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/validation.cpp') 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); -- cgit v1.2.3 From 50830796889ecaa458871f1db878c255dd2554cb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:39 -0700 Subject: 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/validation.cpp | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'src/validation.cpp') 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,30 +437,30 @@ 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"); + 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"); + } } // 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) + 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() } } - // are the actual inputs available? - if (!view.HaveInputs(tx)) - return state.Invalid(false, REJECT_DUPLICATE, "bad-txns-inputs-spent"); - // 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()), -- cgit v1.2.3 From b2af357f39c7d17ab6ddb2938531155bf90126ec Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 25 Apr 2017 11:29:44 -0700 Subject: 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/validation.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/validation.cpp') 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. -- cgit v1.2.3 From 589827975f9f241e2f23eb674a7383592bff1cad Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 30 May 2017 17:58:54 -0700 Subject: 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/validation.cpp | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'src/validation.cpp') 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"); } -- cgit v1.2.3