From 84dc3bf73c71a499245a8a360ad1ac188576f7e2 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 07:58:29 +0300 Subject: [PATCH 01/11] license notice --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 088208c58..defdafe26 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ The main design goals of this mmr implementation are # License -`zcash_mmr` is primarily distributed under the terms of both the MIT +`zcash_mmr` is distributed under the terms of both the MIT license and the Apache License (Version 2.0), at your choice. See LICENSE-APACHE, and LICENSE-MIT for details. From 9d4412103b80f25818f880bd9f9679d300fc8e50 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:00:07 +0300 Subject: [PATCH 02/11] more clear leaf_count --- src/entry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/entry.rs b/src/entry.rs index a3f14a8bb..257b9ddb2 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -29,7 +29,7 @@ impl Entry { /// Number of leaves under this node. pub fn leaf_count(&self) -> u64 { - self.data.end_height - self.data.start_height + 1 + self.data.end_height - (self.data.start_height - 1) } /// Is this node a leaf. From cb818ecbe3798d35ba78b0ccb74b11abdd7d4c2b Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:04:42 +0300 Subject: [PATCH 03/11] serialization sizes notice --- src/node_data.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/node_data.rs b/src/node_data.rs index a018daa88..5d7535bbd 100644 --- a/src/node_data.rs +++ b/src/node_data.rs @@ -3,7 +3,19 @@ use bigint::U256; use blake2::Params as Blake2Params; /// Maximum serialized size of the node metadata. -pub const MAX_NODE_DATA_SIZE: usize = 32 + 4 + 4 + 4 + 4 + 32 + 32 + 32 + 9 + 9 + 9; // 171 +pub const MAX_NODE_DATA_SIZE: usize = + 32 + // subtree commitment + 4 + // start time + 4 + // end time + 4 + // start target + 4 + // end target + 32 + // start sapling tree root + 32 + // end sapling tree root + 32 + // subtree total work + 9 + // start height (compact uint) + 9 + // end height (compact uint) + 9; // shielded tx count (compact uint) + // = total of 171 /// Node metadata. #[repr(C)] From 29b8d7a7563753e958e54a996d7393eb52276583 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:18:36 +0300 Subject: [PATCH 04/11] serialization roundtrip test --- src/node_data.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/node_data.rs b/src/node_data.rs index 5d7535bbd..bbaa98c07 100644 --- a/src/node_data.rs +++ b/src/node_data.rs @@ -20,6 +20,7 @@ pub const MAX_NODE_DATA_SIZE: usize = /// Node metadata. #[repr(C)] #[derive(Debug, Clone, Default)] +#[cfg_attr(test, derive(PartialEq))] pub struct NodeData { /// Consensus branch id, should be provided by deserializing node. pub consensus_branch_id: u32, @@ -192,4 +193,40 @@ impl NodeData { let mut cursor = std::io::Cursor::new(buf); Self::read(consensus_branch_id, &mut cursor) } +} + +#[cfg(test)] +impl quickcheck::Arbitrary for NodeData { + fn arbitrary(gen: &mut G) -> Self { + let mut node_data = NodeData::default(); + node_data.consensus_branch_id = 0; + gen.fill_bytes(&mut node_data.subtree_commitment[..]); + node_data.start_time = gen.next_u32(); + node_data.end_time = gen.next_u32(); + node_data.start_target = gen.next_u32(); + node_data.end_target = gen.next_u32(); + gen.fill_bytes(&mut node_data.start_sapling_root[..]); + gen.fill_bytes(&mut node_data.end_sapling_root[..]); + let mut number = [0u8; 32]; + gen.fill_bytes(&mut number[..]); + node_data.subtree_total_work = U256::from_little_endian(&number[..]); + node_data.start_height = gen.next_u64(); + node_data.end_height = gen.next_u64(); + node_data.shielded_tx = gen.next_u64(); + + node_data + } +} + +#[cfg(test)] +mod tests { + use super::NodeData; + use quickcheck::{quickcheck, TestResult}; + + + quickcheck! { + fn serialization_round_trip(node_data: NodeData) -> TestResult { + TestResult::from_bool(NodeData::from_bytes(0, &node_data.to_bytes()).unwrap() == node_data) + } + } } \ No newline at end of file From e701687b691ea920bc51c867e8a870ef34cb92bf Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:28:50 +0300 Subject: [PATCH 05/11] various small fixes --- README.md | 4 ++-- src/node_data.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index defdafe26..f034137ec 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ # zcash_mmr -Special implementation of merkle mountain ranges (MMR) for ZCash! +Special implementation of Merkle mountain ranges (MMR) for Zcash! [![Build Status](https://travis-ci.org/NikVolf/zcash-mmr.svg?branch=master)](https://travis-ci.org/NikVolf/zcash-mmr) -The main design goals of this mmr implementation are +The main design goals of this MMR implementation are - Allow zero-cache and avoid db callbacks. As it is implemented, calling side must just smartly pre-load MMR nodes from the database (about log2(tree length) for append, twice as much for deletion). diff --git a/src/node_data.rs b/src/node_data.rs index bbaa98c07..ac545e24f 100644 --- a/src/node_data.rs +++ b/src/node_data.rs @@ -127,7 +127,7 @@ impl NodeData { i @ 0..=0xfc => i.into(), 0xfd => reader.read_u16::()?.into(), 0xfe => reader.read_u32::()?.into(), - _ => reader.read_u64::()?.into(), + _ => reader.read_u64::()?, }; Ok(result) From 6b7a3dec9ca47837e61a455b8cb560e8851b7ac6 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:44:20 +0300 Subject: [PATCH 06/11] store generated as vec --- src/tree.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/tree.rs b/src/tree.rs index 4482cd808..d1bf93666 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -12,14 +12,11 @@ use crate::{Entry, EntryLink, NodeData, Error, EntryKind}; pub struct Tree { stored: HashMap, - generated: HashMap, + generated: Vec, // number of persistent(!) tree entries stored_count: u32, - // number of virtual nodes generated - generated_count: u32, - root: EntryLink, } @@ -28,7 +25,7 @@ impl Tree { pub fn resolve_link(&self, link: EntryLink) -> Result { match link { EntryLink::Generated(index) => { - let node = self.generated.get(&index).ok_or(Error::ExpectedInMemory(link))?; + let node = self.generated.get(index as usize).ok_or(Error::ExpectedInMemory(link))?; Ok(IndexedNode { node, link, @@ -52,14 +49,13 @@ impl Tree { } fn push_generated(&mut self, data: Entry) -> EntryLink { - let idx = self.generated_count; - self.generated_count = self.generated_count + 1; - self.generated.insert(idx, data); - EntryLink::Generated(idx) + self.generated.push(data); + EntryLink::Generated(self.generated.len() as u32 - 1) } - /// Populate tree with plain list of the leaves/nodes. Mostly for tests, - /// since this `Tree` structure is for partially loaded tree. + /// Populate tree with plain list of the leaves/nodes. For now, only for tests, + /// since this `Tree` structure is for partially loaded tree (but it might change) + #[cfg(test)] pub fn populate(loaded: Vec, root: EntryLink) -> Self { let mut result = Tree::invalid(); result.stored_count = loaded.len() as u32; @@ -71,12 +67,12 @@ impl Tree { result } + // Empty tree with invalid root fn invalid() -> Self { Tree { root: EntryLink::Generated(0), generated: Default::default(), stored: Default::default(), - generated_count: 0, stored_count: 0, } } From 3082593fccfed7c99fd7779eff6a115bcd48a386 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:50:44 +0300 Subject: [PATCH 07/11] add intented use of the api --- src/tree.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tree.rs b/src/tree.rs index d1bf93666..65e0220d6 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -8,10 +8,15 @@ use crate::{Entry, EntryLink, NodeData, Error, EntryKind}; /// With only some of the leaves/nodes pre-loaded / pre-generated. /// Exact amount of the loaded data can be calculated by the constructing party, /// depending on the length of the tree and maximum amount of operations that are going -/// to happen after construction. +/// to happen after construction. `Tree` should not be used as self-contained data structure, +/// since it's internal state can grow indefinitely after serial operations. +/// Intended use of this `Tree` is to instantiate it based on partially loaded data (see example +/// how to pick right nodes from the array representation of MMR Tree), perform several operations +/// (append-s/delete-s) and then drop it. pub struct Tree { stored: HashMap, + // This can grow indefinitely if `Tree` is misused as a self-contained data structure generated: Vec, // number of persistent(!) tree entries From b9bfc07146f2576d47e9c14bf1df0f2f540c1bc5 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 08:55:43 +0300 Subject: [PATCH 08/11] style fixes --- src/tree.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/tree.rs b/src/tree.rs index 65e0220d6..1d3db06da 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -48,7 +48,7 @@ impl Tree { fn push(&mut self, data: Entry) -> EntryLink { let idx = self.stored_count; - self.stored_count = self.stored_count + 1; + self.stored_count += 1; self.stored.insert(idx, data); EntryLink::Stored(idx) } @@ -84,15 +84,22 @@ impl Tree { /// New view into the the tree array representation /// - /// `length` is total length of the array representation + /// `length` is total length of the array representation (is generally not a sum of + /// peaks.len + extra.len) /// `peaks` is peaks of the mmr tree /// `extra` is some extra nodes that calculated to be required during next one or more /// operations on the tree. + /// + /// # Panics + /// + /// Will panic if `peaks` is empty. pub fn new( length: u32, peaks: Vec<(u32, Entry)>, extra: Vec<(u32, Entry)>, ) -> Self { + assert!(peaks.len() > 0); + let mut result = Tree::invalid(); result.stored_count = length; @@ -157,6 +164,9 @@ impl Tree { let mut merge_stack = Vec::new(); merge_stack.push(new_leaf_link); + // Scan the peaks right-to-left, merging together equal-sized adjacent + // complete subtrees. After this, merge_stack only contains peaks of + // unequal-sized subtrees. while let Some(next_peak) = peaks.pop() { let next_merge = merge_stack.pop().expect("there should be at least one, initial or re-pushed"); @@ -171,12 +181,15 @@ impl Tree { merge_stack.push(link); appended.push(link); continue; + } else { + merge_stack.push(next_merge); + merge_stack.push(next_peak); } - merge_stack.push(next_merge); - merge_stack.push(next_peak); } let mut new_root = merge_stack.pop().expect("Loop above cannot reduce the merge_stack"); + // Scan the peaks left-to-right, producing new generated nodes that + // connect the subtrees while let Some(next_child) = merge_stack.pop() { new_root = self.push_generated( combine_nodes( @@ -210,7 +223,8 @@ impl Tree { /// Truncate one leaf from the end of the tree. /// - /// Returns actual number of nodes that has to be removed from the array representation. + /// Returns actual number of nodes that should be removed by the caller + /// from the end of the array representation. pub fn truncate_leaf(&mut self) -> Result { let root = { let (leaves, root_left_child) = { @@ -245,7 +259,7 @@ impl Tree { } } - let mut new_root = *peaks.iter().nth(0).expect("At lest 2 elements in peaks"); + let mut new_root = *peaks.get(0).expect("At lest 1 elements in peaks"); for next_peak in peaks.into_iter().skip(1) { new_root = self.push_generated( From f24ec04340d8bb816ae95939f14193a15959a09f Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 09:00:47 +0300 Subject: [PATCH 09/11] add is_empty --- src/tree.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tree.rs b/src/tree.rs index 1d3db06da..d1d1d6f13 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -289,6 +289,10 @@ impl Tree { pub fn root_node(&self) -> Result { self.resolve_link(self.root) } + + pub fn is_empty(&self) -> bool { + self.stored_count == 0 + } } /// Reference to the node with link attached. From 481e43689cd2d2aa8e0d856970d74384e06a8c2c Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 09:04:04 +0300 Subject: [PATCH 10/11] use NodeData::combine --- src/tree.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/tree.rs b/src/tree.rs index d1d1d6f13..fbadaba81 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -285,11 +285,12 @@ impl Tree { /// Link to the root node pub fn root(&self) -> EntryLink { self.root } - /// Reference to the root ndoe + /// Reference to the root node. pub fn root_node(&self) -> Result { self.resolve_link(self.root) } + /// If this tree is empty. pub fn is_empty(&self) -> bool { self.stored_count == 0 } @@ -359,29 +360,12 @@ mod tests { } } - fn node(start_height: u64, end_height: u64) -> NodeData { - NodeData { - consensus_branch_id: 1, - subtree_commitment: [0u8; 32], - start_time: 0, - end_time: 0, - start_target: 0, - end_target: 0, - start_sapling_root: [0u8; 32], - end_sapling_root: [0u8; 32], - subtree_total_work: 0.into(), - start_height: start_height, - end_height: end_height, - shielded_tx: 7, - } - } - fn initial() -> Tree { let node1: Entry = leaf(1).into(); let node2: Entry = leaf(2).into(); let node3 = Entry { - data: node(1, 2), + data: NodeData::combine(&node1.data, &node2.data), kind: EntryKind::Leaf, }; From acad37924b7cd4fccaa7988e697d3747e8f87f42 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 11 Oct 2019 09:14:19 +0300 Subject: [PATCH 11/11] test updates --- src/tree.rs | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/tree.rs b/src/tree.rs index fbadaba81..d3b6dcd7a 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -205,7 +205,7 @@ impl Tree { } #[cfg(test)] - fn for_children(&mut self, node: EntryLink, mut f: F) { + fn for_children(&self, node: EntryLink, f: F) { let (left, right) = { let link = self.resolve_link(node).expect("Failed to resolve link in test"); ( @@ -485,7 +485,7 @@ mod tests { // (0) (1) (3) (4) (7) // // new tree: - // (---8g---) + // (---10g--) // / \ // ( 6 ) \ // / \ \ @@ -494,7 +494,7 @@ mod tests { // (0) (1) (3) (4) (7) (8) // // so (7) is added as real leaf - // and new root, (8g) is generated one + // and new root, (10g) is generated one assert_eq!(new_root.data.end_height, 6); assert_eq!(appended.len(), 2); assert_matches!(tree.root(), EntryLink::Generated(_)); @@ -533,13 +533,16 @@ mod tests { // / \ / \ / \ \ // (0) (1) (3) (4) (7) (8) (10) // - // so (7) is added as real leaf - // and new root, (8g) is generated one + // so (10) is added as real leaf + // and new root, (12g) is generated one assert_eq!(new_root.data.end_height, 7); assert_eq!(appended.len(), 1); assert_matches!(tree.root(), EntryLink::Generated(_)); tree.for_children(tree.root(), |l, r| { assert_matches!(l, EntryLink::Generated(_)); + tree.for_children(l, |l, r| + assert_matches!((l, r), (EntryLink::Stored(6), EntryLink::Stored(9))) + ); assert_matches!(r, EntryLink::Stored(10)); }); } @@ -547,7 +550,7 @@ mod tests { #[test] fn truncate_simple() { let mut tree = generated(9); - tree.truncate_leaf().expect("Failed to truncate"); + let total_truncated = tree.truncate_leaf().expect("Failed to truncate"); // initial tree: // @@ -574,6 +577,7 @@ mod tests { // and new root, (14) is a stored one now assert_matches!(tree.root(), EntryLink::Stored(14)); + assert_eq!(total_truncated, 1); assert_eq!(tree.len(), 15); } @@ -609,19 +613,11 @@ mod tests { assert_matches!(tree.root(), EntryLink::Generated(_)); - // left is 14 and right is 15 - let (left_root_child, right_root_child) = { - let root = tree.root_node().expect("Failed to resolve"); - - ( - root.left().expect("Expected node"), - root.right().expect("Expected node"), + tree.for_children(tree.root(),|left, right| + assert_matches!( + (left, right), + (EntryLink::Stored(14), EntryLink::Stored(15)) ) - }; - - assert_matches!( - (left_root_child, right_root_child), - (EntryLink::Stored(14), EntryLink::Stored(15)) ); // two stored nodes should leave us (leaf 16 and no longer needed node 17) @@ -706,7 +702,7 @@ mod tests { } TestResult::from_bool( - if number & number - 1 == 0 { + if number & (number - 1) == 0 { if let EntryLink::Stored(_) = tree.root() { true } else { false } } else {