Merge branch 'grarco/masp-vp-refactor' (#2452)

* origin/grarco/masp-vp-refactor:
  Checked operations in masp vp
  Fixes validation of minted balance key in masp vp
  Changelog #2452
  Fmt
  Fixes benchmarks
  Refactors error in masp vp
  Updates SDK to retrieve changed keys instead of `Transfer` for masp
  Removes wrong comment from masp vp
  Masp vp validation based on changed keys

.changelog/unreleased/SDK/2452-masp-vp-refactor.md

@@ -0,0 +1,3 @@
+- Modified `scan_tx` to require the set of changed keys instead of `Transfer`.
+  `fetch_shielded_transfer` now returns the set of changed keys instead of
+  `Transfer`. ([\#2452](https://github.com/anoma/namada/pull/2452))

.changelog/unreleased/improvements/2452-masp-vp-refactor.md

@@ -0,0 +1,2 @@
+- Modified the MASP VP to validate the changed storage keys instead of the
+  `Transfer` object. ([\#2452](https://github.com/anoma/namada/pull/2452))

crates/apps/src/lib/bench_utils.rs

@@ -1,6 +1,7 @@
 //! Library code for benchmarks provides a wrapper of the ledger's shell
 //! `BenchShell` and helper functions to generate transactions.
 
+use std::collections::BTreeSet;
 use std::fs::{File, OpenOptions};
 use std::io::{Read, Write};
 use std::ops::{Deref, DerefMut};
@@ -59,6 +60,7 @@ use namada::ledger::queries::{
 use namada::state::StorageRead;
 use namada::tendermint_rpc::{self};
 use namada::tx::data::pos::Bond;
+use namada::tx::data::{TxResult, VpsResult};
 use namada::tx::{Code, Data, Section, Signature, Tx};
 use namada::types::address::{self, Address, InternalAddress};
 use namada::types::chain::ChainId;
@@ -122,9 +124,9 @@ static SHELL_INIT: Once = Once::new();
 
 pub struct BenchShell {
     pub inner: Shell,
-    // Cache of the masp transactions in the last block committed, the tx index
-    // coincides with the index in this collection
-    pub last_block_masp_txs: Vec<Tx>,
+    // Cache of the masp transactions and their changed keys in the last block
+    // committed, the tx index coincides with the index in this collection
+    pub last_block_masp_txs: Vec<(Tx, BTreeSet<Key>)>,
     // NOTE: Temporary directory should be dropped last since Shell need to
     // flush data on drop
     tempdir: TempDir,
@@ -554,9 +556,9 @@ impl BenchShell {
             .unwrap();
     }
 
+    // Update the block height in state to guarantee a valid response to the
+    // client queries
     pub fn commit_block(&mut self) {
-        // Update the block height in state to guarantee a valid response to the
-        // client queries
         self.inner
             .wl_storage
             .storage
@@ -568,6 +570,14 @@ impl BenchShell {
 
         self.inner.commit();
     }
+
+    // Commit a masp transaction and cache the tx and the changed keys for
+    // client queries
+    pub fn commit_masp_tx(&mut self, masp_tx: Tx) {
+        self.last_block_masp_txs
+            .push((masp_tx, self.wl_storage.write_log.get_keys()));
+        self.wl_storage.commit_tx();
+    }
 }
 
 pub fn generate_foreign_key_tx(signer: &SecretKey) -> Tx {
@@ -796,7 +806,10 @@ impl Client for BenchShell {
                     evidence_hash: None,
                     proposer_address: tendermint::account::Id::new([0u8; 20]),
                 },
-                last_block_txs.into_iter().map(|tx| tx.to_bytes()).collect(),
+                last_block_txs
+                    .into_iter()
+                    .map(|(tx, _)| tx.to_bytes())
+                    .collect(),
                 tendermint::evidence::List::default(),
                 None,
             )
@@ -827,16 +840,29 @@ impl Client for BenchShell {
                 self.last_block_masp_txs
                     .iter()
                     .enumerate()
-                    .map(|(idx, _tx)| {
+                    .map(|(idx, (_tx, changed_keys))| {
+                        let tx_result = TxResult {
+                            gas_used: 0.into(),
+                            changed_keys: changed_keys.to_owned(),
+                            vps_result: VpsResult::default(),
+                            initialized_accounts: vec![],
+                            ibc_events: BTreeSet::default(),
+                            eth_bridge_events: BTreeSet::default(),
+                        };
                         namada::tendermint::abci::Event {
                             kind: "applied".to_string(),
-                            // Mock only the masp attribute
+                            // Mock the masp and tx attributes
                             attributes: vec![
                                 namada::tendermint::abci::EventAttribute {
                                     key: "is_valid_masp_tx".to_string(),
                                     value: format!("{}", idx),
                                     index: true,
                                 },
+                                namada::tendermint::abci::EventAttribute {
+                                    key: "inner_tx".to_string(),
+                                    value: tx_result.to_string(),
+                                    index: true,
+                                },
                             ],
                         }
                     })

crates/benches/native_vps.rs

@@ -499,7 +499,7 @@ fn setup_storage_for_masp_verification(
         TransferTarget::PaymentAddress(albert_payment_addr),
     );
     shielded_ctx.shell.execute_tx(&shield_tx);
-    shielded_ctx.shell.wl_storage.commit_tx();
+    shielded_ctx.shell.commit_masp_tx(shield_tx);
 
     // Update the anchor in storage
     let tree_key = namada::token::storage_key::masp_commitment_tree_key();
@@ -518,8 +518,6 @@ fn setup_storage_for_masp_verification(
         .write(&anchor_key, ())
         .unwrap();
     shielded_ctx.shell.commit_block();
-    // Cache the masp tx so that it can be returned when queried
-    shielded_ctx.shell.last_block_masp_txs.push(shield_tx);
 
     let (mut shielded_ctx, signed_tx) = match bench_name {
         "shielding" => shielded_ctx.generate_masp_tx(

crates/benches/txs.rs

@@ -86,10 +86,8 @@ fn transfer(c: &mut Criterion) {
                             TransferTarget::PaymentAddress(albert_payment_addr),
                         );
                     shielded_ctx.shell.execute_tx(&shield_tx);
-                    shielded_ctx.shell.wl_storage.commit_tx();
+                    shielded_ctx.shell.commit_masp_tx(shield_tx);
                     shielded_ctx.shell.commit_block();
-                    // Cache the masp tx so that it can be returned when queried
-                    shielded_ctx.shell.last_block_masp_txs.push(shield_tx);
 
                     let (shielded_ctx, signed_tx) = match bench_name {
                         "transparent" => shielded_ctx.generate_masp_tx(