Merge branch 'grarco/spend-description-validation' (#2244)

* origin/grarco/spend-description-validation:
  Changelog #2244
  Updates comment
  Fixes masp vp benchmark
  Fixes commitment tree validation in masp vp. Adds a workaround to update tree from the tx
  Updates the merkle tree anchor only if the tree changed
  Updates commitment tree anchor only once per block
  Refactors masp nullifiers check in a separate function
  Updates masp vp to validate note commitment tree and anchor
  Updates masp tx with note commitment tree and anchor

.changelog/unreleased/bug-fixes/2244-spend-description-validation.md

@@ -0,0 +1,2 @@
+- Updates masp tx to store the notes and the native vp to validate them and the
+  anchors. ([\#2244](https://github.com/anoma/namada/pull/2244))

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

@@ -1,25 +1,34 @@
 //! Implementation of the `FinalizeBlock` ABCI++ method for the Shell
 
 use data_encoding::HEXUPPER;
+use masp_primitives::merkle_tree::CommitmentTree;
+use masp_primitives::sapling::Node;
+use masp_proofs::bls12_381;
 use namada::core::ledger::inflation;
 use namada::core::ledger::masp_conversions::update_allowed_conversions;
 use namada::core::ledger::pgf::ADDRESS as pgf_address;
+use namada::core::types::storage::KeySeg;
 use namada::ledger::events::EventType;
 use namada::ledger::gas::{GasMetering, TxGasMeter};
 use namada::ledger::parameters::storage as params_storage;
 use namada::ledger::pos::{namada_proof_of_stake, staking_token_address};
 use namada::ledger::protocol;
 use namada::ledger::storage::wl_storage::WriteLogAndStorage;
+use namada::ledger::storage::write_log::StorageModification;
 use namada::ledger::storage::EPOCH_SWITCH_BLOCKS_DELAY;
 use namada::ledger::storage_api::token::credit_tokens;
-use namada::ledger::storage_api::{pgf, StorageRead, StorageWrite};
+use namada::ledger::storage_api::{pgf, ResultExt, StorageRead, StorageWrite};
 use namada::proof_of_stake::{
     find_validator_by_raw_hash, read_last_block_proposer_address,
     read_pos_params, read_total_stake, write_last_block_proposer_address,
 };
+use namada::types::address::MASP;
 use namada::types::dec::Dec;
 use namada::types::key::tm_raw_hash_to_string;
 use namada::types::storage::{BlockHash, BlockResults, Epoch, Header};
+use namada::types::token::{
+    MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY,
+};
 use namada::types::transaction::protocol::{
     ethereum_tx_data_variants, ProtocolTxType,
 };
@@ -560,6 +569,25 @@ where
         tracing::info!("{}", stats);
         tracing::info!("{}", stats.format_tx_executed());
 
+        // Update the MASP commitment tree anchor if the tree was updated
+        let tree_key = Key::from(MASP.to_db_key())
+            .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned())
+            .expect("Cannot obtain a storage key");
+        if let Some(StorageModification::Write { value }) =
+            self.wl_storage.write_log.read(&tree_key).0
+        {
+            let updated_tree = CommitmentTree::<Node>::try_from_slice(value)
+                .into_storage_result()?;
+            let anchor_key = Key::from(MASP.to_db_key())
+                .push(&MASP_NOTE_COMMITMENT_ANCHOR_PREFIX.to_owned())
+                .expect("Cannot obtain a storage key")
+                .push(&namada::core::types::hash::Hash(
+                    bls12_381::Scalar::from(updated_tree.root()).to_bytes(),
+                ))
+                .expect("Cannot obtain a storage key");
+            self.wl_storage.write(&anchor_key, ())?;
+        }
+
         if update_for_tendermint {
             self.update_epoch(&mut response);
             // send the latest oracle configs. These may have changed due to

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

@@ -2,6 +2,9 @@
 use std::collections::HashMap;
 use std::hash::Hash;
 
+use masp_primitives::merkle_tree::CommitmentTree;
+use masp_primitives::sapling::Node;
+use masp_proofs::bls12_381;
 use namada::ledger::parameters::Parameters;
 use namada::ledger::storage::traits::StorageHasher;
 use namada::ledger::storage::{DBIter, DB};
@@ -10,9 +13,13 @@ use namada::ledger::storage_api::StorageWrite;
 use namada::ledger::{ibc, pos};
 use namada::proof_of_stake::BecomeValidator;
 use namada::types::address::Address;
+use namada::types::address::MASP;
 use namada::types::hash::Hash as CodeHash;
 use namada::types::key::*;
 use namada::types::time::{DateTimeUtc, TimeZone, Utc};
+use namada::types::token::{
+    MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY,
+};
 use namada::vm::validate_untrusted_wasm;
 use namada_sdk::eth_bridge::EthBridgeStatus;
 use namada_sdk::proof_of_stake::PosParams;
@@ -170,6 +177,27 @@ where
 
         ibc::init_genesis_storage(&mut self.wl_storage);
 
+        // Init masp commitment tree and anchor
+        let empty_commitment_tree: CommitmentTree<Node> =
+            CommitmentTree::empty();
+        let anchor = empty_commitment_tree.root();
+        let note_commitment_tree_key = Key::from(MASP.to_db_key())
+            .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned())
+            .expect("Cannot obtain a storage key");
+        self.wl_storage
+            .write(&note_commitment_tree_key, empty_commitment_tree)
+            .unwrap();
+        let commitment_tree_anchor_key = Key::from(MASP.to_db_key())
+            .push(&MASP_NOTE_COMMITMENT_ANCHOR_PREFIX.to_owned())
+            .expect("Cannot obtain a storage key")
+            .push(&namada::core::types::hash::Hash(
+                bls12_381::Scalar::from(anchor).to_bytes(),
+            ))
+            .expect("Cannot obtain a storage key");
+        self.wl_storage
+            .write(&commitment_tree_anchor_key, ())
+            .unwrap();
+
         // Set the initial validator set
         response.validators = self
             .get_abci_validator_updates(true, |pk, power| {

benches/Cargo.toml

@@ -42,6 +42,8 @@ path = "host_env.rs"
 [dev-dependencies]
 namada = { path = "../shared", features = ["testing"] }
 namada_apps = { path = "../apps", features = ["benches"] }
+masp_primitives.workspace = true
+masp_proofs.workspace = true
 borsh.workspace = true
 borsh-ext.workspace = true
 criterion = { version = "0.5", features = ["html_reports"] }

benches/native_vps.rs

@@ -4,6 +4,8 @@ use std::rc::Rc;
 use std::str::FromStr;
 
 use criterion::{criterion_group, criterion_main, Criterion};
+use masp_primitives::sapling::Node;
+use masp_proofs::bls12_381;
 use namada::core::ledger::governance::storage::proposal::ProposalType;
 use namada::core::ledger::governance::storage::vote::{
     StorageProposalVote, VoteType,
@@ -39,14 +41,18 @@ use namada::ledger::native_vp::{Ctx, NativeVp};
 use namada::ledger::pgf::PgfVp;
 use namada::ledger::pos::PosVP;
 use namada::namada_sdk::masp::verify_shielded_tx;
+use namada::namada_sdk::masp_primitives::merkle_tree::CommitmentTree;
 use namada::namada_sdk::masp_primitives::transaction::Transaction;
 use namada::proof_of_stake;
 use namada::proof_of_stake::KeySeg;
 use namada::proto::{Code, Section, Tx};
-use namada::types::address::InternalAddress;
+use namada::types::address::{InternalAddress, MASP};
 use namada::types::eth_bridge_pool::{GasFee, PendingTransfer};
 use namada::types::masp::{TransferSource, TransferTarget};
 use namada::types::storage::{Epoch, TxIndex};
+use namada::types::token::{
+    MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY,
+};
 use namada::types::transaction::governance::{
     InitProposalData, VoteProposalData,
 };
@@ -501,6 +507,30 @@ fn setup_storage_for_masp_verification(
     );
     shielded_ctx.shell.execute_tx(&shield_tx);
     shielded_ctx.shell.wl_storage.commit_tx();
+
+    // Update the anchor in storage
+    let tree_key = namada::core::types::storage::Key::from(MASP.to_db_key())
+        .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned())
+        .expect("Cannot obtain a storage key");
+    let updated_tree: CommitmentTree<Node> = shielded_ctx
+        .shell
+        .wl_storage
+        .read(&tree_key)
+        .unwrap()
+        .unwrap();
+    let anchor_key = namada::core::types::storage::Key::from(MASP.to_db_key())
+        .push(&MASP_NOTE_COMMITMENT_ANCHOR_PREFIX.to_owned())
+        .expect("Cannot obtain a storage key")
+        .push(&namada::core::types::hash::Hash(
+            bls12_381::Scalar::from(updated_tree.root()).to_bytes(),
+        ))
+        .expect("Cannot obtain a storage key");
+    shielded_ctx
+        .shell
+        .wl_storage
+        .write(&anchor_key, ())
+        .unwrap();
+
     shielded_ctx.shell.commit();
 
     let (mut shielded_ctx, signed_tx) = match bench_name {

core/src/ledger/masp_utils.rs

@@ -1,15 +1,17 @@
 //! MASP utilities
 
+use masp_primitives::merkle_tree::CommitmentTree;
+use masp_primitives::sapling::Node;
 use masp_primitives::transaction::Transaction;
 
 use super::storage_api::{StorageRead, StorageWrite};
-use crate::ledger::storage_api::Result;
+use crate::ledger::storage_api::{Error, Result};
 use crate::types::address::MASP;
 use crate::types::hash::Hash;
 use crate::types::storage::{BlockHeight, Epoch, Key, KeySeg, TxIndex};
 use crate::types::token::{
-    Transfer, HEAD_TX_KEY, MASP_NULLIFIERS_KEY_PREFIX, PIN_KEY_PREFIX,
-    TX_KEY_PREFIX,
+    Transfer, HEAD_TX_KEY, MASP_NOTE_COMMITMENT_TREE_KEY,
+    MASP_NULLIFIERS_KEY_PREFIX, PIN_KEY_PREFIX, TX_KEY_PREFIX,
 };
 
 // Writes the nullifiers of the provided masp transaction to storage
@@ -32,6 +34,40 @@ fn reveal_nullifiers(
     Ok(())
 }
 
+/// Appends the note commitments of the provided transaction to the merkle tree
+/// and updates the anchor
+/// NOTE: this function is public as a temporary workaround because of an issue
+/// when running this function in WASM
+pub fn update_note_commitment_tree(
+    ctx: &mut (impl StorageRead + StorageWrite),
+    transaction: &Transaction,
+) -> Result<()> {
+    if let Some(bundle) = transaction.sapling_bundle() {
+        if !bundle.shielded_outputs.is_empty() {
+            let tree_key = Key::from(MASP.to_db_key())
+                .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned())
+                .expect("Cannot obtain a storage key");
+            let mut commitment_tree: CommitmentTree<Node> =
+                ctx.read(&tree_key)?.ok_or(Error::SimpleMessage(
+                    "Missing note commitment tree in storage",
+                ))?;
+
+            for description in &bundle.shielded_outputs {
+                // Add cmu to the merkle tree
+                commitment_tree
+                    .append(Node::from_scalar(description.cmu))
+                    .map_err(|_| {
+                        Error::SimpleMessage("Note commitment tree is full")
+                    })?;
+            }
+
+            ctx.write(&tree_key, commitment_tree)?;
+        }
+    }
+
+    Ok(())
+}
+
 /// Handle a MASP transaction.
 pub fn handle_masp_tx(
     ctx: &mut (impl StorageRead + StorageWrite),
@@ -59,6 +95,10 @@ pub fn handle_masp_tx(
     );
     ctx.write(&current_tx_key, record)?;
     ctx.write(&head_tx_key, current_tx_idx + 1)?;
+    // TODO: temporarily disabled because of the node aggregation issue in WASM.
+    // Using the host env tx_update_masp_note_commitment_tree or directly the
+    // update_note_commitment_tree function as a  workaround instead
+    // update_note_commitment_tree(ctx, shielded)?;
     reveal_nullifiers(ctx, shielded)?;
 
     // If storage key has been supplied, then pin this transaction to it

core/src/types/token.rs

@@ -984,6 +984,10 @@ pub const TX_KEY_PREFIX: &str = "tx-";
 pub const PIN_KEY_PREFIX: &str = "pin-";
 /// Key segment prefix for the nullifiers
 pub const MASP_NULLIFIERS_KEY_PREFIX: &str = "nullifiers";
+/// Key segment prefix for the note commitment merkle tree
+pub const MASP_NOTE_COMMITMENT_TREE_KEY: &str = "commitment_tree";
+/// Key segment prefix for the note commitment anchor
+pub const MASP_NOTE_COMMITMENT_ANCHOR_PREFIX: &str = "note_commitment_anchor";
 /// Last calculated inflation value handed out
 pub const MASP_LAST_INFLATION_KEY: &str = "last_inflation";
 /// The last locked ratio
@@ -1213,7 +1217,9 @@ pub fn is_masp_key(key: &Key) -> bool {
                 && (key == HEAD_TX_KEY
                     || key.starts_with(TX_KEY_PREFIX)
                     || key.starts_with(PIN_KEY_PREFIX)
-                    || key.starts_with(MASP_NULLIFIERS_KEY_PREFIX)))
+                    || key.starts_with(MASP_NULLIFIERS_KEY_PREFIX)
+                    || key == MASP_NOTE_COMMITMENT_TREE_KEY
+                    || key.starts_with(MASP_NOTE_COMMITMENT_ANCHOR_PREFIX)))
     } else {
         false
     }
@@ -1228,6 +1234,15 @@ pub fn is_masp_nullifier_key(key: &Key) -> bool {
         ] if *addr == MASP && prefix == MASP_NULLIFIERS_KEY_PREFIX)
 }
 
+/// Check if the given storage key is a masp anchor key
+pub fn is_masp_anchor_key(key: &Key) -> bool {
+    matches!(&key.segments[..],
+    [DbKeySeg::AddressSeg(addr),
+             DbKeySeg::StringSeg(prefix),
+             ..
+        ] if *addr == MASP && prefix == MASP_NOTE_COMMITMENT_ANCHOR_PREFIX)
+}
+
 /// Obtain the storage key for the last locked ratio of a token
 pub fn masp_last_locked_ratio_key(token_address: &Address) -> Key {
     key_of_token(

shared/src/ledger/native_vp/ibc/context.rs

@@ -214,7 +214,12 @@ where
     }
 
     fn handle_masp_tx(&mut self, shielded: &IbcShieldedTransfer) -> Result<()> {
-        masp_utils::handle_masp_tx(self, &shielded.transfer, &shielded.masp_tx)
+        masp_utils::handle_masp_tx(
+            self,
+            &shielded.transfer,
+            &shielded.masp_tx,
+        )?;
+        masp_utils::update_note_commitment_tree(self, &shielded.masp_tx)
     }
 
     fn mint_token(