From cf247bc6553902920feffc8a7a0ccedaeb799f95 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 22 Nov 2019 00:25:12 -0800 Subject: [PATCH 01/17] push/pop history with tests Co-authored-by: Jack Grigg --- src/Makefile.am | 2 + src/Makefile.gtest.include | 1 + src/coins.cpp | 311 ++++++++++++++++++++++++++++++++++++- src/coins.h | 46 +++++- src/gtest/test_history.cpp | 171 ++++++++++++++++++++ src/test/coins_tests.cpp | 6 +- src/txdb.cpp | 65 +++++++- src/txdb.h | 9 +- src/zcash/History.cpp | 120 ++++++++++++++ src/zcash/History.hpp | 74 +++++++++ 10 files changed, 793 insertions(+), 12 deletions(-) create mode 100644 src/gtest/test_history.cpp create mode 100644 src/zcash/History.cpp create mode 100644 src/zcash/History.hpp diff --git a/src/Makefile.am b/src/Makefile.am index a512029f9..f50eafa6d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -136,6 +136,7 @@ LIBZCASH_H = \ zcash/address/sapling.hpp \ zcash/address/sprout.hpp \ zcash/address/zip32.h \ + zcash/History.hpp \ zcash/JoinSplit.hpp \ zcash/Note.hpp \ zcash/prf.h \ @@ -565,6 +566,7 @@ libzcash_a_SOURCES = \ zcash/address/sapling.cpp \ zcash/address/sprout.cpp \ zcash/address/zip32.cpp \ + zcash/History.cpp \ zcash/JoinSplit.cpp \ zcash/Proof.cpp \ zcash/Note.cpp \ diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index 68aa6f27a..ce1257599 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -28,6 +28,7 @@ zcash_gtest_SOURCES += \ gtest/test_deprecation.cpp \ gtest/test_dynamicusage.cpp \ gtest/test_equihash.cpp \ + gtest/test_history.cpp \ gtest/test_httprpc.cpp \ gtest/test_joinsplit.cpp \ gtest/test_keys.cpp \ diff --git a/src/coins.cpp b/src/coins.cpp index 09a8ccf98..15ccb21ab 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -49,6 +49,10 @@ bool CCoinsView::GetCoins(const uint256 &txid, CCoins &coins) const { return fal bool CCoinsView::HaveCoins(const uint256 &txid) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } uint256 CCoinsView::GetBestAnchor(ShieldedType type) const { return uint256(); }; +HistoryIndex CCoinsView::GetHistoryLength(uint32_t epochId) const { return 0; } +HistoryNode CCoinsView::GetHistoryAt(uint32_t epochId, HistoryIndex index) const { return HistoryNode(); } +uint256 CCoinsView::GetHistoryRoot(uint32_t epochId) const { return uint256(); } + bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashSproutAnchor, @@ -56,7 +60,8 @@ bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers) { return false; } + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap) { return false; } bool CCoinsView::GetStats(CCoinsStats &stats) const { return false; } @@ -69,6 +74,9 @@ bool CCoinsViewBacked::GetCoins(const uint256 &txid, CCoins &coins) const { retu bool CCoinsViewBacked::HaveCoins(const uint256 &txid) const { return base->HaveCoins(txid); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } uint256 CCoinsViewBacked::GetBestAnchor(ShieldedType type) const { return base->GetBestAnchor(type); } +HistoryIndex CCoinsViewBacked::GetHistoryLength(uint32_t epochId) const { return base->GetHistoryLength(epochId); } +HistoryNode CCoinsViewBacked::GetHistoryAt(uint32_t epochId, HistoryIndex index) const { return base->GetHistoryAt(epochId, index); } +uint256 CCoinsViewBacked::GetHistoryRoot(uint32_t epochId) const { return base->GetHistoryRoot(epochId); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, @@ -77,7 +85,8 @@ bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers) { return base->BatchWrite(mapCoins, hashBlock, hashSproutAnchor, hashSaplingAnchor, mapSproutAnchors, mapSaplingAnchors, mapSproutNullifiers, mapSaplingNullifiers); } + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap) { return base->BatchWrite(mapCoins, hashBlock, hashSproutAnchor, hashSaplingAnchor, mapSproutAnchors, mapSaplingAnchors, mapSproutNullifiers, mapSaplingNullifiers, historyCacheMap); } bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStats(stats); } CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} @@ -95,6 +104,7 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { memusage::DynamicUsage(cacheSaplingAnchors) + memusage::DynamicUsage(cacheSproutNullifiers) + memusage::DynamicUsage(cacheSaplingNullifiers) + + memusage::DynamicUsage(historyCacheMap) + cachedCoinsUsage; } @@ -188,6 +198,31 @@ bool CCoinsViewCache::GetNullifier(const uint256 &nullifier, ShieldedType type) return tmp; } +HistoryIndex CCoinsViewCache::GetHistoryLength(uint32_t epochId) const { + HistoryCache& historyCache = SelectHistoryCache(epochId); + return historyCache.length; +} + +HistoryNode CCoinsViewCache::GetHistoryAt(uint32_t epochId, HistoryIndex index) const { + HistoryCache& historyCache = SelectHistoryCache(epochId); + + if (index >= historyCache.length) { + // Caller should ensure that it is limiting history + // request to 0..GetHistoryLength(epochId)-1 range + throw std::runtime_error("Invalid history request"); + } + + if (index >= historyCache.updateDepth) { + return historyCache.appends[index]; + } + + return base->GetHistoryAt(epochId, index); +} + +uint256 CCoinsViewCache::GetHistoryRoot(uint32_t epochId) const { + return SelectHistoryCache(epochId).root; +} + template void CCoinsViewCache::AbstractPushAnchor( const Tree &tree, @@ -260,6 +295,233 @@ void CCoinsViewCache::BringBestAnchorIntoCache( assert(GetSaplingAnchorAt(currentRoot, tree)); } +void draftMMRNode(std::vector &indices, + std::vector &entries, + HistoryNode nodeData, + uint32_t h, + uint32_t peak_pos) +{ + HistoryEntry newEntry = h == 0 + ? libzcash::LeafToEntry(nodeData) + // peak_pos - (1 << h) is the mmr position of left child, -1 to that is this position of entry in + // array representation. + // + // peak_pos - 1 is the mmr position of right child, -1 to that is this position of entry in + // array representation + : libzcash::NodeToEntry(nodeData, peak_pos - (1 << h) - 1, peak_pos - 2); + + indices.push_back(peak_pos - 1); + entries.push_back(newEntry); +} + +static inline uint32_t log2i(uint32_t x) { + assert(x > 0); + return 31 - __builtin_clz(x); +} + +// Computes floor(log2(x+1)) +static inline uint32_t floor_log2i(uint32_t x) { + return log2i(x + 1) - 1; +} + +uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std::vector &entries, std::vector &entry_indices) { + auto treeLength = GetHistoryLength(epochId); + + if (treeLength <= 0) { + throw std::runtime_error("Invalid PreloadHistoryTree state called - tree should exist"); + } + + uint32_t last_peak_pos = 0; + uint32_t last_peak_h = 0; + uint32_t h = 0; + uint32_t peak_pos = 0; + uint32_t total_peaks = 0; + + if (treeLength == 1) { + entries.push_back(libzcash::LeafToEntry(GetHistoryAt(epochId, 0))); + entry_indices.push_back(0); + return 1; + } else { + // First possible peak is calculated above. + h = floor_log2i(treeLength); + peak_pos = (1 << (h + 1)) - 1; + + // Collecting all peaks starting from first possible one. + while (h != 0) { + + // If peak_pos is out of bounds of the tree, left child of it calculated, + // and that means that we drop down one level in the tree. + if (peak_pos > treeLength) { + // left child, -2^h + peak_pos = peak_pos - (1 << h); + h = h - 1; + } + + // If the peak exists, we take it and then continue with its right sibling + // (which may not exist and that will be covered in next iteration). + if (peak_pos <= treeLength) { + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), h, peak_pos); + + last_peak_pos = peak_pos; + last_peak_h = h; + + // right sibling + peak_pos = peak_pos + (1 << (h + 1)) - 1; + } + } + } + + total_peaks = entries.size(); + + // early return if we don't extra nodes + if (!extra) return total_peaks; + + h = last_peak_h; + peak_pos = last_peak_pos; + + + // P + // / \ + // / \ + // / \ \ + // / \ \ + // _A_ \ \ + // / \_ B \ + // / \ / \ / \ C + // /\ /\ /\ /\ /\ /\ /\ + // D E + // + // + // For extra peaks needed for deletion, we do extra pass on right slope of the last peak + // and add those nodes + their siblings. Extra would be (D, E) for the picture above. + while (h > 0) { + uint32_t left_pos = peak_pos - (1<second; + } else { + auto cache = HistoryCache( + base->GetHistoryLength(epochId), + base->GetHistoryRoot(epochId), + epochId + ); + return historyCacheMap.insert({epochId, cache}).first->second; + } +} + +void CCoinsViewCache::PushHistoryNode(uint32_t epochId, const HistoryNode node) { + HistoryCache& historyCache = SelectHistoryCache(epochId); + + if (historyCache.length == 0) { + // special case, it just goes into the cache right away + historyCache.Extend(node); + + if (librustzcash_mmr_hash_node(epochId, node.data(), historyCache.root.begin()) != 0) { + throw std::runtime_error("hashing node failed"); + }; + + return; + } + + std::vector entries; + std::vector entry_indices; + + PreloadHistoryTree(epochId, false, entries, entry_indices); + + uint256 newRoot; + std::array appendBuf; + + uint32_t appends = librustzcash_mmr_append( + epochId, + historyCache.length, + entry_indices.data(), + entries.data()->data(), + entry_indices.size(), + node.data(), + newRoot.begin(), + appendBuf.data()->data() + ); + + for (size_t i = 0; i < appends; i++) { + historyCache.Extend(appendBuf[i]); + } + + historyCache.root = newRoot; +} + +void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { + HistoryCache& historyCache = SelectHistoryCache(epochId); + uint256 newRoot; + + switch (historyCache.length) { + case 0: + // Caller is not expected to pop from empty tree! Caller should + // switch to previous epoch and pop history from there. + throw std::runtime_error("popping history node from empty history"); + case 1: + // Just resetting tree to empty + historyCache.Truncate(0); + historyCache.root = uint256(); + return; + case 2: + // - A tree with one leaf has length 1. + // - A tree with two leaves has length 3. + throw std::runtime_error("a history tree cannot have two nodes"); + case 3: + // After removing a leaf from a tree with two leaves, we are left + // with a single-node tree, whose root is just the hash of that + // node. + if (librustzcash_mmr_hash_node( + epochId, + GetHistoryAt(epochId, 0).data(), + newRoot.begin() + ) != 0) { + throw std::runtime_error("hashing node failed"); + } + historyCache.Truncate(1); + historyCache.root = newRoot; + return; + default: + // This is a non-elementary pop, so use the full tree logic. + std::vector entries; + std::vector entry_indices; + + uint32_t peak_count = PreloadHistoryTree(epochId, true, entries, entry_indices); + + uint32_t numberOfDeletes = librustzcash_mmr_delete( + epochId, + historyCache.length, + entry_indices.data(), + entries.data()->data(), + peak_count, + entries.size() - peak_count, + newRoot.begin() + ); + + historyCache.Truncate(historyCache.length - numberOfDeletes); + historyCache.root = newRoot; + return; + } +} + template void CCoinsViewCache::AbstractPopAnchor( const uint256 &newrt, @@ -470,6 +732,35 @@ void BatchWriteAnchors( } } +void BatchWriteHistory(CHistoryCacheMap& historyCacheMap, CHistoryCacheMap& historyCacheMapIn) { + for (auto nextHistoryCache = historyCacheMapIn.begin(); nextHistoryCache != historyCacheMapIn.end(); nextHistoryCache++) { + auto historyCacheIn = nextHistoryCache->second; + auto epochId = nextHistoryCache->first; + + auto historyCache = historyCacheMap.find(epochId); + if (historyCache != historyCacheMap.end()) { + // delete old entries since updateDepth + historyCache->second.Truncate(historyCacheIn.updateDepth); + + // Replace/append new/updated entries. HistoryCache.Extend + // auto-indexes the nodes, so we need to extend in the same order as + // this cache is indexed. + for (size_t i = historyCacheIn.updateDepth; i < historyCacheIn.length; i++) { + historyCache->second.Extend(historyCacheIn.appends[i]); + } + + // the lengths should now match + assert(historyCache->second.length == historyCacheIn.length); + + // write current root + historyCache->second.root = historyCacheIn.root; + } else { + // Just insert the history cache into its parent + historyCacheMap.insert({epochId, historyCacheIn}); + } + } +} + bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn, const uint256 &hashSproutAnchorIn, @@ -477,7 +768,8 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers) { + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMapIn) { assert(!hasModifier); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). @@ -520,6 +812,8 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, ::BatchWriteNullifiers(mapSproutNullifiers, cacheSproutNullifiers); ::BatchWriteNullifiers(mapSaplingNullifiers, cacheSaplingNullifiers); + ::BatchWriteHistory(historyCacheMap, historyCacheMapIn); + hashSproutAnchor = hashSproutAnchorIn; hashSaplingAnchor = hashSaplingAnchorIn; hashBlock = hashBlockIn; @@ -527,12 +821,21 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, } bool CCoinsViewCache::Flush() { - bool fOk = base->BatchWrite(cacheCoins, hashBlock, hashSproutAnchor, hashSaplingAnchor, cacheSproutAnchors, cacheSaplingAnchors, cacheSproutNullifiers, cacheSaplingNullifiers); + bool fOk = base->BatchWrite(cacheCoins, + hashBlock, + hashSproutAnchor, + hashSaplingAnchor, + cacheSproutAnchors, + cacheSaplingAnchors, + cacheSproutNullifiers, + cacheSaplingNullifiers, + historyCacheMap); cacheCoins.clear(); cacheSproutAnchors.clear(); cacheSaplingAnchors.clear(); cacheSproutNullifiers.clear(); cacheSaplingNullifiers.clear(); + historyCacheMap.clear(); cachedCoinsUsage = 0; return fOk; } diff --git a/src/coins.h b/src/coins.h index c29c13315..10999e8d9 100644 --- a/src/coins.h +++ b/src/coins.h @@ -17,6 +17,7 @@ #include #include +#include "zcash/History.hpp" #include "zcash/IncrementalMerkleTree.hpp" /** @@ -321,6 +322,7 @@ typedef boost::unordered_map CCoinsM typedef boost::unordered_map CAnchorsSproutMap; typedef boost::unordered_map CAnchorsSaplingMap; typedef boost::unordered_map CNullifiersMap; +typedef boost::unordered_map CHistoryCacheMap; struct CCoinsStats { @@ -362,6 +364,15 @@ public: //! Get the current "tip" or the latest anchored tree root in the chain virtual uint256 GetBestAnchor(ShieldedType type) const; + //! Get the current chain history length (which should be roughly chain height x2) + virtual HistoryIndex GetHistoryLength(uint32_t epochId) const; + + //! Get history node at specified index + virtual HistoryNode GetHistoryAt(uint32_t epochId, HistoryIndex index) const; + + //! Get current history root + virtual uint256 GetHistoryRoot(uint32_t epochId) const; + //! Do a bulk modification (multiple CCoins changes + BestBlock change). //! The passed mapCoins can be modified. virtual bool BatchWrite(CCoinsMap &mapCoins, @@ -371,7 +382,8 @@ public: CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers); + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap); //! Calculate statistics about the unspent transaction output set virtual bool GetStats(CCoinsStats &stats) const; @@ -396,6 +408,9 @@ public: bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; uint256 GetBestAnchor(ShieldedType type) const; + HistoryIndex GetHistoryLength(uint32_t epochId) const; + HistoryNode GetHistoryAt(uint32_t epochId, HistoryIndex index) const; + uint256 GetHistoryRoot(uint32_t epochId) const; void SetBackend(CCoinsView &viewIn); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, @@ -404,7 +419,8 @@ public: CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers); + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap); bool GetStats(CCoinsStats &stats) const; }; @@ -451,6 +467,7 @@ protected: mutable CAnchorsSaplingMap cacheSaplingAnchors; mutable CNullifiersMap cacheSproutNullifiers; mutable CNullifiersMap cacheSaplingNullifiers; + mutable CHistoryCacheMap historyCacheMap; /* Cached dynamic memory usage for the inner CCoins objects. */ mutable size_t cachedCoinsUsage; @@ -467,6 +484,9 @@ public: bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; uint256 GetBestAnchor(ShieldedType type) const; + HistoryIndex GetHistoryLength(uint32_t epochId) const; + HistoryNode GetHistoryAt(uint32_t epochId, HistoryIndex index) const; + uint256 GetHistoryRoot(uint32_t epochId) const; void SetBestBlock(const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, @@ -475,8 +495,8 @@ public: CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers); - + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap); // Adds the tree to mapSproutAnchors (or mapSaplingAnchors based on the type of tree) // and sets the current commitment root to this root. @@ -489,6 +509,12 @@ public: // Marks nullifiers for a given transaction as spent or not. void SetNullifiers(const CTransaction& tx, bool spent); + // Push MMR node history at the end of the history tree + void PushHistoryNode(uint32_t epochId, const HistoryNode node); + + // Pop MMR node history from the end of the history tree + void PopHistoryNode(uint32_t epochId); + /** * Return a pointer to CCoins in the cache, or NULL if not found. This is * more efficient than GetCoins. Modifications to other cache entries are @@ -582,6 +608,18 @@ private: const uint256 ¤tRoot, Tree &tree ); + + //! Preload history tree for further update. + //! + //! If extra = true, extra nodes for deletion are also preloaded. + //! This will allow to delete tail entries from preloaded tree without + //! any further database lookups. + //! + //! Returns number of peaks, not total number of loaded nodes. + uint32_t PreloadHistoryTree(uint32_t epochId, bool extra, std::vector &entries, std::vector &entry_indices); + + //! Selects history cache for specified epoch. + HistoryCache& SelectHistoryCache(uint32_t epochId) const; }; #endif // BITCOIN_COINS_H diff --git a/src/gtest/test_history.cpp b/src/gtest/test_history.cpp new file mode 100644 index 000000000..b271ecef8 --- /dev/null +++ b/src/gtest/test_history.cpp @@ -0,0 +1,171 @@ +#include + +#include "main.h" +#include "utiltest.h" +#include "zcash/History.hpp" + +// Fake an empty view +class FakeCoinsViewDB : public CCoinsView { +public: + FakeCoinsViewDB() {} + + bool GetSproutAnchorAt(const uint256 &rt, SproutMerkleTree &tree) const { + return false; + } + + bool GetSaplingAnchorAt(const uint256 &rt, SaplingMerkleTree &tree) const { + return false; + } + + bool GetNullifier(const uint256 &nf, ShieldedType type) const { + return false; + } + + bool GetCoins(const uint256 &txid, CCoins &coins) const { + return false; + } + + bool HaveCoins(const uint256 &txid) const { + return false; + } + + uint256 GetBestBlock() const { + uint256 a; + return a; + } + + uint256 GetBestAnchor(ShieldedType type) const { + uint256 a; + return a; + } + + bool BatchWrite(CCoinsMap &mapCoins, + const uint256 &hashBlock, + const uint256 &hashSproutAnchor, + const uint256 &hashSaplingAnchor, + CAnchorsSproutMap &mapSproutAnchors, + CAnchorsSaplingMap &mapSaplingAnchors, + CNullifiersMap &mapSproutNullifiers, + CNullifiersMap saplingNullifiersMap) { + return false; + } + + bool GetStats(CCoinsStats &stats) const { + return false; + } + + HistoryIndex GetHistoryLength(uint32_t branchId) const { + return 0; + } + + HistoryNode GetHistoryAt(uint32_t branchId, HistoryIndex index) const { + return HistoryNode(); + } +}; + +HistoryNode getLeafN(uint64_t block_num) { + HistoryNode node = libzcash::NewLeaf( + uint256(), + block_num*10, + block_num*13, + uint256(), + uint256(), + block_num, + 3 + ); + return node; +} + +TEST(History, Smoky) { + // Fake an empty view + FakeCoinsViewDB fakeDB; + CCoinsViewCache view(&fakeDB); + + // Test initial value + EXPECT_EQ(view.GetHistoryLength(0), 0); + + view.PushHistoryNode(1, getLeafN(1)); + + EXPECT_EQ(view.GetHistoryLength(1), 1); + + view.PushHistoryNode(1, getLeafN(2)); + + EXPECT_EQ(view.GetHistoryLength(1), 3); + + view.PushHistoryNode(1, getLeafN(3)); + + EXPECT_EQ(view.GetHistoryLength(1), 4); + + view.PushHistoryNode(1, getLeafN(4)); + + uint256 h4Root = view.GetHistoryRoot(1); + + EXPECT_EQ(view.GetHistoryLength(1), 7); + + view.PushHistoryNode(1, getLeafN(5)); + EXPECT_EQ(view.GetHistoryLength(1), 8); + + view.PopHistoryNode(1); + + EXPECT_EQ(view.GetHistoryLength(1), 7); + EXPECT_EQ(h4Root, view.GetHistoryRoot(1)); +} + + +TEST(History, EpochBoundaries) { + // Fake an empty view + FakeCoinsViewDB fakeDB; + CCoinsViewCache view(&fakeDB); + + view.PushHistoryNode(1, getLeafN(1)); + + EXPECT_EQ(view.GetHistoryLength(1), 1); + + view.PushHistoryNode(1, getLeafN(2)); + + EXPECT_EQ(view.GetHistoryLength(1), 3); + + view.PushHistoryNode(1, getLeafN(3)); + + EXPECT_EQ(view.GetHistoryLength(1), 4); + + view.PushHistoryNode(1, getLeafN(4)); + + uint256 h4Root = view.GetHistoryRoot(1); + + EXPECT_EQ(view.GetHistoryLength(1), 7); + + view.PushHistoryNode(1, getLeafN(5)); + EXPECT_EQ(view.GetHistoryLength(1), 8); + + + // New Epoch(2) + view.PushHistoryNode(2, getLeafN(6)); + EXPECT_EQ(view.GetHistoryLength(1), 8); + EXPECT_EQ(view.GetHistoryLength(2), 1); + + view.PushHistoryNode(2, getLeafN(7)); + EXPECT_EQ(view.GetHistoryLength(1), 8); + EXPECT_EQ(view.GetHistoryLength(2), 3); + + view.PushHistoryNode(2, getLeafN(8)); + EXPECT_EQ(view.GetHistoryLength(1), 8); + EXPECT_EQ(view.GetHistoryLength(2), 4); + + // Rolling epoch back to 1 + view.PopHistoryNode(2); + EXPECT_EQ(view.GetHistoryLength(2), 3); + + view.PopHistoryNode(2); + EXPECT_EQ(view.GetHistoryLength(2), 1); + EXPECT_EQ(view.GetHistoryLength(1), 8); + + // And even rolling epoch 1 back a bit + view.PopHistoryNode(1); + EXPECT_EQ(view.GetHistoryLength(1), 7); + + // And also rolling epoch 2 back to 0 + view.PopHistoryNode(2); + EXPECT_EQ(view.GetHistoryLength(2), 0); + +} diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index c94a91efa..29fa9e143 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -170,7 +170,8 @@ public: CAnchorsSproutMap& mapSproutAnchors, CAnchorsSaplingMap& mapSaplingAnchors, CNullifiersMap& mapSproutNullifiers, - CNullifiersMap& mapSaplingNullifiers) + CNullifiersMap& mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap) { for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { @@ -214,7 +215,8 @@ public: memusage::DynamicUsage(cacheSproutAnchors) + memusage::DynamicUsage(cacheSaplingAnchors) + memusage::DynamicUsage(cacheSproutNullifiers) + - memusage::DynamicUsage(cacheSaplingNullifiers); + memusage::DynamicUsage(cacheSaplingNullifiers) + + memusage::DynamicUsage(historyCacheMap); for (CCoinsMap::iterator it = cacheCoins.begin(); it != cacheCoins.end(); it++) { ret += it->second.coins.DynamicMemoryUsage(); } diff --git a/src/txdb.cpp b/src/txdb.cpp index 0fd95038e..aa5749244 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -35,6 +35,10 @@ static const char DB_FLAG = 'F'; static const char DB_REINDEX_FLAG = 'R'; static const char DB_LAST_BLOCK = 'l'; +static const char DB_MMR_LENGTH = 'M'; +static const char DB_MMR_NODE = 'm'; +static const char DB_MMR_ROOT = 'r'; + // insightexplorer static const char DB_ADDRESSINDEX = 'd'; static const char DB_ADDRESSUNSPENTINDEX = 'u'; @@ -124,6 +128,39 @@ uint256 CCoinsViewDB::GetBestAnchor(ShieldedType type) const { return hashBestAnchor; } +HistoryIndex CCoinsViewDB::GetHistoryLength(uint32_t epochId) const { + HistoryIndex historyLength; + if (!db.Read(make_pair(DB_MMR_LENGTH, epochId), historyLength)) { + // Starting new history + historyLength = 0; + } + + return historyLength; +} + +HistoryNode CCoinsViewDB::GetHistoryAt(uint32_t epochId, HistoryIndex index) const { + HistoryNode mmrNode; + + if (index >= GetHistoryLength(epochId)) { + throw runtime_error("History data inconsistent - reindex?"); + } + + if (!db.Read(make_pair(DB_MMR_NODE, make_pair(epochId, index)), mmrNode)) { + throw runtime_error("History data inconsistent (expected node not found) - reindex?"); + } + + return mmrNode; +} + +uint256 CCoinsViewDB::GetHistoryRoot(uint32_t epochId) const { + uint256 root; + if (!db.Read(make_pair(DB_MMR_ROOT, epochId), root)) + { + root = uint256(); + } + return root; +} + void BatchWriteNullifiers(CDBBatch& batch, CNullifiersMap& mapToUse, const char& dbChar) { for (CNullifiersMap::iterator it = mapToUse.begin(); it != mapToUse.end();) { @@ -158,6 +195,29 @@ void BatchWriteAnchors(CDBBatch& batch, Map& mapToUse, const char& dbChar) } } +void BatchWriteHistory(CDBBatch& batch, CHistoryCacheMap& historyCacheMap) { + for (auto nextHistoryCache = historyCacheMap.begin(); nextHistoryCache != historyCacheMap.end(); nextHistoryCache++) { + auto historyCache = nextHistoryCache->second; + auto epochId = nextHistoryCache->first; + + // delete old entries since updateDepth + for (int i = historyCache.updateDepth + 1; i <= historyCache.length; i++) { + batch.Erase(make_pair(DB_MMR_NODE, make_pair(epochId, i))); + } + + // replace/append new/updated entries + for (auto it = historyCache.appends.begin(); it != historyCache.appends.end(); it++) { + batch.Write(make_pair(DB_MMR_NODE, make_pair(epochId, it->first)), it->second); + } + + // write new length + batch.Write(make_pair(DB_MMR_LENGTH, epochId), historyCache.length); + + // write current root + batch.Write(make_pair(DB_MMR_ROOT, epochId), historyCache.root); + } +} + bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashSproutAnchor, @@ -165,7 +225,8 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers) { + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap) { CDBBatch batch(db); size_t count = 0; size_t changed = 0; @@ -188,6 +249,8 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, ::BatchWriteNullifiers(batch, mapSproutNullifiers, DB_NULLIFIER); ::BatchWriteNullifiers(batch, mapSaplingNullifiers, DB_SAPLING_NULLIFIER); + ::BatchWriteHistory(batch, historyCacheMap); + if (!hashBlock.IsNull()) batch.Write(DB_BEST_BLOCK, hashBlock); if (!hashSproutAnchor.IsNull()) diff --git a/src/txdb.h b/src/txdb.h index b27bc4abb..cc1d576cb 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -15,6 +15,9 @@ #include #include +#include +#include "zcash/History.hpp" + class CBlockIndex; // START insightexplorer @@ -85,6 +88,9 @@ public: bool HaveCoins(const uint256 &txid) const; uint256 GetBestBlock() const; uint256 GetBestAnchor(ShieldedType type) const; + HistoryIndex GetHistoryLength(uint32_t epochId) const; + HistoryNode GetHistoryAt(uint32_t epochId, HistoryIndex index) const; + uint256 GetHistoryRoot(uint32_t epochId) const; bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, const uint256 &hashSproutAnchor, @@ -92,7 +98,8 @@ public: CAnchorsSproutMap &mapSproutAnchors, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, - CNullifiersMap &mapSaplingNullifiers); + CNullifiersMap &mapSaplingNullifiers, + CHistoryCacheMap &historyCacheMap); bool GetStats(CCoinsStats &stats) const; }; diff --git a/src/zcash/History.cpp b/src/zcash/History.cpp new file mode 100644 index 000000000..92f95c686 --- /dev/null +++ b/src/zcash/History.cpp @@ -0,0 +1,120 @@ +#include "zcash/History.hpp" + +#include + +#include + +#include "serialize.h" +#include "streams.h" +#include "uint256.h" +#include "librustzcash.h" + +namespace libzcash { + +void HistoryCache::Extend(const HistoryNode &leaf) { + appends[length++] = leaf; +} + +void HistoryCache::Truncate(HistoryIndex newLength) { + for (HistoryIndex idx = length; idx > newLength; idx--) { + appends.erase(idx); + } + + length = newLength; + // we track how deep updates go back in the tree, so we could later + // update everything starting from `updateDepth` + // + // imagine we rolled two blocks back and then put another 3 blocks on top + // of the rolled back state. In that case `updateDepth` will be H-3, while length + // will be H (where H is a final chain height after such operation). So we know that + // history entries in the range of H-3..H are expected to be pushed into the database + // to replace/append to the persistent nodes there. + if (updateDepth > length) updateDepth = length; +} + +HistoryNode NewNode( + uint256 subtreeCommitment, + uint32_t startTime, + uint32_t endTime, + uint32_t startTarget, + uint32_t endTarget, + uint256 startSaplingRoot, + uint256 endSaplingRoot, + uint256 subtreeTotalWork, + uint64_t startHeight, + uint64_t endHeight, + uint64_t saplingTxCount + ) +{ + CDataStream buf(SER_DISK, 0); + HistoryNode result; + + buf << subtreeCommitment; + buf << startTime; + buf << endTime; + buf << startTarget; + buf << endTarget; + buf << startSaplingRoot; + buf << endSaplingRoot; + buf << subtreeTotalWork; + buf << COMPACTSIZE(startHeight); + buf << COMPACTSIZE(endHeight); + buf << COMPACTSIZE(saplingTxCount); + + std::copy(buf.begin(), buf.end(), result.begin()); + return result; +} + +HistoryNode NewLeaf( + uint256 commitment, + uint32_t time, + uint32_t target, + uint256 saplingRoot, + uint256 totalWork, + uint64_t height, + uint64_t saplingTxCount +) { + return NewNode( + commitment, + time, + time, + target, + target, + saplingRoot, + saplingRoot, + totalWork, + height, + height, + saplingTxCount + ); +} + +HistoryEntry NodeToEntry(const HistoryNode node, uint32_t left, uint32_t right) { + CDataStream buf(SER_DISK, 0); + HistoryEntry result; + + uint8_t code = 0; + buf << code; + buf << left; + buf << right; + buf << node; + + std::copy(std::begin(buf), std::end(buf), std::begin(result)); + + return result; +} + +HistoryEntry LeafToEntry(const HistoryNode node) { + CDataStream buf(SER_DISK, 0); + HistoryEntry result; + + uint8_t code = 1; + buf << code; + buf << node; + + std::copy(std::begin(buf), std::end(buf), std::begin(result)); + + return result; +} + +} \ No newline at end of file diff --git a/src/zcash/History.hpp b/src/zcash/History.hpp new file mode 100644 index 000000000..19a0d4329 --- /dev/null +++ b/src/zcash/History.hpp @@ -0,0 +1,74 @@ +#ifndef ZC_HISTORY_H_ +#define ZC_HISTORY_H_ + +#include +#include +#include + +#include "serialize.h" +#include "streams.h" +#include "uint256.h" + +#include "librustzcash.h" + +namespace libzcash { + +const int NODE_SERIALIZED_LENGTH = 171; +const int ENTRY_SERIALIZED_LENGTH = 180; + +typedef std::array HistoryNode; +typedef std::array HistoryEntry; + +typedef uint64_t HistoryIndex; + +class HistoryCache { +public: + // updates to the persistent(db) layer + std::unordered_map appends; + // current length of the history + HistoryIndex length; + // how much back into the old state current update state + // goes + HistoryIndex updateDepth; + // current root of the history + uint256 root; + // current epoch of this history state + uint32_t epoch; + + HistoryCache(HistoryIndex initialLength, uint256 initialRoot, uint32_t initialEpoch) : + length(initialLength), updateDepth(initialLength), root(initialRoot), epoch(initialEpoch) { }; + + HistoryCache() { } + + // Extends current history update by one history node. + void Extend(const HistoryNode &leaf); + + // Truncates current history to the new length. + void Truncate(HistoryIndex newLength); +}; + +// New history node with metadata based on block state. +HistoryNode NewLeaf( + uint256 commitment, + uint32_t time, + uint32_t target, + uint256 saplingRoot, + uint256 totalWork, + uint64_t height, + uint64_t saplingTxCount +); + +// Convert history node to tree node (with children references) +HistoryEntry NodeToEntry(const HistoryNode node, uint32_t left, uint32_t right); + +// Convert history node to leaf node (end nodes without children) +HistoryEntry LeafToEntry(const HistoryNode node); + +} + +typedef libzcash::HistoryCache HistoryCache; +typedef libzcash::HistoryIndex HistoryIndex; +typedef libzcash::HistoryNode HistoryNode; +typedef libzcash::HistoryEntry HistoryEntry; + +#endif /* ZC_HISTORY_H_ */ \ No newline at end of file From 75d2f782b5e08da6f5ededf7d6cfd67ca2fc398a Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 22 Nov 2019 00:30:00 -0800 Subject: [PATCH 02/17] update chain history in ConnectBlock and DisconnectBlock --- src/main.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index a3d0b058f..bc4007825 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2403,6 +2403,12 @@ static DisconnectResult DisconnectBlock(const CBlock& block, CValidationState& s view.PopAnchor(SaplingMerkleTree::empty_root(), SAPLING); } + auto consensusBranchId = CurrentEpochBranchId(pindex->nHeight, chainparams.GetConsensus()); + + if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + view.PopHistoryNode(consensusBranchId); + } + // move best block pointer to prevout block view.SetBestBlock(pindex->pprev->GetBlockHash()); @@ -2663,6 +2669,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Grab the consensus branch ID for the block's height auto consensusBranchId = CurrentEpochBranchId(pindex->nHeight, chainparams.GetConsensus()); + size_t total_sapling_tx = 0; + std::vector txdata; txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) @@ -2781,6 +2789,10 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin sapling_tree.append(outputDescription.cmu); } + if (!(tx.vShieldedSpend.empty() && tx.vShieldedOutput.empty())) { + total_sapling_tx += 1; + } + vPos.push_back(std::make_pair(tx.GetHash(), pos)); pos.nTxOffset += ::GetSerializeSize(tx, SER_DISK, CLIENT_VERSION); } @@ -2802,6 +2814,21 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } + // History read/write is started with Heartwood update. + if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + auto historyNode = libzcash::NewLeaf( + block.GetHash(), + block.nTime, + block.nBits, + block.hashFinalSaplingRoot, + ArithToUint256(GetBlockProof(*pindex)), + pindex->nHeight, + total_sapling_tx + ); + + view.PushHistoryNode(consensusBranchId, historyNode); + } + int64_t nTime1 = GetTimeMicros(); nTimeConnect += nTime1 - nTimeStart; LogPrint("bench", " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs]\n", (unsigned)block.vtx.size(), 0.001 * (nTime1 - nTimeStart), 0.001 * (nTime1 - nTimeStart) / block.vtx.size(), nInputs <= 1 ? 0 : 0.001 * (nTime1 - nTimeStart) / (nInputs-1), nTimeConnect * 0.000001); From 5608118cc4733719925f0e9e1a51e39c0975a7db Mon Sep 17 00:00:00 2001 From: NikVolf Date: Sat, 14 Mar 2020 01:43:25 -0700 Subject: [PATCH 03/17] use iterative platform-independent log2i --- src/coins.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coins.cpp b/src/coins.cpp index 15ccb21ab..76262064d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -316,7 +316,9 @@ void draftMMRNode(std::vector &indices, static inline uint32_t log2i(uint32_t x) { assert(x > 0); - return 31 - __builtin_clz(x); + int log = 0; + while (x >>= 1) ++log; + return log; } // Computes floor(log2(x+1)) From cf480fe402844f4c95fb57ccf3fc0568bc0aec7b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Apr 2020 23:52:37 +1300 Subject: [PATCH 04/17] Add ZIP 221 logic to block index CBlockHeader.hashFinalSaplingRoot has been renamed to hashLightClient. CBlockIndex now stores: - hashLightClient as from the block header - hashFinalSaplingRoot, which is accurate for all blocks prior to Heartwood activation, and all blocks from Heartwood activation onward that are connected at some point to the main chain in ConnectBlock(). - hashChainHistoryRoot, which is null prior to Heartwood activation, and set per ZIP 221 from Heartwood activation. The new block index fields are only written to disk for client version 2.1.2 and above, which will be the first Heartwood-aware clients (even if Heartwood doesn't have an activation height). --- src/chain.h | 39 +++++++++++++++++++++++++++++++++------ src/main.cpp | 21 +++++++++++++++------ src/miner.cpp | 2 +- src/primitives/block.cpp | 4 ++-- src/primitives/block.h | 10 +++++----- src/rpc/blockchain.cpp | 3 ++- src/rpc/mining.cpp | 2 +- src/test/miner_tests.cpp | 4 ++-- src/txdb.cpp | 23 +++++++++++++++++++++-- src/txdb.h | 4 +++- 10 files changed, 85 insertions(+), 27 deletions(-) diff --git a/src/chain.h b/src/chain.h index 47a8bd82f..71a5b2c42 100644 --- a/src/chain.h +++ b/src/chain.h @@ -16,6 +16,7 @@ static const int SPROUT_VALUE_VERSION = 1001400; static const int SAPLING_VALUE_VERSION = 1010100; +static const int CHAIN_HISTORY_ROOT_VERSION = 2010200; /** * Maximum amount of time that a block timestamp is allowed to be ahead of the @@ -253,10 +254,25 @@ public: //! Will be boost::none if nChainTx is zero. boost::optional nChainSaplingValue; + //! Root of the Sapling commitment tree as of the end of this block. This is only set + //! once a block has been connected to the main chain, and will be null otherwise. + //! + //! For blocks prior to Heartwood activation, this is always equal to + //! hashLightClientRoot. + uint256 hashFinalSaplingRoot; + + //! Root of the ZIP 221 history tree as of the end of the previous block. This is only + //! set once a block has been connected to the main chain, and will be null otherwise. + //! + //! - For blocks prior to and including Heartwood activation, this is always null. + //! - For blocks after Heartwood activation, this is always equal to + //! hashLightClientRoot. + uint256 hashChainHistoryRoot; + //! block header int nVersion; uint256 hashMerkleRoot; - uint256 hashFinalSaplingRoot; + uint256 hashLightClientRoot; unsigned int nTime; unsigned int nBits; uint256 nNonce; @@ -289,7 +305,7 @@ public: nVersion = 0; hashMerkleRoot = uint256(); - hashFinalSaplingRoot = uint256(); + hashLightClientRoot = uint256(); nTime = 0; nBits = 0; nNonce = uint256(); @@ -307,7 +323,7 @@ public: nVersion = block.nVersion; hashMerkleRoot = block.hashMerkleRoot; - hashFinalSaplingRoot = block.hashFinalSaplingRoot; + hashLightClientRoot = block.hashLightClientRoot; nTime = block.nTime; nBits = block.nBits; nNonce = block.nNonce; @@ -339,7 +355,7 @@ public: if (pprev) block.hashPrevBlock = pprev->GetBlockHash(); block.hashMerkleRoot = hashMerkleRoot; - block.hashFinalSaplingRoot = hashFinalSaplingRoot; + block.hashLightClientRoot = hashLightClientRoot; block.nTime = nTime; block.nBits = nBits; block.nNonce = nNonce; @@ -461,7 +477,7 @@ public: READWRITE(this->nVersion); READWRITE(hashPrev); READWRITE(hashMerkleRoot); - READWRITE(hashFinalSaplingRoot); + READWRITE(hashLightClientRoot); READWRITE(nTime); READWRITE(nBits); READWRITE(nNonce); @@ -479,6 +495,17 @@ public: READWRITE(nSaplingValue); } + // Only read/write hashFinalSaplingRoot and hashChainHistoryRoot if the + // client version used to create this index was storing them. + if ((s.GetType() & SER_DISK) && (nVersion >= CHAIN_HISTORY_ROOT_VERSION)) { + READWRITE(hashFinalSaplingRoot); + READWRITE(hashChainHistoryRoot); + } else if (ser_action.ForRead()) { + // For block indices written before the client was Heartwood-aware, + // these are always identical. + hashFinalSaplingRoot = hashLightClientRoot; + } + // If you have just added new serialized fields above, remember to add // them to CBlockTreeDB::LoadBlockIndexGuts() in txdb.cpp :) } @@ -489,7 +516,7 @@ public: block.nVersion = nVersion; block.hashPrevBlock = hashPrev; block.hashMerkleRoot = hashMerkleRoot; - block.hashFinalSaplingRoot = hashFinalSaplingRoot; + block.hashLightClientRoot = hashLightClientRoot; block.nTime = nTime; block.nBits = nBits; block.nNonce = nNonce; diff --git a/src/main.cpp b/src/main.cpp index bc4007825..44d0b5a02 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2666,8 +2666,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin SaplingMerkleTree sapling_tree; assert(view.GetSaplingAnchorAt(view.GetBestAnchor(SAPLING), sapling_tree)); - // Grab the consensus branch ID for the block's height + // Grab the consensus branch ID for this block and its parent auto consensusBranchId = CurrentEpochBranchId(pindex->nHeight, chainparams.GetConsensus()); + auto prevConsensusBranchId = CurrentEpochBranchId(pindex->nHeight - 1, chainparams.GetConsensus()); size_t total_sapling_tx = 0; @@ -2801,15 +2802,22 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin view.PushAnchor(sapling_tree); if (!fJustCheck) { pindex->hashFinalSproutRoot = sprout_tree.root(); + pindex->hashFinalSaplingRoot = sapling_tree.root(); + if (IsActivationHeight(pindex->nHeight, chainparams.GetConsensus(), Consensus::UPGRADE_HEARTWOOD)) { + // The default is null, but let's make it explicit. + pindex->hashChainHistoryRoot.SetNull(); + } else if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + pindex->hashChainHistoryRoot = view.GetHistoryRoot(prevConsensusBranchId); + } } blockundo.old_sprout_tree_root = old_sprout_tree_root; - // If Sapling is active, block.hashFinalSaplingRoot must be the + // If Sapling is active, block.hashLightClientRoot must be the // same as the root of the Sapling tree if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_SAPLING)) { - if (block.hashFinalSaplingRoot != sapling_tree.root()) { + if (block.hashLightClientRoot != sapling_tree.root()) { return state.DoS(100, - error("ConnectBlock(): block's hashFinalSaplingRoot is incorrect"), + error("ConnectBlock(): block's hashLightClientRoot is incorrect"), REJECT_INVALID, "bad-sapling-root-in-block"); } } @@ -2820,7 +2828,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin block.GetHash(), block.nTime, block.nBits, - block.hashFinalSaplingRoot, + pindex->hashFinalSaplingRoot, ArithToUint256(GetBlockProof(*pindex)), pindex->nHeight, total_sapling_tx @@ -3561,6 +3569,7 @@ CBlockIndex* AddToBlockIndex(const CBlockHeader& block) { pindexNew->pprev = (*miPrev).second; pindexNew->nHeight = pindexNew->pprev->nHeight + 1; + // hashFinalSaplingRoot and hashChainHistoryRoot are set in ConnectBlock() pindexNew->BuildSkip(); } pindexNew->nChainWork = (pindexNew->pprev ? pindexNew->pprev->nChainWork : 0) + GetBlockProof(*pindexNew); @@ -4366,7 +4375,7 @@ CBlockIndex * InsertBlockIndex(uint256 hash) bool static LoadBlockIndexDB() { const CChainParams& chainparams = Params(); - if (!pblocktree->LoadBlockIndexGuts(InsertBlockIndex)) + if (!pblocktree->LoadBlockIndexGuts(InsertBlockIndex, chainparams)) return false; boost::this_thread::interruption_point(); diff --git a/src/miner.cpp b/src/miner.cpp index fac01270a..2debf8954 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -520,7 +520,7 @@ CBlockTemplate* CreateNewBlock(const CChainParams& chainparams, const MinerAddre // Fill in header pblock->hashPrevBlock = pindexPrev->GetBlockHash(); - pblock->hashFinalSaplingRoot = sapling_tree.root(); + pblock->hashLightClientRoot = sapling_tree.root(); UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus()); pblock->nSolution.clear(); diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index ab80e3255..3e685b6c3 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -112,12 +112,12 @@ uint256 CBlock::CheckMerkleBranch(uint256 hash, const std::vector& vMer std::string CBlock::ToString() const { std::stringstream s; - s << strprintf("CBlock(hash=%s, ver=%d, hashPrevBlock=%s, hashMerkleRoot=%s, hashFinalSaplingRoot=%s, nTime=%u, nBits=%08x, nNonce=%s, vtx=%u)\n", + s << strprintf("CBlock(hash=%s, ver=%d, hashPrevBlock=%s, hashMerkleRoot=%s, hashLightClientRoot=%s, nTime=%u, nBits=%08x, nNonce=%s, vtx=%u)\n", GetHash().ToString(), nVersion, hashPrevBlock.ToString(), hashMerkleRoot.ToString(), - hashFinalSaplingRoot.ToString(), + hashLightClientRoot.ToString(), nTime, nBits, nNonce.ToString(), vtx.size()); for (unsigned int i = 0; i < vtx.size(); i++) diff --git a/src/primitives/block.h b/src/primitives/block.h index d8f659703..7282700eb 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -26,7 +26,7 @@ public: int32_t nVersion; uint256 hashPrevBlock; uint256 hashMerkleRoot; - uint256 hashFinalSaplingRoot; + uint256 hashLightClientRoot; uint32_t nTime; uint32_t nBits; uint256 nNonce; @@ -44,7 +44,7 @@ public: READWRITE(this->nVersion); READWRITE(hashPrevBlock); READWRITE(hashMerkleRoot); - READWRITE(hashFinalSaplingRoot); + READWRITE(hashLightClientRoot); READWRITE(nTime); READWRITE(nBits); READWRITE(nNonce); @@ -56,7 +56,7 @@ public: nVersion = CBlockHeader::CURRENT_VERSION; hashPrevBlock.SetNull(); hashMerkleRoot.SetNull(); - hashFinalSaplingRoot.SetNull(); + hashLightClientRoot.SetNull(); nTime = 0; nBits = 0; nNonce = uint256(); @@ -118,7 +118,7 @@ public: block.nVersion = nVersion; block.hashPrevBlock = hashPrevBlock; block.hashMerkleRoot = hashMerkleRoot; - block.hashFinalSaplingRoot = hashFinalSaplingRoot; + block.hashLightClientRoot = hashLightClientRoot; block.nTime = nTime; block.nBits = nBits; block.nNonce = nNonce; @@ -158,7 +158,7 @@ public: READWRITE(this->nVersion); READWRITE(hashPrevBlock); READWRITE(hashMerkleRoot); - READWRITE(hashFinalSaplingRoot); + READWRITE(hashLightClientRoot); READWRITE(nTime); READWRITE(nBits); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 796f99805..e176eff0e 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -229,7 +229,8 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx result.push_back(Pair("height", blockindex->nHeight)); result.push_back(Pair("version", block.nVersion)); result.push_back(Pair("merkleroot", block.hashMerkleRoot.GetHex())); - result.push_back(Pair("finalsaplingroot", block.hashFinalSaplingRoot.GetHex())); + result.push_back(Pair("finalsaplingroot", blockindex->hashFinalSaplingRoot.GetHex())); + result.push_back(Pair("chainhistoryroot", blockindex->hashChainHistoryRoot.GetHex())); UniValue txs(UniValue::VARR); BOOST_FOREACH(const CTransaction&tx, block.vtx) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 81ac3872e..dd00b74c5 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -696,7 +696,7 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) result.push_back(Pair("capabilities", aCaps)); result.push_back(Pair("version", pblock->nVersion)); result.push_back(Pair("previousblockhash", pblock->hashPrevBlock.GetHex())); - result.push_back(Pair("finalsaplingroothash", pblock->hashFinalSaplingRoot.GetHex())); + result.push_back(Pair("finalsaplingroothash", pblock->hashLightClientRoot.GetHex())); result.push_back(Pair("transactions", transactions)); if (coinbasetxn) { assert(txCoinbase.isObject()); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 1acbcad16..7dc9773e2 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -273,8 +273,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) } */ - // These tests assume null hashFinalSaplingRoot (before Sapling) - pblock->hashFinalSaplingRoot = uint256(); + // These tests assume null hashLightClientRoot (before Sapling) + pblock->hashLightClientRoot = uint256(); CValidationState state; BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL)); diff --git a/src/txdb.cpp b/src/txdb.cpp index aa5749244..3a11ba123 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -519,7 +519,9 @@ bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) { return true; } -bool CBlockTreeDB::LoadBlockIndexGuts(std::function insertBlockIndex) +bool CBlockTreeDB::LoadBlockIndexGuts( + std::function insertBlockIndex, + const CChainParams& chainParams) { boost::scoped_ptr pcursor(NewIterator()); @@ -542,7 +544,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(std::functionhashSproutAnchor = diskindex.hashSproutAnchor; pindexNew->nVersion = diskindex.nVersion; pindexNew->hashMerkleRoot = diskindex.hashMerkleRoot; - pindexNew->hashFinalSaplingRoot = diskindex.hashFinalSaplingRoot; + pindexNew->hashLightClientRoot = diskindex.hashLightClientRoot; pindexNew->nTime = diskindex.nTime; pindexNew->nBits = diskindex.nBits; pindexNew->nNonce = diskindex.nNonce; @@ -552,6 +554,8 @@ bool CBlockTreeDB::LoadBlockIndexGuts(std::functionnTx = diskindex.nTx; pindexNew->nSproutValue = diskindex.nSproutValue; pindexNew->nSaplingValue = diskindex.nSaplingValue; + pindexNew->hashFinalSaplingRoot = diskindex.hashFinalSaplingRoot; + pindexNew->hashChainHistoryRoot = diskindex.hashChainHistoryRoot; // Consistency checks auto header = pindexNew->GetBlockHeader(); @@ -561,6 +565,21 @@ bool CBlockTreeDB::LoadBlockIndexGuts(std::functionGetBlockHash(), pindexNew->nBits, Params().GetConsensus())) return error("LoadBlockIndex(): CheckProofOfWork failed: %s", pindexNew->ToString()); + // ZIP 221 consistency checks + if (chainParams.GetConsensus().NetworkUpgradeActive(pindexNew->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + if (pindexNew->hashLightClientRoot != pindexNew->hashChainHistoryRoot) { + return error( + "LoadBlockIndex(): block index inconsistency detected (hashLightClientRoot != hashChainHistoryRoot): %s", + pindexNew->ToString()); + } + } else { + if (pindexNew->hashLightClientRoot != pindexNew->hashFinalSaplingRoot) { + return error( + "LoadBlockIndex(): block index inconsistency detected (hashLightClientRoot != hashFinalSaplingRoot): %s", + pindexNew->ToString()); + } + } + pcursor->Next(); } else { return error("LoadBlockIndex() : failed to read value"); diff --git a/src/txdb.h b/src/txdb.h index cc1d576cb..1cd7cc8e9 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -139,7 +139,9 @@ public: bool WriteFlag(const std::string &name, bool fValue); bool ReadFlag(const std::string &name, bool &fValue); - bool LoadBlockIndexGuts(std::function insertBlockIndex); + bool LoadBlockIndexGuts( + std::function insertBlockIndex, + const CChainParams& chainParams); }; #endif // BITCOIN_TXDB_H From 483d35e37cb151428329a10d332160e419998a83 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 3 Apr 2020 00:29:21 +1300 Subject: [PATCH 05/17] Add ZIP 221 support to miner and getblocktemplate The "finalsaplingroothash" field of the getblocktemplate output is no longer guaranteed to match the actual Sapling commitment tree root, and has been deprecated. Users should migrate to "lightclientroothash". --- src/miner.cpp | 8 +++++++- src/rpc/mining.cpp | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 2debf8954..9134024f6 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -520,7 +520,13 @@ CBlockTemplate* CreateNewBlock(const CChainParams& chainparams, const MinerAddre // Fill in header pblock->hashPrevBlock = pindexPrev->GetBlockHash(); - pblock->hashLightClientRoot = sapling_tree.root(); + if (IsActivationHeight(nHeight, chainparams.GetConsensus(), Consensus::UPGRADE_HEARTWOOD)) { + pblock->hashLightClientRoot.SetNull(); + } else if (chainparams.GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_HEARTWOOD)) { + pblock->hashLightClientRoot = view.GetHistoryRoot(consensusBranchId); + } else { + pblock->hashLightClientRoot = sapling_tree.root(); + } UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus()); pblock->nSolution.clear(); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index dd00b74c5..0891b686c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -431,7 +431,8 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) "{\n" " \"version\" : n, (numeric) The block version\n" " \"previousblockhash\" : \"xxxx\", (string) The hash of current highest block\n" - " \"finalsaplingroothash\" : \"xxxx\", (string) The hash of the final sapling root\n" + " \"lightclientroothash\" : \"xxxx\", (string) The hash of the light client root field in the block header\n" + " \"finalsaplingroothash\" : \"xxxx\", (string) (DEPRECATED) The hash of the light client root field in the block header\n" " \"transactions\" : [ (array) contents of non-coinbase transactions that should be included in the next block\n" " {\n" " \"data\" : \"xxxx\", (string) transaction data encoded in hexadecimal (byte-for-byte)\n" @@ -696,6 +697,8 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) result.push_back(Pair("capabilities", aCaps)); result.push_back(Pair("version", pblock->nVersion)); result.push_back(Pair("previousblockhash", pblock->hashPrevBlock.GetHex())); + result.push_back(Pair("lightclientroothash", pblock->hashLightClientRoot.GetHex())); + // Deprecated; remove in a future release. result.push_back(Pair("finalsaplingroothash", pblock->hashLightClientRoot.GetHex())); result.push_back(Pair("transactions", transactions)); if (coinbasetxn) { From b5c7c4a22f163e60bd7c715469e99936518e6671 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 3 Apr 2020 00:33:44 +1300 Subject: [PATCH 06/17] Implement ZIP 221 consensus rules --- src/main.cpp | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 44d0b5a02..4e07373ce 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2812,13 +2812,35 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } blockundo.old_sprout_tree_root = old_sprout_tree_root; - // If Sapling is active, block.hashLightClientRoot must be the - // same as the root of the Sapling tree - if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_SAPLING)) { + if (IsActivationHeight(pindex->nHeight, chainparams.GetConsensus(), Consensus::UPGRADE_HEARTWOOD)) { + // In the block that activates ZIP 221, block.hashLightClientRoot MUST + // be set to all zero bytes. + if (!block.hashLightClientRoot.IsNull()) { + return state.DoS(100, + error("ConnectBlock(): block's hashLightClientRoot is incorrect (should be null)"), + REJECT_INVALID, "bad-heartwood-root-in-block"); + } + } else if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + // If Heartwood is active, block.hashLightClientRoot must be the same as + // the root of the history tree for the previous block. We only store + // one tree per epoch, so we have two possible cases: + // - If the previous block is in the previous epoch, this block won't + // affect that epoch's tree root. + // - If the previous block is in this epoch, this block would affect + // this epoch's tree root, but as we haven't updated the tree for this + // block yet, view.GetHistoryRoot() returns the root we need. + if (block.hashLightClientRoot != view.GetHistoryRoot(prevConsensusBranchId)) { + return state.DoS(100, + error("ConnectBlock(): block's hashLightClientRoot is incorrect (should be history tree root)"), + REJECT_INVALID, "bad-heartwood-root-in-block"); + } + } else if (chainparams.GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_SAPLING)) { + // If Sapling is active, block.hashLightClientRoot must be the + // same as the root of the Sapling tree if (block.hashLightClientRoot != sapling_tree.root()) { return state.DoS(100, - error("ConnectBlock(): block's hashLightClientRoot is incorrect"), - REJECT_INVALID, "bad-sapling-root-in-block"); + error("ConnectBlock(): block's hashLightClientRoot is incorrect (should be Sapling tree root)"), + REJECT_INVALID, "bad-sapling-root-in-block"); } } From 8a658dfd33de0b98b52bd8942e2362ba9d409bfd Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 3 Apr 2020 12:27:33 +1300 Subject: [PATCH 07/17] Return the correct root from librustzcash_mmr_{append, delete} Per https://zips.z.cash/zip-0221#tree-node-specification : Once the MMR has been generated, we produce hashChainHistoryRoot, which we define as the BLAKE2b-256 digest of the serialization of the root node. --- src/rust/src/rustzcash.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rust/src/rustzcash.rs b/src/rust/src/rustzcash.rs index c60b1d55a..8ef52a751 100644 --- a/src/rust/src/rustzcash.rs +++ b/src/rust/src/rustzcash.rs @@ -1257,7 +1257,7 @@ pub extern "system" fn librustzcash_mmr_append( .root_node() .expect("Just added, should resolve always; qed"); unsafe { - *rt_ret = root_node.data().subtree_commitment; + *rt_ret = root_node.data().hash(); for (idx, next_buf) in slice::from_raw_parts_mut(buf_ret, return_count as usize) .iter_mut() @@ -1310,7 +1310,7 @@ pub extern "system" fn librustzcash_mmr_delete( .root_node() .expect("Just generated without errors, root should be resolving") .data() - .subtree_commitment; + .hash(); } truncate_len From 82fe37d22b9ee62aa6b25c2a1cfe3af3df059709 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 3 Apr 2020 15:46:23 +1300 Subject: [PATCH 08/17] Use a C array for HistoryEntry instead of std::array std::vector is guaranteed to store T contiguously. However, there is no guarantee that sizeof(std::array) == N, which prevents us from interpreting std::vector> as &[[u8; N]] on the Rust side of the FFI. Instead, we define HistoryEntry as a struct wrapping a C array, which (as checked by static_assert) contains no padding. --- src/coins.cpp | 4 ++-- src/rust/include/librustzcash.h | 13 +++++++++++-- src/zcash/History.cpp | 6 ++++-- src/zcash/History.hpp | 3 --- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 76262064d..2b50939f0 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -455,7 +455,7 @@ void CCoinsViewCache::PushHistoryNode(uint32_t epochId, const HistoryNode node) epochId, historyCache.length, entry_indices.data(), - entries.data()->data(), + entries.data(), entry_indices.size(), node.data(), newRoot.begin(), @@ -512,7 +512,7 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { epochId, historyCache.length, entry_indices.data(), - entries.data()->data(), + entries.data(), peak_count, entries.size() - peak_count, newRoot.begin() diff --git a/src/rust/include/librustzcash.h b/src/rust/include/librustzcash.h index 1aa85ed20..239093dc3 100644 --- a/src/rust/include/librustzcash.h +++ b/src/rust/include/librustzcash.h @@ -3,6 +3,15 @@ #include +const int ENTRY_SERIALIZED_LENGTH = 180; +typedef struct HistoryEntry { + unsigned char bytes[ENTRY_SERIALIZED_LENGTH]; +} HistoryEntry; +static_assert( + sizeof(HistoryEntry) == ENTRY_SERIALIZED_LENGTH, + "HistoryEntry struct is not the same size as the underlying byte array"); +static_assert(alignof(HistoryEntry) == 1, "HistoryEntry struct alignment is not 1"); + extern "C" { #ifdef WIN32 typedef uint16_t codeunit; @@ -312,7 +321,7 @@ extern "C" { uint32_t cbranch, uint32_t t_len, const uint32_t *ni_ptr, - const unsigned char *n_ptr, + const HistoryEntry *n_ptr, size_t p_len, const unsigned char *nn_ptr, unsigned char *rt_ret, @@ -323,7 +332,7 @@ extern "C" { uint32_t cbranch, uint32_t t_len, const uint32_t *ni_ptr, - const unsigned char *n_ptr, + const HistoryEntry *n_ptr, size_t p_len, size_t e_len, unsigned char *rt_ret diff --git a/src/zcash/History.cpp b/src/zcash/History.cpp index 92f95c686..c53d46671 100644 --- a/src/zcash/History.cpp +++ b/src/zcash/History.cpp @@ -99,7 +99,8 @@ HistoryEntry NodeToEntry(const HistoryNode node, uint32_t left, uint32_t right) buf << right; buf << node; - std::copy(std::begin(buf), std::end(buf), std::begin(result)); + assert(buf.size() <= ENTRY_SERIALIZED_LENGTH); + std::copy(std::begin(buf), std::end(buf), result.bytes); return result; } @@ -112,7 +113,8 @@ HistoryEntry LeafToEntry(const HistoryNode node) { buf << code; buf << node; - std::copy(std::begin(buf), std::end(buf), std::begin(result)); + assert(buf.size() <= ENTRY_SERIALIZED_LENGTH); + std::copy(std::begin(buf), std::end(buf), result.bytes); return result; } diff --git a/src/zcash/History.hpp b/src/zcash/History.hpp index 19a0d4329..3e25bd3e8 100644 --- a/src/zcash/History.hpp +++ b/src/zcash/History.hpp @@ -14,10 +14,8 @@ namespace libzcash { const int NODE_SERIALIZED_LENGTH = 171; -const int ENTRY_SERIALIZED_LENGTH = 180; typedef std::array HistoryNode; -typedef std::array HistoryEntry; typedef uint64_t HistoryIndex; @@ -69,6 +67,5 @@ HistoryEntry LeafToEntry(const HistoryNode node); typedef libzcash::HistoryCache HistoryCache; typedef libzcash::HistoryIndex HistoryIndex; typedef libzcash::HistoryNode HistoryNode; -typedef libzcash::HistoryEntry HistoryEntry; #endif /* ZC_HISTORY_H_ */ \ No newline at end of file From cb57c17eb6212624704443cc918bcb173642a061 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 3 Apr 2020 23:10:38 +1300 Subject: [PATCH 09/17] test: Verify ZIP 221 logic against reference implementation --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/feature_zip221.py | 133 +++++++++++++++++ qa/rpc-tests/test_framework/flyclient.py | 177 +++++++++++++++++++++++ qa/rpc-tests/test_framework/mininode.py | 15 ++ 4 files changed, 326 insertions(+) create mode 100755 qa/rpc-tests/feature_zip221.py create mode 100644 qa/rpc-tests/test_framework/flyclient.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 6fb3c0f3c..66115b51d 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -83,6 +83,7 @@ testScripts=( 'turnstile.py' 'mining_shielded_coinbase.py' 'framework.py' + 'feature_zip221.py' ); testScriptsExt=( 'getblocktemplate_longpoll.py' diff --git a/qa/rpc-tests/feature_zip221.py b/qa/rpc-tests/feature_zip221.py new file mode 100755 index 000000000..f6a9d65e8 --- /dev/null +++ b/qa/rpc-tests/feature_zip221.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php . + + +from test_framework.flyclient import (ZcashMMRNode, append, delete, make_root_commitment) +from test_framework.mininode import (HEARTWOOD_BRANCH_ID, CBlockHeader) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + bytes_to_hex_str, + hex_str_to_bytes, + initialize_chain_clean, + start_nodes, +) + +from io import BytesIO + +NULL_FIELD = "0000000000000000000000000000000000000000000000000000000000000000" +CHAIN_HISTORY_ROOT_VERSION = 2010200 + +# Verify block header field 'hashLightClientRoot' is set correctly for Heartwood blocks. +class Zip221Test(BitcoinTestFramework): + + def setup_chain(self): + print("Initializing test directory "+self.options.tmpdir) + initialize_chain_clean(self.options.tmpdir, 4) + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, extra_args=[[ + '-nuparams=2bb40e60:1', # Blossom + '-nuparams=f5b9230b:10', # Heartwood + '-nurejectoldversions=false', + ]] * 4) + + def node_for_block(self, height): + block_header = CBlockHeader() + block_header.deserialize(BytesIO(hex_str_to_bytes( + self.nodes[0].getblock(str(height), 0)))) + sapling_root = hex_str_to_bytes( + self.nodes[0].getblock(str(height))["finalsaplingroot"])[::-1] + return ZcashMMRNode.from_block( + block_header, height, sapling_root, 0, HEARTWOOD_BRANCH_ID) + + def run_test(self): + self.nodes[0].generate(10) + self.sync_all() + + # Verify all blocks up to and including Heartwood activation set + # hashChainHistoryRoot to null. + print("Verifying blocks up to and including Heartwood activation") + blockcount = self.nodes[0].getblockcount() + for height in range(0, blockcount + 1): + blk = self.nodes[0].getblock(str(height)) + assert_equal(blk["chainhistoryroot"], NULL_FIELD) + + + # Create the initial history tree, containing a single node. + root = self.node_for_block(10) + + # Generate the first block that contains a non-null chain history root. + print("Verifying first non-null chain history root") + self.nodes[0].generate(1) + self.sync_all() + + # Verify that hashChainHistoryRoot is set correctly. + assert_equal( + self.nodes[0].getblock('11')["chainhistoryroot"], + bytes_to_hex_str(make_root_commitment(root)[::-1])) + + # Generate 9 more blocks on node 0, and verify their chain history roots. + print("Mining 9 blocks on node 0") + self.nodes[0].generate(9) + self.sync_all() + + print("Verifying node 0's chain history") + for height in range(12, 21): + leaf = self.node_for_block(height - 1) + root = append(root, leaf) + assert_equal( + self.nodes[0].getblock(str(height))["chainhistoryroot"], + bytes_to_hex_str(make_root_commitment(root)[::-1])) + + # The rest of the test only applies to Heartwood-aware node versions. + # Earlier versions won't serialize chain history roots in the block + # index, and splitting the network below requires restarting the nodes. + if self.nodes[0].getnetworkinfo()["version"] < CHAIN_HISTORY_ROOT_VERSION: + print("Node's block index is not Heartwood-aware, skipping reorg test") + return + + # Split the network so we can test the effect of a reorg. + print("Splitting the network") + self.split_network() + + # Generate 10 more blocks on node 0, and verify their chain history roots. + print("Mining 10 more blocks on node 0") + self.nodes[0].generate(10) + self.sync_all() + + print("Verifying node 0's chain history") + for height in range(21, 31): + leaf = self.node_for_block(height - 1) + root = append(root, leaf) + assert_equal( + self.nodes[0].getblock(str(height))["chainhistoryroot"], + bytes_to_hex_str(make_root_commitment(root)[::-1])) + + # Generate 11 blocks on node 2. + print("Mining alternate chain on node 2") + self.nodes[2].generate(11) + self.sync_all() + + # Reconnect the nodes; node 0 will re-org to node 2's chain. + print("Re-joining the network so that node 0 reorgs") + self.join_network() + + # Verify that node 0's chain history was correctly updated. + print("Deleting orphaned blocks from the expected chain history") + for _ in range(10): + root = delete(root) + + print("Verifying that node 0 is now on node 1's chain history") + for height in range(21, 32): + leaf = self.node_for_block(height - 1) + root = append(root, leaf) + assert_equal( + self.nodes[2].getblock(str(height))["chainhistoryroot"], + bytes_to_hex_str(make_root_commitment(root)[::-1])) + + +if __name__ == '__main__': + Zip221Test().main() diff --git a/qa/rpc-tests/test_framework/flyclient.py b/qa/rpc-tests/test_framework/flyclient.py new file mode 100644 index 000000000..39cfa4627 --- /dev/null +++ b/qa/rpc-tests/test_framework/flyclient.py @@ -0,0 +1,177 @@ +from pyblake2 import blake2b +import struct +from typing import (List, Optional) + +from .mininode import (CBlockHeader, block_work_from_compact, ser_compactsize, ser_uint256) + +def H(msg: bytes, consensusBranchId: int) -> bytes: + digest = blake2b( + digest_size=32, + person=b'ZcashHistory' + struct.pack(" 'ZcashMMRNode': + '''Create a leaf node from a block''' + node = Z() + node.left_child = None + node.right_child = None + node.hashSubtreeCommitment = ser_uint256(block.rehash()) + node.nEarliestTimestamp = block.nTime + node.nLatestTimestamp = block.nTime + node.nEarliestTargetBits = block.nBits + node.nLatestTargetBits = block.nBits + node.hashEarliestSaplingRoot = sapling_root + node.hashLatestSaplingRoot = sapling_root + node.nSubTreeTotalWork = block_work_from_compact(block.nBits) + node.nEarliestHeight = height + node.nLatestHeight = height + node.nSaplingTxCount = sapling_tx_count + node.consensusBranchId = consensusBranchId + return node + + def serialize(self) -> bytes: + '''serializes a node''' + buf = b'' + buf += self.hashSubtreeCommitment + buf += struct.pack(" ZcashMMRNode: + parent = ZcashMMRNode() + parent.left_child = left_child + parent.right_child = right_child + parent.hashSubtreeCommitment = H( + left_child.serialize() + right_child.serialize(), + left_child.consensusBranchId, + ) + parent.nEarliestTimestamp = left_child.nEarliestTimestamp + parent.nLatestTimestamp = right_child.nLatestTimestamp + parent.nEarliestTargetBits = left_child.nEarliestTargetBits + parent.nLatestTargetBits = right_child.nLatestTargetBits + parent.hashEarliestSaplingRoot = left_child.hashEarliestSaplingRoot + parent.hashLatestSaplingRoot = right_child.hashLatestSaplingRoot + parent.nSubTreeTotalWork = left_child.nSubTreeTotalWork + right_child.nSubTreeTotalWork + parent.nEarliestHeight = left_child.nEarliestHeight + parent.nLatestHeight = right_child.nLatestHeight + parent.nSaplingTxCount = left_child.nSaplingTxCount + right_child.nSaplingTxCount + parent.consensusBranchId = left_child.consensusBranchId + return parent + +def make_root_commitment(root: ZcashMMRNode) -> bytes: + '''Makes the root commitment for a blockheader''' + return H(root.serialize(), root.consensusBranchId) + +def get_peaks(node: ZcashMMRNode) -> List[ZcashMMRNode]: + peaks: List[ZcashMMRNode] = [] + + # Get number of leaves. + leaves = node.nLatestHeight - (node.nEarliestHeight - 1) + assert(leaves > 0) + + # Check if the number of leaves is a power of two. + if (leaves & (leaves - 1)) == 0: + # Tree is full, hence a single peak. This also covers the + # case of a single isolated leaf. + peaks.append(node) + else: + # If the number of leaves is not a power of two, then this + # node must be internal, and cannot be a peak. + peaks.extend(get_peaks(node.left_child)) + peaks.extend(get_peaks(node.right_child)) + + return peaks + + +def bag_peaks(peaks: List[ZcashMMRNode]) -> ZcashMMRNode: + ''' + "Bag" a list of peaks, and return the final root + ''' + root = peaks[0] + for i in range(1, len(peaks)): + root = make_parent(root, peaks[i]) + return root + + +def append(root: ZcashMMRNode, leaf: ZcashMMRNode) -> ZcashMMRNode: + '''Append a leaf to an existing tree, return the new tree root''' + # recursively find a list of peaks in the current tree + peaks: List[ZcashMMRNode] = get_peaks(root) + merged: List[ZcashMMRNode] = [] + + # Merge peaks from right to left. + # This will produce a list of peaks in reverse order + current = leaf + for peak in peaks[::-1]: + current_leaves = current.nLatestHeight - (current.nEarliestHeight - 1) + peak_leaves = peak.nLatestHeight - (peak.nEarliestHeight - 1) + + if current_leaves == peak_leaves: + current = make_parent(peak, current) + else: + merged.append(current) + current = peak + merged.append(current) + + # finally, bag the merged peaks + return bag_peaks(merged[::-1]) + +def delete(root: ZcashMMRNode) -> ZcashMMRNode: + ''' + Delete the rightmost leaf node from an existing MMR + Return the new tree root + ''' + + n_leaves = root.nLatestHeight - (root.nEarliestHeight - 1) + # if there were an odd number of leaves, + # simply replace root with left_child + if n_leaves & 1: + return root.left_child + + # otherwise, we need to re-bag the peaks. + else: + # first peak + peaks = [root.left_child] + + # we do this traversing the right (unbalanced) side of the tree + # we keep the left side (balanced subtree or leaf) of each subtree + # until we reach a leaf + subtree_root = root.right_child + while subtree_root.left_child: + peaks.append(subtree_root.left_child) + subtree_root = subtree_root.right_child + + new_root = bag_peaks(peaks) + return new_root diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 1e86e8bc6..359553def 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -57,6 +57,7 @@ SPROUT_BRANCH_ID = 0x00000000 OVERWINTER_BRANCH_ID = 0x5BA81B19 SAPLING_BRANCH_ID = 0x76B809BB BLOSSOM_BRANCH_ID = 0x2BB40E60 +HEARTWOOD_BRANCH_ID = 0xF5B9230B MAX_INV_SZ = 50000 @@ -85,6 +86,15 @@ def hash256(s): return sha256(sha256(s)) +def ser_compactsize(n): + if n < 253: + return struct.pack("B", n) + elif n < 0x10000: + return struct.pack(" Date: Fri, 10 Apr 2020 09:51:17 +1200 Subject: [PATCH 10/17] Comment tweaks and cleanups Co-Authored-By: Daira Hopwood --- qa/rpc-tests/feature_zip221.py | 2 +- src/chain.h | 11 ++++++----- src/coins.cpp | 6 +++++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/feature_zip221.py b/qa/rpc-tests/feature_zip221.py index f6a9d65e8..ba60ef25b 100755 --- a/qa/rpc-tests/feature_zip221.py +++ b/qa/rpc-tests/feature_zip221.py @@ -17,7 +17,7 @@ from test_framework.util import ( from io import BytesIO -NULL_FIELD = "0000000000000000000000000000000000000000000000000000000000000000" +NULL_FIELD = "00" * 32 CHAIN_HISTORY_ROOT_VERSION = 2010200 # Verify block header field 'hashLightClientRoot' is set correctly for Heartwood blocks. diff --git a/src/chain.h b/src/chain.h index 71a5b2c42..7ec5623c6 100644 --- a/src/chain.h +++ b/src/chain.h @@ -257,16 +257,17 @@ public: //! Root of the Sapling commitment tree as of the end of this block. This is only set //! once a block has been connected to the main chain, and will be null otherwise. //! - //! For blocks prior to Heartwood activation, this is always equal to - //! hashLightClientRoot. + //! For blocks prior to (not including) the Heartwood activation block, this is + //! always equal to hashLightClientRoot. uint256 hashFinalSaplingRoot; //! Root of the ZIP 221 history tree as of the end of the previous block. This is only //! set once a block has been connected to the main chain, and will be null otherwise. //! - //! - For blocks prior to and including Heartwood activation, this is always null. - //! - For blocks after Heartwood activation, this is always equal to - //! hashLightClientRoot. + //! - For blocks prior to and including the Heartwood activation block, this is + //! always null. + //! - For blocks after (not including) the Heartwood activation block, this is + //! always equal to hashLightClientRoot. uint256 hashChainHistoryRoot; //! block header diff --git a/src/coins.cpp b/src/coins.cpp index 2b50939f0..78e539f13 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -86,7 +86,11 @@ bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, CAnchorsSaplingMap &mapSaplingAnchors, CNullifiersMap &mapSproutNullifiers, CNullifiersMap &mapSaplingNullifiers, - CHistoryCacheMap &historyCacheMap) { return base->BatchWrite(mapCoins, hashBlock, hashSproutAnchor, hashSaplingAnchor, mapSproutAnchors, mapSaplingAnchors, mapSproutNullifiers, mapSaplingNullifiers, historyCacheMap); } + CHistoryCacheMap &historyCacheMap) { + return base->BatchWrite(mapCoins, hashBlock, hashSproutAnchor, hashSaplingAnchor, + mapSproutAnchors, mapSaplingAnchors, mapSproutNullifiers, mapSaplingNullifiers, + historyCacheMap); +} bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStats(stats); } CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} From d47676fe00d19959509574d550d93a06fb225ceb Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 10 Apr 2020 10:20:21 +1200 Subject: [PATCH 11/17] Refer to altitude instead of height for history tree peaks --- src/coins.cpp | 54 ++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 78e539f13..08ac23126 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -302,32 +302,34 @@ void CCoinsViewCache::BringBestAnchorIntoCache( void draftMMRNode(std::vector &indices, std::vector &entries, HistoryNode nodeData, - uint32_t h, + uint32_t alt, uint32_t peak_pos) { - HistoryEntry newEntry = h == 0 + HistoryEntry newEntry = alt == 0 ? libzcash::LeafToEntry(nodeData) - // peak_pos - (1 << h) is the mmr position of left child, -1 to that is this position of entry in + // peak_pos - (1 << alt) is the mmr position of left child, -1 to that is this position of entry in // array representation. // // peak_pos - 1 is the mmr position of right child, -1 to that is this position of entry in // array representation - : libzcash::NodeToEntry(nodeData, peak_pos - (1 << h) - 1, peak_pos - 2); + : libzcash::NodeToEntry(nodeData, peak_pos - (1 << alt) - 1, peak_pos - 2); indices.push_back(peak_pos - 1); entries.push_back(newEntry); } -static inline uint32_t log2i(uint32_t x) { +// Computes floor(log2(x)). +static inline uint32_t floor_log2(uint32_t x) { assert(x > 0); int log = 0; - while (x >>= 1) ++log; + while (x >>= 1) { ++log; } return log; } -// Computes floor(log2(x+1)) -static inline uint32_t floor_log2i(uint32_t x) { - return log2i(x + 1) - 1; +// Computes the altitude of the largest subtree for an MMR with n nodes, +// which is floor(log2(n + 1)) - 1. +static inline uint32_t altitude(uint32_t n) { + return floor_log2(n + 1) - 1; } uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std::vector &entries, std::vector &entry_indices) { @@ -338,8 +340,8 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: } uint32_t last_peak_pos = 0; - uint32_t last_peak_h = 0; - uint32_t h = 0; + uint32_t last_peak_alt = 0; + uint32_t alt = 0; uint32_t peak_pos = 0; uint32_t total_peaks = 0; @@ -349,30 +351,30 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: return 1; } else { // First possible peak is calculated above. - h = floor_log2i(treeLength); - peak_pos = (1 << (h + 1)) - 1; + alt = altitude(treeLength); + peak_pos = (1 << (alt + 1)) - 1; // Collecting all peaks starting from first possible one. - while (h != 0) { + while (alt != 0) { // If peak_pos is out of bounds of the tree, left child of it calculated, // and that means that we drop down one level in the tree. if (peak_pos > treeLength) { - // left child, -2^h - peak_pos = peak_pos - (1 << h); - h = h - 1; + // left child, -2^alt + peak_pos = peak_pos - (1 << alt); + alt = alt - 1; } // If the peak exists, we take it and then continue with its right sibling // (which may not exist and that will be covered in next iteration). if (peak_pos <= treeLength) { - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), h, peak_pos); + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), alt, peak_pos); last_peak_pos = peak_pos; - last_peak_h = h; + last_peak_alt = alt; // right sibling - peak_pos = peak_pos + (1 << (h + 1)) - 1; + peak_pos = peak_pos + (1 << (alt + 1)) - 1; } } } @@ -382,7 +384,7 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: // early return if we don't extra nodes if (!extra) return total_peaks; - h = last_peak_h; + alt = last_peak_alt; peak_pos = last_peak_pos; @@ -400,16 +402,16 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: // // For extra peaks needed for deletion, we do extra pass on right slope of the last peak // and add those nodes + their siblings. Extra would be (D, E) for the picture above. - while (h > 0) { - uint32_t left_pos = peak_pos - (1< 0) { + uint32_t left_pos = peak_pos - (1 << alt); uint32_t right_pos = peak_pos - 1; - h = h - 1; + alt = alt - 1; // drafting left child - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, left_pos-1), h, left_pos); + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, left_pos-1), alt, left_pos); // drafting right child - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, right_pos-1), h, right_pos); + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, right_pos-1), alt, right_pos); // continuing on right slope peak_pos = right_pos; From 9cfd574eac5dd847f6fdfb26c2daec724ca1f7f0 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 10 Apr 2020 11:06:49 +1200 Subject: [PATCH 12/17] test: Add an extra assertion to feature_zip221.py --- qa/rpc-tests/feature_zip221.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qa/rpc-tests/feature_zip221.py b/qa/rpc-tests/feature_zip221.py index ba60ef25b..e9fec9db9 100755 --- a/qa/rpc-tests/feature_zip221.py +++ b/qa/rpc-tests/feature_zip221.py @@ -51,6 +51,7 @@ class Zip221Test(BitcoinTestFramework): # hashChainHistoryRoot to null. print("Verifying blocks up to and including Heartwood activation") blockcount = self.nodes[0].getblockcount() + assert_equal(blockcount, 10) for height in range(0, blockcount + 1): blk = self.nodes[0].getblock(str(height)) assert_equal(blk["chainhistoryroot"], NULL_FIELD) From 65073157e6156a37d085de3395609763fb4e5dd5 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 10 Apr 2020 17:52:10 +1200 Subject: [PATCH 13/17] Remove unnecessary else case in CCoinsViewCache::PreloadHistoryTree --- src/coins.cpp | 50 ++++++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 08ac23126..415b57f9c 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -337,6 +337,10 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: if (treeLength <= 0) { throw std::runtime_error("Invalid PreloadHistoryTree state called - tree should exist"); + } else if (treeLength == 1) { + entries.push_back(libzcash::LeafToEntry(GetHistoryAt(epochId, 0))); + entry_indices.push_back(0); + return 1; } uint32_t last_peak_pos = 0; @@ -345,37 +349,31 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: uint32_t peak_pos = 0; uint32_t total_peaks = 0; - if (treeLength == 1) { - entries.push_back(libzcash::LeafToEntry(GetHistoryAt(epochId, 0))); - entry_indices.push_back(0); - return 1; - } else { - // First possible peak is calculated above. - alt = altitude(treeLength); - peak_pos = (1 << (alt + 1)) - 1; + // First possible peak is calculated above. + alt = altitude(treeLength); + peak_pos = (1 << (alt + 1)) - 1; - // Collecting all peaks starting from first possible one. - while (alt != 0) { + // Collecting all peaks starting from first possible one. + while (alt != 0) { - // If peak_pos is out of bounds of the tree, left child of it calculated, - // and that means that we drop down one level in the tree. - if (peak_pos > treeLength) { - // left child, -2^alt - peak_pos = peak_pos - (1 << alt); - alt = alt - 1; - } + // If peak_pos is out of bounds of the tree, left child of it calculated, + // and that means that we drop down one level in the tree. + if (peak_pos > treeLength) { + // left child, -2^alt + peak_pos = peak_pos - (1 << alt); + alt = alt - 1; + } - // If the peak exists, we take it and then continue with its right sibling - // (which may not exist and that will be covered in next iteration). - if (peak_pos <= treeLength) { - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), alt, peak_pos); + // If the peak exists, we take it and then continue with its right sibling + // (which may not exist and that will be covered in next iteration). + if (peak_pos <= treeLength) { + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), alt, peak_pos); - last_peak_pos = peak_pos; - last_peak_alt = alt; + last_peak_pos = peak_pos; + last_peak_alt = alt; - // right sibling - peak_pos = peak_pos + (1 << (alt + 1)) - 1; - } + // right sibling + peak_pos = peak_pos + (1 << (alt + 1)) - 1; } } From 15ef73e58665424f9cfc2feacfa1dea7df069a84 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 10 Apr 2020 18:17:14 +1200 Subject: [PATCH 14/17] Improve documentation of CCoinsViewCache::PreloadHistoryTree --- src/coins.cpp | 60 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 415b57f9c..e08454993 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -349,23 +349,54 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: uint32_t peak_pos = 0; uint32_t total_peaks = 0; - // First possible peak is calculated above. + // Assume the following example peak layout with 14 leaves, and 25 nodes in total: + // + // P + // /\ + // / \ + // / \ \ + // / \ \ Altitude + // _A_ \ \ 3 + // _/ \_ B \ 2 + // / \ / \ / \ C 1 + // /\ /\ /\ /\ /\ /\ /\ 0 + // + // We start by determining the altitude of the highest peak (A). alt = altitude(treeLength); + + // We determing the position of the highest peak (A) by pretending it is the right + // sibling in a tree, and its left-most leaf has position 1. Then the left sibling + // of (A) has position 0, and so we can "jump" to the peak's position by computing + // 0 + 2^(alt + 1) - 1. This is also why peak_pos is 1-indexed here; we subtract 1 + // when we index into the underlying array structure. peak_pos = (1 << (alt + 1)) - 1; - // Collecting all peaks starting from first possible one. + // Now that we have the position and altitude of the highest peak (A), we collect + // the remaining peaks (B, C). We navigate the peaks as if they were nodes in this + // Merkle tree (with additional imaginary nodes 1 and 2, that have positions beyond + // the MMR's length): + // + // / \ + // / \ + // / \ + // / \ + // A ==========> 1 + // / \ // \ + // _/ \_ B ==> 2 + // /\ /\ /\ // + // / \ / \ / \ C + // /\ /\ /\ /\ /\ /\ /\ + // while (alt != 0) { - - // If peak_pos is out of bounds of the tree, left child of it calculated, - // and that means that we drop down one level in the tree. + // If peak_pos is out of bounds of the tree, we compute the position of its left + // child, and drop down one level in the tree. if (peak_pos > treeLength) { // left child, -2^alt peak_pos = peak_pos - (1 << alt); alt = alt - 1; } - // If the peak exists, we take it and then continue with its right sibling - // (which may not exist and that will be covered in next iteration). + // If the peak exists, we take it and then continue with its right sibling. if (peak_pos <= treeLength) { draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), alt, peak_pos); @@ -386,18 +417,17 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: peak_pos = last_peak_pos; - // P - // / \ - // / \ - // / \ \ - // / \ \ - // _A_ \ \ - // / \_ B \ + // P + // /\ + // / \ + // / \ \ + // / \ \ + // _A_ \ \ + // _/ \_ B \ // / \ / \ / \ C // /\ /\ /\ /\ /\ /\ /\ // D E // - // // For extra peaks needed for deletion, we do extra pass on right slope of the last peak // and add those nodes + their siblings. Extra would be (D, E) for the picture above. while (alt > 0) { From bc30c57cdbf8b3e37b648f938582f02619cbbc8e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 10 Apr 2020 19:56:13 +1200 Subject: [PATCH 15/17] Truncate HistoryCache.appends correctly for zero-indexed entries --- src/zcash/History.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/zcash/History.cpp b/src/zcash/History.cpp index c53d46671..2ffbec2f0 100644 --- a/src/zcash/History.cpp +++ b/src/zcash/History.cpp @@ -16,8 +16,15 @@ void HistoryCache::Extend(const HistoryNode &leaf) { } void HistoryCache::Truncate(HistoryIndex newLength) { - for (HistoryIndex idx = length; idx > newLength; idx--) { - appends.erase(idx); + // Remove any to-be-appended nodes beyond the new length. The array representation is + // zero-indexed, and HistoryIndex is unsigned, so we handle the truncate-to-zero case + // separately. + if (newLength > 0) { + for (HistoryIndex idx = length; idx >= newLength; idx--) { + appends.erase(idx); + } + } else { + appends.clear(); } length = newLength; From 21d8e5be153bc81f8852740efaad58898a8fc179 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Apr 2020 11:38:36 +1200 Subject: [PATCH 16/17] Comment clarifications and fixes --- qa/rpc-tests/test_framework/flyclient.py | 9 ++++----- src/coins.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/test_framework/flyclient.py b/qa/rpc-tests/test_framework/flyclient.py index 39cfa4627..c8bbb1e62 100644 --- a/qa/rpc-tests/test_framework/flyclient.py +++ b/qa/rpc-tests/test_framework/flyclient.py @@ -101,14 +101,13 @@ def get_peaks(node: ZcashMMRNode) -> List[ZcashMMRNode]: leaves = node.nLatestHeight - (node.nEarliestHeight - 1) assert(leaves > 0) - # Check if the number of leaves is a power of two. + # Check if the number of leaves in this subtree is a power of two. if (leaves & (leaves - 1)) == 0: - # Tree is full, hence a single peak. This also covers the - # case of a single isolated leaf. + # This subtree is full, and therefore a single peak. This also covers + # the case of a single isolated leaf. peaks.append(node) else: - # If the number of leaves is not a power of two, then this - # node must be internal, and cannot be a peak. + # This is one of the generated nodes; search within its children. peaks.extend(get_peaks(node.left_child)) peaks.extend(get_peaks(node.right_child)) diff --git a/src/coins.cpp b/src/coins.cpp index e08454993..a1fc3601e 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -349,7 +349,8 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: uint32_t peak_pos = 0; uint32_t total_peaks = 0; - // Assume the following example peak layout with 14 leaves, and 25 nodes in total: + // Assume the following example peak layout with 14 leaves, and 25 stored nodes in + // total (the "tree length"): // // P // /\ @@ -364,7 +365,7 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: // We start by determining the altitude of the highest peak (A). alt = altitude(treeLength); - // We determing the position of the highest peak (A) by pretending it is the right + // We determine the position of the highest peak (A) by pretending it is the right // sibling in a tree, and its left-most leaf has position 1. Then the left sibling // of (A) has position 0, and so we can "jump" to the peak's position by computing // 0 + 2^(alt + 1) - 1. This is also why peak_pos is 1-indexed here; we subtract 1 @@ -410,7 +411,7 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: total_peaks = entries.size(); - // early return if we don't extra nodes + // Return early if we don't require extra nodes. if (!extra) return total_peaks; alt = last_peak_alt; From 31e5f9cde240fbe6c1b4990e5531afa3229cadeb Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 11 Apr 2020 11:52:56 +1200 Subject: [PATCH 17/17] Make peak_pos zero-indexed in CCoinsViewCache::PreloadHistoryTree --- src/coins.cpp | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index a1fc3601e..9672118eb 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -307,14 +307,11 @@ void draftMMRNode(std::vector &indices, { HistoryEntry newEntry = alt == 0 ? libzcash::LeafToEntry(nodeData) - // peak_pos - (1 << alt) is the mmr position of left child, -1 to that is this position of entry in - // array representation. - // - // peak_pos - 1 is the mmr position of right child, -1 to that is this position of entry in - // array representation - : libzcash::NodeToEntry(nodeData, peak_pos - (1 << alt) - 1, peak_pos - 2); + // peak_pos - (1 << alt) is the array position of left child. + // peak_pos - 1 is the array position of right child. + : libzcash::NodeToEntry(nodeData, peak_pos - (1 << alt), peak_pos - 1); - indices.push_back(peak_pos - 1); + indices.push_back(peak_pos); entries.push_back(newEntry); } @@ -366,11 +363,10 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: alt = altitude(treeLength); // We determine the position of the highest peak (A) by pretending it is the right - // sibling in a tree, and its left-most leaf has position 1. Then the left sibling - // of (A) has position 0, and so we can "jump" to the peak's position by computing - // 0 + 2^(alt + 1) - 1. This is also why peak_pos is 1-indexed here; we subtract 1 - // when we index into the underlying array structure. - peak_pos = (1 << (alt + 1)) - 1; + // sibling in a tree, and its left-most leaf has position 0. Then the left sibling + // of (A) has position -1, and so we can "jump" to the peak's position by computing + // -1 + 2^(alt + 1) - 1. + peak_pos = (1 << (alt + 1)) - 2; // Now that we have the position and altitude of the highest peak (A), we collect // the remaining peaks (B, C). We navigate the peaks as if they were nodes in this @@ -391,15 +387,15 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: while (alt != 0) { // If peak_pos is out of bounds of the tree, we compute the position of its left // child, and drop down one level in the tree. - if (peak_pos > treeLength) { + if (peak_pos >= treeLength) { // left child, -2^alt peak_pos = peak_pos - (1 << alt); alt = alt - 1; } // If the peak exists, we take it and then continue with its right sibling. - if (peak_pos <= treeLength) { - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos-1), alt, peak_pos); + if (peak_pos < treeLength) { + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, peak_pos), alt, peak_pos); last_peak_pos = peak_pos; last_peak_alt = alt; @@ -437,10 +433,10 @@ uint32_t CCoinsViewCache::PreloadHistoryTree(uint32_t epochId, bool extra, std:: alt = alt - 1; // drafting left child - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, left_pos-1), alt, left_pos); + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, left_pos), alt, left_pos); // drafting right child - draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, right_pos-1), alt, right_pos); + draftMMRNode(entry_indices, entries, GetHistoryAt(epochId, right_pos), alt, right_pos); // continuing on right slope peak_pos = right_pos;