Skip to content

Commit 21180ff

Browse files
sipasipa
committed
Simplify return values of GetCoin/HaveCoin(InCache)
This removes the possibility for GetCoin/HaveCoin/HaveCoinInCache to return true while the respective coin is spent. By doing it across all calls, some extra checks can be eliminated. coins_tests is modified to call HaveCoin sometimes before and sometimes after AccessCoin. A further change is needed because the semantics for GetCoin slightly changed, causing a pruned entry in the parent cache to not be pulled into the child in FetchCoin.
1 parent 234ffc6 commit 21180ff

File tree

5 files changed

+28
-19
lines changed

5 files changed

+28
-19
lines changed

‎src/coins.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@
1111
#include <assert.h>
1212

1313
bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; }
14-
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const { return false; }
1514
uint256 CCoinsView::GetBestBlock() const { return uint256(); }
1615
bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; }
1716
CCoinsViewCursor *CCoinsView::Cursor() const { return 0; }
1817

18+
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
19+
{
20+
Coin coin;
21+
return GetCoin(outpoint, coin);
22+
}
1923

2024
CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { }
2125
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 {
5559
CCoinsMap::const_iterator it = FetchCoin(outpoint);
5660
if (it != cacheCoins.end()) {
5761
coin = it->second.coin;
58-
return true;
62+
return !coin.IsSpent();
5963
}
6064
return false;
6165
}

‎src/coins.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,13 @@ class CCoinsViewCursor
145145
class CCoinsView
146146
{
147147
public:
148-
//! Retrieve the Coin (unspent transaction output) for a given outpoint.
148+
/** Retrieve the Coin (unspent transaction output) for a given outpoint.
149+
* Returns true only when an unspent coin was found, which is returned in coin.
150+
* When false is returned, coin's value is unspecified.
151+
*/
149152
virtual bool GetCoin(const COutPoint &outpoint, Coin &coin) const;
150153

151-
//! Just check whether we have data for a given outpoint.
152-
//! This may (but cannot always) return true for spent outputs.
154+
//! Just check whether a given outpoint is unspent.
153155
virtual bool HaveCoin(const COutPoint &outpoint) const;
154156

155157
//! Retrieve the block hash whose state this CCoinsView currently represents

‎src/test/coins_tests.cpp‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,6 @@ class CCoinsViewTest : public CCoinsView
5050
return true;
5151
}
5252

53-
bool HaveCoin(const COutPoint& outpoint) const override
54-
{
55-
Coin coin;
56-
return GetCoin(outpoint, coin);
57-
}
58-
5953
uint256 GetBestBlock() const override { return hashBestBlock_; }
6054

6155
bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override
@@ -147,8 +141,22 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test)
147141
{
148142
uint256 txid = txids[InsecureRandRange(txids.size())]; // txid we're going to modify in this iteration.
149143
Coin& coin = result[COutPoint(txid, 0)];
144+
145+
// Determine whether to test HaveCoin before or after Access* (or both). As these functions
146+
// can influence each other's behaviour by pulling things into the cache, all combinations
147+
// are tested.
148+
bool test_havecoin_before = InsecureRandBits(2) == 0;
149+
bool test_havecoin_after = InsecureRandBits(2) == 0;
150+
151+
bool result_havecoin = test_havecoin_before ? stack.back()->HaveCoin(COutPoint(txid, 0)) : false;
150152
const Coin& entry = (InsecureRandRange(500) == 0) ? AccessByTxid(*stack.back(), txid) : stack.back()->AccessCoin(COutPoint(txid, 0));
151153
BOOST_CHECK(coin == entry);
154+
BOOST_CHECK(!test_havecoin_before || result_havecoin == !entry.IsSpent());
155+
156+
if (test_havecoin_after) {
157+
bool ret = stack.back()->HaveCoin(COutPoint(txid, 0));
158+
BOOST_CHECK(ret == !entry.IsSpent());
159+
}
152160

153161
if (InsecureRandRange(5) == 0 || coin.IsSpent()) {
154162
Coin newcoin;
@@ -628,7 +636,7 @@ BOOST_AUTO_TEST_CASE(ccoins_access)
628636
CheckAccessCoin(ABSENT, VALUE2, VALUE2, FRESH , FRESH );
629637
CheckAccessCoin(ABSENT, VALUE2, VALUE2, DIRTY , DIRTY );
630638
CheckAccessCoin(ABSENT, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH);
631-
CheckAccessCoin(PRUNED, ABSENT, PRUNED, NO_ENTRY , FRESH );
639+
CheckAccessCoin(PRUNED, ABSENT, ABSENT, NO_ENTRY , NO_ENTRY );
632640
CheckAccessCoin(PRUNED, PRUNED, PRUNED, 0 , 0 );
633641
CheckAccessCoin(PRUNED, PRUNED, PRUNED, FRESH , FRESH );
634642
CheckAccessCoin(PRUNED, PRUNED, PRUNED, DIRTY , DIRTY );

‎src/txmempool.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -903,11 +903,7 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const {
903903
return false;
904904
}
905905
}
906-
return (base->GetCoin(outpoint, coin) && !coin.IsSpent());
907-
}
908-
909-
bool CCoinsViewMemPool::HaveCoin(const COutPoint &outpoint) const {
910-
return mempool.exists(outpoint) || base->HaveCoin(outpoint);
906+
return base->GetCoin(outpoint, coin);
911907
}
912908

913909
size_t CTxMemPool::DynamicMemoryUsage() const {

‎src/txmempool.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,8 +684,7 @@ class CCoinsViewMemPool : public CCoinsViewBacked
684684

685685
public:
686686
CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn);
687-
bool GetCoin(const COutPoint &outpoint, Coin &coin) const;
688-
bool HaveCoin(const COutPoint &outpoint) const;
687+
bool GetCoin(const COutPoint &outpoint, Coin &coin) const override;
689688
};
690689

691690
/**

0 commit comments

Comments
 (0)