diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp --- a/src/bench/ccoins_caching.cpp +++ b/src/bench/ccoins_caching.cpp @@ -36,8 +36,7 @@ dummyTransactions[0].vout[1].nValue = 50 * CENT; dummyTransactions[0].vout[1].scriptPubKey << ToByteVector(key[1].GetPubKey()) << OP_CHECKSIG; - coinsRet.ModifyCoins(dummyTransactions[0].GetId()) - ->FromTx(dummyTransactions[0], 0); + AddCoins(coinsRet, dummyTransactions[0], 0); dummyTransactions[1].vout.resize(2); dummyTransactions[1].vout[0].nValue = 21 * CENT; @@ -46,8 +45,7 @@ dummyTransactions[1].vout[1].nValue = 22 * CENT; dummyTransactions[1].vout[1].scriptPubKey = GetScriptForDestination(key[3].GetPubKey().GetID()); - coinsRet.ModifyCoins(dummyTransactions[1].GetId()) - ->FromTx(dummyTransactions[1], 0); + AddCoins(coinsRet, dummyTransactions[1], 0); return dummyTransactions; } diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -622,29 +622,29 @@ 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.IsSpent() && + coin.GetTxOut().scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); - err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + + err = err + ScriptToAsmStr(coin.GetTxOut().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; + CTxOut txout; + txout.scriptPubKey = scriptPubKey; + txout.nValue = 0; if (prevOut.exists("amount")) { - coins->vout[nOut].nValue = AmountFromValue(prevOut["amount"]); + txout.nValue = AmountFromValue(prevOut["amount"]); } + + view.AddCoin(out, Coin(txout, 1, false), true); } // If redeemScript given and private keys given, add redeemScript to the diff --git a/src/coins.h b/src/coins.h --- a/src/coins.h +++ b/src/coins.h @@ -516,13 +516,6 @@ */ 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); - /** * Add a coin. Set potential_overwrite to true if a non-pruned version may * already exist. diff --git a/src/coins.cpp b/src/coins.cpp --- a/src/coins.cpp +++ b/src/coins.cpp @@ -123,29 +123,6 @@ 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_DONOTUSE(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); -} - void CCoinsViewCache::AddCoin(const COutPoint &outpoint, const Coin &coin, bool possible_overwrite) { assert(!coin.IsSpent()); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -915,27 +915,25 @@ "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.IsSpent() && + coin.GetTxOut().scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); - err = err + ScriptToAsmStr(coins->vout[nOut].scriptPubKey) + + err = err + ScriptToAsmStr(coin.GetTxOut().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; + CTxOut txout; + txout.scriptPubKey = scriptPubKey; + txout.nValue = 0; if (prevOut.exists("amount")) { - coins->vout[nOut].nValue = + txout.nValue = AmountFromValue(find_value(prevOut, "amount")); } else { // amount param is required in replay-protected txs. @@ -949,6 +947,8 @@ // eg getbalance returns "3.14152" rather than 3.14152 throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing amount"); } + + view.AddCoin(out, Coin(txout, 1, false), true); } // If redeemScript given and not using the local wallet (private diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -30,14 +30,6 @@ a.GetTxOut() == b.GetTxOut(); } -//! equality test -bool operator==(const Coin &c, const CCoins &coins) { - if (coins.vout.empty()) { - return c.IsSpent(); - } - return c == Coin(coins.vout[0], coins.nHeight, coins.fCoinBase); -} - class CCoinsViewTest : public CCoinsView { uint256 hashBestBlock_; std::map map_; @@ -119,6 +111,7 @@ 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; @@ -126,7 +119,7 @@ bool uncached_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. // A CCoinsViewTest at the bottom. @@ -140,7 +133,7 @@ // entries. std::vector txids; txids.resize(NUM_SIMULATION_ITERATIONS / 8); - for (unsigned int i = 0; i < txids.size(); i++) { + for (size_t i = 0; i < txids.size(); i++) { txids[i] = GetRandHash(); } @@ -149,22 +142,35 @@ { // txid we're going to modify in this iteration. uint256 txid = txids[insecure_rand() % txids.size()]; - CCoins &coins = result[txid]; - CCoinsModifier entry = stack.back()->ModifyCoins(txid); - BOOST_CHECK(coins == *entry); - if (insecure_rand() % 5 == 0 || coins.IsPruned()) { - if (coins.IsPruned()) { - added_an_entry = true; + Coin &coin = result[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.IsSpent()) { + CTxOut txout; + txout.nValue = insecure_rand(); + if (insecure_rand() % 16 == 0 && coin.IsSpent()) { + txout.scriptPubKey.assign(1 + (insecure_rand() & 0x3F), + OP_RETURN); + BOOST_CHECK(txout.scriptPubKey.IsUnspendable()); + added_an_unspendable_entry = true; } else { - updated_an_entry = true; + // Random sizes so we can test memory usage accounting + txout.scriptPubKey.assign(insecure_rand() & 0x3F, 0); + (coin.IsSpent() ? added_an_entry : updated_an_entry) = true; + coin = Coin(txout, 1, false); } - coins.vout.resize(1); - coins.vout[0].nValue = insecure_rand(); - *entry = coins; + + Coin newcoin(txout, 1, false); + stack.back()->AddCoin(COutPoint(txid, 0), newcoin, + !coin.IsSpent() || insecure_rand() & 1); } else { - coins.Clear(); - entry->Clear(); removed_an_entry = true; + coin.Clear(); + stack.back()->SpendCoin(COutPoint(txid, 0)); } } @@ -179,16 +185,14 @@ // 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()->HaveCoin(COutPoint(it->first, 0)); - const Coin &coin = - stack.back()->AccessCoin(COutPoint(it->first, 0)); + bool have = stack.back()->HaveCoin(it->first); + const Coin &coin = stack.back()->AccessCoin(it->first); BOOST_CHECK(have == !coin.IsSpent()); BOOST_CHECK(coin == it->second); if (coin.IsSpent()) { missed_an_entry = true; } else { - BOOST_CHECK( - stack.back()->HaveCoinInCache(COutPoint(it->first, 0))); + BOOST_CHECK(stack.back()->HaveCoinInCache(it->first)); found_an_entry = true; } } @@ -197,8 +201,8 @@ } } + // Every 100 iterations, flush an intermediate cache if (insecure_rand() % 100 == 0) { - // Every 100 iterations, flush an intermediate cache if (stack.size() > 1 && insecure_rand() % 2 == 0) { unsigned int flushIndex = insecure_rand() % (stack.size() - 1); stack[flushIndex]->Flush(); @@ -239,6 +243,7 @@ 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); @@ -246,19 +251,19 @@ BOOST_CHECK(uncached_an_entry); } -typedef std::tuple TxData; // Store of all necessary tx and undo data for next test -std::map alltxs; - -TxData &FindRandomFrom(const std::set &txidset) { - assert(txidset.size()); - std::set::iterator txIt = txidset.lower_bound(GetRandHash()); - if (txIt == txidset.end()) { - txIt = txidset.begin(); +typedef std::map> UtxoData; +UtxoData utxoData; + +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; } // This test is similar to the previous test except the emphasis is on testing @@ -269,7 +274,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. // A CCoinsViewTest at the bottom. @@ -280,10 +285,10 @@ stack.push_back(new CCoinsViewCacheTest(&base)); // Track the txids we've used in various sets - std::set coinbaseids; - std::set disconnectedids; - std::set duplicateids; - std::set utxoset; + 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++) { uint32_t randiter = insecure_rand(); @@ -298,22 +303,22 @@ // Random sizes so we can test memory usage accounting tx.vout[0].scriptPubKey.assign(insecure_rand() & 0x3F, 0); unsigned int height = insecure_rand(); - CCoins 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()) { - TxData &txd = FindRandomFrom(coinbaseids); + if (insecure_rand() % 10 == 0 && coinbase_coins.size()) { + auto utxod = FindRandomFrom(coinbase_coins); // 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.GetId()); + disconnected_coins.erase(utxod->first); - duplicateids.insert(tx.GetId()); + duplicate_coins.insert(utxod->first); } else { - coinbaseids.insert(tx.GetId()); + coinbase_coins.insert(COutPoint(tx.GetId(), 0)); } assert(CTransaction(tx).IsCoinBase()); } @@ -321,120 +326,132 @@ // 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 (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(prevouthash)) { - disconnectedids.erase(tx.GetId()); + !utxoset.count(prevout)) { + 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(tx.GetId())) { + if (utxoset.count(utxod->first)) { assert(CTransaction(tx).IsCoinBase()); - assert(duplicateids.count(tx.GetId())); + assert(duplicate_coins.count(utxod->first)); } - disconnectedids.erase(tx.GetId()); + disconnected_coins.erase(utxod->first); } // 16/20 times create a regular tx else { - TxData &txd = FindRandomFrom(utxoset); - prevouthash = std::get<0>(txd).GetId(); + 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 = prevout; tx.vin[0].prevout.n = 0; 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]; + old_coin = 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 (duplicate_coins.count(prevout)) { spent_a_duplicate_coinbase = true; + } } // Update the expected result to know about the new output coins - result[tx.GetId()].FromTx(tx, height); + assert(tx.vout.size() == 1); + const COutPoint outpoint(tx.GetId(), 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.GetId()); + utxoset.insert(outpoint); // Track this tx and undo info to use later - alltxs.insert(std::make_pair(tx.GetId(), - std::make_tuple(tx, undo, oldcoins))); + utxoData.emplace(outpoint, std::make_tuple(tx, undo, old_coin)); } // 1/20 times undo a previous transaction else if (utxoset.size()) { - TxData &txd = FindRandomFrom(utxoset); - - CTransaction &tx = std::get<0>(txd); - CTxUndo &undo = std::get<1>(txd); - CCoins &origcoins = std::get<2>(txd); + auto utxod = FindRandomFrom(utxoset); - uint256 undohash = tx.GetId(); + CTransaction &tx = std::get<0>(utxod->second); + CTxUndo &undo = std::get<1>(utxod->second); + Coin &orig_coin = 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] = orig_coin; } // Disconnect the tx from the current UTXO // See code in DisconnectBlock // remove outputs - { - CCoinsModifier outs = stack.back()->ModifyCoins(undohash); - outs->Clear(); - } + stack.back()->SpendCoin(utxod->first); + // restore inputs if (!tx.IsCoinBase()) { const COutPoint &out = tx.vin[0].prevout; - const Coin &undoin = undo.vprevout[0]; - UndoCoinSpend(undoin, *(stack.back()), out); + UndoCoinSpend(undo.vprevout[0], *(stack.back()), out); } // Store as a candidate for reconnection - disconnectedids.insert(undohash); + disconnected_coins.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 (auto it = result.begin(); it != result.end(); it++) { - bool have = stack.back()->HaveCoin(COutPoint(it->first, 0)); - const Coin &coin = - stack.back()->AccessCoin(COutPoint(it->first, 0)); + bool have = stack.back()->HaveCoin(it->first); + const Coin &coin = stack.back()->AccessCoin(it->first); BOOST_CHECK(have == !coin.IsSpent()); 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 (disconnected_coins.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache( + FindRandomFrom(disconnected_coins)->first); + } + if (duplicate_coins.size() > 1 && insecure_rand() % 30) { + stack[insecure_rand() % stack.size()]->Uncache( + FindRandomFrom(duplicate_coins)->first); + } + if (insecure_rand() % 100 == 0) { // Every 100 iterations, flush an intermediate cache if (stack.size() > 1 && insecure_rand() % 2 == 0) { @@ -737,90 +754,6 @@ 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) { - SingleEntryCacheTest test(base_value, cache_value, cache_flags); - SetCoinsValue(modify_value, *test.cache.ModifyCoins(TXID)); - test.cache.SelfTest(); - - CAmount result_value; - char result_flags; - GetCoinsMapEntry(test.cache.map(), result_value, result_flags); - BOOST_CHECK_EQUAL(result_value, expected_value); - BOOST_CHECK_EQUAL(result_flags, expected_flags); -}; - -BOOST_AUTO_TEST_CASE(ccoins_modify) { - /* 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 - * the resulting entry in the cache after the modification. - * - * Base Cache Write Result Cache Result - * Value 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); -} - void CheckSpendCoins(CAmount base_value, CAmount cache_value, CAmount expected_value, char cache_flags, char expected_flags) { diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -378,7 +378,7 @@ GetScriptForDestination(CScriptID(twentySigops)); txFrom.vout[6].nValue = 6000; - coins.ModifyCoins(txFrom.GetId())->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 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -104,7 +104,7 @@ spendingTx.vout[0].nValue = 1; spendingTx.vout[0].scriptPubKey = CScript(); - coins.ModifyCoins(creationTx.GetId())->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 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -276,8 +276,7 @@ dummyTransactions[0].vout[1].nValue = 50 * CENT; dummyTransactions[0].vout[1].scriptPubKey << ToByteVector(key[1].GetPubKey()) << OP_CHECKSIG; - coinsRet.ModifyCoins(dummyTransactions[0].GetId()) - ->FromTx(dummyTransactions[0], 0); + AddCoins(coinsRet, dummyTransactions[0], 0); dummyTransactions[1].vout.resize(2); dummyTransactions[1].vout[0].nValue = 21 * CENT; @@ -286,8 +285,7 @@ dummyTransactions[1].vout[1].nValue = 22 * CENT; dummyTransactions[1].vout[1].scriptPubKey = GetScriptForDestination(key[3].GetPubKey().GetID()); - coinsRet.ModifyCoins(dummyTransactions[1].GetId()) - ->FromTx(dummyTransactions[1], 0); + AddCoins(coinsRet, dummyTransactions[1], 0); return dummyTransactions; } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1500,39 +1500,30 @@ const COutPoint &out) { bool fClean = true; - CCoinsModifier coins = view.ModifyCoins(out.hash); - if (undo.GetHeight() != 0) { - if (!coins->IsPruned()) { - if (coins->fCoinBase != undo.IsCoinBase() || - uint32_t(coins->nHeight) != undo.GetHeight()) { - // Metadata mismatch. - fClean = false; - } - } - - // Restore height/coinbase tx metadata from undo data. - coins->fCoinBase = undo.IsCoinBase(); - coins->nHeight = undo.GetHeight(); - } 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()) { - // Adding output to missing transaction. - fClean = false; - } - } - - if (coins->IsAvailable(out.n)) { - // Overwriting existing output. + if (view.HaveCoin(out)) { + // Overwriting transaction output. fClean = false; } - if (coins->vout.size() < out.n + 1) { - coins->vout.resize(out.n + 1); + if (undo.GetHeight() == 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.IsSpent()) { + // Adding output for transaction without known metadata + return DISCONNECT_FAILED; + } + + // This is somewhat ugly, but hopefully utility is limited. This is only + // useful when working from legacy on disck data. In any case, putting + // the correct information in there doesn't hurt. + const_cast(undo) = Coin(undo.GetTxOut(), alternate.GetHeight(), + alternate.IsCoinBase()); } - coins->vout[out.n] = undo.GetTxOut(); + view.AddCoin(out, undo, undo.IsCoinBase()); return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } @@ -1579,18 +1570,18 @@ // Check that all outputs are available and match the outputs in the // block itself exactly. - { - CCoinsModifier outs = view.ModifyCoins(txid); - outs->ClearUnspendable(); + for (size_t o = 0; o < tx.vout.size(); o++) { + if (tx.vout[o].scriptPubKey.IsUnspendable()) { + continue; + } - CCoins outsBlock(tx, pindex->nHeight); - if (*outs != outsBlock) { - // Transaction mismatch. + COutPoint out(txid, o); + Coin coin; + bool is_spent = view.SpendCoin(out, &coin); + if (!is_spent || tx.vout[o] != coin.GetTxOut()) { + // transaction output mismatch fClean = false; } - - // Remove outputs. - outs->Clear(); } // Restore inputs.