Merge pull request #16 from olivmath/claude/issue-11-duplicate-leaves-01VwesaAX93Kwmeq6bLVcZvR

olivmath · web-flow · commit 1c1764c15f4c · 2025-12-05T05:26:17.000-03:00
docs: investigate and document Issue #11 - duplicate leaves behavior
diff --git a/ISSUE_11_FINDINGS.md b/ISSUE_11_FINDINGS.md
@@ -0,0 +1,163 @@
+# Issue #11: Duplicate Leaves Investigation
+
+## Summary
+
+This document describes the behavior of the Merkle Tree implementation when duplicate leaves are present in the tree.
+
+## Question
+
+What happens when there are duplicate leaves in the leaf list?
+- How are the proofs generated?
+- How is the root generated?
+
+Example: `["m","e","r","k","l","e","t","r","e","e","r","s"]`
+
+## Findings
+
+### 1. Root Generation ✅
+
+**The root is generated correctly even with duplicate leaves.**
+
+- The Merkle root calculation works as expected
+- Duplicate leaves are treated as independent nodes at their respective positions
+- The tree structure is built correctly regardless of duplicate values
+
+Example:
+```rust
+let data = ["a", "b", "a", "c"]; // "a" appears twice
+let tree = MerkleTree::new(leaves);
+// Root is calculated successfully
+```
+
+### 2. Proof Generation ⚠️
+
+**Proofs are always generated for the FIRST occurrence of a duplicate leaf.**
+
+The implementation uses `iter().position()` which returns the index of the **first matching element**.
+
+Example:
+```rust
+let data = ["a", "b", "c", "a", "d", "a"]; // "a" at indices 0, 3, 5
+
+let mut leaf_a = hash("a");
+let proof = tree.make_proof(leaf_a);
+// This proof will ALWAYS be for the "a" at index 0
+```
+
+**Important implications:**
+- You cannot generate proofs for the 2nd, 3rd, etc. occurrences of a duplicate leaf
+- The library has no way to distinguish between different positions of the same value
+- All proofs for a duplicate value will prove the first occurrence
+
+### 3. Proof Verification ✅
+
+**Proof verification works correctly.**
+
+- The generated proof can be successfully verified
+- `check_proof()` correctly reconstructs the root
+- The proof is mathematically valid for the first occurrence
+
+### 4. Test Results
+
+Tested with the exact example from Issue #11:
+```
+["m","e","r","k","l","e","t","r","e","e","r","s"]
+     ^         ^      ^  ^  ^  ^      (duplicates)
+```
+
+Results:
+- **Root generated**: ✅ Success
+- **Proofs for 'm'**: ✅ Valid (4 nodes)
+- **Proofs for 'e'**: ✅ Valid (4 nodes) - proves FIRST 'e'
+- **Proofs for 'r'**: ✅ Valid (4 nodes) - proves FIRST 'r'
+- **Proofs for 'k'**: ✅ Valid (4 nodes)
+- **Proofs for 'l'**: ✅ Valid (4 nodes)
+- **Proofs for 't'**: ✅ Valid (4 nodes)
+- **Proofs for 's'**: ✅ Valid (3 nodes)
+
+## Recommendations
+
+### For Library Users
+
+1. **Avoid duplicate leaves if possible**
+   - Use unique identifiers or add position/index information to values
+   - Consider hashing `value + position` instead of just `value`
+
+2. **If duplicates are necessary**
+   - Be aware that proofs will always reference the first occurrence
+   - Document this behavior in your application
+   - Consider adding metadata to distinguish duplicate values
+
+3. **Example workaround**:
+   ```rust
+   // Instead of this:
+   let leaves = ["a", "b", "a", "c"];
+
+   // Do this:
+   let leaves_with_index = [
+       ("a", 0),
+       ("b", 1),
+       ("a", 2),  // Now unique!
+       ("c", 3),
+   ];
+   ```
+
+### For Library Maintainers
+
+#### Option 1: Keep Current Behavior (Recommended)
+- Document the "first occurrence" behavior clearly
+- Add warning in documentation about duplicate leaves
+- This is the simplest and most performant approach
+
+#### Option 2: Add Position-based Proof API
+Add a new method to specify which occurrence:
+```rust
+// Current API (keeps first occurrence behavior)
+pub fn make_proof(&self, leaf: Leaf) -> Vec<Node>
+
+// New API (specify position)
+pub fn make_proof_at(&self, leaf: Leaf, position: usize) -> Result<Vec<Node>, Error>
+```
+
+#### Option 3: Reject Duplicates
+Add validation to reject duplicate leaves:
+```rust
+impl MerkleTree {
+    pub fn new(leaves: Vec<Leaf>) -> Result<Self, Error> {
+        // Check for duplicates
+        let unique: HashSet<_> = leaves.iter().collect();
+        if unique.len() != leaves.len() {
+            return Err(Error::DuplicateLeaves);
+        }
+        // ... rest of implementation
+    }
+}
+```
+
+## Conclusion
+
+The current implementation handles duplicate leaves **gracefully but with limitations**:
+
+✅ **Pros:**
+- No crashes or panics
+- Root generation works correctly
+- Proofs are mathematically valid
+- Performance is not impacted
+
+⚠️ **Cons:**
+- Cannot generate proofs for non-first occurrences of duplicates
+- No way to specify which duplicate to prove
+- Behavior may be surprising to users
+
+**Recommendation**: Document the current behavior clearly and advise users to avoid duplicates or add position information to their values.
+
+## Test Coverage
+
+All tests pass successfully:
+- `test_duplicate_leaves_root_generation` ✅
+- `test_duplicate_leaves_proof_generation` ✅
+- `test_duplicate_leaves_multiple_proofs` ✅
+- `test_unique_leaves_vs_duplicate_leaves` ✅
+- `test_issue_11_exact_example` ✅
+
+See `tests/test_issue_11_duplicates.rs` for complete test implementation.
diff --git a/tests/test_issue_11_duplicates.rs b/tests/test_issue_11_duplicates.rs
@@ -0,0 +1,229 @@
+/// Investigation of Issue #11: Duplicate leaves behavior
+///
+/// This test investigates what happens when there are duplicate leaves in the tree.
+/// Questions to answer:
+/// 1. How are the proofs generated?
+/// 2. How is the root generated?
+/// 3. What happens when we try to make a proof for a duplicate leaf?
+
+use merkletreers::tree::MerkleTree;
+use merkletreers::utils::hash_it;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_duplicate_leaves_root_generation() {
+        // Example from issue #11: ["m","e","r","k","l","e","t","r","e","e","r","s"]
+        // Note: "e" appears 4 times, "r" appears 3 times
+        let data = ["m", "e", "r", "k", "l", "e", "t", "r", "e", "e", "r", "s"];
+
+        let leaves = data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        // Create tree with duplicate leaves
+        let tree = MerkleTree::new(leaves.clone());
+
+        // Root should be generated successfully
+        println!("Root with duplicates: {:?}", tree.root);
+        assert_ne!(tree.root, [0u8; 32], "Root should not be zero");
+
+        // Let's also test with a smaller example for clarity
+        let simple_duplicates = ["a", "b", "a", "c"];
+        let simple_leaves = simple_duplicates
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        let simple_tree = MerkleTree::new(simple_leaves);
+        println!("Simple root with duplicates: {:?}", simple_tree.root);
+        assert_ne!(simple_tree.root, [0u8; 32], "Simple root should not be zero");
+    }
+
+    #[test]
+    fn test_duplicate_leaves_proof_generation() {
+        // When we have duplicates, the proof is generated for the FIRST occurrence
+        let data = ["a", "b", "a", "c"];
+
+        let leaves = data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        let tree = MerkleTree::new(leaves.clone());
+
+        // Hash of "a"
+        let mut leaf_a = [0u8; 32];
+        hash_it("a".as_bytes(), &mut leaf_a);
+
+        // Make proof for "a" - this will find the FIRST occurrence at index 0
+        let proof = tree.make_proof(leaf_a);
+
+        println!("Proof for duplicate 'a': {:?}", proof);
+
+        // Save root before check_proof consumes tree
+        let expected_root = tree.root;
+
+        // Verify the proof
+        let computed_root = tree.check_proof(proof, leaf_a);
+        println!("Computed root: {:?}", computed_root);
+        println!("Expected root: {:?}", expected_root);
+
+        // The proof is valid because it proves the FIRST occurrence
+        assert_eq!(
+            computed_root, expected_root,
+            "Proof verification should succeed for first occurrence"
+        );
+
+        // Important note: We cannot distinguish between different positions of the same leaf value
+        // The proof will always be for the FIRST occurrence found by iter().position()
+    }
+
+    #[test]
+    fn test_duplicate_leaves_multiple_proofs() {
+        // Test what happens when we try to make proofs for all instances of a duplicate
+        let data = ["a", "b", "c", "a", "d", "a"];
+
+        let leaves = data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        let tree = MerkleTree::new(leaves.clone());
+
+        // Hash of "a"
+        let mut leaf_a = [0u8; 32];
+        hash_it("a".as_bytes(), &mut leaf_a);
+
+        // Try to make proof for "a"
+        // The current implementation will find the FIRST occurrence at index 0
+        let proof = tree.make_proof(leaf_a);
+
+        println!("Number of proof nodes: {}", proof.len());
+        println!("Proof for first 'a': {:?}", proof);
+
+        // Save root before check_proof consumes tree
+        let expected_root = tree.root;
+
+        // Verify the proof
+        let computed_root = tree.check_proof(proof.clone(), leaf_a);
+        println!("Computed root: {:?}", computed_root);
+        println!("Expected root: {:?}", expected_root);
+
+        assert_eq!(
+            computed_root, expected_root,
+            "Proof verification should succeed for first occurrence"
+        );
+
+        // The issue is: we cannot distinguish between different positions of the same leaf value
+        // The proof will always be for the FIRST occurrence
+    }
+
+    #[test]
+    fn test_unique_leaves_vs_duplicate_leaves() {
+        // Compare behavior with unique vs duplicate leaves
+
+        // Unique leaves
+        let unique_data = ["a", "b", "c", "d"];
+        let unique_leaves = unique_data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        let unique_tree = MerkleTree::new(unique_leaves.clone());
+
+        // Duplicate leaves (same data but "a" appears twice)
+        let duplicate_data = ["a", "b", "a", "d"];
+        let duplicate_leaves = duplicate_data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        let duplicate_tree = MerkleTree::new(duplicate_leaves.clone());
+
+        // Roots should be different because the tree structure is different
+        assert_ne!(
+            unique_tree.root, duplicate_tree.root,
+            "Different leaf arrangements should produce different roots"
+        );
+
+        println!("Unique tree root: {:?}", unique_tree.root);
+        println!("Duplicate tree root: {:?}", duplicate_tree.root);
+    }
+
+    #[test]
+    fn test_issue_11_exact_example() {
+        // Exact example from issue #11
+        let data = ["m", "e", "r", "k", "l", "e", "t", "r", "e", "e", "r", "s"];
+
+        let leaves = data
+            .iter()
+            .map(|d| {
+                let mut buffer = [0u8; 32];
+                hash_it(d.as_bytes(), &mut buffer);
+                buffer
+            })
+            .collect::<Vec<[u8; 32]>>();
+
+        println!("Number of leaves: {}", leaves.len());
+
+        let tree = MerkleTree::new(leaves.clone());
+
+        println!("Root: {:?}", tree.root);
+        let expected_root = tree.root;
+
+        // Try to make proof for each unique letter
+        let unique_letters = ["m", "e", "r", "k", "l", "t", "s"];
+
+        for letter in unique_letters.iter() {
+            let mut leaf = [0u8; 32];
+            hash_it(letter.as_bytes(), &mut leaf);
+
+            let tree_for_proof = MerkleTree::new(leaves.clone());
+            let proof = tree_for_proof.make_proof(leaf);
+
+            let tree_for_check = MerkleTree::new(leaves.clone());
+            let computed_root = tree_for_check.check_proof(proof.clone(), leaf);
+
+            println!(
+                "Letter '{}': proof length = {}, verification = {}",
+                letter,
+                proof.len(),
+                computed_root == expected_root
+            );
+
+            assert_eq!(
+                computed_root, expected_root,
+                "Proof verification should succeed for '{}'",
+                letter
+            );
+        }
+    }
+}
