diff --git a/src/coins.cpp b/src/coins.cpp index 250354614..8171b0e28 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) const { return false; } +bool CCoinsView::GetNullifier(const uint256 &nullifier, bool isSapling) 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(); } @@ -52,14 +52,15 @@ bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { return false; } + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers) { return false; } 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) const { return base->GetNullifier(nullifier); } +bool CCoinsViewBacked::GetNullifier(const uint256 &nullifier, bool isSapling) const { return base->GetNullifier(nullifier, isSapling); } 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(); } @@ -69,7 +70,8 @@ bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { return base->BatchWrite(mapCoins, hashBlock, hashAnchor, mapAnchors, mapNullifiers); } + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers) { return base->BatchWrite(mapCoins, hashBlock, hashAnchor, mapAnchors, mapNullifiers, mapSaplingNullifiers); } bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStats(stats); } CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} @@ -85,6 +87,7 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + memusage::DynamicUsage(cacheAnchors) + memusage::DynamicUsage(cacheNullifiers) + + memusage::DynamicUsage(cacheSaplingNullifiers) + cachedCoinsUsage; } @@ -130,16 +133,17 @@ bool CCoinsViewCache::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tr return true; } -bool CCoinsViewCache::GetNullifier(const uint256 &nullifier) const { - CNullifiersMap::iterator it = cacheNullifiers.find(nullifier); - if (it != cacheNullifiers.end()) +bool CCoinsViewCache::GetNullifier(const uint256 &nullifier, bool isSapling) const { + CNullifiersMap* cacheToUse = isSapling ? &cacheSaplingNullifiers : &cacheNullifiers; + CNullifiersMap::iterator it = cacheToUse->find(nullifier); + if (it != cacheToUse->end()) return it->second.entered; CNullifiersCacheEntry entry; - bool tmp = base->GetNullifier(nullifier); + bool tmp = base->GetNullifier(nullifier, isSapling); entry.entered = tmp; - cacheNullifiers.insert(std::make_pair(nullifier, entry)); + cacheToUse->insert(std::make_pair(nullifier, entry)); return tmp; } @@ -196,10 +200,15 @@ void CCoinsViewCache::PopAnchor(const uint256 &newrt) { } } -void CCoinsViewCache::SetNullifier(const uint256 &nullifier, bool spent) { - std::pair ret = cacheNullifiers.insert(std::make_pair(nullifier, CNullifiersCacheEntry())); - ret.first->second.entered = spent; - ret.first->second.flags |= CNullifiersCacheEntry::DIRTY; +void CCoinsViewCache::SetNullifiers(const CTransaction& tx, bool spent) { + for (const JSDescription &joinsplit : tx.vjoinsplit) { + for (const uint256 &nullifier : joinsplit.nullifiers) { + std::pair ret = cacheNullifiers.insert(std::make_pair(nullifier, CNullifiersCacheEntry())); + ret.first->second.entered = spent; + ret.first->second.flags |= CNullifiersCacheEntry::DIRTY; + } + } + // TODO add sapling nullifiers } bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { @@ -267,11 +276,34 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { hashBlock = hashBlockIn; } +void BatchWriteNullifiers(CNullifiersMap &mapNullifiers, CNullifiersMap &cacheNullifiers) +{ + for (CNullifiersMap::iterator child_it = mapNullifiers.begin(); child_it != mapNullifiers.end();) { + if (child_it->second.flags & CNullifiersCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). + CNullifiersMap::iterator parent_it = cacheNullifiers.find(child_it->first); + + if (parent_it == cacheNullifiers.end()) { + CNullifiersCacheEntry& entry = cacheNullifiers[child_it->first]; + entry.entered = child_it->second.entered; + entry.flags = CNullifiersCacheEntry::DIRTY; + } else { + if (parent_it->second.entered != child_it->second.entered) { + parent_it->second.entered = child_it->second.entered; + parent_it->second.flags |= CNullifiersCacheEntry::DIRTY; + } + } + } + CNullifiersMap::iterator itOld = child_it++; + mapNullifiers.erase(itOld); + } +} + bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn, const uint256 &hashAnchorIn, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers) { assert(!hasModifier); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). @@ -333,25 +365,8 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, mapAnchors.erase(itOld); } - for (CNullifiersMap::iterator child_it = mapNullifiers.begin(); child_it != mapNullifiers.end();) - { - if (child_it->second.flags & CNullifiersCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). - CNullifiersMap::iterator parent_it = cacheNullifiers.find(child_it->first); - - if (parent_it == cacheNullifiers.end()) { - CNullifiersCacheEntry& entry = cacheNullifiers[child_it->first]; - entry.entered = child_it->second.entered; - entry.flags = CNullifiersCacheEntry::DIRTY; - } else { - if (parent_it->second.entered != child_it->second.entered) { - parent_it->second.entered = child_it->second.entered; - parent_it->second.flags |= CNullifiersCacheEntry::DIRTY; - } - } - } - CNullifiersMap::iterator itOld = child_it++; - mapNullifiers.erase(itOld); - } + ::BatchWriteNullifiers(mapNullifiers, cacheNullifiers); + ::BatchWriteNullifiers(mapSaplingNullifiers, cacheSaplingNullifiers); hashAnchor = hashAnchorIn; hashBlock = hashBlockIn; @@ -359,10 +374,11 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, } bool CCoinsViewCache::Flush() { - bool fOk = base->BatchWrite(cacheCoins, hashBlock, hashAnchor, cacheAnchors, cacheNullifiers); + bool fOk = base->BatchWrite(cacheCoins, hashBlock, hashAnchor, cacheAnchors, cacheNullifiers, cacheSaplingNullifiers); cacheCoins.clear(); cacheAnchors.clear(); cacheNullifiers.clear(); + cacheSaplingNullifiers.clear(); cachedCoinsUsage = 0; return fOk; } @@ -400,7 +416,7 @@ bool CCoinsViewCache::HaveJoinSplitRequirements(const CTransaction& tx) const { BOOST_FOREACH(const uint256& nullifier, joinsplit.nullifiers) { - if (GetNullifier(nullifier)) { + if (GetNullifier(nullifier, false)) { // If the nullifier is set, this transaction // double-spends! return false; @@ -422,6 +438,7 @@ bool CCoinsViewCache::HaveJoinSplitRequirements(const CTransaction& tx) const intermediates.insert(std::make_pair(tree.root(), tree)); } + // TODO check sapling nullifiers return true; } diff --git a/src/coins.h b/src/coins.h index 4e1cac438..2a48bebaf 100644 --- a/src/coins.h +++ b/src/coins.h @@ -324,7 +324,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) const; + virtual bool GetNullifier(const uint256 &nullifier, bool isSapling) const; //! Retrieve the CCoins (unspent transaction outputs) for a given txid virtual bool GetCoins(const uint256 &txid, CCoins &coins) const; @@ -345,7 +345,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers); + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers); //! Calculate statistics about the unspent transaction output set virtual bool GetStats(CCoinsStats &stats) const; @@ -364,7 +365,7 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; - bool GetNullifier(const uint256 &nullifier) const; + bool GetNullifier(const uint256 &nullifier, bool isSapling) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; @@ -374,7 +375,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers); + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers); bool GetStats(CCoinsStats &stats) const; }; @@ -418,6 +420,7 @@ protected: mutable uint256 hashAnchor; mutable CAnchorsMap cacheAnchors; mutable CNullifiersMap cacheNullifiers; + mutable CNullifiersMap cacheSaplingNullifiers; /* Cached dynamic memory usage for the inner CCoins objects. */ mutable size_t cachedCoinsUsage; @@ -428,7 +431,7 @@ public: // Standard CCoinsView methods bool GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) const; - bool GetNullifier(const uint256 &nullifier) const; + bool GetNullifier(const uint256 &nullifier, bool isSapling) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; @@ -438,7 +441,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers); + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers); // Adds the tree to mapAnchors and sets the current commitment @@ -449,8 +453,8 @@ public: // the new current root. void PopAnchor(const uint256 &rt); - // Marks a nullifier as spent or not. - void SetNullifier(const uint256 &nullifier, bool spent); + // Marks a nullifiers for a given transaction as spent or not. + void SetNullifiers(const CTransaction& tx, bool spent); /** * Return a pointer to CCoins in the cache, or NULL if not found. This is diff --git a/src/gtest/test_mempool.cpp b/src/gtest/test_mempool.cpp index 6ee3eb1b1..c1ff5ccb8 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) const { + bool GetNullifier(const uint256 &nf, bool isSapling) const { return false; } @@ -56,7 +56,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers) { return false; } diff --git a/src/gtest/test_validation.cpp b/src/gtest/test_validation.cpp index 4cd3eacca..127e5b260 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) const { + bool GetNullifier(const uint256 &nf, bool isSapling) const { return false; } @@ -51,7 +51,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { + CNullifiersMap &mapNullifiers, + CNullifiersMap saplingNullifiersMap) { return false; } diff --git a/src/main.cpp b/src/main.cpp index fb6196c70..629df138d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1270,12 +1270,11 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa } BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { BOOST_FOREACH(const uint256 &nf, joinsplit.nullifiers) { - if (pool.mapNullifiers.count(nf)) - { + if (pool.nullifierExists(nf, false)) return false; - } } } + // TODO check sapling nullifiers } { @@ -1775,11 +1774,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund } // spend nullifiers - BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { - BOOST_FOREACH(const uint256 &nf, joinsplit.nullifiers) { - inputs.SetNullifier(nf, true); - } - } + inputs.SetNullifiers(tx, true); // add outputs inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight); @@ -2097,11 +2092,7 @@ bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex } // unspend nullifiers - BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { - BOOST_FOREACH(const uint256 &nf, joinsplit.nullifiers) { - view.SetNullifier(nf, false); - } - } + view.SetNullifiers(tx, false); // restore inputs if (i > 0) { // not coinbases diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 383616ae0..524d18b80 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -11,6 +11,7 @@ #include "consensus/validation.h" #include "main.h" #include "undo.h" +#include "primitives/transaction.h" #include "pubkey.h" #include @@ -28,6 +29,7 @@ class CCoinsViewTest : public CCoinsView std::map map_; std::map mapAnchors_; std::map mapNullifiers_; + std::map mapSaplingNullifiers_; public: CCoinsViewTest() { @@ -50,11 +52,12 @@ public: } } - bool GetNullifier(const uint256 &nf) const + bool GetNullifier(const uint256 &nf, bool isSapling) const { - std::map::const_iterator it = mapNullifiers_.find(nf); + const std::map* mapToUse = isSapling ? &mapSaplingNullifiers_ : &mapNullifiers_; + std::map::const_iterator it = mapToUse->find(nf); - if (it == mapNullifiers_.end()) { + if (it == mapToUse->end()) { return false; } else { // The map shouldn't contain any false entries. @@ -87,11 +90,25 @@ public: uint256 GetBestBlock() const { return hashBestBlock_; } + void BatchWriteNullifiers(CNullifiersMap& mapNullifiers, std::map& cacheNullifiers) + { + for (CNullifiersMap::iterator it = mapNullifiers.begin(); it != mapNullifiers.end(); ) { + if (it->second.entered) { + cacheNullifiers[it->first] = true; + } else { + cacheNullifiers.erase(it->first); + } + mapNullifiers.erase(it++); + } + mapNullifiers.clear(); + } + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock, const uint256& hashAnchor, CAnchorsMap& mapAnchors, - CNullifiersMap& mapNullifiers) + CNullifiersMap& mapNullifiers, + CNullifiersMap& mapSaplingNullifiers) { for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { map_[it->first] = it->second.coins; @@ -112,17 +129,12 @@ public: } mapAnchors.erase(it++); } - for (CNullifiersMap::iterator it = mapNullifiers.begin(); it != mapNullifiers.end(); ) { - if (it->second.entered) { - mapNullifiers_[it->first] = true; - } else { - mapNullifiers_.erase(it->first); - } - mapNullifiers.erase(it++); - } + + BatchWriteNullifiers(mapNullifiers, mapNullifiers_); + BatchWriteNullifiers(mapSaplingNullifiers, mapSaplingNullifiers_); + mapCoins.clear(); mapAnchors.clear(); - mapNullifiers.clear(); hashBestBlock_ = hashBlock; hashBestAnchor_ = hashAnchor; return true; @@ -141,7 +153,8 @@ public: // Manually recompute the dynamic usage of the whole data, and compare it. size_t ret = memusage::DynamicUsage(cacheCoins) + memusage::DynamicUsage(cacheAnchors) + - memusage::DynamicUsage(cacheNullifiers); + memusage::DynamicUsage(cacheNullifiers) + + memusage::DynamicUsage(cacheSaplingNullifiers); for (CCoinsMap::iterator it = cacheCoins.begin(); it != cacheCoins.end(); it++) { ret += it->second.coins.DynamicMemoryUsage(); } @@ -164,6 +177,17 @@ uint256 appendRandomCommitment(ZCIncrementalMerkleTree &tree) return cm; } +std::pair createTxWithNullifier() +{ + CMutableTransaction mutableTx; + JSDescription jsd; + uint256 nullifier = GetRandHash(); + mutableTx.vjoinsplit.push_back(jsd); + jsd.nullifiers[0] = nullifier; + CTransaction tx(mutableTx); + return std::make_pair(tx, nullifier); +} + BOOST_FIXTURE_TEST_SUITE(coins_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(nullifier_regression_test) @@ -174,15 +198,15 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - uint256 nf = GetRandHash(); - cache1.SetNullifier(nf, true); + auto txWithNullifier = createTxWithNullifier(); + cache1.SetNullifiers(txWithNullifier.first, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifier(nf, false); + cache1.SetNullifiers(txWithNullifier.first, false); // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(nf)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); } // Also correct behavior: @@ -191,16 +215,16 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - uint256 nf = GetRandHash(); - cache1.SetNullifier(nf, true); + auto txWithNullifier = createTxWithNullifier(); + cache1.SetNullifiers(txWithNullifier.first, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifier(nf, false); + cache1.SetNullifiers(txWithNullifier.first, false); cache1.Flush(); // Flush to base. // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(nf)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); } // Works because we bring it from the parent cache: @@ -209,21 +233,21 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - uint256 nf = GetRandHash(); - cache1.SetNullifier(nf, true); + auto txWithNullifier = createTxWithNullifier(); + cache1.SetNullifiers(txWithNullifier.first, true); cache1.Flush(); // Empties cache. // Create cache on top. { // Remove the nullifier. CCoinsViewCacheTest cache2(&cache1); - BOOST_CHECK(cache2.GetNullifier(nf)); - cache2.SetNullifier(nf, false); + BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, false)); + cache1.SetNullifiers(txWithNullifier.first, false); cache2.Flush(); // Empties cache, flushes to cache1. } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(nf)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); } // Was broken: @@ -232,20 +256,20 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - uint256 nf = GetRandHash(); - cache1.SetNullifier(nf, true); + auto txWithNullifier = createTxWithNullifier(); + cache1.SetNullifiers(txWithNullifier.first, true); cache1.Flush(); // Empties cache. // Create cache on top. { // Remove the nullifier. CCoinsViewCacheTest cache2(&cache1); - cache2.SetNullifier(nf, false); + cache2.SetNullifiers(txWithNullifier.first, false); cache2.Flush(); // Empties cache, flushes to cache1. } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(nf)); + BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, false)); } } @@ -414,23 +438,26 @@ BOOST_AUTO_TEST_CASE(nullifiers_test) CCoinsViewTest base; CCoinsViewCacheTest cache(&base); - uint256 nf = GetRandHash(); + auto txWithNullifier = createTxWithNullifier(); - BOOST_CHECK(!cache.GetNullifier(nf)); - cache.SetNullifier(nf, true); - BOOST_CHECK(cache.GetNullifier(nf)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, true)); + cache.SetNullifiers(txWithNullifier.first, true); + BOOST_CHECK(cache.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, true)); cache.Flush(); CCoinsViewCacheTest cache2(&base); - BOOST_CHECK(cache2.GetNullifier(nf)); - cache2.SetNullifier(nf, false); - BOOST_CHECK(!cache2.GetNullifier(nf)); + BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, false)); + BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, true)); + cache2.SetNullifiers(txWithNullifier.first, false); + BOOST_CHECK(!cache2.GetNullifier(txWithNullifier.second, false)); cache2.Flush(); CCoinsViewCacheTest cache3(&base); - BOOST_CHECK(!cache3.GetNullifier(nf)); + BOOST_CHECK(!cache3.GetNullifier(txWithNullifier.second, false)); } BOOST_AUTO_TEST_CASE(anchors_flush_test) diff --git a/src/txdb.cpp b/src/txdb.cpp index 24ab0b4b5..396e34a6a 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -19,6 +19,7 @@ using namespace std; static const char DB_ANCHOR = 'A'; static const char DB_NULLIFIER = 's'; +static const char DB_SAPLING_NULLIFIER = 'S'; static const char DB_COINS = 'c'; static const char DB_BLOCK_FILES = 'f'; static const char DB_TXINDEX = 't'; @@ -51,11 +52,9 @@ bool CCoinsViewDB::GetAnchorAt(const uint256 &rt, ZCIncrementalMerkleTree &tree) return read; } -bool CCoinsViewDB::GetNullifier(const uint256 &nf) const { +bool CCoinsViewDB::GetNullifier(const uint256 &nf, bool isSapling) const { bool spent = false; - bool read = db.Read(make_pair(DB_NULLIFIER, nf), spent); - - return read; + return db.Read(make_pair(isSapling ? DB_SAPLING_NULLIFIER : DB_NULLIFIER, nf), spent); } bool CCoinsViewDB::GetCoins(const uint256 &txid, CCoins &coins) const { @@ -80,11 +79,27 @@ uint256 CCoinsViewDB::GetBestAnchor() const { return hashBestAnchor; } +void BatchWriteNullifiers(CDBBatch& batch, CNullifiersMap& mapToUse, const char& dbChar) +{ + for (CNullifiersMap::iterator it = mapToUse.begin(); it != mapToUse.end();) { + if (it->second.flags & CNullifiersCacheEntry::DIRTY) { + if (!it->second.entered) + batch.Erase(make_pair(dbChar, it->first)); + else + batch.Write(make_pair(dbChar, it->first), true); + // TODO: changed++? ... See comment in CCoinsViewDB::BatchWrite. If this is needed we could return an int + } + CNullifiersMap::iterator itOld = it++; + mapToUse.erase(itOld); + } +} + bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers) { CDBBatch batch(db); size_t count = 0; size_t changed = 0; @@ -114,17 +129,8 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, mapAnchors.erase(itOld); } - for (CNullifiersMap::iterator it = mapNullifiers.begin(); it != mapNullifiers.end();) { - if (it->second.flags & CNullifiersCacheEntry::DIRTY) { - if (!it->second.entered) - batch.Erase(make_pair(DB_NULLIFIER, it->first)); - else - batch.Write(make_pair(DB_NULLIFIER, it->first), true); - // TODO: changed++? - } - CNullifiersMap::iterator itOld = it++; - mapNullifiers.erase(itOld); - } + ::BatchWriteNullifiers(batch, mapNullifiers, DB_NULLIFIER); + ::BatchWriteNullifiers(batch, mapSaplingNullifiers, DB_SAPLING_NULLIFIER); if (!hashBlock.IsNull()) batch.Write(DB_BEST_BLOCK, hashBlock); diff --git a/src/txdb.h b/src/txdb.h index f96b07676..1d268bacb 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) const; + bool GetNullifier(const uint256 &nf, bool isSapling) const; bool GetCoins(const uint256 &txid, CCoins &coins) const; bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; @@ -45,7 +45,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers); + CNullifiersMap &mapNullifiers, + CNullifiersMap &mapSaplingNullifiers); bool GetStats(CCoinsStats &stats) const; }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 650239e89..edfa9adf2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -110,6 +110,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, mapNullifiers[nf] = &tx; } } + // TODO add sapling nullifiers nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); cachedInnerUsage += entry.DynamicMemoryUsage(); @@ -118,7 +119,6 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, return true; } - void CTxMemPool::remove(const CTransaction &origTx, std::list& removed, bool fRecursive) { // Remove transaction from memory pool @@ -160,6 +160,7 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem mapNullifiers.erase(nf); } } + // TODO remove sapling nullifiers removed.push_back(tx); totalTxSize -= mapTx.find(hash)->GetTxSize(); @@ -254,6 +255,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list } } } + // TODO remove sapling nullifiers } void CTxMemPool::removeExpired(unsigned int nBlockHeight) @@ -381,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)); + assert(!pcoins->GetNullifier(nf, false)); } ZCIncrementalMerkleTree tree; @@ -399,6 +401,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const intermediates.insert(std::make_pair(tree.root(), tree)); } + // TODO check sapling nullifiers if (fDependsWait) waitingOnDependants.push_back(&(*it)); else { @@ -436,18 +439,25 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const assert(it->first == it->second.ptx->vin[it->second.n].prevout); } - for (std::map::const_iterator it = mapNullifiers.begin(); it != mapNullifiers.end(); it++) { - uint256 hash = it->second->GetHash(); - indexed_transaction_set::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->GetTx(); - assert(it2 != mapTx.end()); - assert(&tx == it->second); - } + checkNullifiers(false); + checkNullifiers(true); assert(totalTxSize == checkTotal); assert(innerUsage == cachedInnerUsage); } +void CTxMemPool::checkNullifiers(bool isSapling) const +{ + const std::map* mapToUse = isSapling ? &mapSaplingNullifiers : &mapNullifiers; + for (const auto& entry : *mapToUse) { + uint256 hash = entry.second->GetHash(); + CTxMemPool::indexed_transaction_set::const_iterator findTx = mapTx.find(hash); + const CTransaction& tx = findTx->GetTx(); + assert(findTx != mapTx.end()); + assert(&tx == entry.second); + } +} + void CTxMemPool::queryHashes(vector& vtxid) { vtxid.clear(); @@ -549,13 +559,16 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const return true; } +bool CTxMemPool::nullifierExists(const uint256& nullifier, bool isSapling) const +{ + return isSapling ? mapSaplingNullifiers.count(nullifier) : mapNullifiers.count(nullifier); +} + CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetNullifier(const uint256 &nf) const { - if (mempool.mapNullifiers.count(nf)) - return true; - - return base->GetNullifier(nf); +bool CCoinsViewMemPool::GetNullifier(const uint256 &nf, bool isSapling) const +{ + return mempool.nullifierExists(nf, isSapling) || base->GetNullifier(nf, isSapling); } bool CCoinsViewMemPool::GetCoins(const uint256 &txid, CCoins &coins) const { diff --git a/src/txmempool.h b/src/txmempool.h index fd8758741..79a76c1f6 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -131,6 +131,11 @@ private: uint64_t totalTxSize = 0; //! sum of all mempool tx' byte sizes uint64_t cachedInnerUsage; //! sum of dynamic memory usage of all the map elements (NOT the maps themselves) + std::map mapNullifiers; + std::map mapSaplingNullifiers; + + void checkNullifiers(bool isSapling) const; + public: typedef boost::multi_index_container< CTxMemPoolEntry, @@ -148,9 +153,8 @@ public: mutable CCriticalSection cs; indexed_transaction_set mapTx; std::map mapNextTx; - std::map mapNullifiers; std::map > mapDeltas; - + CTxMemPool(const CFeeRate& _minRelayFee); ~CTxMemPool(); @@ -188,6 +192,8 @@ public: void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta); void ClearPrioritisation(const uint256 hash); + bool nullifierExists(const uint256& nullifier, bool isSapling) const; + unsigned long size() { LOCK(cs); @@ -237,7 +243,7 @@ protected: public: CCoinsViewMemPool(CCoinsView *baseIn, CTxMemPool &mempoolIn); - bool GetNullifier(const uint256 &txid) const; + bool GetNullifier(const uint256 &txid, bool isSapling) 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 c268e6e12..dcee234b4 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -366,7 +366,7 @@ public: return false; } - bool GetNullifier(const uint256 &nf) const { + bool GetNullifier(const uint256 &nf, bool isSapling) const { return false; } @@ -382,7 +382,8 @@ public: const uint256 &hashBlock, const uint256 &hashAnchor, CAnchorsMap &mapAnchors, - CNullifiersMap &mapNullifiers) { + CNullifiersMap &mapNullifiers, + CNullifiersMap& mapSaplingNullifiers) { return false; }