From 4fb5142c7326717c7662c7c4e99e5d994081ba4b Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 1 Dec 2023 19:07:29 +0100 Subject: [PATCH 1/9] Updates masp tx with note commitment tree and anchor --- Cargo.lock | 1 + apps/src/lib/node/ledger/shell/init_chain.rs | 28 +++++++++++ core/Cargo.toml | 1 + core/src/ledger/masp_utils.rs | 52 +++++++++++++++++++- core/src/types/token.rs | 17 ++++++- wasm/Cargo.lock | 1 + wasm_for_tests/wasm_source/Cargo.lock | 1 + 7 files changed, 99 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a760f39701..c87d9e7c49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3930,6 +3930,7 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", + "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", diff --git a/apps/src/lib/node/ledger/shell/init_chain.rs b/apps/src/lib/node/ledger/shell/init_chain.rs index aad1fcc362..12a18469b2 100644 --- a/apps/src/lib/node/ledger/shell/init_chain.rs +++ b/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}; @@ -9,10 +12,14 @@ use namada::ledger::storage_api::token::{credit_tokens, write_denom}; use namada::ledger::storage_api::StorageWrite; use namada::ledger::{ibc, pos}; use namada::proof_of_stake::BecomeValidator; +use namada::types::address::MASP; use namada::types::hash::Hash as CodeHash; use namada::types::key::*; use namada::types::storage::KeySeg; use namada::types::time::{DateTimeUtc, TimeZone, Utc}; +use namada::types::token::{ + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, +}; use namada::vm::validate_untrusted_wasm; use namada_sdk::eth_bridge::EthBridgeStatus; use namada_sdk::proof_of_stake::types::ValidatorMetaData; @@ -172,6 +179,27 @@ where ibc::init_genesis_storage(&mut self.wl_storage); + // Init masp commitment tree and anchor + let empty_commitment_tree: CommitmentTree = + CommitmentTree::empty(); + let anchor = empty_commitment_tree.root(); + let note_commitment_tree_key = Key::from(MASP.to_db_key()) + .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) + .expect("Cannot obtain a storage key"); + self.wl_storage + .write(¬e_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| { diff --git a/core/Cargo.toml b/core/Cargo.toml index 53ee824077..e2e2351087 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -51,6 +51,7 @@ index-set.workspace = true itertools.workspace = true k256.workspace = true masp_primitives.workspace = true +masp_proofs.workspace = true num256.workspace = true num_enum = "0.7.0" num-integer = "0.1.45" diff --git a/core/src/ledger/masp_utils.rs b/core/src/ledger/masp_utils.rs index 4ddc2bc49a..9eaeb2faa7 100644 --- a/core/src/ledger/masp_utils.rs +++ b/core/src/ledger/masp_utils.rs @@ -1,9 +1,12 @@ //! MASP utilities +use masp_primitives::merkle_tree::CommitmentTree; +use masp_primitives::sapling::Node; use masp_primitives::transaction::Transaction; +use masp_proofs::bls12_381; 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}; @@ -11,6 +14,10 @@ use crate::types::token::{ Transfer, HEAD_TX_KEY, MASP_NULLIFIERS_KEY_PREFIX, PIN_KEY_PREFIX, TX_KEY_PREFIX, }; +use crate::types::token::{ + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, + MASP_NULLIFIERS_KEY_PREFIX, +}; // Writes the nullifiers of the provided masp transaction to storage fn reveal_nullifiers( @@ -32,6 +39,48 @@ fn reveal_nullifiers( Ok(()) } +// Appends the note commitments of the provided transaction to the merkle tree +// and updates the anchor +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.to_owned()) + .expect("Cannot obtain a storage key"); + let mut spend_tree = 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 + spend_tree + .append(Node::from_scalar(description.cmu)) + .map_err(|_| { + Error::SimpleMessage("Note commitment tree is full") + })?; + } + + // Write the tree back to storage and update the anchor + let updated_anchor = spend_tree.root(); + ctx.write(&tree_key, spend_tree)?; + let anchor_key = Key::from(MASP.to_db_key()) + .push(&MASP_NOTE_COMMITMENT_ANCHOR_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(&Hash(bls12_381::Scalar::from(updated_anchor).to_bytes())) + .expect("Cannot obtain a storage key"); + + ctx.write(&anchor_key, ())?; + } + } + + Ok(()) +} + /// Handle a MASP transaction. pub fn handle_masp_tx( ctx: &mut (impl StorageRead + StorageWrite), @@ -59,6 +108,7 @@ pub fn handle_masp_tx( ); ctx.write(¤t_tx_key, record)?; ctx.write(&head_tx_key, current_tx_idx + 1)?; + update_note_commitment_tree(ctx, shielded)?; reveal_nullifiers(ctx, shielded)?; // If storage key has been supplied, then pin this transaction to it diff --git a/core/src/types/token.rs b/core/src/types/token.rs index 3cb820f4a3..1df1ea37da 100644 --- a/core/src/types/token.rs +++ b/core/src/types/token.rs @@ -902,6 +902,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: &str = "spend_tree"; +/// Key segment prefix for the note commitment anchor +pub const MASP_NOTE_COMMITMENT_ANCHOR_PREFIX: &str = "spend_anchor"; /// Last calculated inflation value handed out pub const MASP_LAST_INFLATION_KEY: &str = "last_inflation"; /// The last locked ratio @@ -1127,7 +1131,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.starts_with(MASP_NOTE_COMMITMENT_ANCHOR_PREFIX))) } /// Check if the given storage key is a masp nullifier key @@ -1139,6 +1145,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( diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 66a3bb5817..7c6464415f 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3020,6 +3020,7 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", + "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 133935e7f0..5f66a044d6 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -3020,6 +3020,7 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", + "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", From cf1ad179c7374a1d91c1a0d96d08bde15c3dbf6e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 2 Dec 2023 22:27:08 +0100 Subject: [PATCH 2/9] Updates masp vp to validate note commitment tree and anchor --- shared/src/ledger/native_vp/masp.rs | 139 +++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-) diff --git a/shared/src/ledger/native_vp/masp.rs b/shared/src/ledger/native_vp/masp.rs index d2b9a9b56b..607843adf8 100644 --- a/shared/src/ledger/native_vp/masp.rs +++ b/shared/src/ledger/native_vp/masp.rs @@ -5,7 +5,11 @@ use std::collections::{BTreeSet, HashSet}; use borsh_ext::BorshSerializeExt; use masp_primitives::asset_type::AssetType; +use masp_primitives::merkle_tree::CommitmentTree; +use masp_primitives::sapling::Node; use masp_primitives::transaction::components::I128Sum; +use masp_primitives::transaction::Transaction; +use masp_proofs::bls12_381; use namada_core::ledger::gas::MASP_VERIFY_SHIELDED_TX_GAS; use namada_core::ledger::storage; use namada_core::ledger::storage_api::OptionExt; @@ -15,7 +19,9 @@ use namada_core::types::address::InternalAddress::Masp; use namada_core::types::address::{Address, MASP}; use namada_core::types::storage::{Epoch, Key, KeySeg}; use namada_core::types::token::{ - self, is_masp_nullifier_key, MASP_NULLIFIERS_KEY_PREFIX, + self, is_masp_anchor_key, is_masp_nullifier_key, + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, + MASP_NULLIFIERS_KEY_PREFIX, }; use namada_sdk::masp::verify_shielded_tx; use ripemd::Digest as RipemdDigest; @@ -110,6 +116,121 @@ fn convert_amount( (asset_type, amount) } +impl<'a, DB, H, CA> MaspVp<'a, DB, H, CA> +where + DB: 'static + storage::DB + for<'iter> storage::DBIter<'iter>, + H: 'static + storage::StorageHasher, + CA: 'static + WasmCacheAccess, +{ + // Check that a transaction carrying output descriptions correctly updates + // the tree and anchor in storage + fn validate_note_commitment_update( + &self, + keys_changed: &BTreeSet, + transaction: &Transaction, + ) -> Result { + // Check that the merkle tree in storage has been correctly updated with + // the output descriptions cmu + let tree_key = Key::from(MASP.to_db_key()) + .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) + .expect("Cannot obtain a storage key"); + let mut previous_tree: CommitmentTree = + self.ctx.read_pre(&tree_key)?.ok_or(Error::NativeVpError( + native_vp::Error::SimpleMessage("Cannot read storage"), + ))?; + let post_tree: CommitmentTree = + self.ctx.read_post(&tree_key)?.ok_or(Error::NativeVpError( + native_vp::Error::SimpleMessage("Cannot read storage"), + ))?; + + // Based on the output descriptions of the transaction, update the + // previous tree in storage + for description in transaction + .sapling_bundle() + .map_or(&vec![], |bundle| &bundle.shielded_outputs) + { + previous_tree + .append(Node::from_scalar(description.cmu)) + .map_err(|()| { + Error::NativeVpError(native_vp::Error::SimpleMessage( + "Failed to update the commitment tree", + )) + })?; + } + // Check that the updated previous tree matches the actual post tree. + // This verifies that all and only the necessary notes have been + // appended to the tree + if previous_tree != post_tree { + tracing::debug!("The note commitment tree was incorrectly updated"); + return Ok(false); + } + + // Check that only one anchor was published + if keys_changed + .iter() + .filter(|key| is_masp_anchor_key(key)) + .count() + != 1 + { + tracing::debug!( + "More than one MASP anchor keys havebeen revealed by the \ + transaction" + ); + return Ok(false); + } + + // Check that the new anchor has been written to storage + let updated_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(previous_tree.root()).to_bytes(), + )) + .expect("Cannot obtain a storage key"); + + match self.ctx.read_bytes_post(&updated_anchor_key)? { + Some(value) if value.is_empty() => (), + _ => { + tracing::debug!( + "The commitment tree anchor was not updated correctly" + ); + return Ok(false); + } + } + + Ok(true) + } + + // Check that the spend descriptions anchors of a transaction are valid + fn validate_spend_descriptions_anchor( + &self, + transaction: &Transaction, + ) -> Result { + for description in transaction + .sapling_bundle() + .map_or(&vec![], |bundle| &bundle.shielded_spends) + { + 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( + description.anchor.to_bytes(), + )) + .expect("Cannot obtain a storage key"); + + // Check if the provided anchor was published before + if !self.ctx.has_key_pre(&anchor_key)? { + tracing::debug!( + "Spend description refers to an invalid anchor" + ); + return Ok(false); + } + } + + Ok(true) + } +} + impl<'a, DB, H, CA> NativeVp for MaspVp<'a, DB, H, CA> where DB: 'static + storage::DB + for<'iter> storage::DBIter<'iter>, @@ -153,7 +274,8 @@ where // 2. the transparent transaction value pool's amount must equal // the containing wrapper transaction's fee // amount Satisfies 1. - // 3. The nullifiers provided by the transaction have not been + // 3. The spend descriptions' anchors are valid + // 4. The nullifiers provided by the transaction have not been // revealed previously (even in the same tx) and no unneeded // nullifier is being revealed by the tx if let Some(transp_bundle) = shielded_tx.transparent_bundle() { @@ -167,6 +289,10 @@ where } } + if !self.validate_spend_descriptions_anchor(&shielded_tx)? { + return Ok(false); + } + let mut revealed_nullifiers = HashSet::new(); for description in shielded_tx .sapling_bundle() @@ -304,6 +430,8 @@ where // Handle shielded output // The following boundary conditions must be satisfied // 1. Zero transparent output + // 2. The transaction must correctly update the note commitment tree + // and the anchor in storage with the new output descriptions // Satisfies 1. if let Some(transp_bundle) = shielded_tx.transparent_bundle() { @@ -316,6 +444,13 @@ where return Ok(false); } } + + // Satisfies 2 + if !self + .validate_note_commitment_update(keys_changed, &shielded_tx)? + { + return Ok(false); + } } match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { From 61a46de2e3461fbee48e176734a43e59c628fc2c Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 2 Dec 2023 22:46:56 +0100 Subject: [PATCH 3/9] Refactors masp nullifiers check in a separate function --- shared/src/ledger/native_vp/masp.rs | 113 +++++++++++++++------------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/shared/src/ledger/native_vp/masp.rs b/shared/src/ledger/native_vp/masp.rs index 607843adf8..3e705f9606 100644 --- a/shared/src/ledger/native_vp/masp.rs +++ b/shared/src/ledger/native_vp/masp.rs @@ -122,9 +122,63 @@ where H: 'static + storage::StorageHasher, CA: 'static + WasmCacheAccess, { + // Check that the transaction correctly revealed the nullifiers + fn valid_nullifiers_reveal( + &self, + keys_changed: &BTreeSet, + transaction: &Transaction, + ) -> Result { + let mut revealed_nullifiers = HashSet::new(); + for description in transaction + .sapling_bundle() + .map_or(&vec![], |description| &description.shielded_spends) + { + let nullifier_key = Key::from(MASP.to_db_key()) + .push(&MASP_NULLIFIERS_KEY_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(&namada_core::types::hash::Hash(description.nullifier.0)) + .expect("Cannot obtain a storage key"); + if self.ctx.has_key_pre(&nullifier_key)? + || revealed_nullifiers.contains(&nullifier_key) + { + tracing::debug!( + "MASP double spending attempt, the nullifier {:#?} has \ + already been revealed previously", + description.nullifier.0 + ); + return Ok(false); + } + + // Check that the nullifier is indeed committed (no temp write + // and no delete) and carries no associated data (the latter not + // strictly necessary for validation, but we don't expect any + // value for this key anyway) + match self.ctx.read_bytes_post(&nullifier_key)? { + Some(value) if value.is_empty() => (), + _ => return Ok(false), + } + + revealed_nullifiers.insert(nullifier_key); + } + + for nullifier_key in + keys_changed.iter().filter(|key| is_masp_nullifier_key(key)) + { + if !revealed_nullifiers.contains(nullifier_key) { + tracing::debug!( + "An unexpected MASP nullifier key {nullifier_key} has \ + been revealed by the transaction" + ); + return Ok(false); + } + } + + Ok(true) + } + // Check that a transaction carrying output descriptions correctly updates // the tree and anchor in storage - fn validate_note_commitment_update( + fn valid_note_commitment_update( &self, keys_changed: &BTreeSet, transaction: &Transaction, @@ -202,7 +256,7 @@ where } // Check that the spend descriptions anchors of a transaction are valid - fn validate_spend_descriptions_anchor( + fn valid_spend_descriptions_anchor( &self, transaction: &Transaction, ) -> Result { @@ -289,55 +343,10 @@ where } } - if !self.validate_spend_descriptions_anchor(&shielded_tx)? { - return Ok(false); - } - - let mut revealed_nullifiers = HashSet::new(); - for description in shielded_tx - .sapling_bundle() - .map_or(&vec![], |description| &description.shielded_spends) - { - let nullifier_key = Key::from(MASP.to_db_key()) - .push(&MASP_NULLIFIERS_KEY_PREFIX.to_owned()) - .expect("Cannot obtain a storage key") - .push(&namada_core::types::hash::Hash( - description.nullifier.0, - )) - .expect("Cannot obtain a storage key"); - if self.ctx.has_key_pre(&nullifier_key)? - || revealed_nullifiers.contains(&nullifier_key) - { - tracing::debug!( - "MASP double spending attempt, the nullifier {:#?} \ - has already been revealed previously", - description.nullifier.0 - ); - return Ok(false); - } - - // Check that the nullifier is indeed committed (no temp write - // and no delete) and carries no associated data (the latter not - // strictly necessary for validation, but we don't expect any - // value for this key anyway) - match self.ctx.read_bytes_post(&nullifier_key)? { - Some(value) if value.is_empty() => (), - _ => return Ok(false), - } - - revealed_nullifiers.insert(nullifier_key); - } - - for nullifier_key in - keys_changed.iter().filter(|key| is_masp_nullifier_key(key)) + if !(self.valid_spend_descriptions_anchor(&shielded_tx)? + && self.valid_nullifiers_reveal(keys_changed, &shielded_tx)?) { - if !revealed_nullifiers.contains(nullifier_key) { - tracing::debug!( - "An unexpected MASP nullifier key {nullifier_key} has \ - been revealed by the transaction" - ); - return Ok(false); - } + return Ok(false); } } @@ -446,9 +455,7 @@ where } // Satisfies 2 - if !self - .validate_note_commitment_update(keys_changed, &shielded_tx)? - { + if !self.valid_note_commitment_update(keys_changed, &shielded_tx)? { return Ok(false); } } From 2183aaa999e8fe8e8409a4001f644b18d6c5ad9a Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 2 Dec 2023 23:38:37 +0100 Subject: [PATCH 4/9] Updates commitment tree anchor only once per block --- Cargo.lock | 1 - .../lib/node/ledger/shell/finalize_block.rs | 25 +++++++++++++++++ core/Cargo.toml | 1 - core/src/ledger/masp_utils.rs | 23 ++++----------- shared/src/ledger/native_vp/masp.rs | 28 +++---------------- wasm/Cargo.lock | 1 - wasm_for_tests/wasm_source/Cargo.lock | 1 - 7 files changed, 35 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c87d9e7c49..a760f39701 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3930,7 +3930,6 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", - "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 257b766a03..6c3c67d3ac 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -1,9 +1,13 @@ //! 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; @@ -17,9 +21,13 @@ 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, +}; use namada::types::transaction::protocol::{ ethereum_tx_data_variants, ProtocolTxType, }; @@ -557,6 +565,23 @@ where tracing::info!("{}", stats); tracing::info!("{}", stats.format_tx_executed()); + // Update the MASP commitment tree anchor + let tree_key = Key::from(MASP.to_db_key()) + .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) + .expect("Cannot obtain a storage key"); + let updated_tree: CommitmentTree = self + .wl_storage + .read(&tree_key)? + .expect("Missing note commitment tree in storage"); + 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 diff --git a/core/Cargo.toml b/core/Cargo.toml index e2e2351087..53ee824077 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -51,7 +51,6 @@ index-set.workspace = true itertools.workspace = true k256.workspace = true masp_primitives.workspace = true -masp_proofs.workspace = true num256.workspace = true num_enum = "0.7.0" num-integer = "0.1.45" diff --git a/core/src/ledger/masp_utils.rs b/core/src/ledger/masp_utils.rs index 9eaeb2faa7..eb9918a707 100644 --- a/core/src/ledger/masp_utils.rs +++ b/core/src/ledger/masp_utils.rs @@ -3,7 +3,6 @@ use masp_primitives::merkle_tree::CommitmentTree; use masp_primitives::sapling::Node; use masp_primitives::transaction::Transaction; -use masp_proofs::bls12_381; use super::storage_api::{StorageRead, StorageWrite}; use crate::ledger::storage_api::{Error, Result}; @@ -15,8 +14,7 @@ use crate::types::token::{ TX_KEY_PREFIX, }; use crate::types::token::{ - MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, - MASP_NULLIFIERS_KEY_PREFIX, + MASP_NOTE_COMMITMENT_TREE, MASP_NULLIFIERS_KEY_PREFIX, }; // Writes the nullifiers of the provided masp transaction to storage @@ -50,31 +48,22 @@ fn update_note_commitment_tree( let tree_key = Key::from(MASP.to_db_key()) .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) .expect("Cannot obtain a storage key"); - let mut spend_tree = ctx + let mut commitment_tree = ctx .read::>(&tree_key)? .ok_or(Error::SimpleMessage( - "Missing note commitment tree in storage", - ))?; + "Missing note commitment tree in storage", + ))?; for description in &bundle.shielded_outputs { // Add cmu to the merkle tree - spend_tree + commitment_tree .append(Node::from_scalar(description.cmu)) .map_err(|_| { Error::SimpleMessage("Note commitment tree is full") })?; } - // Write the tree back to storage and update the anchor - let updated_anchor = spend_tree.root(); - ctx.write(&tree_key, spend_tree)?; - let anchor_key = Key::from(MASP.to_db_key()) - .push(&MASP_NOTE_COMMITMENT_ANCHOR_PREFIX.to_owned()) - .expect("Cannot obtain a storage key") - .push(&Hash(bls12_381::Scalar::from(updated_anchor).to_bytes())) - .expect("Cannot obtain a storage key"); - - ctx.write(&anchor_key, ())?; + ctx.write(&tree_key, commitment_tree)?; } } diff --git a/shared/src/ledger/native_vp/masp.rs b/shared/src/ledger/native_vp/masp.rs index 3e705f9606..e4b8f3f53b 100644 --- a/shared/src/ledger/native_vp/masp.rs +++ b/shared/src/ledger/native_vp/masp.rs @@ -9,7 +9,6 @@ use masp_primitives::merkle_tree::CommitmentTree; use masp_primitives::sapling::Node; use masp_primitives::transaction::components::I128Sum; use masp_primitives::transaction::Transaction; -use masp_proofs::bls12_381; use namada_core::ledger::gas::MASP_VERIFY_SHIELDED_TX_GAS; use namada_core::ledger::storage; use namada_core::ledger::storage_api::OptionExt; @@ -219,39 +218,20 @@ where return Ok(false); } - // Check that only one anchor was published + // Check that no anchor was published (this is to be done by the + // protocol) if keys_changed .iter() .filter(|key| is_masp_anchor_key(key)) .count() - != 1 + != 0 { tracing::debug!( - "More than one MASP anchor keys havebeen revealed by the \ - transaction" + "The transaction revealed one or more MASP anchor keys" ); return Ok(false); } - // Check that the new anchor has been written to storage - let updated_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(previous_tree.root()).to_bytes(), - )) - .expect("Cannot obtain a storage key"); - - match self.ctx.read_bytes_post(&updated_anchor_key)? { - Some(value) if value.is_empty() => (), - _ => { - tracing::debug!( - "The commitment tree anchor was not updated correctly" - ); - return Ok(false); - } - } - Ok(true) } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 7c6464415f..66a3bb5817 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3020,7 +3020,6 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", - "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 5f66a044d6..133935e7f0 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -3020,7 +3020,6 @@ dependencies = [ "itertools 0.10.5", "k256", "masp_primitives", - "masp_proofs", "namada_macros", "num-integer", "num-rational 0.4.1", From f7da77b2d4f62d2bc09f148d6199a6766e0ad8ab Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sun, 3 Dec 2023 18:57:53 +0100 Subject: [PATCH 5/9] Updates the merkle tree anchor only if the tree changed --- .../lib/node/ledger/shell/finalize_block.rs | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 6c3c67d3ac..077feef2ad 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -14,9 +14,10 @@ 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, @@ -565,22 +566,24 @@ where tracing::info!("{}", stats); tracing::info!("{}", stats.format_tx_executed()); - // Update the MASP commitment tree anchor + // 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.to_owned()) .expect("Cannot obtain a storage key"); - let updated_tree: CommitmentTree = self - .wl_storage - .read(&tree_key)? - .expect("Missing note commitment tree in storage"); - 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 let Some(StorageModification::Write { value }) = + self.wl_storage.write_log.read(&tree_key).0 + { + let updated_tree = CommitmentTree::::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); From 694b94970fe1536b572acb045432df126034ba08 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 6 Dec 2023 11:57:58 +0100 Subject: [PATCH 6/9] Fixes commitment tree validation in masp vp. Adds a workaround to update tree from the tx --- .../lib/node/ledger/shell/finalize_block.rs | 4 +- apps/src/lib/node/ledger/shell/init_chain.rs | 4 +- core/src/ledger/masp_utils.rs | 27 ++++++------ core/src/types/token.rs | 6 +-- shared/src/ledger/native_vp/ibc/context.rs | 7 ++- shared/src/ledger/native_vp/masp.rs | 17 ++++---- shared/src/vm/host_env.rs | 43 ++++++++++++++++++- shared/src/vm/wasm/host_env.rs | 1 + tx_prelude/src/ibc.rs | 7 ++- tx_prelude/src/lib.rs | 18 ++++++++ vm_env/src/lib.rs | 5 +++ wasm/wasm_source/src/tx_transfer.rs | 1 + 12 files changed, 108 insertions(+), 32 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 077feef2ad..ca3972b560 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -27,7 +27,7 @@ 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, + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY, }; use namada::types::transaction::protocol::{ ethereum_tx_data_variants, ProtocolTxType, @@ -568,7 +568,7 @@ where // 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.to_owned()) + .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 diff --git a/apps/src/lib/node/ledger/shell/init_chain.rs b/apps/src/lib/node/ledger/shell/init_chain.rs index 12a18469b2..18d7acd749 100644 --- a/apps/src/lib/node/ledger/shell/init_chain.rs +++ b/apps/src/lib/node/ledger/shell/init_chain.rs @@ -18,7 +18,7 @@ use namada::types::key::*; use namada::types::storage::KeySeg; use namada::types::time::{DateTimeUtc, TimeZone, Utc}; use namada::types::token::{ - MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY, }; use namada::vm::validate_untrusted_wasm; use namada_sdk::eth_bridge::EthBridgeStatus; @@ -184,7 +184,7 @@ where CommitmentTree::empty(); let anchor = empty_commitment_tree.root(); let note_commitment_tree_key = Key::from(MASP.to_db_key()) - .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) + .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned()) .expect("Cannot obtain a storage key"); self.wl_storage .write(¬e_commitment_tree_key, empty_commitment_tree) diff --git a/core/src/ledger/masp_utils.rs b/core/src/ledger/masp_utils.rs index eb9918a707..d15834f7a1 100644 --- a/core/src/ledger/masp_utils.rs +++ b/core/src/ledger/masp_utils.rs @@ -10,11 +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_NULLIFIERS_KEY_PREFIX, PIN_KEY_PREFIX, - TX_KEY_PREFIX, -}; -use crate::types::token::{ - MASP_NOTE_COMMITMENT_TREE, MASP_NULLIFIERS_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 @@ -37,20 +34,21 @@ fn reveal_nullifiers( Ok(()) } -// Appends the note commitments of the provided transaction to the merkle tree -// and updates the anchor -fn update_note_commitment_tree( +/// 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.to_owned()) + .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned()) .expect("Cannot obtain a storage key"); - let mut commitment_tree = ctx - .read::>(&tree_key)? - .ok_or(Error::SimpleMessage( + let mut commitment_tree: CommitmentTree = + ctx.read(&tree_key)?.ok_or(Error::SimpleMessage( "Missing note commitment tree in storage", ))?; @@ -97,7 +95,10 @@ pub fn handle_masp_tx( ); ctx.write(¤t_tx_key, record)?; ctx.write(&head_tx_key, current_tx_idx + 1)?; - update_note_commitment_tree(ctx, shielded)?; + // 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 diff --git a/core/src/types/token.rs b/core/src/types/token.rs index 1df1ea37da..b5737120d8 100644 --- a/core/src/types/token.rs +++ b/core/src/types/token.rs @@ -903,9 +903,9 @@ 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: &str = "spend_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 = "spend_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 @@ -1132,7 +1132,7 @@ pub fn is_masp_key(key: &Key) -> bool { || 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 == MASP_NOTE_COMMITMENT_TREE_KEY || key.starts_with(MASP_NOTE_COMMITMENT_ANCHOR_PREFIX))) } diff --git a/shared/src/ledger/native_vp/ibc/context.rs b/shared/src/ledger/native_vp/ibc/context.rs index 1c806afe35..c8d162c000 100644 --- a/shared/src/ledger/native_vp/ibc/context.rs +++ b/shared/src/ledger/native_vp/ibc/context.rs @@ -213,7 +213,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( diff --git a/shared/src/ledger/native_vp/masp.rs b/shared/src/ledger/native_vp/masp.rs index e4b8f3f53b..1ae3b6bdae 100644 --- a/shared/src/ledger/native_vp/masp.rs +++ b/shared/src/ledger/native_vp/masp.rs @@ -19,7 +19,7 @@ use namada_core::types::address::{Address, MASP}; use namada_core::types::storage::{Epoch, Key, KeySeg}; use namada_core::types::token::{ self, is_masp_anchor_key, is_masp_nullifier_key, - MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE, + MASP_NOTE_COMMITMENT_ANCHOR_PREFIX, MASP_NOTE_COMMITMENT_TREE_KEY, MASP_NULLIFIERS_KEY_PREFIX, }; use namada_sdk::masp::verify_shielded_tx; @@ -185,7 +185,7 @@ where // Check that the merkle tree in storage has been correctly updated with // the output descriptions cmu let tree_key = Key::from(MASP.to_db_key()) - .push(&MASP_NOTE_COMMITMENT_TREE.to_owned()) + .push(&MASP_NOTE_COMMITMENT_TREE_KEY.to_owned()) .expect("Cannot obtain a storage key"); let mut previous_tree: CommitmentTree = self.ctx.read_pre(&tree_key)?.ok_or(Error::NativeVpError( @@ -330,6 +330,12 @@ where } } + // The transaction must correctly update the note commitment tree + // and the anchor in storage with the new output descriptions + if !self.valid_note_commitment_update(keys_changed, &shielded_tx)? { + return Ok(false); + } + if transfer.target != Address::Internal(Masp) { // Handle transparent output // The following boundary conditions must be satisfied @@ -419,8 +425,6 @@ where // Handle shielded output // The following boundary conditions must be satisfied // 1. Zero transparent output - // 2. The transaction must correctly update the note commitment tree - // and the anchor in storage with the new output descriptions // Satisfies 1. if let Some(transp_bundle) = shielded_tx.transparent_bundle() { @@ -433,11 +437,6 @@ where return Ok(false); } } - - // Satisfies 2 - if !self.valid_note_commitment_update(keys_changed, &shielded_tx)? { - return Ok(false); - } } match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index 0e3fc21929..f95b8a24dd 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -6,6 +6,7 @@ use std::num::TryFromIntError; use borsh::BorshDeserialize; use borsh_ext::BorshSerializeExt; +use masp_primitives::transaction::Transaction; use namada_core::ledger::gas::{ GasMetering, TxGasMeter, MEMORY_ACCESS_GAS_PER_BYTE, }; @@ -2164,6 +2165,41 @@ where } } +/// Appends the new note commitments to the tree in storage +pub fn tx_update_masp_note_commitment_tree( + env: &TxVmEnv, + transaction_ptr: u64, + transaction_len: u64, +) -> TxResult +where + MEM: VmMemory, + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + CA: WasmCacheAccess, +{ + let _sentinel = unsafe { env.ctx.sentinel.get() }; + let _gas_meter = unsafe { env.ctx.gas_meter.get() }; + let (serialized_transaction, gas) = env + .memory + .read_bytes(transaction_ptr, transaction_len as _) + .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; + + tx_charge_gas(env, gas)?; + let transaction = Transaction::try_from_slice(&serialized_transaction) + .map_err(TxRuntimeError::EncodingError)?; + + let mut ctx = env.ctx.clone(); + match masp_utils::update_note_commitment_tree(&mut ctx, &transaction) { + Ok(()) => Ok(HostEnvResult::Success.to_i64()), + Err(_) => { + // NOTE: sentinel for gas errors is already set by the + // update_note_commitment_tree function which in turn calls other + // host functions + Ok(HostEnvResult::Fail.to_i64()) + } + } +} + /// Evaluate a validity predicate with the given input data. pub fn vp_eval( env: &VpVmEnv<'static, MEM, DB, H, EVAL, CA>, @@ -2519,7 +2555,12 @@ where &mut self, shielded: &IbcShieldedTransfer, ) -> Result<(), storage_api::Error> { - 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( diff --git a/shared/src/vm/wasm/host_env.rs b/shared/src/vm/wasm/host_env.rs index 8c897f1a29..304a27a7f2 100644 --- a/shared/src/vm/wasm/host_env.rs +++ b/shared/src/vm/wasm/host_env.rs @@ -88,6 +88,7 @@ where "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), "namada_tx_verify_tx_section_signature" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_verify_tx_section_signature), + "namada_tx_update_masp_note_commitment_tree" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_update_masp_note_commitment_tree) }, } } diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index 3e9382b59a..38e907a4fe 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -53,7 +53,12 @@ impl IbcStorageContext for Ctx { &mut self, shielded: &IbcShieldedTransfer, ) -> Result<(), Error> { - 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( diff --git a/tx_prelude/src/lib.rs b/tx_prelude/src/lib.rs index 29e9881488..fbb6e03a78 100644 --- a/tx_prelude/src/lib.rs +++ b/tx_prelude/src/lib.rs @@ -19,6 +19,7 @@ use std::marker::PhantomData; pub use borsh::{BorshDeserialize, BorshSerialize}; pub use borsh_ext; use borsh_ext::BorshSerializeExt; +use masp_primitives::transaction::Transaction; pub use namada_core::ledger::governance::storage as gov_storage; pub use namada_core::ledger::parameters::storage as parameters_storage; pub use namada_core::ledger::storage::types::encode; @@ -404,3 +405,20 @@ pub fn verify_signatures_of_pks( Ok(HostEnvResult::is_success(valid)) } + +/// Update the masp note commitment tree in storage with the new notes +pub fn update_masp_note_commitment_tree( + transaction: &Transaction, +) -> EnvResult { + // Serialize transaction + let transaction = transaction.serialize_to_vec(); + + let valid = unsafe { + namada_tx_update_masp_note_commitment_tree( + transaction.as_ptr() as _, + transaction.len() as _, + ) + }; + + Ok(HostEnvResult::is_success(valid)) +} diff --git a/vm_env/src/lib.rs b/vm_env/src/lib.rs index 3b2c5c4dcc..f46ad44b94 100644 --- a/vm_env/src/lib.rs +++ b/vm_env/src/lib.rs @@ -133,6 +133,11 @@ pub mod tx { max_signatures_len: u64, ) -> i64; + /// Update the masp note commitment tree with the new notes + pub fn namada_tx_update_masp_note_commitment_tree( + transaction_ptr: u64, + transaction_len: u64, + ) -> i64; } } diff --git a/wasm/wasm_source/src/tx_transfer.rs b/wasm/wasm_source/src/tx_transfer.rs index e473ef1d9e..e521be4f75 100644 --- a/wasm/wasm_source/src/tx_transfer.rs +++ b/wasm/wasm_source/src/tx_transfer.rs @@ -39,6 +39,7 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { .transpose()?; if let Some(shielded) = shielded { token::masp_utils::handle_masp_tx(ctx, &transfer, &shielded)?; + update_masp_note_commitment_tree(&shielded)?; } Ok(()) } From 8841aacf624303cc11f5612531a435363c0100ca Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 6 Dec 2023 15:43:38 +0100 Subject: [PATCH 7/9] Fixes masp vp benchmark --- Cargo.lock | 2 ++ benches/Cargo.toml | 2 ++ benches/native_vps.rs | 32 +++++++++++++++++++++++++++++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index a760f39701..0ed05112b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3896,6 +3896,8 @@ dependencies = [ "borsh", "borsh-ext", "criterion", + "masp_primitives", + "masp_proofs", "namada", "namada_apps", "rand 0.8.5", diff --git a/benches/Cargo.toml b/benches/Cargo.toml index 57de47dba4..b9d15a22f4 100644 --- a/benches/Cargo.toml +++ b/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"] } diff --git a/benches/native_vps.rs b/benches/native_vps.rs index fdaed5d1f9..def2e3c135 100644 --- a/benches/native_vps.rs +++ b/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, @@ -40,14 +42,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, }; @@ -502,6 +508,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 = 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 signed_tx = match bench_name { From c09875fc14edb00f77995bee1aae6b27f25be880 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 6 Dec 2023 15:48:58 +0100 Subject: [PATCH 8/9] Updates comment --- shared/src/ledger/native_vp/masp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/src/ledger/native_vp/masp.rs b/shared/src/ledger/native_vp/masp.rs index 1ae3b6bdae..d506da9c8f 100644 --- a/shared/src/ledger/native_vp/masp.rs +++ b/shared/src/ledger/native_vp/masp.rs @@ -331,7 +331,7 @@ where } // The transaction must correctly update the note commitment tree - // and the anchor in storage with the new output descriptions + // in storage with the new output descriptions if !self.valid_note_commitment_update(keys_changed, &shielded_tx)? { return Ok(false); } From 6276330eb964716011811540e734a979a1e428a3 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 6 Dec 2023 12:20:44 +0100 Subject: [PATCH 9/9] Changelog #2244 --- .../unreleased/bug-fixes/2244-spend-description-validation.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/2244-spend-description-validation.md diff --git a/.changelog/unreleased/bug-fixes/2244-spend-description-validation.md b/.changelog/unreleased/bug-fixes/2244-spend-description-validation.md new file mode 100644 index 0000000000..c531894144 --- /dev/null +++ b/.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)) \ No newline at end of file