Merge branch 'grarco/replay-protection-storage' (#1977)

* origin/grarco/replay-protection-storage:
  Changelog #1977
  Expose replay protection methods from `WlStorage`
  Reworks replay protection check
  Fixes check for replay protection keys
  Writes only one hash at a time for replay protection
  Removes wrapper hash when committed inner tx
  Renames `finalize_tx_hashes`
  Fixes unit tests
  Improves replay protection `WriteLog` API
  New field in `WriteLog` for replay protection changes
  Refactors replay protection helper functions
  Updates `DB` and `DBIter` traits for replay protection
  Removes replay protecion internal address and vp
  Renames replay protection storage key getter
  Removes hacky solution for replay protection merkle tree
  Writes replay protection keys under separate storage root
  Replay protection column family and related methods in `DB` trait and `Storage`

.changelog/unreleased/improvements/1977-replay-protection-storage.md

@@ -0,0 +1,2 @@
+- Reduced the storage consumption of replay protection.
+  ([\#1977](https://github.com/anoma/namada/pull/1977))

apps/src/lib/node/ledger/shell/mod.rs

@@ -42,13 +42,14 @@ use namada::ledger::protocol::{
     apply_wasm_tx, get_fee_unshielding_transaction,
     get_transfer_hash_from_storage, ShellParams,
 };
+use namada::ledger::storage::wl_storage::WriteLogAndStorage;
 use namada::ledger::storage::write_log::WriteLog;
 use namada::ledger::storage::{
     DBIter, Sha256Hasher, Storage, StorageHasher, TempWlStorage, WlStorage, DB,
     EPOCH_SWITCH_BLOCKS_DELAY,
 };
 use namada::ledger::storage_api::{self, StorageRead};
-use namada::ledger::{parameters, pos, protocol, replay_protection};
+use namada::ledger::{parameters, pos, protocol};
 use namada::proof_of_stake::{self, process_slashes, read_pos_params, slash};
 use namada::proto::{self, Section, Tx};
 use namada::types::address::Address;
@@ -63,7 +64,7 @@ use namada::types::transaction::{
     hash_tx, verify_decrypted_correctly, AffineCurve, DecryptedTx,
     EllipticCurve, PairingEngine, TxType, WrapperTx,
 };
-use namada::types::{address, hash, token};
+use namada::types::{address, token};
 use namada::vm::wasm::{TxCache, VpCache};
 use namada::vm::{WasmCacheAccess, WasmCacheRwAccess};
 use namada_sdk::eth_bridge::{EthBridgeQueries, EthereumOracleConfig};
@@ -935,51 +936,40 @@ where
     pub fn replay_protection_checks(
         &self,
         wrapper: &Tx,
-        tx_bytes: &[u8],
         temp_wl_storage: &mut TempWlStorage<D, H>,
     ) -> Result<()> {
-        let inner_tx_hash =
-            wrapper.clone().update_header(TxType::Raw).header_hash();
-        let inner_hash_key =
-            replay_protection::get_replay_protection_key(&inner_tx_hash);
+        let wrapper_hash = wrapper.header_hash();
         if temp_wl_storage
-            .has_key(&inner_hash_key)
-            .expect("Error while checking inner tx hash key in storage")
+            .has_replay_protection_entry(&wrapper_hash)
+            .expect("Error while checking wrapper tx hash key in storage")
         {
             return Err(Error::ReplayAttempt(format!(
-                "Inner transaction hash {} already in storage",
-                &inner_tx_hash,
+                "Wrapper transaction hash {} already in storage",
+                wrapper_hash
             )));
         }
 
-        // Write inner hash to tx WAL
+        // Write wrapper hash to tx WAL
         temp_wl_storage
-            .write_log
-            .write(&inner_hash_key, vec![])
-            .expect("Couldn't write inner transaction hash to write log");
+            .write_tx_hash(wrapper_hash)
+            .map_err(|e| Error::ReplayAttempt(e.to_string()))?;
 
-        let tx =
-            Tx::try_from(tx_bytes).expect("Deserialization shouldn't fail");
-        let wrapper_hash = tx.header_hash();
-        let wrapper_hash_key =
-            replay_protection::get_replay_protection_key(&wrapper_hash);
+        let inner_tx_hash =
+            wrapper.clone().update_header(TxType::Raw).header_hash();
         if temp_wl_storage
-            .has_key(&wrapper_hash_key)
-            .expect("Error while checking wrapper tx hash key in storage")
+            .has_replay_protection_entry(&inner_tx_hash)
+            .expect("Error while checking inner tx hash key in storage")
         {
             return Err(Error::ReplayAttempt(format!(
-                "Wrapper transaction hash {} already in storage",
-                wrapper_hash
+                "Inner transaction hash {} already in storage",
+                &inner_tx_hash,
             )));
         }
 
-        // Write wrapper hash to tx WAL
+        // Write inner hash to tx WAL
         temp_wl_storage
-            .write_log
-            .write(&wrapper_hash_key, vec![])
-            .expect("Couldn't write wrapper tx hash to write log");
-
-        Ok(())
+            .write_tx_hash(inner_tx_hash)
+            .map_err(|e| Error::ReplayAttempt(e.to_string()))
     }
 
     /// If a handle to an Ethereum oracle was provided to the [`Shell`], attempt
@@ -1272,14 +1262,11 @@ where
                 let mut inner_tx = tx;
                 inner_tx.update_header(TxType::Raw);
                 let inner_tx_hash = &inner_tx.header_hash();
-                let inner_hash_key =
-                    replay_protection::get_replay_protection_key(inner_tx_hash);
                 if self
                     .wl_storage
                     .storage
-                    .has_key(&inner_hash_key)
+                    .has_replay_protection_entry(inner_tx_hash)
                     .expect("Error while checking inner tx hash key in storage")
-                    .0
                 {
                     response.code = ErrorCodes::ReplayTx.into();
                     response.log = format!(
@@ -1292,17 +1279,14 @@ where
 
                 let tx = Tx::try_from(tx_bytes)
                     .expect("Deserialization shouldn't fail");
-                let wrapper_hash = hash::Hash(tx.header_hash().0);
-                let wrapper_hash_key =
-                    replay_protection::get_replay_protection_key(&wrapper_hash);
+                let wrapper_hash = &tx.header_hash();
                 if self
                     .wl_storage
                     .storage
-                    .has_key(&wrapper_hash_key)
+                    .has_replay_protection_entry(wrapper_hash)
                     .expect(
                         "Error while checking wrapper tx hash key in storage",
                     )
-                    .0
                 {
                     response.code = ErrorCodes::ReplayTx.into();
                     response.log = format!(
@@ -2512,13 +2496,15 @@ mod shell_tests {
         )));
 
         // Write wrapper hash to storage
+        let mut batch =
+            namada::core::ledger::storage::testing::TestStorage::batch();
         let wrapper_hash = wrapper.header_hash();
         let wrapper_hash_key =
-            replay_protection::get_replay_protection_key(&wrapper_hash);
+            replay_protection::get_replay_protection_last_subkey(&wrapper_hash);
         shell
             .wl_storage
             .storage
-            .write(&wrapper_hash_key, wrapper_hash)
+            .write_replay_protection_entry(&mut batch, &wrapper_hash_key)
             .expect("Test failed");
 
         // Try wrapper tx replay attack
@@ -2554,11 +2540,13 @@ mod shell_tests {
             wrapper.clone().update_header(TxType::Raw).header_hash();
         // Write inner hash in storage
         let inner_hash_key =
-            replay_protection::get_replay_protection_key(&inner_tx_hash);
+            replay_protection::get_replay_protection_last_subkey(
+                &inner_tx_hash,
+            );
         shell
             .wl_storage
             .storage
-            .write(&inner_hash_key, inner_tx_hash)
+            .write_replay_protection_entry(&mut batch, &inner_hash_key)
             .expect("Test failed");
 
         // Try inner tx replay attack

apps/src/lib/node/ledger/shell/prepare_proposal.rs

@@ -245,8 +245,11 @@ where
             let mut tx_gas_meter = TxGasMeter::new(wrapper.gas_limit);
             tx_gas_meter.add_tx_size_gas(tx_bytes).map_err(|_| ())?;
 
-            // Check replay protection
-            self.replay_protection_checks(&tx, tx_bytes, temp_wl_storage)
+            // Check replay protection, safe to do here. Even if the tx is a
+            // replay attempt, we can leave its hashes in the write log since,
+            // having we already checked the signature, no other tx with the
+            // same hash can ba deemed valid
+            self.replay_protection_checks(&tx, temp_wl_storage)
                 .map_err(|_| ())?;
 
             // Check fees
@@ -1184,7 +1187,7 @@ mod test_prepare_proposal {
 
         // Write wrapper hash to storage
         let wrapper_unsigned_hash = wrapper.header_hash();
-        let hash_key = replay_protection::get_replay_protection_key(
+        let hash_key = replay_protection::get_replay_protection_last_key(
             &wrapper_unsigned_hash,
         );
         shell
@@ -1279,8 +1282,9 @@ mod test_prepare_proposal {
             wrapper.clone().update_header(TxType::Raw).header_hash();
 
         // Write inner hash to storage
-        let hash_key =
-            replay_protection::get_replay_protection_key(&inner_unsigned_hash);
+        let hash_key = replay_protection::get_replay_protection_last_key(
+            &inner_unsigned_hash,
+        );
         shell
             .wl_storage
             .storage
@@ -1309,7 +1313,7 @@ mod test_prepare_proposal {
         let (shell, _recv, _, _) = test_utils::setup();
 
         let keypair = crate::wallet::defaults::daewon_keypair();
-        let keypair_2 = crate::wallet::defaults::daewon_keypair();
+        let keypair_2 = crate::wallet::defaults::albert_keypair();
         let mut wrapper =
             Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new(
                 Fee {

apps/src/lib/node/ledger/shell/process_proposal.rs

@@ -878,11 +878,9 @@ where
                     }
                 } else {
                     // Replay protection checks
-                    if let Err(e) = self.replay_protection_checks(
-                        &tx,
-                        tx_bytes,
-                        temp_wl_storage,
-                    ) {
+                    if let Err(e) =
+                        self.replay_protection_checks(&tx, temp_wl_storage)
+                    {
                         return TxResult {
                             code: ErrorCodes::ReplayTx.into(),
                             info: e.to_string(),
@@ -988,6 +986,7 @@ mod test_process_proposal {
 
     #[cfg(feature = "abcipp")]
     use assert_matches::assert_matches;
+    use namada::ledger::replay_protection;
     use namada::ledger::storage_api::StorageWrite;
     use namada::proto::{
         Code, Data, Section, SignableEthMessage, Signature, Signed,
@@ -2131,14 +2130,16 @@ mod test_process_proposal {
         )));
 
         // Write wrapper hash to storage
+        let mut batch =
+            namada::core::ledger::storage::testing::TestStorage::batch();
         let wrapper_unsigned_hash = wrapper.header_hash();
-        let hash_key = replay_protection::get_replay_protection_key(
+        let hash_key = replay_protection::get_replay_protection_last_subkey(
             &wrapper_unsigned_hash,
         );
         shell
             .wl_storage
             .storage
-            .write(&hash_key, vec![])
+            .write_replay_protection_entry(&mut batch, &hash_key)
             .expect("Test failed");
 
         // Run validation
@@ -2221,12 +2222,9 @@ mod test_process_proposal {
                 assert_eq!(
                     response[1].result.info,
                     format!(
-                        "Transaction replay attempt: Inner transaction hash \
+                        "Transaction replay attempt: Wrapper transaction hash \
                          {} already in storage",
-                        wrapper
-                            .clone()
-                            .update_header(TxType::Raw)
-                            .header_hash(),
+                        wrapper.header_hash(),
                     )
                 );
             }
@@ -2264,12 +2262,15 @@ mod test_process_proposal {
             wrapper.clone().update_header(TxType::Raw).header_hash();
 
         // Write inner hash to storage
-        let hash_key =
-            replay_protection::get_replay_protection_key(&inner_unsigned_hash);
+        let mut batch =
+            namada::core::ledger::storage::testing::TestStorage::batch();
+        let hash_key = replay_protection::get_replay_protection_last_subkey(
+            &inner_unsigned_hash,
+        );
         shell
             .wl_storage
             .storage
-            .write(&hash_key, vec![])
+            .write_replay_protection_entry(&mut batch, &hash_key)
             .expect("Test failed");
 
         // Run validation
@@ -2302,7 +2303,7 @@ mod test_process_proposal {
         let (shell, _recv, _, _) = test_utils::setup();
 
         let keypair = crate::wallet::defaults::daewon_keypair();
-        let keypair_2 = crate::wallet::defaults::daewon_keypair();
+        let keypair_2 = crate::wallet::defaults::albert_keypair();
 
         let mut wrapper =
             Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new(