Remove SMT nodes along the path during update (#627)

* Removed subtraction of predicate used gas from the gas limit. * Updated values of teh GTF * Implemented `Policies` structure. Updated transaction validity rules according to policies. Updated fee calculation and refund logic. * Applied change for VM. Fixed all tests. * Minor nits * Fix test in `fuel-core` * Fixed review ocmments * Update fuel-tx/src/transaction/types/create.rs Co-authored-by: Mitchell Turner <[email protected]> * Fixed comments from the PR. Updated CHANGELOG.md Simplified `refund` calculation logic * Added a test for refund * Remove SMT nodes along the path during `update` * Apply suggestions from code review Co-authored-by: Brandon Kite <[email protected]> * Updated CHANGELOG.md * Renamed `GasLimit` to `ScriptGasLimit` * Make CI happy * Update CHANGELOG.md Co-authored-by: Brandon Kite <[email protected]> * Update CHANGELOG.md Co-authored-by: Brandon Kite <[email protected]> * Renamed `gas_limit` to `script_gas_limit` * Renamed `gas_limit` to `script_gas_limit` * Use correct values for policies GTF * Address comments * Update CHANGELOG.md Co-authored-by: Brandon Vrooman <[email protected]> * Fixed the issue during inserting hte same key --------- Co-authored-by: Mitchell Turner <[email protected]> Co-authored-by: Brandon Kite <[email protected]> Co-authored-by: Brandon Vrooman <[email protected]>
FuelLabs · Nov 9, 2023 · 1405549 · 1405549
1 parent 61c6992
commit 1405549
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
     - If the `witnessses_size > policies.witness_limit`, then transaction will be rejected.
     - GTF opcode changed its hardcoded constants for fields. It should be updated according to the values from the specification on the Sway side.
 
+### Fixed
+
+- [#627](https://github.com/FuelLabs/fuel-vm/pull/627): Added removal of obsolete SMT nodes along the path during `update` and `delete` operations.
+
 ## [Version 0.41.0]
 
 #### Breaking

diff --git a/fuel-merkle/src/common/storage_map.rs b/fuel-merkle/src/common/storage_map.rs
@@ -36,6 +36,14 @@ where
             map: Default::default(),
         }
     }
+
+    pub fn is_empty(&self) -> bool {
+        self.map.is_empty()
+    }
+
+    pub fn len(&self) -> usize {
+        self.map.len()
+    }
 }
 
 impl<Type> StorageInspect<Type> for StorageMap<Type>

diff --git a/fuel-merkle/src/sparse/merkle_tree.rs b/fuel-merkle/src/sparse/merkle_tree.rs
@@ -126,6 +126,10 @@ impl<TableType, StorageType> MerkleTree<TableType, StorageType> {
         self.storage
     }
 
+    pub fn storage(&self) -> &StorageType {
+        &self.storage
+    }
+
     // PRIVATE
 
     fn root_node(&self) -> &Node {
@@ -421,6 +425,10 @@ where
         let path = requested_leaf_node.leaf_key();
         let actual_leaf_node = &path_nodes[0];
 
+        if requested_leaf_node == actual_leaf_node {
+            return Ok(())
+        }
+
         // Build the tree upwards starting with the requested leaf node.
         let mut current_node = requested_leaf_node.clone();
 
@@ -465,6 +473,8 @@ where
                 self.storage
                     .insert(current_node.hash(), &current_node.as_ref().into())?;
             }
+        } else {
+            self.storage.remove(actual_leaf_node.hash())?;
         }
 
         // Merge side nodes
@@ -474,6 +484,10 @@ where
                 .insert(current_node.hash(), &current_node.as_ref().into())?;
         }
 
+        for node in path_nodes.iter().skip(1 /* leaf */) {
+            self.storage.remove(node.hash())?;
+        }
+
         self.set_root_node(current_node);
 
         Ok(())
@@ -924,6 +938,81 @@ mod test {
         assert_eq!(hex::encode(root), expected_root);
     }
 
+    #[test]
+    fn test_update_removes_old_entries() {
+        let mut storage = StorageMap::<TestTable>::new();
+        let mut tree = MerkleTree::new(&mut storage);
+        let tenth_index = 9u32;
+
+        for i in 0_u32..tenth_index {
+            let key = key(i.to_be_bytes());
+            tree.update(key, b"DATA").unwrap();
+        }
+        let size_before_tenth = tree.storage().len();
+        let tenth_key = key(tenth_index.to_be_bytes());
+
+        // Given
+        tree.update(tenth_key, b"DATA").unwrap();
+        let size_after_tenth = tree.storage().len();
+        assert_ne!(size_after_tenth, size_before_tenth);
+
+        // When
+        tree.update(tenth_key, b"ANOTHER_DATA").unwrap();
+
+        // Then
+        assert_eq!(tree.storage().len(), size_after_tenth);
+    }
+
+    #[test]
+    fn test_update_with_the_same_value_does_not_remove_old_entries() {
+        let mut storage = StorageMap::<TestTable>::new();
+        let mut tree = MerkleTree::new(&mut storage);
+        let tenth_index = 9u32;
+
+        for i in 0_u32..tenth_index {
+            let key = key(i.to_be_bytes());
+            tree.update(key, b"DATA").unwrap();
+        }
+        let size_before_tenth = tree.storage().len();
+        let tenth_key = key(tenth_index.to_be_bytes());
+
+        // Given
+        tree.update(tenth_key, b"DATA").unwrap();
+        let size_after_tenth = tree.storage().len();
+        assert_ne!(size_after_tenth, size_before_tenth);
+
+        // When
+        tree.update(tenth_key, b"DATA").unwrap();
+
+        // Then
+        assert_eq!(tree.storage().len(), size_after_tenth);
+    }
+
+    #[test]
+    fn test_delete_removes_path_entries() {
+        let mut storage = StorageMap::<TestTable>::new();
+        let mut tree = MerkleTree::new(&mut storage);
+        let tenth_index = 9u32;
+
+        for i in 0_u32..tenth_index {
+            let key = key(i.to_be_bytes());
+            tree.update(key, b"DATA").unwrap();
+        }
+        let size_before_tenth = tree.storage().len();
+        let tenth_key = key(tenth_index.to_be_bytes());
+
+        // Given
+        tree.update(tenth_key, b"DATA").unwrap();
+        let size_after_tenth = tree.storage().len();
+        assert_ne!(size_after_tenth, size_before_tenth);
+
+        // When
+        tree.delete(tenth_key).unwrap();
+
+        // Then
+        assert_eq!(tree.storage().len(), size_before_tenth);
+    }
+
     #[test]
     fn test_delete_sparse_union() {
         let mut storage = StorageMap::<TestTable>::new();