From 708c87f16d5fde0919e0dc2e25ed855e149051df Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Mon, 23 Apr 2018 14:37:17 -0600 Subject: [PATCH] Add enum for nullifier type --- src/coins.cpp | 22 ++++++++++++++----- src/coins.h | 12 +++++++--- src/gtest/test_mempool.cpp | 2 +- src/gtest/test_validation.cpp | 2 +- src/main.cpp | 2 +- src/test/coins_tests.cpp | 41 +++++++++++++++++++++-------------- src/txdb.cpp | 15 +++++++++++-- src/txdb.h | 2 +- src/txmempool.cpp | 35 ++++++++++++++++++++++-------- src/txmempool.h | 6 ++--- src/zcbenchmarks.cpp | 2 +- 11 files changed, 97 insertions(+), 44 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 8171b0e28..e79490d90 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -43,7 +43,7 @@ bool CCoins::Spend(uint32_t nPos) return true; } bool CCoinsView::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const { return false; } -bool CCoinsView::GetNullifier(const uint256 &nullifier, bool isSapling) const { return false; } +bool CCoinsView::GetNullifier(const uint256 &nullifier, NullifierType type) const { return false; } bool CCoinsView::GetCoins(const uint256 &txid, CCoins &coins) const { return false; } bool CCoinsView::HaveCoins(const uint256 &txid) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } @@ -60,7 +60,7 @@ bool CCoinsView::GetStats(CCoinsStats &stats) const { return false; } CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } bool CCoinsViewBacked::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const { return base->GetAnchorAt(rt, tree); } -bool CCoinsViewBacked::GetNullifier(const uint256 &nullifier, bool isSapling) const { return base->GetNullifier(nullifier, isSapling); } +bool CCoinsViewBacked::GetNullifier(const uint256 &nullifier, NullifierType type) const { return base->GetNullifier(nullifier, type); } bool CCoinsViewBacked::GetCoins(const uint256 &txid, CCoins &coins) const { return base->GetCoins(txid, coins); } bool CCoinsViewBacked::HaveCoins(const uint256 &txid) const { return base->HaveCoins(txid); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } @@ -133,14 +133,24 @@ bool CCoinsViewCache::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tr return true; } -bool CCoinsViewCache::GetNullifier(const uint256 &nullifier, bool isSapling) const { - CNullifiersMap* cacheToUse = isSapling ? &cacheSaplingNullifiers : &cacheNullifiers; +bool CCoinsViewCache::GetNullifier(const uint256 &nullifier, NullifierType type) const { + CNullifiersMap* cacheToUse; + switch (type) { + case SPROUT_NULLIFIER: + cacheToUse = &cacheNullifiers; + break; + case SAPLING_NULLIFIER: + cacheToUse = &cacheSaplingNullifiers; + break; + default: + throw std::runtime_error("Unknown nullifier type " + type); + } CNullifiersMap::iterator it = cacheToUse->find(nullifier); if (it != cacheToUse->end()) return it->second.entered; CNullifiersCacheEntry entry; - bool tmp = base->GetNullifier(nullifier, isSapling); + bool tmp = base->GetNullifier(nullifier, type); entry.entered = tmp; cacheToUse->insert(std::make_pair(nullifier, entry)); @@ -416,7 +426,7 @@ bool CCoinsViewCache::HaveJoinSplitRequirements(const CTransaction& tx) const { BOOST_FOREACH(const uint256& nullifier, joinsplit.nullifiers) { - if (GetNullifier(nullifier, false)) { + if (GetNullifier(nullifier, SPROUT_NULLIFIER)) { // If the nullifier is set, this transaction // double-spends! return false; diff --git a/src/coins.h b/src/coins.h index 2a48bebaf..7fffa8dab 100644 --- a/src/coins.h +++ b/src/coins.h @@ -298,6 +298,12 @@ struct CNullifiersCacheEntry CNullifiersCacheEntry() : entered(false), flags(0) {} }; +enum NullifierType +{ + SPROUT_NULLIFIER, + SAPLING_NULLIFIER, +}; + typedef boost::unordered_map CCoinsMap; typedef boost::unordered_map CAnchorsMap; typedef boost::unordered_map CNullifiersMap; @@ -324,7 +330,7 @@ public: virtual bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; //! Determine whether a nullifier is spent or not - virtual bool GetNullifier(const uint256 &nullifier, bool isSapling) const; + virtual bool GetNullifier(const uint256 &nullifier, NullifierType type) const; //! Retrieve the CCoins (unspent transaction outputs) for a given txid virtual bool GetCoins(const uint256 &txid, CCoins &coins) const; @@ -365,7 +371,7 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; - bool GetNullifier(const uint256 &nullifier, bool isSapling) const; + bool GetNullifier(const uint256 &nullifier, NullifierType type) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; @@ -431,7 +437,7 @@ public: // Standard CCoinsView methods bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; - bool GetNullifier(const uint256 &nullifier, bool isSapling) const; + bool GetNullifier(const uint256 &nullifier, NullifierType type) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; diff --git a/src/gtest/test_mempool.cpp b/src/gtest/test_mempool.cpp index c1ff5ccb8..4203cf6f1 100644 --- a/src/gtest/test_mempool.cpp +++ b/src/gtest/test_mempool.cpp @@ -23,7 +23,7 @@ public: return false; } - bool GetNullifier(const uint256 &nf, bool isSapling) const { + bool GetNullifier(const uint256 &nf, NullifierType type) const { return false; } diff --git a/src/gtest/test_validation.cpp b/src/gtest/test_validation.cpp index 127e5b260..d321e11e0 100644 --- a/src/gtest/test_validation.cpp +++ b/src/gtest/test_validation.cpp @@ -25,7 +25,7 @@ public: return false; } - bool GetNullifier(const uint256 &nf, bool isSapling) const { + bool GetNullifier(const uint256 &nf, NullifierType type) const { return false; } diff --git a/src/main.cpp b/src/main.cpp index 629df138d..f70859413 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1270,7 +1270,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa } BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { BOOST_FOREACH(const uint256 &nf, joinsplit.nullifiers) { - if (pool.nullifierExists(nf, false)) + if (pool.nullifierExists(nf, SPROUT_NULLIFIER)) return false; } } diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 524d18b80..1c68b888b 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -52,11 +52,20 @@ public: } } - bool GetNullifier(const uint256 &nf, bool isSapling) const + bool GetNullifier(const uint256 &nf, NullifierType type) const { - const std::map* mapToUse = isSapling ? &mapSaplingNullifiers_ : &mapNullifiers_; + const std::map* mapToUse; + switch (type) { + case SPROUT_NULLIFIER: + mapToUse = &mapNullifiers_; + break; + case SAPLING_NULLIFIER: + mapToUse = &mapSaplingNullifiers_; + break; + default: + throw std::runtime_error("Unknown nullifier type " + type); + } std::map::const_iterator it = mapToUse->find(nf); - if (it == mapToUse->end()) { return false; } else { @@ -206,7 +215,7 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) cache1.SetNullifiers(txWithNullifier.first, false); // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); } // Also correct behavior: @@ -224,7 +233,7 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) cache1.Flush(); // Flush to base. // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); } // Works because we bring it from the parent cache: @@ -241,13 +250,13 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) { // Remove the nullifier. CCoinsViewCacheTest cache2(&cache1); - BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); cache1.SetNullifiers(txWithNullifier.first, false); cache2.Flush(); // Empties cache, flushes to cache1. } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); } // Was broken: @@ -269,7 +278,7 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); } } @@ -440,24 +449,24 @@ BOOST_AUTO_TEST_CASE(nullifiers_test) auto txWithNullifier = createTxWithNullifier(); - BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, false)); - BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, true)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, SAPLING_NULLIFIER)); cache.SetNullifiers(txWithNullifier.first, true); - BOOST_CHECK(cache.GetNullifier(txWithNullifier.second, false)); - BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, true)); + BOOST_CHECK(cache.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, SAPLING_NULLIFIER)); cache.Flush(); CCoinsViewCacheTest cache2(&base); - BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, false)); - BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, true)); + BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, SAPLING_NULLIFIER)); cache2.SetNullifiers(txWithNullifier.first, false); - BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); cache2.Flush(); CCoinsViewCacheTest cache3(&base); - BOOST_CHECK(!cache3.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache3.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); } BOOST_AUTO_TEST_CASE(anchors_flush_test) diff --git a/src/txdb.cpp b/src/txdb.cpp index 396e34a6a..715f295f7 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -52,9 +52,20 @@ bool CCoinsViewDB::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) return read; } -bool CCoinsViewDB::GetNullifier(const uint256 &nf, bool isSapling) const { +bool CCoinsViewDB::GetNullifier(const uint256 &nf, NullifierType type) const { bool spent = false; - return db.Read(make_pair(isSapling ? DB_SAPLING_NULLIFIER : DB_NULLIFIER, nf), spent); + char dbChar; + switch (type) { + case SPROUT_NULLIFIER: + dbChar = DB_NULLIFIER; + break; + case SAPLING_NULLIFIER: + dbChar = DB_SAPLING_NULLIFIER; + break; + default: + throw runtime_error("Unknown nullifier type " + type); + } + return db.Read(make_pair(dbChar, nf), spent); } bool CCoinsViewDB::GetCoins(const uint256 &txid, CCoins &coins) const { diff --git a/src/txdb.h b/src/txdb.h index 1d268bacb..01bc576fc 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -36,7 +36,7 @@ public: CCoinsViewDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; - bool GetNullifier(const uint256 &nf, bool isSapling) const; + bool GetNullifier(const uint256 &nf, NullifierType type) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index edfa9adf2..f239def7c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -383,7 +383,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { BOOST_FOREACH(const uint256 &nf, joinsplit.nullifiers) { - assert(!pcoins->GetNullifier(nf, false)); + assert(!pcoins->GetNullifier(nf, SPROUT_NULLIFIER)); } ZCIncrementalMerkleTree tree; @@ -439,16 +439,26 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const assert(it->first == it->second.ptx->vin[it->second.n].prevout); } - checkNullifiers(false); - checkNullifiers(true); + checkNullifiers(SPROUT_NULLIFIER); + checkNullifiers(SAPLING_NULLIFIER); assert(totalTxSize == checkTotal); assert(innerUsage == cachedInnerUsage); } -void CTxMemPool::checkNullifiers(bool isSapling) const +void CTxMemPool::checkNullifiers(NullifierType type) const { - const std::map* mapToUse = isSapling ? &mapSaplingNullifiers : &mapNullifiers; + const std::map* mapToUse; + switch (type) { + case SPROUT_NULLIFIER: + mapToUse = &mapNullifiers; + break; + case SAPLING_NULLIFIER: + mapToUse = &mapSaplingNullifiers; + break; + default: + throw runtime_error("Unknown nullifier type " + type); + } for (const auto& entry : *mapToUse) { uint256 hash = entry.second->GetHash(); CTxMemPool::indexed_transaction_set::const_iterator findTx = mapTx.find(hash); @@ -559,16 +569,23 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const return true; } -bool CTxMemPool::nullifierExists(const uint256& nullifier, bool isSapling) const +bool CTxMemPool::nullifierExists(const uint256& nullifier, NullifierType type) const { - return isSapling ? mapSaplingNullifiers.count(nullifier) : mapNullifiers.count(nullifier); + switch (type) { + case SPROUT_NULLIFIER: + return mapNullifiers.count(nullifier); + case SAPLING_NULLIFIER: + return mapSaplingNullifiers.count(nullifier); + default: + throw runtime_error("Unknown nullifier type " + type); + } } CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetNullifier(const uint256 &nf, bool isSapling) const +bool CCoinsViewMemPool::GetNullifier(const uint256 &nf, NullifierType type) const { - return mempool.nullifierExists(nf, isSapling) || base->GetNullifier(nf, isSapling); + return mempool.nullifierExists(nf, type) || base->GetNullifier(nf, type); } bool CCoinsViewMemPool::GetCoins(const uint256 &txid, CCoins &coins) const { diff --git a/src/txmempool.h b/src/txmempool.h index 79a76c1f6..1b5b3e6b0 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -134,7 +134,7 @@ private: std::map mapNullifiers; std::map mapSaplingNullifiers; - void checkNullifiers(bool isSapling) const; + void checkNullifiers(NullifierType type) const; public: typedef boost::multi_index_container< @@ -192,7 +192,7 @@ public: void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta); void ClearPrioritisation(const uint256 hash); - bool nullifierExists(const uint256& nullifier, bool isSapling) const; + bool nullifierExists(const uint256& nullifier, NullifierType type) const; unsigned long size() { @@ -243,7 +243,7 @@ protected: public: CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn); - bool GetNullifier(const uint256 &txid, bool isSapling) const; + bool GetNullifier(const uint256 &txid, NullifierType type) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; }; diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index dcee234b4..236a22686 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -366,7 +366,7 @@ public: return false; } - bool GetNullifier(const uint256 &nf, bool isSapling) const { + bool GetNullifier(const uint256 &nf, NullifierType type) const { return false; }