Merge pull request #130 from nuttycom/permit_invalid_legacy_witnesses

Permit invalid legacy witnesses

incrementalmerkletree-testing/src/complete_tree.rs

@@ -106,7 +106,7 @@ impl<H: Hashable, C: Clone + Ord + core::fmt::Debug, const DEPTH: u8> CompleteTr
                 self.mark();
             }
             Retention::Checkpoint { id, marking } => {
-                let latest_checkpoint = self.checkpoints.keys().rev().next();
+                let latest_checkpoint = self.checkpoints.keys().next_back();
                 if Some(&id) > latest_checkpoint {
                     append(&mut self.leaves, value, DEPTH)?;
                     if marking == Marking::Marked {
@@ -145,7 +145,7 @@ impl<H: Hashable, C: Clone + Ord + core::fmt::Debug, const DEPTH: u8> CompleteTr
             if !self.marks.contains(&pos) {
                 self.marks.insert(pos);
 
-                if let Some(checkpoint) = self.checkpoints.values_mut().rev().next() {
+                if let Some(checkpoint) = self.checkpoints.values_mut().next_back() {
                     checkpoint.marked.insert(pos);
                 }
             }
@@ -277,7 +277,7 @@ impl<H: Hashable + PartialEq + Clone, C: Ord + Clone + core::fmt::Debug, const D
 
     fn remove_mark(&mut self, position: Position) -> bool {
         if self.marks.contains(&position) {
-            if let Some(c) = self.checkpoints.values_mut().rev().next() {
+            if let Some(c) = self.checkpoints.values_mut().next_back() {
                 c.forgotten.insert(position);
             } else {
                 self.marks.remove(&position);
@@ -289,7 +289,7 @@ impl<H: Hashable + PartialEq + Clone, C: Ord + Clone + core::fmt::Debug, const D
     }
 
     fn checkpoint(&mut self, id: C) -> bool {
-        if Some(&id) > self.checkpoints.keys().rev().next() {
+        if Some(&id) > self.checkpoints.keys().next_back() {
             Self::checkpoint(self, id, self.current_position());
             true
         } else {
@@ -335,8 +335,6 @@ impl<H: Hashable + PartialEq + Clone, C: Ord + Clone + core::fmt::Debug, const D
 
 #[cfg(test)]
 mod tests {
-    use std::convert::TryFrom;
-
     use super::CompleteTree;
     use crate::{
         check_append, check_checkpoint_rewind, check_rewind_remove_mark, check_root_hashes,
@@ -432,7 +430,7 @@ mod tests {
         assert_eq!(tree.root(Some(0)), Some(expected.clone()));
 
         for i in 0u64..(1 << DEPTH) {
-            let position = Position::try_from(i).unwrap();
+            let position = Position::from(i);
             let path = tree.witness(position, 0).unwrap();
             assert_eq!(
                 compute_root_from_witness(SipHashable(i), position, &path),

incrementalmerkletree-testing/src/lib.rs

@@ -1062,7 +1062,7 @@ pub fn check_checkpoint_rewind<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usiz
 }
 
 pub fn check_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) -> T>(new_tree: F) {
-    let samples = vec![
+    let samples = [
         vec![
             append_str("a", Retention::Ephemeral),
             append_str(
@@ -1126,7 +1126,7 @@ pub fn check_rewind_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usi
     // test framework itself previously did not correctly handle
     // chain state restoration.
 
-    let samples = vec![
+    let samples = [
         vec![
             append_str("x", Retention::Marked),
             Checkpoint(C::from_u64(1)),

incrementalmerkletree/CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to Rust's notion of
 
 ## Unreleased
 
+## [0.8.2] - 2025-01-31
+
+### Added
+- `incrementalmerkletree::witness::IncrementalWitness::invalid_empty_witness`
+  has been under the `test-dependencies` feature flag to permit use testing
+  against `zcashd` test vectors that depend upon appending nodes to the
+  (invalid) empty witness.
+
 ## [0.8.1] - 2024-12-11
 
 ### Changed

incrementalmerkletree/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "incrementalmerkletree"
 description = "Common types, interfaces, and utilities for Merkle tree data structures"
-version = "0.8.1"
+version = "0.8.2"
 authors = [
     "Sean Bowe <[email protected]>",
     "Kris Nuttycombe <[email protected]>",

incrementalmerkletree/src/lib.rs

@@ -466,13 +466,13 @@ impl Address {
     /// Returns the minimum value among the range of leaf positions that are contained within the
     /// tree with its root at this address.
     pub fn position_range_start(&self) -> Position {
-        (self.index << self.level.0).try_into().unwrap()
+        (self.index << self.level.0).into()
     }
 
     /// Returns the (exclusive) end of the range of leaf positions that are contained within the
     /// tree with its root at this address.
     pub fn position_range_end(&self) -> Position {
-        ((self.index + 1) << self.level.0).try_into().unwrap()
+        ((self.index + 1) << self.level.0).into()
     }
 
     /// Returns the maximum value among the range of leaf positions that are contained within the

incrementalmerkletree/src/witness.rs

@@ -57,16 +57,32 @@ impl<H, const DEPTH: u8> IncrementalWitness<H, DEPTH> {
         })
     }
 
+    /// Creates an invalid empty `IncrementalWitness`. This constructor is provided for backwards
+    /// compatibility with the encodings of `zcashd` `IncrementalWitness` values; that type permits
+    /// multiple distinct encodings of the same witness state, and in some cases it is necessary to
+    /// create an empty witness and then append leaves in order to obtain the encoded forms
+    /// produced by `zcashd` (and which we must reproduce in order to demonstrate interoperability
+    /// with `zcashd` test vectors.) This should not be used except in a testing context.
+    #[cfg(feature = "test-dependencies")]
+    pub fn invalid_empty_witness() -> Self {
+        Self {
+            tree: CommitmentTree::empty(),
+            filled: vec![],
+            cursor_depth: 0,
+            cursor: None,
+        }
+    }
+
     /// Constructs an `IncrementalWitness` from its parts.
     ///
-    /// Returns `None` if the parts do not form a valid witness, for example if `tree` is
-    /// empty (and thus there is no position to witness).
+    /// Returns `None` if the parts do not form a valid witness, for example if all of the parts
+    /// are empty (and thus there is no position to witness).
     pub fn from_parts(
         tree: CommitmentTree<H, DEPTH>,
         filled: Vec<H>,
         cursor: Option<CommitmentTree<H, DEPTH>>,
     ) -> Option<Self> {
-        (!tree.is_empty()).then(|| {
+        (!(tree.is_empty() && filled.is_empty() && cursor.is_none())).then(|| {
             let mut witness = IncrementalWitness {
                 tree,
                 filled,

shardtree/src/store/caching.rs

@@ -1034,7 +1034,7 @@ mod tests {
 
     #[test]
     fn check_remove_mark() {
-        let samples = vec![
+        let samples = [
             vec![
                 append_str("a", Retention::Ephemeral),
                 append_str(
@@ -1128,7 +1128,7 @@ mod tests {
         // test framework itself previously did not correctly handle
         // chain state restoration.
 
-        let samples = vec![
+        let samples = [
             vec![
                 append_str("x", Retention::Marked),
                 Checkpoint(1),

shardtree/src/testing.rs

@@ -1,5 +1,3 @@
-use std::convert::TryFrom;
-
 use assert_matches::assert_matches;
 use proptest::bool::weighted;
 use proptest::collection::vec;
@@ -96,17 +94,19 @@ where
     })
 }
 
+/// A random shardtree of size up to 2^6 with shards of size 2^3, along with vectors of the
+/// checkpointed and marked positions within the tree.
+type ArbShardtreeParts<H> = (
+    ShardTree<MemoryShardStore<<H as Strategy>::Value, usize>, 6, 3>,
+    Vec<Position>,
+    Vec<Position>,
+);
+
 /// Constructs a random shardtree of size up to 2^6 with shards of size 2^3. Returns the tree,
-/// along with vectors of the checkpoint and mark positions.
+/// along with vectors of the checkpointed and marked positions.
 pub fn arb_shardtree<H: Strategy + Clone>(
     arb_leaf: H,
-) -> impl Strategy<
-    Value = (
-        ShardTree<MemoryShardStore<H::Value, usize>, 6, 3>,
-        Vec<Position>,
-        Vec<Position>,
-    ),
->
+) -> impl Strategy<Value = ArbShardtreeParts<H>>
 where
     H::Value: Hashable + Clone + PartialEq,
 {