Merge pull request #3075 from massalabs/fix-executed-ops-extend-xor-c…

…omputing

Fix ExecutedOps extend XOR computing.

massa-final-state/src/executed_ops.rs

@@ -3,17 +3,15 @@
 //! This file defines a structure to list and prune previously executed operations.
 //! Used to detect operation reuse.
 
-use massa_hash::{Hash, HashDeserializer, HashSerializer};
+use massa_hash::{Hash, HashDeserializer, HashSerializer, HASH_SIZE_BYTES};
 use massa_models::{
     error::ModelsError,
     operation::{OperationId, OperationIdDeserializer},
     prehash::PreHashMap,
     slot::{Slot, SlotDeserializer, SlotSerializer},
-    wrapped::Id,
 };
 use massa_serialization::{
-    Deserializer, OptionDeserializer, OptionSerializer, SerializeError, Serializer,
-    U64VarIntDeserializer, U64VarIntSerializer,
+    Deserializer, SerializeError, Serializer, U64VarIntDeserializer, U64VarIntSerializer,
 };
 use nom::{
     error::{context, ContextError, ParseError},
@@ -23,16 +21,32 @@ use nom::{
 };
 use std::ops::Bound::{Excluded, Included};
 
+const EXECUTED_OPS_INITIAL_BYTES: &[u8; 32] = &[0; HASH_SIZE_BYTES];
+
 /// A structure to list and prune previously executed operations
-#[derive(Debug, Default, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ExecutedOps {
     /// Map of the executed operations
     ops: PreHashMap<OperationId, Slot>,
     /// Cumulated hash of the executed operations
-    pub hash: Option<Hash>,
+    pub hash: Hash,
+}
+
+impl Default for ExecutedOps {
+    fn default() -> Self {
+        Self::new()
+    }
 }
 
 impl ExecutedOps {
+    /// Creates a new ExecutedOps
+    pub fn new() -> Self {
+        Self {
+            ops: PreHashMap::default(),
+            hash: Hash::from_bytes(EXECUTED_OPS_INITIAL_BYTES),
+        }
+    }
+
     /// returns the number of executed operations
     pub fn len(&self) -> usize {
         self.ops.len()
@@ -45,13 +59,12 @@ impl ExecutedOps {
 
     /// extends with another `ExecutedOps`
     pub fn extend(&mut self, other: ExecutedOps) {
-        if let Some(other_hash) = other.hash {
-            if let Some(current_hash) = self.hash.as_mut() {
-                *current_hash ^= other_hash;
-            } else {
-                self.hash = Some(other_hash);
+        for (op_id, slot) in other.ops {
+            if self.ops.try_insert(op_id, slot).is_ok() {
+                let hash =
+                    Hash::compute_from(&[&op_id.to_bytes()[..], &slot.to_bytes_key()[..]].concat());
+                self.hash ^= hash;
             }
-            self.ops.extend(other.ops);
         }
     }
 
@@ -62,11 +75,10 @@ impl ExecutedOps {
 
     /// marks an op as executed
     pub fn insert(&mut self, op_id: OperationId, last_valid_slot: Slot) {
-        if let Some(current_hash) = self.hash.as_mut() {
-            *current_hash ^= *op_id.get_hash();
-        } else {
-            self.hash = Some(*op_id.get_hash());
-        }
+        let hash = Hash::compute_from(
+            &[&op_id.to_bytes()[..], &last_valid_slot.to_bytes_key()[..]].concat(),
+        );
+        self.hash ^= hash;
         self.ops.insert(op_id, last_valid_slot);
     }
 
@@ -77,15 +89,10 @@ impl ExecutedOps {
             .ops
             .iter()
             .partition(|(_, &last_valid_slot)| last_valid_slot >= max_slot);
-        if removed.is_empty() {
-            return;
-        }
-        let hash = self
-            .hash
-            .as_mut()
-            .expect("critical: an ExecutedOps object with ops must also contain a hash");
-        for (op_id, _) in removed {
-            *hash ^= *op_id.get_hash();
+        for (op_id, slot) in removed {
+            let hash =
+                Hash::compute_from(&[&op_id.to_bytes()[..], &slot.to_bytes_key()[..]].concat());
+            self.hash ^= hash;
         }
         self.ops = kept;
     }
@@ -142,20 +149,22 @@ impl ExecutedOps {
 
 #[test]
 fn test_executed_ops_xor_computing() {
+    use massa_models::wrapped::Id;
     let mut a = ExecutedOps::default();
     let mut b = ExecutedOps::default();
     let mut c = ExecutedOps::default();
     // initialize the three different objects
     for i in 0u8..20 {
-        if i < 10 {
+        if i < 12 {
             a.insert(
                 OperationId::new(Hash::compute_from(&[i])),
                 Slot {
                     period: i as u64,
                     thread: 0,
                 },
             );
-        } else {
+        }
+        if i > 8 {
             b.insert(
                 OperationId::new(Hash::compute_from(&[i])),
                 Slot {
@@ -182,7 +191,7 @@ fn test_executed_ops_xor_computing() {
         thread: 0,
     });
     // at this point the hash should have been XORed with itself
-    assert_eq!(a.hash, Some(Hash::from_bytes(&[0; 32])));
+    assert_eq!(a.hash, Hash::from_bytes(&[0; 32]));
 }
 
 /// Executed operations bootstrap streaming steps
@@ -281,7 +290,7 @@ impl Deserializer<ExecutedOpsStreamingStep> for ExecutedOpsStreamingStepDeserial
 pub struct ExecutedOpsSerializer {
     slot_serializer: SlotSerializer,
     u64_serializer: U64VarIntSerializer,
-    opt_hash_serializer: OptionSerializer<Hash, HashSerializer>,
+    hash_serializer: HashSerializer,
 }
 
 impl Default for ExecutedOpsSerializer {
@@ -296,7 +305,7 @@ impl ExecutedOpsSerializer {
         ExecutedOpsSerializer {
             slot_serializer: SlotSerializer::new(),
             u64_serializer: U64VarIntSerializer::new(),
-            opt_hash_serializer: OptionSerializer::new(HashSerializer::new()),
+            hash_serializer: HashSerializer::new(),
         }
     }
 }
@@ -316,7 +325,7 @@ impl Serializer<ExecutedOps> for ExecutedOpsSerializer {
         }
 
         // encode the hash
-        self.opt_hash_serializer.serialize(&value.hash, buffer)?;
+        self.hash_serializer.serialize(&value.hash, buffer)?;
         Ok(())
     }
 }
@@ -326,7 +335,7 @@ pub struct ExecutedOpsDeserializer {
     operation_id_deserializer: OperationIdDeserializer,
     slot_deserializer: SlotDeserializer,
     u64_deserializer: U64VarIntDeserializer,
-    opt_hash_deserializer: OptionDeserializer<Hash, HashDeserializer>,
+    hash_deserializer: HashDeserializer,
 }
 
 impl ExecutedOpsDeserializer {
@@ -339,7 +348,7 @@ impl ExecutedOpsDeserializer {
                 (Included(0), Excluded(thread_count)),
             ),
             u64_deserializer: U64VarIntDeserializer::new(Included(u64::MIN), Included(u64::MAX)),
-            opt_hash_deserializer: OptionDeserializer::new(HashDeserializer::new()),
+            hash_deserializer: HashDeserializer::new(),
         }
     }
 }
@@ -365,7 +374,7 @@ impl Deserializer<ExecutedOps> for ExecutedOpsDeserializer {
                     ),
                 ),
                 context("Failed hash deserialization", |input| {
-                    self.opt_hash_deserializer.deserialize(input)
+                    self.hash_deserializer.deserialize(input)
                 }),
             )),
         )

massa-final-state/src/lib.rs

@@ -38,6 +38,7 @@
 #![feature(hash_drain_filter)]
 #![feature(map_first_last)]
 #![feature(async_closure)]
+#![feature(map_try_insert)]
 
 mod config;
 mod error;