Merge branch 'grarco/masp-no-transfer-dep' (#3232)

.changelog/unreleased/improvements/3232-masp-no-transfer-dep.md

@@ -0,0 +1,2 @@
+- Removed any dependency on the specific transaction data from the masp vp.
+  ([\#3232](https://github.com/anoma/namada/pull/3232))

crates/core/src/masp.rs

@@ -14,6 +14,7 @@ use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 
 use crate::address::{Address, DecodeError, HASH_HEX_LEN, MASP};
+use crate::hash::Hash;
 use crate::impl_display_and_from_str_via_format;
 use crate::storage::Epoch;
 use crate::string_encoding::{
@@ -532,3 +533,21 @@ impl FromStr for MaspValue {
             })
     }
 }
+
+/// The masp transactions' references of a given batch
+#[derive(Default, Clone, Serialize, Deserialize)]
+pub struct MaspTxRefs(pub Vec<Hash>);
+
+impl Display for MaspTxRefs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", serde_json::to_string(self).unwrap())
+    }
+}
+
+impl FromStr for MaspTxRefs {
+    type Err = serde_json::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        serde_json::from_str(s)
+    }
+}

crates/events/src/extend.rs

@@ -7,6 +7,7 @@ use std::str::FromStr;
 
 use namada_core::collections::HashMap;
 use namada_core::hash::Hash;
+use namada_core::masp::MaspTxRefs;
 use namada_core::storage::{BlockHeight, TxIndex};
 
 use super::*;
@@ -498,28 +499,13 @@ impl EventAttributeEntry<'static> for MaspTxBlockIndex {
     }
 }
 
-/// A displyable collection of hashes.
-#[derive(Serialize, Deserialize)]
-pub struct DisplayableHashVec(Vec<Hash>);
-
-impl Display for DisplayableHashVec {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", serde_json::to_string(self).unwrap())
-    }
-}
-
-impl From<Vec<Hash>> for DisplayableHashVec {
-    fn from(value: Vec<Hash>) -> Self {
-        Self(value)
-    }
-}
-
 /// Extend an [`Event`] with `masp_tx_batch_refs` data, indicating the specific
-/// inner transactions inside the batch that are valid masp txs.
-pub struct MaspTxBatchRefs(pub DisplayableHashVec);
+/// inner transactions inside the batch that are valid masp txs and the
+/// references to the relative masp sections.
+pub struct MaspTxBatchRefs(pub MaspTxRefs);
 
 impl EventAttributeEntry<'static> for MaspTxBatchRefs {
-    type Value = DisplayableHashVec;
+    type Value = MaspTxRefs;
     type ValueOwned = Self::Value;
 
     const KEY: &'static str = "masp_tx_batch_refs";

crates/namada/src/ledger/native_vp/masp.rs

@@ -350,7 +350,25 @@ where
     ) -> Result<()> {
         let epoch = self.ctx.get_block_epoch()?;
         let conversion_state = self.ctx.state.in_mem().get_conversion_state();
-        let shielded_tx = self.ctx.get_shielded_action(tx_data)?;
+
+        // Get the Transaction object from the actions
+        let masp_section_ref = namada_tx::action::get_masp_section_ref(
+            &self.ctx,
+        )?
+        .ok_or_else(|| {
+            native_vp::Error::new_const(
+                "Missing MASP section reference in action",
+            )
+        })?;
+        let shielded_tx = tx_data
+            .tx
+            .get_section(&masp_section_ref)
+            .and_then(|section| section.masp_tx())
+            .ok_or_else(|| {
+                native_vp::Error::new_const(
+                    "Missing MASP section in transaction",
+                )
+            })?;
 
         if u64::from(self.ctx.get_block_height()?)
             > u64::from(shielded_tx.expiry_height())

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

@@ -18,8 +18,8 @@ use namada_state::StorageWrite;
 use namada_token::event::{TokenEvent, TokenOperation, UserAccount};
 use namada_tx::data::protocol::{ProtocolTx, ProtocolTxType};
 use namada_tx::data::{
-    BatchResults, BatchedTxResult, TxResult, VpStatusFlags, VpsResult,
-    WrapperTx,
+    BatchResults, BatchedTxResult, ExtendedTxResult, TxResult, 
+    VpStatusFlags, VpsResult, WrapperTx,
 };
 use namada_tx::{BatchedTxRef, Tx};
 use namada_vote_ext::EthereumTxData;
@@ -63,8 +63,6 @@ pub enum Error {
     TxRunnerError(vm::wasm::run::Error),
     #[error("{0:?}")]
     ProtocolTxError(#[from] eyre::Error),
-    #[error("Txs must either be encrypted or a decryption of an encrypted tx")]
-    TxTypeError,
     #[error("The atomic batch failed at inner transaction {0}")]
     FailingAtomicBatch(Hash),
     #[error("Gas error: {0}")]
@@ -165,8 +163,9 @@ pub type Result<T> = std::result::Result<T, Error>;
 pub struct DispatchError {
     /// The result of the function call
     pub error: Error,
-    /// The tx result produced. It could produced even in case of an error
-    pub tx_result: Option<TxResult<Error>>,
+    /// The extended tx result produced. It could be produced even in case of
+    /// an error
+    pub tx_result: Option<ExtendedTxResult<Error>>,
 }
 
 impl From<Error> for DispatchError {
@@ -213,7 +212,7 @@ pub fn dispatch_tx<'a, D, H, CA>(
     dispatch_args: DispatchArgs<'a, CA>,
     tx_gas_meter: &'a RefCell<TxGasMeter>,
     state: &'a mut WlState<D, H>,
-) -> std::result::Result<TxResult<Error>, DispatchError>
+) -> std::result::Result<ExtendedTxResult<Error>, DispatchError>
 where
     D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
     H: 'static + StorageHasher + Sync,
@@ -227,75 +226,30 @@ where
             tx_wasm_cache,
         } => {
             if let Some(mut tx_result) = wrapper_tx_result {
-                // Replay protection check on the batch
-                let tx_hash = tx.raw_header_hash();
-                if state.write_log().has_replay_protection_entry(&tx_hash) {
-                    // If the same batch has already been committed in
-                    // this block, skip execution and return
-                    return Err(DispatchError {
-                        error: Error::ReplayAttempt(tx_hash),
-                        tx_result: None,
-                    });
-                }
-
                 // TODO(namada#2597): handle masp fee payment in the first inner
                 // tx if necessary
-                for cmt in tx.commitments() {
-                    match apply_wasm_tx(
-                        tx.batch_ref_tx(cmt),
-                        &tx_index,
-                        ShellParams {
-                            tx_gas_meter,
-                            state,
-                            vp_wasm_cache,
-                            tx_wasm_cache,
-                        },
-                    ) {
-                        Err(Error::GasError(ref msg)) => {
-                            // Gas error aborts the execution of the entire
-                            // batch
-                            tx_result.gas_used =
-                                tx_gas_meter.borrow().get_tx_consumed_gas();
-                            tx_result.batch_results.0.insert(
-                                cmt.get_hash(),
-                                Err(Error::GasError(msg.to_owned())),
-                            );
-                            state.write_log_mut().drop_tx();
-                            return Err(DispatchError {
-                                error: Error::GasError(msg.to_owned()),
-                                tx_result: Some(tx_result),
-                            });
-                        }
-                        res => {
-                            let is_accepted = matches!(&res, Ok(result) if result.is_accepted());
-
-                            tx_result
-                                .batch_results
-                                .0
-                                .insert(cmt.get_hash(), res);
-                            tx_result.gas_used =
-                                tx_gas_meter.borrow().get_tx_consumed_gas();
-                            if is_accepted {
-                                state.write_log_mut().commit_tx_to_batch();
-                            } else {
-                                state.write_log_mut().drop_tx();
-
-                                if tx.header.atomic {
-                                    // Stop the execution of an atomic batch at
-                                    // the first failed transaction
-                                    return Err(DispatchError {
-                                        error: Error::FailingAtomicBatch(
-                                            cmt.get_hash(),
-                                        ),
-                                        tx_result: Some(tx_result),
-                                    });
-                                }
-                            }
-                        }
-                    };
-                }
+  // Replay protection check on the batch
+            let tx_hash = tx.raw_header_hash();
+            if state.write_log().has_replay_protection_entry(&tx_hash) {
+                // If the same batch has already been committed in
+                // this block, skip execution and return
+                return Err(DispatchError {
+                    error: Error::ReplayAttempt(tx_hash),
+                    tx_result: None,
+                });
+            }
+
+            dispatch_inner_txs(
+                tx,
+                tx_result,
+                tx_index,
+                tx_gas_meter,
+                state,
+                vp_wasm_cache,
+                tx_wasm_cache,
+            )
+
 
-                Ok(tx_result)
             } else {
                 // Governance proposal. We don't allow tx batches in this case,
                 // just take the first one
@@ -318,7 +272,9 @@ where
                             .into_iter()
                             .collect(),
                     ),
-                })
+                }
+                    .to_extended_result(None))
+        )
             }
         }
         DispatchArgs::Protocol(protocol_tx) => {
@@ -334,7 +290,8 @@ where
                         .collect(),
                 ),
                 ..Default::default()
-            })
+            }
+            .to_extended_result(None))
         }
         DispatchArgs::Wrapper {
             wrapper,
@@ -356,6 +313,92 @@ where
     }
 }
 
+fn dispatch_inner_txs<'a, D, H, CA>(
+    tx: Tx,
+    tx_result: TxResult<Error>,
+    tx_index: TxIndex,
+    tx_gas_meter: &'a RefCell<TxGasMeter>,
+    state: &'a mut WlState<D, H>,
+    vp_wasm_cache: &'a mut VpCache<CA>,
+    tx_wasm_cache: &'a mut TxCache<CA>,
+) -> std::result::Result<ExtendedTxResult<Error>, DispatchError>
+where
+    D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
+    H: 'static + StorageHasher + Sync,
+    CA: 'static + WasmCacheAccess + Sync,
+{
+    let mut extended_tx_result = tx_result.to_extended_result(None);
+
+    // TODO(namada#2597): handle masp fee payment in the first inner tx
+    // if necessary
+    for cmt in tx.commitments() {
+        match apply_wasm_tx(
+            tx.batch_ref_tx(cmt),
+            &tx_index,
+            ShellParams {
+                tx_gas_meter,
+                state,
+                vp_wasm_cache,
+                tx_wasm_cache,
+            },
+        ) {
+            Err(Error::GasError(ref msg)) => {
+                // Gas error aborts the execution of the entire batch
+                extended_tx_result.tx_result.gas_used =
+                    tx_gas_meter.borrow().get_tx_consumed_gas();
+                extended_tx_result.tx_result.batch_results.0.insert(
+                    cmt.get_hash(),
+                    Err(Error::GasError(msg.to_owned())),
+                );
+                state.write_log_mut().drop_tx();
+                return Err(DispatchError {
+                    error: Error::GasError(msg.to_owned()),
+                    tx_result: Some(extended_tx_result),
+                });
+            }
+            res => {
+                let is_accepted =
+                    matches!(&res, Ok(result) if result.is_accepted());
+
+                extended_tx_result
+                    .tx_result
+                    .batch_results
+                    .0
+                    .insert(cmt.get_hash(), res);
+                extended_tx_result.tx_result.gas_used =
+                    tx_gas_meter.borrow().get_tx_consumed_gas();
+                if is_accepted {
+                    // If the transaction was a masp one append the
+                    // transaction refs for the events
+                    if let Some(masp_section_ref) =
+                        namada_tx::action::get_masp_section_ref(state)
+                            .map_err(Error::StateError)?
+                    {
+                        extended_tx_result
+                            .masp_tx_refs
+                            .0
+                            .push(masp_section_ref);
+                    }
+                    state.write_log_mut().commit_tx_to_batch();
+                } else {
+                    state.write_log_mut().drop_tx();
+
+                    if tx.header.atomic {
+                        // Stop the execution of an atomic batch at the
+                        // first failed transaction
+                        return Err(DispatchError {
+                            error: Error::FailingAtomicBatch(cmt.get_hash()),
+                            tx_result: Some(extended_tx_result),
+                        });
+                    }
+                }
+            }
+        };
+    }
+
+    Ok(extended_tx_result)
+}
+
 /// Load the wasm hash for a transfer from storage.
 ///
 /// # Panics

crates/node/src/bench_utils.rs

@@ -74,6 +74,7 @@ use namada::ledger::native_vp::ibc::get_dummy_header;
 use namada::ledger::queries::{
     Client, EncodedResponseQuery, RequestCtx, RequestQuery, Router, RPC,
 };
+use namada::masp::MaspTxRefs;
 use namada::state::StorageRead;
 use namada::tx::data::pos::Bond;
 use namada::tx::data::{
@@ -922,12 +923,18 @@ impl Client for BenchShell {
                             .with(MaspTxBlockIndex(TxIndex::must_from_usize(
                                 idx,
                             )))
-                            .with(MaspTxBatchRefs(
-                                vec![
-                                    tx.first_commitments().unwrap().get_hash(),
-                                ]
-                                .into(),
-                            ))
+                            .with(MaspTxBatchRefs(MaspTxRefs(vec![
+                                tx.sections
+                                    .iter()
+                                    .find_map(|section| {
+                                        if let Section::MaspTx(_) = section {
+                                            Some(section.get_hash())
+                                        } else {
+                                            None
+                                        }
+                                    })
+                                    .unwrap(),
+                            ])))
                             .into();
                         namada::tendermint::abci::Event::from(event)
                     })