Merge branch 'bat/reprot-merklized-commitment' (#3284)

* origin/bat/reprot-merklized-commitment:
  Fix write to storage
  Only initialize a hasher if necessary
  Reworking hash concatenation
  Update crates/core/src/address.rs
  Update crates/core/src/address.rs
  changelog
  [feat]: A commitment to all replay protection entries is now merklized and contributes to the total app hash

.changelog/unreleased/improvements/3284-reprot-merklized-commitment.md

@@ -0,0 +1,3 @@
+- Replay protection entries need to be verifiable and thus should contribute to the app hash. This PR makes
+  a cryptographic commitment to all replay protection entries (the root of some implicit merkle tree) which is itself
+  merklized. ([\#3284](https://github.com/anoma/namada/pull/3284))

crates/core/src/address.rs

@@ -143,6 +143,9 @@ impl From<raw::Address<'_, raw::Validated>> for Address {
             raw::Discriminant::TempStorage => {
                 Address::Internal(InternalAddress::TempStorage)
             }
+            raw::Discriminant::ReplayProtection => {
+                Address::Internal(InternalAddress::ReplayProtection)
+            }
         }
     }
 }
@@ -242,6 +245,13 @@ impl<'addr> From<&'addr Address> for raw::Address<'addr, raw::Validated> {
                     .validate()
                     .expect("This raw address is valid")
             }
+            Address::Internal(InternalAddress::ReplayProtection) => {
+                raw::Address::from_discriminant(
+                    raw::Discriminant::ReplayProtection,
+                )
+                .validate()
+                .expect("This raw address is valid")
+            }
         }
     }
 }
@@ -575,6 +585,8 @@ pub enum InternalAddress {
     Pgf,
     /// Masp
     Masp,
+    /// Replay protection
+    ReplayProtection,
     /// Address with temporary storage is used to pass data from txs to VPs
     /// which is never committed to DB
     TempStorage,
@@ -599,6 +611,7 @@ impl Display for InternalAddress {
                 Self::Multitoken => "Multitoken".to_string(),
                 Self::Pgf => "PublicGoodFundings".to_string(),
                 Self::Masp => "MASP".to_string(),
+                Self::ReplayProtection => "ReplayProtection".to_string(),
                 Self::TempStorage => "TempStorage".to_string(),
             }
         )
@@ -615,6 +628,7 @@ impl InternalAddress {
             "bridgepool" => Some(InternalAddress::EthBridgePool),
             "governance" => Some(InternalAddress::Governance),
             "masp" => Some(InternalAddress::Masp),
+            "replayprotection" => Some(InternalAddress::ReplayProtection),
             _ => None,
         }
     }
@@ -821,6 +835,7 @@ pub mod testing {
             InternalAddress::Pgf => {}
             InternalAddress::Masp => {}
             InternalAddress::Multitoken => {}
+            InternalAddress::ReplayProtection => {}
             InternalAddress::TempStorage => {} /* Add new addresses in the
                                                 * `prop_oneof` below. */
         };
@@ -838,6 +853,7 @@ pub mod testing {
             Just(InternalAddress::Multitoken),
             Just(InternalAddress::Pgf),
             Just(InternalAddress::Masp),
+            Just(InternalAddress::ReplayProtection),
             Just(InternalAddress::TempStorage),
         ]
     }

crates/core/src/address/raw.rs

@@ -67,6 +67,8 @@ pub enum Discriminant {
     Masp = 14,
     /// Temporary storage address.
     TempStorage = 15,
+    /// Replay protection
+    ReplayProtection = 16,
 }
 
 /// Raw address representation.

crates/core/src/hash.rs

@@ -154,6 +154,22 @@ impl Hash {
     pub const fn as_ptr(&self) -> *const u8 {
         self.0.as_ptr()
     }
+
+    /// Given hashes A and B, compute Sha256(A||B),
+    /// but if one value is the zero hash, the other
+    /// value is returned.
+    pub fn concat(self, rhs: &Hash) -> Self {
+        if self.is_zero() {
+            *rhs
+        } else if rhs.is_zero() {
+            self
+        } else {
+            let mut hasher = Sha256::default();
+            hasher.update(self.as_ref());
+            hasher.update(rhs.as_ref());
+            Self(hasher.finalize().into())
+        }
+    }
 }
 
 impl From<Hash> for crate::tendermint::Hash {

crates/namada/src/ledger/protocol/mod.rs

@@ -1021,6 +1021,12 @@ where
                             // Temp storage changes must never be committed
                             Error::AccessForbidden((*internal_addr).clone()),
                         ),
+                        InternalAddress::ReplayProtection => Err(
+                            // Replay protection entries should never be
+                            // written to
+                            // via transactions
+                            Error::AccessForbidden((*internal_addr).clone()),
+                        ),
                     }
                 }
             };

crates/node/src/shell/finalize_block.rs

@@ -2817,6 +2817,15 @@ mod test_finalize_block {
                 .has_key(&wrapper_hash_key)
                 .unwrap()
         );
+
+        // test that a commitment to replay protection gets added.
+        let reprot_key = replay_protection::commitment_key();
+        let reprot_commitment: Hash = shell
+            .state
+            .read(&reprot_key)
+            .expect("Test failed")
+            .expect("Test failed");
+        assert_eq!(wrapper_tx.raw_header_hash(), reprot_commitment);
     }
 
     /// Test that masp anchor keys are added to the merkle tree

crates/replay_protection/src/lib.rs

@@ -17,11 +17,22 @@
     clippy::print_stderr
 )]
 
+use namada_core::address::{Address, InternalAddress};
 use namada_core::hash::Hash;
-use namada_core::storage::Key;
+use namada_core::storage::{DbKeySeg, Key};
 
 const ERROR_MSG: &str = "Cannot obtain a valid db key";
 
+/// Get the key under which we store a hash which is commitment
+/// to all replay protection entries.
+pub fn commitment_key() -> Key {
+    Key::from(DbKeySeg::AddressSeg(Address::Internal(
+        InternalAddress::ReplayProtection,
+    )))
+    .push(&"commitment".to_string())
+    .expect("Should be able to form this key")
+}
+
 /// Get the transaction hash key
 pub fn key(hash: &Hash) -> Key {
     Key::parse(hash.to_string()).expect(ERROR_MSG)

crates/state/src/wl_state.rs

@@ -11,7 +11,9 @@ use namada_events::{EmitEvents, EventToEmit};
 use namada_parameters::EpochDuration;
 use namada_replay_protection as replay_protection;
 use namada_storage::conversion_state::{ConversionState, WithConversionState};
-use namada_storage::{BlockHeight, BlockStateRead, BlockStateWrite, ResultExt};
+use namada_storage::{
+    BlockHeight, BlockStateRead, BlockStateWrite, ResultExt, StorageRead,
+};
 
 use crate::in_memory::InMemory;
 use crate::write_log::{StorageModification, WriteLog};
@@ -227,14 +229,24 @@ where
         // hashes from the previous block to the general bucket
         self.move_current_replay_protection_entries(batch)?;
 
-        for hash in
-            std::mem::take(&mut self.0.write_log.replay_protection).iter()
-        {
-            self.write_replay_protection_entry(
-                batch,
-                &replay_protection::current_key(hash),
-            )?;
-        }
+        let replay_prot_key = replay_protection::commitment_key();
+        let commitment: Hash = self
+            .read(&replay_prot_key)
+            .expect("Could not read db")
+            .unwrap_or_default();
+        let new_commitment =
+            std::mem::take(&mut self.0.write_log.replay_protection)
+                .iter()
+                .try_fold(commitment, |mut acc, hash| {
+                    self.write_replay_protection_entry(
+                        batch,
+                        &replay_protection::current_key(hash),
+                    )?;
+                    acc = acc.concat(hash);
+                    Ok::<_, Error>(acc)
+                })?;
+        self.batch_write_subspace_val(batch, &replay_prot_key, new_commitment)?;
+
         debug_assert!(self.0.write_log.replay_protection.is_empty());
 
         if let Some(address_gen) = self.0.write_log.block_address_gen.take() {