From 13aa74aa453a799c424918ca9ca6d2311f486ff3 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Tue, 21 Jul 2020 09:45:32 +0800 Subject: [PATCH 1/2] Pass HistoryNode struct to librustzcash FFI --- src/coins.cpp | 19 +++++++++++++++---- src/rust/include/librustzcash.h | 17 +++++++++++++---- src/txdb.cpp | 12 ++++++++++-- src/zcash/History.cpp | 15 ++++++++++++--- src/zcash/History.hpp | 5 ----- 5 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 365b984fd..4e9ee2d9d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -467,7 +467,7 @@ void CCoinsViewCache::PushHistoryNode(uint32_t epochId, const HistoryNode node) // 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) { + if (librustzcash_mmr_hash_node(epochId, &node, historyCache.root.begin()) != 0) { throw std::runtime_error("hashing node failed"); }; @@ -488,9 +488,9 @@ void CCoinsViewCache::PushHistoryNode(uint32_t epochId, const HistoryNode node) entry_indices.data(), entries.data(), entry_indices.size(), - node.data(), + &node, newRoot.begin(), - appendBuf.data()->data() + appendBuf.data() ); for (size_t i = 0; i < appends; i++) { @@ -506,6 +506,7 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { switch (historyCache.length) { case 0: + { // Caller is generally not expected to pop from empty tree! Caller // should switch to previous epoch and pop history from there. @@ -517,22 +518,29 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { // back. // Sensible action is to truncate the history cache: + } 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: + { + const HistoryNode tmpHistoryRoot = GetHistoryAt(epochId, 0); // 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(), + &tmpHistoryRoot, newRoot.begin() ) != 0) { throw std::runtime_error("hashing node failed"); @@ -540,7 +548,9 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { 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; @@ -560,6 +570,7 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { historyCache.Truncate(historyCache.length - numberOfDeletes); historyCache.root = newRoot; return; + } } } diff --git a/src/rust/include/librustzcash.h b/src/rust/include/librustzcash.h index bc43f8601..0dd261cbe 100644 --- a/src/rust/include/librustzcash.h +++ b/src/rust/include/librustzcash.h @@ -8,7 +8,16 @@ #include #endif -#define ENTRY_SERIALIZED_LENGTH 180 +#define NODE_SERIALIZED_LENGTH 171 +#define ENTRY_SERIALIZED_LENGTH (NODE_SERIALIZED_LENGTH + 9) + +typedef struct HistoryNode { + unsigned char bytes[NODE_SERIALIZED_LENGTH]; +} HistoryNode; +static_assert( + sizeof(HistoryNode) == NODE_SERIALIZED_LENGTH, + "HistoryNode struct is not the same size as the underlying byte array"); +static_assert(alignof(HistoryNode) == 1, "HistoryNode struct alignment is not 1"); typedef struct HistoryEntry { unsigned char bytes[ENTRY_SERIALIZED_LENGTH]; @@ -331,9 +340,9 @@ extern "C" { const uint32_t *ni_ptr, const HistoryEntry *n_ptr, size_t p_len, - const unsigned char *nn_ptr, + const HistoryNode *nn_ptr, unsigned char *rt_ret, - unsigned char *buf_ret + HistoryNode *buf_ret ); uint32_t librustzcash_mmr_delete( @@ -348,7 +357,7 @@ extern "C" { uint32_t librustzcash_mmr_hash_node( uint32_t cbranch, - const unsigned char *n_ptr, + const HistoryNode *n_ptr, unsigned char *h_ret ); diff --git a/src/txdb.cpp b/src/txdb.cpp index 78843beea..d2f9b05e5 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -145,10 +145,15 @@ HistoryNode CCoinsViewDB::GetHistoryAt(uint32_t epochId, HistoryIndex index) con throw runtime_error("History data inconsistent - reindex?"); } - if (!db.Read(make_pair(DB_MMR_NODE, make_pair(epochId, index)), mmrNode)) { + // Read mmrNode into tmp std::array + std::array tmpMmrNode; + + if (!db.Read(make_pair(DB_MMR_NODE, make_pair(epochId, index)), tmpMmrNode)) { throw runtime_error("History data inconsistent (expected node not found) - reindex?"); } + std::copy(std::begin(tmpMmrNode), std::end(tmpMmrNode), mmrNode.bytes); + return mmrNode; } @@ -205,7 +210,10 @@ void BatchWriteHistory(CDBBatch& batch, CHistoryCacheMap& historyCacheMap) { // 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 mmrNode into tmp std::array + std::array tmpMmrNode; + std::copy((it->second).bytes, (it->second).bytes + NODE_SERIALIZED_LENGTH, std::begin(tmpMmrNode)); + batch.Write(make_pair(DB_MMR_NODE, make_pair(epochId, it->first)), tmpMmrNode); } // write new length diff --git a/src/zcash/History.cpp b/src/zcash/History.cpp index 2ffbec2f0..1d6ef76f8 100644 --- a/src/zcash/History.cpp +++ b/src/zcash/History.cpp @@ -68,7 +68,8 @@ HistoryNode NewNode( buf << COMPACTSIZE(endHeight); buf << COMPACTSIZE(saplingTxCount); - std::copy(buf.begin(), buf.end(), result.begin()); + assert(buf.size() <= NODE_SERIALIZED_LENGTH); + std::copy(std::begin(buf), std::end(buf), result.bytes); return result; } @@ -104,7 +105,11 @@ HistoryEntry NodeToEntry(const HistoryNode node, uint32_t left, uint32_t right) buf << code; buf << left; buf << right; - buf << node; + + std::array tmpMmrNode; + std::copy(node.bytes, node.bytes + NODE_SERIALIZED_LENGTH, std::begin(tmpMmrNode)); + + buf << tmpMmrNode; assert(buf.size() <= ENTRY_SERIALIZED_LENGTH); std::copy(std::begin(buf), std::end(buf), result.bytes); @@ -118,7 +123,11 @@ HistoryEntry LeafToEntry(const HistoryNode node) { uint8_t code = 1; buf << code; - buf << node; + + std::array tmpMmrNode; + std::copy(node.bytes, node.bytes + NODE_SERIALIZED_LENGTH, std::begin(tmpMmrNode)); + + buf << tmpMmrNode; assert(buf.size() <= ENTRY_SERIALIZED_LENGTH); std::copy(std::begin(buf), std::end(buf), result.bytes); diff --git a/src/zcash/History.hpp b/src/zcash/History.hpp index 3e25bd3e8..990764a32 100644 --- a/src/zcash/History.hpp +++ b/src/zcash/History.hpp @@ -13,10 +13,6 @@ namespace libzcash { -const int NODE_SERIALIZED_LENGTH = 171; - -typedef std::array HistoryNode; - typedef uint64_t HistoryIndex; class HistoryCache { @@ -66,6 +62,5 @@ HistoryEntry LeafToEntry(const HistoryNode node); typedef libzcash::HistoryCache HistoryCache; typedef libzcash::HistoryIndex HistoryIndex; -typedef libzcash::HistoryNode HistoryNode; #endif /* ZC_HISTORY_H_ */ \ No newline at end of file From 04b4d427677e8209c61abb2bcd114f71f2f8c034 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 22 Jul 2020 10:06:18 -0600 Subject: [PATCH 2/2] Zero-initialize HistoryNode values. --- src/coins.cpp | 2 +- src/txdb.cpp | 2 +- src/zcash/History.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 4e9ee2d9d..f2999df13 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -480,7 +480,7 @@ void CCoinsViewCache::PushHistoryNode(uint32_t epochId, const HistoryNode node) PreloadHistoryTree(epochId, false, entries, entry_indices); uint256 newRoot; - std::array appendBuf; + std::array appendBuf = {}; uint32_t appends = librustzcash_mmr_append( epochId, diff --git a/src/txdb.cpp b/src/txdb.cpp index d2f9b05e5..1abdc56ca 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -139,7 +139,7 @@ HistoryIndex CCoinsViewDB::GetHistoryLength(uint32_t epochId) const { } HistoryNode CCoinsViewDB::GetHistoryAt(uint32_t epochId, HistoryIndex index) const { - HistoryNode mmrNode; + HistoryNode mmrNode = {}; if (index >= GetHistoryLength(epochId)) { throw runtime_error("History data inconsistent - reindex?"); diff --git a/src/zcash/History.cpp b/src/zcash/History.cpp index 1d6ef76f8..72aae033d 100644 --- a/src/zcash/History.cpp +++ b/src/zcash/History.cpp @@ -54,7 +54,7 @@ HistoryNode NewNode( ) { CDataStream buf(SER_DISK, 0); - HistoryNode result; + HistoryNode result = {}; buf << subtreeCommitment; buf << startTime; @@ -135,4 +135,4 @@ HistoryEntry LeafToEntry(const HistoryNode node) { return result; } -} \ No newline at end of file +}