From 835fdc017cac2bfca7bfecf158edff5cd370b527 Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 8 Jul 2025 11:07:36 +0200 Subject: [PATCH 01/24] Adding a shielding fee for normal shielding txs. Still need to check IBC --- crates/apps_lib/src/cli.rs | 33 + crates/apps_lib/src/client/tx.rs | 9 +- .../src/config/genesis/transactions.rs | 1 + crates/ibc/src/msg.rs | 26 +- crates/node/src/shell/testing/client.rs | 106 ++- crates/node/src/shell/testing/node.rs | 83 +- crates/sdk/src/args.rs | 32 +- crates/sdk/src/eth_bridge/bridge_pool.rs | 1 + crates/sdk/src/lib.rs | 4 + crates/sdk/src/rpc.rs | 12 + crates/sdk/src/signing.rs | 41 +- crates/sdk/src/tx.rs | 222 ++++- crates/shielded_token/src/storage_key.rs | 12 + crates/shielded_token/src/vp.rs | 9 +- crates/tests/src/integration/masp.rs | 767 +++++++++++++++++- crates/tests/src/integration/setup.rs | 14 + crates/token/src/lib.rs | 4 +- crates/token/src/tx.rs | 52 +- crates/tx/src/section.rs | 15 + crates/tx/src/types.rs | 16 + genesis/README.md | 2 +- 21 files changed, 1417 insertions(+), 44 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 2840804b014..5c5a0468033 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -500,6 +500,7 @@ pub mod cmds { } #[derive(Clone, Debug)] + #[allow(clippy::large_enum_variant)] pub enum NamadaClientWithContext { // Ledger cmds TxCustom(TxCustom), @@ -3672,6 +3673,10 @@ pub mod args { arg("self-bond-amount"); pub const SENDER: Arg = arg("sender"); pub const SHIELDED: ArgFlag = flag("shielded"); + pub const SHIELDING_FEE_PAYER: Arg = + arg("shielding-fee-payer"); + pub const SHIELDING_FEE_TOKEN: Arg = + arg("shielding-fee-token"); pub const SHOW_IBC_TOKENS: ArgFlag = flag("show-ibc-tokens"); pub const SLIPPAGE: ArgOpt = arg_opt("slippage-percentage"); pub const SIGNING_KEYS: ArgMulti = @@ -4946,12 +4951,15 @@ pub mod args { amount: transfer_data.amount, }); } + let shielding_fee_payer = chain_ctx.get(&self.shielding_fee_payer); Ok(TxShieldingTransfer:: { tx, sources: data, targets, tx_code_path: self.tx_code_path.to_path_buf(), + shielding_fee_payer, + shielding_fee_token: chain_ctx.get(&self.shielding_fee_token), }) } } @@ -4964,6 +4972,8 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); + let shielding_fee_payer = SHIELDING_FEE_PAYER.parse(matches); + let shielding_fee_token = SHIELDING_FEE_TOKEN.parse(matches); let data = vec![TxTransparentSource { source, token: token.clone(), @@ -4980,6 +4990,8 @@ pub mod args { sources: data, targets, tx_code_path, + shielding_fee_payer, + shielding_fee_token, } } @@ -5000,6 +5012,13 @@ pub mod args { .def() .help(wrap!("The amount to transfer in decimal.")), ) + .arg(SHIELDING_FEE_PAYER.def().help(wrap!( + "The implicit account which will pay the fee for \ + shielding tokens" + ))) + .arg(SHIELDING_FEE_TOKEN.def().help(wrap!( + "The token which will be used to pay the shielding fee" + ))) } } @@ -7243,6 +7262,8 @@ pub mod args { IbcShieldingTransferAsset::Address(chain_ctx.get(&addr)) } }, + shielding_fee_payer: chain_ctx.get(&self.shielding_fee_payer), + shielding_fee_token: chain_ctx.get(&self.shielding_fee_token), }) } } @@ -7266,6 +7287,8 @@ pub mod args { None => TxExpiration::Default, } }; + let shielding_fee_payer = SHIELDING_FEE_PAYER.parse(matches); + let shielding_fee_token = SHIELDING_FEE_TOKEN.parse(matches); Self { query, @@ -7278,6 +7301,8 @@ pub mod args { channel_id, token, }, + shielding_fee_payer, + shielding_fee_token, } } @@ -7319,6 +7344,14 @@ pub mod args { .arg(CHANNEL_ID.def().help(wrap!( "The channel ID via which the token is received." ))) + .arg(SHIELDING_FEE_PAYER.def().help(wrap!( + "The implicit account paying the fee for shielding tokens" + ))) + .arg( + SHIELDING_FEE_TOKEN + .def() + .help(wrap!("The token used to pay the shielding fee")), + ) } } diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index dca699d28ba..6dafd45edd4 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -79,6 +79,7 @@ pub async fn aux_signing_data( owner, default_signer, vec![], + None, disposable_signing_key, signatures, wrapper_signature, @@ -2011,6 +2012,8 @@ pub async fn gen_ibc_shielding_transfer( ) -> Result<(), error::Error> { let output_folder = args.output_folder.clone(); + let shielding_fee_payer = args.shielding_fee_payer.clone(); + let shielding_fee_token = args.shielding_fee_token.clone(); if let Some(masp_tx) = tx::gen_ibc_shielding_transfer(context, args).await? { let tx_id = masp_tx.txid().to_string(); @@ -2021,7 +2024,11 @@ pub async fn gen_ibc_shielding_transfer( }; let mut out = File::create(&output_path) .expect("Creating a new file for IBC MASP transaction failed."); - let bytes = convert_masp_tx_to_ibc_memo(&masp_tx); + let bytes = convert_masp_tx_to_ibc_memo( + &masp_tx, + shielding_fee_payer, + shielding_fee_token, + ); out.write_all(bytes.as_bytes()) .expect("Writing IBC MASP transaction file failed."); println!( diff --git a/crates/apps_lib/src/config/genesis/transactions.rs b/crates/apps_lib/src/config/genesis/transactions.rs index 6fc5eca6133..b3b46374f7e 100644 --- a/crates/apps_lib/src/config/genesis/transactions.rs +++ b/crates/apps_lib/src/config/genesis/transactions.rs @@ -770,6 +770,7 @@ impl Signed { public_keys: pks.clone(), threshold, fee_payer: Either::Left((genesis_fee_payer_pk(), false)), + shielding_fee_payer: None, shielded_hash: None, signatures: vec![], }; diff --git a/crates/ibc/src/msg.rs b/crates/ibc/src/msg.rs index d38466bb5e5..9edea421cee 100644 --- a/crates/ibc/src/msg.rs +++ b/crates/ibc/src/msg.rs @@ -19,7 +19,9 @@ use ibc::core::handler::types::msgs::MsgEnvelope; use ibc::core::host::types::identifiers::PortId; use ibc::primitives::proto::Protobuf; use masp_primitives::transaction::Transaction as MaspTransaction; +use namada_core::address::Address; use namada_core::borsh::BorshSerializeExt; +use namada_core::key::common::PublicKey; use namada_core::string_encoding::StringEncoded; use serde::{Deserialize, Serialize}; @@ -238,7 +240,14 @@ impl BorshSchema for MsgNftTransfer { /// Shielding data in IBC packet memo #[derive(Debug, Clone, BorshDeserialize, BorshSerialize)] -pub struct IbcShieldingData(pub MaspTransaction); +pub struct IbcShieldingData { + /// The MASP transaction that does the shielding + pub masp_tx: MaspTransaction, + /// The account that will pay the shielding fee + pub shielding_fee_payer: PublicKey, + /// The token that the shielding fee will be paid in + pub shielding_fee_token: Address, +} impl From<&IbcShieldingData> for String { fn from(data: &IbcShieldingData) -> Self { @@ -301,7 +310,7 @@ pub fn decode_ibc_shielding_data( /// Extract MASP transaction from IBC packet memo pub fn extract_masp_tx_from_packet(packet: &Packet) -> Option { let memo = extract_memo_from_packet(packet, &packet.port_id_on_b)?; - decode_ibc_shielding_data(memo).map(|data| data.0) + decode_ibc_shielding_data(memo).map(|data| data.masp_tx) } fn extract_memo_from_packet( @@ -366,6 +375,15 @@ pub fn extract_traces_from_recv_msg( } /// Get IBC memo string from MASP transaction for receiving -pub fn convert_masp_tx_to_ibc_memo(transaction: &MaspTransaction) -> String { - IbcShieldingData(transaction.clone()).into() +pub fn convert_masp_tx_to_ibc_memo( + transaction: &MaspTransaction, + shielding_fee_payer: PublicKey, + shielding_fee_token: Address, +) -> String { + IbcShieldingData { + masp_tx: transaction.clone(), + shielding_fee_payer, + shielding_fee_token, + } + .into() } diff --git a/crates/node/src/shell/testing/client.rs b/crates/node/src/shell/testing/client.rs index 03834746a97..857b44b9c93 100644 --- a/crates/node/src/shell/testing/client.rs +++ b/crates/node/src/shell/testing/client.rs @@ -1,12 +1,17 @@ use clap::Command as App; -use eyre::Report; +use eyre::{Report, WrapErr}; use namada_apps_lib::cli::api::{CliApi, CliClient}; use namada_apps_lib::cli::args::Global; use namada_apps_lib::cli::{ Cmd, Context, NamadaClient, NamadaRelayer, args, cmds, }; +use namada_sdk::args::{SdkTypes, TxExpiration}; use namada_sdk::error::Error as SdkError; use namada_sdk::io::Io; +use namada_sdk::signing::{SigningTxData, default_sign}; +use namada_sdk::tx::data::GasLimit; +use namada_sdk::tx::{ProcessTxResponse, Tx}; +use namada_sdk::{signing, tendermint_rpc, tx}; use super::node::MockNode; use crate::shell::testing::utils::{Bin, TestingIo}; @@ -109,3 +114,102 @@ impl CliClient for MockNode { Ok(()) } } + +/// Manually sign a tx. This can be used to sign a tx that was dumped +pub fn sign_tx( + node: &MockNode, + tx: Tx, + signing: SigningTxData, + // this is only used to give the password for decrypting keys to the wallet + args: &args::Tx, +) -> Result { + use namada_sdk::Namada; + let global = { + let locked = node.shell.lock().unwrap(); + Global { + is_pre_genesis: false, + chain_id: Some(locked.chain_id.clone()), + base_dir: locked.base_dir.clone(), + wasm_dir: Some(locked.wasm_dir.clone()), + } + }; + let ctx = Context::new::(global.clone()) + .wrap_err("Failed to build context")? + .to_sdk(node.clone(), TestingIo); + let rt = tokio::runtime::Runtime::new().unwrap(); + + let (mut batched_tx, batched_signing_data) = + tx::build_batch(vec![(tx, signing)]) + .wrap_err("Failed to build tx batch")?; + rt.block_on(async { + for sig_data in batched_signing_data { + signing::sign_tx( + ctx.wallet_lock(), + args, + &mut batched_tx, + sig_data, + default_sign, + (), + ) + .await + .wrap_err("Signing tx failed")?; + } + Ok::<(), Report>(()) + })?; + Ok(batched_tx) +} + +/// Manually submit a tx. Used for txs that have been manually constructed +/// instead of by the CLI +pub fn submit_custom( + node: &MockNode, + tx: Tx, + // this is only used to give the password for decrypting keys to the wallet + args: &args::Tx, +) -> Result { + use namada_sdk::Namada; + let global = { + let locked = node.shell.lock().unwrap(); + Global { + is_pre_genesis: false, + chain_id: Some(locked.chain_id.clone()), + base_dir: locked.base_dir.clone(), + wasm_dir: Some(locked.wasm_dir.clone()), + } + }; + let ctx = Context::new::(global.clone()) + .wrap_err("Failed to build context")? + .to_sdk(node.clone(), TestingIo); + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { ctx.submit(tx, args).await }) + .wrap_err("Failed to submit tx") +} + +pub fn dummy_args(node: &MockNode) -> args::Tx { + use std::str::FromStr; + args::Tx { + dry_run: false, + dry_run_wrapper: false, + dump_tx: false, + dump_wrapper_tx: false, + output_folder: None, + force: false, + broadcast_only: false, + ledger_address: tendermint_rpc::Url::from_str("http://127.0.0.1:26567") + .unwrap(), + initialized_account_alias: None, + wallet_alias_force: false, + fee_amount: None, + wrapper_fee_payer: None, + fee_token: node.native_token(), + gas_limit: GasLimit::from(1000000), + expiration: TxExpiration::NoExpiration, + chain_id: Some(node.chain_id()), + signing_keys: vec![], + tx_reveal_code_path: Default::default(), + password: None, + memo: None, + use_device: false, + device_transport: Default::default(), + } +} diff --git a/crates/node/src/shell/testing/node.rs b/crates/node/src/shell/testing/node.rs index cf26fc2e4ee..968b3937746 100644 --- a/crates/node/src/shell/testing/node.rs +++ b/crates/node/src/shell/testing/node.rs @@ -8,10 +8,13 @@ use std::task::Poll; use color_eyre::eyre::{Report, Result}; use data_encoding::HEXUPPER; +use eyre::{WrapErr, eyre}; use itertools::Either; use lazy_static::lazy_static; +use namada_apps_lib::cli::Context; +use namada_apps_lib::cli::args::Global; use namada_sdk::address::Address; -use namada_sdk::chain::{BlockHeader, BlockHeight, Epoch}; +use namada_sdk::chain::{BlockHeader, BlockHeight, ChainId, Epoch}; use namada_sdk::collections::HashMap; use namada_sdk::control_flow::time::Duration; use namada_sdk::eth_bridge::oracle::config::Config as OracleConfig; @@ -21,7 +24,7 @@ use namada_sdk::events::extend::Height as HeightAttr; use namada_sdk::events::log::dumb_queries; use namada_sdk::hash::Hash; use namada_sdk::io::Client; -use namada_sdk::key::tm_consensus_key_raw_hash; +use namada_sdk::key::{common, tm_consensus_key_raw_hash}; use namada_sdk::proof_of_stake::storage::{ read_consensus_validator_set_addresses_with_stake, read_pos_params, validator_consensus_key_handle, @@ -49,7 +52,7 @@ use crate::ethereum_oracle::test_tools::mock_web3_client::{ use crate::ethereum_oracle::{ control, last_processed_block, try_process_eth_events, }; -use crate::shell::testing::utils::TestDir; +use crate::shell::testing::utils::{TestDir, TestingIo}; use crate::shell::token::MaspEpoch; use crate::shell::{EthereumOracleChannels, Shell}; use crate::shims::abcipp_shim_types::shim::request::{ @@ -307,6 +310,10 @@ impl Drop for SalvageableTestDir { } impl MockNode { + pub fn chain_id(&self) -> ChainId { + self.shell.lock().unwrap().chain_id.clone() + } + pub async fn handle_service_action(&self, action: MockServiceAction) { match action { MockServiceAction::BroadcastTxs(txs) => { @@ -789,6 +796,76 @@ impl MockNode { } self.clear_results(); } + + pub fn lookup_pk(&self, alias: &str) -> Result { + use namada_sdk::Namada; + let global = { + let locked = self.shell.lock().unwrap(); + Global { + is_pre_genesis: false, + chain_id: Some(locked.chain_id.clone()), + base_dir: locked.base_dir.clone(), + wasm_dir: Some(locked.wasm_dir.clone()), + } + }; + let rt = tokio::runtime::Runtime::new().unwrap(); + let ctx = Context::new::(global.clone()) + .wrap_err("Failed to build context")? + .to_sdk(self.clone(), TestingIo); + rt.block_on(async { + let wallet = ctx.wallet_lock().read().await; + wallet + .find_public_key(alias) + .wrap_err("Could not find the given public key") + }) + } + + pub fn lookup_sk(&self, alias: &str) -> Result { + use namada_sdk::Namada; + let global = { + let locked = self.shell.lock().unwrap(); + Global { + is_pre_genesis: false, + chain_id: Some(locked.chain_id.clone()), + base_dir: locked.base_dir.clone(), + wasm_dir: Some(locked.wasm_dir.clone()), + } + }; + let rt = tokio::runtime::Runtime::new().unwrap(); + let ctx = Context::new::(global.clone()) + .wrap_err("Failed to build context")? + .to_sdk(self.clone(), TestingIo); + rt.block_on(async { + let mut wallet = ctx.wallet_lock().write().await; + wallet + .find_secret_key(alias, None) + .wrap_err("Could not find the given public key") + }) + } + + pub fn lookup_address(&self, alias: &str) -> Result { + use namada_sdk::Namada; + let global = { + let locked = self.shell.lock().unwrap(); + Global { + is_pre_genesis: false, + chain_id: Some(locked.chain_id.clone()), + base_dir: locked.base_dir.clone(), + wasm_dir: Some(locked.wasm_dir.clone()), + } + }; + let rt = tokio::runtime::Runtime::new().unwrap(); + let ctx = Context::new::(global.clone()) + .wrap_err("Failed to build context")? + .to_sdk(self.clone(), TestingIo); + rt.block_on(async { + let wallet = ctx.wallet_lock().read().await; + wallet + .find_address(alias) + .ok_or_else(|| eyre!("Could not find the given public key")) + .map(|a| a.into_owned()) + }) + } } #[async_trait::async_trait(?Send)] diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 0c9df9db0c7..4ce9b467ccc 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -386,6 +386,10 @@ pub struct TxShieldingTransfer { pub targets: Vec>, /// Transfer-specific data pub sources: Vec>, + /// The account which will pay the fees for shielding + pub shielding_fee_payer: C::PublicKey, + /// The token in which the fees for shielding will be paid + pub shielding_fee_token: C::Address, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -693,7 +697,13 @@ impl TxOsmosisSwap { yet" ), }; - + let Some(ibc_shielding_data) = + transfer.ibc_shielding_data.as_ref() + else { + return Err(Error::Other( + "Expected IBC shielding data".to_string(), + )); + }; let shielding_tx = tx::gen_ibc_shielding_transfer( ctx, GenIbcShieldingTransfer { @@ -715,6 +725,12 @@ impl TxOsmosisSwap { ), ), expiration: transfer.tx.expiration.clone(), + shielding_fee_payer: ibc_shielding_data + .shielding_fee_payer + .clone(), + shielding_fee_token: ibc_shielding_data + .shielding_fee_token + .clone(), }, ) .await? @@ -728,7 +744,15 @@ impl TxOsmosisSwap { serde_json::to_value(&NamadaMemo { namada: NamadaMemoData::OsmosisSwap { shielding_data: StringEncoded::new( - IbcShieldingData(shielding_tx), + IbcShieldingData { + masp_tx: shielding_tx, + shielding_fee_payer: ibc_shielding_data + .shielding_fee_payer + .clone(), + shielding_fee_token: ibc_shielding_data + .shielding_fee_token + .clone(), + }, ), shielded_amount: amount_to_shield, overflow_receiver, @@ -3236,6 +3260,10 @@ pub struct GenIbcShieldingTransfer { pub expiration: TxExpiration, /// Asset to shield over IBC to Namada pub asset: IbcShieldingTransferAsset, + /// Account to pay the shielding fee. + pub shielding_fee_payer: C::PublicKey, + /// Token which will be used to pay shielding fee + pub shielding_fee_token: C::Address, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/eth_bridge/bridge_pool.rs b/crates/sdk/src/eth_bridge/bridge_pool.rs index fb919a5a6a5..0a5c03eeada 100644 --- a/crates/sdk/src/eth_bridge/bridge_pool.rs +++ b/crates/sdk/src/eth_bridge/bridge_pool.rs @@ -84,6 +84,7 @@ pub async fn build_bridge_pool_tx( // tx signer Some(sender_), vec![], + None, false, vec![], None diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 48132e3d6e1..b202fcc16f4 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -191,12 +191,16 @@ pub trait Namada: NamadaIo { &self, targets: Vec, sources: Vec, + shielding_fee_payer: common::PublicKey, + shielding_fee_token: Address, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { sources, targets, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), + shielding_fee_payer, + shielding_fee_token, } } diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index e7e7372a372..216e2ba1521 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -264,6 +264,18 @@ pub async fn get_staking_rewards_rate( ) } +/// Query the effective total supply of the native token +pub async fn get_shielding_fee_amount( + client: &C, + token: &Address, +) -> Result { + query_storage_value( + client, + &namada_token::storage_key::masp_shielding_fee_amount(token), + ) + .await +} + /// Check if the given address is a known validator. pub async fn is_validator( client: &C, diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 8acd99ad979..d66fc7567e1 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -64,7 +64,7 @@ use crate::wallet::{Wallet, WalletIo}; use crate::{Namada, args, rpc}; /// A structure holding the signing data to craft a transaction -#[derive(Clone, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct SigningTxData { /// The address owning the transaction pub owner: Option
, @@ -80,6 +80,8 @@ pub struct SigningTxData { pub fee_payer: either::Either<(common::PublicKey, bool), Vec>, /// ID of the Transaction needing signing pub shielded_hash: Option, + /// The key of the account paying for shielding fees + pub shielding_fee_payer: Option, /// List of serialized signatures to attach to the transaction pub signatures: Vec>, } @@ -288,6 +290,23 @@ where } } + if let Some(shielding_fee_payer) = signing_data.shielding_fee_payer.as_ref() + { + let mut wallet = wallet.write().await; + // If the secret key is not found, continue because the + // hardware wallet may still be able to sign this + if let Ok(secret_key) = + find_key_by_pk(&mut wallet, args, shielding_fee_payer) + { + used_pubkeys.insert(shielding_fee_payer.clone()); + tx.add_section(Section::Authorization(Authorization::new( + vec![tx.raw_header_hash()], + (0..).zip(vec![secret_key]).collect(), + None, + ))); + } + } + // Then try to sign the raw header using the hardware wallet for pubkey in &signing_data.public_keys { if !used_pubkeys.contains(pubkey) { @@ -310,6 +329,22 @@ where } } } + // try to sign the shielding fee with the hardware wallet if necessary + if let Some(payer) = signing_data.shielding_fee_payer { + if !used_pubkeys.contains(&payer) { + if let Ok(ntx) = sign( + tx.clone(), + payer.clone(), + Signable::RawHeader, + user_data.clone(), + ) + .await + { + *tx = ntx; + used_pubkeys.insert(payer.clone()); + } + } + } // Before signing the wrapper tx prune all the possible duplicated sections // (including duplicated raw signatures) @@ -381,6 +416,7 @@ pub async fn aux_signing_data( owner: Option
, default_signer: Option
, extra_public_keys: Vec, + shielding_fee_payer: Option, is_shielded_source: bool, signatures: Vec>, wrapper_signature: Option>, @@ -444,6 +480,7 @@ pub async fn aux_signing_data( threshold, account_public_keys_map, fee_payer, + shielding_fee_payer, shielded_hash: None, signatures, }) @@ -2585,6 +2622,7 @@ mod test_signing { account_public_keys_map: Some(Default::default()), fee_payer: either::Either::Left((public_key_fee.clone(), false)), shielded_hash: None, + shielding_fee_payer: None, signatures: vec![], }; @@ -2622,6 +2660,7 @@ mod test_signing { account_public_keys_map: Some(Default::default()), fee_payer: either::Left((public_key.clone(), false)), shielded_hash: None, + shielding_fee_payer: None, signatures: vec![], }; sign_tx( diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index f043f11fe66..151cf78b0fc 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -41,7 +41,7 @@ use namada_core::ibc::core::client::types::Height as IbcHeight; use namada_core::ibc::core::host::types::identifiers::{ChannelId, PortId}; use namada_core::ibc::primitives::{IntoTimestamp, Timestamp as IbcTimestamp}; use namada_core::key::{self, *}; -use namada_core::masp::{AssetData, MaspEpoch, TransferSource, TransferTarget}; +use namada_core::masp::{AssetData, MaspEpoch, MaspTxId, TransferSource, TransferTarget}; use namada_core::storage; use namada_core::time::DateTimeUtc; use namada_events::extend::EventAttributeEntry; @@ -55,7 +55,7 @@ use namada_governance::storage::proposal::{ use namada_governance::storage::vote::ProposalVote; use namada_ibc::storage::channel_key; use namada_ibc::trace::is_nft_trace; -use namada_ibc::{MsgNftTransfer, MsgTransfer}; +use namada_ibc::{IbcShieldingData, MsgNftTransfer, MsgTransfer}; use namada_io::{Client, Io, display_line, edisplay_line}; use namada_proof_of_stake::parameters::{ MAX_VALIDATOR_METADATA_LEN, PosParams, @@ -84,7 +84,8 @@ use crate::rpc::{ query_wasm_code_hash, validate_amount, }; use crate::signing::{ - self, SigningTxData, validate_fee, validate_transparent_fee, + self, SigningTxData, TxSourcePostBalance, validate_fee, + validate_transparent_fee, }; use crate::tendermint_rpc::endpoint::broadcast::tx_sync::Response; use crate::tendermint_rpc::error::Error as RpcError; @@ -332,6 +333,7 @@ pub async fn build_reveal_pk( None, Some(public_key.into()), vec![], + None, false, vec![], None, @@ -701,6 +703,7 @@ pub async fn build_change_consensus_key( None, None, vec![consensus_key.clone()], + None, false, vec![], None, @@ -741,6 +744,7 @@ pub async fn build_validator_commission_change( Some(validator.clone()), default_signer, vec![], + None, false, vec![], None, @@ -886,6 +890,7 @@ pub async fn build_validator_metadata_change( Some(validator.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1114,6 +1119,7 @@ pub async fn build_update_steward_commission( Some(steward.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1185,6 +1191,7 @@ pub async fn build_resign_steward( Some(steward.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1236,6 +1243,7 @@ pub async fn build_unjail_validator( Some(validator.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1343,6 +1351,7 @@ pub async fn build_deactivate_validator( Some(validator.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1421,6 +1430,7 @@ pub async fn build_reactivate_validator( Some(validator.clone()), default_signer, vec![], + None, false, vec![], None, @@ -1651,6 +1661,7 @@ pub async fn build_redelegation( Some(default_address), default_signer, vec![], + None, false, vec![], None, @@ -1698,6 +1709,7 @@ pub async fn build_withdraw( Some(default_address), default_signer, vec![], + None, false, vec![], None, @@ -1788,6 +1800,7 @@ pub async fn build_claim_rewards( Some(default_address), default_signer, vec![], + None, false, vec![], None, @@ -1895,6 +1908,7 @@ pub async fn build_unbond( Some(default_address), default_signer, vec![], + None, false, vec![], None, @@ -2133,6 +2147,7 @@ pub async fn build_bond( Some(default_address.clone()), default_signer, vec![], + None, false, vec![], None, @@ -2201,6 +2216,7 @@ pub async fn build_default_proposal( Some(proposal.proposal.author.clone()), default_signer, vec![], + None, false, vec![], None, @@ -2265,6 +2281,7 @@ pub async fn build_vote_proposal( default_signer.clone(), default_signer.clone(), vec![], + None, false, vec![], None, @@ -2577,6 +2594,7 @@ pub async fn build_become_validator( None, None, all_pks, + None, false, vec![], None, @@ -2619,6 +2637,7 @@ pub async fn build_pgf_funding_proposal( Some(proposal.proposal.author.clone()), default_signer, vec![], + None, false, vec![], None, @@ -2669,6 +2688,7 @@ pub async fn build_pgf_stewards_proposal( Some(proposal.proposal.author.clone()), default_signer, vec![], + None, false, vec![], None, @@ -2725,23 +2745,50 @@ pub async fn build_ibc_transfer( Some(source.clone()), Some(source.clone()), vec![], + if let Some(IbcShieldingData { + shielding_fee_payer: payer, + .. + }) = &args.ibc_shielding_data + { + Some(payer.clone()) + } else { + None + }, args.source.spending_key().is_some(), vec![], None, ) .await?; let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_per_gas_unit, updated_balance) = + let (fee_per_gas_unit, updated_balance, shielding_fee_payer) = if let TransferSource::ExtendedKey(_) = args.source { // MASP fee payment - (validate_fee(context, &args.tx).await?, None) + (validate_fee(context, &args.tx).await?, None, None) } else { // Transparent fee payment - validate_transparent_fee(context, &args.tx, fee_payer) - .await - .map(|(fee_amount, updated_balance)| { - (fee_amount, Some(updated_balance)) - })? + let (fee_per_gas_unit, mut updated_balance) = + validate_transparent_fee(context, &args.tx, fee_payer).await?; + // check that if there is a shielding fee payer, they have enough + // balance to cover the fee + + let shielding_fee_payer = if let Some(IbcShieldingData { + shielding_fee_payer: payer, + shielding_fee_token, + .. + }) = &args.ibc_shielding_data + { + validate_shielding_fee( + context, + Some(&mut updated_balance), + payer, + shielding_fee_token, + args.tx.force, + ) + .await? + } else { + None + }; + (fee_per_gas_unit, Some(updated_balance), shielding_fee_payer) }; // Check that the source address exists on chain @@ -2762,6 +2809,14 @@ pub async fn build_ibc_transfer( && updated_balance.token == args.token { CheckBalance::Balance(updated_balance.post_balance) + } else if let Some(updated_fee_payer) = &shielding_fee_payer { + if updated_fee_payer.source == source + && updated_fee_payer.token == args.token + { + CheckBalance::Balance(updated_fee_payer.post_balance) + } else { + CheckBalance::Query(balance_key(&args.token, &source)) + } } else { CheckBalance::Query(balance_key(&args.token, &source)) }; @@ -2869,6 +2924,18 @@ pub async fn build_ibc_transfer( if let Some(memo) = &args.tx.memo { tx.add_memo(memo); } + if let Some(IbcShieldingData { + shielding_fee_payer: payer, + shielding_fee_token: token, + masp_tx, + }) = &args.ibc_shielding_data + { + tx.add_section(Section::ShieldingFee { + payer: payer.clone(), + token: token.clone(), + cmt: MaspTxId::from(masp_tx.txid()), + }); + } let transfer = shielded_parts .map(|(shielded_transfer, asset_types)| { @@ -2979,6 +3046,81 @@ pub async fn build_ibc_transfer( Ok((tx, signing_data, shielded_tx_epoch)) } +/// Check if a shielding fee payer is necessary. If so, +/// guarantee one is provided and has sufficient balance. +/// If it is the same account as the provided `updated_balance` +/// arg, fold the change into that struct. Otherwise, return +/// a new updated balance. +pub async fn validate_shielding_fee( + context: &N, + updated_balance: Option<&mut TxSourcePostBalance>, + shielding_fee_payer: &common::PublicKey, + shielding_fee_token: &Address, + force: bool, +) -> Result> { + let payer = Address::from(shielding_fee_payer); + + let shielding_fee = + rpc::get_shielding_fee_amount(context.client(), shielding_fee_token) + .await + .map_err(|_| { + Error::Other(format!( + "The MASP shielding fee cannot be paid with token \ + {shielding_fee_token}" + )) + })?; + // fee payer is the same as shielded fee payer and paying with the same + // token + let Some(updated_balance) = updated_balance else { + return Some( + check_balance_too_low_err( + shielding_fee_token, + &payer, + shielding_fee.amount(), + CheckBalance::Query(balance_key(shielding_fee_token, &payer)), + force, + context, + ) + .await, + ) + .transpose(); + }; + if updated_balance.token == *shielding_fee_token + && updated_balance.source == payer + { + updated_balance.post_balance = + checked!(updated_balance.post_balance - shielding_fee.amount()) + .map_err(|_| { + Error::Other(format!( + "{} does not have enough balance to pay for fees and \ + shielding fees. Short by {} {}", + updated_balance.source, + checked!( + shielding_fee.amount() + - updated_balance.post_balance + ) + .unwrap(), + updated_balance.token + )) + })?; + Ok(None) + } else { + // check if the shielding fee payer has enough balance + Some( + check_balance_too_low_err( + shielding_fee_token, + &payer, + shielding_fee.amount(), + CheckBalance::Query(balance_key(shielding_fee_token, &payer)), + force, + context, + ) + .await, + ) + .transpose() + } +} + /// Abstraction for helping build transactions #[allow(clippy::too_many_arguments)] async fn build( @@ -3137,6 +3279,7 @@ pub async fn build_transparent_transfer( source.clone(), source, vec![], + None, false, vec![], None, @@ -3242,6 +3385,7 @@ pub async fn build_shielded_transfer( Some(MASP), Some(MASP), vec![], + None, true, vec![], None, @@ -3425,6 +3569,7 @@ pub async fn build_shielding_transfer( source.clone(), source, vec![], + Some(args.shielding_fee_payer.clone()), false, vec![], None, @@ -3433,12 +3578,20 @@ pub async fn build_shielding_transfer( // Transparent fee payment let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, updated_balance) = + let (fee_amount, mut updated_balance) = validate_transparent_fee(context, &args.tx, fee_payer) .await .map(|(fee_amount, updated_balance)| { (fee_amount, Some(updated_balance)) })?; + let shielding_fee_balance = validate_shielding_fee( + context, + updated_balance.as_mut(), + &args.shielding_fee_payer, + &args.shielding_fee_token, + args.tx.force, + ) + .await?; let mut transfer_data = MaspTransferData::default(); let mut data = token::Transfer::default(); @@ -3455,10 +3608,18 @@ pub async fn build_shielding_transfer( // Check the balance of the source if let Some(updated_balance) = &updated_balance { - let check_balance = if &updated_balance.source == source - && &updated_balance.token == token + let check_balance = if updated_balance.source == *source + && updated_balance.token == *token { CheckBalance::Balance(updated_balance.post_balance) + } else if let Some(updated_fee_payer) = &shielding_fee_balance { + if updated_fee_payer.source == *source + && updated_fee_payer.token == *token + { + CheckBalance::Balance(updated_fee_payer.post_balance) + } else { + CheckBalance::Query(balance_key(token, source)) + } } else { CheckBalance::Query(balance_key(token, source)) }; @@ -3544,6 +3705,14 @@ pub async fn build_shielding_transfer( data.shielded_section_hash = Some(shielded_section_hash); signing_data.shielded_hash = Some(shielded_section_hash); + + + tx.add_section(Section::ShieldingFee { + payer: args.shielding_fee_payer.clone(), + token: args.shielding_fee_token.clone(), + cmt: shielded_section_hash + }); + tracing::debug!("Transfer data {data:?}"); Ok(()) }; @@ -3573,6 +3742,7 @@ pub async fn build_unshielding_transfer( Some(MASP), Some(MASP), vec![], + None, true, vec![], None, @@ -3766,6 +3936,7 @@ pub async fn build_init_account( None, None, vec![], + None, false, vec![], None, @@ -3857,6 +4028,7 @@ pub async fn build_update_account( Some(addr.clone()), default_signer, vec![], + None, false, vec![], None, @@ -4057,6 +4229,7 @@ pub async fn build_custom( owner.clone(), default_signer, vec![], + None, false, signatures.to_owned(), None, @@ -4360,6 +4533,9 @@ enum CheckBalance { /// Checks the balance at the given address is enough to transfer the /// given amount, along with the balance even existing. Force /// overrides this. +/// +/// Returns the balance after the amount has been debited if +/// successful. async fn check_balance_too_low_err( token: &Address, source: &Address, @@ -4367,7 +4543,7 @@ async fn check_balance_too_low_err( balance: CheckBalance, force: bool, context: &N, -) -> Result<()> { +) -> Result { let balance = match balance { CheckBalance::Balance(amt) => amt, CheckBalance::Query(ref balance_key) => { @@ -4388,7 +4564,11 @@ async fn check_balance_too_low_err( source, token ); - return Ok(()); + return Ok(TxSourcePostBalance { + post_balance: Default::default(), + source: source.clone(), + token: token.clone(), + }); } else { return Err(Error::from( TxSubmitError::NoBalanceForToken( @@ -4406,7 +4586,11 @@ async fn check_balance_too_low_err( }; match balance.checked_sub(amount) { - Some(_) => Ok(()), + Some(amt) => Ok(TxSourcePostBalance { + post_balance: amt, + source: source.clone(), + token: token.clone(), + }), None => { if force { edisplay_line!( @@ -4419,7 +4603,11 @@ async fn check_balance_too_low_err( context.format_amount(token, amount).await, context.format_amount(token, balance).await, ); - Ok(()) + Ok(TxSourcePostBalance { + post_balance: Default::default(), + source: source.clone(), + token: token.clone(), + }) } else { Err(Error::from(TxSubmitError::BalanceTooLow( source.clone(), diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index ad6f9505001..7f3feef9064 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -48,6 +48,8 @@ pub const MASP_KD_GAIN_KEY: &str = "derivative_gain"; pub const MASP_LOCKED_AMOUNT_TARGET_KEY: &str = "locked_amount_target"; /// The key for the max reward rate for a given asset pub const MASP_MAX_REWARD_RATE_KEY: &str = "max_reward_rate"; +/// The key for looking up the shielding fee amount in a given token +pub const MASP_SHIELDING_FEE_PREFIX: &str = "shielding_fee"; /// The key for the total inflation rewards minted by MASP pub const MASP_TOTAL_REWARDS: &str = "max_total_rewards"; /// The key for the reward precision for a given asset @@ -388,3 +390,13 @@ pub fn masp_total_rewards() -> storage::Key { .push(&MASP_TOTAL_REWARDS.to_owned()) .expect("Cannot obtain a storage key") } + +/// The key for getting the shielding fee amount of the provided +/// token. +pub fn masp_shielding_fee_amount(token: &Address) -> storage::Key { + storage::Key::from(address::MASP.to_db_key()) + .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(token) + .expect("Cannot obtain a storage key") +} diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 1875937c5c6..c112c932c24 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -625,8 +625,13 @@ where } } // The transaction shall not push masp authorizer actions that are not - // needed cause this might lead vps to run a wrong validation logic - if !actions_authorizers.is_empty() { + // needed because this might lead vps to run a wrong validation logic + if !actions_authorizers.is_empty() + && shielded_tx + .transparent_bundle() + .map(|b| b.vin.is_empty()) + .unwrap_or(true) + { let error = Error::new_const( "Found masp authorizer actions that are not required", ); diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index c624a7f8ccb..87f331c8712 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::path::PathBuf; use std::str::FromStr; +use borsh::BorshDeserialize; use color_eyre::eyre::Result; use color_eyre::owo_colors::OwoColorize; use itertools::Either; @@ -13,9 +14,12 @@ use namada_apps_lib::wallet::defaults::{ get_unencrypted_keypair, is_use_device, }; use namada_core::address::Address; +use namada_core::collections::HashSet; use namada_core::dec::Dec; use namada_core::masp::{MaspTxId, Precision, TokenMap, encode_asset_type}; -use namada_node::shell::testing::client::run; +use namada_node::shell::testing::client::{ + dummy_args, run, sign_tx, submit_custom, +}; use namada_node::shell::testing::node::NodeResults; use namada_node::shell::testing::utils::{Bin, CapturedOutput}; use namada_sdk::account::AccountPublicKeysMap; @@ -42,7 +46,10 @@ use crate::e2e::setup::constants::{ C_SPENDING_KEY, CHRISTEL, CHRISTEL_KEY, ETH, FRANK_KEY, MASP, NAM, }; use crate::integration::helpers::make_temp_account; +use crate::integration::ledger_tests::find_files_with_ext; use crate::strings::TX_APPLIED_SUCCESS; +use crate::token::storage_key::masp_shielding_fee_amount; +use crate::tx::Authorization; /// Enable masp rewards before some token is shielded, /// but the max reward rate is null. @@ -126,6 +133,10 @@ fn init_null_rewards() -> Result<()> { TEST_TOKEN_ADDR, "--amount", "1000000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -490,6 +501,10 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { TEST_TOKEN_ADDR, "--amount", HALF_TEST_TOKEN_INITIAL_SUPPLY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -659,6 +674,10 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { TEST_TOKEN_ADDR, "--amount", HALF_TEST_TOKEN_INITIAL_SUPPLY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -805,6 +824,10 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { "1", "--gas-payer", BERTHA_KEY, + "--shielding-fee-payer", + CHRISTEL_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -967,6 +990,10 @@ fn enable_rewards_after_shielding() -> Result<()> { TEST_TOKEN_ADDR, "--amount", "1000000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -1190,6 +1217,10 @@ fn enable_rewards_after_shielding() -> Result<()> { TEST_TOKEN_ADDR, "--amount", "1000000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", RPC, ]), @@ -1451,6 +1482,538 @@ fn enable_rewards_after_shielding() -> Result<()> { Ok(()) } +/// This tests that shielding fees are enforced. We test the following +/// cases +/// +/// 1. Test the happy flow. +/// 2. Test that if the shielding fee section is missing from the tx, it is +/// rejected +/// 3. Test that if the shielding fee holds the wrong Masp Tx id, it is rejected. +/// 4. Test that if the account listed in the shielding fee does not sign the +/// tx, it is rejected +/// +#[test] +fn test_shielding_fee_protocol_checks() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + let native_token = node.native_token(); + node.shell + .lock() + .unwrap() + .state + .write( + &masp_shielding_fee_amount(&native_token), + DenominatedAmount::native(Amount::native_whole(2000)), + ) + .expect("Test failed"); + node.next_masp_epoch(); + // test that the shielding fee debits from the correct account + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + BERTHA_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // 1998000 = 2000000 - 2000 + assert!(captured.contains("nam: 2000000")); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + BERTHA_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // 1998000 = 2000000 - 2000 + assert!(captured.contains("nam: 1998000")); + + // we now show that signing and submitting manually is successful + let output_folder = node.test_dir.path(); + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--dump-wrapper-tx", + "--output-folder-path", + output_folder.to_str().unwrap(), + "--node", + validator_one_rpc, + ]), + )?; + let tx_path = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + let tx: Tx = serde_json::from_reader( + std::fs::File::open(tx_path.clone()).expect("Test Failed"), + ) + .expect("Test failed"); + let signing_data = SigningTxData { + owner: Some(node.lookup_address(ALBERT)?), + public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), + threshold: 1, + account_public_keys_map: Some( + [node.lookup_pk(ALBERT_KEY)?].into_iter().collect(), + ), + fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), + shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + signatures: vec![], + }; + let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; + let captured = + CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + std::fs::remove_file(tx_path).expect("Test failed"); + + // we remove the shielding fee section + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--dump-wrapper-tx", + "--output-folder-path", + output_folder.to_str().unwrap(), + "--node", + validator_one_rpc, + ]), + )?; + let tx_path = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + let tx: Tx = serde_json::from_reader( + std::fs::File::open(tx_path.clone()).expect("Test Failed"), + ) + .expect("Test failed"); + let signing_data = SigningTxData { + owner: Some(node.lookup_address(ALBERT)?), + public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), + threshold: 1, + account_public_keys_map: Some( + [node.lookup_pk(ALBERT_KEY)?].into_iter().collect(), + ), + fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), + shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + signatures: vec![], + }; + let mut signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; + signed + .sections + .retain(|s| !matches!(s, Section::ShieldingFee { .. })); + let captured = + CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); + assert!(captured.result.is_ok()); + assert!( + captured.contains( + "A fee payer was not provided for a shielding transaction" + ) + ); + std::fs::remove_file(tx_path).expect("Test failed"); + + // now we make the shielding fee section contain the wrong masp tx id + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--dump-wrapper-tx", + "--output-folder-path", + output_folder.to_str().unwrap(), + "--node", + validator_one_rpc, + ]), + )?; + let tx_path = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + let mut tx: Tx = serde_json::from_reader( + std::fs::File::open(tx_path.clone()).expect("Test Failed"), + ) + .expect("Test failed"); + let signing_data = SigningTxData { + owner: Some(node.lookup_address(ALBERT)?), + public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), + threshold: 1, + account_public_keys_map: Some( + [node.lookup_pk(ALBERT_KEY)?].into_iter().collect(), + ), + fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), + shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + signatures: vec![], + }; + for sec in tx.sections.iter_mut() { + if let Section::ShieldingFee {cmt, ..} = sec { + *cmt = MaspTxId::from(masp_primitives::transaction::TxId::from_bytes([0u8; 32])); + } + } + let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; + + let captured = + CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); + assert!(captured.result.is_ok()); + assert!( + captured.contains( + "A fee payer was not provided for a shielding transaction" + ) + ); + std::fs::remove_file(tx_path).expect("Test failed"); + + // now we sign the shielding fee with the wrong key + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--dump-wrapper-tx", + "--output-folder-path", + output_folder.to_str().unwrap(), + "--node", + validator_one_rpc, + ]), + )?; + let tx_path = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + let tx: Tx = serde_json::from_reader( + std::fs::File::open(tx_path).expect("Test Failed"), + ) + .expect("Test failed"); + let signing_data = SigningTxData { + owner: Some(node.lookup_address(ALBERT)?), + public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), + threshold: 1, + account_public_keys_map: Some( + [node.lookup_pk(ALBERT_KEY)?].into_iter().collect(), + ), + fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), + shielded_hash: None, + // this should be Bertha's key + shielding_fee_payer: Some(node.lookup_pk(ALBERT_KEY)?), + signatures: vec![], + }; + let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; + let captured = + CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); + assert!(captured.result.is_ok()); + assert!(captured.contains("The section signature is invalid")); + + Ok(()) +} + +/// Test the client balance checks for shielding fees +/// +/// Check insufficient balance when +/// 1. Shielding fee payer is unique +/// 2. Check when fee payer and shielding fee payer are the same +/// 3. Check when shielding account and shielding fee payer are the same +/// 4. All three are the same +/// +/// Check when paying with token not whitelisted for shielding fee payments +#[test] +fn test_shielding_fee_client_balance_checks() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + let native_token = node.native_token(); + node.shell + .lock() + .unwrap() + .state + .write( + &masp_shielding_fee_amount(&native_token), + DenominatedAmount::native(Amount::native_whole(2000001)), + ) + .expect("Test failed"); + node.next_masp_epoch(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains(r"balance of the source \w+ of token \w+ is lower than the amount to be transferred.")); + node.shell + .lock() + .unwrap() + .state + .write( + &masp_shielding_fee_amount(&native_token), + DenominatedAmount::native(Amount::native_whole(2000000)), + ) + .expect("Test failed"); + node.next_masp_epoch(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + BERTHA_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "does not have enough balance to pay for fees and shielding fees." + )); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + BERTHA_KEY, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains(r"balance of the source \w+ of token \w+ is lower than the amount to be transferred.")); + node.shell + .lock() + .unwrap() + .state + .write( + &masp_shielding_fee_amount(&native_token), + DenominatedAmount::native(Amount::native_whole(1000000)), + ) + .expect("Test failed"); + node.next_masp_epoch(); + + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + BERTHA_KEY, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000000", + "--signing-keys", + BERTHA_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains(r"balance of the source \w+ of token \w+ is lower than the amount to be transferred.")); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT_KEY, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + BTC, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!( + captured.contains("The MASP shielding fee cannot be paid with token") + ); + Ok(()) +} + /// In this test we verify that the results of auto-compounding are /// approximately equal to what is obtained by manually unshielding and /// reshielding each time. @@ -1482,6 +2045,10 @@ fn auto_compounding() -> Result<()> { ALBERT_KEY, "--node", validator_one_rpc, + "--shielding-fee-payer", + ALBERT_KEY, + "--shielding-fee-token", + NAM, ]), ) }); @@ -1507,6 +2074,10 @@ fn auto_compounding() -> Result<()> { ALBERT_KEY, "--node", validator_one_rpc, + "--shielding-fee-payer", + ALBERT_KEY, + "--shielding-fee-token", + NAM, ]), ) }); @@ -1731,6 +2302,10 @@ fn auto_compounding() -> Result<()> { bal_b, "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + ALBERT_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -1835,6 +2410,10 @@ fn base_precision_effective() -> Result<()> { "0.1", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -2001,6 +2580,10 @@ fn reset_conversions() -> Result<()> { "1", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -2478,6 +3061,10 @@ fn dynamic_precision() -> Result<()> { "1", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -2836,6 +3423,10 @@ fn dynamic_precision() -> Result<()> { "1", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -3061,6 +3652,10 @@ fn masp_incentives() -> Result<()> { "1", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -3341,6 +3936,10 @@ fn masp_incentives() -> Result<()> { "0.001", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -3912,6 +4511,10 @@ fn spend_unconverted_asset_type() -> Result<()> { BTC, "--amount", "20", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -3936,6 +4539,10 @@ fn spend_unconverted_asset_type() -> Result<()> { NAM, "--amount", "0.000001", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -4106,6 +4713,10 @@ fn masp_txs_and_queries() -> Result<()> { BTC, "--amount", "20", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -4386,6 +4997,10 @@ fn multiple_unfetched_txs_same_block() -> Result<()> { NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -4408,6 +5023,10 @@ fn multiple_unfetched_txs_same_block() -> Result<()> { NAM, "--amount", "200", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -4430,6 +5049,10 @@ fn multiple_unfetched_txs_same_block() -> Result<()> { NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -4627,6 +5250,10 @@ fn expired_masp_tx() -> Result<()> { NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -4796,6 +5423,10 @@ fn cross_epoch_unshield() -> Result<()> { "1000", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -4940,6 +5571,10 @@ fn dynamic_assets() -> Result<()> { BTC, "--amount", "1", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -5076,6 +5711,10 @@ fn dynamic_assets() -> Result<()> { BTC, "--amount", "1", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -5544,6 +6183,10 @@ fn masp_fee_payment() -> Result<()> { "500000", "--gas-payer", CHRISTEL_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -5818,6 +6461,10 @@ fn masp_fee_payment_gas_limit() -> Result<()> { NAM, "--amount", "1000000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -5940,6 +6587,10 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { // Pay gas transparently "--gas-payer", BERTHA_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6097,6 +6748,10 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { NAM, "--amount", "10000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6118,6 +6773,10 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { NAM, "--amount", "300000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6298,6 +6957,10 @@ fn masp_fee_payment_with_different_token() -> Result<()> { NAM, "--amount", "1", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6321,6 +6984,10 @@ fn masp_fee_payment_with_different_token() -> Result<()> { "1000", "--gas-payer", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6344,6 +7011,10 @@ fn masp_fee_payment_with_different_token() -> Result<()> { "300000", "--gas-payer", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6566,10 +7237,14 @@ fn identical_output_descriptions() -> Result<()> { "--gas-payer", bradley_alias.as_ref(), "--gas-limit", - "60000", + "70000", "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -6601,6 +7276,7 @@ fn identical_output_descriptions() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -6617,8 +7293,15 @@ fn identical_output_descriptions() -> Result<()> { ), None, ); + // signatures for the shielded fee sections + batched_tx.add_section(Section::Authorization(Authorization::new( + vec![batched_tx.raw_header_hash()], + (0..).zip(vec![node.lookup_sk(BERTHA_KEY)?]).collect(), + None, + ))); batched_tx.sign_wrapper(bradley_key); + let wrapper_hash = batched_tx.wrapper_hash(); let inner_cmts = batched_tx.commitments(); @@ -6870,12 +7553,16 @@ fn masp_batch() -> Result<()> { "--amount", "1000", "--gas-limit", - "60000", + "80000", "--gas-payer", cooper_alias.as_ref(), "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ], @@ -6906,6 +7593,7 @@ fn masp_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -6945,13 +7633,18 @@ fn masp_batch() -> Result<()> { ), None, ); + // signatures for the shielded fee sections + batched_tx.add_section(Section::Authorization(Authorization::new( + vec![batched_tx.raw_header_hash()], + (0..).zip(vec![node.lookup_sk(BERTHA_KEY)?]).collect(), + None, + ))); batched_tx.sign_wrapper(cooper_key.clone()); wrapper_hashes.push(batched_tx.wrapper_hash()); for cmt in batched_tx.commitments() { inner_cmts.push(cmt.to_owned()); } - txs.push(batched_tx.to_bytes()); } @@ -7033,7 +7726,7 @@ fn masp_batch() -> Result<()> { ], )?; - // Assert NAM balances at VK(A), Bob and Bertha + // Assert NAM balances at VK(A), Adam and Bradley for (owner, balance) in [ (AA_VIEWING_KEY, 2_000), (adam_alias.as_ref(), 498_000), @@ -7127,12 +7820,16 @@ fn masp_atomic_batch() -> Result<()> { "--amount", "1000", "--gas-limit", - "60000", + "80000", "--gas-payer", cooper_alias.as_ref(), "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ], @@ -7162,6 +7859,7 @@ fn masp_atomic_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -7201,6 +7899,12 @@ fn masp_atomic_batch() -> Result<()> { ), None, ); + // signatures for the shielded fee sections + batched_tx.add_section(Section::Authorization(Authorization::new( + vec![batched_tx.raw_header_hash()], + (0..).zip(vec![node.lookup_sk(BERTHA_KEY)?]).collect(), + None, + ))); batched_tx.sign_wrapper(cooper_key.clone()); wrapper_hashes.push(batched_tx.wrapper_hash()); @@ -7369,6 +8073,10 @@ fn masp_failing_atomic_batch() -> Result<()> { NAM, "--amount", "1000", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -7506,6 +8214,7 @@ fn masp_failing_atomic_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, + shielding_fee_payer: None, signatures: vec![], }; @@ -7700,6 +8409,10 @@ fn tricky_masp_txs() -> Result<()> { "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ], @@ -7774,6 +8487,11 @@ fn tricky_masp_txs() -> Result<()> { None, ); tx0.sign_wrapper(get_unencrypted_keypair("frank-key")); + let cmt = tx0.first_commitments().unwrap(); + + let data = tx0.data(cmt).unwrap(); + let transfers = token::Transfer::try_from_slice(&data[..]).unwrap(); + assert!(transfers.shielded_section_hash.is_none()); // Generate second tx let captured = CapturedOutput::of(|| { @@ -7795,6 +8513,10 @@ fn tricky_masp_txs() -> Result<()> { "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", + "--shielding-fee-payer", + CHRISTEL_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ], @@ -7824,6 +8546,12 @@ fn tricky_masp_txs() -> Result<()> { ), None, ); + // signatures for the shielded fee sections + tx1.add_section(Section::Authorization(Authorization::new( + vec![tx1.raw_header_hash()], + (0..).zip(vec![node.lookup_sk(CHRISTEL_KEY)?]).collect(), + None, + ))); tx1.sign_wrapper(get_unencrypted_keypair("frank-key")); let txs = vec![tx0.to_bytes(), tx1.to_bytes()]; @@ -7902,6 +8630,10 @@ fn speculative_context() -> Result<()> { NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -8137,7 +8869,7 @@ fn speculative_context() -> Result<()> { Ok(()) } -// Test that mixed masp tranfers and fee payments are correctly labeld by the +// Test that mixed masp transfers and fee payments are correctly labeled by the // protocol (by means of events) and reconstructed in the correct order by the // client #[test] @@ -8187,6 +8919,10 @@ fn masp_events() -> Result<()> { NAM, "--amount", "500", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), @@ -8269,6 +9005,10 @@ fn masp_events() -> Result<()> { "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -8415,6 +9155,7 @@ fn masp_events() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((cooper_pk.clone(), false)), shielded_hash: None, + shielding_fee_payer: None, signatures: vec![], }; @@ -8517,6 +9258,14 @@ fn masp_events() -> Result<()> { AccountPublicKeysMap::from_iter(vec![(pk)].into_iter()), None, ); + if idx == 0 { + // signatures for the shielded fee sections + tx.add_section(Section::Authorization(Authorization::new( + vec![tx.raw_header_hash()], + (0..).zip(vec![node.lookup_sk(BERTHA_KEY)?]).collect(), + None, + ))); + } tx.sign_wrapper(sk); txs.push(tx.to_bytes()); @@ -8765,6 +9514,10 @@ fn multiple_inputs_from_single_note() -> Result<()> { "10", "--signing-keys", ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", validator_one_rpc, ]), diff --git a/crates/tests/src/integration/setup.rs b/crates/tests/src/integration/setup.rs index 2479e13d905..e20a68fc82d 100644 --- a/crates/tests/src/integration/setup.rs +++ b/crates/tests/src/integration/setup.rs @@ -18,6 +18,7 @@ use namada_apps_lib::wallet::defaults::derive_template_dir; use namada_apps_lib::wallet::pre_genesis; use namada_core::chain::ChainIdPrefix; use namada_core::collections::HashMap; +use namada_core::token::{Amount, DenominatedAmount}; use namada_node::shell::Shell; use namada_node::shell::testing::node::{ InnerMockNode, MockNode, MockServicesCfg, MockServicesController, @@ -29,6 +30,8 @@ use namada_sdk::token; use namada_sdk::wallet::alias::Alias; use crate::e2e::setup::copy_wasm_to_chain_dir; +use crate::proof_of_stake::StorageWrite; +use crate::token::storage_key::masp_shielding_fee_amount; /// Env. var for keeping temporary files created by the integration tests const ENV_VAR_KEEP_TEMP: &str = "NAMADA_INT_KEEP_TEMP"; @@ -271,6 +274,17 @@ fn create_node( locked.state.in_mem_mut().block.height = 1.into(); locked.commit(); } + // enable shielding fees + let native_token = node.native_token(); + node.shell + .lock() + .unwrap() + .state + .write( + &masp_shielding_fee_amount(&native_token), + DenominatedAmount::native(Amount::native_whole(1)), + ) + .expect("Test failed"); Ok((node, controller)) } diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 937292c2e33..a2c99b5d45f 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -58,8 +58,8 @@ pub mod storage_key { masp_base_native_precision_key, masp_commitment_anchor_key, masp_commitment_tree_key, masp_conversion_key, masp_convert_anchor_key, masp_nullifier_key, masp_scheduled_base_native_precision_key, - masp_scheduled_reward_precision_key, masp_token_map_key, - masp_total_rewards, + masp_scheduled_reward_precision_key, masp_shielding_fee_amount, + masp_token_map_key, masp_total_rewards, }; pub use namada_trans_token::storage_key::*; diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index d60eeb1a247..f8a4d88e1c0 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -3,15 +3,18 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; +use namada_core::address::PGF; use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; use namada_core::masp::encode_asset_type; use namada_core::masp_primitives::transaction::Transaction; -use namada_core::token::MaspDigitPos; +use namada_core::token::{DenominatedAmount, MaspDigitPos}; use namada_core::uint::I320; use namada_core::{masp, token}; use namada_events::EmitEvents; -use namada_shielded_token::storage_key::masp_undated_balance_key; +use namada_shielded_token::storage_key::{ + masp_shielding_fee_amount, masp_undated_balance_key, +}; use namada_shielded_token::{MaspTxId, read_undated_balance, utils}; use namada_storage::{Error, OptionExt, ResultExt}; use namada_trans_token::read_denom; @@ -20,7 +23,7 @@ use namada_tx::BatchedTx; use namada_tx::action::{self, Action, MaspAction}; use namada_tx_env::{Address, Result, TxEnv}; -use crate::{Transfer, TransparentTransfersRef}; +use crate::{Account, Transfer, TransparentTransfersRef}; /// Transparent and shielded token transfers that can be used in a transaction. pub fn multi_transfer( @@ -207,6 +210,49 @@ where for authorizer in masp_authorizers { env.push_action(Action::Masp(MaspAction::MaspAuthorizer(authorizer)))?; } + if !vin_addresses.is_empty() { + if let Some((payer, token)) = tx_data.tx.get_shielding_fee_section(&masp_section_ref) { + env.push_action(Action::Masp(MaspAction::MaspAuthorizer( + Address::from(payer), + )))?; + // transfer the shielding fee + let amount: DenominatedAmount = env + .read(&masp_shielding_fee_amount(token)) + .expect("Failed to read storage") + .ok_or_else(|| { + Error::AllocMessage(format!( + "The token {token} cannot be used to pay shielding \ + fees" + )) + })?; + let sources = BTreeMap::from([( + Account { + owner: Address::from(payer), + token: token.clone(), + }, + amount, + )]); + let targets = BTreeMap::from([( + Account { + owner: PGF, + token: token.clone(), + }, + amount, + )]); + apply_transparent_transfers( + env, + TransparentTransfersRef { + sources: &sources, + targets: &targets, + }, + Cow::Borrowed("shielding-fee-from-wasm"), + )?; + } else { + return Err(Error::SimpleMessage( + "A fee payer was not provided for a shielding transaction", + )); + } + } Ok(()) } diff --git a/crates/tx/src/section.rs b/crates/tx/src/section.rs index d288de3e058..7e37d2ce7ca 100644 --- a/crates/tx/src/section.rs +++ b/crates/tx/src/section.rs @@ -61,6 +61,15 @@ pub enum Section { MaspBuilder(MaspBuilder), /// Wrap a header with a section for the purposes of computing hashes Header(Header), + /// The account paying the fee for a shielding transaction + ShieldingFee { + /// The key for the implicit account + payer: common::PublicKey, + /// The address of the token paying the fee + token: Address, + /// The inner tx this fee is for + cmt: MaspTxId, + }, } /// A Namada transaction header indicating where transaction subcomponents can @@ -149,6 +158,12 @@ impl Section { hasher.update(tx.serialize_to_vec()); hasher } + Self::ShieldingFee { payer, token, cmt } => { + hasher.update(payer.serialize_to_vec()); + hasher.update(token.serialize_to_vec()); + hasher.update(cmt.serialize_to_vec()); + hasher + } Self::Header(header) => header.hash(hasher), } } diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index b7f2c5c74a5..ff571d1d3a5 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -299,6 +299,22 @@ impl Tx { }); } + /// Get the section containing the shielding fee payer + /// if it exists. There should be at most one + pub fn get_shielding_fee_section( + &self, + masp_tx: &MaspTxId + ) -> Option<(&common::PublicKey, &Address)> { + for section in &self.sections { + if let Section::ShieldingFee { payer, token, cmt } = section { + if cmt == masp_tx { + return Some((payer, token)); + } + } + } + None + } + /// Get the MASP builder section with the given hash pub fn get_masp_builder(&self, hash: &MaspTxId) -> Option<&MaspBuilder> { for section in &self.sections { diff --git a/genesis/README.md b/genesis/README.md index 541b9aae89c..95ae4592b24 100644 --- a/genesis/README.md +++ b/genesis/README.md @@ -120,7 +120,7 @@ For non-validator transactions, a helper tool for producing signatures for trans ```shell namada client utils \ - sign-genesis-tx \ + sign-genesis-txs \ --path "unsigned-tx.toml" \ --output "signed-txs.toml" ``` From 5b27b614827426383f7fbe51438be23f2894fed7 Mon Sep 17 00:00:00 2001 From: satan Date: Thu, 10 Jul 2025 14:59:34 +0200 Subject: [PATCH 02/24] Fixing tests --- crates/apps_lib/src/cli.rs | 39 +++++++++++++- crates/apps_lib/src/client/rpc.rs | 16 ++++++ crates/apps_lib/src/config/genesis/chain.rs | 2 + .../apps_lib/src/config/genesis/templates.rs | 27 +++++++++- crates/core/src/parameters.rs | 3 ++ crates/parameters/src/lib.rs | 21 +++++++- crates/parameters/src/storage.rs | 12 ++++- crates/proof_of_stake/src/lib.rs | 1 + crates/sdk/src/args.rs | 40 ++++++++------- crates/sdk/src/rpc.rs | 2 +- crates/sdk/src/tx.rs | 7 +-- crates/shielded_token/src/storage_key.rs | 9 ---- crates/state/src/lib.rs | 1 + crates/tests/src/e2e.rs | 2 +- crates/tests/src/e2e/ibc_tests.rs | 51 ++++++++++++++----- crates/tests/src/e2e/ledger_tests.rs | 11 ++-- crates/tests/src/integration/masp.rs | 15 +++--- crates/tests/src/integration/setup.rs | 2 +- crates/token/src/lib.rs | 2 +- crates/token/src/tx.rs | 22 ++++++-- crates/tx/src/types.rs | 2 +- genesis/hardware/parameters.toml | 2 + genesis/localnet/parameters.toml | 2 + genesis/starter/parameters.toml | 2 + 24 files changed, 222 insertions(+), 71 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 5c5a0468033..2202c612db9 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3675,8 +3675,12 @@ pub mod args { pub const SHIELDED: ArgFlag = flag("shielded"); pub const SHIELDING_FEE_PAYER: Arg = arg("shielding-fee-payer"); + pub const SHIELDING_FEE_PAYER_OPT: ArgOpt = + arg_opt("shielding-fee-payer"); pub const SHIELDING_FEE_TOKEN: Arg = arg("shielding-fee-token"); + pub const SHIELDING_FEE_TOKEN_OPT: ArgOpt = + arg_opt("shielding-fee-token"); pub const SHOW_IBC_TOKENS: ArgFlag = flag("show-ibc-tokens"); pub const SLIPPAGE: ArgOpt = arg_opt("slippage-percentage"); pub const SIGNING_KEYS: ArgMulti = @@ -5249,7 +5253,14 @@ pub mod args { let chain_ctx = ctx.borrow_mut_chain_or_exit(); let recipient = match self.recipient { Either::Left(r) => Either::Left(chain_ctx.get(&r)), - Either::Right(r) => Either::Right(chain_ctx.get(&r)), + Either::Right(r) => { + let shielded_recipient = ShieldedSwapRecipient { + recipient: chain_ctx.get(&r.recipient), + shielding_fee_payer: chain_ctx.get(&r.shielding_fee_payer), + shielding_fee_token: chain_ctx.get(&r.shielding_fee_token), + }; + Either::Right(shielded_recipient) + }, }; let overflow = self.overflow.map(|r| chain_ctx.get(&r)); Ok(TxOsmosisSwap { @@ -5273,6 +5284,8 @@ pub mod args { let maybe_trans_recipient = TARGET_OPT.parse(matches); let maybe_shielded_recipient = PAYMENT_ADDRESS_TARGET_OPT.parse(matches); + let maybe_shielding_fee_payer = SHIELDING_FEE_PAYER_OPT.parse(matches); + let maybe_shielding_fee_token = SHIELDING_FEE_TOKEN_OPT.parse(matches); let maybe_overflow = OVERFLOW_OPT.parse(matches); let slippage_percent = SLIPPAGE.parse(matches); if slippage_percent @@ -5314,7 +5327,11 @@ pub mod args { recipient: if let Some(target) = maybe_trans_recipient { Either::Left(target) } else { - Either::Right(maybe_shielded_recipient.unwrap()) + Either::Right(ShieldedSwapRecipient { + recipient: maybe_shielded_recipient.unwrap(), + shielding_fee_payer: maybe_shielding_fee_payer.unwrap(), + shielding_fee_token: maybe_shielding_fee_token.unwrap(), + }) }, overflow: maybe_overflow, slippage, @@ -5357,12 +5374,30 @@ pub mod args { .arg( PAYMENT_ADDRESS_TARGET_OPT .def() + .requires(SHIELDING_FEE_PAYER_OPT.name) + .requires(SHIELDING_FEE_TOKEN_OPT.name) .conflicts_with(TARGET_OPT.name) .help(wrap!( "Namada payment address that shall receive the \ minimum amount of tokens swapped on Osmosis." )), ) + .arg( + SHIELDING_FEE_PAYER_OPT + .def() + .conflicts_with(TARGET_OPT.name) + .help(wrap!( + "Namada address that will pay the shielding fee." + )), + ) + .arg( + SHIELDING_FEE_TOKEN_OPT + .def() + .conflicts_with(TARGET_OPT.name) + .help(wrap!( + "Namada token address that the shielding fee will be paid in." + )), + ) .arg(OVERFLOW_OPT.def().help(wrap!( "Transparent address that receives the amount of target \ asset exceeding the minimum trade amount. Only \ diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 0406695484e..3390daacb61 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -843,6 +843,14 @@ pub async fn query_protocol_parameters( query_storage_value(context.client(), &key) .await .expect("Parameter should be defined."); + let native_token = rpc::query_native_token(context.client()) + .await + .expect("Native token should be defined"); + let key = param_storage::masp_shielding_fee_amount(&native_token); + let masp_nam_shielding_fee: DenominatedAmount = + query_storage_value(context.client(), &key) + .await + .expect("Parameter should be defined."); let key = param_storage::get_gas_cost_key(); let minimum_gas_price: BTreeMap = query_storage_value(context.client(), &key) @@ -874,6 +882,7 @@ pub async fn query_protocol_parameters( epochs_per_year, masp_epoch_multiplier, masp_fee_payment_gas_limit, + masp_nam_shielding_fee: masp_nam_shielding_fee.to_string_precise(), gas_scale, minimum_gas_price, is_native_token_transferable, @@ -890,6 +899,7 @@ pub async fn query_protocol_parameters( epochs_per_year, masp_epoch_multiplier, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, minimum_gas_price, is_native_token_transferable, @@ -946,6 +956,12 @@ pub async fn query_protocol_parameters( "", masp_fee_payment_gas_limit ); + display_line!( + context.io(), + "{:4}Masp shielding fee payment (NAM): {:?} gas units", + "", + masp_nam_shielding_fee + ); display_line!(context.io(), "{:4}Minimum gas costs:", ""); for (token, gas_cost) in minimum_gas_price { let denom = rpc::query_denom(context.client(), &token) diff --git a/crates/apps_lib/src/config/genesis/chain.rs b/crates/apps_lib/src/config/genesis/chain.rs index f3604c3bbe2..95e0f682117 100644 --- a/crates/apps_lib/src/config/genesis/chain.rs +++ b/crates/apps_lib/src/config/genesis/chain.rs @@ -331,6 +331,7 @@ impl Finalized { epochs_per_year, masp_epoch_multiplier, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, max_block_gas, minimum_gas_price, @@ -373,6 +374,7 @@ impl Finalized { masp_epoch_multiplier, max_proposal_bytes, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, max_block_gas, minimum_gas_price: minimum_gas_price diff --git a/crates/apps_lib/src/config/genesis/templates.rs b/crates/apps_lib/src/config/genesis/templates.rs index 4df147be939..b2cbf536b96 100644 --- a/crates/apps_lib/src/config/genesis/templates.rs +++ b/crates/apps_lib/src/config/genesis/templates.rs @@ -3,7 +3,8 @@ use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; use std::path::Path; - +use std::str::FromStr; +use eyre::eyre; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] use namada_migrations::*; @@ -290,6 +291,8 @@ pub struct ChainParams { pub max_block_gas: u64, /// Gas limit of a masp transaction paying fees pub masp_fee_payment_gas_limit: u64, + /// The amount of NAM the MASP shielding fee costs + pub masp_nam_shielding_fee: String, /// Gas scale pub gas_scale: u64, /// Map of the cost per gas unit for every token allowed for fee payment @@ -314,6 +317,7 @@ impl ChainParams { masp_epoch_multiplier, max_block_gas, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, minimum_gas_price, } = self; @@ -345,6 +349,26 @@ impl ChainParams { })?; min_gas_prices.insert(token, amount); } + let masp_nam_shielding_fee = match DenominatedAmount::from_str(&masp_nam_shielding_fee) { + Ok(amount) => { + amount.increase_precision(NATIVE_MAX_DECIMAL_PLACES.into()).map_err(|e| { + eprintln!( + "A MASP shielding fee (in NAM) in the parameters.toml file was \ + incorrectly denominated:\n{}", + e + ); + e + })? + } + Err(e) => { + eprintln!( + "A MASP shielding fee (in NAM) in the parameters.toml file was \ + incorrectly formatted:\n{}", + e + ); + return Err(eyre!("{}", e.to_string())); + } + }.to_string_precise(); Ok(ChainParams { max_tx_bytes, @@ -359,6 +383,7 @@ impl ChainParams { masp_epoch_multiplier, max_block_gas, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, minimum_gas_price: min_gas_prices, }) diff --git a/crates/core/src/parameters.rs b/crates/core/src/parameters.rs index 7588ea43383..95dcab76e42 100644 --- a/crates/core/src/parameters.rs +++ b/crates/core/src/parameters.rs @@ -52,6 +52,8 @@ pub struct Parameters { pub masp_epoch_multiplier: u64, /// The gas limit for a masp transaction paying fees pub masp_fee_payment_gas_limit: u64, + /// The amount of NAM the MASP shielding fee costs + pub masp_nam_shielding_fee: String, /// Gas scale pub gas_scale: u64, /// Map of the cost per gas unit for every token allowed for fee payment @@ -98,6 +100,7 @@ impl Default for Parameters { epochs_per_year: 365, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 0, + masp_nam_shielding_fee: 0.to_string(), gas_scale: 100_000_000, minimum_gas_price: Default::default(), is_native_token_transferable: true, diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index fba32e66179..d59b5be665d 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -22,7 +22,7 @@ pub mod vp; mod wasm_allowlist; use std::collections::BTreeMap; use std::marker::PhantomData; - +use std::str::FromStr; use namada_core::address::{Address, InternalAddress}; use namada_core::arith::checked; use namada_core::chain::BlockHeight; @@ -33,6 +33,7 @@ use namada_state::{Error, Key, ResultExt, StorageRead, StorageWrite}; pub use namada_systems::parameters::*; pub use storage::{get_gas_scale, get_max_block_gas}; use thiserror::Error; +use namada_core::token::{DenominatedAmount, NATIVE_MAX_DECIMAL_PLACES}; pub use wasm_allowlist::{is_tx_allowed, is_vp_allowed}; /// Parameters storage `Keys/Read/Write` implementation @@ -115,6 +116,7 @@ pub enum WriteError { SerializeError(String), } + /// Initialize parameters in storage in the genesis block. pub fn init_storage(parameters: &Parameters, storage: &mut S) -> Result<()> where @@ -131,6 +133,7 @@ where epochs_per_year, masp_epoch_multiplier, minimum_gas_price, + masp_nam_shielding_fee, masp_fee_payment_gas_limit, gas_scale, is_native_token_transferable, @@ -158,6 +161,13 @@ where storage .write(&masp_fee_payment_gas_limit_key, masp_fee_payment_gas_limit)?; + let native_token = &storage.get_native_token()?; + let masp_nam_shielding_fee_key = storage::masp_shielding_fee_amount(native_token); + let masp_nam_shielding_fee = DenominatedAmount::from_str(masp_nam_shielding_fee) + .map_err(|e| Error::AllocMessage(e.to_string()))? + .redenominate(NATIVE_MAX_DECIMAL_PLACES); + storage.write(&masp_nam_shielding_fee_key, masp_nam_shielding_fee)?; + // write the gas scale let gas_scale_key = storage::get_gas_scale_key(); storage.write(&gas_scale_key, gas_scale)?; @@ -409,6 +419,13 @@ where .ok_or(ReadError::ParametersMissing) .into_storage_result()?; + let masp_nam_shielding_fee_key = storage::masp_shielding_fee_amount(&storage.get_native_token()?); + let value = storage.read::(&masp_nam_shielding_fee_key)?; + let masp_nam_shielding_fee = value + .ok_or(ReadError::ParametersMissing) + .into_storage_result()? + .to_string_precise(); + // read gas scale let gas_scale_key = storage::get_gas_scale_key(); let value = storage.read(&gas_scale_key)?; @@ -459,6 +476,7 @@ where masp_epoch_multiplier, minimum_gas_price, masp_fee_payment_gas_limit, + masp_nam_shielding_fee, gas_scale, is_native_token_transferable, }) @@ -500,6 +518,7 @@ where epochs_per_year: 365, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 0, + masp_nam_shielding_fee: 0.to_string(), gas_scale: 10_000_000, minimum_gas_price: Default::default(), is_native_token_transferable: true, diff --git a/crates/parameters/src/storage.rs b/crates/parameters/src/storage.rs index 0e9eaff8a61..55c0a9da0a8 100644 --- a/crates/parameters/src/storage.rs +++ b/crates/parameters/src/storage.rs @@ -5,7 +5,6 @@ use namada_core::storage::DbKeySeg; pub use namada_core::storage::Key; use namada_macros::StorageKeys; use namada_state::{Error, Result, StorageRead}; - use super::ADDRESS; #[derive(StorageKeys)] @@ -192,3 +191,14 @@ pub fn is_native_token_transferable( ), ) } + +/// The key for getting the shielding fee amount of the provided +/// token. +pub fn masp_shielding_fee_amount(token: &Address) -> Key { + pub const MASP_SHIELDING_FEE_PREFIX: &str = "shielding_fee"; + namada_core::storage::Key::from(DbKeySeg::AddressSeg(ADDRESS)) + .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(token) + .expect("Cannot obtain a storage key") +} diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 0efb9dac8d4..841246c165a 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -3316,6 +3316,7 @@ pub mod test_utils { epochs_per_year: 10000000, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 10000, + masp_nam_shielding_fee: 0.to_string(), gas_scale: 100_000_000, minimum_gas_price: BTreeMap::new(), is_native_token_transferable: true, diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 4ce9b467ccc..d83a4165d50 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -515,6 +515,18 @@ pub enum Slippage { }, } +/// Data needed for an Osmosis swap to shield the output of a swap +/// on Namada +#[derive(Debug, Clone)] +pub struct ShieldedSwapRecipient { + /// The recipient + pub recipient: C::PaymentAddress, + /// The account paying the shielding fee + pub shielding_fee_payer: C::PublicKey, + /// The token the shielding fee is being paid in + pub shielding_fee_token: C::Address, +} + /// An token swap on Osmosis #[derive(Debug, Clone)] pub struct TxOsmosisSwap { @@ -523,7 +535,7 @@ pub struct TxOsmosisSwap { /// The token we wish to receive (on Namada) pub output_denom: String, /// Address of the recipient on Namada - pub recipient: Either, + pub recipient: Either>, /// Address to receive funds exceeding the minimum amount, /// in case of IBC shieldings /// @@ -686,7 +698,7 @@ impl TxOsmosisSwap { (transparent_recipient.to_string(), slippage, None) } Either::Right(fut) => { - let (payment_addr, overflow_receiver) = fut.await; + let (shielded_recipient, overflow_receiver) = fut.await; let amount_to_shield = match slippage { Slippage::MinOutputAmount(amount_to_shield) => { @@ -697,13 +709,7 @@ impl TxOsmosisSwap { yet" ), }; - let Some(ibc_shielding_data) = - transfer.ibc_shielding_data.as_ref() - else { - return Err(Error::Other( - "Expected IBC shielding data".to_string(), - )); - }; + let shielding_tx = tx::gen_ibc_shielding_transfer( ctx, GenIbcShieldingTransfer { @@ -713,7 +719,7 @@ impl TxOsmosisSwap { output_folder: None, target: namada_core::masp::TransferTarget::PaymentAddress( - payment_addr, + shielded_recipient.recipient, ), asset: IbcShieldingTransferAsset::Address( namada_output_addr, @@ -725,10 +731,10 @@ impl TxOsmosisSwap { ), ), expiration: transfer.tx.expiration.clone(), - shielding_fee_payer: ibc_shielding_data + shielding_fee_payer: shielded_recipient .shielding_fee_payer .clone(), - shielding_fee_token: ibc_shielding_data + shielding_fee_token: shielded_recipient .shielding_fee_token .clone(), }, @@ -744,14 +750,10 @@ impl TxOsmosisSwap { serde_json::to_value(&NamadaMemo { namada: NamadaMemoData::OsmosisSwap { shielding_data: StringEncoded::new( - IbcShieldingData { + IbcShieldingData{ masp_tx: shielding_tx, - shielding_fee_payer: ibc_shielding_data - .shielding_fee_payer - .clone(), - shielding_fee_token: ibc_shielding_data - .shielding_fee_token - .clone(), + shielding_fee_payer: shielded_recipient.shielding_fee_payer, + shielding_fee_token: shielded_recipient.shielding_fee_token }, ), shielded_amount: amount_to_shield, diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index 216e2ba1521..9fbba35ae50 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -271,7 +271,7 @@ pub async fn get_shielding_fee_amount( ) -> Result { query_storage_value( client, - &namada_token::storage_key::masp_shielding_fee_amount(token), + &namada_parameters::storage::masp_shielding_fee_amount(token), ) .await } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 151cf78b0fc..d48a9e6c22b 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -41,7 +41,9 @@ use namada_core::ibc::core::client::types::Height as IbcHeight; use namada_core::ibc::core::host::types::identifiers::{ChannelId, PortId}; use namada_core::ibc::primitives::{IntoTimestamp, Timestamp as IbcTimestamp}; use namada_core::key::{self, *}; -use namada_core::masp::{AssetData, MaspEpoch, MaspTxId, TransferSource, TransferTarget}; +use namada_core::masp::{ + AssetData, MaspEpoch, MaspTxId, TransferSource, TransferTarget, +}; use namada_core::storage; use namada_core::time::DateTimeUtc; use namada_events::extend::EventAttributeEntry; @@ -3706,11 +3708,10 @@ pub async fn build_shielding_transfer( data.shielded_section_hash = Some(shielded_section_hash); signing_data.shielded_hash = Some(shielded_section_hash); - tx.add_section(Section::ShieldingFee { payer: args.shielding_fee_payer.clone(), token: args.shielding_fee_token.clone(), - cmt: shielded_section_hash + cmt: shielded_section_hash, }); tracing::debug!("Transfer data {data:?}"); diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index 7f3feef9064..884edf376b6 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -391,12 +391,3 @@ pub fn masp_total_rewards() -> storage::Key { .expect("Cannot obtain a storage key") } -/// The key for getting the shielding fee amount of the provided -/// token. -pub fn masp_shielding_fee_amount(token: &Address) -> storage::Key { - storage::Key::from(address::MASP.to_db_key()) - .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) - .expect("Cannot obtain a storage key") - .push(token) - .expect("Cannot obtain a storage key") -} diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index d35ff52ba36..f1b377c1fe9 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -759,6 +759,7 @@ mod tests { epochs_per_year: 100, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 20_000, + masp_nam_shielding_fee: 0.to_string(), gas_scale: 10_000_000, minimum_gas_price: BTreeMap::default(), is_native_token_transferable: true, diff --git a/crates/tests/src/e2e.rs b/crates/tests/src/e2e.rs index b1cfb67a9d5..169e55b3ac9 100644 --- a/crates/tests/src/e2e.rs +++ b/crates/tests/src/e2e.rs @@ -17,4 +17,4 @@ pub mod helpers; pub mod ibc_tests; pub mod ledger_tests; pub mod setup; -pub mod wallet_tests; +//pub mod wallet_tests; diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 758d6c12227..c7c0079d1c9 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -94,7 +94,7 @@ const NFT_ID: &str = "test_nft"; /// 6. Malformed shielded actions /// - Missing memo /// - Wrong memo -#[test] +//#[test] fn ibc_transfers() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -540,7 +540,7 @@ fn ibc_transfers() -> Result<()> { Ok(()) } -#[test] +//#[test] fn ibc_nft_transfers() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -689,7 +689,7 @@ fn ibc_nft_transfers() -> Result<()> { Ok(()) } -#[test] +//#[test] fn pgf_over_ibc() -> Result<()> { const PIPELINE_LEN: u64 = 5; let update_genesis = @@ -789,7 +789,7 @@ fn pgf_over_ibc() -> Result<()> { // 1. Submit governance proposal to allow fee payment with the IBC token // 2. Transfer some IBC tokens from gaia // 3. Transparent transfer in Namada with ibc token gas payment -#[test] +//#[test] fn fee_payment_with_ibc_token() -> Result<()> { const PIPELINE_LEN: u64 = 2; let update_genesis = @@ -959,6 +959,19 @@ fn ibc_token_inflation() -> Result<()> { epoch = epoch_sleep(&test, &rpc, 120)?; } + // check that the proposal passed + let args = vec![ + "query-proposal-result", + "--proposal-id", + "0", + "--node", + &rpc, + ]; + + let mut client = run!(test, Bin::Client, args, Some(120))?; + client.exp_string("Passed with")?; + client.assert_success(); + // Check the target balance is zero before the inflation check_shielded_balance(&test, AA_VIEWING_KEY, NAM, 0)?; // Shielding transfer 1 samoleans from Gaia to Namada @@ -998,7 +1011,7 @@ fn ibc_token_inflation() -> Result<()> { Ok(()) } -#[test] +//#[test] fn ibc_upgrade_client() -> Result<()> { const UPGRADE_HEIGHT_OFFSET: u64 = 20; @@ -1065,7 +1078,7 @@ fn ibc_upgrade_client() -> Result<()> { /// 2. Test the mint limit /// - The mint limit is 1 /// - Receiving 2 samoleans from Gaia will fail -#[test] +//#[test] fn ibc_rate_limit() -> Result<()> { const DEFAULT_RATE_LIMIT: Amount = Amount::from_u64(1_000_000); const DEFAULT_MINT_LIMIT: Amount = Amount::from_u64(1); @@ -1322,7 +1335,7 @@ fn shielded_recv_memo_value( /// 2. Test sending the above transfer back to first cosmos chain via PFM /// 3. Send wrapped NAM from first cosmos chain to the second via PFM /// 4. Reverse the transaction in the step above -#[test] +//#[test] fn ibc_pfm_happy_flows() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -1619,7 +1632,7 @@ fn ibc_pfm_happy_flows() -> Result<()> { /// failing to send it back to the second due to an error /// 6. Same as above except that the failure occurs due to a time-out on the /// second cosmos chain. -#[test] +//#[test] fn ibc_pfm_unhappy_flows() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -2045,7 +2058,7 @@ fn ibc_pfm_unhappy_flows() -> Result<()> { /// Test that we are able to use the shielded-receive /// middleware to shield funds specified in the memo /// message. -#[test] +//#[test] fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -2089,7 +2102,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { NAM, 10, ALBERT_KEY, - &[], + &["--shielding-fee-payer", BERTHA_KEY, "--shielding-fee-token", NAM], )?; check_shielded_balance(&test, AA_VIEWING_KEY, NAM, 10)?; @@ -2145,7 +2158,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { /// Test that if the received amount underflows the minimum /// amount, we error out and refund assets. -#[test] +//#[test] fn ibc_shielded_recv_middleware_unhappy_flow() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -3212,6 +3225,10 @@ fn gen_ibc_shielding_data( port_id.as_ref(), "--channel-id", channel_id.as_ref(), + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", &rpc, ]; @@ -3537,7 +3554,7 @@ fn nft_transfer_from_cosmos( } /// Basic Osmosis test that checks if the chain has been set up correctly. -#[test] +//#[test] fn osmosis_basic() -> Result<()> { let (osmosis, test_osmosis) = setup_and_boot_cosmos(CosmosChainType::Osmosis)?; @@ -3550,7 +3567,7 @@ fn osmosis_basic() -> Result<()> { Ok(()) } -#[test] +//#[test] fn osmosis_xcs() -> Result<()> { // ========================================================== // This test requires quite a long setup. Jump to the next @@ -4020,6 +4037,10 @@ fn osmosis_xcs() -> Result<()> { "0.000056", "--token", NAM, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--node", &rpc_namada, ], @@ -4144,6 +4165,10 @@ fn osmosis_xcs() -> Result<()> { "10", "--target-pa", AA_PAYMENT_ADDRESS, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--overflow-addr", ALBERT, "--pool-hop", diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index d3ab2306b48..4036c6a59b0 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -91,7 +91,7 @@ pub fn start_namada_ledger_node_wait_wasm( wait_for_wasm_pre_compile(&mut node)?; Ok(node) } - +/* /// Test that when we "run-ledger" with all the possible command /// combinations from fresh state, the node starts-up successfully for both a /// validator and non-validator user. @@ -1118,7 +1118,7 @@ fn ledger_many_txs_in_a_block() -> Result<()> { Ok(()) } - +*/ pub fn write_json_file(proposal_path: &std::path::Path, proposal_content: T) where T: Serialize, @@ -1132,7 +1132,7 @@ where serde_json::to_writer(intent_writer, &proposal_content).unwrap(); } - +/* /// In this test we intentionally make a validator node double sign blocks /// to test that slashing evidence is received and processed by the ledger /// correctly: @@ -1536,7 +1536,7 @@ fn test_epoch_sleep() -> Result<()> { Ok(()) } - +*/ /// Prepare proposal data in the test's temp dir from the given source address. /// This can be submitted with "init-proposal" command. pub fn prepare_proposal_data( @@ -1571,7 +1571,7 @@ pub fn prepare_proposal_data( write_json_file(valid_proposal_json_path.as_path(), valid_proposal_json); valid_proposal_json_path } - +/* #[test] fn deactivate_and_reactivate_validator() -> Result<()> { let pipeline_len = 2; @@ -2844,3 +2844,4 @@ fn test_genesis_manipulation() -> Result<()> { Ok(()) } +*/ \ No newline at end of file diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 87f331c8712..d1bea18620e 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -47,8 +47,8 @@ use crate::e2e::setup::constants::{ }; use crate::integration::helpers::make_temp_account; use crate::integration::ledger_tests::find_files_with_ext; +use crate::parameters::storage::masp_shielding_fee_amount; use crate::strings::TX_APPLIED_SUCCESS; -use crate::token::storage_key::masp_shielding_fee_amount; use crate::tx::Authorization; /// Enable masp rewards before some token is shielded, @@ -1488,10 +1488,10 @@ fn enable_rewards_after_shielding() -> Result<()> { /// 1. Test the happy flow. /// 2. Test that if the shielding fee section is missing from the tx, it is /// rejected -/// 3. Test that if the shielding fee holds the wrong Masp Tx id, it is rejected. +/// 3. Test that if the shielding fee holds the wrong Masp Tx id, it is +/// rejected. /// 4. Test that if the account listed in the shielding fee does not sign the /// tx, it is rejected -/// #[test] fn test_shielding_fee_protocol_checks() -> Result<()> { // This address doesn't matter for tests. But an argument is required. @@ -1734,7 +1734,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { let mut tx: Tx = serde_json::from_reader( std::fs::File::open(tx_path.clone()).expect("Test Failed"), ) - .expect("Test failed"); + .expect("Test failed"); let signing_data = SigningTxData { owner: Some(node.lookup_address(ALBERT)?), public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), @@ -1748,8 +1748,10 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { signatures: vec![], }; for sec in tx.sections.iter_mut() { - if let Section::ShieldingFee {cmt, ..} = sec { - *cmt = MaspTxId::from(masp_primitives::transaction::TxId::from_bytes([0u8; 32])); + if let Section::ShieldingFee { cmt, .. } = sec { + *cmt = MaspTxId::from( + masp_primitives::transaction::TxId::from_bytes([0u8; 32]), + ); } } let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; @@ -7301,7 +7303,6 @@ fn identical_output_descriptions() -> Result<()> { ))); batched_tx.sign_wrapper(bradley_key); - let wrapper_hash = batched_tx.wrapper_hash(); let inner_cmts = batched_tx.commitments(); diff --git a/crates/tests/src/integration/setup.rs b/crates/tests/src/integration/setup.rs index e20a68fc82d..fc88251daeb 100644 --- a/crates/tests/src/integration/setup.rs +++ b/crates/tests/src/integration/setup.rs @@ -30,8 +30,8 @@ use namada_sdk::token; use namada_sdk::wallet::alias::Alias; use crate::e2e::setup::copy_wasm_to_chain_dir; +use crate::parameters::storage::masp_shielding_fee_amount; use crate::proof_of_stake::StorageWrite; -use crate::token::storage_key::masp_shielding_fee_amount; /// Env. var for keeping temporary files created by the integration tests const ENV_VAR_KEEP_TEMP: &str = "NAMADA_INT_KEEP_TEMP"; diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index a2c99b5d45f..4d14d7407fc 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -58,7 +58,7 @@ pub mod storage_key { masp_base_native_precision_key, masp_commitment_anchor_key, masp_commitment_tree_key, masp_conversion_key, masp_convert_anchor_key, masp_nullifier_key, masp_scheduled_base_native_precision_key, - masp_scheduled_reward_precision_key, masp_shielding_fee_amount, + masp_scheduled_reward_precision_key, masp_token_map_key, masp_total_rewards, }; pub use namada_trans_token::storage_key::*; diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index f8a4d88e1c0..6ee30204725 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; -use namada_core::address::PGF; +use namada_core::address::{InternalAddress, PGF}; use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; use namada_core::masp::encode_asset_type; @@ -11,10 +11,9 @@ use namada_core::masp_primitives::transaction::Transaction; use namada_core::token::{DenominatedAmount, MaspDigitPos}; use namada_core::uint::I320; use namada_core::{masp, token}; +use namada_core::storage::{DbKeySeg, Key}; use namada_events::EmitEvents; -use namada_shielded_token::storage_key::{ - masp_shielding_fee_amount, masp_undated_balance_key, -}; +use namada_shielded_token::storage_key::masp_undated_balance_key; use namada_shielded_token::{MaspTxId, read_undated_balance, utils}; use namada_storage::{Error, OptionExt, ResultExt}; use namada_trans_token::read_denom; @@ -211,7 +210,9 @@ where env.push_action(Action::Masp(MaspAction::MaspAuthorizer(authorizer)))?; } if !vin_addresses.is_empty() { - if let Some((payer, token)) = tx_data.tx.get_shielding_fee_section(&masp_section_ref) { + if let Some((payer, token)) = + tx_data.tx.get_shielding_fee_section(&masp_section_ref) + { env.push_action(Action::Masp(MaspAction::MaspAuthorizer( Address::from(payer), )))?; @@ -257,6 +258,17 @@ where Ok(()) } +/// The key for getting the shielding fee amount of the provided +/// token. +fn masp_shielding_fee_amount(token: &Address) -> Key { + pub const MASP_SHIELDING_FEE_PREFIX: &str = "shielding_fee"; + namada_core::storage::Key::from(DbKeySeg::AddressSeg(Address::Internal(InternalAddress::Parameters))) + .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(token) + .expect("Cannot obtain a storage key") +} + #[cfg(test)] #[allow(clippy::arithmetic_side_effects, clippy::disallowed_types)] mod test { diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index ff571d1d3a5..884dd731725 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -303,7 +303,7 @@ impl Tx { /// if it exists. There should be at most one pub fn get_shielding_fee_section( &self, - masp_tx: &MaspTxId + masp_tx: &MaspTxId, ) -> Option<(&common::PublicKey, &Address)> { for section in &self.sections { if let Section::ShieldingFee { payer, token, cmt } = section { diff --git a/genesis/hardware/parameters.toml b/genesis/hardware/parameters.toml index aa044b73ddf..f04144717c9 100644 --- a/genesis/hardware/parameters.toml +++ b/genesis/hardware/parameters.toml @@ -22,6 +22,8 @@ masp_epoch_multiplier = 2 max_block_gas = 3_000_000 # Masp fee payment gas limit masp_fee_payment_gas_limit = 100_000 +# Fee for shieling MASP txs in NAM +masp_nam_shielding_fee = "0.000000" # Gas scale gas_scale = 50_000 diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index a299b5fc65a..1fcb8da1772 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -22,6 +22,8 @@ masp_epoch_multiplier = 2 max_block_gas = 3_000_000 # Masp fee payment gas limit masp_fee_payment_gas_limit = 100_000 +# Fee for shieling MASP txs in NAM +masp_nam_shielding_fee = "0.000000" # Gas scale gas_scale = 50_000 diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 1f373c66754..a27335877b8 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -22,6 +22,8 @@ masp_epoch_multiplier = 2 max_block_gas = 3_000_000 # Masp fee payment gas limit masp_fee_payment_gas_limit = 100_000 +# Fee for shieling MASP txs in NAM +masp_nam_shielding_fee = "0.000000" # Gas scale gas_scale = 50_000 From 5187a624b8c22b339e4649da42389f52cb0a2400 Mon Sep 17 00:00:00 2001 From: satan Date: Fri, 11 Jul 2025 11:09:31 +0200 Subject: [PATCH 03/24] Formatting --- crates/apps_lib/src/cli.rs | 17 +++++---- .../apps_lib/src/config/genesis/templates.rs | 36 ++++++++++--------- crates/parameters/src/lib.rs | 20 ++++++----- crates/parameters/src/storage.rs | 1 + crates/sdk/src/args.rs | 8 +++-- crates/shielded_token/src/storage_key.rs | 1 - crates/tests/src/e2e.rs | 2 +- crates/tests/src/e2e/ibc_tests.rs | 33 +++++++++-------- crates/tests/src/e2e/ledger_tests.rs | 11 +++--- crates/token/src/lib.rs | 4 +-- crates/token/src/tx.rs | 14 ++++---- 11 files changed, 84 insertions(+), 63 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 2202c612db9..9388f7bfe76 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -5256,11 +5256,13 @@ pub mod args { Either::Right(r) => { let shielded_recipient = ShieldedSwapRecipient { recipient: chain_ctx.get(&r.recipient), - shielding_fee_payer: chain_ctx.get(&r.shielding_fee_payer), - shielding_fee_token: chain_ctx.get(&r.shielding_fee_token), + shielding_fee_payer: chain_ctx + .get(&r.shielding_fee_payer), + shielding_fee_token: chain_ctx + .get(&r.shielding_fee_token), }; Either::Right(shielded_recipient) - }, + } }; let overflow = self.overflow.map(|r| chain_ctx.get(&r)); Ok(TxOsmosisSwap { @@ -5284,8 +5286,10 @@ pub mod args { let maybe_trans_recipient = TARGET_OPT.parse(matches); let maybe_shielded_recipient = PAYMENT_ADDRESS_TARGET_OPT.parse(matches); - let maybe_shielding_fee_payer = SHIELDING_FEE_PAYER_OPT.parse(matches); - let maybe_shielding_fee_token = SHIELDING_FEE_TOKEN_OPT.parse(matches); + let maybe_shielding_fee_payer = + SHIELDING_FEE_PAYER_OPT.parse(matches); + let maybe_shielding_fee_token = + SHIELDING_FEE_TOKEN_OPT.parse(matches); let maybe_overflow = OVERFLOW_OPT.parse(matches); let slippage_percent = SLIPPAGE.parse(matches); if slippage_percent @@ -5395,7 +5399,8 @@ pub mod args { .def() .conflicts_with(TARGET_OPT.name) .help(wrap!( - "Namada token address that the shielding fee will be paid in." + "Namada token address that the shielding fee will \ + be paid in." )), ) .arg(OVERFLOW_OPT.def().help(wrap!( diff --git a/crates/apps_lib/src/config/genesis/templates.rs b/crates/apps_lib/src/config/genesis/templates.rs index b2cbf536b96..cc4d50e3a75 100644 --- a/crates/apps_lib/src/config/genesis/templates.rs +++ b/crates/apps_lib/src/config/genesis/templates.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; use std::path::Path; use std::str::FromStr; + use eyre::eyre; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] @@ -349,26 +350,29 @@ impl ChainParams { })?; min_gas_prices.insert(token, amount); } - let masp_nam_shielding_fee = match DenominatedAmount::from_str(&masp_nam_shielding_fee) { - Ok(amount) => { - amount.increase_precision(NATIVE_MAX_DECIMAL_PLACES.into()).map_err(|e| { + let masp_nam_shielding_fee = + match DenominatedAmount::from_str(&masp_nam_shielding_fee) { + Ok(amount) => amount + .increase_precision(NATIVE_MAX_DECIMAL_PLACES.into()) + .map_err(|e| { + eprintln!( + "A MASP shielding fee (in NAM) in the \ + parameters.toml file was incorrectly \ + denominated:\n{}", + e + ); + e + })?, + Err(e) => { eprintln!( - "A MASP shielding fee (in NAM) in the parameters.toml file was \ - incorrectly denominated:\n{}", + "A MASP shielding fee (in NAM) in the parameters.toml \ + file was incorrectly formatted:\n{}", e ); - e - })? - } - Err(e) => { - eprintln!( - "A MASP shielding fee (in NAM) in the parameters.toml file was \ - incorrectly formatted:\n{}", - e - ); - return Err(eyre!("{}", e.to_string())); + return Err(eyre!("{}", e.to_string())); + } } - }.to_string_precise(); + .to_string_precise(); Ok(ChainParams { max_tx_bytes, diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index d59b5be665d..a2aa687d173 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -23,17 +23,18 @@ mod wasm_allowlist; use std::collections::BTreeMap; use std::marker::PhantomData; use std::str::FromStr; + use namada_core::address::{Address, InternalAddress}; use namada_core::arith::checked; use namada_core::chain::BlockHeight; pub use namada_core::parameters::ProposalBytes; use namada_core::time::DurationSecs; +use namada_core::token::{DenominatedAmount, NATIVE_MAX_DECIMAL_PLACES}; use namada_core::{hints, token}; use namada_state::{Error, Key, ResultExt, StorageRead, StorageWrite}; pub use namada_systems::parameters::*; pub use storage::{get_gas_scale, get_max_block_gas}; use thiserror::Error; -use namada_core::token::{DenominatedAmount, NATIVE_MAX_DECIMAL_PLACES}; pub use wasm_allowlist::{is_tx_allowed, is_vp_allowed}; /// Parameters storage `Keys/Read/Write` implementation @@ -116,7 +117,6 @@ pub enum WriteError { SerializeError(String), } - /// Initialize parameters in storage in the genesis block. pub fn init_storage(parameters: &Parameters, storage: &mut S) -> Result<()> where @@ -162,10 +162,12 @@ where .write(&masp_fee_payment_gas_limit_key, masp_fee_payment_gas_limit)?; let native_token = &storage.get_native_token()?; - let masp_nam_shielding_fee_key = storage::masp_shielding_fee_amount(native_token); - let masp_nam_shielding_fee = DenominatedAmount::from_str(masp_nam_shielding_fee) - .map_err(|e| Error::AllocMessage(e.to_string()))? - .redenominate(NATIVE_MAX_DECIMAL_PLACES); + let masp_nam_shielding_fee_key = + storage::masp_shielding_fee_amount(native_token); + let masp_nam_shielding_fee = + DenominatedAmount::from_str(masp_nam_shielding_fee) + .map_err(|e| Error::AllocMessage(e.to_string()))? + .redenominate(NATIVE_MAX_DECIMAL_PLACES); storage.write(&masp_nam_shielding_fee_key, masp_nam_shielding_fee)?; // write the gas scale @@ -419,8 +421,10 @@ where .ok_or(ReadError::ParametersMissing) .into_storage_result()?; - let masp_nam_shielding_fee_key = storage::masp_shielding_fee_amount(&storage.get_native_token()?); - let value = storage.read::(&masp_nam_shielding_fee_key)?; + let masp_nam_shielding_fee_key = + storage::masp_shielding_fee_amount(&storage.get_native_token()?); + let value = + storage.read::(&masp_nam_shielding_fee_key)?; let masp_nam_shielding_fee = value .ok_or(ReadError::ParametersMissing) .into_storage_result()? diff --git a/crates/parameters/src/storage.rs b/crates/parameters/src/storage.rs index 55c0a9da0a8..84de4e624dd 100644 --- a/crates/parameters/src/storage.rs +++ b/crates/parameters/src/storage.rs @@ -5,6 +5,7 @@ use namada_core::storage::DbKeySeg; pub use namada_core::storage::Key; use namada_macros::StorageKeys; use namada_state::{Error, Result, StorageRead}; + use super::ADDRESS; #[derive(StorageKeys)] diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index d83a4165d50..d116440f89b 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -750,10 +750,12 @@ impl TxOsmosisSwap { serde_json::to_value(&NamadaMemo { namada: NamadaMemoData::OsmosisSwap { shielding_data: StringEncoded::new( - IbcShieldingData{ + IbcShieldingData { masp_tx: shielding_tx, - shielding_fee_payer: shielded_recipient.shielding_fee_payer, - shielding_fee_token: shielded_recipient.shielding_fee_token + shielding_fee_payer: shielded_recipient + .shielding_fee_payer, + shielding_fee_token: shielded_recipient + .shielding_fee_token, }, ), shielded_amount: amount_to_shield, diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index 884edf376b6..48168f749b9 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -390,4 +390,3 @@ pub fn masp_total_rewards() -> storage::Key { .push(&MASP_TOTAL_REWARDS.to_owned()) .expect("Cannot obtain a storage key") } - diff --git a/crates/tests/src/e2e.rs b/crates/tests/src/e2e.rs index 169e55b3ac9..b1cfb67a9d5 100644 --- a/crates/tests/src/e2e.rs +++ b/crates/tests/src/e2e.rs @@ -17,4 +17,4 @@ pub mod helpers; pub mod ibc_tests; pub mod ledger_tests; pub mod setup; -//pub mod wallet_tests; +pub mod wallet_tests; diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index c7c0079d1c9..0cfafa41ce0 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -94,7 +94,7 @@ const NFT_ID: &str = "test_nft"; /// 6. Malformed shielded actions /// - Missing memo /// - Wrong memo -//#[test] +#[test] fn ibc_transfers() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -540,7 +540,7 @@ fn ibc_transfers() -> Result<()> { Ok(()) } -//#[test] +#[test] fn ibc_nft_transfers() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -689,7 +689,7 @@ fn ibc_nft_transfers() -> Result<()> { Ok(()) } -//#[test] +#[test] fn pgf_over_ibc() -> Result<()> { const PIPELINE_LEN: u64 = 5; let update_genesis = @@ -789,7 +789,7 @@ fn pgf_over_ibc() -> Result<()> { // 1. Submit governance proposal to allow fee payment with the IBC token // 2. Transfer some IBC tokens from gaia // 3. Transparent transfer in Namada with ibc token gas payment -//#[test] +#[test] fn fee_payment_with_ibc_token() -> Result<()> { const PIPELINE_LEN: u64 = 2; let update_genesis = @@ -1000,7 +1000,7 @@ fn ibc_token_inflation() -> Result<()> { // wait the next masp epoch to dispense the reward let mut epoch = get_epoch(&test, &rpc).unwrap(); - let new_epoch = epoch + MASP_EPOCH_MULTIPLIER; + let new_epoch = Epoch(epoch.0 * MASP_EPOCH_MULTIPLIER); while epoch < new_epoch { epoch = epoch_sleep(&test, &rpc, 120)?; } @@ -1011,7 +1011,7 @@ fn ibc_token_inflation() -> Result<()> { Ok(()) } -//#[test] +#[test] fn ibc_upgrade_client() -> Result<()> { const UPGRADE_HEIGHT_OFFSET: u64 = 20; @@ -1078,7 +1078,7 @@ fn ibc_upgrade_client() -> Result<()> { /// 2. Test the mint limit /// - The mint limit is 1 /// - Receiving 2 samoleans from Gaia will fail -//#[test] +#[test] fn ibc_rate_limit() -> Result<()> { const DEFAULT_RATE_LIMIT: Amount = Amount::from_u64(1_000_000); const DEFAULT_MINT_LIMIT: Amount = Amount::from_u64(1); @@ -1335,7 +1335,7 @@ fn shielded_recv_memo_value( /// 2. Test sending the above transfer back to first cosmos chain via PFM /// 3. Send wrapped NAM from first cosmos chain to the second via PFM /// 4. Reverse the transaction in the step above -//#[test] +#[test] fn ibc_pfm_happy_flows() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -1632,7 +1632,7 @@ fn ibc_pfm_happy_flows() -> Result<()> { /// failing to send it back to the second due to an error /// 6. Same as above except that the failure occurs due to a time-out on the /// second cosmos chain. -//#[test] +#[test] fn ibc_pfm_unhappy_flows() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -2058,7 +2058,7 @@ fn ibc_pfm_unhappy_flows() -> Result<()> { /// Test that we are able to use the shielded-receive /// middleware to shield funds specified in the memo /// message. -//#[test] +#[test] fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -2102,7 +2102,12 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { NAM, 10, ALBERT_KEY, - &["--shielding-fee-payer", BERTHA_KEY, "--shielding-fee-token", NAM], + &[ + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + ], )?; check_shielded_balance(&test, AA_VIEWING_KEY, NAM, 10)?; @@ -2158,7 +2163,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { /// Test that if the received amount underflows the minimum /// amount, we error out and refund assets. -//#[test] +#[test] fn ibc_shielded_recv_middleware_unhappy_flow() -> Result<()> { let update_genesis = |mut genesis: templates::All, base_dir: &_| { @@ -3554,7 +3559,7 @@ fn nft_transfer_from_cosmos( } /// Basic Osmosis test that checks if the chain has been set up correctly. -//#[test] +#[test] fn osmosis_basic() -> Result<()> { let (osmosis, test_osmosis) = setup_and_boot_cosmos(CosmosChainType::Osmosis)?; @@ -3567,7 +3572,7 @@ fn osmosis_basic() -> Result<()> { Ok(()) } -//#[test] +#[test] fn osmosis_xcs() -> Result<()> { // ========================================================== // This test requires quite a long setup. Jump to the next diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 4036c6a59b0..d3ab2306b48 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -91,7 +91,7 @@ pub fn start_namada_ledger_node_wait_wasm( wait_for_wasm_pre_compile(&mut node)?; Ok(node) } -/* + /// Test that when we "run-ledger" with all the possible command /// combinations from fresh state, the node starts-up successfully for both a /// validator and non-validator user. @@ -1118,7 +1118,7 @@ fn ledger_many_txs_in_a_block() -> Result<()> { Ok(()) } -*/ + pub fn write_json_file(proposal_path: &std::path::Path, proposal_content: T) where T: Serialize, @@ -1132,7 +1132,7 @@ where serde_json::to_writer(intent_writer, &proposal_content).unwrap(); } -/* + /// In this test we intentionally make a validator node double sign blocks /// to test that slashing evidence is received and processed by the ledger /// correctly: @@ -1536,7 +1536,7 @@ fn test_epoch_sleep() -> Result<()> { Ok(()) } -*/ + /// Prepare proposal data in the test's temp dir from the given source address. /// This can be submitted with "init-proposal" command. pub fn prepare_proposal_data( @@ -1571,7 +1571,7 @@ pub fn prepare_proposal_data( write_json_file(valid_proposal_json_path.as_path(), valid_proposal_json); valid_proposal_json_path } -/* + #[test] fn deactivate_and_reactivate_validator() -> Result<()> { let pipeline_len = 2; @@ -2844,4 +2844,3 @@ fn test_genesis_manipulation() -> Result<()> { Ok(()) } -*/ \ No newline at end of file diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 4d14d7407fc..937292c2e33 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -58,8 +58,8 @@ pub mod storage_key { masp_base_native_precision_key, masp_commitment_anchor_key, masp_commitment_tree_key, masp_conversion_key, masp_convert_anchor_key, masp_nullifier_key, masp_scheduled_base_native_precision_key, - masp_scheduled_reward_precision_key, - masp_token_map_key, masp_total_rewards, + masp_scheduled_reward_precision_key, masp_token_map_key, + masp_total_rewards, }; pub use namada_trans_token::storage_key::*; diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index 6ee30204725..17aa0cc5e9c 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -8,10 +8,10 @@ use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; use namada_core::masp::encode_asset_type; use namada_core::masp_primitives::transaction::Transaction; +use namada_core::storage::{DbKeySeg, Key}; use namada_core::token::{DenominatedAmount, MaspDigitPos}; use namada_core::uint::I320; use namada_core::{masp, token}; -use namada_core::storage::{DbKeySeg, Key}; use namada_events::EmitEvents; use namada_shielded_token::storage_key::masp_undated_balance_key; use namada_shielded_token::{MaspTxId, read_undated_balance, utils}; @@ -262,11 +262,13 @@ where /// token. fn masp_shielding_fee_amount(token: &Address) -> Key { pub const MASP_SHIELDING_FEE_PREFIX: &str = "shielding_fee"; - namada_core::storage::Key::from(DbKeySeg::AddressSeg(Address::Internal(InternalAddress::Parameters))) - .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) - .expect("Cannot obtain a storage key") - .push(token) - .expect("Cannot obtain a storage key") + namada_core::storage::Key::from(DbKeySeg::AddressSeg(Address::Internal( + InternalAddress::Parameters, + ))) + .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") + .push(token) + .expect("Cannot obtain a storage key") } #[cfg(test)] From 9ef5631eaeedf2b3a0e9821778830a35c517e560 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 14 Jul 2025 10:20:23 +0200 Subject: [PATCH 04/24] Add changelog --- .../unreleased/features/4276-add-shielding-fee-section.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/4276-add-shielding-fee-section.md diff --git a/.changelog/unreleased/features/4276-add-shielding-fee-section.md b/.changelog/unreleased/features/4276-add-shielding-fee-section.md new file mode 100644 index 00000000000..f21904c1476 --- /dev/null +++ b/.changelog/unreleased/features/4276-add-shielding-fee-section.md @@ -0,0 +1,2 @@ +- Adds a shielding fee sectio to txs. This forces a fee to paid when shielding + to the masp ([\#4276](https://github.com/anoma/namada/issues/4276)) \ No newline at end of file From 3ec74e1d8105c332e465a992b2c36326080be862 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 14 Jul 2025 11:10:11 +0200 Subject: [PATCH 05/24] Added shielding fee to an e2e test --- crates/tests/src/e2e/ledger_tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index d3ab2306b48..60bdbcf3625 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -2550,6 +2550,10 @@ fn masp_txs_and_queries() -> Result<()> { "--token", BTC, "--amount", + "--shielding-fee-payer", + ALBERT_KEY, + "--shielding-token", + NAM, "20", ]), TX_APPLIED_SUCCESS, From cd35e300d3e18436c874fe34d1ad2a64db4842fd Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 14 Jul 2025 11:44:11 +0200 Subject: [PATCH 06/24] Fixing e2e tests --- crates/tests/src/e2e/ibc_tests.rs | 2 +- crates/tests/src/e2e/ledger_tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 0cfafa41ce0..0c113b7dfe0 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -1000,7 +1000,7 @@ fn ibc_token_inflation() -> Result<()> { // wait the next masp epoch to dispense the reward let mut epoch = get_epoch(&test, &rpc).unwrap(); - let new_epoch = Epoch(epoch.0 * MASP_EPOCH_MULTIPLIER); + let new_epoch = epoch + MASP_EPOCH_MULTIPLIER; while epoch < new_epoch { epoch = epoch_sleep(&test, &rpc, 120)?; } diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 60bdbcf3625..55d06d568d1 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -2550,11 +2550,11 @@ fn masp_txs_and_queries() -> Result<()> { "--token", BTC, "--amount", + "20", "--shielding-fee-payer", ALBERT_KEY, "--shielding-token", NAM, - "20", ]), TX_APPLIED_SUCCESS, ), From 61f34b7418ffaa0edcbc6fb58bf48b431447ab6e Mon Sep 17 00:00:00 2001 From: Jacob Turner Date: Mon, 14 Jul 2025 14:45:36 +0200 Subject: [PATCH 07/24] Update crates/tests/src/e2e/ledger_tests.rs Co-authored-by: Tomas Zemanovic --- crates/tests/src/e2e/ledger_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 55d06d568d1..a8ae64ec4e0 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -2553,7 +2553,7 @@ fn masp_txs_and_queries() -> Result<()> { "20", "--shielding-fee-payer", ALBERT_KEY, - "--shielding-token", + "--shielding-fee-token", NAM, ]), TX_APPLIED_SUCCESS, From 521ae102318a272eb5656b192a402c868e67dde0 Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 15 Jul 2025 12:27:18 +0200 Subject: [PATCH 08/24] Fixed benches --- crates/apps_lib/src/client/rpc.rs | 3 +-- crates/benches/native_vps.rs | 2 +- crates/node/src/bench_utils.rs | 24 +++++++++++++++++++----- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 3390daacb61..9128e243ee0 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -958,8 +958,7 @@ pub async fn query_protocol_parameters( ); display_line!( context.io(), - "{:4}Masp shielding fee payment (NAM): {:?} gas units", - "", + "{:4}Masp shielding fee payment (NAM)", masp_nam_shielding_fee ); display_line!(context.io(), "{:4}Minimum gas costs:", ""); diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index 63ba6afebe2..563466ae253 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -1832,4 +1832,4 @@ criterion_group!( ibc_vp_validate_action, ibc_vp_execute_action ); -criterion_main!(native_vps); +criterion_main!(native_vps); \ No newline at end of file diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 7cf954a68ee..ea39619775b 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -22,6 +22,7 @@ use masp_proofs::prover::LocalTxProver; use namada_apps_lib::cli; use namada_apps_lib::cli::Context; use namada_apps_lib::cli::context::FromContext; +use namada_apps_lib::key::SchemeType; use namada_apps_lib::wallet::{CliWalletUtils, defaults}; use namada_sdk::address::{self, Address, InternalAddress, MASP}; use namada_sdk::args::ShieldedSync; @@ -132,6 +133,7 @@ pub const WASM_DIR: &str = "../../wasm"; pub const ALBERT_PAYMENT_ADDRESS: &str = "albert_payment"; pub const ALBERT_SPENDING_KEY: &str = "albert_spending"; +pub const ALBERT_KEY: &str = "albert_key"; pub const BERTHA_PAYMENT_ADDRESS: &str = "bertha_payment"; const BERTHA_SPENDING_KEY: &str = "bertha_spending"; @@ -186,9 +188,7 @@ impl BenchShellInner { if let Some(sections) = extra_sections { for section in sections { - if let Section::ExtraData(_) = section { - tx.add_section(section); - } + tx.add_section(section); } } @@ -1191,6 +1191,13 @@ impl Default for BenchShieldedCtx { ) .unwrap(); } + _ = chain_ctx.wallet.gen_store_secret_key( + SchemeType::Ed25519, + Some(ALBERT_KEY.to_string()), + true, + None, + &mut OsRng, + ); namada_apps_lib::wallet::save(&chain_ctx.wallet).unwrap(); @@ -1219,6 +1226,8 @@ impl BenchShieldedCtx { spending_key, self.wallet.find_birthday(ALBERT_SPENDING_KEY).copied(), ); + let shielding_fee_key = + self.wallet.find_public_key(ALBERT_KEY).unwrap(); self.shielded = async_runtime .block_on(namada_apps_lib::client::masp::syncing( self.shielded, @@ -1245,7 +1254,7 @@ impl BenchShieldedCtx { self.wallet, self.shielded.into(), StdIo, - native_token, + native_token.clone(), ); let masp_transfer_data = MaspTransferData { sources: vec![( @@ -1308,7 +1317,11 @@ impl BenchShieldedCtx { ) .unwrap(), Some(shielded), - None, + Some(vec![Section::ShieldingFee { + payer: shielding_fee_key, + token: native_token, + cmt: shielded_section_hash, + }]), vec![&defaults::albert_keypair()], ) } else { @@ -1327,6 +1340,7 @@ impl BenchShieldedCtx { vec![&defaults::albert_keypair()], ) }; + let NamadaImpl { client, wallet, From 929d320830ffc0d777c7bcd1ec5e236dd3981c02 Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 15 Jul 2025 14:33:09 +0200 Subject: [PATCH 09/24] Fixing pr comments --- crates/benches/native_vps.rs | 2 +- crates/node/src/shell/testing/client.rs | 32 ++++++++++--------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index 563466ae253..63ba6afebe2 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -1832,4 +1832,4 @@ criterion_group!( ibc_vp_validate_action, ibc_vp_execute_action ); -criterion_main!(native_vps); \ No newline at end of file +criterion_main!(native_vps); diff --git a/crates/node/src/shell/testing/client.rs b/crates/node/src/shell/testing/client.rs index 857b44b9c93..228c41680c4 100644 --- a/crates/node/src/shell/testing/client.rs +++ b/crates/node/src/shell/testing/client.rs @@ -11,7 +11,7 @@ use namada_sdk::io::Io; use namada_sdk::signing::{SigningTxData, default_sign}; use namada_sdk::tx::data::GasLimit; use namada_sdk::tx::{ProcessTxResponse, Tx}; -use namada_sdk::{signing, tendermint_rpc, tx}; +use namada_sdk::{signing, tendermint_rpc}; use super::node::MockNode; use crate::shell::testing::utils::{Bin, TestingIo}; @@ -118,7 +118,7 @@ impl CliClient for MockNode { /// Manually sign a tx. This can be used to sign a tx that was dumped pub fn sign_tx( node: &MockNode, - tx: Tx, + mut tx: Tx, signing: SigningTxData, // this is only used to give the password for decrypting keys to the wallet args: &args::Tx, @@ -138,25 +138,19 @@ pub fn sign_tx( .to_sdk(node.clone(), TestingIo); let rt = tokio::runtime::Runtime::new().unwrap(); - let (mut batched_tx, batched_signing_data) = - tx::build_batch(vec![(tx, signing)]) - .wrap_err("Failed to build tx batch")?; rt.block_on(async { - for sig_data in batched_signing_data { - signing::sign_tx( - ctx.wallet_lock(), - args, - &mut batched_tx, - sig_data, - default_sign, - (), - ) - .await - .wrap_err("Signing tx failed")?; - } - Ok::<(), Report>(()) + signing::sign_tx( + ctx.wallet_lock(), + args, + &mut tx, + signing, + default_sign, + (), + ) + .await + .wrap_err("Signing tx failed") })?; - Ok(batched_tx) + Ok(tx) } /// Manually submit a tx. Used for txs that have been manually constructed From cae58a68c771275c43ef1101e1628af95f57047b Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 15 Jul 2025 15:56:29 +0200 Subject: [PATCH 10/24] Added fee shielding checks to masp vp --- crates/sdk/src/tx.rs | 12 +++--------- crates/shielded_token/src/vp.rs | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index d48a9e6c22b..ac1e17f63b1 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2747,15 +2747,9 @@ pub async fn build_ibc_transfer( Some(source.clone()), Some(source.clone()), vec![], - if let Some(IbcShieldingData { - shielding_fee_payer: payer, - .. - }) = &args.ibc_shielding_data - { - Some(payer.clone()) - } else { - None - }, + args.ibc_shielding_data + .as_ref() + .map(|shielding_data| shielding_data.shielding_fee_payer.clone()), args.source.spending_key().is_some(), vec![], None, diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index c112c932c24..1915d7856b4 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -624,13 +624,15 @@ where return Err(error); } } + let masp_tx_id = shielded_tx.txid().into(); + let has_trans_inputs = shielded_tx + .transparent_bundle() + .map(|b| !b.vin.empty()) + .unwrap_or(false); // The transaction shall not push masp authorizer actions that are not // needed because this might lead vps to run a wrong validation logic if !actions_authorizers.is_empty() - && shielded_tx - .transparent_bundle() - .map(|b| b.vin.is_empty()) - .unwrap_or(true) + && !has_trans_inputs { let error = Error::new_const( "Found masp authorizer actions that are not required", @@ -638,6 +640,15 @@ where tracing::debug!("{error}"); return Err(error); } + // check that the tx has a shielding fee section + if has_trans_inputs && batched_tx.tx.get_shielding_fee_section(&masp_tx_id).is_none() + { + let error = Error::new_const( + "Found a shielding transaction with a shielding fee section", + ); + tracing::debug!("{error}"); + return Err(error); + } // Verify the proofs verify_shielded_tx(&shielded_tx, |gas| ctx.charge_gas(gas)) From 8fdc6df5a3606e0ec341ac322337e61d7c234ba3 Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 15 Jul 2025 15:58:02 +0200 Subject: [PATCH 11/24] typo --- crates/shielded_token/src/vp.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 1915d7856b4..c8f20bf67a4 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -627,13 +627,11 @@ where let masp_tx_id = shielded_tx.txid().into(); let has_trans_inputs = shielded_tx .transparent_bundle() - .map(|b| !b.vin.empty()) + .map(|b| !b.vin.is_empty()) .unwrap_or(false); // The transaction shall not push masp authorizer actions that are not // needed because this might lead vps to run a wrong validation logic - if !actions_authorizers.is_empty() - && !has_trans_inputs - { + if !actions_authorizers.is_empty() && !has_trans_inputs { let error = Error::new_const( "Found masp authorizer actions that are not required", ); @@ -641,7 +639,11 @@ where return Err(error); } // check that the tx has a shielding fee section - if has_trans_inputs && batched_tx.tx.get_shielding_fee_section(&masp_tx_id).is_none() + if has_trans_inputs + && batched_tx + .tx + .get_shielding_fee_section(&masp_tx_id) + .is_none() { let error = Error::new_const( "Found a shielding transaction with a shielding fee section", From 249b89c836dc8c4b5f25246df7a94e4fa36a52fc Mon Sep 17 00:00:00 2001 From: satan Date: Wed, 16 Jul 2025 16:41:29 +0200 Subject: [PATCH 12/24] Strengthened vp checks and add some test cases --- crates/apps_lib/src/client/rpc.rs | 9 +- crates/apps_lib/src/config/genesis/chain.rs | 2 +- .../apps_lib/src/config/genesis/templates.rs | 37 ++--- crates/core/src/parameters.rs | 5 +- crates/parameters/src/lib.rs | 18 ++- crates/proof_of_stake/src/lib.rs | 3 +- crates/shielded_token/src/vp.rs | 128 +++++++++++++++--- crates/state/src/lib.rs | 2 +- crates/systems/src/parameters.rs | 9 ++ crates/tests/src/integration/masp.rs | 105 +++++++++++++- crates/token/src/tx.rs | 18 +-- crates/tx_env/src/lib.rs | 7 + crates/tx_prelude/src/lib.rs | 9 ++ 13 files changed, 273 insertions(+), 79 deletions(-) diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 9128e243ee0..4c9d6088029 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -847,10 +847,11 @@ pub async fn query_protocol_parameters( .await .expect("Native token should be defined"); let key = param_storage::masp_shielding_fee_amount(&native_token); - let masp_nam_shielding_fee: DenominatedAmount = - query_storage_value(context.client(), &key) + let masp_nam_shielding_fee = + query_storage_value::<_, DenominatedAmount>(context.client(), &key) .await - .expect("Parameter should be defined."); + .expect("Parameter should be defined.") + .amount(); let key = param_storage::get_gas_cost_key(); let minimum_gas_price: BTreeMap = query_storage_value(context.client(), &key) @@ -882,7 +883,7 @@ pub async fn query_protocol_parameters( epochs_per_year, masp_epoch_multiplier, masp_fee_payment_gas_limit, - masp_nam_shielding_fee: masp_nam_shielding_fee.to_string_precise(), + masp_nam_shielding_fee, gas_scale, minimum_gas_price, is_native_token_transferable, diff --git a/crates/apps_lib/src/config/genesis/chain.rs b/crates/apps_lib/src/config/genesis/chain.rs index 95e0f682117..d3f22d5305b 100644 --- a/crates/apps_lib/src/config/genesis/chain.rs +++ b/crates/apps_lib/src/config/genesis/chain.rs @@ -374,7 +374,7 @@ impl Finalized { masp_epoch_multiplier, max_proposal_bytes, masp_fee_payment_gas_limit, - masp_nam_shielding_fee, + masp_nam_shielding_fee: masp_nam_shielding_fee.amount(), gas_scale, max_block_gas, minimum_gas_price: minimum_gas_price diff --git a/crates/apps_lib/src/config/genesis/templates.rs b/crates/apps_lib/src/config/genesis/templates.rs index cc4d50e3a75..5d854dfa936 100644 --- a/crates/apps_lib/src/config/genesis/templates.rs +++ b/crates/apps_lib/src/config/genesis/templates.rs @@ -3,9 +3,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; use std::path::Path; -use std::str::FromStr; -use eyre::eyre; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] use namada_migrations::*; @@ -293,7 +291,7 @@ pub struct ChainParams { /// Gas limit of a masp transaction paying fees pub masp_fee_payment_gas_limit: u64, /// The amount of NAM the MASP shielding fee costs - pub masp_nam_shielding_fee: String, + pub masp_nam_shielding_fee: DenominatedAmount, /// Gas scale pub gas_scale: u64, /// Map of the cost per gas unit for every token allowed for fee payment @@ -350,29 +348,16 @@ impl ChainParams { })?; min_gas_prices.insert(token, amount); } - let masp_nam_shielding_fee = - match DenominatedAmount::from_str(&masp_nam_shielding_fee) { - Ok(amount) => amount - .increase_precision(NATIVE_MAX_DECIMAL_PLACES.into()) - .map_err(|e| { - eprintln!( - "A MASP shielding fee (in NAM) in the \ - parameters.toml file was incorrectly \ - denominated:\n{}", - e - ); - e - })?, - Err(e) => { - eprintln!( - "A MASP shielding fee (in NAM) in the parameters.toml \ - file was incorrectly formatted:\n{}", - e - ); - return Err(eyre!("{}", e.to_string())); - } - } - .to_string_precise(); + let masp_nam_shielding_fee = masp_nam_shielding_fee + .increase_precision(NATIVE_MAX_DECIMAL_PLACES.into()) + .map_err(|e| { + eprintln!( + "A MASP shielding fee (in NAM) in the parameters.toml \ + file was incorrectly denominated:\n{}", + e + ); + e + })?; Ok(ChainParams { max_tx_bytes, diff --git a/crates/core/src/parameters.rs b/crates/core/src/parameters.rs index 95dcab76e42..407d1c3b0b9 100644 --- a/crates/core/src/parameters.rs +++ b/crates/core/src/parameters.rs @@ -15,6 +15,7 @@ use super::hash::Hash; use super::time::DurationSecs; use super::token; use crate::borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; +use crate::token::Amount; /// Protocol parameters #[derive( @@ -53,7 +54,7 @@ pub struct Parameters { /// The gas limit for a masp transaction paying fees pub masp_fee_payment_gas_limit: u64, /// The amount of NAM the MASP shielding fee costs - pub masp_nam_shielding_fee: String, + pub masp_nam_shielding_fee: Amount, /// Gas scale pub gas_scale: u64, /// Map of the cost per gas unit for every token allowed for fee payment @@ -100,7 +101,7 @@ impl Default for Parameters { epochs_per_year: 365, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 0, - masp_nam_shielding_fee: 0.to_string(), + masp_nam_shielding_fee: Amount::zero(), gas_scale: 100_000_000, minimum_gas_price: Default::default(), is_native_token_transferable: true, diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index a2aa687d173..41b2b2f6a15 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -22,7 +22,6 @@ pub mod vp; mod wasm_allowlist; use std::collections::BTreeMap; use std::marker::PhantomData; -use std::str::FromStr; use namada_core::address::{Address, InternalAddress}; use namada_core::arith::checked; @@ -37,6 +36,8 @@ pub use storage::{get_gas_scale, get_max_block_gas}; use thiserror::Error; pub use wasm_allowlist::{is_tx_allowed, is_vp_allowed}; +use crate::storage::masp_shielding_fee_amount; + /// Parameters storage `Keys/Read/Write` implementation #[derive(Debug)] pub struct Store(PhantomData); @@ -82,6 +83,14 @@ where num_blocks_to_read, ) } + + fn masp_shielding_fee_amount( + storage: &S, + token: &Address, + ) -> Result> { + let key = masp_shielding_fee_amount(token); + storage.read(&key) + } } impl Write for Store @@ -165,8 +174,7 @@ where let masp_nam_shielding_fee_key = storage::masp_shielding_fee_amount(native_token); let masp_nam_shielding_fee = - DenominatedAmount::from_str(masp_nam_shielding_fee) - .map_err(|e| Error::AllocMessage(e.to_string()))? + DenominatedAmount::native(*masp_nam_shielding_fee) .redenominate(NATIVE_MAX_DECIMAL_PLACES); storage.write(&masp_nam_shielding_fee_key, masp_nam_shielding_fee)?; @@ -428,7 +436,7 @@ where let masp_nam_shielding_fee = value .ok_or(ReadError::ParametersMissing) .into_storage_result()? - .to_string_precise(); + .amount(); // read gas scale let gas_scale_key = storage::get_gas_scale_key(); @@ -522,7 +530,7 @@ where epochs_per_year: 365, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 0, - masp_nam_shielding_fee: 0.to_string(), + masp_nam_shielding_fee: namada_core::token::Amount::zero(), gas_scale: 10_000_000, minimum_gas_price: Default::default(), is_native_token_transferable: true, diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 841246c165a..843a87d4564 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -3214,6 +3214,7 @@ pub mod test_utils { use namada_core::hash::Hash; use namada_core::parameters::{EpochDuration, ProposalBytes}; use namada_core::time::DurationSecs; + use namada_core::token::Amount; use super::*; use crate::types::GenesisValidator; @@ -3316,7 +3317,7 @@ pub mod test_utils { epochs_per_year: 10000000, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 10000, - masp_nam_shielding_fee: 0.to_string(), + masp_nam_shielding_fee: Amount::zero(), gas_scale: 100_000_000, minimum_gas_price: BTreeMap::new(), is_native_token_transferable: true, diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index c8f20bf67a4..8000d6b5857 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -13,14 +13,14 @@ use masp_primitives::transaction::components::{ I128Sum, TxIn, TxOut, ValueSum, }; use masp_primitives::transaction::{Transaction, TransparentAddress}; -use namada_core::address::{self, Address}; +use namada_core::address::{self, Address, PGF}; use namada_core::arith::{CheckedAdd, CheckedSub, checked}; use namada_core::booleans::BoolResultUnitExt; use namada_core::collections::HashSet; use namada_core::masp::{MaspEpoch, TAddrData, addr_taddr, encode_asset_type}; use namada_core::storage::Key; use namada_core::token; -use namada_core::token::{Amount, MaspDigitPos}; +use namada_core::token::{Amount, DenominatedAmount, MaspDigitPos}; use namada_core::uint::I320; use namada_state::{ ConversionState, OptionExt, ReadConversionState, ResultExt, @@ -416,6 +416,98 @@ where }) } + /// Check that the tx has a shielding fee section. The signer in that + /// section is checked to be in the `actions_authorizers` set and + /// `verifiers` set. + fn check_masp_fees( + ctx: &'ctx CTX, + shielded_tx: &Transaction, + batched_tx: &BatchedTxRef<'_>, + verifiers: &BTreeSet
, + actions_authorizers: &mut HashSet<&Address>, + ) -> Result<()> { + let masp_tx_id = shielded_tx.txid().into(); + let has_trans_inputs = shielded_tx + .transparent_bundle() + .map(|b| !b.vin.is_empty()) + .unwrap_or(false); + let (fee_authorizer, fee_token) = + batched_tx.tx.get_shielding_fee_section(&masp_tx_id).unzip(); + if has_trans_inputs { + match fee_authorizer { + None => { + let error = Error::new_const( + "Found a shielding transaction without a shielding \ + fee section", + ); + tracing::debug!("{error}"); + return Err(error); + } + Some(pk) => { + let signer = Address::from(pk); + if !verifiers.contains(&signer) { + let error = Error::new_alloc(format!( + "The required vp of address {signer} was not \ + triggered" + )); + tracing::debug!("{error}"); + return Err(error); + } + if !actions_authorizers.swap_remove(&signer) { + let error = Error::new_const( + "Shielding fee payer is not in the authorizer set", + ); + tracing::debug!("{error}"); + return Err(error); + }; + } + } + let fee_token = fee_token.expect("This cannot fail"); + let Some(denom) = TransToken::read_denom(&ctx.pre(), fee_token) + .expect("Could not read storage") + else { + let error = Error::new_alloc(format!( + "Cannot pay shielding fees with token {fee_token}: No \ + denomination known." + )); + tracing::debug!("{error}"); + return Err(error); + }; + let balance_key = TransToken::balance_key(fee_token, &PGF); + let pre_balance = DenominatedAmount::new( + ctx.read_pre::(&balance_key) + .expect("Could not read storage") + .unwrap_or_default(), + denom, + ); + let post_balance = DenominatedAmount::new( + ctx.read_post::(&balance_key) + .expect("Could not read storage") + .unwrap_or_default(), + denom, + ); + let delta = checked!(post_balance - pre_balance)?; + let Some(fee_amount) = + Params::masp_shielding_fee_amount(&ctx.pre(), fee_token) + .expect("Could not read storage") + else { + let error = Error::new_alloc(format!( + "Cannot pay shielding fees with token {fee_token}" + )); + tracing::debug!("{error}"); + return Err(error); + }; + if fee_amount != delta { + let error = Error::new_const( + "The fee for shielding was not correctly paid.", + ); + tracing::debug!("{error}"); + return Err(error); + } + } + Ok(()) + } + // Check that MASP Transaction and state changes are valid fn is_valid_masp_transfer( ctx: &'ctx CTX, @@ -558,6 +650,15 @@ where } }) .collect(); + + Self::check_masp_fees( + ctx, + &shielded_tx, + batched_tx, + verifiers, + &mut actions_authorizers, + )?; + // Ensure that this transaction is authorized by all involved parties for authorizer in authorizers { if let Some(TAddrData::Addr(address::IBC)) = @@ -602,7 +703,7 @@ where return Err(error); } - // The action is required becuse the target vp might have been + // The action is required because the target vp might have been // triggered for other reasons but we need to signal it that it // is required to validate a discrepancy in its balance change // because of a masp transaction, which might require a @@ -624,33 +725,16 @@ where return Err(error); } } - let masp_tx_id = shielded_tx.txid().into(); - let has_trans_inputs = shielded_tx - .transparent_bundle() - .map(|b| !b.vin.is_empty()) - .unwrap_or(false); + // The transaction shall not push masp authorizer actions that are not // needed because this might lead vps to run a wrong validation logic - if !actions_authorizers.is_empty() && !has_trans_inputs { + if !actions_authorizers.is_empty() { let error = Error::new_const( "Found masp authorizer actions that are not required", ); tracing::debug!("{error}"); return Err(error); } - // check that the tx has a shielding fee section - if has_trans_inputs - && batched_tx - .tx - .get_shielding_fee_section(&masp_tx_id) - .is_none() - { - let error = Error::new_const( - "Found a shielding transaction with a shielding fee section", - ); - tracing::debug!("{error}"); - return Err(error); - } // Verify the proofs verify_shielded_tx(&shielded_tx, |gas| ctx.charge_gas(gas)) diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index f1b377c1fe9..bcca285be92 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -759,7 +759,7 @@ mod tests { epochs_per_year: 100, masp_epoch_multiplier: 2, masp_fee_payment_gas_limit: 20_000, - masp_nam_shielding_fee: 0.to_string(), + masp_nam_shielding_fee: namada_core::token::Amount::zero(), gas_scale: 10_000_000, minimum_gas_price: BTreeMap::default(), is_native_token_transferable: true, diff --git a/crates/systems/src/parameters.rs b/crates/systems/src/parameters.rs index a8fd89e4098..bfcc8ffadc8 100644 --- a/crates/systems/src/parameters.rs +++ b/crates/systems/src/parameters.rs @@ -1,9 +1,11 @@ //! Parameters abstract interfaces +use namada_core::address::Address; use namada_core::chain::BlockHeight; pub use namada_core::parameters::*; use namada_core::storage; use namada_core::time::DurationSecs; +use namada_core::token::DenominatedAmount; pub use namada_storage::Result; /// Abstract parameters storage keys interface @@ -37,6 +39,13 @@ pub trait Read { last_block_height: BlockHeight, num_blocks_to_read: u64, ) -> Result; + + /// Return the amount of the given token is necessary to pay + /// a shielding fee + fn masp_shielding_fee_amount( + storage: &S, + token: &Address, + ) -> Result>; } /// Abstract parameters storage write interface diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index d1bea18620e..2f8f5d079e9 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -1492,6 +1492,8 @@ fn enable_rewards_after_shielding() -> Result<()> { /// rejected. /// 4. Test that if the account listed in the shielding fee does not sign the /// tx, it is rejected +/// 5. Test that duplicating a shielding fee section does not result in paying +/// the fee twice #[test] fn test_shielding_fee_protocol_checks() -> Result<()> { // This address doesn't matter for tests. But an argument is required. @@ -1632,6 +1634,24 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); std::fs::remove_file(tx_path).expect("Test failed"); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + BERTHA_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // 1998000 = 2000000 - 4000 + assert!(captured.contains("nam: 1996000")); // we remove the shielding fee section run( @@ -1801,7 +1821,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { .display() .to_string(); let tx: Tx = serde_json::from_reader( - std::fs::File::open(tx_path).expect("Test Failed"), + std::fs::File::open(&tx_path).expect("Test Failed"), ) .expect("Test failed"); let signing_data = SigningTxData { @@ -1822,6 +1842,89 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); assert!(captured.result.is_ok()); assert!(captured.contains("The section signature is invalid")); + std::fs::remove_file(tx_path).expect("Test failed"); + + // now we duplicate the shielding fee section + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + "0.1", + "--signing-keys", + ALBERT_KEY, + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, + "--dump-wrapper-tx", + "--output-folder-path", + output_folder.to_str().unwrap(), + "--node", + validator_one_rpc, + ]), + )?; + let tx_path = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + let mut tx: Tx = serde_json::from_reader( + std::fs::File::open(tx_path).expect("Test Failed"), + ) + .expect("Test failed"); + let shielding_fee_section = tx + .sections + .iter() + .find(|s| matches!(s, Section::ShieldingFee { .. })) + .expect("Test failed") + .clone(); + tx.add_section(shielding_fee_section); + let signing_data = SigningTxData { + owner: Some(node.lookup_address(ALBERT)?), + public_keys: HashSet::from([node.lookup_pk(ALBERT_KEY)?]), + threshold: 1, + account_public_keys_map: Some( + [node.lookup_pk(ALBERT_KEY)?].into_iter().collect(), + ), + fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), + shielded_hash: None, + // this should be Bertha's key + shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + signatures: vec![], + }; + let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; + let captured = + CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + BERTHA_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // 1998000 = 2000000 - 6000 + assert!(captured.contains("nam: 1994000")); Ok(()) } diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index 17aa0cc5e9c..834f1e0c8d2 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -3,12 +3,11 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; -use namada_core::address::{InternalAddress, PGF}; +use namada_core::address::PGF; use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; use namada_core::masp::encode_asset_type; use namada_core::masp_primitives::transaction::Transaction; -use namada_core::storage::{DbKeySeg, Key}; use namada_core::token::{DenominatedAmount, MaspDigitPos}; use namada_core::uint::I320; use namada_core::{masp, token}; @@ -218,7 +217,7 @@ where )))?; // transfer the shielding fee let amount: DenominatedAmount = env - .read(&masp_shielding_fee_amount(token)) + .get_masp_shielding_fee_amount(token) .expect("Failed to read storage") .ok_or_else(|| { Error::AllocMessage(format!( @@ -258,19 +257,6 @@ where Ok(()) } -/// The key for getting the shielding fee amount of the provided -/// token. -fn masp_shielding_fee_amount(token: &Address) -> Key { - pub const MASP_SHIELDING_FEE_PREFIX: &str = "shielding_fee"; - namada_core::storage::Key::from(DbKeySeg::AddressSeg(Address::Internal( - InternalAddress::Parameters, - ))) - .push(&MASP_SHIELDING_FEE_PREFIX.to_owned()) - .expect("Cannot obtain a storage key") - .push(token) - .expect("Cannot obtain a storage key") -} - #[cfg(test)] #[allow(clippy::arithmetic_side_effects, clippy::disallowed_types)] mod test { diff --git a/crates/tx_env/src/lib.rs b/crates/tx_env/src/lib.rs index 2bbb513f02e..da68ed12b96 100644 --- a/crates/tx_env/src/lib.rs +++ b/crates/tx_env/src/lib.rs @@ -24,6 +24,7 @@ pub use namada_core::borsh::{ }; pub use namada_core::masp::MaspTransaction; pub use namada_core::storage; +use namada_core::token::DenominatedAmount; pub use namada_events::{Event, EventToEmit, EventType}; pub use namada_storage::{Result, ResultExt, StorageRead, StorageWrite}; @@ -109,4 +110,10 @@ pub trait TxEnv: StorageRead + StorageWrite { fn update_masp_note_commitment_tree( transaction: &MaspTransaction, ) -> Result; + + /// The amount of the given token needed to pay a MASP shielding fee + fn get_masp_shielding_fee_amount( + &self, + token: &Address, + ) -> Result>; } diff --git a/crates/tx_prelude/src/lib.rs b/crates/tx_prelude/src/lib.rs index ea73a4cec7a..d570d290a58 100644 --- a/crates/tx_prelude/src/lib.rs +++ b/crates/tx_prelude/src/lib.rs @@ -37,6 +37,7 @@ pub use namada_core::ethereum_events::EthAddress; use namada_core::internal::HostEnvResult; use namada_core::key::common; use namada_core::storage::TxIndex; +use namada_core::token::DenominatedAmount; pub use namada_core::{address, encode, eth_bridge_pool, storage, *}; pub use namada_events::extend::Log; pub use namada_events::{ @@ -45,6 +46,7 @@ pub use namada_events::{ pub use namada_governance::storage as gov_storage; pub use namada_macros::transaction; pub use namada_parameters::storage as parameters_storage; +use namada_parameters::storage::masp_shielding_fee_amount; pub use namada_state::{ Error, OptionExt, Result, ResultExt, StorageRead, StorageWrite, collections, iter_prefix, iter_prefix_bytes, @@ -412,6 +414,13 @@ impl TxEnv for Ctx { ) -> Result { update_masp_note_commitment_tree(transaction) } + + fn get_masp_shielding_fee_amount( + &self, + token: &Address, + ) -> Result> { + self.read(&masp_shielding_fee_amount(token)) + } } impl namada_tx::action::Read for Ctx { From ff00f67bc0c472771dbbf0780486e9cc45691a2f Mon Sep 17 00:00:00 2001 From: satan Date: Thu, 17 Jul 2025 10:38:51 +0200 Subject: [PATCH 13/24] Fixing masp vp --- crates/shielded_token/src/vp.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 8000d6b5857..a77cb77f91c 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -445,14 +445,6 @@ where } Some(pk) => { let signer = Address::from(pk); - if !verifiers.contains(&signer) { - let error = Error::new_alloc(format!( - "The required vp of address {signer} was not \ - triggered" - )); - tracing::debug!("{error}"); - return Err(error); - } if !actions_authorizers.swap_remove(&signer) { let error = Error::new_const( "Shielding fee payer is not in the authorizer set", From 20346b9a2fe4cf4c52d96e34191c83bf8b780180 Mon Sep 17 00:00:00 2001 From: satan Date: Thu, 24 Jul 2025 15:52:08 +0200 Subject: [PATCH 14/24] Added new flow for shielding over ibc --- Cargo.lock | 2 + crates/apps_lib/src/client/tx.rs | 18 +++-- crates/core/src/masp.rs | 13 ++++ crates/ibc/src/lib.rs | 6 +- crates/ibc/src/msg.rs | 72 ++----------------- crates/sdk/src/args.rs | 19 +++-- crates/sdk/src/masp.rs | 2 +- crates/sdk/src/tx.rs | 70 ++++++++++++------ crates/shielded_token/src/lib.rs | 2 +- crates/shielded_token/src/vp.rs | 116 +++++++++++++++++++++++------- crates/systems/Cargo.toml | 2 + crates/systems/src/ibc.rs | 64 +++++++++++++++-- crates/tests/src/e2e/ibc_tests.rs | 2 +- crates/token/src/tx.rs | 85 ++++++++++++---------- crates/tx/src/action.rs | 34 ++++++++- crates/tx_prelude/src/token.rs | 2 +- crates/vm/src/host_env.rs | 59 ++++++++++++++- crates/vm/src/wasm/host_env.rs | 1 + crates/vm_env/src/lib.rs | 8 +++ crates/vp_prelude/src/lib.rs | 55 ++++++++++++++ wasm/Cargo.lock | 2 + wasm/tx_ibc/src/lib.rs | 19 ++++- wasm/vp_implicit/src/lib.rs | 15 +++- wasm/vp_user/src/lib.rs | 15 +++- wasm_for_tests/Cargo.lock | 2 + 25 files changed, 502 insertions(+), 183 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb8285bc040..923c61dd917 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6249,11 +6249,13 @@ dependencies = [ name = "namada_systems" version = "0.150.0" dependencies = [ + "borsh", "cargo_metadata 0.19.1", "lazy_static", "namada_core", "namada_events", "namada_storage", + "namada_tx", ] [[package]] diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 6dafd45edd4..15c44e58032 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -22,13 +22,15 @@ use namada_sdk::collections::HashMap; use namada_sdk::governance::cli::onchain::{ DefaultProposal, PgfFundingProposal, PgfStewardProposal, }; -use namada_sdk::ibc::convert_masp_tx_to_ibc_memo; use namada_sdk::io::{Io, display_line, edisplay_line}; use namada_sdk::key::*; use namada_sdk::rpc::{InnerTxResult, TxBroadcastData, TxResponse}; use namada_sdk::state::EPOCH_SWITCH_BLOCKS_DELAY; use namada_sdk::tx::data::compute_inner_tx_hash; -use namada_sdk::tx::{CompressedAuthorization, Section, Signer, Tx}; +use namada_sdk::tx::{ + CompressedAuthorization, Section, Signer, Tx, + convert_masp_tx_to_ibc_memo_data, +}; use namada_sdk::wallet::alias::{validator_address, validator_consensus_key}; use namada_sdk::wallet::{Wallet, WalletIo}; use namada_sdk::{ExtendedViewingKey, Namada, error, signing, tx}; @@ -2024,10 +2026,14 @@ pub async fn gen_ibc_shielding_transfer( }; let mut out = File::create(&output_path) .expect("Creating a new file for IBC MASP transaction failed."); - let bytes = convert_masp_tx_to_ibc_memo( - &masp_tx, - shielding_fee_payer, - shielding_fee_token, + let bytes = String::from( + convert_masp_tx_to_ibc_memo_data( + context, + &masp_tx, + shielding_fee_payer, + shielding_fee_token, + ) + .await?, ); out.write_all(bytes.as_bytes()) .expect("Writing IBC MASP transaction file failed."); diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index fe8fecee579..ae97cf349fb 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -23,6 +23,7 @@ use sha2::Sha256; use crate::address::{Address, DecodeError, HASH_HEX_LEN, IBC, MASP}; use crate::borsh::BorshSerializeExt; use crate::chain::Epoch; +use crate::hash::Hash; use crate::impl_display_and_from_str_via_format; use crate::string_encoding::{ self, MASP_EXT_FULL_VIEWING_KEY_HRP, MASP_EXT_SPENDING_KEY_HRP, @@ -79,6 +80,18 @@ impl From for MaspTxId { } } +impl From for MaspTxId { + fn from(hash: Hash) -> Self { + MaspTxId(TxIdInner::from_bytes(hash.0)) + } +} + +impl From for Hash { + fn from(tx_id: MaspTxId) -> Self { + Hash(*tx_id.0.as_ref()) + } +} + impl Display for MaspTxId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.0) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 4a98d76afc0..9eef61c1302 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -83,7 +83,6 @@ use ibc::core::router::types::error::RouterError; use ibc::primitives::proto::Any; pub use ibc::*; use ibc_middleware_packet_forward::PacketMetadata; -use masp_primitives::transaction::Transaction as MaspTransaction; pub use msg::*; use namada_core::address::{self, Address}; use namada_core::arith::{CheckedAdd, CheckedSub, checked}; @@ -101,6 +100,7 @@ use namada_state::{ State, StorageHasher, StorageRead, StorageWrite, WlState, }; use namada_systems::ibc::ChangedBalances; +pub use namada_systems::ibc::IbcShieldingData; use namada_systems::trans_token; pub use nft::*; use prost::Message; @@ -239,7 +239,7 @@ where { fn try_extract_masp_tx_from_envelope( tx_data: &[u8], - ) -> StorageResult> { + ) -> StorageResult> { let msg = decode_message::(tx_data) .into_storage_result() .ok(); @@ -578,7 +578,7 @@ pub struct InternalData { /// The transparent transfer that happens in parallel to IBC processes pub transparent: Option, /// The shielded transaction that happens in parallel to IBC processes - pub shielded: Option, + pub shielded: Option, /// IBC tokens that are credited/debited to internal accounts pub ibc_tokens: BTreeSet
, } diff --git a/crates/ibc/src/msg.rs b/crates/ibc/src/msg.rs index 9edea421cee..ed8baf095db 100644 --- a/crates/ibc/src/msg.rs +++ b/crates/ibc/src/msg.rs @@ -1,10 +1,7 @@ use std::collections::BTreeMap; -use std::fmt; -use std::str::FromStr; use borsh::schema::{Declaration, Definition, Fields}; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; -use data_encoding::HEXUPPER; use ibc::apps::nft_transfer::types::PORT_ID_STR as NFT_PORT_ID_STR; use ibc::apps::nft_transfer::types::msgs::transfer::MsgTransfer as IbcMsgNftTransfer; use ibc::apps::nft_transfer::types::packet::PacketData as NftPacketData; @@ -18,13 +15,9 @@ use ibc::core::channel::types::packet::Packet; use ibc::core::handler::types::msgs::MsgEnvelope; use ibc::core::host::types::identifiers::PortId; use ibc::primitives::proto::Protobuf; -use masp_primitives::transaction::Transaction as MaspTransaction; -use namada_core::address::Address; -use namada_core::borsh::BorshSerializeExt; -use namada_core::key::common::PublicKey; -use namada_core::string_encoding::StringEncoded; use serde::{Deserialize, Serialize}; - +use namada_core::string_encoding::StringEncoded; +use namada_systems::ibc::IbcShieldingData; use crate::trace; trait Sealed {} @@ -238,50 +231,11 @@ impl BorshSchema for MsgNftTransfer { } } -/// Shielding data in IBC packet memo -#[derive(Debug, Clone, BorshDeserialize, BorshSerialize)] -pub struct IbcShieldingData { - /// The MASP transaction that does the shielding - pub masp_tx: MaspTransaction, - /// The account that will pay the shielding fee - pub shielding_fee_payer: PublicKey, - /// The token that the shielding fee will be paid in - pub shielding_fee_token: Address, -} - -impl From<&IbcShieldingData> for String { - fn from(data: &IbcShieldingData) -> Self { - HEXUPPER.encode(&data.serialize_to_vec()) - } -} - -impl From for String { - fn from(data: IbcShieldingData) -> Self { - (&data).into() - } -} - -impl fmt::Display for IbcShieldingData { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", String::from(self)) - } -} - -impl FromStr for IbcShieldingData { - type Err = String; - - fn from_str(s: &str) -> Result { - let bytes = HEXUPPER - .decode(s.as_bytes()) - .map_err(|err| err.to_string())?; - IbcShieldingData::try_from_slice(&bytes).map_err(|err| err.to_string()) - } -} /// Extract MASP transaction from IBC envelope pub fn extract_masp_tx_from_envelope( envelope: &MsgEnvelope, -) -> Option { +) -> Option { match envelope { MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { extract_masp_tx_from_packet(&msg.packet) @@ -308,9 +262,11 @@ pub fn decode_ibc_shielding_data( } /// Extract MASP transaction from IBC packet memo -pub fn extract_masp_tx_from_packet(packet: &Packet) -> Option { +pub fn extract_masp_tx_from_packet( + packet: &Packet, +) -> Option { let memo = extract_memo_from_packet(packet, &packet.port_id_on_b)?; - decode_ibc_shielding_data(memo).map(|data| data.masp_tx) + decode_ibc_shielding_data(memo) } fn extract_memo_from_packet( @@ -373,17 +329,3 @@ pub fn extract_traces_from_recv_msg( _ => Ok(vec![]), } } - -/// Get IBC memo string from MASP transaction for receiving -pub fn convert_masp_tx_to_ibc_memo( - transaction: &MaspTransaction, - shielding_fee_payer: PublicKey, - shielding_fee_token: Address, -) -> String { - IbcShieldingData { - masp_tx: transaction.clone(), - shielding_fee_payer, - shielding_fee_token, - } - .into() -} diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index d116440f89b..74654000930 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -40,6 +40,7 @@ use crate::rpc::{ query_ibc_denom, query_osmosis_pool_routes, }; use crate::signing::{SigningTxData, gen_disposable_signing_key}; +use crate::tx::convert_masp_tx_to_ibc_memo_data; use crate::wallet::{DatedSpendingKey, DatedViewingKey}; use crate::{Namada, rpc, tx}; @@ -745,19 +746,17 @@ impl TxOsmosisSwap { "Failed to generate IBC shielding transfer".to_owned(), ) })?; - + let ibc_memo_data = convert_masp_tx_to_ibc_memo_data( + ctx, + &shielding_tx, + shielded_recipient.shielding_fee_payer, + shielded_recipient.shielding_fee_token, + ) + .await?; let memo = assert_json_obj( serde_json::to_value(&NamadaMemo { namada: NamadaMemoData::OsmosisSwap { - shielding_data: StringEncoded::new( - IbcShieldingData { - masp_tx: shielding_tx, - shielding_fee_payer: shielded_recipient - .shielding_fee_payer, - shielding_fee_token: shielded_recipient - .shielding_fee_token, - }, - ), + shielding_data: StringEncoded::new(ibc_memo_data), shielded_amount: amount_to_shield, overflow_receiver, }, diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 53f173c2bf0..f4c959c9822 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -81,7 +81,7 @@ fn extract_masp_tx( if let Some(transaction) = extract_masp_tx_from_envelope(&envelope) { - Ok(transaction) + Ok(transaction.masp_tx) } else { Err(Error::Other( "Failed to retrieve MASP over IBC transaction".to_string(), diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index ac1e17f63b1..56a3754df29 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -92,7 +92,7 @@ use crate::signing::{ use crate::tendermint_rpc::endpoint::broadcast::tx_sync::Response; use crate::tendermint_rpc::error::Error as RpcError; use crate::wallet::WalletIo; -use crate::{Namada, args, events}; +use crate::{Namada, args, error, events}; /// Initialize account transaction WASM pub const TX_INIT_ACCOUNT_WASM: &str = "tx_init_account.wasm"; @@ -2747,9 +2747,12 @@ pub async fn build_ibc_transfer( Some(source.clone()), Some(source.clone()), vec![], - args.ibc_shielding_data - .as_ref() - .map(|shielding_data| shielding_data.shielding_fee_payer.clone()), + args.ibc_shielding_data.as_ref().map(|shielding_data| { + shielding_data + .get_signer() + .cloned() + .expect("A MASP fee payer should have been provided") + }), args.source.spending_key().is_some(), vec![], None, @@ -2767,17 +2770,15 @@ pub async fn build_ibc_transfer( // check that if there is a shielding fee payer, they have enough // balance to cover the fee - let shielding_fee_payer = if let Some(IbcShieldingData { - shielding_fee_payer: payer, - shielding_fee_token, - .. - }) = &args.ibc_shielding_data + let shielding_fee_payer = if let Some(data) = + &args.ibc_shielding_data { validate_shielding_fee( context, Some(&mut updated_balance), - payer, - shielding_fee_token, + data.get_signer() + .expect("A MASP fee payer should have been provided"), + &data.shielding_fee_token, args.tx.force, ) .await? @@ -2920,16 +2921,14 @@ pub async fn build_ibc_transfer( if let Some(memo) = &args.tx.memo { tx.add_memo(memo); } - if let Some(IbcShieldingData { - shielding_fee_payer: payer, - shielding_fee_token: token, - masp_tx, - }) = &args.ibc_shielding_data - { + if let Some(data) = &args.ibc_shielding_data { tx.add_section(Section::ShieldingFee { - payer: payer.clone(), - token: token.clone(), - cmt: MaspTxId::from(masp_tx.txid()), + payer: data + .get_signer() + .cloned() + .expect("A MASP fee payer should have been provided"), + token: data.shielding_fee_token.clone(), + cmt: MaspTxId::from(data.masp_tx.txid()), }); } @@ -4634,3 +4633,34 @@ fn proposal_to_vec(proposal: OnChainProposal) -> Result> { borsh::to_vec(&proposal.content) .map_err(|e| Error::from(EncodingError::Conversion(e.to_string()))) } + +/// Get IBC memo string from MASP transaction for receiving +pub async fn convert_masp_tx_to_ibc_memo_data( + context: &impl Namada, + transaction: &MaspTransaction, + shielding_fee_payer: common::PublicKey, + shielding_fee_token: Address, +) -> std::result::Result { + let mut wallet = context.wallet_lock().write().await; + let secret_key = wallet + .find_key_by_pk(&shielding_fee_payer, None) + .map_err(|err| { + Error::Other(format!( + "Unable to load the keypair from the wallet for public key \ + {}. Failed with: {}", + shielding_fee_payer, err + )) + })?; + + let target = MaspTxId::from(transaction.txid()); + let authorization = Authorization::new( + vec![target.into()], + BTreeMap::from([(0, secret_key)]), + None, + ); + Ok(IbcShieldingData { + masp_tx: transaction.clone(), + shielding_fee_authorization: authorization, + shielding_fee_token, + }) +} diff --git a/crates/shielded_token/src/lib.rs b/crates/shielded_token/src/lib.rs index cf9f837961e..d86c6fad365 100644 --- a/crates/shielded_token/src/lib.rs +++ b/crates/shielded_token/src/lib.rs @@ -78,4 +78,4 @@ impl Default for ShieldedParams { locked_amount_target: 10_000_u64, } } -} +} \ No newline at end of file diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index a77cb77f91c..04eefcec95b 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -17,7 +17,10 @@ use namada_core::address::{self, Address, PGF}; use namada_core::arith::{CheckedAdd, CheckedSub, checked}; use namada_core::booleans::BoolResultUnitExt; use namada_core::collections::HashSet; -use namada_core::masp::{MaspEpoch, TAddrData, addr_taddr, encode_asset_type}; +use namada_core::key::common; +use namada_core::masp::{ + MaspEpoch, MaspTxId, TAddrData, addr_taddr, encode_asset_type, +}; use namada_core::storage::Key; use namada_core::token; use namada_core::token::{Amount, DenominatedAmount, MaspDigitPos}; @@ -26,7 +29,8 @@ use namada_state::{ ConversionState, OptionExt, ReadConversionState, ResultExt, }; use namada_systems::{governance, ibc, parameters, trans_token}; -use namada_tx::BatchedTxRef; +use namada_systems::ibc::IbcShieldingData; +use namada_tx::{BatchedTxRef, Signer}; use namada_vp_env::{Error, Result, VpEnv}; use crate::storage_key::{ @@ -37,6 +41,57 @@ use crate::storage_key::{ }; use crate::validation::verify_shielded_tx; +/// Determines how to validate the MASP shielding fee +pub enum MaspFeeType { + /// MASP shielding from Namada transparent address + Normal(Transaction), + /// MASP shielding over IBC + IBC(IbcShieldingData), +} + +impl MaspFeeType { + fn masp_tx(&self) -> &Transaction { + match self { + MaspFeeType::Normal(tx) => tx, + MaspFeeType::IBC(IbcShieldingData { masp_tx, .. }) => masp_tx, + } + } + + fn tx_id(&self) -> MaspTxId { + self.masp_tx().txid().into() + } + + fn get_masp_fee_data<'a>( + &'a self, + batched_tx: &'a BatchedTxRef<'_>, + ) -> Option<(&'a common::PublicKey, &'a Address)> { + let masp_tx_id = self.tx_id(); + match self { + MaspFeeType::Normal(_) => { + batched_tx.tx.get_shielding_fee_section(&masp_tx_id) + } + MaspFeeType::IBC(data) => { + // Check the authorization is for the given MASP tx + if data + .shielding_fee_authorization + .targets + .first() + .copied() + .map(MaspTxId::from) + .is_none_or(|id| id != masp_tx_id) + { + return None; + } + let signer = match &data.shielding_fee_authorization.signer { + Signer::Address(_) => None, + Signer::PubKeys(pks) => pks.first(), + }?; + Some((signer, &data.shielding_fee_token)) + } + } + } +} + /// MASP VP pub struct MaspVp<'ctx, CTX, Params, Gov, Ibc, TransToken, Transfer> { /// Generic types for DI @@ -84,7 +139,9 @@ where + ReadConversionState, Params: parameters::Read<>::Pre>, Gov: governance::Read<>::Pre>, - Ibc: ibc::Read<>::Post>, + Ibc: ibc::Read< + >::Post, + >, TransToken: trans_token::Keys + trans_token::Read<>::Pre>, Transfer: BorshDeserialize, @@ -421,18 +478,16 @@ where /// `verifiers` set. fn check_masp_fees( ctx: &'ctx CTX, - shielded_tx: &Transaction, + shielded_tx: &MaspFeeType, batched_tx: &BatchedTxRef<'_>, - verifiers: &BTreeSet
, actions_authorizers: &mut HashSet<&Address>, ) -> Result<()> { - let masp_tx_id = shielded_tx.txid().into(); let has_trans_inputs = shielded_tx + .masp_tx() .transparent_bundle() - .map(|b| !b.vin.is_empty()) - .unwrap_or(false); + .is_some_and(|b| !b.vin.is_empty()); let (fee_authorizer, fee_token) = - batched_tx.tx.get_shielding_fee_section(&masp_tx_id).unzip(); + shielded_tx.get_masp_fee_data(batched_tx).unzip(); if has_trans_inputs { match fee_authorizer { None => { @@ -524,7 +579,7 @@ where let shielded_tx = if let Some(tx) = Ibc::try_extract_masp_tx_from_envelope::(&tx_data)? { - tx + MaspFeeType::IBC(tx) } else { let masp_section_ref = namada_tx::action::get_masp_section_ref(&actions) @@ -535,17 +590,19 @@ where ) })?; - batched_tx - .tx - .get_masp_section(&masp_section_ref) - .cloned() - .ok_or_else(|| { - Error::new_const("Missing MASP section in transaction") - })? + MaspFeeType::Normal( + batched_tx + .tx + .get_masp_section(&masp_section_ref) + .cloned() + .ok_or_else(|| { + Error::new_const("Missing MASP section in transaction") + })?, + ) }; if u64::from(ctx.get_block_height()?) - > u64::from(shielded_tx.expiry_height()) + > u64::from(shielded_tx.masp_tx().expiry_height()) { let error = Error::new_const("MASP transaction is expired"); tracing::debug!("{error}"); @@ -573,7 +630,7 @@ where .unwrap_or(&zero), &changed_balances.undated_pre, &changed_balances.undated_post, - &shielded_tx.sapling_value_balance(), + &shielded_tx.masp_tx().sapling_value_balance(), masp_epoch, &changed_balances.undated_tokens, conversion_state, @@ -590,15 +647,19 @@ where // nullifier is being revealed by the tx // 4. The transaction must correctly update the note commitment tree // in storage with the new output descriptions - Self::valid_spend_descriptions_anchor(ctx, &shielded_tx)?; - Self::valid_convert_descriptions_anchor(ctx, &shielded_tx)?; - Self::valid_nullifiers_reveal(ctx, keys_changed, &shielded_tx)?; - Self::valid_note_commitment_update(ctx, &shielded_tx)?; + Self::valid_spend_descriptions_anchor(ctx, shielded_tx.masp_tx())?; + Self::valid_convert_descriptions_anchor(ctx, shielded_tx.masp_tx())?; + Self::valid_nullifiers_reveal( + ctx, + keys_changed, + shielded_tx.masp_tx(), + )?; + Self::valid_note_commitment_update(ctx, shielded_tx.masp_tx())?; // Checks on the transparent bundle, if present let mut changed_bals_minus_txn = changed_balances.clone(); validate_transparent_bundle( - &shielded_tx, + shielded_tx.masp_tx(), &mut changed_bals_minus_txn, masp_epoch, conversion_state, @@ -647,7 +708,6 @@ where ctx, &shielded_tx, batched_tx, - verifiers, &mut actions_authorizers, )?; @@ -668,7 +728,9 @@ where // Transactions inside this Tx. We achieve this by not allowing // the IBC to be in the transparent output of any of the // Transaction(s). - if let Some(transp_bundle) = shielded_tx.transparent_bundle() { + if let Some(transp_bundle) = + shielded_tx.masp_tx().transparent_bundle() + { for vout in transp_bundle.vout.iter() { if let Some(TAddrData::Ibc(_)) = changed_bals_minus_txn.decoder.get(&vout.address) @@ -729,7 +791,7 @@ where } // Verify the proofs - verify_shielded_tx(&shielded_tx, |gas| ctx.charge_gas(gas)) + verify_shielded_tx(shielded_tx.masp_tx(), |gas| ctx.charge_gas(gas)) } } diff --git a/crates/systems/Cargo.toml b/crates/systems/Cargo.toml index 2c724f46abf..08be8577e8f 100644 --- a/crates/systems/Cargo.toml +++ b/crates/systems/Cargo.toml @@ -17,7 +17,9 @@ rust-version.workspace = true namada_core.workspace = true namada_events.workspace = true namada_storage.workspace = true +namada_tx.workspace = true +borsh.workspace = true [dev-dependencies] cargo_metadata.workspace = true lazy_static.workspace = true diff --git a/crates/systems/src/ibc.rs b/crates/systems/src/ibc.rs index b492513b9e4..e0a7dd374a3 100644 --- a/crates/systems/src/ibc.rs +++ b/crates/systems/src/ibc.rs @@ -1,21 +1,77 @@ //! IBC abstract interfaces use std::collections::{BTreeMap, BTreeSet}; - +use std::fmt; +use std::str::FromStr; use masp_primitives::transaction::TransparentAddress; use masp_primitives::transaction::components::ValueSum; use namada_core::address::Address; -use namada_core::borsh::BorshDeserialize; -use namada_core::masp::TAddrData; +use borsh::{BorshDeserialize, BorshSerialize}; +use namada_core::masp::{MaspTransaction, TAddrData}; use namada_core::{masp_primitives, storage, token}; +use namada_core::borsh::BorshSerializeExt; +use namada_core::bytes::HEXUPPER; +use namada_core::key::common; pub use namada_storage::Result; +use namada_tx::{Authorization, Signer}; + +/// Shielding data in IBC packet memo +#[derive(Debug, Clone, BorshDeserialize, BorshSerialize)] +pub struct IbcShieldingData { + /// The MASP transaction that does the shielding + pub masp_tx: MaspTransaction, + /// The account that will pay the shielding fee + pub shielding_fee_authorization: Authorization, + /// The token that the shielding fee will be paid in + pub shielding_fee_token: Address, +} + +impl IbcShieldingData { + /// Get the public key of the account that is paying the shielding fee + pub fn get_signer(&self) -> Option<&common::PublicKey> { + match &self.shielding_fee_authorization.signer { + Signer::Address(_) => None, + Signer::PubKeys(pks) => pks.first(), + } + } +} + +impl From<&IbcShieldingData> for String { + fn from(data: &IbcShieldingData) -> Self { + HEXUPPER.encode(&data.serialize_to_vec()) + } +} + +impl From for String { + fn from(data: IbcShieldingData) -> Self { + (&data).into() + } +} + +impl fmt::Display for IbcShieldingData { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", String::from(self)) + } +} + +impl FromStr for IbcShieldingData { + type Err = String; + + fn from_str(s: &str) -> std::result::Result { + let bytes = HEXUPPER + .decode(s.as_bytes()) + .map_err(|err| err.to_string())?; + IbcShieldingData::try_from_slice(&bytes).map_err(|err| err.to_string()) + } +} /// Abstract IBC storage read interface pub trait Read { + /// Extract MASP transaction from IBC envelope fn try_extract_masp_tx_from_envelope( tx_data: &[u8], - ) -> Result>; + ) -> Result>; /// Apply relevant IBC packets to the changed balances structure fn apply_ibc_packet( diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 0c113b7dfe0..f49490e47cb 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -2988,7 +2988,7 @@ fn transfer_from_cosmos( let chain_type = CosmosChainType::chain_type(test.net.chain_id.as_str()).unwrap(); let rpc = format!("tcp://127.0.0.1:{}", chain_type.get_rpc_port_number()); - // If the receiver is a pyament address we want to mask it to the more + // If the receiver is a payment address we want to mask it to the more // general MASP internal address to improve on privacy let receiver = match PaymentAddress::from_str(receiver.as_ref()) { Ok(_) => MASP.to_string(), diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index 834f1e0c8d2..34d26ff601a 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -6,6 +6,7 @@ use std::collections::{BTreeMap, BTreeSet}; use namada_core::address::PGF; use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; +use namada_core::key::common; use namada_core::masp::encode_asset_type; use namada_core::masp_primitives::transaction::Transaction; use namada_core::token::{DenominatedAmount, MaspDigitPos}; @@ -204,49 +205,18 @@ where "Transfer transaction does not debit all the expected accounts", )); } - for authorizer in masp_authorizers { env.push_action(Action::Masp(MaspAction::MaspAuthorizer(authorizer)))?; } if !vin_addresses.is_empty() { - if let Some((payer, token)) = - tx_data.tx.get_shielding_fee_section(&masp_section_ref) + if let Some((fee_payer, fee_token)) = tx_data + .tx + .get_shielding_fee_section(&shielded.txid().into()) { + apply_masp_fees(env, fee_payer, fee_token)?; env.push_action(Action::Masp(MaspAction::MaspAuthorizer( - Address::from(payer), + Address::from(fee_payer), )))?; - // transfer the shielding fee - let amount: DenominatedAmount = env - .get_masp_shielding_fee_amount(token) - .expect("Failed to read storage") - .ok_or_else(|| { - Error::AllocMessage(format!( - "The token {token} cannot be used to pay shielding \ - fees" - )) - })?; - let sources = BTreeMap::from([( - Account { - owner: Address::from(payer), - token: token.clone(), - }, - amount, - )]); - let targets = BTreeMap::from([( - Account { - owner: PGF, - token: token.clone(), - }, - amount, - )]); - apply_transparent_transfers( - env, - TransparentTransfersRef { - sources: &sources, - targets: &targets, - }, - Cow::Borrowed("shielding-fee-from-wasm"), - )?; } else { return Err(Error::SimpleMessage( "A fee payer was not provided for a shielding transaction", @@ -257,6 +227,49 @@ where Ok(()) } +/// Transfer the fee for MASP shielding to PGF +pub fn apply_masp_fees( + env: &mut ENV, + fee_payer: &common::PublicKey, + fee_token: &Address, +) -> Result<()> +where + ENV: TxEnv + EmitEvents + action::Write, +{ + // transfer the shielding fee + let amount: DenominatedAmount = env + .get_masp_shielding_fee_amount(fee_token) + .expect("Failed to read storage") + .ok_or_else(|| { + Error::AllocMessage(format!( + "The token {fee_token} cannot be used to pay shielding fees" + )) + })?; + let sources = BTreeMap::from([( + Account { + owner: Address::from(fee_payer), + token: fee_token.clone(), + }, + amount, + )]); + let targets = BTreeMap::from([( + Account { + owner: PGF, + token: fee_token.clone(), + }, + amount, + )]); + apply_transparent_transfers( + env, + TransparentTransfersRef { + sources: &sources, + targets: &targets, + }, + Cow::Borrowed("shielding-fee-from-wasm"), + )?; + Ok(()) +} + #[cfg(test)] #[allow(clippy::arithmetic_side_effects, clippy::disallowed_types)] mod test { diff --git a/crates/tx/src/action.rs b/crates/tx/src/action.rs index 4b477a2d2ce..8b5b51bee62 100644 --- a/crates/tx/src/action.rs +++ b/crates/tx/src/action.rs @@ -10,6 +10,7 @@ use std::fmt; use namada_core::address::Address; use namada_core::borsh::{BorshDeserialize, BorshSerialize}; +use namada_core::key::common; use namada_core::masp::MaspTxId; use namada_core::storage::KeySeg; use namada_core::{address, storage}; @@ -17,19 +18,48 @@ use namada_core::{address, storage}; pub use crate::data::pos::{ Bond, ClaimRewards, Redelegation, Unbond, Withdraw, }; +use crate::{Authorization, Signer}; /// Actions applied from txs. pub type Actions = Vec; /// An action applied from a tx. #[allow(missing_docs)] +#[allow(clippy::large_enum_variant)] #[derive(Clone, Debug, BorshDeserialize, BorshSerialize, PartialEq)] pub enum Action { Pos(PosAction), Gov(GovAction), Pgf(PgfAction), Masp(MaspAction), - IbcShielding, + IbcShielding(IbcShieldingAction), +} + +#[allow(missing_docs)] +#[derive(Clone, Debug, BorshDeserialize, BorshSerialize, PartialEq)] +pub struct IbcShieldingAction { + /// The account that will pay the shielding fee + pub shielding_fee_authorization: Authorization, + /// The token that the shielding fee will be paid in + pub shielding_fee_token: Address, +} + +impl IbcShieldingAction { + /// Creat a new [`IbcShieldingAction`] + pub fn new(auth: Authorization, token: Address) -> Self { + Self { + shielding_fee_authorization: auth, + shielding_fee_token: token, + } + } + + /// Get the public key of the account that is paying the shielding fee + pub fn get_signer(&self) -> Option<&common::PublicKey> { + match &self.shielding_fee_authorization.signer { + Signer::Address(_) => None, + Signer::PubKeys(pks) => pks.first(), + } + } } /// PoS tx actions. @@ -155,5 +185,5 @@ pub fn is_ibc_shielding_transfer( Ok(reader .read_actions()? .iter() - .any(|action| matches!(action, Action::IbcShielding))) + .any(|action| matches!(action, Action::IbcShielding(_)))) } diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index 1827f10fb4c..5ee5394aeb4 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -5,7 +5,7 @@ use namada_core::masp_primitives::transaction::Transaction; use namada_token::TransparentTransfersRef; #[cfg(any(test, feature = "testing"))] pub use namada_token::testing; -pub use namada_token::tx::apply_shielded_transfer; +pub use namada_token::tx::{apply_masp_fees, apply_shielded_transfer}; pub use namada_token::{ Amount, DenominatedAmount, Denomination, MaspDigitPos, Store, Transfer, storage_key, utils, validate_transfer_in_out, diff --git a/crates/vm/src/host_env.rs b/crates/vm/src/host_env.rs index 6a09231527f..c0af37c29b0 100644 --- a/crates/vm/src/host_env.rs +++ b/crates/vm/src/host_env.rs @@ -35,7 +35,7 @@ use namada_token::storage_key::{ is_any_token_parameter_key, }; use namada_tx::data::{InnerTxId, TxSentinel}; -use namada_tx::{BatchedTx, BatchedTxRef, Tx, TxCommitments}; +use namada_tx::{Authorization, BatchedTx, BatchedTxRef, Tx, TxCommitments}; use namada_vp::vp_host_fns; use thiserror::Error; @@ -2010,6 +2010,63 @@ where } } +/// Verify a signature over an [`Authorization`] in the host environment for +/// better performance +#[allow(clippy::too_many_arguments)] +pub fn vp_verify_signature( + env: &mut VpVmEnv, + public_keys_map_ptr: u64, + public_keys_map_len: u64, + auth_ptr: u64, + auth_len: u64, +) -> Result<()> +where + MEM: VmMemory, + D: 'static + DB + for<'iter> DBIter<'iter>, + H: 'static + StorageHasher, + EVAL: VpEvaluator, + CA: WasmCacheAccess, +{ + let (public_keys_map, gas) = env + .memory + .read_bytes(public_keys_map_ptr, public_keys_map_len.try_into()?) + .map_err(Into::into)?; + let gas_meter = env.ctx.gas_meter(); + vp_host_fns::add_gas(gas_meter, gas)?; + let public_keys_map: AccountPublicKeysMap = decode(public_keys_map)?; + + let (auth, gas) = env + .memory + .read_bytes(auth_ptr, auth_len.try_into()?) + .map_err(Into::into)?; + vp_host_fns::add_gas(gas_meter, gas)?; + let auth: Authorization = decode(auth)?; + + match auth.verify_signature( + &mut Default::default(), + &public_keys_map, + &None, + &mut || { + gas_meter + .borrow_mut() + .consume(gas::VERIFY_TX_SIG_GAS.into()) + }, + ) { + Ok(_) => Ok(()), + Err(err) => match err { + namada_tx::VerifySigError::Gas(inner) => { + Err(vp_host_fns::RuntimeError::OutOfGas(inner)) + .into_storage_result() + } + namada_tx::VerifySigError::InvalidSectionSignature(inner) => { + Err(vp_host_fns::RuntimeError::InvalidSectionSignature(inner)) + .into_storage_result() + } + err => Err(Error::new_alloc(err.to_string())), + }, + } +} + /// Log a string from exposed to the wasm VM Tx environment. The message will be /// printed at the [`tracing::Level::INFO`]. This function is for development /// only. diff --git a/crates/vm/src/wasm/host_env.rs b/crates/vm/src/wasm/host_env.rs index 4518096d540..e48dc4f8153 100644 --- a/crates/vm/src/wasm/host_env.rs +++ b/crates/vm/src/wasm/host_env.rs @@ -100,6 +100,7 @@ where "namada_vp_read_temp" => Function::new_typed_with_env(wasm_store, &env, wrap_vp::_2(host_env::vp_read_temp)), "namada_vp_result_buffer" => Function::new_typed_with_env(wasm_store, &env, wrap_vp::_1(host_env::vp_result_buffer)), "namada_vp_verify_tx_section_signature" => Function::new_typed_with_env(wasm_store, &env, wrap_vp::_7(host_env::vp_verify_tx_section_signature)), + "namada_vp_verify_signature" => Function::new_typed_with_env(wasm_store, &env, wrap_vp::_4(host_env::vp_verify_signature)), "namada_vp_yield_value" => Function::new_typed_with_env(wasm_store, &env, wrap_vp::_2(host_env::vp_yield_value)), }, } diff --git a/crates/vm_env/src/lib.rs b/crates/vm_env/src/lib.rs index 2dcf341bd18..429d6727c24 100644 --- a/crates/vm_env/src/lib.rs +++ b/crates/vm_env/src/lib.rs @@ -270,6 +270,14 @@ pub mod vp { threshold: u8, ); + /// Verify the signatures of an authorization + pub fn namada_vp_verify_signature( + public_key_ptr: u64, + public_key_len: u64, + auth_ptr: u64, + auth_len: u64, + ); + /// Evaluate a validity-predicate pub fn namada_vp_eval( vp_code_hash_ptr: u64, diff --git a/crates/vp_prelude/src/lib.rs b/crates/vp_prelude/src/lib.rs index cbfb25ee74e..b1cf735def1 100644 --- a/crates/vp_prelude/src/lib.rs +++ b/crates/vp_prelude/src/lib.rs @@ -26,6 +26,7 @@ use std::marker::PhantomData; use std::str::FromStr; use chain::ChainId; +use namada_account::AccountPublicKeysMap; pub use namada_core::address::Address; pub use namada_core::borsh::{ BorshDeserialize, BorshSerialize, BorshSerializeExt, @@ -47,6 +48,8 @@ pub use namada_macros::validity_predicate; pub use namada_storage::{ Error, OptionExt, ResultExt, StorageRead, iter_prefix, iter_prefix_bytes, }; +use namada_tx::Signer; +use namada_tx::action::IbcShieldingAction; pub use namada_tx::{BatchedTx, Section, Tx}; use namada_vm_env::vp::*; use namada_vm_env::{read_from_buffer, read_key_val_bytes_from_buffer}; @@ -170,6 +173,58 @@ impl VerifySigGadget { } Ok(()) } + + /// Verify the shielding fee authorization from an IBC shielding tx + /// if the predicate returns true. + #[inline(always)] + pub fn verify_masp_fee_signatures_when bool>( + &mut self, + predicate: F, + tx_data: &Tx, + cmt: &TxCommitments, + ibc_shielding_action: &IbcShieldingAction, + ) -> VpResult { + if predicate() && !self.has_validated_sig { + // First check that the memo section of this inner tx has not + // been tampered with + if cmt.memo_hash != namada_core::hash::Hash::zero() { + tx_data.get_section(&cmt.memo_hash).ok_or_else(|| { + VpError::Erased(format!( + "Memo section with hash {} is missing", + cmt.memo_hash + )) + })?; + } + let auth = ibc_shielding_action + .shielding_fee_authorization + .serialize_to_vec(); + let public_keys_index_map: AccountPublicKeysMap = + match &ibc_shielding_action.shielding_fee_authorization.signer { + Signer::Address(_) => { + return Err(VpError::Erased( + "Expected IBC shielding fee authorization to be \ + signed with public keys" + .to_string(), + )); + } + Signer::PubKeys(pks) => pks.iter().cloned().collect(), + }; + let public_keys_map = public_keys_index_map.serialize_to_vec(); + + // Then check the signature + unsafe { + namada_vp_verify_signature( + public_keys_map.as_ptr() as _, + public_keys_map.len() as _, + auth.as_ptr() as _, + auth.len() as _, + ); + } + self.has_validated_sig = true; + } + + Ok(()) + } } /// Format and log a string in a debug build. diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index da862097512..71edf497d61 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -4806,9 +4806,11 @@ dependencies = [ name = "namada_systems" version = "0.150.0" dependencies = [ + "borsh", "namada_core", "namada_events", "namada_storage", + "namada_tx", ] [[package]] diff --git a/wasm/tx_ibc/src/lib.rs b/wasm/tx_ibc/src/lib.rs index 82891522032..ac0a3aac0b3 100644 --- a/wasm/tx_ibc/src/lib.rs +++ b/wasm/tx_ibc/src/lib.rs @@ -48,8 +48,23 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { ctx.set_commitment_sentinel(); })?, ) + } else if let Some(shielded) = data.shielded { + let signer = shielded.get_signer().ok_or_err_msg( + "Unable to find a public key signing a shielding IBC tx.", + )?; + token::apply_masp_fees(ctx, signer, &shielded.shielding_fee_token) + .wrap_err( + "Encountered error while paying the shielding fee for IBC", + )?; + ctx.push_action(Action::IbcShielding( + action::IbcShieldingAction::new( + shielded.shielding_fee_authorization, + shielded.shielding_fee_token, + ), + ))?; + Some(shielded.masp_tx) } else { - data.shielded + None }; if let Some(shielded) = shielded { token::utils::handle_masp_tx(ctx, &shielded) @@ -60,8 +75,6 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { ctx.push_action(Action::Masp(MaspAction::MaspSectionRef( masp_section_ref, )))?; - } else { - ctx.push_action(Action::IbcShielding)?; } token::update_undated_balances(ctx, &shielded, token_addrs)?; } diff --git a/wasm/vp_implicit/src/lib.rs b/wasm/vp_implicit/src/lib.rs index 8ad708c6de6..1947272ae6c 100644 --- a/wasm/vp_implicit/src/lib.rs +++ b/wasm/vp_implicit/src/lib.rs @@ -114,7 +114,20 @@ fn validate_tx( &addr, )?, Action::Masp(MaspAction::MaspSectionRef(_)) => (), - Action::IbcShielding => (), + Action::IbcShielding(ibc_shielding_action) => gadget + .verify_masp_fee_signatures_when( + || { + if let Some(signer) = ibc_shielding_action.get_signer() + { + Address::from(signer) == addr + } else { + false + } + }, + &tx, + cmt, + &ibc_shielding_action, + )?, } } diff --git a/wasm/vp_user/src/lib.rs b/wasm/vp_user/src/lib.rs index d53ea783044..2bf553c9b71 100644 --- a/wasm/vp_user/src/lib.rs +++ b/wasm/vp_user/src/lib.rs @@ -113,7 +113,20 @@ fn validate_tx( &addr, )?, Action::Masp(MaspAction::MaspSectionRef(_)) => (), - Action::IbcShielding => (), + Action::IbcShielding(ibc_shielding_action) => gadget + .verify_masp_fee_signatures_when( + || { + if let Some(signer) = ibc_shielding_action.get_signer() + { + Address::from(signer) == addr + } else { + false + } + }, + &tx, + cmt, + &ibc_shielding_action, + )?, } } diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index f687c172ed9..9358f8c40bd 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2754,9 +2754,11 @@ dependencies = [ name = "namada_systems" version = "0.150.0" dependencies = [ + "borsh", "namada_core", "namada_events", "namada_storage", + "namada_tx", ] [[package]] From 6754f065ed9a73c9010a21e4ee072a36e00874f5 Mon Sep 17 00:00:00 2001 From: satan Date: Fri, 25 Jul 2025 12:12:53 +0200 Subject: [PATCH 15/24] Fix bug in finding authorizers masp vp must check from the actions --- crates/shielded_token/src/vp.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 04eefcec95b..20b7629e368 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -1,5 +1,6 @@ //! MASP native VP +use std::borrow::Cow; use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; @@ -480,7 +481,7 @@ where ctx: &'ctx CTX, shielded_tx: &MaspFeeType, batched_tx: &BatchedTxRef<'_>, - actions_authorizers: &mut HashSet<&Address>, + actions_authorizers: &mut HashSet>, ) -> Result<()> { let has_trans_inputs = shielded_tx .masp_tx() @@ -690,14 +691,17 @@ where } } - let mut actions_authorizers: HashSet<&Address> = actions + let mut actions_authorizers: HashSet> = actions .iter() .filter_map(|action| { if let namada_tx::action::Action::Masp( namada_tx::action::MaspAction::MaspAuthorizer(addr), ) = action { - Some(addr) + Some(Cow::Borrowed(addr)) + } else if let namada_tx::action::Action::IbcShielding ( data ) = action + { + data.get_signer().map(|s| Cow::Owned(Address::from(s))) } else { None } From 41cda8f139a1752c15fda698cbe05b886c77b010 Mon Sep 17 00:00:00 2001 From: satan Date: Fri, 25 Jul 2025 16:09:10 +0200 Subject: [PATCH 16/24] Formatting --- crates/ibc/src/msg.rs | 4 ++-- crates/shielded_token/src/lib.rs | 2 +- crates/shielded_token/src/vp.rs | 9 ++++----- crates/systems/src/ibc.rs | 8 ++++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/ibc/src/msg.rs b/crates/ibc/src/msg.rs index ed8baf095db..d26292ef7e0 100644 --- a/crates/ibc/src/msg.rs +++ b/crates/ibc/src/msg.rs @@ -15,9 +15,10 @@ use ibc::core::channel::types::packet::Packet; use ibc::core::handler::types::msgs::MsgEnvelope; use ibc::core::host::types::identifiers::PortId; use ibc::primitives::proto::Protobuf; -use serde::{Deserialize, Serialize}; use namada_core::string_encoding::StringEncoded; use namada_systems::ibc::IbcShieldingData; +use serde::{Deserialize, Serialize}; + use crate::trace; trait Sealed {} @@ -231,7 +232,6 @@ impl BorshSchema for MsgNftTransfer { } } - /// Extract MASP transaction from IBC envelope pub fn extract_masp_tx_from_envelope( envelope: &MsgEnvelope, diff --git a/crates/shielded_token/src/lib.rs b/crates/shielded_token/src/lib.rs index d86c6fad365..cf9f837961e 100644 --- a/crates/shielded_token/src/lib.rs +++ b/crates/shielded_token/src/lib.rs @@ -78,4 +78,4 @@ impl Default for ShieldedParams { locked_amount_target: 10_000_u64, } } -} \ No newline at end of file +} diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 20b7629e368..efc7b7a4fca 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -29,8 +29,8 @@ use namada_core::uint::I320; use namada_state::{ ConversionState, OptionExt, ReadConversionState, ResultExt, }; -use namada_systems::{governance, ibc, parameters, trans_token}; use namada_systems::ibc::IbcShieldingData; +use namada_systems::{governance, ibc, parameters, trans_token}; use namada_tx::{BatchedTxRef, Signer}; use namada_vp_env::{Error, Result, VpEnv}; @@ -140,9 +140,7 @@ where + ReadConversionState, Params: parameters::Read<>::Pre>, Gov: governance::Read<>::Pre>, - Ibc: ibc::Read< - >::Post, - >, + Ibc: ibc::Read<>::Post>, TransToken: trans_token::Keys + trans_token::Read<>::Pre>, Transfer: BorshDeserialize, @@ -699,7 +697,8 @@ where ) = action { Some(Cow::Borrowed(addr)) - } else if let namada_tx::action::Action::IbcShielding ( data ) = action + } else if let namada_tx::action::Action::IbcShielding(data) = + action { data.get_signer().map(|s| Cow::Owned(Address::from(s))) } else { diff --git a/crates/systems/src/ibc.rs b/crates/systems/src/ibc.rs index e0a7dd374a3..8d3b814d42c 100644 --- a/crates/systems/src/ibc.rs +++ b/crates/systems/src/ibc.rs @@ -3,15 +3,16 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt; use std::str::FromStr; + +use borsh::{BorshDeserialize, BorshSerialize}; use masp_primitives::transaction::TransparentAddress; use masp_primitives::transaction::components::ValueSum; use namada_core::address::Address; -use borsh::{BorshDeserialize, BorshSerialize}; -use namada_core::masp::{MaspTransaction, TAddrData}; -use namada_core::{masp_primitives, storage, token}; use namada_core::borsh::BorshSerializeExt; use namada_core::bytes::HEXUPPER; use namada_core::key::common; +use namada_core::masp::{MaspTransaction, TAddrData}; +use namada_core::{masp_primitives, storage, token}; pub use namada_storage::Result; use namada_tx::{Authorization, Signer}; @@ -67,7 +68,6 @@ impl FromStr for IbcShieldingData { /// Abstract IBC storage read interface pub trait Read { - /// Extract MASP transaction from IBC envelope fn try_extract_masp_tx_from_envelope( tx_data: &[u8], From ba86de7e94637ec16680f5c2fd7f14bcdccca14f Mon Sep 17 00:00:00 2001 From: satan Date: Fri, 25 Jul 2025 16:11:00 +0200 Subject: [PATCH 17/24] Formatting --- crates/tests/src/integration/masp.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 1503f2a307a..92cc8b7750f 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -19,10 +19,8 @@ use namada_core::dec::Dec; use namada_core::masp::{MaspTxId, Precision, TokenMap, encode_asset_type}; use namada_node::shell::ResultCode; use namada_node::shell::testing::client::{ - dummy_args, run, sign_tx, submit_custom, + dummy_args, run, run, sign_tx, submit_custom, }; - -use namada_node::shell::testing::client::run; use namada_node::shell::testing::node::NodeResults; use namada_node::shell::testing::utils::{Bin, CapturedOutput}; use namada_sdk::account::AccountPublicKeysMap; From e331c610f09ae7dc0448ed4a1f92ce2d4dcd0c3c Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 10:10:50 +0200 Subject: [PATCH 18/24] Fixed an import mistake --- crates/tests/src/integration/masp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 92cc8b7750f..6512f748b5b 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -19,7 +19,7 @@ use namada_core::dec::Dec; use namada_core::masp::{MaspTxId, Precision, TokenMap, encode_asset_type}; use namada_node::shell::ResultCode; use namada_node::shell::testing::client::{ - dummy_args, run, run, sign_tx, submit_custom, + dummy_args, run, sign_tx, submit_custom, }; use namada_node::shell::testing::node::NodeResults; use namada_node::shell::testing::utils::{Bin, CapturedOutput}; From 6b49a2d0f4981fb712b5e32172403c1048b2c498 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 11:04:10 +0200 Subject: [PATCH 19/24] Fixed test vp env --- crates/tests/src/integration/masp.rs | 4 ++++ crates/tests/src/vm_host_env/vp.rs | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 6512f748b5b..70865b4e213 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -5547,6 +5547,10 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), diff --git a/crates/tests/src/vm_host_env/vp.rs b/crates/tests/src/vm_host_env/vp.rs index 58a59e7e406..8e2ee4f2d7f 100644 --- a/crates/tests/src/vm_host_env/vp.rs +++ b/crates/tests/src/vm_host_env/vp.rs @@ -352,6 +352,12 @@ mod native_vp_host_env { signer_len: u64, threshold: u8, )); + native_host_fn!(namada_vp_verify_signature( + public_key_ptr: u64, + public_key_len: u64, + auth_ptr: u64, + auth_len: u64, + )); native_host_fn!(vp_charge_gas(used_gas: u64)); native_host_fn!(vp_yield_value(buf_ptr: u64, buf_len: u64)); } From 528aaaf5592e529bcf55207ae9d7fd89b6eacb24 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 11:18:08 +0200 Subject: [PATCH 20/24] Added balance check to shielding fee payer when call gen_ibc_transfer --- crates/sdk/src/tx.rs | 8 +++++++- crates/tests/src/vm_host_env/vp.rs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 75c348f6b89..8e9866839ee 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -4678,7 +4678,13 @@ pub async fn convert_masp_tx_to_ibc_memo_data( shielding_fee_payer, err )) })?; - + let _ = validate_shielding_fee( + context, + None, + &shielding_fee_payer, + &shielding_fee_token, + false + )?; let target = MaspTxId::from(transaction.txid()); let authorization = Authorization::new( vec![target.into()], diff --git a/crates/tests/src/vm_host_env/vp.rs b/crates/tests/src/vm_host_env/vp.rs index 8e2ee4f2d7f..9a30c0580c9 100644 --- a/crates/tests/src/vm_host_env/vp.rs +++ b/crates/tests/src/vm_host_env/vp.rs @@ -352,7 +352,7 @@ mod native_vp_host_env { signer_len: u64, threshold: u8, )); - native_host_fn!(namada_vp_verify_signature( + native_host_fn!(vp_verify_signature( public_key_ptr: u64, public_key_len: u64, auth_ptr: u64, From 05f285e0f1c3fe06a1723f88d458cc3005391522 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 11:24:44 +0200 Subject: [PATCH 21/24] Forgot a goddamn await ofc --- crates/sdk/src/tx.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 8e9866839ee..4747f55cc9f 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -4684,7 +4684,7 @@ pub async fn convert_masp_tx_to_ibc_memo_data( &shielding_fee_payer, &shielding_fee_token, false - )?; + ).await?; let target = MaspTxId::from(transaction.txid()); let authorization = Authorization::new( vec![target.into()], From f39cea78d2f2e13a218a89c7b5cf2f50886b821f Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 11:41:20 +0200 Subject: [PATCH 22/24] Formatting --- crates/sdk/src/tx.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 4747f55cc9f..f79799865ba 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -4683,8 +4683,9 @@ pub async fn convert_masp_tx_to_ibc_memo_data( None, &shielding_fee_payer, &shielding_fee_token, - false - ).await?; + false, + ) + .await?; let target = MaspTxId::from(transaction.txid()); let authorization = Authorization::new( vec![target.into()], From fa08fed94457ebca94a9b8eaaedce7e80af508d2 Mon Sep 17 00:00:00 2001 From: satan Date: Mon, 28 Jul 2025 15:57:20 +0200 Subject: [PATCH 23/24] Some light renaming in anticipation of unshielding fees --- .../4276-add-shielding-fee-section.md | 2 +- .../src/config/genesis/transactions.rs | 2 +- crates/node/src/bench_utils.rs | 2 +- crates/sdk/src/signing.rs | 24 +++---- crates/sdk/src/tx.rs | 65 +++++++++++-------- crates/shielded_token/src/vp.rs | 19 +++--- crates/tests/src/integration/masp.rs | 30 +++++---- crates/token/src/tx.rs | 36 +++++----- crates/tx/src/section.rs | 6 +- crates/tx/src/types.rs | 10 +-- crates/tx_prelude/src/token.rs | 2 +- crates/vp_prelude/src/lib.rs | 2 +- wasm/tx_ibc/src/lib.rs | 11 ++-- wasm/vp_implicit/src/lib.rs | 2 +- wasm/vp_user/src/lib.rs | 2 +- 15 files changed, 121 insertions(+), 94 deletions(-) diff --git a/.changelog/unreleased/features/4276-add-shielding-fee-section.md b/.changelog/unreleased/features/4276-add-shielding-fee-section.md index f21904c1476..d5c96321bf0 100644 --- a/.changelog/unreleased/features/4276-add-shielding-fee-section.md +++ b/.changelog/unreleased/features/4276-add-shielding-fee-section.md @@ -1,2 +1,2 @@ -- Adds a shielding fee sectio to txs. This forces a fee to paid when shielding +- Adds a shielding fee section to txs. This forces a fee to paid when shielding to the masp ([\#4276](https://github.com/anoma/namada/issues/4276)) \ No newline at end of file diff --git a/crates/apps_lib/src/config/genesis/transactions.rs b/crates/apps_lib/src/config/genesis/transactions.rs index b3b46374f7e..fdfc295a2d5 100644 --- a/crates/apps_lib/src/config/genesis/transactions.rs +++ b/crates/apps_lib/src/config/genesis/transactions.rs @@ -770,7 +770,7 @@ impl Signed { public_keys: pks.clone(), threshold, fee_payer: Either::Left((genesis_fee_payer_pk(), false)), - shielding_fee_payer: None, + masp_sus_fee_payer: None, shielded_hash: None, signatures: vec![], }; diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 6a115b72e4e..21c3bc454f4 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1331,7 +1331,7 @@ impl BenchShieldedCtx { ) .unwrap(), Some(shielded), - Some(vec![Section::ShieldingFee { + Some(vec![Section::MaspSustainabilityFee { payer: shielding_fee_key, token: native_token, cmt: shielded_section_hash, diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 322f8ab1d10..73b11045ab4 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -80,8 +80,8 @@ pub struct SigningTxData { pub fee_payer: either::Either<(common::PublicKey, bool), Vec>, /// ID of the Transaction needing signing pub shielded_hash: Option, - /// The key of the account paying for shielding fees - pub shielding_fee_payer: Option, + /// The key of the account paying for MASP sustainability fees + pub masp_sus_fee_payer: Option, /// List of serialized signatures to attach to the transaction pub signatures: Vec>, } @@ -290,15 +290,14 @@ where } } - if let Some(shielding_fee_payer) = signing_data.shielding_fee_payer.as_ref() - { + if let Some(masp_sus_fee_payer) = signing_data.masp_sus_fee_payer.as_ref() { let mut wallet = wallet.write().await; // If the secret key is not found, continue because the // hardware wallet may still be able to sign this if let Ok(secret_key) = - find_key_by_pk(&mut wallet, args, shielding_fee_payer) + find_key_by_pk(&mut wallet, args, masp_sus_fee_payer) { - used_pubkeys.insert(shielding_fee_payer.clone()); + used_pubkeys.insert(masp_sus_fee_payer.clone()); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], (0..).zip(vec![secret_key]).collect(), @@ -329,8 +328,9 @@ where } } } - // try to sign the shielding fee with the hardware wallet if necessary - if let Some(payer) = signing_data.shielding_fee_payer { + // try to sign the MASP sustainability fee with the hardware wallet if + // necessary + if let Some(payer) = signing_data.masp_sus_fee_payer { if !used_pubkeys.contains(&payer) { if let Ok(ntx) = sign( tx.clone(), @@ -416,7 +416,7 @@ pub async fn aux_signing_data( owner: Option
, default_signer: Option
, extra_public_keys: Vec, - shielding_fee_payer: Option, + masp_sus_fee_payer: Option, is_shielded_source: bool, signatures: Vec>, wrapper_signature: Option>, @@ -480,7 +480,7 @@ pub async fn aux_signing_data( threshold, account_public_keys_map, fee_payer, - shielding_fee_payer, + masp_sus_fee_payer, shielded_hash: None, signatures, }) @@ -2622,7 +2622,7 @@ mod test_signing { account_public_keys_map: Some(Default::default()), fee_payer: either::Either::Left((public_key_fee.clone(), false)), shielded_hash: None, - shielding_fee_payer: None, + masp_sus_fee_payer: None, signatures: vec![], }; @@ -2660,7 +2660,7 @@ mod test_signing { account_public_keys_map: Some(Default::default()), fee_payer: either::Left((public_key.clone(), false)), shielded_hash: None, - shielding_fee_payer: None, + masp_sus_fee_payer: None, signatures: vec![], }; sign_tx( diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index f79799865ba..7c15159611b 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2774,12 +2774,13 @@ pub async fn build_ibc_transfer( let shielding_fee_payer = if let Some(data) = &args.ibc_shielding_data { - validate_shielding_fee( + validate_masp_sus_fee( context, Some(&mut updated_balance), data.get_signer() .expect("A MASP fee payer should have been provided"), &data.shielding_fee_token, + true, args.tx.force, ) .await? @@ -2931,7 +2932,7 @@ pub async fn build_ibc_transfer( tx.add_memo(memo); } if let Some(data) = &args.ibc_shielding_data { - tx.add_section(Section::ShieldingFee { + tx.add_section(Section::MaspSustainabilityFee { payer: data .get_signer() .cloned() @@ -3050,38 +3051,46 @@ pub async fn build_ibc_transfer( Ok((tx, signing_data, shielded_tx_epoch)) } -/// Check if a shielding fee payer is necessary. If so, +/// Check if a MASP sustainability fee payer is necessary. If so, /// guarantee one is provided and has sufficient balance. /// If it is the same account as the provided `updated_balance` /// arg, fold the change into that struct. Otherwise, return /// a new updated balance. -pub async fn validate_shielding_fee( +pub async fn validate_masp_sus_fee( context: &N, updated_balance: Option<&mut TxSourcePostBalance>, - shielding_fee_payer: &common::PublicKey, - shielding_fee_token: &Address, + masp_sus_fee_payer: &common::PublicKey, + masp_sus_fee_token: &Address, + shielding: bool, force: bool, ) -> Result> { - let payer = Address::from(shielding_fee_payer); + let payer = Address::from(masp_sus_fee_payer); - let shielding_fee = - rpc::get_shielding_fee_amount(context.client(), shielding_fee_token) + let masp_sus_fee = if shielding { + rpc::get_shielding_fee_amount(context.client(), masp_sus_fee_token) .await .map_err(|_| { Error::Other(format!( "The MASP shielding fee cannot be paid with token \ - {shielding_fee_token}" + {masp_sus_fee_token}" )) - })?; - // fee payer is the same as shielded fee payer and paying with the same - // token + })? + } else { + // TODO: Implement unshielding fees + return Err(Error::Other(format!( + "The MASP unshielding fee cannot be paid with token \ + {masp_sus_fee_token}" + ))); + }; + // fee payer is the same as MASP sustainability fee payer and paying with + // the same token let Some(updated_balance) = updated_balance else { return Some( check_balance_too_low_err( - shielding_fee_token, + masp_sus_fee_token, &payer, - shielding_fee.amount(), - CheckBalance::Query(balance_key(shielding_fee_token, &payer)), + masp_sus_fee.amount(), + CheckBalance::Query(balance_key(masp_sus_fee_token, &payer)), force, context, ) @@ -3089,18 +3098,18 @@ pub async fn validate_shielding_fee( ) .transpose(); }; - if updated_balance.token == *shielding_fee_token + if updated_balance.token == *masp_sus_fee_token && updated_balance.source == payer { updated_balance.post_balance = - checked!(updated_balance.post_balance - shielding_fee.amount()) + checked!(updated_balance.post_balance - masp_sus_fee.amount()) .map_err(|_| { Error::Other(format!( "{} does not have enough balance to pay for fees and \ - shielding fees. Short by {} {}", + MASP sustainability fees. Short by {} {}", updated_balance.source, checked!( - shielding_fee.amount() + masp_sus_fee.amount() - updated_balance.post_balance ) .unwrap(), @@ -3109,13 +3118,13 @@ pub async fn validate_shielding_fee( })?; Ok(None) } else { - // check if the shielding fee payer has enough balance + // check if the MASP sustainability fee payer has enough balance Some( check_balance_too_low_err( - shielding_fee_token, + masp_sus_fee_token, &payer, - shielding_fee.amount(), - CheckBalance::Query(balance_key(shielding_fee_token, &payer)), + masp_sus_fee.amount(), + CheckBalance::Query(balance_key(masp_sus_fee_token, &payer)), force, context, ) @@ -3597,11 +3606,12 @@ pub async fn build_shielding_transfer( .map(|(fee_amount, updated_balance)| { (fee_amount, Some(updated_balance)) })?; - let shielding_fee_balance = validate_shielding_fee( + let shielding_fee_balance = validate_masp_sus_fee( context, updated_balance.as_mut(), &args.shielding_fee_payer, &args.shielding_fee_token, + true, args.tx.force, ) .await?; @@ -3719,7 +3729,7 @@ pub async fn build_shielding_transfer( data.shielded_section_hash = Some(shielded_section_hash); signing_data.shielded_hash = Some(shielded_section_hash); - tx.add_section(Section::ShieldingFee { + tx.add_section(Section::MaspSustainabilityFee { payer: args.shielding_fee_payer.clone(), token: args.shielding_fee_token.clone(), cmt: shielded_section_hash, @@ -4678,11 +4688,12 @@ pub async fn convert_masp_tx_to_ibc_memo_data( shielding_fee_payer, err )) })?; - let _ = validate_shielding_fee( + let _ = validate_masp_sus_fee( context, None, &shielding_fee_payer, &shielding_fee_token, + true, false, ) .await?; diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 0526da75872..8d6246a2c62 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -42,9 +42,9 @@ use crate::storage_key::{ }; use crate::validation::verify_shielded_tx; -/// Determines how to validate the MASP shielding fee +/// Determines how to validate the MASP sustainability fee pub enum MaspFeeType { - /// MASP shielding from Namada transparent address + /// MASP tx from Namada transparent address Normal(Transaction), /// MASP shielding over IBC IBC(IbcShieldingData), @@ -62,14 +62,15 @@ impl MaspFeeType { self.masp_tx().txid().into() } - fn get_masp_fee_data<'a>( + /// Get the payer and token for the MASP sustainability fee if possible + fn get_masp_sus_fee_data<'a>( &'a self, batched_tx: &'a BatchedTxRef<'_>, ) -> Option<(&'a common::PublicKey, &'a Address)> { let masp_tx_id = self.tx_id(); match self { MaspFeeType::Normal(_) => { - batched_tx.tx.get_shielding_fee_section(&masp_tx_id) + batched_tx.tx.get_masp_sus_fee_section(&masp_tx_id) } MaspFeeType::IBC(data) => { // Check the authorization is for the given MASP tx @@ -472,10 +473,10 @@ where }) } - /// Check that the tx has a shielding fee section. The signer in that - /// section is checked to be in the `actions_authorizers` set and + /// Check that the tx has a MASP sustainability fee section. The signer in + /// that section is checked to be in the `actions_authorizers` set and /// `verifiers` set. - fn check_masp_fees( + fn check_masp_sus_fees( ctx: &'ctx CTX, shielded_tx: &MaspFeeType, batched_tx: &BatchedTxRef<'_>, @@ -486,7 +487,7 @@ where .transparent_bundle() .is_some_and(|b| !b.vin.is_empty()); let (fee_authorizer, fee_token) = - shielded_tx.get_masp_fee_data(batched_tx).unzip(); + shielded_tx.get_masp_sus_fee_data(batched_tx).unzip(); if has_trans_inputs { match fee_authorizer { None => { @@ -707,7 +708,7 @@ where }) .collect(); - Self::check_masp_fees( + Self::check_masp_sus_fees( ctx, &shielded_tx, batched_tx, diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 70865b4e213..5459fa5018c 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -1626,7 +1626,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { ), fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; @@ -1701,13 +1701,13 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { ), fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; let mut signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; signed .sections - .retain(|s| !matches!(s, Section::ShieldingFee { .. })); + .retain(|s| !matches!(s, Section::MaspSustainabilityFee { .. })); let captured = CapturedOutput::of(|| submit_custom(&node, signed, &dummy_args(&node))); assert!(captured.result.is_ok()); @@ -1765,11 +1765,11 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { ), fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; for sec in tx.sections.iter_mut() { - if let Section::ShieldingFee { cmt, .. } = sec { + if let Section::MaspSustainabilityFee { cmt, .. } = sec { *cmt = MaspTxId::from( masp_primitives::transaction::TxId::from_bytes([0u8; 32]), ); @@ -1835,7 +1835,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), shielded_hash: None, // this should be Bertha's key - shielding_fee_payer: Some(node.lookup_pk(ALBERT_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(ALBERT_KEY)?), signatures: vec![], }; let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; @@ -1886,7 +1886,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { let shielding_fee_section = tx .sections .iter() - .find(|s| matches!(s, Section::ShieldingFee { .. })) + .find(|s| matches!(s, Section::MaspSustainabilityFee { .. })) .expect("Test failed") .clone(); tx.add_section(shielding_fee_section); @@ -1900,7 +1900,7 @@ fn test_shielding_fee_protocol_checks() -> Result<()> { fee_payer: Either::Left((node.lookup_pk(ALBERT_KEY)?, false)), shielded_hash: None, // this should be Bertha's key - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; let signed = sign_tx(&node, tx, signing_data, &dummy_args(&node))?; @@ -5716,6 +5716,10 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { NAM, "--amount", "100", + "--shielding-fee-payer", + BERTHA_KEY, + "--shielding-fee-token", + NAM, "--ledger-address", validator_one_rpc, ]), @@ -7742,7 +7746,7 @@ fn identical_output_descriptions() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -8058,7 +8062,7 @@ fn masp_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -8324,7 +8328,7 @@ fn masp_atomic_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, - shielding_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), + masp_sus_fee_payer: Some(node.lookup_pk(BERTHA_KEY)?), signatures: vec![], }; @@ -8679,7 +8683,7 @@ fn masp_failing_atomic_batch() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, - shielding_fee_payer: None, + masp_sus_fee_payer: None, signatures: vec![], }; @@ -9620,7 +9624,7 @@ fn masp_events() -> Result<()> { account_public_keys_map: None, fee_payer: Either::Left((cooper_pk.clone(), false)), shielded_hash: None, - shielding_fee_payer: None, + masp_sus_fee_payer: None, signatures: vec![], }; diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index 34d26ff601a..4878ee73cb1 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -209,11 +209,10 @@ where env.push_action(Action::Masp(MaspAction::MaspAuthorizer(authorizer)))?; } if !vin_addresses.is_empty() { - if let Some((fee_payer, fee_token)) = tx_data - .tx - .get_shielding_fee_section(&shielded.txid().into()) + if let Some((fee_payer, fee_token)) = + tx_data.tx.get_masp_sus_fee_section(&shielded.txid().into()) { - apply_masp_fees(env, fee_payer, fee_token)?; + apply_masp_sus_fees(env, fee_payer, fee_token, true)?; env.push_action(Action::Masp(MaspAction::MaspAuthorizer( Address::from(fee_payer), )))?; @@ -227,24 +226,31 @@ where Ok(()) } -/// Transfer the fee for MASP shielding to PGF -pub fn apply_masp_fees( +/// Transfer the MASP sustainability fee to PGF +pub fn apply_masp_sus_fees( env: &mut ENV, fee_payer: &common::PublicKey, fee_token: &Address, + shielding: bool, ) -> Result<()> where ENV: TxEnv + EmitEvents + action::Write, { // transfer the shielding fee - let amount: DenominatedAmount = env - .get_masp_shielding_fee_amount(fee_token) - .expect("Failed to read storage") - .ok_or_else(|| { - Error::AllocMessage(format!( - "The token {fee_token} cannot be used to pay shielding fees" - )) - })?; + let amount: DenominatedAmount = if shielding { + env.get_masp_shielding_fee_amount(fee_token) + .expect("Failed to read storage") + .ok_or_else(|| { + Error::AllocMessage(format!( + "The token {fee_token} cannot be used to pay shielding \ + fees" + )) + })? + } else { + return Err(Error::AllocMessage(format!( + "The token {fee_token} cannot be used to pay unshielding fees" + ))); + }; let sources = BTreeMap::from([( Account { owner: Address::from(fee_payer), @@ -265,7 +271,7 @@ where sources: &sources, targets: &targets, }, - Cow::Borrowed("shielding-fee-from-wasm"), + Cow::Borrowed("masp_sustainability-fee-from-wasm"), )?; Ok(()) } diff --git a/crates/tx/src/section.rs b/crates/tx/src/section.rs index 7e37d2ce7ca..c811ce08904 100644 --- a/crates/tx/src/section.rs +++ b/crates/tx/src/section.rs @@ -61,8 +61,8 @@ pub enum Section { MaspBuilder(MaspBuilder), /// Wrap a header with a section for the purposes of computing hashes Header(Header), - /// The account paying the fee for a shielding transaction - ShieldingFee { + /// Data associated with paying the fee for an (un)shielding transaction + MaspSustainabilityFee { /// The key for the implicit account payer: common::PublicKey, /// The address of the token paying the fee @@ -158,7 +158,7 @@ impl Section { hasher.update(tx.serialize_to_vec()); hasher } - Self::ShieldingFee { payer, token, cmt } => { + Self::MaspSustainabilityFee { payer, token, cmt } => { hasher.update(payer.serialize_to_vec()); hasher.update(token.serialize_to_vec()); hasher.update(cmt.serialize_to_vec()); diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 2b9cf854dc1..27d9962f97f 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -299,14 +299,16 @@ impl Tx { }); } - /// Get the section containing the shielding fee payer - /// if it exists. There should be at most one - pub fn get_shielding_fee_section( + /// Get the section containing the MASP sustainability fee payer + /// if it exists. There should be at most one for a given MASP tx + pub fn get_masp_sus_fee_section( &self, masp_tx: &MaspTxId, ) -> Option<(&common::PublicKey, &Address)> { for section in &self.sections { - if let Section::ShieldingFee { payer, token, cmt } = section { + if let Section::MaspSustainabilityFee { payer, token, cmt } = + section + { if cmt == masp_tx { return Some((payer, token)); } diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index 5ee5394aeb4..ca5dc4f4623 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -5,7 +5,7 @@ use namada_core::masp_primitives::transaction::Transaction; use namada_token::TransparentTransfersRef; #[cfg(any(test, feature = "testing"))] pub use namada_token::testing; -pub use namada_token::tx::{apply_masp_fees, apply_shielded_transfer}; +pub use namada_token::tx::{apply_masp_sus_fees, apply_shielded_transfer}; pub use namada_token::{ Amount, DenominatedAmount, Denomination, MaspDigitPos, Store, Transfer, storage_key, utils, validate_transfer_in_out, diff --git a/crates/vp_prelude/src/lib.rs b/crates/vp_prelude/src/lib.rs index b1cf735def1..1972a9c2f4e 100644 --- a/crates/vp_prelude/src/lib.rs +++ b/crates/vp_prelude/src/lib.rs @@ -177,7 +177,7 @@ impl VerifySigGadget { /// Verify the shielding fee authorization from an IBC shielding tx /// if the predicate returns true. #[inline(always)] - pub fn verify_masp_fee_signatures_when bool>( + pub fn verify_masp_sus_fee_signatures_when bool>( &mut self, predicate: F, tx_data: &Tx, diff --git a/wasm/tx_ibc/src/lib.rs b/wasm/tx_ibc/src/lib.rs index ac0a3aac0b3..5d49abb378e 100644 --- a/wasm/tx_ibc/src/lib.rs +++ b/wasm/tx_ibc/src/lib.rs @@ -52,10 +52,13 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { let signer = shielded.get_signer().ok_or_err_msg( "Unable to find a public key signing a shielding IBC tx.", )?; - token::apply_masp_fees(ctx, signer, &shielded.shielding_fee_token) - .wrap_err( - "Encountered error while paying the shielding fee for IBC", - )?; + token::apply_masp_sus_fees( + ctx, + signer, + &shielded.shielding_fee_token, + true, + ) + .wrap_err("Encountered error while paying the shielding fee for IBC")?; ctx.push_action(Action::IbcShielding( action::IbcShieldingAction::new( shielded.shielding_fee_authorization, diff --git a/wasm/vp_implicit/src/lib.rs b/wasm/vp_implicit/src/lib.rs index 1947272ae6c..a65f90c42df 100644 --- a/wasm/vp_implicit/src/lib.rs +++ b/wasm/vp_implicit/src/lib.rs @@ -115,7 +115,7 @@ fn validate_tx( )?, Action::Masp(MaspAction::MaspSectionRef(_)) => (), Action::IbcShielding(ibc_shielding_action) => gadget - .verify_masp_fee_signatures_when( + .verify_masp_sus_fee_signatures_when( || { if let Some(signer) = ibc_shielding_action.get_signer() { diff --git a/wasm/vp_user/src/lib.rs b/wasm/vp_user/src/lib.rs index 2bf553c9b71..3c8af98437b 100644 --- a/wasm/vp_user/src/lib.rs +++ b/wasm/vp_user/src/lib.rs @@ -114,7 +114,7 @@ fn validate_tx( )?, Action::Masp(MaspAction::MaspSectionRef(_)) => (), Action::IbcShielding(ibc_shielding_action) => gadget - .verify_masp_fee_signatures_when( + .verify_masp_sus_fee_signatures_when( || { if let Some(signer) = ibc_shielding_action.get_signer() { From 3bd65a362abe49288ad5f806546d61d62aa5b58c Mon Sep 17 00:00:00 2001 From: satan Date: Tue, 29 Jul 2025 12:10:05 +0200 Subject: [PATCH 24/24] Fixed a silly assertion error in integration test --- crates/tests/src/integration/masp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 5459fa5018c..46ca7a1799a 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -2021,7 +2021,7 @@ fn test_shielding_fee_client_balance_checks() -> Result<()> { }); assert!(captured.result.is_err()); assert!(captured.contains( - "does not have enough balance to pay for fees and shielding fees." + "does not have enough balance to pay for fees and MASP sustainability fees." )); let captured = CapturedOutput::of(|| { run(