diff --git a/src/coins.cpp b/src/coins.cpp index 80e05864f..17ab90ed2 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -11,11 +11,15 @@ #include 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; } +bool CCoinsView::HaveCoin(const COutPoint &outpoint) const +{ + Coin coin; + return GetCoin(outpoint, coin); +} CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } bool CCoinsViewBacked::GetCoin(const COutPoint &outpoint, Coin &coin) const { return base->GetCoin(outpoint, coin); } @@ -55,7 +59,7 @@ 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; + return !coin.IsSpent(); } return false; } diff --git a/src/coins.h b/src/coins.h index 7753f0b81..7b6395a40 100644 --- a/src/coins.h +++ b/src/coins.h @@ -146,11 +146,13 @@ private: class CCoinsView { public: - //! Retrieve the Coin (unspent transaction output) for a given outpoint. + /** Retrieve the Coin (unspent transaction output) for a given outpoint. + * Returns true only when an unspent coin was found, which is returned in coin. + * When false is returned, coin's value is unspecified. + */ 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. + //! Just check whether a given outpoint is unspent. virtual bool HaveCoin(const COutPoint &outpoint) const; //! Retrieve the block hash whose state this CCoinsView currently represents diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 3d9c9e5b9..90a01dd41 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -51,12 +51,6 @@ public: return true; } - bool HaveCoin(const COutPoint& outpoint) const override - { - Coin coin; - return GetCoin(outpoint, coin); - } - uint256 GetBestBlock() const override { return hashBestBlock_; } bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override @@ -148,8 +142,22 @@ 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)]; + + // Determine whether to test HaveCoin before or after Access* (or both). As these functions + // can influence each other's behaviour by pulling things into the cache, all combinations + // are tested. + bool test_havecoin_before = (insecure_rand() & 0x3) == 0; // TODO change to InsecureRandBits(2) when backporting Bitcoin #10321 + bool test_havecoin_after = (insecure_rand() & 0x3) == 0; // TODO change to InsecureRandBits(2) when backporting Bitcoin #10321 + + bool result_havecoin = test_havecoin_before ? stack.back()->HaveCoin(COutPoint(txid, 0)) : false; const Coin& entry = (insecure_rand() % 500 == 0) ? AccessByTxid(*stack.back(), txid) : stack.back()->AccessCoin(COutPoint(txid, 0)); BOOST_CHECK(coin == entry); + BOOST_CHECK(!test_havecoin_before || result_havecoin == !entry.IsSpent()); + + if (test_havecoin_after) { + bool ret = stack.back()->HaveCoin(COutPoint(txid, 0)); + BOOST_CHECK(ret == !entry.IsSpent()); + } if (insecure_rand() % 5 == 0 || coin.IsSpent()) { Coin newcoin; @@ -631,7 +639,7 @@ BOOST_AUTO_TEST_CASE(ccoins_access) 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, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY ); CheckAccessCoin(PRUNED, PRUNED, PRUNED, 0 , 0 ); CheckAccessCoin(PRUNED, PRUNED, PRUNED, FRESH , FRESH ); CheckAccessCoin(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY ); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 432b45da8..c8830ebcb 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -988,11 +988,7 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; } } - return (base->GetCoin(outpoint, coin) && !coin.IsSpent()); -} - -bool CCoinsViewMemPool::HaveCoin(const COutPoint &outpoint) const { - return mempool.exists(outpoint) || base->HaveCoin(outpoint); + return base->GetCoin(outpoint, coin); } size_t CTxMemPool::DynamicMemoryUsage() const { diff --git a/src/txmempool.h b/src/txmempool.h index eb487ef3c..3e5c22060 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -663,8 +663,7 @@ protected: public: CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn); - bool GetCoin(const COutPoint &outpoint, Coin &coin) const; - bool HaveCoin(const COutPoint &outpoint) const; + bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; }; // We want to sort transactions by coin age priority