Removes tx hash if invalid section commitment

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

@@ -519,12 +519,16 @@ where
                     );
 
                     // If transaction type is Decrypted and failed because of
-                    // out of gas, remove its hash from storage to allow
-                    // rewrapping it
+                    // out of gas or invalid section commtiment, remove its hash
+                    // from storage to allow rewrapping it
                     if let Some(wrapper) = embedding_wrapper {
-                        if let Error::TxApply(protocol::Error::GasError(_)) =
-                            msg
-                        {
+                        if matches!(
+                            msg,
+                            Error::TxApply(protocol::Error::GasError(_))
+                                | Error::TxApply(
+                                    protocol::Error::MissingSection(_)
+                                )
+                        ) {
                             self.allow_tx_replay(wrapper);
                         }
                     } else if let Some(wrapper) = wrapper {
@@ -2308,9 +2312,9 @@ mod test_finalize_block {
     }
 
     /// Test that if a decrypted transaction fails because of out-of-gas,
-    /// undecryptable or invalid signature, its hash is removed from storage.
-    /// Also checks that a tx failing for other reason has its hash persisted to
-    /// storage.
+    /// undecryptable, invalid signature or wrong section commitment, its hash
+    /// is removed from storage. Also checks that a tx failing for other
+    /// reason has its hash persisted to storage.
     #[test]
     fn test_tx_hash_handling() {
         let (mut shell, _, _, _) = setup();
@@ -2357,17 +2361,21 @@ mod test_finalize_block {
         failing_wrapper.set_data(Data::new(
             "Encrypted transaction data".as_bytes().to_owned(),
         ));
+        let mut wrong_commitment_wrapper = failing_wrapper.clone();
+        wrong_commitment_wrapper.set_code_sechash(Hash::default());
 
         let mut out_of_gas_inner = out_of_gas_wrapper.clone();
         let mut undecryptable_inner = undecryptable_wrapper.clone();
         let mut unsigned_inner = unsigned_wrapper.clone();
+        let mut wrong_commitment_inner = wrong_commitment_wrapper.clone();
         let mut failing_inner = failing_wrapper.clone();
 
         undecryptable_inner
             .update_header(TxType::Decrypted(DecryptedTx::Undecryptable));
         for inner in [
             &mut out_of_gas_inner,
             &mut unsigned_inner,
+            &mut wrong_commitment_inner,
             &mut failing_inner,
         ] {
             inner.update_header(TxType::Decrypted(DecryptedTx::Decrypted));
@@ -2378,6 +2386,7 @@ mod test_finalize_block {
             &out_of_gas_inner,
             &undecryptable_inner,
             &unsigned_inner,
+            &wrong_commitment_inner,
             &failing_inner,
         ] {
             let hash_subkey =
@@ -2396,6 +2405,7 @@ mod test_finalize_block {
             &out_of_gas_inner,
             &undecryptable_inner,
             &unsigned_inner,
+            &wrong_commitment_inner,
             &failing_inner,
         ] {
             processed_txs.push(ProcessedTx {
@@ -2413,6 +2423,10 @@ mod test_finalize_block {
             GAS_LIMIT_MULTIPLIER.into(),
         );
         shell.enqueue_tx(unsigned_wrapper.clone(), u64::MAX.into()); // Prevent out of gas which would still make the test pass
+        shell.enqueue_tx(
+            wrong_commitment_wrapper.clone(),
+            GAS_LIMIT_MULTIPLIER.into(),
+        );
         shell.enqueue_tx(failing_wrapper.clone(), GAS_LIMIT_MULTIPLIER.into());
         // merkle tree root before finalize_block
         let root_pre = shell.shell.wl_storage.storage.block.tree.root();
@@ -2457,51 +2471,35 @@ mod test_finalize_block {
             .expect("Testfailed")
             .as_str();
         assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str());
+        assert_eq!(event[4].event_type.to_string(), String::from("applied"));
+        let code = event[4]
+            .attributes
+            .get("code")
+            .expect("Testfailed")
+            .as_str();
+        assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str());
 
-        assert!(
-            !shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(&out_of_gas_inner.header_hash())
-                .unwrap_or_default()
-        );
-        assert!(
-            shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(&out_of_gas_wrapper.header_hash())
-                .unwrap_or_default()
-        );
-        assert!(
-            !shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(&undecryptable_inner.header_hash())
-                .unwrap_or_default()
-        );
-        assert!(
-            shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(
-                    &undecryptable_wrapper.header_hash()
-                )
-                .unwrap_or_default()
-        );
-        assert!(
-            !shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(&unsigned_inner.header_hash())
-                .unwrap_or_default()
-        );
-        assert!(
-            shell
-                .wl_storage
-                .write_log
-                .has_replay_protection_entry(&unsigned_wrapper.header_hash())
-                .unwrap_or_default()
-        );
+        for (invalid_inner, valid_wrapper) in [
+            (out_of_gas_inner, out_of_gas_wrapper),
+            (undecryptable_inner, undecryptable_wrapper),
+            (unsigned_inner, unsigned_wrapper),
+            (wrong_commitment_inner, wrong_commitment_wrapper),
+        ] {
+            assert!(
+                !shell
+                    .wl_storage
+                    .write_log
+                    .has_replay_protection_entry(&invalid_inner.header_hash())
+                    .unwrap_or_default()
+            );
+            assert!(
+                shell
+                    .wl_storage
+                    .write_log
+                    .has_replay_protection_entry(&valid_wrapper.header_hash())
+                    .unwrap_or_default()
+            );
+        }
         assert!(
             shell
                 .wl_storage

core/src/ledger/tx_env.rs

@@ -64,4 +64,7 @@ pub trait TxEnv: StorageRead + StorageWrite {
         &self,
         event_type: impl AsRef<str>,
     ) -> Result<Vec<IbcEvent>, storage_api::Error>;
+
+    /// Set the sentinel for an invalid section commitment
+    fn set_commitment_sentinel(&mut self);
 }

core/src/types/transaction/mod.rs

@@ -168,6 +168,31 @@ impl TxType {
     }
 }
 
+/// Sentinel used in transactions to signal events that require special
+/// replay protection handling back to the protocol.
+#[derive(Debug, Default)]
+pub enum TxSentinel {
+    /// No action required
+    #[default]
+    None,
+    /// Exceeded gas limit
+    OutOfGas,
+    /// Found invalid commtiment to one of the transaction's sections
+    InvalidCommitment,
+}
+
+impl TxSentinel {
+    /// Set the sentinel for an out of gas error
+    pub fn set_out_of_gas(&mut self) {
+        *self = Self::OutOfGas
+    }
+
+    /// Set the sentinel for an invalid section commitment error
+    pub fn set_invalid_commitment(&mut self) {
+        *self = Self::InvalidCommitment
+    }
+}
+
 #[cfg(test)]
 mod test_process_tx {
     use super::*;

shared/src/ledger/protocol/mod.rs

@@ -42,8 +42,8 @@ use crate::vm::{self, wasm, WasmCacheAccess};
 #[allow(missing_docs)]
 #[derive(Error, Debug)]
 pub enum Error {
-    #[error("Missing wasm code error")]
-    MissingCode,
+    #[error("Missing tx section: {0}")]
+    MissingSection(String),
     #[error("Storage error: {0}")]
     StorageError(crate::ledger::storage::Error),
     #[error("Error decoding a transaction from bytes: {0}")]
@@ -715,6 +715,7 @@ where
     )
     .map_err(|err| match err {
         wasm::run::Error::GasError(msg) => Error::GasError(msg),
+        wasm::run::Error::MissingSection(msg) => Error::MissingSection(msg),
         _ => Error::TxRunnerError(err),
     })
 }

shared/src/vm/host_env.rs

@@ -9,6 +9,7 @@ use borsh_ext::BorshSerializeExt;
 use masp_primitives::transaction::Transaction;
 use namada_core::ledger::gas::{GasMetering, TxGasMeter};
 use namada_core::types::internal::KeyVal;
+use namada_core::types::transaction::TxSentinel;
 use namada_core::types::validity_predicate::VpSentinel;
 use thiserror::Error;
 
@@ -101,8 +102,8 @@ where
     pub iterators: MutHostRef<'a, &'a PrefixIterators<'a, DB>>,
     /// Transaction gas meter.
     pub gas_meter: MutHostRef<'a, &'a TxGasMeter>,
-    /// Out-of-gas sentinel
-    pub out_of_gas: MutHostRef<'a, &'a bool>,
+    /// Transaction sentinel
+    pub sentinel: MutHostRef<'a, &'a TxSentinel>,
     /// The transaction code is used for signature verification
     pub tx: HostRef<'a, &'a Tx>,
     /// The transaction index is used to identify a shielded transaction's
@@ -145,7 +146,7 @@ where
         write_log: &mut WriteLog,
         iterators: &mut PrefixIterators<'a, DB>,
         gas_meter: &mut TxGasMeter,
-        out_of_gas: &mut bool,
+        sentinel: &mut TxSentinel,
         tx: &Tx,
         tx_index: &TxIndex,
         verifiers: &mut BTreeSet<Address>,
@@ -157,7 +158,7 @@ where
         let write_log = unsafe { MutHostRef::new(write_log) };
         let iterators = unsafe { MutHostRef::new(iterators) };
         let gas_meter = unsafe { MutHostRef::new(gas_meter) };
-        let out_of_gas = unsafe { MutHostRef::new(out_of_gas) };
+        let sentinel = unsafe { MutHostRef::new(sentinel) };
         let tx = unsafe { HostRef::new(tx) };
         let tx_index = unsafe { HostRef::new(tx_index) };
         let verifiers = unsafe { MutHostRef::new(verifiers) };
@@ -171,7 +172,7 @@ where
             write_log,
             iterators,
             gas_meter,
-            out_of_gas,
+            sentinel,
             tx,
             tx_index,
             verifiers,
@@ -215,7 +216,7 @@ where
             write_log: self.write_log.clone(),
             iterators: self.iterators.clone(),
             gas_meter: self.gas_meter.clone(),
-            out_of_gas: self.out_of_gas.clone(),
+            sentinel: self.sentinel.clone(),
             tx: self.tx.clone(),
             tx_index: self.tx_index.clone(),
             verifiers: self.verifiers.clone(),
@@ -488,8 +489,8 @@ where
     let gas_meter = unsafe { env.ctx.gas_meter.get() };
     // if we run out of gas, we need to stop the execution
     gas_meter.consume(used_gas).map_err(|err| {
-        let sentinel = unsafe { env.ctx.out_of_gas.get() };
-        *sentinel = true;
+        let sentinel = unsafe { env.ctx.sentinel.get() };
+        sentinel.set_out_of_gas();
         tracing::info!(
             "Stopping transaction execution because of gas error: {}",
             err
@@ -2005,8 +2006,11 @@ where
 
     use namada_core::ledger::ibc::{IbcActions, TransferModule};
 
-    let tx_data = unsafe { env.ctx.tx.get().data() }
-        .ok_or(TxRuntimeError::MissingTxData)?;
+    let tx_data = unsafe { env.ctx.tx.get().data() }.ok_or_else(|| {
+        let sentinel = unsafe { env.ctx.sentinel.get() };
+        sentinel.set_invalid_commitment();
+        TxRuntimeError::MissingTxData
+    })?;
     let ctx = Rc::new(RefCell::new(env.ctx.clone()));
     let mut actions = IbcActions::new(ctx.clone());
     let module = TransferModule::new(ctx);
@@ -2048,6 +2052,18 @@ where
     Ok(())
 }
 
+/// Set the sentinel for an invalid tx section commitment
+pub fn tx_set_commitment_sentinel<MEM, DB, H, CA>(env: &TxVmEnv<MEM, DB, H, CA>)
+where
+    MEM: VmMemory,
+    DB: storage::DB + for<'iter> storage::DBIter<'iter>,
+    H: StorageHasher,
+    CA: WasmCacheAccess,
+{
+    let sentinel = unsafe { env.ctx.sentinel.get() };
+    sentinel.set_invalid_commitment();
+}
+
 /// Evaluate a validity predicate with the given input data.
 pub fn vp_eval<MEM, DB, H, EVAL, CA>(
     env: &VpVmEnv<'static, MEM, DB, H, EVAL, CA>,
@@ -2574,7 +2590,7 @@ pub mod testing {
         iterators: &mut PrefixIterators<'static, DB>,
         verifiers: &mut BTreeSet<Address>,
         gas_meter: &mut TxGasMeter,
-        out_of_gas: &mut bool,
+        sentinel: &mut TxSentinel,
         tx: &Tx,
         tx_index: &TxIndex,
         result_buffer: &mut Option<Vec<u8>>,
@@ -2592,7 +2608,7 @@ pub mod testing {
             write_log,
             iterators,
             gas_meter,
-            out_of_gas,
+            sentinel,
             tx,
             tx_index,
             verifiers,

shared/src/vm/wasm/host_env.rs

@@ -60,7 +60,7 @@ where
         // default namespace
         "env" => {
             "memory" => initial_memory,
-            // Wasm middleware gas injectiong hook
+            // Wasm middleware gas injection hook
             "gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_charge_gas),
             // Whitelisted gas exposed function, we need two different functions just because of colliding names in the vm_host_env macro to generate implementations
             "namada_tx_charge_gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_charge_gas),
@@ -86,6 +86,7 @@ where
             "namada_tx_get_native_token" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_native_token),
             "namada_tx_log_string" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_log_string),
             "namada_tx_ibc_execute" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_ibc_execute),
+            "namada_tx_set_commitment_sentinel" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_set_commitment_sentinel)
         },
     }
 }
@@ -107,7 +108,7 @@ where
         // default namespace
         "env" => {
             "memory" => initial_memory,
-            // Wasm middleware gas injectiong hook
+            // Wasm middleware gas injection hook
             "gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_charge_gas),
             // Whitelisted gas exposed function, we need two different functions just because of colliding names in the vm_host_env macro to generate implementations
             "namada_vp_charge_gas" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_charge_gas),