diff --git a/src/coins.cpp b/src/coins.cpp index e79490d90..a8e270fc5 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -218,7 +218,11 @@ void CCoinsViewCache::SetNullifiers(const CTransaction& tx, bool spent) { ret.first->second.flags |= CNullifiersCacheEntry::DIRTY; } } - // TODO add sapling nullifiers + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + std::pair ret = cacheSaplingNullifiers.insert(std::make_pair(spendDescription.nullifier, CNullifiersCacheEntry())); + ret.first->second.entered = spent; + ret.first->second.flags |= CNullifiersCacheEntry::DIRTY; + } } bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { @@ -448,7 +452,11 @@ bool CCoinsViewCache::HaveJoinSplitRequirements(const CTransaction& tx) const intermediates.insert(std::make_pair(tree.root(), tree)); } - // TODO check sapling nullifiers + + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + if (GetNullifier(spendDescription.nullifier, SAPLING_NULLIFIER)) // Prevent double spends + return false; + } return true; } diff --git a/src/main.cpp b/src/main.cpp index f70859413..9d7e38099 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1274,7 +1274,10 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return false; } } - // TODO check sapling nullifiers + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + if (pool.nullifierExists(spendDescription.nullifier, SAPLING_NULLIFIER)) + return false; + } } { diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 1c68b888b..2d29338ce 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -172,6 +172,31 @@ public: }; +class TxWithNullifiers +{ +public: + CTransaction tx; + uint256 sproutNullifier; + uint256 saplingNullifier; + + TxWithNullifiers() + { + CMutableTransaction mutableTx; + + sproutNullifier = GetRandHash(); + JSDescription jsd; + jsd.nullifiers[0] = sproutNullifier; + mutableTx.vjoinsplit.emplace_back(jsd); + + saplingNullifier = GetRandHash(); + SpendDescription sd; + sd.nullifier = saplingNullifier; + mutableTx.vShieldedSpend.push_back(sd); + + tx = CTransaction(mutableTx); + } +}; + } uint256 appendRandomCommitment(ZCIncrementalMerkleTree &tree) @@ -186,19 +211,19 @@ 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) +void checkNullifierCache(const CCoinsViewCacheTest &cache, const TxWithNullifiers &txWithNullifiers, bool shouldBeInCache) { + // Make sure the nullifiers have not gotten mixed up + BOOST_CHECK(!cache.GetNullifier(txWithNullifiers.sproutNullifier, SAPLING_NULLIFIER)); + BOOST_CHECK(!cache.GetNullifier(txWithNullifiers.saplingNullifier, SPROUT_NULLIFIER)); + // Check if the nullifiers either are or are not in the cache + bool containsSproutNullifier = cache.GetNullifier(txWithNullifiers.sproutNullifier, SPROUT_NULLIFIER); + bool containsSaplingNullifier = cache.GetNullifier(txWithNullifiers.saplingNullifier, SAPLING_NULLIFIER); + BOOST_CHECK(containsSproutNullifier == shouldBeInCache); + BOOST_CHECK(containsSaplingNullifier == shouldBeInCache); +} + BOOST_AUTO_TEST_CASE(nullifier_regression_test) { // Correct behavior: @@ -206,16 +231,18 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewTest base; CCoinsViewCacheTest cache1(&base); + TxWithNullifiers txWithNullifiers; + // Insert a nullifier into the base. - auto txWithNullifier = createTxWithNullifier(); - cache1.SetNullifiers(txWithNullifier.first, true); + cache1.SetNullifiers(txWithNullifiers.tx, true); + checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifiers(txWithNullifier.first, false); + cache1.SetNullifiers(txWithNullifiers.tx, false); // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + checkNullifierCache(cache1, txWithNullifiers, false); } // Also correct behavior: @@ -223,17 +250,19 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewTest base; CCoinsViewCacheTest cache1(&base); + TxWithNullifiers txWithNullifiers; + // Insert a nullifier into the base. - auto txWithNullifier = createTxWithNullifier(); - cache1.SetNullifiers(txWithNullifier.first, true); + cache1.SetNullifiers(txWithNullifiers.tx, true); + checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifiers(txWithNullifier.first, false); + cache1.SetNullifiers(txWithNullifiers.tx, false); cache1.Flush(); // Flush to base. // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + checkNullifierCache(cache1, txWithNullifiers, false); } // Works because we bring it from the parent cache: @@ -242,21 +271,22 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - auto txWithNullifier = createTxWithNullifier(); - cache1.SetNullifiers(txWithNullifier.first, true); + TxWithNullifiers txWithNullifiers; + cache1.SetNullifiers(txWithNullifiers.tx, true); + checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Empties cache. // Create cache on top. { // Remove the nullifier. CCoinsViewCacheTest cache2(&cache1); - BOOST_CHECK(cache2.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); - cache1.SetNullifiers(txWithNullifier.first, false); + checkNullifierCache(cache2, txWithNullifiers, true); + cache1.SetNullifiers(txWithNullifiers.tx, false); cache2.Flush(); // Empties cache, flushes to cache1. } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + checkNullifierCache(cache1, txWithNullifiers, false); } // Was broken: @@ -265,20 +295,20 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) CCoinsViewCacheTest cache1(&base); // Insert a nullifier into the base. - auto txWithNullifier = createTxWithNullifier(); - cache1.SetNullifiers(txWithNullifier.first, true); + TxWithNullifiers txWithNullifiers; + cache1.SetNullifiers(txWithNullifiers.tx, true); cache1.Flush(); // Empties cache. // Create cache on top. { // Remove the nullifier. CCoinsViewCacheTest cache2(&cache1); - cache2.SetNullifiers(txWithNullifier.first, false); + cache2.SetNullifiers(txWithNullifiers.tx, false); cache2.Flush(); // Empties cache, flushes to cache1. } // The nullifier now should be `false`. - BOOST_CHECK(!cache1.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + checkNullifierCache(cache1, txWithNullifiers, false); } } @@ -447,26 +477,22 @@ BOOST_AUTO_TEST_CASE(nullifiers_test) CCoinsViewTest base; CCoinsViewCacheTest cache(&base); - auto txWithNullifier = createTxWithNullifier(); - - 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, SPROUT_NULLIFIER)); - BOOST_CHECK(!cache.GetNullifier(txWithNullifier.second, SAPLING_NULLIFIER)); + TxWithNullifiers txWithNullifiers; + checkNullifierCache(cache, txWithNullifiers, false); + cache.SetNullifiers(txWithNullifiers.tx, true); + checkNullifierCache(cache, txWithNullifiers, true); cache.Flush(); CCoinsViewCacheTest cache2(&base); - 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, SPROUT_NULLIFIER)); + checkNullifierCache(cache2, txWithNullifiers, true); + cache2.SetNullifiers(txWithNullifiers.tx, false); + checkNullifierCache(cache2, txWithNullifiers, false); cache2.Flush(); CCoinsViewCacheTest cache3(&base); - BOOST_CHECK(!cache3.GetNullifier(txWithNullifier.second, SPROUT_NULLIFIER)); + checkNullifierCache(cache3, txWithNullifiers, false); } BOOST_AUTO_TEST_CASE(anchors_flush_test) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f239def7c..626bd9df6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -110,7 +110,9 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, mapNullifiers[nf] = &tx; } } - // TODO add sapling nullifiers + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + mapSaplingNullifiers[spendDescription.nullifier] = &tx; + } nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); cachedInnerUsage += entry.DynamicMemoryUsage(); @@ -160,8 +162,9 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem mapNullifiers.erase(nf); } } - // TODO remove sapling nullifiers - + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + mapSaplingNullifiers.erase(spendDescription.nullifier); + } removed.push_back(tx); totalTxSize -= mapTx.find(hash)->GetTxSize(); cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); @@ -249,13 +252,18 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list if (it != mapNullifiers.end()) { const CTransaction &txConflict = *it->second; if (txConflict != tx) - { remove(txConflict, removed, true); - } } } } - // TODO remove sapling nullifiers + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + std::map::iterator it = mapSaplingNullifiers.find(spendDescription.nullifier); + if (it != mapSaplingNullifiers.end()) { + const CTransaction &txConflict = *it->second; + if (txConflict != tx) + remove(txConflict, removed, true); + } + } } void CTxMemPool::removeExpired(unsigned int nBlockHeight) @@ -401,7 +409,9 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const intermediates.insert(std::make_pair(tree.root(), tree)); } - // TODO check sapling nullifiers + for (const SpendDescription &spendDescription : tx.vShieldedSpend) { + assert(!pcoins->GetNullifier(spendDescription.nullifier, SAPLING_NULLIFIER)); + } if (fDependsWait) waitingOnDependants.push_back(&(*it)); else {