refactor!(stump): Move updated data to its own function

Before this change, `modify` would return the data needed to update
a proof for the new block. This requires additional internal
computation and extra allocations. During IBD we may never use this,
since proof update is only meant for updating a few blocks worth of
changes. Now there's a method specific to pull the modify_data, and
`modify` itself will only return a new Stump.

When refactoring the addition function, I've modified it a little
to allow a smarter one with a very nice property: it can pretend that
it added a node, but actually represent it as deleted. The goal here
is to do a Swift Sync-style protocol where you don't need deletions.

If a txout is already spent, you give an empty hash
(BitcoinNodeHash::empty()). This will be exactly equivalent as giving
this txo's hash and later on calling delete for with this txo's hash.
However, it does this with only one call to modify and doesn't require
proofs to achieve that.

This is an API breaking change, as modify now only returns one parameter,
otherwise the behavior stays unchanged.

Author: Davidson-Souza

benches/proof_benchmarks.rs

@@ -55,7 +55,7 @@ fn proof_verification(c: &mut Criterion) {
     let accumulator_size = 100;
     let hashes = generate_test_hashes(accumulator_size, 42);
     let stump = Stump::new();
-    let (stump, _) = stump.modify(&hashes, &[], &Proof::default()).unwrap();
+    let stump = stump.modify(&hashes, &[], &Proof::default()).unwrap();
 
     for target_count in [1, 10].iter() {
         let del_hashes = hashes[..*target_count].to_vec();

benches/stump_benchmarks.rs

@@ -50,7 +50,7 @@ fn stump_verify(c: &mut Criterion) {
     let test_size = 1000;
     let hashes = generate_test_hashes(test_size, 42);
     let stump = Stump::new();
-    let (stump, _) = stump.modify(&hashes, &[], &Proof::default()).unwrap();
+    let stump = stump.modify(&hashes, &[], &Proof::default()).unwrap();
 
     for proof_size in [1, 10, 100].iter() {
         let del_hashes = hashes[..*proof_size].to_vec();

examples/full-accumulator.rs

@@ -29,8 +29,7 @@ fn main() {
     // Verify the proof. Notice how we use the del_hashes returned by `prove` here.
     let s = Stump::new()
         .modify(&elements, &[], &Proof::default())
-        .unwrap()
-        .0;
+        .unwrap();
     assert_eq!(s.verify(&proof, &[elements[0]]), Ok(true));
     // Now we want to update the MemForest, by removing the first utxo, and adding a new one.
     // This would be in case we received a new block with a transaction spending the first utxo,

examples/proof-update.rs

@@ -22,7 +22,9 @@ fn main() {
     let utxos = get_utxo_hashes1();
     // Add the UTXOs to the accumulator. update_data is the data we need to update the proof
     // after the accumulator is updated.
-    let (s, update_data) = s.modify(&utxos, &[], &Proof::default()).unwrap();
+    let update_data = s.get_update_data(&utxos, &[], &Proof::default()).unwrap();
+    let s = s.modify(&utxos, &[], &Proof::default()).unwrap();
+
     // Create an empty proof, we'll update it to hold our UTXOs
     let p = Proof::default();
     // Update the proof with the UTXOs we added to the accumulator. This proof was initially empty,
@@ -48,8 +50,10 @@ fn main() {
     // We'll remove `0` as it got spent, and add 1..7 to our cache.
     let new_utxos = get_utxo_hashes2();
     // First, update the accumulator
-    let (stump, update_data) = s.modify(&new_utxos, &[utxos[0]], &p1).unwrap();
+    let stump = s.modify(&new_utxos, &[utxos[0]], &p1).unwrap();
+
     // and the proof
+    let update_data = s.get_update_data(&utxos, &[], &Proof::default()).unwrap();
     let (p2, cached_hashes) = p
         .update(
             cached_hashes,

examples/simple-stump-update.rs

@@ -27,10 +27,7 @@ fn main() {
     // Create a new Stump, and add the utxos to it. Notice how we don't use the full return here,
     // but only the Stump. To understand what is the second return value, see the documentation
     // for `Stump::modify`, or the proof-update example.
-    let s = Stump::new()
-        .modify(&utxos, &[], &Proof::default())
-        .unwrap()
-        .0;
+    let s = Stump::new().modify(&utxos, &[], &Proof::default()).unwrap();
     // Create a proof that the first utxo is in the Stump.
     let proof = Proof::new(vec![0], vec![utxos[1]]);
     assert_eq!(s.verify(&proof, &[utxos[0]]), Ok(true));
@@ -42,7 +39,7 @@ fn main() {
         "d3bd63d53c5a70050a28612a2f4b2019f40951a653ae70736d93745efb1124fa",
     )
     .unwrap();
-    let s = s.modify(&[new_utxo], &[utxos[0]], &proof).unwrap().0;
+    let s = s.modify(&[new_utxo], &[utxos[0]], &proof).unwrap();
     // Now we can verify that the new utxo is in the Stump, and the old one is not.
     let new_proof = Proof::new(vec![2], vec![new_utxo]);
     assert_eq!(s.verify(&new_proof, &[new_utxo]), Ok(true));

src/accumulator/proof.rs

@@ -48,7 +48,7 @@
 //!     hashes.push(sha256::Hash::from_engine(engine).into())
 //! }
 //! // Add the UTXOs to the accumulator
-//! let s = s.modify(&hashes, &vec![], &Proof::default()).unwrap().0;
+//! let s = s.modify(&hashes, &vec![], &Proof::default()).unwrap();
 //! // Create a proof for the targets
 //! let p = Proof::new(targets, proof_hashes);
 //! // Verify the proof
@@ -123,7 +123,7 @@ pub(crate) type NodesAndRootsCurrent<Hash> = (Vec<(u64, Hash)>, Vec<Hash>);
 /// This is used when we need to return the nodes and roots for a proof
 /// if we are concerned with deleting those elements. The difference is that
 /// we need to retun the old and updatated roots in the accumulator.
-pub(crate) type NodesAndRootsOldNew<Hash> = (Vec<(u64, Hash)>, Vec<(Hash, Hash)>);
+pub(crate) type RootsOldNew<Hash> = Vec<(Hash, Hash)>;
 
 impl Proof {
     /// Creates a proof from a vector of target and hashes.
@@ -260,7 +260,7 @@ impl<Hash: AccumulatorHash> Proof<Hash> {
     ///     engine.input(&[i]);
     ///     hashes.push(sha256::Hash::from_engine(engine).into())
     /// }
-    /// let s = s.modify(&hashes, &vec![], &Proof::default()).unwrap().0;
+    /// let s = s.modify(&hashes, &vec![], &Proof::default()).unwrap();
     /// let p = Proof::new(targets, proof_hashes);
     /// assert!(s.verify(&p, &[hashes[0]]).expect("This proof is valid"));
     /// ```
@@ -434,7 +434,7 @@ impl<Hash: AccumulatorHash> Proof<Hash> {
         &self,
         del_hashes: &[(Hash, Hash)],
         num_leaves: u64,
-    ) -> Result<NodesAndRootsOldNew<Hash>, String> {
+    ) -> Result<RootsOldNew<Hash>, String> {
         // Where all the root hashes that we've calculated will go to.
         let total_rows = util::tree_rows(num_leaves);
 
@@ -495,13 +495,7 @@ impl<Hash: AccumulatorHash> Proof<Hash> {
             computed.push((parent, (old_parent_hash, parent_hash)));
         }
 
-        // we shouldn't return the hashes in the proof
-        nodes.extend(computed);
-        let nodes = nodes
-            .into_iter()
-            .map(|(pos, (_, new_hash))| (pos, new_hash))
-            .collect();
-        Ok((nodes, calculated_root_hashes))
+        Ok(calculated_root_hashes)
     }
 
     /// This function computes a set of roots from a proof.
@@ -565,7 +559,13 @@ impl<Hash: AccumulatorHash> Proof<Hash> {
                 return Err(format!("Missing sibling for {next_pos}"));
             }
 
-            let parent_hash = AccumulatorHash::parent_hash(&next_hash, &sibling_hash);
+            let parent_hash = match (next_hash.is_empty(), sibling_hash.is_empty()) {
+                (true, true) => AccumulatorHash::empty(),
+                (true, false) => sibling_hash,
+                (false, true) => next_hash,
+                (false, false) => AccumulatorHash::parent_hash(&next_hash, &sibling_hash),
+            };
+
             let parent = util::parent(next_pos, total_rows);
             computed.push((parent, parent_hash));
         }
@@ -1006,7 +1006,10 @@ mod tests {
 
             let block_proof =
                 Proof::new(case_values.update.proof.targets.clone(), block_proof_hashes);
-            let (stump, updated) = stump.modify(&utxos, &del_hashes, &block_proof).unwrap();
+            let new_stump = stump.modify(&utxos, &del_hashes, &block_proof).unwrap();
+            let updated = stump
+                .get_update_data(&utxos, &del_hashes, &block_proof)
+                .unwrap();
             let (cached_proof, cached_hashes) = cached_proof
                 .update(
                     cached_hashes.clone(),
@@ -1017,7 +1020,7 @@ mod tests {
                 )
                 .unwrap();
 
-            let res = stump.verify(&cached_proof, &cached_hashes);
+            let res = new_stump.verify(&cached_proof, &cached_hashes);
 
             let expected_roots: Vec<_> = case_values
                 .expected_roots
@@ -1032,7 +1035,7 @@ mod tests {
                 .collect();
             assert_eq!(res, Ok(true));
             assert_eq!(cached_proof.targets, case_values.expected_targets);
-            assert_eq!(stump.roots, expected_roots);
+            assert_eq!(new_stump.roots, expected_roots);
             assert_eq!(cached_hashes, expected_cached_hashes);
         }
     }
@@ -1192,7 +1195,7 @@ mod tests {
     fn test_update_proof_delete() {
         let preimages = vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
         let hashes = preimages.into_iter().map(hash_from_u8).collect::<Vec<_>>();
-        let (stump, _) = Stump::new()
+        let stump = Stump::new()
             .modify(&hashes, &[], &Proof::default())
             .unwrap();
 
@@ -1220,13 +1223,22 @@ mod tests {
 
         let proof = Proof::new(vec![1, 2, 6], proof_hashes);
 
-        let (stump, modified) = stump
+        let new_stump = stump
             .modify(
                 &[],
                 &[hash_from_u8(1), hash_from_u8(2), hash_from_u8(6)],
                 &proof,
             )
             .unwrap();
+
+        let modified = stump
+            .get_update_data(
+                &[],
+                &[hash_from_u8(1), hash_from_u8(2), hash_from_u8(6)],
+                &proof,
+            )
+            .unwrap();
+
         let (new_proof, _) = cached_proof
             .update_proof_remove(
                 vec![1, 2, 6],
@@ -1236,7 +1248,7 @@ mod tests {
             )
             .unwrap();
 
-        let res = stump.verify(&new_proof, &[hash_from_u8(0), hash_from_u8(7)]);
+        let res = new_stump.verify(&new_proof, &[hash_from_u8(0), hash_from_u8(7)]);
         assert_eq!(res, Ok(true));
     }
 
@@ -1249,8 +1261,7 @@ mod tests {
         // Create a new stump with 8 leaves and 1 root
         let s = Stump::new()
             .modify(&hashes, &[], &Proof::default())
-            .expect("This stump is valid")
-            .0;
+            .expect("This stump is valid");
 
         // Nodes that will be deleted
         let del_hashes = vec![hashes[0], hashes[2], hashes[4], hashes[6]];
@@ -1342,35 +1353,18 @@ mod tests {
             .map(|hash| (hash, BitcoinNodeHash::empty()))
             .collect::<Vec<_>>();
 
-        let (computed, roots) = p.calculate_hashes_delete(&del_hashes, 8).unwrap();
+        let roots = p.calculate_hashes_delete(&del_hashes, 8).unwrap();
         let expected_root_old = BitcoinNodeHash::from_str(
             "b151a956139bb821d4effa34ea95c17560e0135d1e4661fc23cedc3af49dac42",
         )
         .unwrap();
+
         let expected_root_new = BitcoinNodeHash::from_str(
             "726fdd3b432cc59e68487d126e70f0db74a236267f8daeae30b31839a4e7ebed",
         )
         .unwrap();
 
-        let computed_positions = [0_u64, 1, 9, 13, 8, 12, 14].to_vec();
-        let computed_hashes = [
-            "0000000000000000000000000000000000000000000000000000000000000000",
-            "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a",
-            "9576f4ade6e9bc3a6458b506ce3e4e890df29cb14cb5d3d887672aef55647a2b",
-            "29590a14c1b09384b94a2c0e94bf821ca75b62eacebc47893397ca88e3bbcbd7",
-            "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a",
-            "2b77298feac78ab51bc5079099a074c6d789bd350442f5079fcba2b3402694e5",
-            "726fdd3b432cc59e68487d126e70f0db74a236267f8daeae30b31839a4e7ebed",
-        ]
-        .iter()
-        .map(|hash| BitcoinNodeHash::from_str(hash).unwrap())
-        .collect::<Vec<_>>();
-        let expected_computed: Vec<_> = computed_positions
-            .into_iter()
-            .zip(computed_hashes)
-            .collect();
         assert_eq!(roots, vec![(expected_root_old, expected_root_new)]);
-        assert_eq!(computed, expected_computed);
     }
 
     #[test]
@@ -1392,8 +1386,7 @@ mod tests {
         // Create a new stump with 8 leaves and 1 root
         let s = Stump::new()
             .modify(&hashes, &[], &Proof::default())
-            .expect("This stump is valid")
-            .0;
+            .expect("This stump is valid");
 
         // Nodes that will be deleted
         let del_hashes = vec![hashes[0], hashes[2], hashes[4], hashes[6]];