Merge branch 'grarco/convert-description-validation' (#2248)

* origin/grarco/convert-description-validation:
  Stricter checks on sapling bundle components
  Changelog #2248
  Fixes masp key validation
  Fixes conversion anchor handling
  Removes redundant masp dependency from bench crate
  Improves masp vp keys verification
  Masp VP verifies the anchors of convert descriptions
  `update_allowed_conversions` to publish the updated convert anchor

.changelog/unreleased/bug-fixes/2248-convert-description-validation.md

@@ -0,0 +1,2 @@
+- Updates the masp vp to validate the convert description's anchor.
+  ([\#2248](https://github.com/anoma/namada/pull/2248))

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

@@ -18,7 +18,8 @@ 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,
+    MASP_CONVERT_ANCHOR_KEY, MASP_NOTE_COMMITMENT_ANCHOR_PREFIX,
+    MASP_NOTE_COMMITMENT_TREE_KEY,
 };
 use namada::vm::validate_untrusted_wasm;
 use namada_sdk::eth_bridge::EthBridgeStatus;
@@ -198,6 +199,20 @@ where
             .write(&commitment_tree_anchor_key, ())
             .unwrap();
 
+        // Init masp convert anchor
+        let convert_anchor_key = Key::from(MASP.to_db_key())
+            .push(&MASP_CONVERT_ANCHOR_KEY.to_owned())
+            .expect("Cannot obtain a storage key");
+        self.wl_storage.write(
+            &convert_anchor_key,
+            namada::core::types::hash::Hash(
+                bls12_381::Scalar::from(
+                    self.wl_storage.storage.conversion_state.tree.root(),
+                )
+                .to_bytes(),
+            ),
+        )?;
+
         // Set the initial validator set
         response.validators = self
             .get_abci_validator_updates(true, |pk, power| {

benches/Cargo.toml

@@ -43,7 +43,6 @@ path = "host_env.rs"
 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,8 +4,8 @@ use std::rc::Rc;
 use std::str::FromStr;
 
 use criterion::{criterion_group, criterion_main, Criterion};
+use masp_primitives::bls12_381;
 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,

core/src/ledger/masp_conversions.rs

@@ -204,6 +204,7 @@ where
 {
     use std::cmp::Ordering;
 
+    use masp_primitives::bls12_381;
     use masp_primitives::ff::PrimeField;
     use masp_primitives::transaction::components::I128Sum as MaspAmount;
     use rayon::iter::{
@@ -212,6 +213,8 @@ where
     use rayon::prelude::ParallelSlice;
 
     use crate::types::address;
+    use crate::types::storage::{Key, KeySeg};
+    use crate::types::token::MASP_CONVERT_ANCHOR_KEY;
 
     // The derived conversions will be placed in MASP address space
     let masp_addr = MASP;
@@ -456,6 +459,19 @@ where
     // obtained
     wl_storage.storage.conversion_state.tree =
         FrozenCommitmentTree::merge(&tree_parts);
+    // Update the anchor in storage
+    let anchor_key = Key::from(MASP.to_db_key())
+        .push(&MASP_CONVERT_ANCHOR_KEY.to_owned())
+        .expect("Cannot obtain a storage key");
+    wl_storage.write(
+        &anchor_key,
+        crate::types::hash::Hash(
+            bls12_381::Scalar::from(
+                wl_storage.storage.conversion_state.tree.root(),
+            )
+            .to_bytes(),
+        ),
+    )?;
 
     // Add purely decoding entries to the assets map. These will be
     // overwritten before the creation of the next commitment tree

core/src/ledger/masp_utils.rs

@@ -10,8 +10,8 @@ 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_NOTE_COMMITMENT_TREE_KEY,
-    MASP_NULLIFIERS_KEY_PREFIX, PIN_KEY_PREFIX, TX_KEY_PREFIX,
+    Transfer, HEAD_TX_KEY, MASP_NOTE_COMMITMENT_TREE_KEY, MASP_NULLIFIERS_KEY,
+    PIN_KEY_PREFIX, TX_KEY_PREFIX,
 };
 
 // Writes the nullifiers of the provided masp transaction to storage
@@ -24,7 +24,7 @@ fn reveal_nullifiers(
         .map_or(&vec![], |description| &description.shielded_spends)
     {
         let nullifier_key = Key::from(MASP.to_db_key())
-            .push(&MASP_NULLIFIERS_KEY_PREFIX.to_owned())
+            .push(&MASP_NULLIFIERS_KEY.to_owned())
             .expect("Cannot obtain a storage key")
             .push(&Hash(description.nullifier.0))
             .expect("Cannot obtain a storage key");

core/src/types/token.rs

@@ -983,11 +983,13 @@ pub const TX_KEY_PREFIX: &str = "tx-";
 /// Key segment prefix for pinned shielded transactions
 pub const PIN_KEY_PREFIX: &str = "pin-";
 /// Key segment prefix for the nullifiers
-pub const MASP_NULLIFIERS_KEY_PREFIX: &str = "nullifiers";
+pub const MASP_NULLIFIERS_KEY: &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";
+/// Key segment prefix for the convert anchor
+pub const MASP_CONVERT_ANCHOR_KEY: &str = "convert_anchor";
 /// Last calculated inflation value handed out
 pub const MASP_LAST_INFLATION_KEY: &str = "last_inflation";
 /// The last locked ratio
@@ -1210,37 +1212,56 @@ pub fn is_denom_key(token_addr: &Address, key: &Key) -> bool {
 
 /// Check if the given storage key is a masp key
 pub fn is_masp_key(key: &Key) -> bool {
-    if key.segments.len() >= 2 {
-        matches!(&key.segments[..2],
+    matches!(&key.segments[..],
+        [DbKeySeg::AddressSeg(addr), ..] if *addr == MASP
+    )
+}
+
+/// Check if the given storage key is allowed to be touched by a masp transfer
+pub fn is_masp_allowed_key(key: &Key) -> bool {
+    match &key.segments[..] {
         [DbKeySeg::AddressSeg(addr), DbKeySeg::StringSeg(key)]
             if *addr == MASP
                 && (key == HEAD_TX_KEY
                     || key.starts_with(TX_KEY_PREFIX)
                     || key.starts_with(PIN_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
+                    || key == MASP_NOTE_COMMITMENT_TREE_KEY) =>
+        {
+            true
+        }
+
+        [
+            DbKeySeg::AddressSeg(addr),
+            DbKeySeg::StringSeg(key),
+            DbKeySeg::StringSeg(_nullifier),
+        ] if *addr == MASP && key == MASP_NULLIFIERS_KEY => true,
+        _ => false,
     }
 }
 
-/// Check if the given storage key is a masp nullifier key
-pub fn is_masp_nullifier_key(key: &Key) -> bool {
+/// Check if the given storage key is a masp tx prefix key
+pub fn is_masp_tx_prefix_key(key: &Key) -> bool {
     matches!(&key.segments[..],
-    [DbKeySeg::AddressSeg(addr),
+        [DbKeySeg::AddressSeg(addr),
              DbKeySeg::StringSeg(prefix),
-             ..
-        ] if *addr == MASP && prefix == MASP_NULLIFIERS_KEY_PREFIX)
+        ] if *addr == MASP && prefix.starts_with(TX_KEY_PREFIX))
 }
 
-/// Check if the given storage key is a masp anchor key
-pub fn is_masp_anchor_key(key: &Key) -> bool {
+/// Check if the given storage key is a masp tx pin key
+pub fn is_masp_tx_pin_key(key: &Key) -> bool {
+    matches!(&key.segments[..],
+        [DbKeySeg::AddressSeg(addr),
+             DbKeySeg::StringSeg(prefix),
+        ] if *addr == MASP && prefix.starts_with(PIN_KEY_PREFIX))
+}
+
+/// Check if the given storage key is a masp nullifier key
+pub fn is_masp_nullifier_key(key: &Key) -> bool {
     matches!(&key.segments[..],
-    [DbKeySeg::AddressSeg(addr),
+        [DbKeySeg::AddressSeg(addr),
              DbKeySeg::StringSeg(prefix),
-             ..
-        ] if *addr == MASP && prefix == MASP_NOTE_COMMITMENT_ANCHOR_PREFIX)
+             DbKeySeg::StringSeg(_nullifier)
+        ] if *addr == MASP && prefix == MASP_NULLIFIERS_KEY)
 }
 
 /// Obtain the storage key for the last locked ratio of a token