Removes ReProtStorageModification

crates/apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -452,7 +452,7 @@ where
                             let header_hash = replay_protection_hashes
                                 .expect("This cannot fail")
                                 .header_hash;
-                            self.state.redundant_tx_hash(header_hash).expect(
+                            self.state.redundant_tx_hash(&header_hash).expect(
                                 "Error while marking tx hash as redundant",
                             );
                         }
@@ -619,7 +619,7 @@ where
                 .expect("Error while writing tx hash to storage");
 
             self.state
-                .redundant_tx_hash(header_hash)
+                .redundant_tx_hash(&header_hash)
                 .expect("Error while marking tx hash as redundant");
         }
     }
@@ -2516,7 +2516,7 @@ mod test_finalize_block {
                     .has_replay_protection_entry(&wrapper.raw_header_hash())
             );
             assert!(
-                shell
+                !shell
                     .state
                     .write_log()
                     .has_replay_protection_entry(&wrapper.header_hash())
@@ -2812,7 +2812,7 @@ mod test_finalize_block {
             )
         );
         assert!(
-            shell
+            !shell
                 .state
                 .write_log()
                 .has_replay_protection_entry(&failing_wrapper.header_hash())

crates/state/src/wl_state.rs

@@ -12,9 +12,7 @@ use namada_storage::conversion_state::{ConversionState, WithConversionState};
 use namada_storage::{BlockHeight, BlockStateRead, BlockStateWrite, ResultExt};
 
 use crate::in_memory::InMemory;
-use crate::write_log::{
-    ReProtStorageModification, StorageModification, WriteLog,
-};
+use crate::write_log::{StorageModification, WriteLog};
 use crate::{
     is_pending_transfer_key, DBIter, Epoch, Error, Hash, Key, LastBlock,
     MembershipProof, MerkleTree, MerkleTreeError, ProofOps, Result, State,
@@ -219,18 +217,13 @@ where
         // hashes from the previous block to the general bucket
         self.move_current_replay_protection_entries(batch)?;
 
-        for (hash, entry) in
-            std::mem::take(&mut self.0.write_log.replay_protection).into_iter()
+        for hash in
+            std::mem::take(&mut self.0.write_log.replay_protection).iter()
         {
-            match entry {
-                ReProtStorageModification::Write => self
-                    .write_replay_protection_entry(
-                        batch,
-                        &replay_protection::current_key(&hash),
-                    )?,
-                // Redundant hashes should not be committed to storage
-                ReProtStorageModification::Redundant => (),
-            }
+            self.write_replay_protection_entry(
+                batch,
+                &replay_protection::current_key(hash),
+            )?;
         }
         debug_assert!(self.0.write_log.replay_protection.is_empty());
 
@@ -603,7 +596,7 @@ where
     /// it to storage.
     pub fn redundant_tx_hash(
         &mut self,
-        hash: Hash,
+        hash: &Hash,
     ) -> crate::write_log::Result<()> {
         self.write_log.redundant_tx_hash(hash)
     }

crates/state/src/write_log.rs

@@ -63,21 +63,6 @@ pub enum StorageModification {
     },
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-/// A replay protection storage modification
-pub(crate) enum ReProtStorageModification {
-    /// Write an entry
-    Write,
-    /// Mark an entry as redundant, another hash is already preventing the
-    /// execution of this one. Redundant hashes should not be committed to
-    /// storage
-    // FIXME: only wrapper can be deleted, should I find a way to make it
-    // clearer?
-    // FIXME: manual test rollback on local devnet
-    // FIXME: manual test replay attack on local devnet
-    Redundant,
-}
-
 /// The write log storage
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct WriteLog {
@@ -102,7 +87,7 @@ pub struct WriteLog {
     pub(crate) ibc_events: BTreeSet<IbcEvent>,
     /// Storage modifications for the replay protection storage, always
     /// committed regardless of the result of the transaction
-    pub(crate) replay_protection: HashMap<Hash, ReProtStorageModification>,
+    pub(crate) replay_protection: HashSet<Hash>,
 }
 
 /// Write log prefix iterator
@@ -129,7 +114,7 @@ impl Default for WriteLog {
             tx_write_log: HashMap::with_capacity(100),
             tx_precommit_write_log: HashMap::with_capacity(100),
             ibc_events: BTreeSet::new(),
-            replay_protection: HashMap::with_capacity(1_000),
+            replay_protection: HashSet::with_capacity(1_000),
         }
     }
 }
@@ -588,43 +573,31 @@ impl WriteLog {
 
     /// Check if the given tx hash has already been processed
     pub fn has_replay_protection_entry(&self, hash: &Hash) -> bool {
-        self.replay_protection.contains_key(hash)
+        self.replay_protection.contains(hash)
     }
 
     /// Write the transaction hash
     pub fn write_tx_hash(&mut self, hash: Hash) -> Result<()> {
-        if self
-            .replay_protection
-            .insert(hash, ReProtStorageModification::Write)
-            .is_some()
-        {
-            // Cannot write an hash if other requests have already been
-            // committed for the same hash
+        if !self.replay_protection.insert(hash) {
+            // Cannot write an hash if it's already present in the set
             return Err(Error::ReplayProtection(format!(
-                "Requested a write on hash {hash} over a previous request"
+                "Requested a write of hash {hash} which has already been \
+                 processed"
             )));
         }
 
         Ok(())
     }
 
-    /// Mark the transaction hash as redundant
-    pub(crate) fn redundant_tx_hash(&mut self, hash: Hash) -> Result<()> {
-        match self
-            .replay_protection
-            .insert(hash, ReProtStorageModification::Redundant)
-        {
-            Some(ReProtStorageModification::Write) => Ok(()),
-            Some(ReProtStorageModification::Redundant) => {
-                Err(Error::ReplayProtection(format!(
-                    "Requested a redundant modification on hash {hash} over a \
-                     previous redundant"
-                )))
-            }
-            None => Err(Error::ReplayProtection(format!(
-                "Requested a redundant modification on unknown hash {hash}"
-            ))),
+    /// Remove the transaction hash because redundant
+    pub(crate) fn redundant_tx_hash(&mut self, hash: &Hash) -> Result<()> {
+        if !self.replay_protection.remove(hash) {
+            return Err(Error::ReplayProtection(format!(
+                "Requested a redundant modification on hash {hash} which is \
+                 unknown"
+            )));
         }
+        Ok(())
     }
 }
 
@@ -897,7 +870,7 @@ mod tests {
 
             // Mark one hash as redundant
             write_log
-                .redundant_tx_hash(Hash::sha256("tx4".as_bytes()))
+                .redundant_tx_hash(&Hash::sha256("tx4".as_bytes()))
                 .unwrap();
         }
 
@@ -922,27 +895,17 @@ mod tests {
             write_log
                 .write_tx_hash(Hash::sha256("tx7".as_bytes()))
                 .unwrap();
-            // Mark one hash as redundant
-            write_log
-                .redundant_tx_hash(Hash::sha256("tx7".as_bytes()))
-                .unwrap();
-            // Try to rewrite the same hash and check that it fails
-            assert!(
-                write_log
-                    .write_tx_hash(Hash::sha256("tx7".as_bytes()))
-                    .is_err()
-            );
 
             // mark as redundant a missing hash and check that it fails
             assert!(
                 state
                     .write_log
-                    .redundant_tx_hash(Hash::sha256("tx8".as_bytes()))
+                    .redundant_tx_hash(&Hash::sha256("tx8".as_bytes()))
                     .is_err()
             );
 
-            // Do not assert the state of replay protections because these
-            // errors will actually trigger a shut down of the node. Also, since
+            // Do not assert the state of replay protection because this
+            // error will actually trigger a shut down of the node. Also, since
             // we write the values before validating them, the state would be
             // wrong
         }