diff options
| -rw-r--r-- | contrib/test-patches/README.md | 7 | ||||
| -rw-r--r-- | contrib/test-patches/temp-revert-2.patch | 20 | ||||
| -rw-r--r-- | doc/README.md | 2 | ||||
| -rw-r--r-- | src/coins.cpp | 9 | ||||
| -rw-r--r-- | src/coins.h | 11 | ||||
| -rw-r--r-- | src/main.cpp | 15 | ||||
| -rw-r--r-- | src/qt/sendcoinsdialog.cpp | 2 | ||||
| -rw-r--r-- | src/qt/walletmodel.cpp | 6 | ||||
| -rw-r--r-- | src/test/coins_tests.cpp | 146 |
9 files changed, 177 insertions, 41 deletions
diff --git a/contrib/test-patches/README.md b/contrib/test-patches/README.md deleted file mode 100644 index def40b0d6c..0000000000 --- a/contrib/test-patches/README.md +++ /dev/null @@ -1,7 +0,0 @@ -### Test Patches ### - -These patches are applied when the automated pull-tester -tests each pull and when master is tested using jenkins. -You can find more information about the tests run at -[http://jenkins.bluematt.me/pull-tester/files/ -](http://jenkins.bluematt.me/pull-tester/files/)
\ No newline at end of file diff --git a/contrib/test-patches/temp-revert-2.patch b/contrib/test-patches/temp-revert-2.patch deleted file mode 100644 index 1cd043d0d7..0000000000 --- a/contrib/test-patches/temp-revert-2.patch +++ /dev/null @@ -1,20 +0,0 @@ -commit cfae26916dba311f6f75d444301c1f9362267c3e -Author: Matt Corallo <git@bluematt.me> -Date: Sun Mar 24 20:45:50 2013 -0400 - - Revert "Checkpoint at first block in 11 March chain fork" - - This reverts commit f817c496a1482d05b22c8e539de67f07db1c09d9. - -diff --git a/src/checkpoints.cpp b/src/checkpoints.cpp -index 62234b9..9b11f0b 100644 ---- a/src/checkpoints.cpp -+++ b/src/checkpoints.cpp -@@ -44,7 +44,6 @@ namespace Checkpoints - (193000, uint256("0x000000000000059f452a5f7340de6682a977387c17010ff6e6c3bd83ca8b1317")) - (210000, uint256("0x000000000000048b95347e83192f69cf0366076336c639f9b7228e9ba171342e")) - (216116, uint256("0x00000000000001b4f4b433e81ee46494af945cf96014816a4e2370f11b23df4e")) -- (225430, uint256("0x00000000000001c108384350f74090433e7fcf79a606b8e797f065b130575932")) - ; - static const CCheckpointData data = { - &mapCheckpoints, diff --git a/doc/README.md b/doc/README.md index 0594d20dd4..f6df28a89b 100644 --- a/doc/README.md +++ b/doc/README.md @@ -28,7 +28,7 @@ Unpack the files into a directory, and then run bitcoin-qt.exe. ### OS X -Drag Bitcoin-Qt to your applications folder, and then run Bitcoin-Qt. +Drag Bitcoin-Core to your applications folder, and then run Bitcoin-Core. ### Need Help? diff --git a/src/coins.cpp b/src/coins.cpp index f02949de53..96b336ce77 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -117,6 +117,15 @@ CCoinsModifier CCoinsViewCache::ModifyCoins(const uint256 &txid) { return CCoinsModifier(*this, ret.first, cachedCoinUsage); } +CCoinsModifier CCoinsViewCache::ModifyNewCoins(const uint256 &txid) { + assert(!hasModifier); + std::pair<CCoinsMap::iterator, bool> ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())); + ret.first->second.coins.Clear(); + ret.first->second.flags = CCoinsCacheEntry::FRESH; + ret.first->second.flags |= CCoinsCacheEntry::DIRTY; + return CCoinsModifier(*this, ret.first, 0); +} + const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { CCoinsMap::const_iterator it = FetchCoins(txid); if (it == cacheCoins.end()) { diff --git a/src/coins.h b/src/coins.h index bf4a777b8a..3b45cb0a34 100644 --- a/src/coins.h +++ b/src/coins.h @@ -420,6 +420,17 @@ public: 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); + + /** * Push the modifications applied to this cache to its base. * Failure to call this method before destruction will cause the changes to be forgotten. * If false is returned, the state of this cache (and its backing view) will be undefined. diff --git a/src/main.cpp b/src/main.cpp index a1f326fb13..8fb121c00d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1310,10 +1310,17 @@ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCach undo.nVersion = coins->nVersion; } } + // add outputs + inputs.ModifyNewCoins(tx.GetHash())->FromTx(tx, nHeight); + } + else { + // add outputs for coinbase tx + // In this case call the full ModifyCoins which will do a database + // lookup to be sure the coins do not already exist otherwise we do not + // know whether to mark them fresh or not. We want the duplicate coinbases + // before BIP30 to still be properly overwritten. + inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight); } - - // add outputs - inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight); } void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight) @@ -2579,7 +2586,7 @@ bool FindBlockPos(CValidationState &state, CDiskBlockPos &pos, unsigned int nAdd if ((int)nFile != nLastBlockFile) { if (!fKnown) { - LogPrintf("Leaving block file %i: %s\n", nFile, vinfoBlockFile[nFile].ToString()); + LogPrintf("Leaving block file %i: %s\n", nLastBlockFile, vinfoBlockFile[nLastBlockFile].ToString()); } FlushBlockFile(!fKnown); nLastBlockFile = nFile; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 0ee08a1b0c..4b030fdaa0 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -531,7 +531,7 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn msgParams.second = CClientUIInterface::MSG_ERROR; break; case WalletModel::AbsurdFee: - msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), 10000000)); + msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), maxTxFee)); break; case WalletModel::PaymentRequestExpired: msgParams.first = tr("Payment request expired."); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 5c21db8bdf..690ea0811e 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -290,8 +290,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact return TransactionCreationFailed; } - // reject absurdly high fee > 0.1 bitcoin - if (nFeeRequired > 10000000) + // reject absurdly high fee. (This can never happen because the + // wallet caps the fee at maxTxFee. This merely serves as a + // belt-and-suspenders check) + if (nFeeRequired > maxTxFee) return AbsurdFee; } diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 13d848311a..946f904dfa 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -6,6 +6,8 @@ #include "random.h" #include "uint256.h" #include "test/test_bitcoin.h" +#include "main.h" +#include "consensus/validation.h" #include <vector> #include <map> @@ -45,15 +47,18 @@ public: bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) { for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { - map_[it->first] = it->second.coins; - if (it->second.coins.IsPruned() && insecure_rand() % 3 == 0) { - // Randomly delete empty entries on write. - map_.erase(it->first); + 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) { + // Randomly delete empty entries on write. + map_.erase(it->first); + } } mapCoins.erase(it++); } - mapCoins.clear(); - hashBestBlock_ = hashBlock; + if (!hashBlock.IsNull()) + hashBestBlock_ = hashBlock; return true; } @@ -197,4 +202,133 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) BOOST_CHECK(missed_an_entry); } +// This test is similar to the previous test +// except the emphasis is on testing the functionality of UpdateCoins +// random txs are created and UpdateCoins is used to update the cache stack +// In particular it is tested that spending a duplicate coinbase tx +// has the expected effect (the other duplicate is overwitten at all cache levels) +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<uint256, CCoins> result; + + // The cache stack. + CCoinsViewTest base; // A CCoinsViewTest at the bottom. + std::vector<CCoinsViewCacheTest*> stack; // A stack of CCoinsViewCaches on top. + stack.push_back(new CCoinsViewCacheTest(&base)); // Start with one cache. + + // Track the txids we've used and whether they have been spent or not + std::map<uint256, CAmount> coinbaseids; + std::set<uint256> alltxids; + std::set<uint256> duplicateids; + + for (unsigned int i = 0; i < NUM_SIMULATION_ITERATIONS; i++) { + { + CMutableTransaction tx; + tx.vin.resize(1); + tx.vout.resize(1); + tx.vout[0].nValue = i; //Keep txs unique unless intended to duplicate + unsigned int height = insecure_rand(); + + // 1/10 times create a coinbase + if (insecure_rand() % 10 == 0 || coinbaseids.size() < 10) { + // 1/100 times create a duplicate coinbase + if (insecure_rand() % 10 == 0 && coinbaseids.size()) { + std::map<uint256, CAmount>::iterator coinbaseIt = coinbaseids.lower_bound(GetRandHash()); + if (coinbaseIt == coinbaseids.end()) { + coinbaseIt = coinbaseids.begin(); + } + //Use same random value to have same hash and be a true duplicate + tx.vout[0].nValue = coinbaseIt->second; + assert(tx.GetHash() == coinbaseIt->first); + duplicateids.insert(coinbaseIt->first); + } + else { + coinbaseids[tx.GetHash()] = tx.vout[0].nValue; + } + assert(CTransaction(tx).IsCoinBase()); + } + // 9/10 times create a regular tx + else { + uint256 prevouthash; + // equally likely to spend coinbase or non coinbase + std::set<uint256>::iterator txIt = alltxids.lower_bound(GetRandHash()); + if (txIt == alltxids.end()) { + txIt = alltxids.begin(); + } + prevouthash = *txIt; + + // Construct the tx to spend the coins of prevouthash + tx.vin[0].prevout.hash = prevouthash; + tx.vin[0].prevout.n = 0; + + // Update the expected result of prevouthash to know these coins are spent + CCoins& oldcoins = result[prevouthash]; + oldcoins.Clear(); + + // It is of particular importance here that once we spend a coinbase tx hash + // it is no longer available to be duplicated (or spent again) + // BIP 34 in conjunction with enforcing BIP 30 (at least until BIP 34 was active) + // results in the fact that no coinbases were duplicated after they were already spent + alltxids.erase(prevouthash); + coinbaseids.erase(prevouthash); + + // 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)) + spent_a_duplicate_coinbase = true; + + assert(!CTransaction(tx).IsCoinBase()); + } + // Track this tx to possibly spend later + alltxids.insert(tx.GetHash()); + + // Update the expected result to know about the new output coins + CCoins &coins = result[tx.GetHash()]; + coins.FromTx(tx, height); + + CValidationState dummy; + UpdateCoins(tx, dummy, *(stack.back()), height); + } + + // 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<uint256, CCoins>::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()); + } + } + } + + if (insecure_rand() % 100 == 0) { + // Every 100 iterations, change the cache stack. + if (stack.size() > 0 && insecure_rand() % 2 == 0) { + stack.back()->Flush(); + delete stack.back(); + stack.pop_back(); + } + if (stack.size() == 0 || (stack.size() < 4 && insecure_rand() % 2)) { + CCoinsView* tip = &base; + if (stack.size() > 0) { + tip = stack.back(); + } + stack.push_back(new CCoinsViewCacheTest(tip)); + } + } + } + + // Clean up the stack. + while (stack.size() > 0) { + delete stack.back(); + stack.pop_back(); + } + + // Verify coverage. + BOOST_CHECK(spent_a_duplicate_coinbase); +} + BOOST_AUTO_TEST_SUITE_END() |