Merge pull request #2348 from AleoHQ/feat/cleanup-finalize-operations

Cleanup `UpdateKeyValue` and `RemoveKeyValue` finalize operations
AleoNet · Feb 12, 2024 · 1bfe459 · 1bfe459
2 parents d1f4e17 + e231cb7
commit 1bfe459
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 71 deletions.
diff --git a/ledger/block/src/transactions/confirmed/mod.rs b/ledger/block/src/transactions/confirmed/mod.rs
@@ -356,12 +356,7 @@ pub mod test_helpers {
             true => vec![FinalizeOperation::InitializeMapping(Uniform::rand(rng))],
             false => vec![
                 FinalizeOperation::InitializeMapping(Uniform::rand(rng)),
-                FinalizeOperation::UpdateKeyValue(
-                    Uniform::rand(rng),
-                    Uniform::rand(rng),
-                    Uniform::rand(rng),
-                    Uniform::rand(rng),
-                ),
+                FinalizeOperation::UpdateKeyValue(Uniform::rand(rng), Uniform::rand(rng), Uniform::rand(rng)),
             ],
         };
 
@@ -461,12 +456,7 @@ mod test {
         // Create an `AcceptedExecution` with valid `FinalizeOperation`s.
         let finalize_operations = vec![
             FinalizeOperation::InsertKeyValue(Uniform::rand(rng), Uniform::rand(rng), Uniform::rand(rng)),
-            FinalizeOperation::UpdateKeyValue(
-                Uniform::rand(rng),
-                Uniform::rand(rng),
-                Uniform::rand(rng),
-                Uniform::rand(rng),
-            ),
+            FinalizeOperation::UpdateKeyValue(Uniform::rand(rng), Uniform::rand(rng), Uniform::rand(rng)),
             FinalizeOperation::RemoveKeyValue(Uniform::rand(rng), Uniform::rand(rng)),
         ];
         let confirmed = ConfirmedTransaction::accepted_execute(index, tx.clone(), finalize_operations.clone()).unwrap();

diff --git a/ledger/store/src/program/finalize.rs b/ledger/store/src/program/finalize.rs
@@ -244,7 +244,7 @@ pub trait FinalizeStorage<N: Network>: 'static + Clone + Send + Sync {
         })?;
 
         // Return the finalize operation.
-        Ok(FinalizeOperation::UpdateKeyValue(to_mapping_id(&program_id, &mapping_name)?, 0u64, key_id, value_id))
+        Ok(FinalizeOperation::UpdateKeyValue(to_mapping_id(&program_id, &mapping_name)?, key_id, value_id))
     }
 
     /// Removes the key-value pair for the given `program ID`, `mapping name`, and `key` from storage.
@@ -264,6 +264,9 @@ pub trait FinalizeStorage<N: Network>: 'static + Clone + Send + Sync {
             return Ok(None);
         }
 
+        // Compute the key ID.
+        let key_id = to_key_id(&program_id, &mapping_name, key)?;
+
         atomic_batch_scope!(self, {
             // Update the key-value map with the new key.
             self.key_value_map().remove_key(&(program_id, mapping_name), key)?;
@@ -272,7 +275,7 @@ pub trait FinalizeStorage<N: Network>: 'static + Clone + Send + Sync {
         })?;
 
         // Return the finalize operation.
-        Ok(Some(FinalizeOperation::RemoveKeyValue(to_mapping_id(&program_id, &mapping_name)?, 0u64)))
+        Ok(Some(FinalizeOperation::RemoveKeyValue(to_mapping_id(&program_id, &mapping_name)?, key_id)))
     }
 
     /// Replaces the mapping for the given `program ID` and `mapping name` from storage,

diff --git a/parameters/src/mainnet/genesis.rs b/parameters/src/mainnet/genesis.rs
@@ -27,6 +27,6 @@ mod tests {
     #[test]
     fn test_genesis_block() {
         let bytes = GenesisBytes::load_bytes();
-        assert_eq!(13989, bytes.len() as u64, "Update me if serialization has changed");
+        assert_eq!(13925, bytes.len() as u64, "Update me if serialization has changed");
     }
 }
diff --git a/parameters/src/mainnet/resources/block.genesis b/parameters/src/mainnet/resources/block.genesis
diff --git a/synthesizer/program/src/logic/finalize_operation/bits.rs b/synthesizer/program/src/logic/finalize_operation/bits.rs
@@ -52,22 +52,20 @@ impl<N: Network> FromBits for FinalizeOperation<N> {
             2 => {
                 // Read the mapping ID.
                 let mapping_id = Field::from_bits_le(&next_bits(Field::<N>::size_in_bits())?)?;
-                // Read the index.
-                let index = u64::from_bits_le(&next_bits(64)?)?;
                 // Read the key ID.
                 let key_id = Field::from_bits_le(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Read the value ID.
                 let value_id = Field::from_bits_le(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Return the finalize operation.
-                Ok(Self::UpdateKeyValue(mapping_id, index, key_id, value_id))
+                Ok(Self::UpdateKeyValue(mapping_id, key_id, value_id))
             }
             3 => {
                 // Read the mapping ID.
                 let mapping_id = Field::from_bits_le(&next_bits(Field::<N>::size_in_bits())?)?;
-                // Read the index.
-                let index = u64::from_bits_le(&next_bits(64)?)?;
+                // Read the key ID.
+                let key_id = Field::from_bits_le(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Return the finalize operation.
-                Ok(Self::RemoveKeyValue(mapping_id, index))
+                Ok(Self::RemoveKeyValue(mapping_id, key_id))
             }
             4 => {
                 // Read the mapping ID.
@@ -122,22 +120,20 @@ impl<N: Network> FromBits for FinalizeOperation<N> {
             2 => {
                 // Read the mapping ID.
                 let mapping_id = Field::from_bits_be(&next_bits(Field::<N>::size_in_bits())?)?;
-                // Read the index.
-                let index = u64::from_bits_be(&next_bits(64)?)?;
                 // Read the key ID.
                 let key_id = Field::from_bits_be(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Read the value ID.
                 let value_id = Field::from_bits_be(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Return the finalize operation.
-                Ok(Self::UpdateKeyValue(mapping_id, index, key_id, value_id))
+                Ok(Self::UpdateKeyValue(mapping_id, key_id, value_id))
             }
             3 => {
                 // Read the mapping ID.
                 let mapping_id = Field::from_bits_be(&next_bits(Field::<N>::size_in_bits())?)?;
-                // Read the index.
-                let index = u64::from_bits_be(&next_bits(64)?)?;
+                // Read the key ID.
+                let key_id = Field::from_bits_be(&next_bits(Field::<N>::size_in_bits())?)?;
                 // Return the finalize operation.
-                Ok(Self::RemoveKeyValue(mapping_id, index))
+                Ok(Self::RemoveKeyValue(mapping_id, key_id))
             }
             4 => {
                 // Read the mapping ID.
@@ -176,25 +172,23 @@ impl<N: Network> ToBits for FinalizeOperation<N> {
                 // Write the value ID.
                 value_id.write_bits_le(vec);
             }
-            Self::UpdateKeyValue(mapping_id, index, key_id, value_id) => {
+            Self::UpdateKeyValue(mapping_id, key_id, value_id) => {
                 // Write the variant.
                 2u8.write_bits_le(vec);
                 // Write the mapping ID.
                 mapping_id.write_bits_le(vec);
-                // Write the index.
-                index.write_bits_le(vec);
                 // Write the key ID.
                 key_id.write_bits_le(vec);
                 // Write the value ID.
                 value_id.write_bits_le(vec);
             }
-            Self::RemoveKeyValue(mapping_id, index) => {
+            Self::RemoveKeyValue(mapping_id, key_id) => {
                 // Write the variant.
                 3u8.write_bits_le(vec);
                 // Write the mapping ID.
                 mapping_id.write_bits_le(vec);
-                // Write the index.
-                index.write_bits_le(vec);
+                // Write the key ID.
+                key_id.write_bits_le(vec);
             }
             Self::ReplaceMapping(mapping_id) => {
                 // Write the variant.
@@ -230,25 +224,23 @@ impl<N: Network> ToBits for FinalizeOperation<N> {
                 // Write the value ID.
                 value_id.write_bits_be(vec);
             }
-            Self::UpdateKeyValue(mapping_id, index, key_id, value_id) => {
+            Self::UpdateKeyValue(mapping_id, key_id, value_id) => {
                 // Write the variant.
                 2u8.write_bits_be(vec);
                 // Write the mapping ID.
                 mapping_id.write_bits_be(vec);
-                // Write the index.
-                index.write_bits_be(vec);
                 // Write the key ID.
                 key_id.write_bits_be(vec);
                 // Write the value ID.
                 value_id.write_bits_be(vec);
             }
-            Self::RemoveKeyValue(mapping_id, index) => {
+            Self::RemoveKeyValue(mapping_id, key_id) => {
                 // Write the variant.
                 3u8.write_bits_be(vec);
                 // Write the mapping ID.
                 mapping_id.write_bits_be(vec);
-                // Write the index.
-                index.write_bits_be(vec);
+                // Write the key ID.
+                key_id.write_bits_be(vec);
             }
             Self::ReplaceMapping(mapping_id) => {
                 // Write the variant.

diff --git a/synthesizer/program/src/logic/finalize_operation/bytes.rs b/synthesizer/program/src/logic/finalize_operation/bytes.rs
@@ -40,22 +40,20 @@ impl<N: Network> FromBytes for FinalizeOperation<N> {
             2 => {
                 // Read the mapping ID.
                 let mapping_id = Field::read_le(&mut reader)?;
-                // Read the index.
-                let index = u64::read_le(&mut reader)?;
                 // Read the key ID.
                 let key_id = Field::read_le(&mut reader)?;
                 // Read the value ID.
                 let value_id = Field::read_le(&mut reader)?;
                 // Return the finalize operation.
-                Ok(Self::UpdateKeyValue(mapping_id, index, key_id, value_id))
+                Ok(Self::UpdateKeyValue(mapping_id, key_id, value_id))
             }
             3 => {
                 // Read the mapping ID.
                 let mapping_id = Field::read_le(&mut reader)?;
-                // Read the index.
-                let index = u64::read_le(&mut reader)?;
+                // Read the key ID.
+                let key_id = Field::read_le(&mut reader)?;
                 // Return the finalize operation.
-                Ok(Self::RemoveKeyValue(mapping_id, index))
+                Ok(Self::RemoveKeyValue(mapping_id, key_id))
             }
             4 => {
                 // Read the mapping ID.
@@ -94,25 +92,23 @@ impl<N: Network> ToBytes for FinalizeOperation<N> {
                 // Write the value ID.
                 value_id.write_le(&mut writer)?;
             }
-            Self::UpdateKeyValue(mapping_id, index, key_id, value_id) => {
+            Self::UpdateKeyValue(mapping_id, key_id, value_id) => {
                 // Write the variant.
                 2u8.write_le(&mut writer)?;
                 // Write the mapping ID.
                 mapping_id.write_le(&mut writer)?;
-                // Write the index.
-                index.write_le(&mut writer)?;
                 // Write the key ID.
                 key_id.write_le(&mut writer)?;
                 // Write the value ID.
                 value_id.write_le(&mut writer)?;
             }
-            Self::RemoveKeyValue(mapping_id, index) => {
+            Self::RemoveKeyValue(mapping_id, key_id) => {
                 // Write the variant.
                 3u8.write_le(&mut writer)?;
                 // Write the mapping ID.
                 mapping_id.write_le(&mut writer)?;
-                // Write the index.
-                index.write_le(&mut writer)?;
+                // Write the key ID.
+                key_id.write_le(&mut writer)?;
             }
             Self::ReplaceMapping(mapping_id) => {
                 // Write the variant.

diff --git a/synthesizer/program/src/logic/finalize_operation/mod.rs b/synthesizer/program/src/logic/finalize_operation/mod.rs
@@ -27,12 +27,12 @@ pub enum FinalizeOperation<N: Network> {
     /// Inserts a key-value leaf into the mapping tree,
     /// as (`mapping ID`, `key ID`, `value ID`).
     InsertKeyValue(Field<N>, Field<N>, Field<N>),
-    /// Updates the key-value leaf at the given index in the mapping tree,
-    /// as (`mapping ID`, `index`, `key ID`, `value ID`).
-    UpdateKeyValue(Field<N>, u64, Field<N>, Field<N>),
-    /// Removes the key-value leaf at the given index in the mapping tree,
-    /// as (`mapping ID`, `index`).
-    RemoveKeyValue(Field<N>, u64),
+    /// Updates the key-value leaf in the mapping tree,
+    /// as (`mapping ID`, `key ID`, `value ID`).
+    UpdateKeyValue(Field<N>, Field<N>, Field<N>),
+    /// Removes the key-value leaf in the mapping tree,
+    /// as (`mapping ID`, `key ID`).
+    RemoveKeyValue(Field<N>, Field<N>),
     /// Replaces a mapping from the program tree, as (`mapping ID`).
     ReplaceMapping(Field<N>),
     /// Removes a mapping from the program tree, as (`mapping ID`).
@@ -58,12 +58,12 @@ pub(crate) mod test_helpers {
 
     /// Samples a random `UpdateKeyValue`.
     pub(crate) fn sample_update_key_value(rng: &mut TestRng) -> FinalizeOperation<CurrentNetwork> {
-        FinalizeOperation::UpdateKeyValue(Uniform::rand(rng), rng.gen(), Uniform::rand(rng), Uniform::rand(rng))
+        FinalizeOperation::UpdateKeyValue(Uniform::rand(rng), Uniform::rand(rng), Uniform::rand(rng))
     }
 
     /// Samples a random `RemoveKeyValue`.
     pub(crate) fn sample_remove_key_value(rng: &mut TestRng) -> FinalizeOperation<CurrentNetwork> {
-        FinalizeOperation::RemoveKeyValue(Uniform::rand(rng), rng.gen())
+        FinalizeOperation::RemoveKeyValue(Uniform::rand(rng), Uniform::rand(rng))
     }
 
     /// Samples a random `ReplaceMapping`.

diff --git a/synthesizer/program/src/logic/finalize_operation/serialize.rs b/synthesizer/program/src/logic/finalize_operation/serialize.rs
@@ -35,20 +35,19 @@ impl<N: Network> Serialize for FinalizeOperation<N> {
                         operation.serialize_field("value_id", value_id)?;
                         operation.end()
                     }
-                    Self::UpdateKeyValue(mapping_id, index, key_id, value_id) => {
-                        let mut operation = serializer.serialize_struct("FinalizeOperation", 5)?;
+                    Self::UpdateKeyValue(mapping_id, key_id, value_id) => {
+                        let mut operation = serializer.serialize_struct("FinalizeOperation", 4)?;
                         operation.serialize_field("type", "update_key_value")?;
                         operation.serialize_field("mapping_id", mapping_id)?;
-                        operation.serialize_field("index", index)?;
                         operation.serialize_field("key_id", key_id)?;
                         operation.serialize_field("value_id", value_id)?;
                         operation.end()
                     }
-                    Self::RemoveKeyValue(mapping_id, index) => {
+                    Self::RemoveKeyValue(mapping_id, key_id) => {
                         let mut operation = serializer.serialize_struct("FinalizeOperation", 3)?;
                         operation.serialize_field("type", "remove_key_value")?;
                         operation.serialize_field("mapping_id", mapping_id)?;
-                        operation.serialize_field("index", index)?;
+                        operation.serialize_field("key_id", key_id)?;
                         operation.end()
                     }
                     Self::ReplaceMapping(mapping_id) => {
@@ -97,22 +96,20 @@ impl<'de, N: Network> Deserialize<'de> for FinalizeOperation<N> {
                     Some("update_key_value") => {
                         // Deserialize the mapping ID.
                         let mapping_id = DeserializeExt::take_from_value::<D>(&mut operation, "mapping_id")?;
-                        // Deserialize the index.
-                        let index = DeserializeExt::take_from_value::<D>(&mut operation, "index")?;
                         // Deserialize the key ID.
                         let key_id = DeserializeExt::take_from_value::<D>(&mut operation, "key_id")?;
                         // Deserialize the value ID.
                         let value_id = DeserializeExt::take_from_value::<D>(&mut operation, "value_id")?;
                         // Return the operation.
-                        Self::UpdateKeyValue(mapping_id, index, key_id, value_id)
+                        Self::UpdateKeyValue(mapping_id, key_id, value_id)
                     }
                     Some("remove_key_value") => {
                         // Deserialize the mapping ID.
                         let mapping_id = DeserializeExt::take_from_value::<D>(&mut operation, "mapping_id")?;
-                        // Deserialize the index.
-                        let index = DeserializeExt::take_from_value::<D>(&mut operation, "index")?;
+                        // Deserialize the key ID.
+                        let key_id = DeserializeExt::take_from_value::<D>(&mut operation, "key_id")?;
                         // Return the operation.
-                        Self::RemoveKeyValue(mapping_id, index)
+                        Self::RemoveKeyValue(mapping_id, key_id)
                     }
                     Some("replace_mapping") => {
                         // Deserialize the mapping ID.

diff --git a/synthesizer/tests/expectations/vm/execute_and_finalize/test_rand.out b/synthesizer/tests/expectations/vm/execute_and_finalize/test_rand.out
@@ -19,7 +19,7 @@ outputs:
     test_rand.aleo/rand_chacha_check:
       outputs:
       - '{"type":"future","id":"5655280628674362392666464396476127281186411758739892961608160465159658100070field","value":"{\n  program_id: test_rand.aleo,\n  function_name: rand_chacha_check,\n  arguments: [\n    0field,\n    false\n  ]\n}"}'
-  speculate: the execution was rejected
+  speculate: the execution was accepted
   add_next_block: succeeded.
 - verified: true
   execute: