From 80c62efbe0c0facc6390804c4c865143627f62e3 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 29 Jul 2025 17:54:07 +0200 Subject: [PATCH 01/18] Extends SDK's arguments to include a frontend sustainability fee (cherry picked from commit acd54b88d2eb8b4f5910c47ce1b7e33acac3e083) --- crates/apps_lib/src/cli.rs | 13 +++++++++++-- crates/apps_lib/src/cli/client.rs | 3 ++- crates/sdk/src/args.rs | 32 ++++++++++++++++++++++++++----- crates/sdk/src/lib.rs | 4 ++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index baf03a69597..9072366ead3 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -318,7 +318,9 @@ pub mod cmds { Self::parse_with_ctx(matches, TxShieldingTransfer); let tx_unshielding_transfer = Self::parse_with_ctx(matches, TxUnshieldingTransfer); - let tx_ibc_transfer = Self::parse_with_ctx(matches, TxIbcTransfer); + let tx_ibc_transfer = Self::parse_with_ctx(matches, |cmd| { + TxIbcTransfer(Box::new(cmd)) + }); let tx_osmosis_swap = Self::parse_with_ctx(matches, |cmd| { TxOsmosisSwap(Box::new(cmd)) }); @@ -507,7 +509,7 @@ pub mod cmds { TxShieldedTransfer(TxShieldedTransfer), TxShieldingTransfer(TxShieldingTransfer), TxUnshieldingTransfer(TxUnshieldingTransfer), - TxIbcTransfer(TxIbcTransfer), + TxIbcTransfer(Box), TxOsmosisSwap(Box), QueryResult(QueryResult), TxUpdateAccount(TxUpdateAccount), @@ -4953,6 +4955,7 @@ pub mod args { sources: data, targets, tx_code_path: self.tx_code_path.to_path_buf(), + frontend_sus_fee: None, }) } } @@ -4981,6 +4984,7 @@ pub mod args { sources: data, targets, tx_code_path, + frontend_sus_fee: None, } } @@ -5128,6 +5132,7 @@ pub mod args { ibc_memo: self.ibc_memo, gas_spending_key, tx_code_path: self.tx_code_path.to_path_buf(), + frontend_sus_fee: None, }) } } @@ -5169,6 +5174,8 @@ pub mod args { ibc_memo, gas_spending_key, tx_code_path, + // FIXME: is it ok to skip the frontend fee from the cli? + frontend_sus_fee: None, } } @@ -7231,6 +7238,7 @@ pub mod args { IbcShieldingTransferAsset::Address(chain_ctx.get(&addr)) } }, + frontend_sus_fee: None, }) } } @@ -7266,6 +7274,7 @@ pub mod args { channel_id, token, }, + frontend_sus_fee: None, } } diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index 4faa979e4d8..c55dac91df3 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -100,7 +100,8 @@ impl CliApi { let namada = ctx.to_sdk(client, io); tx::submit_unshielding_transfer(&namada, args).await?; } - Sub::TxIbcTransfer(TxIbcTransfer(args)) => { + Sub::TxIbcTransfer(args) => { + let TxIbcTransfer(args) = *args; let chain_ctx = ctx.borrow_mut_chain_or_exit(); let ledger_address = chain_ctx.get(&args.tx.ledger_address); diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index c42ec761a0d..f884172eb8a 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -383,10 +383,13 @@ pub struct TxTransparentSource { pub struct TxShieldingTransfer { /// Common tx arguments pub tx: Tx, - /// Transfer target address + /// Transfer target data pub targets: Vec>, - /// Transfer-specific data + /// Transfer source data pub sources: Vec>, + // FIXME: update the build function to use this + /// The optional data for the frontend sustainability fee + pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -430,11 +433,14 @@ pub struct TxTransparentTarget { pub struct TxUnshieldingTransfer { /// Common tx arguments pub tx: Tx, - /// Transfer source spending key + /// Transfer source data pub sources: Vec>, - /// Transfer-specific data + // FIXME: check if we need to do anything when building this tx to handle + // the new fee + /// Transfer target data (potentially also carries data for the frontend + /// sustainability fee) pub targets: Vec>, - /// Optional additional keys for gas payment + /// Optional additional key for gas payment pub gas_spending_key: Option, /// Path to the TX WASM code file pub tx_code_path: PathBuf, @@ -512,11 +518,17 @@ pub enum Slippage { /// An token swap on Osmosis #[derive(Debug, Clone)] pub struct TxOsmosisSwap { + // FIXME: this field contains the frontend fee, make sure to use it in the + // building function /// The IBC transfer data pub transfer: TxIbcTransfer, /// The token we wish to receive (on Namada) pub output_denom: String, /// Address of the recipient on Namada + // FIXME: actually, in the case of an unshield and reshield we might not + // take the fee at all. If either side is shielded we take the fees. If + // both are shielded we should either take the fees only once or not take + // them at all pub recipient: Either, /// Address to receive funds exceeding the minimum amount, /// in case of IBC shieldings @@ -711,6 +723,8 @@ impl TxOsmosisSwap { ), ), expiration: transfer.tx.expiration.clone(), + // The frontend fee should be set in the transfer object + frontend_sus_fee: None, }, ) .await? @@ -810,6 +824,11 @@ pub struct TxIbcTransfer { pub ibc_memo: Option, /// Optional additional keys for gas payment pub gas_spending_key: Option, + // FIXME: can join this with other arguments? I don't think so + // FIXME: update the build function to use this field + // FIXME: can we join this with refund_target? + /// The optional data for the frontend sustainability fee + pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -3217,6 +3236,9 @@ pub struct GenIbcShieldingTransfer { pub expiration: TxExpiration, /// Asset to shield over IBC to Namada pub asset: IbcShieldingTransferAsset, + // FIXME: update the build function to use this + /// The optional data for the frontend sustainability fee + pub frontend_sus_fee: Option>, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 79f5e81987b..3bcede80022 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -183,12 +183,14 @@ pub trait Namada: NamadaIo { &self, targets: Vec, sources: Vec, + frontend_sus_fee: Option, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { sources, targets, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), + frontend_sus_fee, } } @@ -293,6 +295,7 @@ pub trait Namada: NamadaIo { token: Address, amount: InputAmount, channel_id: ChannelId, + frontend_sus_fee: Option, ) -> args::TxIbcTransfer { args::TxIbcTransfer { source, @@ -309,6 +312,7 @@ pub trait Namada: NamadaIo { gas_spending_key: Default::default(), tx: self.tx_builder(), tx_code_path: PathBuf::from(TX_IBC_WASM), + frontend_sus_fee, } } From 827fdd111a8491383d10225fcd7a90f1308a96e7 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 31 Jul 2025 12:49:20 +0200 Subject: [PATCH 02/18] Updates the SDK builder functions to account for the sus fee (cherry picked from commit 14cc724d8acd108fcfcc42c620e70c0303c24048) --- crates/apps_lib/src/cli.rs | 3 +- crates/sdk/src/args.rs | 35 ++++++++------ crates/sdk/src/tx.rs | 97 +++++++++++++++++++++++++++++++++++++- 3 files changed, 117 insertions(+), 18 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 9072366ead3..b23f801b21e 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -5174,7 +5174,6 @@ pub mod args { ibc_memo, gas_spending_key, tx_code_path, - // FIXME: is it ok to skip the frontend fee from the cli? frontend_sus_fee: None, } } @@ -5251,6 +5250,7 @@ pub mod args { route: self.route, osmosis_lcd_rpc: self.osmosis_lcd_rpc, osmosis_sqs_rpc: self.osmosis_sqs_rpc, + frontend_sus_fee: None, }) } } @@ -5303,6 +5303,7 @@ pub mod args { route, osmosis_lcd_rpc, osmosis_sqs_rpc, + frontend_sus_fee: None, } } diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index f884172eb8a..be89efd0b76 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -387,7 +387,6 @@ pub struct TxShieldingTransfer { pub targets: Vec>, /// Transfer source data pub sources: Vec>, - // FIXME: update the build function to use this /// The optional data for the frontend sustainability fee pub frontend_sus_fee: Option>, /// Path to the TX WASM code file @@ -435,8 +434,6 @@ pub struct TxUnshieldingTransfer { pub tx: Tx, /// Transfer source data pub sources: Vec>, - // FIXME: check if we need to do anything when building this tx to handle - // the new fee /// Transfer target data (potentially also carries data for the frontend /// sustainability fee) pub targets: Vec>, @@ -515,20 +512,20 @@ pub enum Slippage { }, } +// FIXME: do we need a new event for the frontend fee? +// FIXME: should the fee be taken shielded? It might avoid us to update the ibc +// methods actually. Yes but it would produce a substantial amount of notes with +// very little amounts FIXME: what happens to the sus fee if we wanto to +// shield/unshield more than one asset? It should probably be a vector of +// targets, one for every asset /// An token swap on Osmosis #[derive(Debug, Clone)] pub struct TxOsmosisSwap { - // FIXME: this field contains the frontend fee, make sure to use it in the - // building function /// The IBC transfer data pub transfer: TxIbcTransfer, /// The token we wish to receive (on Namada) pub output_denom: String, /// Address of the recipient on Namada - // FIXME: actually, in the case of an unshield and reshield we might not - // take the fee at all. If either side is shielded we take the fees. If - // both are shielded we should either take the fees only once or not take - // them at all pub recipient: Either, /// Address to receive funds exceeding the minimum amount, /// in case of IBC shieldings @@ -546,6 +543,11 @@ pub struct TxOsmosisSwap { pub osmosis_lcd_rpc: Option, /// REST rpc endpoint to Osmosis SQS pub osmosis_sqs_rpc: Option, + /// The optional data for the frontend sustainability fee + /// NOTE: if the swap is shielded (from MASP to MASP), no sustainability + /// fee should be taken + // FIXME: try to join this with recipient + pub frontend_sus_fee: Option>, } impl TxOsmosisSwap { @@ -613,6 +615,7 @@ impl TxOsmosisSwap { osmosis_lcd_rpc, osmosis_sqs_rpc, output_denom: namada_output_denom, + frontend_sus_fee, } = self; let osmosis_lcd_rpc = osmosis_lcd_rpc @@ -723,8 +726,7 @@ impl TxOsmosisSwap { ), ), expiration: transfer.tx.expiration.clone(), - // The frontend fee should be set in the transfer object - frontend_sus_fee: None, + frontend_sus_fee, }, ) .await? @@ -819,15 +821,17 @@ pub struct TxIbcTransfer { /// Refund target address when the shielded transfer failure pub refund_target: Option, /// IBC shielding transfer data for the destination chain + // FIXME: here the shielding transaction to reapply to namada, it should + // carry the sus fee pub ibc_shielding_data: Option, /// Memo for IBC transfer packet pub ibc_memo: Option, /// Optional additional keys for gas payment pub gas_spending_key: Option, - // FIXME: can join this with other arguments? I don't think so - // FIXME: update the build function to use this field - // FIXME: can we join this with refund_target? /// The optional data for the frontend sustainability fee + // FIXME: this should probably be an either with ibc_shielding_data. Yes + // but there would still be the room for errors, maybe need marker traits? + // Not sure... pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, @@ -3236,8 +3240,9 @@ pub struct GenIbcShieldingTransfer { pub expiration: TxExpiration, /// Asset to shield over IBC to Namada pub asset: IbcShieldingTransferAsset, - // FIXME: update the build function to use this /// The optional data for the frontend sustainability fee + /// NOTE: if the shielding operation is part of a swap, and this is + /// shielded (from MASP to MASP), no sustainability fee should be taken pub frontend_sus_fee: Option>, } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 94381b78506..ed695e5f9f1 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2857,7 +2857,7 @@ pub async fn build_ibc_transfer( query_wasm_code_hash(context, args.tx_code_path.to_str().unwrap()) .await .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; - let masp_transfer_data = MaspTransferData { + let mut masp_transfer_data = MaspTransferData { sources: vec![( args.source.clone(), args.token.clone(), @@ -2900,6 +2900,57 @@ pub async fn build_ibc_transfer( None }; + if let Some(TxTransparentTarget { + target, + token, + amount, + }) = &args.frontend_sus_fee + { + match (&source, &args.ibc_shielding_data) { + (&MASP, None) => { + // Validate the amount given + let validated_amount = validate_amount( + context, + amount.to_owned(), + token, + args.tx.force, + ) + .await?; + masp_transfer_data.targets.push(( + TransferTarget::Address(target.to_owned()), + token.to_owned(), + validated_amount, + )); + + transfer = transfer + .credit( + target.to_owned(), + token.to_owned(), + validated_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; + } + (&MASP, Some(_)) => { + return Err(Error::Other( + "A frontend sustainability fee was requested but the ibc \ + roundtrip is shielded" + .to_string(), + )); + } + (_, _) => { + return Err(Error::Other( + "A frontend sustainability fee was requested but the ibc \ + source is transparent. If the transaction is a roundtrip \ + (e.g. swap), and the return target is shielded, the fee \ + should be taken from the shielding transaction instead." + .to_string(), + )); + } + } + } + // For transfer from a spending key let shielded_parts = construct_shielded_parts( context, @@ -3558,6 +3609,7 @@ async fn get_masp_fee_payment_amount( } /// Build a shielding transfer +// FIXME: need to test the fee pub async fn build_shielding_transfer( context: &N, args: &mut args::TxShieldingTransfer, @@ -3655,6 +3707,22 @@ pub async fn build_shielding_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } + if let Some(TxTransparentTarget { + target, + token, + amount, + }) = &args.frontend_sus_fee + { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + + data = data + .credit(target.to_owned(), token.to_owned(), validated_amount) + .ok_or(Error::Other("Combined transfer overflows".to_string()))?; + } + let shielded_parts = construct_shielded_parts( context, transfer_data, @@ -4334,14 +4402,39 @@ pub async fn gen_ibc_shielding_transfer( .precompute_asset_types(context.client(), tokens) .await; + let extra_target = match &args.frontend_sus_fee { + Some(TxTransparentTarget { + target, + token, + amount, + }) => { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, false) + .await?; + + vec![( + TransferTarget::Address(target.to_owned()), + token.to_owned(), + validated_amount, + )] + } + None => vec![], + }; + let masp_transfer_data = MaspTransferData { sources: vec![( TransferSource::Address(source.clone()), token.clone(), validated_amount, )], - targets: vec![(args.target, token.clone(), validated_amount)], + targets: [ + extra_target, + vec![(args.target, token.clone(), validated_amount)], + ] + .concat(), }; + let shielded_transfer = { let mut shielded = context.shielded_mut().await; shielded From f9e483e0ff689265512140ce63bde9c11394c4fa Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 6 Aug 2025 18:38:24 +0200 Subject: [PATCH 03/18] Integration tests for frontend fees and fixes to the sdk builders (cherry picked from commit 765f3d93cbbe7a48a580f04e5617d9144e2a23f5) --- crates/apps_lib/src/cli.rs | 132 +++++++++++++++++--- crates/sdk/src/tx.rs | 77 ++++++++---- crates/tests/src/integration/masp.rs | 177 +++++++++++++++++++++++++++ 3 files changed, 344 insertions(+), 42 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index b23f801b21e..e53c9a721ed 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3548,6 +3548,9 @@ pub mod args { pub const FEE_PAYER_OPT: ArgOpt = arg_opt("gas-payer"); pub const FILE_PATH: Arg = arg("file"); pub const FORCE: ArgFlag = flag("force"); + #[cfg(any(test, feature = "testing"))] + pub const FRONTEND_SUS_FEE: ArgOpt = + arg_opt("frontend-sus-fee"); pub const FULL_RESET: ArgFlag = flag("full-reset"); pub const GAS_LIMIT: ArgDefault = arg_default( "gas-limit", @@ -4930,11 +4933,11 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; - let mut data = vec![]; + let mut sources = vec![]; let chain_ctx = ctx.borrow_mut_chain_or_exit(); for transfer_data in self.sources { - data.push(TxTransparentSource { + sources.push(TxTransparentSource { source: chain_ctx.get(&transfer_data.source), token: chain_ctx.get(&transfer_data.token), amount: transfer_data.amount, @@ -4949,13 +4952,19 @@ pub mod args { amount: transfer_data.amount, }); } + let frontend_sus_fee = + self.frontend_sus_fee.map(|fee| TxTransparentTarget { + target: chain_ctx.get(&fee.target), + token: chain_ctx.get(&fee.token), + amount: fee.amount, + }); Ok(TxShieldingTransfer:: { tx, - sources: data, + sources, targets, tx_code_path: self.tx_code_path.to_path_buf(), - frontend_sus_fee: None, + frontend_sus_fee, }) } } @@ -4966,13 +4975,49 @@ pub mod args { let source = SOURCE.parse(matches); let target = PAYMENT_ADDRESS_TARGET.parse(matches); let token = TOKEN.parse(matches); - let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); + let raw_amount = AMOUNT.parse(matches); + let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); - let data = vec![TxTransparentSource { + + #[cfg(any(test, feature = "testing"))] + let frontend_sus_fee = FRONTEND_SUS_FEE.parse(matches).map( + |target| // Take a constant fee of 1 on top of the input amount + TxTransparentTarget { + target, + token: token.clone(), + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + }, + ); + + #[cfg(not(any(test, feature = "testing")))] + let frontend_sus_fee = None; + + let mut sources = if frontend_sus_fee.is_some() { + // Adjust the inputs to account for the extra token + vec![TxTransparentSource { + source: source.clone(), + token: token.clone(), + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + }] + } else { + vec![] + }; + + sources.push(TxTransparentSource { source, token: token.clone(), amount, - }]; + }); let targets = vec![TxShieldedTarget { target, token, @@ -4981,15 +5026,16 @@ pub mod args { Self { tx, - sources: data, + sources, targets, tx_code_path, - frontend_sus_fee: None, + frontend_sus_fee, } } fn def(app: App) -> App { - app.add_args::>() + let app = app + .add_args::>() .arg(SOURCE.def().help(wrap!( "The transparent source account address. The source's key \ will be used to produce the signature." @@ -5004,7 +5050,15 @@ pub mod args { AMOUNT .def() .help(wrap!("The amount to transfer in decimal.")), - ) + ); + + #[cfg(any(test, feature = "testing"))] + let app = app.arg(FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will take \ + the masp sustainability fee." + ))); + + app } } @@ -5056,31 +5110,63 @@ pub mod args { let source = SPENDING_KEY_SOURCE.parse(matches); let target = TARGET.parse(matches); let token = TOKEN.parse(matches); - let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); + let raw_amount = AMOUNT.parse(matches); + let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); - let data = vec![TxTransparentTarget { - target, + let targets = vec![TxTransparentTarget { + target: target.clone(), token: token.clone(), amount, }]; let sources = vec![TxShieldedSource { - source, - token, + source: source.clone(), + token: token.clone(), amount, }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); + #[cfg(any(test, feature = "testing"))] + let mut sources = sources; + #[cfg(any(test, feature = "testing"))] + let mut targets = targets; + #[cfg(any(test, feature = "testing"))] + if let Some(fee_target) = FRONTEND_SUS_FEE.parse(matches) { + // Take a constant fee of 1 on top of the input amount + targets.push(TxTransparentTarget { + target: fee_target, + token: token.clone(), + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + }); + + sources.push(TxShieldedSource { + source, + token, + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + }) + }; + Self { tx, sources, - targets: data, + targets, gas_spending_key, tx_code_path, } } fn def(app: App) -> App { - app.add_args::>() + let app = app + .add_args::>() .arg( SPENDING_KEY_SOURCE .def() @@ -5101,7 +5187,15 @@ pub mod args { "The optional spending key that will be used for gas \ payment. When not provided the source spending key will \ be used." - ))) + ))); + + #[cfg(any(test, feature = "testing"))] + let app = app.arg(FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will take \ + the masp sustainability fee." + ))); + + app } } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index ed695e5f9f1..6359fa9abba 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2900,6 +2900,7 @@ pub async fn build_ibc_transfer( None }; + // FIXME: adjust this if let Some(TxTransparentTarget { target, token, @@ -3609,7 +3610,6 @@ async fn get_masp_fee_payment_amount( } /// Build a shielding transfer -// FIXME: need to test the fee pub async fn build_shielding_transfer( context: &N, args: &mut args::TxShieldingTransfer, @@ -3643,11 +3643,14 @@ pub async fn build_shielding_transfer( let mut transfer_data = MaspTransferData::default(); let mut data = token::Transfer::default(); - for TxTransparentSource { - source, - token, - amount, - } in &args.sources + for ( + id, + TxTransparentSource { + source, + token, + amount, + }, + ) in args.sources.iter().enumerate() { // Validate the amount given let validated_amount = @@ -3675,10 +3678,53 @@ pub async fn build_shielding_transfer( .await?; } + // The frontend sustainability fee (when provided) must be paid by the + // first source + let masp_shield_amount = match (id, &args.frontend_sus_fee) { + ( + 0, + Some(TxTransparentTarget { + target: sus_fee_target, + token: sus_fee_token, + amount: sus_fee_amt, + }), + ) => { + if sus_fee_token != token { + return Err(Error::Other( + "The sustainability fee token does not match the \ + token of the first transaction's source" + .to_string(), + )); + } + + // Validate the amount given + let validated_fee_amount = validate_amount( + context, + sus_fee_amt.to_owned(), + sus_fee_token, + args.tx.force, + ) + .await?; + + data = data + .credit( + sus_fee_target.to_owned(), + sus_fee_token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; + + checked!(validated_amount - validated_fee_amount)? + } + _ => validated_amount, + }; + transfer_data.sources.push(( TransferSource::Address(source.to_owned()), token.to_owned(), - validated_amount, + masp_shield_amount, )); data = data @@ -3707,22 +3753,6 @@ pub async fn build_shielding_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } - if let Some(TxTransparentTarget { - target, - token, - amount, - }) = &args.frontend_sus_fee - { - // Validate the amount given - let validated_amount = - validate_amount(context, amount.to_owned(), token, args.tx.force) - .await?; - - data = data - .credit(target.to_owned(), token.to_owned(), validated_amount) - .ok_or(Error::Other("Combined transfer overflows".to_string()))?; - } - let shielded_parts = construct_shielded_parts( context, transfer_data, @@ -4402,6 +4432,7 @@ pub async fn gen_ibc_shielding_transfer( .precompute_asset_types(context.client(), tokens) .await; + // FIXME: need to adjust this let extra_target = match &args.frontend_sus_fee { Some(TxTransparentTarget { target, diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index aad55ee8dce..b750343ebb4 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -10173,3 +10173,180 @@ fn history_with_conversions() -> Result<()> { Ok(()) } + +// Test that shielding and unshielding transactions can pay a small fee to a +// transparent address as a form of sustainability fee for frontend providers +#[test] +fn frontend_sus_fee() -> 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()?; + // Wait till epoch boundary + node.next_masp_epoch(); + + // Initialize address of the frontend provider with no balance + let (frontend_alias, _frontend_key) = + make_temp_account(&node, validator_one_rpc, "Frontend", NAM, 0)?; + + // Test sus fee when shielding. Send 10 NAM from Albert to PA and 1 NAM to a + // transparent address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "10", + "--frontend-sus-fee", + frontend_alias, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + + // Assert NAM balance at VK(A) is 10 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 10")); + + // Assert NAM balance at the frontend is 1 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + frontend_alias, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 1")); + + // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 1 NAM to + // a transparent address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "9", + "--frontend-sus-fee", + frontend_alias, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + + // Assert NAM balance at VK(A) is 0 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 0")); + + // Assert NAM balance at the frontend is 2 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + frontend_alias, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 2")); + + Ok(()) +} From 8633c00c96f8f522dbc3f5cca2fb1e743cd22912 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 12 Aug 2025 16:33:40 +0200 Subject: [PATCH 04/18] Tests masp frontend fee for ibc transactions (cherry picked from commit 35777190f7fed604e6d5045018d4f67df83c0dfa) --- .github/workflows/scripts/e2e.json | 1 + crates/apps/Cargo.toml | 2 + crates/apps_lib/src/cli.rs | 76 +++++++++++++-- crates/sdk/src/args.rs | 9 +- crates/sdk/src/tx.rs | 38 ++++---- crates/tests/src/e2e/ibc_tests.rs | 148 ++++++++++++++++++++++++++++- 6 files changed, 246 insertions(+), 28 deletions(-) diff --git a/.github/workflows/scripts/e2e.json b/.github/workflows/scripts/e2e.json index e542f119779..432ed520db2 100644 --- a/.github/workflows/scripts/e2e.json +++ b/.github/workflows/scripts/e2e.json @@ -1,5 +1,6 @@ { "e2e::eth_bridge_tests::everything": 4, + "e2e::ibc_tests::frontend_sus_fee": 415, "e2e::ibc_tests::ibc_transfers": 414, "e2e::ibc_tests::ibc_nft_transfers": 224, "e2e::ibc_tests::pgf_over_ibc": 415, diff --git a/crates/apps/Cargo.toml b/crates/apps/Cargo.toml index e5b09287ba1..f3769df8b53 100644 --- a/crates/apps/Cargo.toml +++ b/crates/apps/Cargo.toml @@ -54,6 +54,8 @@ mainnet = ["namada_apps_lib/mainnet"] jemalloc = ["namada_node/jemalloc"] migrations = ["namada_apps_lib/migrations"] namada-eth-bridge = ["namada_apps_lib/namada-eth-bridge"] +# FIXME: how to enforce this in ci? +testing = ["namada_apps_lib/testing"] [dependencies] namada_apps_lib.workspace = true diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index e53c9a721ed..6d450aca371 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3551,6 +3551,9 @@ pub mod args { #[cfg(any(test, feature = "testing"))] pub const FRONTEND_SUS_FEE: ArgOpt = arg_opt("frontend-sus-fee"); + #[cfg(any(test, feature = "testing"))] + pub const FRONTEND_SUS_FEE_IBC: ArgOpt = + arg_opt("frontend-sus-fee-ibc"); pub const FULL_RESET: ArgFlag = flag("full-reset"); pub const GAS_LIMIT: ArgDefault = arg_default( "gas-limit", @@ -5237,7 +5240,9 @@ pub mod args { let source = TRANSFER_SOURCE.parse(matches); let receiver = RECEIVER.parse(matches); let token = TOKEN.parse(matches); - let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); + + let raw_amount = AMOUNT.parse(matches); + let amount = InputAmount::Unvalidated(raw_amount); let port_id = PORT_ID.parse(matches); let channel_id = CHANNEL_ID.parse(matches); let timeout_height = TIMEOUT_HEIGHT.parse(matches); @@ -5253,6 +5258,28 @@ pub mod args { let ibc_memo = IBC_MEMO.parse(matches); let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); + // FIXME: this api is to confusing, split the amount into two, + // source amount and target amount + #[cfg(any(test, feature = "testing"))] + let frontend_sus_fee = + FRONTEND_SUS_FEE.parse(matches).map(|target| + // Take a constant fee of 1 on top of the input amount + TxTransparentTarget { + target, + token: token.clone(), + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + }); + + #[cfg(not(any(test, feature = "testing")))] + let frontend_sus_fee = None; + + eprintln!("AMOUNT IN CLI: {:#?}", amount); //FIXME: remove + Self { tx, source, @@ -5263,12 +5290,13 @@ pub mod args { channel_id, timeout_height, timeout_sec_offset, + // FIXME: check this refund, we should not refund the fee refund_target, ibc_shielding_data, ibc_memo, gas_spending_key, tx_code_path, - frontend_sus_fee: None, + frontend_sus_fee, } } @@ -7312,6 +7340,9 @@ pub mod args { ) -> Result, Self::Error> { let query = self.query.to_sdk(ctx)?; let chain_ctx = ctx.borrow_chain_or_exit(); + let frontend_sus_fee = self + .frontend_sus_fee + .map(|(target, amount)| (chain_ctx.get(&target), amount)); Ok(GenIbcShieldingTransfer:: { query, @@ -7333,7 +7364,7 @@ pub mod args { IbcShieldingTransferAsset::Address(chain_ctx.get(&addr)) } }, - frontend_sus_fee: None, + frontend_sus_fee, }) } } @@ -7344,7 +7375,8 @@ pub mod args { let output_folder = OUTPUT_FOLDER_PATH.parse(matches); let target = TRANSFER_TARGET.parse(matches); let token = TOKEN_STR.parse(matches); - let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); + let raw_amount = AMOUNT.parse(matches); + let amount = InputAmount::Unvalidated(raw_amount); let expiration = EXPIRATION_OPT.parse(matches); let port_id = PORT_ID.parse(matches); let channel_id = CHANNEL_ID.parse(matches); @@ -7357,6 +7389,27 @@ pub mod args { None => TxExpiration::Default, } }; + // FIXME: this api is to confusing, split the amount into two, + // source amount and target amount + #[cfg(any(test, feature = "testing"))] + let frontend_sus_fee = FRONTEND_SUS_FEE_IBC.parse(matches).map( + |target| // Take a constant fee of 1 on top of the input amount + ( + target, + //FIXME: this means we can't do anything when it comes to nfts for this frontend fee + InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ), + ), + ); + + #[cfg(not(any(test, feature = "testing")))] + let frontend_sus_fee = None; + + eprintln!("AMOUNT IN CLI: {:#?}", amount); //FIXME: remove Self { query, @@ -7369,12 +7422,13 @@ pub mod args { channel_id, token, }, - frontend_sus_fee: None, + frontend_sus_fee, } } fn def(app: App) -> App { - app.add_args::>() + let app = app + .add_args::>() .arg(OUTPUT_FOLDER_PATH.def().help(wrap!( "The output folder path where the artifact will be stored." ))) @@ -7410,7 +7464,15 @@ pub mod args { ))) .arg(CHANNEL_ID.def().help(wrap!( "The channel ID via which the token is received." - ))) + ))); + + #[cfg(any(test, feature = "testing"))] + let app = app.arg(FRONTEND_SUS_FEE_IBC.def().help(wrap!( + "The optional address of the frontend provider that will take \ + the masp sustainability fee." + ))); + + app } } diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index be89efd0b76..a9802e1edda 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -547,7 +547,7 @@ pub struct TxOsmosisSwap { /// NOTE: if the swap is shielded (from MASP to MASP), no sustainability /// fee should be taken // FIXME: try to join this with recipient - pub frontend_sus_fee: Option>, + pub frontend_sus_fee: Option<(C::TransferTarget, InputAmount)>, } impl TxOsmosisSwap { @@ -832,6 +832,7 @@ pub struct TxIbcTransfer { // FIXME: this should probably be an either with ibc_shielding_data. Yes // but there would still be the room for errors, maybe need marker traits? // Not sure... + // FIXME: support this in the client for testing only pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, @@ -3240,10 +3241,12 @@ pub struct GenIbcShieldingTransfer { pub expiration: TxExpiration, /// Asset to shield over IBC to Namada pub asset: IbcShieldingTransferAsset, - /// The optional data for the frontend sustainability fee + /// The optional data for the frontend sustainability fee (the target and + /// the amount, the token must be the same as the one involved in the + /// shielding transaction since ics-20 only supports a single asset) /// NOTE: if the shielding operation is part of a swap, and this is /// shielded (from MASP to MASP), no sustainability fee should be taken - pub frontend_sus_fee: Option>, + pub frontend_sus_fee: Option<(C::TransferTarget, InputAmount)>, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 6359fa9abba..27c6bde05d1 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -4432,32 +4432,32 @@ pub async fn gen_ibc_shielding_transfer( .precompute_asset_types(context.client(), tokens) .await; - // FIXME: need to adjust this - let extra_target = match &args.frontend_sus_fee { - Some(TxTransparentTarget { - target, - token, - amount, - }) => { + let (extra_target, source_amount) = match &args.frontend_sus_fee { + Some((target, amount)) => { // Validate the amount given - let validated_amount = - validate_amount(context, amount.to_owned(), token, false) + let validated_fee_amount = + validate_amount(context, amount.to_owned(), &token, false) .await?; + let source_amount = + checked!(validated_amount + validated_fee_amount)?; - vec![( - TransferTarget::Address(target.to_owned()), - token.to_owned(), - validated_amount, - )] + ( + vec![( + target.to_owned(), + token.to_owned(), + validated_fee_amount, + )], + source_amount, + ) } - None => vec![], + None => (vec![], validated_amount), }; let masp_transfer_data = MaspTransferData { sources: vec![( TransferSource::Address(source.clone()), token.clone(), - validated_amount, + source_amount, )], targets: [ extra_target, @@ -4466,6 +4466,9 @@ pub async fn gen_ibc_shielding_transfer( .concat(), }; + // eprintln!("MASP TRANSFER DATA: {:#?}", masp_transfer_data); //FIXME: + // remove + let shielded_transfer = { let mut shielded = context.shielded_mut().await; shielded @@ -4481,6 +4484,9 @@ pub async fn gen_ibc_shielding_transfer( .map_err(|err| TxSubmitError::MaspError(err.to_string()))? }; + // eprintln!("GENERATED MASP BUNDLE: {:#?}", shielded_transfer); //FIXME: + // remove + Ok(shielded_transfer.map(|st| st.masp_tx)) } diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 3d3a2298bf5..1cfef2ccf58 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -146,6 +146,7 @@ fn ibc_transfers() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -224,6 +225,7 @@ fn ibc_transfers() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -254,6 +256,7 @@ fn ibc_transfers() -> Result<()> { 100, &port_id_namada, &channel_id_namada, + None, )?; transfer_from_cosmos( &test_gaia, @@ -305,6 +308,7 @@ fn ibc_transfers() -> Result<()> { None, None, true, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -324,6 +328,7 @@ fn ibc_transfers() -> Result<()> { 1, &port_id_namada, &channel_id_namada, + None, )?; transfer_from_cosmos( &test_gaia, @@ -358,6 +363,7 @@ fn ibc_transfers() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -387,6 +393,7 @@ fn ibc_transfers() -> Result<()> { None, None, false, + None, )?; // wait for the timeout sleep(10); @@ -420,6 +427,7 @@ fn ibc_transfers() -> Result<()> { None, None, true, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -451,6 +459,7 @@ fn ibc_transfers() -> Result<()> { None, None, true, + None, )?; // wait for the timeout sleep(10); @@ -506,6 +515,7 @@ fn ibc_transfers() -> Result<()> { 100, &port_id_namada, &channel_id_namada, + None, )?; transfer_from_cosmos( &test_gaia, @@ -610,6 +620,7 @@ fn ibc_nft_transfers() -> Result<()> { None, None, false, + None, )?; clear_packet(&hermes_dir, &port_id_namada, &channel_id_namada, &test)?; check_balance(&test, &namada_receiver, &ibc_trace_on_namada, 0)?; @@ -623,6 +634,7 @@ fn ibc_nft_transfers() -> Result<()> { 1, &port_id_namada, &channel_id_namada, + None, )?; nft_transfer_from_cosmos( &test_cosmwasm, @@ -672,6 +684,7 @@ fn ibc_nft_transfers() -> Result<()> { None, None, true, + None, )?; clear_packet(&hermes_dir, &port_id_namada, &channel_id_namada, &test)?; check_shielded_balance(&test, AB_VIEWING_KEY, &ibc_trace_on_namada, 0)?; @@ -959,6 +972,7 @@ fn ibc_token_inflation() -> Result<()> { 1, &port_id_namada, &channel_id_namada, + None, )?; transfer_from_cosmos( &test_gaia, @@ -1150,6 +1164,7 @@ fn ibc_rate_limit() -> Result<()> { None, None, false, + None, )?; // Transfer 1 NAM from Namada to Gaia again will fail @@ -1170,6 +1185,7 @@ fn ibc_rate_limit() -> Result<()> { ), None, false, + None, )?; // wait for the next epoch @@ -1194,6 +1210,7 @@ fn ibc_rate_limit() -> Result<()> { None, None, false, + None, )?; // wait for the next epoch @@ -1304,6 +1321,7 @@ fn ibc_unlimited_channel() -> Result<()> { ), None, false, + None, )?; // Proposal on Namada @@ -1362,6 +1380,7 @@ fn ibc_unlimited_channel() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -1693,6 +1712,7 @@ fn ibc_pfm_happy_flows() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( @@ -1990,6 +2010,7 @@ fn ibc_pfm_unhappy_flows() -> Result<()> { None, None, false, + None, )?; wait_for_packet_relay( @@ -2314,6 +2335,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { 8, &port_id_namada, &channel_id_namada, + None, )?; let masp_receiver = match iter { // Test addresses encoded using `bech32m`... @@ -2353,6 +2375,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { None, Some(&memo), true, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -2419,6 +2442,7 @@ fn ibc_shielded_recv_middleware_unhappy_flow() -> Result<()> { 8, &port_id_namada, &channel_id_namada, + None, )?; let memo = packet_forward_memo( MASP.to_string().into(), @@ -2445,6 +2469,7 @@ fn ibc_shielded_recv_middleware_unhappy_flow() -> Result<()> { None, Some(&memo), false, + None, )?; wait_for_packet_relay(&hermes_dir, &port_id_gaia, &channel_id_gaia, &test)?; @@ -2703,6 +2728,7 @@ fn try_invalid_transfers( Some(&format!("Invalid IBC denom: {nam_addr}")), None, false, + None, )?; // invalid channel @@ -2720,6 +2746,7 @@ fn try_invalid_transfers( Some("No channel end: port transfer, channel channel-42"), None, false, + None, )?; Ok(()) @@ -2776,6 +2803,7 @@ fn transfer( expected_err: Option<&str>, ibc_memo: Option<&str>, gen_refund_target: bool, + frontend_sus_fee: Option<&str>, ) -> Result { let rpc = get_actor_rpc(test, Who::Validator(0)); @@ -2797,7 +2825,7 @@ fn transfer( "--port-id", &port_id, "--gas-limit", - "60000", + "70000", "--node", &rpc, ]); @@ -2843,6 +2871,11 @@ fn transfer( tx_args.push(IBC_REFUND_TARGET_ALIAS); } + if let Some(target) = frontend_sus_fee { + tx_args.push("--frontend-sus-fee"); + tx_args.push(target.as_ref()); + } + let mut client = run!(test, Bin::Client, tx_args, Some(300))?; match expected_err { Some(err) => { @@ -3464,12 +3497,13 @@ fn gen_ibc_shielding_data( amount: u64, port_id: &PortId, channel_id: &ChannelId, + frontend_sus_fee: Option<&str>, ) -> Result { let rpc = get_actor_rpc(dst_test, Who::Validator(0)); let output_folder = dst_test.test_dir.path().to_string_lossy(); let amount = amount.to_string(); - let args = vec![ + let mut args = vec![ "ibc-gen-shielding", "--output-folder-path", &output_folder, @@ -3486,6 +3520,10 @@ fn gen_ibc_shielding_data( "--node", &rpc, ]; + if let Some(target) = frontend_sus_fee { + args.push("--frontend-sus-fee-ibc"); + args.push(target.as_ref()); + } let mut client = run!(dst_test, Bin::Client, args, Some(120))?; let (_unread, matched) = @@ -3807,6 +3845,111 @@ fn nft_transfer_from_cosmos( Ok(()) } +// Verify that MASP frontend fees can be paid also when doing IBC transactions, +// specifically an unshielding originating from Namada and a shielding tx +// originating from a foreign chain +#[test] +fn frontend_sus_fee() -> Result<()> { + let update_genesis = + |mut genesis: templates::All, base_dir: &_| { + genesis.parameters.parameters.epochs_per_year = + epochs_per_year_from_min_duration(1800); + genesis.parameters.ibc_params.default_mint_limit = + Amount::max_signed(); + genesis + .parameters + .ibc_params + .default_per_epoch_throughput_limit = Amount::max_signed(); + setup::set_validators(1, genesis, base_dir, |_| 0, vec![]) + }; + let (ledger, gaia, test, test_gaia) = + run_namada_cosmos(CosmosChainType::Gaia(None), update_genesis)?; + let _bg_ledger = ledger.background(); + let _bg_gaia = gaia.background(); + + let hermes_dir = setup_hermes(&test, &test_gaia)?; + let port_id_namada = FT_PORT_ID.parse().unwrap(); + let port_id_gaia = FT_PORT_ID.parse().unwrap(); + let (channel_id_namada, channel_id_gaia) = create_channel_with_hermes( + &hermes_dir, + &test, + &test_gaia, + &port_id_namada, + &port_id_gaia, + )?; + + // Start relaying + let hermes = run_hermes(&hermes_dir)?; + let _bg_hermes = hermes.background(); + + // Shielding transfer 100 samoleans from Gaia to Namada (this command will + // actually add another extra token in the output as the frontend sus fee) + let shielding_data_path = gen_ibc_shielding_data( + &test, + AA_PAYMENT_ADDRESS, + COSMOS_COIN, + 100, + &port_id_namada, + &channel_id_namada, + Some(ESTER), + )?; + transfer_from_cosmos( + &test_gaia, + COSMOS_USER, + MASP.to_string(), + COSMOS_COIN, + // 1 extra token for the frontend sus fee + 101, + &port_id_gaia, + &channel_id_gaia, + Some(Either::Left(shielding_data_path)), + None, + )?; + wait_for_packet_relay( + &hermes_dir, + &port_id_gaia, + &channel_id_gaia, + &test_gaia, + )?; + // Check the token on Namada + let ibc_denom_on_namada = + format!("{port_id_namada}/{channel_id_namada}/{COSMOS_COIN}"); + check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 100)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 899)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; + + // Unshielding transfer 10 samoleans from Namada to Gaia + let gaia_receiver = find_cosmos_address(&test_gaia, COSMOS_USER)?; + transfer( + &test, + A_SPENDING_KEY, + &gaia_receiver, + &ibc_denom_on_namada, + // An extra token will be added to this amount as a frontend masp fee + 10, + Some(BERTHA_KEY), + &port_id_namada, + &channel_id_namada, + None, + None, + None, + None, + true, + Some(ESTER), + )?; + wait_for_packet_relay( + &hermes_dir, + &port_id_namada, + &channel_id_namada, + &test, + )?; + check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 89)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 909)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 2)?; + + Ok(()) +} + /// Basic Osmosis test that checks if the chain has been set up correctly. #[test] fn osmosis_basic() -> Result<()> { @@ -3930,6 +4073,7 @@ fn osmosis_xcs() -> Result<()> { None, None, false, + None, )?; // Transfer Samoleans from Gaia transfer_from_cosmos( From 02994e3caff9ea1ea874afd4345654ddfbe9e3a5 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 21 Aug 2025 16:48:21 +0200 Subject: [PATCH 05/18] Vectorized frontend sus fee for shielding ops (cherry picked from commit 1d0872f528d66faae8e46b652dda7c5891c203cb) --- crates/apps_lib/src/cli.rs | 41 +++++++++-------- crates/sdk/src/args.rs | 13 +++--- crates/sdk/src/lib.rs | 2 +- crates/sdk/src/tx.rs | 90 ++++++++++++++++---------------------- 4 files changed, 67 insertions(+), 79 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 6d450aca371..6f195fdc06c 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -4955,12 +4955,14 @@ pub mod args { amount: transfer_data.amount, }); } - let frontend_sus_fee = - self.frontend_sus_fee.map(|fee| TxTransparentTarget { + let mut frontend_sus_fee = vec![]; + for fee in self.frontend_sus_fee { + frontend_sus_fee.push(TxTransparentTarget { target: chain_ctx.get(&fee.target), token: chain_ctx.get(&fee.token), amount: fee.amount, }); + } Ok(TxShieldingTransfer:: { tx, @@ -4983,24 +4985,28 @@ pub mod args { let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); #[cfg(any(test, feature = "testing"))] - let frontend_sus_fee = FRONTEND_SUS_FEE.parse(matches).map( - |target| // Take a constant fee of 1 on top of the input amount - TxTransparentTarget { - target, - token: token.clone(), - amount: InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), + let frontend_sus_fee = + FRONTEND_SUS_FEE.parse(matches).map_or(vec![], |target| { + vec![TxTransparentTarget { + target, + token: token.clone(), + amount: InputAmount::Unvalidated( + token::DenominatedAmount::new( + // Take a constant fee of 1 on top of the input + // amount + 1.into(), + raw_amount.denom(), ), - }, - ); + ), + }] + }); #[cfg(not(any(test, feature = "testing")))] - let frontend_sus_fee = None; + let frontend_sus_fee = vec![]; - let mut sources = if frontend_sus_fee.is_some() { + let mut sources = if frontend_sus_fee.is_empty() { + vec![] + } else { // Adjust the inputs to account for the extra token vec![TxTransparentSource { source: source.clone(), @@ -5012,8 +5018,6 @@ pub mod args { ), ), }] - } else { - vec![] }; sources.push(TxTransparentSource { @@ -5128,6 +5132,7 @@ pub mod args { }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); + // FIXME: single cfg here #[cfg(any(test, feature = "testing"))] let mut sources = sources; #[cfg(any(test, feature = "testing"))] diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index a9802e1edda..9fa305a5f03 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -388,7 +388,7 @@ pub struct TxShieldingTransfer { /// Transfer source data pub sources: Vec>, /// The optional data for the frontend sustainability fee - pub frontend_sus_fee: Option>, + pub frontend_sus_fee: Vec>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -512,12 +512,6 @@ pub enum Slippage { }, } -// FIXME: do we need a new event for the frontend fee? -// FIXME: should the fee be taken shielded? It might avoid us to update the ibc -// methods actually. Yes but it would produce a substantial amount of notes with -// very little amounts FIXME: what happens to the sus fee if we wanto to -// shield/unshield more than one asset? It should probably be a vector of -// targets, one for every asset /// An token swap on Osmosis #[derive(Debug, Clone)] pub struct TxOsmosisSwap { @@ -832,7 +826,9 @@ pub struct TxIbcTransfer { // FIXME: this should probably be an either with ibc_shielding_data. Yes // but there would still be the room for errors, maybe need marker traits? // Not sure... - // FIXME: support this in the client for testing only + // FIXME: should this be a tuple (receiver, amount) to force the token to + // be the same as that of the transfer? FIXME: should the frontend fees + // always be specified just as a percentage and a target address? pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, @@ -3246,6 +3242,7 @@ pub struct GenIbcShieldingTransfer { /// shielding transaction since ics-20 only supports a single asset) /// NOTE: if the shielding operation is part of a swap, and this is /// shielded (from MASP to MASP), no sustainability fee should be taken + // FIXME: transparent target only pub frontend_sus_fee: Option<(C::TransferTarget, InputAmount)>, } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 3bcede80022..ef55e39f731 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -183,7 +183,7 @@ pub trait Namada: NamadaIo { &self, targets: Vec, sources: Vec, - frontend_sus_fee: Option, + frontend_sus_fee: Vec, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { sources, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 27c6bde05d1..22302b5cac7 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -3643,14 +3643,11 @@ pub async fn build_shielding_transfer( let mut transfer_data = MaspTransferData::default(); let mut data = token::Transfer::default(); - for ( - id, - TxTransparentSource { - source, - token, - amount, - }, - ) in args.sources.iter().enumerate() + for TxTransparentSource { + source, + token, + amount, + } in &args.sources { // Validate the amount given let validated_amount = @@ -3678,53 +3675,10 @@ pub async fn build_shielding_transfer( .await?; } - // The frontend sustainability fee (when provided) must be paid by the - // first source - let masp_shield_amount = match (id, &args.frontend_sus_fee) { - ( - 0, - Some(TxTransparentTarget { - target: sus_fee_target, - token: sus_fee_token, - amount: sus_fee_amt, - }), - ) => { - if sus_fee_token != token { - return Err(Error::Other( - "The sustainability fee token does not match the \ - token of the first transaction's source" - .to_string(), - )); - } - - // Validate the amount given - let validated_fee_amount = validate_amount( - context, - sus_fee_amt.to_owned(), - sus_fee_token, - args.tx.force, - ) - .await?; - - data = data - .credit( - sus_fee_target.to_owned(), - sus_fee_token.to_owned(), - validated_fee_amount, - ) - .ok_or(Error::Other( - "Combined transfer overflows".to_string(), - ))?; - - checked!(validated_amount - validated_fee_amount)? - } - _ => validated_amount, - }; - transfer_data.sources.push(( TransferSource::Address(source.to_owned()), token.to_owned(), - masp_shield_amount, + validated_amount, )); data = data @@ -3753,6 +3707,38 @@ pub async fn build_shielding_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } + for args::TxTransparentTarget { + target: sus_fee_target, + token: sus_fee_token, + amount: sus_fee_amt, + } in &args.frontend_sus_fee + { + // Validate the amount given + let validated_fee_amount = validate_amount( + context, + sus_fee_amt.to_owned(), + sus_fee_token, + args.tx.force, + ) + .await?; + + // Decrease the amount to shield to account for this frontend fee, take + // it from the first shielding input that matches the fee token + for (_, token, amount) in &mut transfer_data.sources { + if token == sus_fee_token { + *amount = checked!(amount - validated_fee_amount)?; + break; + } + } + data = data + .credit( + sus_fee_target.to_owned(), + sus_fee_token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other("Combined transfer overflows".to_string()))?; + } + let shielded_parts = construct_shielded_parts( context, transfer_data, From 5f000110db41c4a78d5e773ff599a6ccf5a719b0 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 21 Aug 2025 16:57:27 +0200 Subject: [PATCH 06/18] Updates frontend fee interface (cherry picked from commit b33f48b4e0981894478d7af3885356c3c50cac5b) --- .github/workflows/scripts/e2e.json | 2 +- crates/apps/Cargo.toml | 2 - crates/apps_lib/src/cli.rs | 142 ++++++++++++----------------- crates/sdk/src/args.rs | 14 +-- crates/sdk/src/tx.rs | 9 +- 5 files changed, 60 insertions(+), 109 deletions(-) diff --git a/.github/workflows/scripts/e2e.json b/.github/workflows/scripts/e2e.json index 432ed520db2..d965f0bdc55 100644 --- a/.github/workflows/scripts/e2e.json +++ b/.github/workflows/scripts/e2e.json @@ -1,6 +1,6 @@ { "e2e::eth_bridge_tests::everything": 4, - "e2e::ibc_tests::frontend_sus_fee": 415, + "e2e::ibc_tests::frontend_sus_fee": 350, "e2e::ibc_tests::ibc_transfers": 414, "e2e::ibc_tests::ibc_nft_transfers": 224, "e2e::ibc_tests::pgf_over_ibc": 415, diff --git a/crates/apps/Cargo.toml b/crates/apps/Cargo.toml index f3769df8b53..e5b09287ba1 100644 --- a/crates/apps/Cargo.toml +++ b/crates/apps/Cargo.toml @@ -54,8 +54,6 @@ mainnet = ["namada_apps_lib/mainnet"] jemalloc = ["namada_node/jemalloc"] migrations = ["namada_apps_lib/migrations"] namada-eth-bridge = ["namada_apps_lib/namada-eth-bridge"] -# FIXME: how to enforce this in ci? -testing = ["namada_apps_lib/testing"] [dependencies] namada_apps_lib.workspace = true diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 6f195fdc06c..c86c7f545f6 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3548,12 +3548,6 @@ pub mod args { pub const FEE_PAYER_OPT: ArgOpt = arg_opt("gas-payer"); pub const FILE_PATH: Arg = arg("file"); pub const FORCE: ArgFlag = flag("force"); - #[cfg(any(test, feature = "testing"))] - pub const FRONTEND_SUS_FEE: ArgOpt = - arg_opt("frontend-sus-fee"); - #[cfg(any(test, feature = "testing"))] - pub const FRONTEND_SUS_FEE_IBC: ArgOpt = - arg_opt("frontend-sus-fee-ibc"); pub const FULL_RESET: ArgFlag = flag("full-reset"); pub const GAS_LIMIT: ArgDefault = arg_default( "gas-limit", @@ -3699,6 +3693,14 @@ pub mod args { pub const TARGET: Arg = arg("target"); pub const TARGET_OPT: ArgOpt = arg_opt("target"); pub const TEMPLATES_PATH: Arg = arg("templates-path"); + // WARNING: use only for testing purposes, MASP frontend fees don't make + // sense when operating from the CLI + pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = + arg_opt("frontend-sus-fee"); + // WARNING: use only for testing purposes, MASP frontend fees don't make + // sense when operating from the CLI + pub const __TEST_FRONTEND_SUS_FEE_IBC: ArgOpt = + arg_opt("frontend-sus-fee-ibc"); pub const TIMEOUT_HEIGHT: ArgOpt = arg_opt("timeout-height"); pub const TIMEOUT_SEC_OFFSET: ArgOpt = arg_opt("timeout-sec-offset"); pub const TM_ADDRESS_OPT: ArgOpt = arg_opt("tm-address"); @@ -4983,10 +4985,9 @@ pub mod args { let raw_amount = AMOUNT.parse(matches); let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); - - #[cfg(any(test, feature = "testing"))] - let frontend_sus_fee = - FRONTEND_SUS_FEE.parse(matches).map_or(vec![], |target| { + let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE + .parse(matches) + .map_or(vec![], |target| { vec![TxTransparentTarget { target, token: token.clone(), @@ -5001,9 +5002,6 @@ pub mod args { }] }); - #[cfg(not(any(test, feature = "testing")))] - let frontend_sus_fee = vec![]; - let mut sources = if frontend_sus_fee.is_empty() { vec![] } else { @@ -5041,8 +5039,7 @@ pub mod args { } fn def(app: App) -> App { - let app = app - .add_args::>() + app.add_args::>() .arg(SOURCE.def().help(wrap!( "The transparent source account address. The source's key \ will be used to produce the signature." @@ -5057,15 +5054,11 @@ pub mod args { AMOUNT .def() .help(wrap!("The amount to transfer in decimal.")), - ); - - #[cfg(any(test, feature = "testing"))] - let app = app.arg(FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will take \ - the masp sustainability fee." - ))); - - app + ) + .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } @@ -5120,25 +5113,19 @@ pub mod args { let raw_amount = AMOUNT.parse(matches); let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); - let targets = vec![TxTransparentTarget { + let mut targets = vec![TxTransparentTarget { target: target.clone(), token: token.clone(), amount, }]; - let sources = vec![TxShieldedSource { + let mut sources = vec![TxShieldedSource { source: source.clone(), token: token.clone(), amount, }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); - // FIXME: single cfg here - #[cfg(any(test, feature = "testing"))] - let mut sources = sources; - #[cfg(any(test, feature = "testing"))] - let mut targets = targets; - #[cfg(any(test, feature = "testing"))] - if let Some(fee_target) = FRONTEND_SUS_FEE.parse(matches) { + if let Some(fee_target) = __TEST_FRONTEND_SUS_FEE.parse(matches) { // Take a constant fee of 1 on top of the input amount targets.push(TxTransparentTarget { target: fee_target, @@ -5161,7 +5148,7 @@ pub mod args { ), ), }) - }; + } Self { tx, @@ -5173,8 +5160,7 @@ pub mod args { } fn def(app: App) -> App { - let app = app - .add_args::>() + app.add_args::>() .arg( SPENDING_KEY_SOURCE .def() @@ -5195,15 +5181,11 @@ pub mod args { "The optional spending key that will be used for gas \ payment. When not provided the source spending key will \ be used." - ))); - - #[cfg(any(test, feature = "testing"))] - let app = app.arg(FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will take \ - the masp sustainability fee." - ))); - - app + ))) + .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } @@ -5263,11 +5245,8 @@ pub mod args { let ibc_memo = IBC_MEMO.parse(matches); let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); - // FIXME: this api is to confusing, split the amount into two, - // source amount and target amount - #[cfg(any(test, feature = "testing"))] let frontend_sus_fee = - FRONTEND_SUS_FEE.parse(matches).map(|target| + __TEST_FRONTEND_SUS_FEE.parse(matches).map(|target| // Take a constant fee of 1 on top of the input amount TxTransparentTarget { target, @@ -5280,11 +5259,6 @@ pub mod args { ), }); - #[cfg(not(any(test, feature = "testing")))] - let frontend_sus_fee = None; - - eprintln!("AMOUNT IN CLI: {:#?}", amount); //FIXME: remove - Self { tx, source, @@ -5295,7 +5269,6 @@ pub mod args { channel_id, timeout_height, timeout_sec_offset, - // FIXME: check this refund, we should not refund the fee refund_target, ibc_shielding_data, ibc_memo, @@ -5351,6 +5324,15 @@ pub mod args { payment (if this is a shielded action). When not \ provided the source spending key will be used." ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .conflicts_with(IBC_SHIELDING_DATA_PATH.name) + .help(wrap!( + "The optional address of the frontend provider \ + that will take the masp sustainability fee." + )), + ) } } @@ -7394,27 +7376,20 @@ pub mod args { None => TxExpiration::Default, } }; - // FIXME: this api is to confusing, split the amount into two, - // source amount and target amount - #[cfg(any(test, feature = "testing"))] - let frontend_sus_fee = FRONTEND_SUS_FEE_IBC.parse(matches).map( - |target| // Take a constant fee of 1 on top of the input amount - ( - target, - //FIXME: this means we can't do anything when it comes to nfts for this frontend fee - InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), + let frontend_sus_fee = + __TEST_FRONTEND_SUS_FEE_IBC.parse(matches).map(|target| { + ( + target, + InputAmount::Unvalidated( + token::DenominatedAmount::new( + // Take a constant fee of 1 on top of the input + // amount + 1.into(), + raw_amount.denom(), ), - ), - ); - - #[cfg(not(any(test, feature = "testing")))] - let frontend_sus_fee = None; - - eprintln!("AMOUNT IN CLI: {:#?}", amount); //FIXME: remove + ), + ) + }); Self { query, @@ -7432,8 +7407,7 @@ pub mod args { } fn def(app: App) -> App { - let app = app - .add_args::>() + app.add_args::>() .arg(OUTPUT_FOLDER_PATH.def().help(wrap!( "The output folder path where the artifact will be stored." ))) @@ -7469,15 +7443,11 @@ pub mod args { ))) .arg(CHANNEL_ID.def().help(wrap!( "The channel ID via which the token is received." - ))); - - #[cfg(any(test, feature = "testing"))] - let app = app.arg(FRONTEND_SUS_FEE_IBC.def().help(wrap!( - "The optional address of the frontend provider that will take \ - the masp sustainability fee." - ))); - - app + ))) + .arg(__TEST_FRONTEND_SUS_FEE_IBC.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 9fa305a5f03..ac1ea5dde3c 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -540,8 +540,7 @@ pub struct TxOsmosisSwap { /// The optional data for the frontend sustainability fee /// NOTE: if the swap is shielded (from MASP to MASP), no sustainability /// fee should be taken - // FIXME: try to join this with recipient - pub frontend_sus_fee: Option<(C::TransferTarget, InputAmount)>, + pub frontend_sus_fee: Option<(C::Address, InputAmount)>, } impl TxOsmosisSwap { @@ -815,20 +814,12 @@ pub struct TxIbcTransfer { /// Refund target address when the shielded transfer failure pub refund_target: Option, /// IBC shielding transfer data for the destination chain - // FIXME: here the shielding transaction to reapply to namada, it should - // carry the sus fee pub ibc_shielding_data: Option, /// Memo for IBC transfer packet pub ibc_memo: Option, /// Optional additional keys for gas payment pub gas_spending_key: Option, /// The optional data for the frontend sustainability fee - // FIXME: this should probably be an either with ibc_shielding_data. Yes - // but there would still be the room for errors, maybe need marker traits? - // Not sure... - // FIXME: should this be a tuple (receiver, amount) to force the token to - // be the same as that of the transfer? FIXME: should the frontend fees - // always be specified just as a percentage and a target address? pub frontend_sus_fee: Option>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, @@ -3242,8 +3233,7 @@ pub struct GenIbcShieldingTransfer { /// shielding transaction since ics-20 only supports a single asset) /// NOTE: if the shielding operation is part of a swap, and this is /// shielded (from MASP to MASP), no sustainability fee should be taken - // FIXME: transparent target only - pub frontend_sus_fee: Option<(C::TransferTarget, InputAmount)>, + pub frontend_sus_fee: Option<(C::Address, InputAmount)>, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 22302b5cac7..a307e1f15c8 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2900,7 +2900,6 @@ pub async fn build_ibc_transfer( None }; - // FIXME: adjust this if let Some(TxTransparentTarget { target, token, @@ -4429,7 +4428,7 @@ pub async fn gen_ibc_shielding_transfer( ( vec![( - target.to_owned(), + TransferTarget::Address(target.to_owned()), token.to_owned(), validated_fee_amount, )], @@ -4452,9 +4451,6 @@ pub async fn gen_ibc_shielding_transfer( .concat(), }; - // eprintln!("MASP TRANSFER DATA: {:#?}", masp_transfer_data); //FIXME: - // remove - let shielded_transfer = { let mut shielded = context.shielded_mut().await; shielded @@ -4470,9 +4466,6 @@ pub async fn gen_ibc_shielding_transfer( .map_err(|err| TxSubmitError::MaspError(err.to_string()))? }; - // eprintln!("GENERATED MASP BUNDLE: {:#?}", shielded_transfer); //FIXME: - // remove - Ok(shielded_transfer.map(|st| st.masp_tx)) } From d8c9aa7e6ac5d5168fc6b787dc2a2b9583f9f465 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 5 Sep 2025 17:06:59 +0200 Subject: [PATCH 07/18] Shielded frontend fee target for incoming IBC packets (cherry picked from commit 4b1398f2e523bae2bbe76609b4f59e0427d6eff6) --- crates/apps_lib/src/cli.rs | 11 +++++++++-- crates/sdk/src/args.rs | 5 +++-- crates/sdk/src/tx.rs | 11 +++++++++-- crates/tests/src/e2e/ibc_tests.rs | 14 +++++++++----- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index c86c7f545f6..e332a1748ab 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3693,13 +3693,14 @@ pub mod args { pub const TARGET: Arg = arg("target"); pub const TARGET_OPT: ArgOpt = arg_opt("target"); pub const TEMPLATES_PATH: Arg = arg("templates-path"); + // FIXME: add the test prelude in the cli args too // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = arg_opt("frontend-sus-fee"); // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI - pub const __TEST_FRONTEND_SUS_FEE_IBC: ArgOpt = + pub const __TEST_FRONTEND_SUS_FEE_IBC: ArgOpt = arg_opt("frontend-sus-fee-ibc"); pub const TIMEOUT_HEIGHT: ArgOpt = arg_opt("timeout-height"); pub const TIMEOUT_SEC_OFFSET: ArgOpt = arg_opt("timeout-sec-offset"); @@ -5200,6 +5201,12 @@ pub mod args { let chain_ctx = ctx.borrow_mut_chain_or_exit(); let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); + let frontend_sus_fee = + self.frontend_sus_fee.map(|fee| TxTransparentTarget { + target: chain_ctx.get(&fee.target), + token: chain_ctx.get(&fee.token), + amount: fee.amount, + }); Ok(TxIbcTransfer:: { tx, @@ -5216,7 +5223,7 @@ pub mod args { ibc_memo: self.ibc_memo, gas_spending_key, tx_code_path: self.tx_code_path.to_path_buf(), - frontend_sus_fee: None, + frontend_sus_fee, }) } } diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index ac1ea5dde3c..c113811e8e4 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -540,7 +540,7 @@ pub struct TxOsmosisSwap { /// The optional data for the frontend sustainability fee /// NOTE: if the swap is shielded (from MASP to MASP), no sustainability /// fee should be taken - pub frontend_sus_fee: Option<(C::Address, InputAmount)>, + pub frontend_sus_fee: Option<(C::PaymentAddress, InputAmount)>, } impl TxOsmosisSwap { @@ -3221,6 +3221,7 @@ pub struct GenIbcShieldingTransfer { /// The output directory path to where serialize the data pub output_folder: Option, /// The target address + // FIXME: why isn't this a payment address? pub target: C::TransferTarget, /// Transferred token amount pub amount: InputAmount, @@ -3233,7 +3234,7 @@ pub struct GenIbcShieldingTransfer { /// shielding transaction since ics-20 only supports a single asset) /// NOTE: if the shielding operation is part of a swap, and this is /// shielded (from MASP to MASP), no sustainability fee should be taken - pub frontend_sus_fee: Option<(C::Address, InputAmount)>, + pub frontend_sus_fee: Option<(C::PaymentAddress, InputAmount)>, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index a307e1f15c8..82cd16458da 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2916,6 +2916,12 @@ pub async fn build_ibc_transfer( args.tx.force, ) .await?; + + masp_transfer_data.sources.push(( + args.source.clone(), + args.token.clone(), + validated_amount, + )); masp_transfer_data.targets.push(( TransferTarget::Address(target.to_owned()), token.to_owned(), @@ -2923,7 +2929,8 @@ pub async fn build_ibc_transfer( )); transfer = transfer - .credit( + .transfer( + source.to_owned(), target.to_owned(), token.to_owned(), validated_amount, @@ -4428,7 +4435,7 @@ pub async fn gen_ibc_shielding_transfer( ( vec![( - TransferTarget::Address(target.to_owned()), + TransferTarget::PaymentAddress(target.to_owned()), token.to_owned(), validated_fee_amount, )], diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 1cfef2ccf58..225ffcbc5d1 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -3274,7 +3274,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(), @@ -3292,6 +3292,8 @@ fn transfer_from_cosmos( sender.as_ref(), "--gas-prices", "0.001stake", + "--gas", + "250000", "--node", &rpc, "--keyring-backend", @@ -3891,7 +3893,7 @@ fn frontend_sus_fee() -> Result<()> { 100, &port_id_namada, &channel_id_namada, - Some(ESTER), + Some(AC_PAYMENT_ADDRESS), )?; transfer_from_cosmos( &test_gaia, @@ -3915,10 +3917,11 @@ fn frontend_sus_fee() -> Result<()> { let ibc_denom_on_namada = format!("{port_id_namada}/{channel_id_namada}/{COSMOS_COIN}"); check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 100)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 1)?; check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 899)?; - check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; - // Unshielding transfer 10 samoleans from Namada to Gaia + // Unshielding transfer 10 samoleans from Namada to Gaia with transparent + // frontend fee FIXME: need to test also the transparent version let gaia_receiver = find_cosmos_address(&test_gaia, COSMOS_USER)?; transfer( &test, @@ -3944,8 +3947,9 @@ fn frontend_sus_fee() -> Result<()> { &test, )?; check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 89)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 1)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 909)?; - check_balance(&test, ESTER, &ibc_denom_on_namada, 2)?; Ok(()) } From 5d04fc64b8032acdecb9b39172162e93d48ae503 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 6 Sep 2025 12:10:49 +0200 Subject: [PATCH 08/18] Adds support for shielded masp frontend fees (cherry picked from commit 2418cf553ccda8f89111ac93276b7d4580f7d309) --- crates/apps_lib/src/cli.rs | 213 +++++++++++++++++++++++-------------- crates/sdk/src/args.rs | 14 ++- crates/sdk/src/lib.rs | 12 ++- crates/sdk/src/tx.rs | 166 ++++++++++++++++++++++++----- 4 files changed, 290 insertions(+), 115 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index e332a1748ab..aa22ac518ed 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3696,7 +3696,7 @@ pub mod args { // FIXME: add the test prelude in the cli args too // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI - pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = + pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = arg_opt("frontend-sus-fee"); // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI @@ -4960,11 +4960,27 @@ pub mod args { } let mut frontend_sus_fee = vec![]; for fee in self.frontend_sus_fee { - frontend_sus_fee.push(TxTransparentTarget { - target: chain_ctx.get(&fee.target), - token: chain_ctx.get(&fee.token), - amount: fee.amount, - }); + match fee { + Either::Left(transparent_target) => { + frontend_sus_fee.push(Either::Left( + TxTransparentTarget { + target: chain_ctx + .get(&transparent_target.target), + token: chain_ctx.get(&transparent_target.token), + amount: transparent_target.amount, + }, + )); + } + Either::Right(shielded_target) => { + frontend_sus_fee.push(Either::Right( + TxShieldedTarget { + target: chain_ctx.get(&shielded_target.target), + token: chain_ctx.get(&shielded_target.token), + amount: shielded_target.amount, + }, + )); + } + } } Ok(TxShieldingTransfer:: { @@ -4986,49 +5002,40 @@ pub mod args { let raw_amount = AMOUNT.parse(matches); let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); + let sources = vec![TxTransparentSource { + source: source.clone(), + token: token.clone(), + amount, + }]; + let targets = vec![TxShieldedTarget { + target, + token: token.clone(), + amount, + }]; let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE .parse(matches) - .map_or(vec![], |target| { - vec![TxTransparentTarget { - target, - token: token.clone(), - amount: InputAmount::Unvalidated( - token::DenominatedAmount::new( - // Take a constant fee of 1 on top of the input - // amount - 1.into(), - raw_amount.denom(), - ), - ), - }] - }); - - let mut sources = if frontend_sus_fee.is_empty() { - vec![] - } else { - // Adjust the inputs to account for the extra token - vec![TxTransparentSource { - source: source.clone(), - token: token.clone(), - amount: InputAmount::Unvalidated( + .map_or(Default::default(), |fee_target| { + // Take a constant fee of 1 on top of the input amount + let amount = InputAmount::Unvalidated( token::DenominatedAmount::new( 1.into(), raw_amount.denom(), ), - ), - }] - }; + ); - sources.push(TxTransparentSource { - source, - token: token.clone(), - amount, - }); - let targets = vec![TxShieldedTarget { - target, - token, - amount, - }]; + vec![match PaymentAddress::from_str(&fee_target.raw) { + Ok(_) => Either::Right(TxShieldedTarget { + target: fee_target.to_payment_address(), + token: token.clone(), + amount, + }), + Err(_) => Either::Left(TxTransparentTarget { + target: fee_target.to_address(), + token, + amount, + }), + }] + }); Self { tx, @@ -5092,6 +5099,31 @@ pub mod args { amount: transfer_data.amount, }); } + + let mut frontend_sus_fee = vec![]; + for fee in self.frontend_sus_fee { + match fee { + Either::Left(transparent_target) => { + frontend_sus_fee.push(Either::Left( + TxTransparentTarget { + target: chain_ctx + .get(&transparent_target.target), + token: chain_ctx.get(&transparent_target.token), + amount: transparent_target.amount, + }, + )); + } + Either::Right(shielded_target) => { + frontend_sus_fee.push(Either::Right( + TxShieldedTarget { + target: chain_ctx.get(&shielded_target.target), + token: chain_ctx.get(&shielded_target.token), + amount: shielded_target.amount, + }, + )); + } + } + } let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); @@ -5101,6 +5133,7 @@ pub mod args { gas_spending_key, sources, tx_code_path: self.tx_code_path.to_path_buf(), + frontend_sus_fee, }) } } @@ -5114,42 +5147,42 @@ pub mod args { let raw_amount = AMOUNT.parse(matches); let amount = InputAmount::Unvalidated(raw_amount); let tx_code_path = PathBuf::from(TX_TRANSFER_WASM); - let mut targets = vec![TxTransparentTarget { + let targets = vec![TxTransparentTarget { target: target.clone(), token: token.clone(), amount, }]; - let mut sources = vec![TxShieldedSource { + let sources = vec![TxShieldedSource { source: source.clone(), token: token.clone(), amount, }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); - if let Some(fee_target) = __TEST_FRONTEND_SUS_FEE.parse(matches) { - // Take a constant fee of 1 on top of the input amount - targets.push(TxTransparentTarget { - target: fee_target, - token: token.clone(), - amount: InputAmount::Unvalidated( + let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE + .parse(matches) + .map_or(Default::default(), |fee_target| { + // Take a constant fee of 1 on top of the input amount + let amount = InputAmount::Unvalidated( token::DenominatedAmount::new( 1.into(), raw_amount.denom(), ), - ), - }); + ); - sources.push(TxShieldedSource { - source, - token, - amount: InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ), - }) - } + vec![match PaymentAddress::from_str(&fee_target.raw) { + Ok(_) => Either::Right(TxShieldedTarget { + target: fee_target.to_payment_address(), + token: token.clone(), + amount, + }), + Err(_) => Either::Left(TxTransparentTarget { + target: fee_target.to_address(), + token, + amount, + }), + }] + }); Self { tx, @@ -5157,6 +5190,7 @@ pub mod args { targets, gas_spending_key, tx_code_path, + frontend_sus_fee, } } @@ -5201,12 +5235,22 @@ pub mod args { let chain_ctx = ctx.borrow_mut_chain_or_exit(); let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); - let frontend_sus_fee = - self.frontend_sus_fee.map(|fee| TxTransparentTarget { - target: chain_ctx.get(&fee.target), - token: chain_ctx.get(&fee.token), - amount: fee.amount, - }); + let frontend_sus_fee = self.frontend_sus_fee.map(|fee| match fee { + Either::Left(transparent_target) => { + Either::Left(TxTransparentTarget { + target: chain_ctx.get(&transparent_target.target), + token: chain_ctx.get(&transparent_target.token), + amount: transparent_target.amount, + }) + } + Either::Right(shielded_target) => { + Either::Right(TxShieldedTarget { + target: chain_ctx.get(&shielded_target.target), + token: chain_ctx.get(&shielded_target.token), + amount: shielded_target.amount, + }) + } + }); Ok(TxIbcTransfer:: { tx, @@ -5253,18 +5297,27 @@ pub mod args { let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); let frontend_sus_fee = - __TEST_FRONTEND_SUS_FEE.parse(matches).map(|target| - // Take a constant fee of 1 on top of the input amount - TxTransparentTarget { - target, + __TEST_FRONTEND_SUS_FEE.parse(matches).map(|fee_target| { + // Take a constant fee of 1 on top of the input amount + let amount = InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ); + match PaymentAddress::from_str(&fee_target.raw) { + Ok(_) => Either::Right(TxShieldedTarget { + target: fee_target.to_payment_address(), token: token.clone(), - amount: InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ), - }); + amount, + }), + Err(_) => Either::Left(TxTransparentTarget { + target: fee_target.to_address(), + token: token.clone(), + amount, + }), + } + }); Self { tx, diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index c113811e8e4..ee4ace33630 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -388,7 +388,10 @@ pub struct TxShieldingTransfer { /// Transfer source data pub sources: Vec>, /// The optional data for the frontend sustainability fee - pub frontend_sus_fee: Vec>, + // FIXME: should join this with sources? No maybe better to define a single + // percentage to apply to all inputs and a single receiver + pub frontend_sus_fee: + Vec, TxShieldedTarget>>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -434,11 +437,13 @@ pub struct TxUnshieldingTransfer { pub tx: Tx, /// Transfer source data pub sources: Vec>, - /// Transfer target data (potentially also carries data for the frontend - /// sustainability fee) + /// Transfer target data pub targets: Vec>, /// Optional additional key for gas payment pub gas_spending_key: Option, + /// The optional data for the frontend sustainability fee + pub frontend_sus_fee: + Vec, TxShieldedTarget>>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -820,7 +825,8 @@ pub struct TxIbcTransfer { /// Optional additional keys for gas payment pub gas_spending_key: Option, /// The optional data for the frontend sustainability fee - pub frontend_sus_fee: Option>, + pub frontend_sus_fee: + Option, TxShieldedTarget>>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index ef55e39f731..2ff7784891f 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -183,7 +183,9 @@ pub trait Namada: NamadaIo { &self, targets: Vec, sources: Vec, - frontend_sus_fee: Vec, + frontend_sus_fee: Vec< + either::Either, + >, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { sources, @@ -201,6 +203,9 @@ pub trait Namada: NamadaIo { sources: Vec, targets: Vec, gas_spending_key: Option, + frontend_sus_fee: Vec< + either::Either, + >, ) -> args::TxUnshieldingTransfer { args::TxUnshieldingTransfer { sources, @@ -208,6 +213,7 @@ pub trait Namada: NamadaIo { gas_spending_key, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), + frontend_sus_fee, } } @@ -295,7 +301,9 @@ pub trait Namada: NamadaIo { token: Address, amount: InputAmount, channel_id: ChannelId, - frontend_sus_fee: Option, + frontend_sus_fee: Option< + either::Either, + >, ) -> args::TxIbcTransfer { args::TxIbcTransfer { source, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 82cd16458da..c12a10d9c13 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -76,7 +76,10 @@ pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; -use crate::args::{TxTransparentSource, TxTransparentTarget, Wrapper}; +use crate::args::{ + SdkTypes, TxShieldedSource, TxShieldedTarget, TxTransparentSource, + TxTransparentTarget, Wrapper, +}; use crate::borsh::BorshSerializeExt; use crate::control_flow::time; use crate::error::{EncodingError, Error, QueryError, Result, TxSubmitError}; @@ -2900,12 +2903,20 @@ pub async fn build_ibc_transfer( None }; - if let Some(TxTransparentTarget { - target, - token, - amount, - }) = &args.frontend_sus_fee - { + if let Some(target) = &args.frontend_sus_fee { + let (target, token, amount) = match target { + either::Either::Left(TxTransparentTarget { + target, + token, + amount, + }) => (TransferTarget::Address(target.to_owned()), token, amount), + either::Either::Right(TxShieldedTarget { + target, + token, + amount, + }) => (TransferTarget::PaymentAddress(*target), token, amount), + }; + match (&source, &args.ibc_shielding_data) { (&MASP, None) => { // Validate the amount given @@ -2923,7 +2934,7 @@ pub async fn build_ibc_transfer( validated_amount, )); masp_transfer_data.targets.push(( - TransferTarget::Address(target.to_owned()), + target.clone(), token.to_owned(), validated_amount, )); @@ -2931,7 +2942,7 @@ pub async fn build_ibc_transfer( transfer = transfer .transfer( source.to_owned(), - target.to_owned(), + target.effective_address(), token.to_owned(), validated_amount, ) @@ -3618,7 +3629,7 @@ async fn get_masp_fee_payment_amount( /// Build a shielding transfer pub async fn build_shielding_transfer( context: &N, - args: &mut args::TxShieldingTransfer, + args: &args::TxShieldingTransfer, bparams: &mut impl BuildParams, ) -> Result<(Tx, SigningData, MaspEpoch)> { let source = if args.sources.len() == 1 { @@ -3713,12 +3724,20 @@ pub async fn build_shielding_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } - for args::TxTransparentTarget { - target: sus_fee_target, - token: sus_fee_token, - amount: sus_fee_amt, - } in &args.frontend_sus_fee - { + for (idx, target) in args.frontend_sus_fee.iter().enumerate() { + let (sus_fee_target, sus_fee_token, sus_fee_amt) = match target { + either::Either::Left(TxTransparentTarget { + target, + token, + amount, + }) => (TransferTarget::Address(target.to_owned()), token, amount), + either::Either::Right(TxShieldedTarget { + target, + token, + amount, + }) => (TransferTarget::PaymentAddress(*target), token, amount), + }; + // Validate the amount given let validated_fee_amount = validate_amount( context, @@ -3728,21 +3747,44 @@ pub async fn build_shielding_transfer( ) .await?; - // Decrease the amount to shield to account for this frontend fee, take - // it from the first shielding input that matches the fee token - for (_, token, amount) in &mut transfer_data.sources { - if token == sus_fee_token { - *amount = checked!(amount - validated_fee_amount)?; - break; + // Transfer the frontend fee, take the amount from the source matching + // the index of this fee entry + match &args.sources.get(idx) { + // FIXME: better to remove the token from the fee argument and just + // use the source one + Some(TxTransparentSource { source, token, .. }) + if token == sus_fee_token => + { + data = data + .transfer( + source.to_owned(), + sus_fee_target.effective_address(), + sus_fee_token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; + + if let TransferTarget::PaymentAddress(_) = sus_fee_target { + // FIXME: in this case also need to update the masp sources + // Add the extra shielding target for the masp frontend + // sustainability fee + transfer_data.targets.push(( + sus_fee_target, + sus_fee_token.to_owned(), + validated_fee_amount, + )); + } + } + _ => { + return Err(Error::Other( + "Token mismatch between shielding input and the \ + corresponding frontend masp fee" + .to_string(), + )); } } - data = data - .credit( - sus_fee_target.to_owned(), - sus_fee_token.to_owned(), - validated_fee_amount, - ) - .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } let shielded_parts = construct_shielded_parts( @@ -3875,6 +3917,72 @@ pub async fn build_unshielding_transfer( .debit(MASP, token.to_owned(), validated_amount) .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } + for (idx, target) in args.frontend_sus_fee.iter().enumerate() { + let (sus_fee_target, sus_fee_token, sus_fee_amt) = match target { + either::Either::Left(TxTransparentTarget { + target, + token, + amount, + }) => (TransferTarget::Address(target.to_owned()), token, amount), + either::Either::Right(TxShieldedTarget { + target, + token, + amount, + }) => (TransferTarget::PaymentAddress(*target), token, amount), + }; + + // Validate the amount given + let validated_fee_amount = validate_amount( + context, + sus_fee_amt.to_owned(), + sus_fee_token, + args.tx.force, + ) + .await?; + + // Transfer the frontend fee, take the amount from the source matching + // the index of this fee entry + match &args.sources.get(idx) { + // FIXME: better to remove the token from the fee argument and just + // use the source one + Some(TxShieldedSource { source, token, .. }) + if token == sus_fee_token => + { + let source = TransferSource::ExtendedKey(*source); + + data = data + .transfer( + source.effective_address(), + sus_fee_target.effective_address(), + sus_fee_token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; + + // Add the extra unshielding source and target for the masp + // frontend sustainability fee + transfer_data.sources.push(( + source, + sus_fee_token.to_owned(), + validated_fee_amount, + )); + transfer_data.targets.push(( + sus_fee_target, + sus_fee_token.to_owned(), + validated_fee_amount, + )); + } + _ => { + return Err(Error::Other( + "Token mismatch between shielding input and the \ + corresponding frontend masp fee" + .to_string(), + )); + } + } + } // Add masp fee payment if necessary let masp_fee_data = if let Some(wrap_tx) = &wrap_args { From 3ba8015003e6e0710826d61b1c0a4282117658e3 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 6 Sep 2025 14:37:41 +0200 Subject: [PATCH 09/18] Adds tests for shielded masp frontend fees (cherry picked from commit 8b38f4b810e56053c1a39015866ed2b7a3ca0171) --- crates/apps_lib/src/cli.rs | 215 +++++++++++++++++++-------- crates/sdk/src/tx.rs | 14 +- crates/tests/src/e2e/ibc_tests.rs | 56 ++++++- crates/tests/src/integration/masp.rs | 108 +++++++++++++- 4 files changed, 311 insertions(+), 82 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index aa22ac518ed..48e7ccd3d82 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3693,15 +3693,18 @@ pub mod args { pub const TARGET: Arg = arg("target"); pub const TARGET_OPT: ArgOpt = arg_opt("target"); pub const TEMPLATES_PATH: Arg = arg("templates-path"); - // FIXME: add the test prelude in the cli args too // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI - pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = - arg_opt("frontend-sus-fee"); + pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = + arg_opt("test-frontend-sus-fee"); + // WARNING: use only for testing purposes, MASP frontend fees don't make + // sense when operating from the CLI + pub const __TEST_FRONTEND_SUS_FEE_SHIELDED: ArgOpt = + arg_opt("test-frontend-sus-fee-shielded"); // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI pub const __TEST_FRONTEND_SUS_FEE_IBC: ArgOpt = - arg_opt("frontend-sus-fee-ibc"); + arg_opt("test-frontend-sus-fee-ibc"); pub const TIMEOUT_HEIGHT: ArgOpt = arg_opt("timeout-height"); pub const TIMEOUT_SEC_OFFSET: ArgOpt = arg_opt("timeout-sec-offset"); pub const TM_ADDRESS_OPT: ArgOpt = arg_opt("tm-address"); @@ -5012,9 +5015,9 @@ pub mod args { token: token.clone(), amount, }]; - let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE - .parse(matches) - .map_or(Default::default(), |fee_target| { + let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) + { + Some(address) => { // Take a constant fee of 1 on top of the input amount let amount = InputAmount::Unvalidated( token::DenominatedAmount::new( @@ -5023,19 +5026,33 @@ pub mod args { ), ); - vec![match PaymentAddress::from_str(&fee_target.raw) { - Ok(_) => Either::Right(TxShieldedTarget { - target: fee_target.to_payment_address(), - token: token.clone(), - amount, - }), - Err(_) => Either::Left(TxTransparentTarget { - target: fee_target.to_address(), - token, - amount, - }), - }] - }); + vec![Either::Left(TxTransparentTarget { + target: address, + token, + amount, + })] + } + None => { + __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map_or( + Default::default(), + |payment_address| { + // Take a constant fee of 1 on top of the input + // amount + let amount = InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ); + vec![Either::Right(TxShieldedTarget { + target: payment_address, + token: token.clone(), + amount, + })] + }, + ) + } + }; Self { tx, @@ -5063,10 +5080,26 @@ pub mod args { .def() .help(wrap!("The amount to transfer in decimal.")), ) - .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .help(wrap!( + "The optional transparent address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), + ) + .arg( + __TEST_FRONTEND_SUS_FEE_SHIELDED + .def() + .help(wrap!( + "The optional payment address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), + ) } } @@ -5159,9 +5192,9 @@ pub mod args { }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); - let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE - .parse(matches) - .map_or(Default::default(), |fee_target| { + let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) + { + Some(address) => { // Take a constant fee of 1 on top of the input amount let amount = InputAmount::Unvalidated( token::DenominatedAmount::new( @@ -5170,19 +5203,33 @@ pub mod args { ), ); - vec![match PaymentAddress::from_str(&fee_target.raw) { - Ok(_) => Either::Right(TxShieldedTarget { - target: fee_target.to_payment_address(), - token: token.clone(), - amount, - }), - Err(_) => Either::Left(TxTransparentTarget { - target: fee_target.to_address(), - token, - amount, - }), - }] - }); + vec![Either::Left(TxTransparentTarget { + target: address, + token, + amount, + })] + } + None => { + __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map_or( + Default::default(), + |payment_address| { + // Take a constant fee of 1 on top of the input + // amount + let amount = InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ); + vec![Either::Right(TxShieldedTarget { + target: payment_address, + token: token.clone(), + amount, + })] + }, + ) + } + }; Self { tx, @@ -5217,10 +5264,26 @@ pub mod args { payment. When not provided the source spending key will \ be used." ))) - .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .help(wrap!( + "The optional transparent address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), + ) + .arg( + __TEST_FRONTEND_SUS_FEE_SHIELDED + .def() + .help(wrap!( + "The optional payment address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), + ) } } @@ -5296,8 +5359,9 @@ pub mod args { let ibc_memo = IBC_MEMO.parse(matches); let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); - let frontend_sus_fee = - __TEST_FRONTEND_SUS_FEE.parse(matches).map(|fee_target| { + let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) + { + Some(address) => { // Take a constant fee of 1 on top of the input amount let amount = InputAmount::Unvalidated( token::DenominatedAmount::new( @@ -5305,19 +5369,33 @@ pub mod args { raw_amount.denom(), ), ); - match PaymentAddress::from_str(&fee_target.raw) { - Ok(_) => Either::Right(TxShieldedTarget { - target: fee_target.to_payment_address(), - token: token.clone(), - amount, - }), - Err(_) => Either::Left(TxTransparentTarget { - target: fee_target.to_address(), - token: token.clone(), - amount, - }), - } - }); + + Some(Either::Left(TxTransparentTarget { + target: address, + token: token.clone(), + amount, + })) + } + None => { + __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map( + |payment_address| { + // Take a constant fee of 1 on top of the input + // amount + let amount = InputAmount::Unvalidated( + token::DenominatedAmount::new( + 1.into(), + raw_amount.denom(), + ), + ); + Either::Right(TxShieldedTarget { + target: payment_address, + token: token.clone(), + amount, + }) + }, + ) + } + }; Self { tx, @@ -5387,11 +5465,22 @@ pub mod args { .arg( __TEST_FRONTEND_SUS_FEE .def() - .conflicts_with(IBC_SHIELDING_DATA_PATH.name) .help(wrap!( - "The optional address of the frontend provider \ - that will take the masp sustainability fee." - )), + "The optional transparent address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), + ) + .arg( + __TEST_FRONTEND_SUS_FEE_SHIELDED + .def() + .help(wrap!( + "The optional payment address of the frontend \ + provider that will take the masp sustainability \ + fee." + )) + .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), ) } } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index c12a10d9c13..eb720db9cd9 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -3750,8 +3750,6 @@ pub async fn build_shielding_transfer( // Transfer the frontend fee, take the amount from the source matching // the index of this fee entry match &args.sources.get(idx) { - // FIXME: better to remove the token from the fee argument and just - // use the source one Some(TxTransparentSource { source, token, .. }) if token == sus_fee_token => { @@ -3767,9 +3765,13 @@ pub async fn build_shielding_transfer( ))?; if let TransferTarget::PaymentAddress(_) = sus_fee_target { - // FIXME: in this case also need to update the masp sources - // Add the extra shielding target for the masp frontend - // sustainability fee + // Add the extra shielding source and target for the masp + // frontend sustainability fee + transfer_data.sources.push(( + TransferSource::Address(source.to_owned()), + sus_fee_token.to_owned(), + validated_fee_amount, + )); transfer_data.targets.push(( sus_fee_target, sus_fee_token.to_owned(), @@ -3943,8 +3945,6 @@ pub async fn build_unshielding_transfer( // Transfer the frontend fee, take the amount from the source matching // the index of this fee entry match &args.sources.get(idx) { - // FIXME: better to remove the token from the fee argument and just - // use the source one Some(TxShieldedSource { source, token, .. }) if token == sus_fee_token => { diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 225ffcbc5d1..307518d7d29 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -2788,6 +2788,11 @@ fn transfer_on_chain( Ok(()) } +enum MaspFrontendFee<'alias> { + Transparent(&'alias str), + Shielded(&'alias str), +} + #[allow(clippy::too_many_arguments)] fn transfer( test: &Test, @@ -2803,7 +2808,7 @@ fn transfer( expected_err: Option<&str>, ibc_memo: Option<&str>, gen_refund_target: bool, - frontend_sus_fee: Option<&str>, + frontend_sus_fee: Option, ) -> Result { let rpc = get_actor_rpc(test, Who::Validator(0)); @@ -2872,8 +2877,16 @@ fn transfer( } if let Some(target) = frontend_sus_fee { - tx_args.push("--frontend-sus-fee"); - tx_args.push(target.as_ref()); + match target { + MaspFrontendFee::Transparent(address) => { + tx_args.push("--test-frontend-sus-fee"); + tx_args.push(address); + } + MaspFrontendFee::Shielded(payment_address) => { + tx_args.push("--test-frontend-sus-fee-shielded"); + tx_args.push(payment_address); + } + } } let mut client = run!(test, Bin::Client, tx_args, Some(300))?; @@ -3523,7 +3536,7 @@ fn gen_ibc_shielding_data( &rpc, ]; if let Some(target) = frontend_sus_fee { - args.push("--frontend-sus-fee-ibc"); + args.push("--test-frontend-sus-fee-ibc"); args.push(target.as_ref()); } @@ -3921,7 +3934,7 @@ fn frontend_sus_fee() -> Result<()> { check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 899)?; // Unshielding transfer 10 samoleans from Namada to Gaia with transparent - // frontend fee FIXME: need to test also the transparent version + // frontend fee let gaia_receiver = find_cosmos_address(&test_gaia, COSMOS_USER)?; transfer( &test, @@ -3938,7 +3951,7 @@ fn frontend_sus_fee() -> Result<()> { None, None, true, - Some(ESTER), + Some(MaspFrontendFee::Transparent(ESTER)), )?; wait_for_packet_relay( &hermes_dir, @@ -3951,6 +3964,37 @@ fn frontend_sus_fee() -> Result<()> { check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 909)?; + // Unshielding transfer 10 samoleans from Namada to Gaia with shielded + // frontend fee + let gaia_receiver = find_cosmos_address(&test_gaia, COSMOS_USER)?; + transfer( + &test, + A_SPENDING_KEY, + &gaia_receiver, + &ibc_denom_on_namada, + // An extra token will be added to this amount as a frontend masp fee + 10, + Some(BERTHA_KEY), + &port_id_namada, + &channel_id_namada, + None, + None, + None, + None, + true, + Some(MaspFrontendFee::Shielded(AC_PAYMENT_ADDRESS)), + )?; + wait_for_packet_relay( + &hermes_dir, + &port_id_namada, + &channel_id_namada, + &test, + )?; + check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 78)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 2)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 919)?; + Ok(()) } diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index b750343ebb4..967eea43bef 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -10206,7 +10206,7 @@ fn frontend_sus_fee() -> Result<()> { NAM, "--amount", "10", - "--frontend-sus-fee", + "--test-frontend-sus-fee", frontend_alias, "--signing-keys", ALBERT_KEY, @@ -10218,6 +10218,34 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); + // Test sus fee when shielding. Send 10 NAM from Albert to PA and 1 NAM to a + // shielded address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "10", + "--test-frontend-sus-fee-shielded", + AC_PAYMENT_ADDRESS, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + // sync the shielded context run( &node, @@ -10226,12 +10254,13 @@ fn frontend_sus_fee() -> Result<()> { "shielded-sync", "--viewing-keys", AA_VIEWING_KEY, + AC_VIEWING_KEY, "--node", validator_one_rpc, ], )?; - // Assert NAM balance at VK(A) is 10 + // Assert NAM balance at VK(A) is 20 let captured = CapturedOutput::of(|| { run( &node, @@ -10248,9 +10277,9 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 10")); + assert!(captured.contains("nam: 20")); - // Assert NAM balance at the frontend is 1 + // Assert NAM balance at the transparent frontend is 1 let captured = CapturedOutput::of(|| { run( &node, @@ -10269,6 +10298,25 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 1")); + // Assert NAM balance at the shielded frontend is 1 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AC_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 1")); + // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 1 NAM to // a transparent address owned by the frontend provider let captured = CapturedOutput::of(|| { @@ -10285,7 +10333,7 @@ fn frontend_sus_fee() -> Result<()> { NAM, "--amount", "9", - "--frontend-sus-fee", + "--test-frontend-sus-fee", frontend_alias, "--signing-keys", ALBERT_KEY, @@ -10297,6 +10345,34 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); + // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 1 NAM to + // a shielded address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "9", + "--test-frontend-sus-fee-shielded", + AC_PAYMENT_ADDRESS, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + // sync the shielded context run( &node, @@ -10305,6 +10381,7 @@ fn frontend_sus_fee() -> Result<()> { "shielded-sync", "--viewing-keys", AA_VIEWING_KEY, + AC_VIEWING_KEY, "--node", validator_one_rpc, ], @@ -10329,7 +10406,7 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 0")); - // Assert NAM balance at the frontend is 2 + // Assert NAM balance at the transparent frontend is 2 let captured = CapturedOutput::of(|| { run( &node, @@ -10348,5 +10425,24 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 2")); + // Assert NAM balance at the shielded frontend is 2 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AC_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 2")); + Ok(()) } From 33ebd28f232d217f1dd9348ce63a85948c5d1ecc Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 6 Sep 2025 14:49:26 +0200 Subject: [PATCH 10/18] Forces ibc shielding target argument to be a payment address (cherry picked from commit 3cacd32174603d6c1416b2f89f2ba65a93c053b8) --- crates/apps_lib/src/cli.rs | 8 ++++++-- crates/sdk/src/args.rs | 8 ++------ crates/sdk/src/tx.rs | 6 +++++- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 48e7ccd3d82..aeb30813314 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -7509,7 +7509,7 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let query = Query::parse(matches); let output_folder = OUTPUT_FOLDER_PATH.parse(matches); - let target = TRANSFER_TARGET.parse(matches); + let target = PAYMENT_ADDRESS_TARGET.parse(matches); let token = TOKEN_STR.parse(matches); let raw_amount = AMOUNT.parse(matches); let amount = InputAmount::Unvalidated(raw_amount); @@ -7560,7 +7560,11 @@ pub mod args { .arg(OUTPUT_FOLDER_PATH.def().help(wrap!( "The output folder path where the artifact will be stored." ))) - .arg(TRANSFER_TARGET.def().help(wrap!("The target address."))) + .arg( + PAYMENT_ADDRESS_TARGET + .def() + .help(wrap!("The shielded target account address.")), + ) .arg(TOKEN.def().help(wrap!("The transfer token."))) .arg( AMOUNT diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index ee4ace33630..5b079b6b523 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -710,10 +710,7 @@ impl TxOsmosisSwap { ledger_address: transfer.tx.ledger_address.clone(), }, output_folder: None, - target: - namada_core::masp::TransferTarget::PaymentAddress( - payment_addr, - ), + target: payment_addr, asset: IbcShieldingTransferAsset::Address( namada_output_addr, ), @@ -3227,8 +3224,7 @@ pub struct GenIbcShieldingTransfer { /// The output directory path to where serialize the data pub output_folder: Option, /// The target address - // FIXME: why isn't this a payment address? - pub target: C::TransferTarget, + pub target: C::PaymentAddress, /// Transferred token amount pub amount: InputAmount, /// The optional expiration of the masp shielding transaction diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index eb720db9cd9..528d0eb8a8c 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -4561,7 +4561,11 @@ pub async fn gen_ibc_shielding_transfer( )], targets: [ extra_target, - vec![(args.target, token.clone(), validated_amount)], + vec![( + TransferTarget::PaymentAddress(args.target), + token.clone(), + validated_amount, + )], ] .concat(), }; From 7cdd61379af8d50158a95d7e523e711a01434aa9 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sun, 7 Sep 2025 14:01:58 +0200 Subject: [PATCH 11/18] Frontend MASP fee argument as a percentage (cherry picked from commit ce7150d9305c7bdb86670cdc4e6ef8d0dec6a809) --- crates/apps_lib/src/cli.rs | 301 ++++------------------- crates/sdk/src/args.rs | 15 +- crates/sdk/src/lib.rs | 12 +- crates/sdk/src/tx.rs | 353 ++++++++++++++++----------- crates/tests/src/e2e/ibc_tests.rs | 41 ++-- crates/tests/src/integration/masp.rs | 21 +- 6 files changed, 286 insertions(+), 457 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index aeb30813314..265e91e6e5a 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3695,14 +3695,10 @@ pub mod args { pub const TEMPLATES_PATH: Arg = arg("templates-path"); // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI - pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = + pub const __TEST_FRONTEND_SUS_FEE: ArgOpt = arg_opt("test-frontend-sus-fee"); // WARNING: use only for testing purposes, MASP frontend fees don't make // sense when operating from the CLI - pub const __TEST_FRONTEND_SUS_FEE_SHIELDED: ArgOpt = - arg_opt("test-frontend-sus-fee-shielded"); - // WARNING: use only for testing purposes, MASP frontend fees don't make - // sense when operating from the CLI pub const __TEST_FRONTEND_SUS_FEE_IBC: ArgOpt = arg_opt("test-frontend-sus-fee-ibc"); pub const TIMEOUT_HEIGHT: ArgOpt = arg_opt("timeout-height"); @@ -4961,30 +4957,10 @@ pub mod args { amount: transfer_data.amount, }); } - let mut frontend_sus_fee = vec![]; - for fee in self.frontend_sus_fee { - match fee { - Either::Left(transparent_target) => { - frontend_sus_fee.push(Either::Left( - TxTransparentTarget { - target: chain_ctx - .get(&transparent_target.target), - token: chain_ctx.get(&transparent_target.token), - amount: transparent_target.amount, - }, - )); - } - Either::Right(shielded_target) => { - frontend_sus_fee.push(Either::Right( - TxShieldedTarget { - target: chain_ctx.get(&shielded_target.target), - token: chain_ctx.get(&shielded_target.token), - amount: shielded_target.amount, - }, - )); - } - } - } + let frontend_sus_fee = + self.frontend_sus_fee.map(|(target, percentage)| { + (chain_ctx.get(&target), percentage) + }); Ok(TxShieldingTransfer:: { tx, @@ -5015,44 +4991,11 @@ pub mod args { token: token.clone(), amount, }]; - let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) - { - Some(address) => { - // Take a constant fee of 1 on top of the input amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - - vec![Either::Left(TxTransparentTarget { - target: address, - token, - amount, - })] - } - None => { - __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map_or( - Default::default(), - |payment_address| { - // Take a constant fee of 1 on top of the input - // amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - vec![Either::Right(TxShieldedTarget { - target: payment_address, - token: token.clone(), - amount, - })] - }, - ) - } - }; + let frontend_sus_fee = + __TEST_FRONTEND_SUS_FEE.parse(matches).map(|target| { + // Take a constant fee of 10% on top of the input amount + (target, Dec::new(1, 1).unwrap()) + }); Self { tx, @@ -5080,26 +5023,10 @@ pub mod args { .def() .help(wrap!("The amount to transfer in decimal.")), ) - .arg( - __TEST_FRONTEND_SUS_FEE - .def() - .help(wrap!( - "The optional transparent address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), - ) - .arg( - __TEST_FRONTEND_SUS_FEE_SHIELDED - .def() - .help(wrap!( - "The optional payment address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), - ) + .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } @@ -5133,30 +5060,11 @@ pub mod args { }); } - let mut frontend_sus_fee = vec![]; - for fee in self.frontend_sus_fee { - match fee { - Either::Left(transparent_target) => { - frontend_sus_fee.push(Either::Left( - TxTransparentTarget { - target: chain_ctx - .get(&transparent_target.target), - token: chain_ctx.get(&transparent_target.token), - amount: transparent_target.amount, - }, - )); - } - Either::Right(shielded_target) => { - frontend_sus_fee.push(Either::Right( - TxShieldedTarget { - target: chain_ctx.get(&shielded_target.target), - token: chain_ctx.get(&shielded_target.token), - amount: shielded_target.amount, - }, - )); - } - } - } + let frontend_sus_fee = + self.frontend_sus_fee.map(|(target, percentage)| { + (chain_ctx.get(&target), percentage) + }); + let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); @@ -5192,44 +5100,11 @@ pub mod args { }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); - let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) - { - Some(address) => { - // Take a constant fee of 1 on top of the input amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - - vec![Either::Left(TxTransparentTarget { - target: address, - token, - amount, - })] - } - None => { - __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map_or( - Default::default(), - |payment_address| { - // Take a constant fee of 1 on top of the input - // amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - vec![Either::Right(TxShieldedTarget { - target: payment_address, - token: token.clone(), - amount, - })] - }, - ) - } - }; + let frontend_sus_fee = + __TEST_FRONTEND_SUS_FEE.parse(matches).map(|target| { + // Take a constant fee of 10% on top of the input amount + (target, Dec::new(1, 1).unwrap()) + }); Self { tx, @@ -5264,26 +5139,10 @@ pub mod args { payment. When not provided the source spending key will \ be used." ))) - .arg( - __TEST_FRONTEND_SUS_FEE - .def() - .help(wrap!( - "The optional transparent address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), - ) - .arg( - __TEST_FRONTEND_SUS_FEE_SHIELDED - .def() - .help(wrap!( - "The optional payment address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), - ) + .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } @@ -5298,22 +5157,10 @@ pub mod args { let chain_ctx = ctx.borrow_mut_chain_or_exit(); let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); - let frontend_sus_fee = self.frontend_sus_fee.map(|fee| match fee { - Either::Left(transparent_target) => { - Either::Left(TxTransparentTarget { - target: chain_ctx.get(&transparent_target.target), - token: chain_ctx.get(&transparent_target.token), - amount: transparent_target.amount, - }) - } - Either::Right(shielded_target) => { - Either::Right(TxShieldedTarget { - target: chain_ctx.get(&shielded_target.target), - token: chain_ctx.get(&shielded_target.token), - amount: shielded_target.amount, - }) - } - }); + let frontend_sus_fee = + self.frontend_sus_fee.map(|(target, percentage)| { + (chain_ctx.get(&target), percentage) + }); Ok(TxIbcTransfer:: { tx, @@ -5359,43 +5206,11 @@ pub mod args { let ibc_memo = IBC_MEMO.parse(matches); let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); - let frontend_sus_fee = match __TEST_FRONTEND_SUS_FEE.parse(matches) - { - Some(address) => { - // Take a constant fee of 1 on top of the input amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - - Some(Either::Left(TxTransparentTarget { - target: address, - token: token.clone(), - amount, - })) - } - None => { - __TEST_FRONTEND_SUS_FEE_SHIELDED.parse(matches).map( - |payment_address| { - // Take a constant fee of 1 on top of the input - // amount - let amount = InputAmount::Unvalidated( - token::DenominatedAmount::new( - 1.into(), - raw_amount.denom(), - ), - ); - Either::Right(TxShieldedTarget { - target: payment_address, - token: token.clone(), - amount, - }) - }, - ) - } - }; + let frontend_sus_fee = + __TEST_FRONTEND_SUS_FEE.parse(matches).map(|target| { + // Take a constant fee of 10% on top of the input amount + (target, Dec::new(1, 1).unwrap()) + }); Self { tx, @@ -5462,26 +5277,10 @@ pub mod args { payment (if this is a shielded action). When not \ provided the source spending key will be used." ))) - .arg( - __TEST_FRONTEND_SUS_FEE - .def() - .help(wrap!( - "The optional transparent address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE_SHIELDED.name), - ) - .arg( - __TEST_FRONTEND_SUS_FEE_SHIELDED - .def() - .help(wrap!( - "The optional payment address of the frontend \ - provider that will take the masp sustainability \ - fee." - )) - .conflicts_with(__TEST_FRONTEND_SUS_FEE.name), - ) + .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( + "The optional address of the frontend provider that will \ + take the masp sustainability fee." + ))) } } @@ -7525,19 +7324,11 @@ pub mod args { None => TxExpiration::Default, } }; - let frontend_sus_fee = - __TEST_FRONTEND_SUS_FEE_IBC.parse(matches).map(|target| { - ( - target, - InputAmount::Unvalidated( - token::DenominatedAmount::new( - // Take a constant fee of 1 on top of the input - // amount - 1.into(), - raw_amount.denom(), - ), - ), - ) + let frontend_sus_fee = __TEST_FRONTEND_SUS_FEE_IBC + .parse(matches) + .map(|payment_address| { + // Take a constant fee of 10% on top of the input amount + (payment_address, Dec::new(1, 1).unwrap()) }); Self { diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 5b079b6b523..d70e035bc58 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -388,10 +388,7 @@ pub struct TxShieldingTransfer { /// Transfer source data pub sources: Vec>, /// The optional data for the frontend sustainability fee - // FIXME: should join this with sources? No maybe better to define a single - // percentage to apply to all inputs and a single receiver - pub frontend_sus_fee: - Vec, TxShieldedTarget>>, + pub frontend_sus_fee: Option<(C::TransferTarget, Dec)>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -442,8 +439,7 @@ pub struct TxUnshieldingTransfer { /// Optional additional key for gas payment pub gas_spending_key: Option, /// The optional data for the frontend sustainability fee - pub frontend_sus_fee: - Vec, TxShieldedTarget>>, + pub frontend_sus_fee: Option<(C::TransferTarget, Dec)>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -545,7 +541,7 @@ pub struct TxOsmosisSwap { /// The optional data for the frontend sustainability fee /// NOTE: if the swap is shielded (from MASP to MASP), no sustainability /// fee should be taken - pub frontend_sus_fee: Option<(C::PaymentAddress, InputAmount)>, + pub frontend_sus_fee: Option<(C::PaymentAddress, Dec)>, } impl TxOsmosisSwap { @@ -822,8 +818,7 @@ pub struct TxIbcTransfer { /// Optional additional keys for gas payment pub gas_spending_key: Option, /// The optional data for the frontend sustainability fee - pub frontend_sus_fee: - Option, TxShieldedTarget>>, + pub frontend_sus_fee: Option<(C::TransferTarget, Dec)>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -3236,7 +3231,7 @@ pub struct GenIbcShieldingTransfer { /// shielding transaction since ics-20 only supports a single asset) /// NOTE: if the shielding operation is part of a swap, and this is /// shielded (from MASP to MASP), no sustainability fee should be taken - pub frontend_sus_fee: Option<(C::PaymentAddress, InputAmount)>, + pub frontend_sus_fee: Option<(C::PaymentAddress, Dec)>, } /// IBC shielding transfer asset, to be used by [`GenIbcShieldingTransfer`] diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 2ff7784891f..677f0bdffe3 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -183,9 +183,7 @@ pub trait Namada: NamadaIo { &self, targets: Vec, sources: Vec, - frontend_sus_fee: Vec< - either::Either, - >, + frontend_sus_fee: Option<(TransferTarget, Dec)>, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { sources, @@ -203,9 +201,7 @@ pub trait Namada: NamadaIo { sources: Vec, targets: Vec, gas_spending_key: Option, - frontend_sus_fee: Vec< - either::Either, - >, + frontend_sus_fee: Option<(TransferTarget, Dec)>, ) -> args::TxUnshieldingTransfer { args::TxUnshieldingTransfer { sources, @@ -301,9 +297,7 @@ pub trait Namada: NamadaIo { token: Address, amount: InputAmount, channel_id: ChannelId, - frontend_sus_fee: Option< - either::Either, - >, + frontend_sus_fee: Option<(TransferTarget, Dec)>, ) -> args::TxIbcTransfer { args::TxIbcTransfer { source, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 528d0eb8a8c..e9f35b83216 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -27,7 +27,7 @@ use namada_core::address::{Address, IBC, MASP}; use namada_core::arith::checked; use namada_core::chain::Epoch; use namada_core::collections::HashSet; -use namada_core::dec::Dec; +use namada_core::dec::{Dec, POS_DECIMAL_PRECISION}; use namada_core::hash::Hash; use namada_core::ibc::apps::nft_transfer::types::PrefixedClassId; use namada_core::ibc::apps::nft_transfer::types::msgs::transfer::MsgTransfer as IbcMsgNftTransfer; @@ -76,10 +76,7 @@ pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; -use crate::args::{ - SdkTypes, TxShieldedSource, TxShieldedTarget, TxTransparentSource, - TxTransparentTarget, Wrapper, -}; +use crate::args::{TxTransparentSource, TxTransparentTarget, Wrapper}; use crate::borsh::BorshSerializeExt; use crate::control_flow::time; use crate::error::{EncodingError, Error, QueryError, Result, TxSubmitError}; @@ -2903,27 +2900,39 @@ pub async fn build_ibc_transfer( None }; - if let Some(target) = &args.frontend_sus_fee { - let (target, token, amount) = match target { - either::Either::Left(TxTransparentTarget { - target, - token, - amount, - }) => (TransferTarget::Address(target.to_owned()), token, amount), - either::Either::Right(TxShieldedTarget { - target, - token, - amount, - }) => (TransferTarget::PaymentAddress(*target), token, amount), - }; - + if let Some((target, percentage)) = &args.frontend_sus_fee { match (&source, &args.ibc_shielding_data) { (&MASP, None) => { + // NOTE: The frontend fee should NOT account for the masp fee + // payment amount + let sus_fee_amt = namada_token::Amount::from_uint( + validated_amount + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10( + POS_DECIMAL_PRECISION as _, + ), + ) + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation" + .to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; // Validate the amount given - let validated_amount = validate_amount( + let validated_fee_amount = validate_amount( context, - amount.to_owned(), - token, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + validated_amount.denom(), + )), + &args.token, args.tx.force, ) .await?; @@ -2931,20 +2940,20 @@ pub async fn build_ibc_transfer( masp_transfer_data.sources.push(( args.source.clone(), args.token.clone(), - validated_amount, + validated_fee_amount, )); masp_transfer_data.targets.push(( target.clone(), - token.to_owned(), - validated_amount, + args.token.to_owned(), + validated_fee_amount, )); transfer = transfer .transfer( source.to_owned(), target.effective_address(), - token.to_owned(), - validated_amount, + args.token.to_owned(), + validated_fee_amount, ) .ok_or(Error::Other( "Combined transfer overflows".to_string(), @@ -3724,69 +3733,75 @@ pub async fn build_shielding_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } - for (idx, target) in args.frontend_sus_fee.iter().enumerate() { - let (sus_fee_target, sus_fee_token, sus_fee_amt) = match target { - either::Either::Left(TxTransparentTarget { - target, - token, - amount, - }) => (TransferTarget::Address(target.to_owned()), token, amount), - either::Either::Right(TxShieldedTarget { - target, - token, - amount, - }) => (TransferTarget::PaymentAddress(*target), token, amount), - }; - - // Validate the amount given - let validated_fee_amount = validate_amount( - context, - sus_fee_amt.to_owned(), - sus_fee_token, - args.tx.force, - ) - .await?; - - // Transfer the frontend fee, take the amount from the source matching - // the index of this fee entry - match &args.sources.get(idx) { - Some(TxTransparentSource { source, token, .. }) - if token == sus_fee_token => - { - data = data - .transfer( - source.to_owned(), - sus_fee_target.effective_address(), - sus_fee_token.to_owned(), - validated_fee_amount, + if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { + let mut extra_transfer: Vec<( + Address, + Address, + Address, + DenominatedAmount, + )> = Default::default(); + // Transfer the frontend fee, take the fee percentage from every source + for (account, denominated_amt) in &data.sources { + let sus_fee_amt = namada_token::Amount::from_uint( + denominated_amt + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10( + POS_DECIMAL_PRECISION as _, + ), ) - .ok_or(Error::Other( - "Combined transfer overflows".to_string(), - ))?; + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation" + .to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; + // Validate the amount given + let validated_fee_amount = validate_amount( + context, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + denominated_amt.denom(), + )), + &account.token, + args.tx.force, + ) + .await?; + extra_transfer.push(( + account.owner.to_owned(), + sus_fee_target.effective_address(), + account.token.to_owned(), + validated_fee_amount, + )); - if let TransferTarget::PaymentAddress(_) = sus_fee_target { - // Add the extra shielding source and target for the masp - // frontend sustainability fee - transfer_data.sources.push(( - TransferSource::Address(source.to_owned()), - sus_fee_token.to_owned(), - validated_fee_amount, - )); - transfer_data.targets.push(( - sus_fee_target, - sus_fee_token.to_owned(), - validated_fee_amount, - )); - } - } - _ => { - return Err(Error::Other( - "Token mismatch between shielding input and the \ - corresponding frontend masp fee" - .to_string(), + if sus_fee_target.payment_address().is_some() { + // Add the extra shielding source and target for the masp + // frontend sustainability fee + transfer_data.sources.push(( + TransferSource::Address(account.owner.to_owned()), + account.token.to_owned(), + validated_fee_amount, + )); + transfer_data.targets.push(( + sus_fee_target.to_owned(), + account.token.to_owned(), + validated_fee_amount, )); } } + + // Merge the extra transfer data with the default one + for (source, target, token, amount) in extra_transfer { + data = data.transfer(source, target, token, amount).ok_or( + Error::Other("Combined transfer overflows".to_string()), + )?; + } } let shielded_parts = construct_shielded_parts( @@ -3919,68 +3934,85 @@ pub async fn build_unshielding_transfer( .debit(MASP, token.to_owned(), validated_amount) .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } - for (idx, target) in args.frontend_sus_fee.iter().enumerate() { - let (sus_fee_target, sus_fee_token, sus_fee_amt) = match target { - either::Either::Left(TxTransparentTarget { - target, - token, - amount, - }) => (TransferTarget::Address(target.to_owned()), token, amount), - either::Either::Right(TxShieldedTarget { - target, - token, - amount, - }) => (TransferTarget::PaymentAddress(*target), token, amount), - }; - - // Validate the amount given - let validated_fee_amount = validate_amount( - context, - sus_fee_amt.to_owned(), - sus_fee_token, - args.tx.force, - ) - .await?; - // Transfer the frontend fee, take the amount from the source matching - // the index of this fee entry - match &args.sources.get(idx) { - Some(TxShieldedSource { source, token, .. }) - if token == sus_fee_token => - { - let source = TransferSource::ExtendedKey(*source); - - data = data - .transfer( - source.effective_address(), - sus_fee_target.effective_address(), - sus_fee_token.to_owned(), - validated_fee_amount, + if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { + let mut extra_transfer: Vec<( + TransferSource, + Address, + DenominatedAmount, + )> = Default::default(); + // Transfer the frontend fee, take the fee percentage from every source + // FIXME: wrong this should be computed on the targets. Actually maybe + // not if all the sources are unshielded which might be the case + // FIXME: should we loop on the masp transfer data instead also in the + // other builders? What if we add dummy notes? It doesn't matter since + // we will produce them later not here + // FIXME: actually, we should probably loop on the args.sources, same in + // the other builders + for (source, token, denominated_amt) in &transfer_data.sources { + // NOTE: The frontend fee should NOT account for the masp fee + // payment amount + let sus_fee_amt = namada_token::Amount::from_uint( + denominated_amt + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10( + POS_DECIMAL_PRECISION as _, + ), ) - .ok_or(Error::Other( - "Combined transfer overflows".to_string(), - ))?; + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation" + .to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; + // Validate the amount given + let validated_fee_amount = validate_amount( + context, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + denominated_amt.denom(), + )), + token, + args.tx.force, + ) + .await?; + extra_transfer.push(( + source.to_owned(), + token.to_owned(), + validated_fee_amount, + )); - // Add the extra unshielding source and target for the masp - // frontend sustainability fee - transfer_data.sources.push(( - source, - sus_fee_token.to_owned(), - validated_fee_amount, - )); - transfer_data.targets.push(( - sus_fee_target, - sus_fee_token.to_owned(), + data = data + .transfer( + source.effective_address(), + sus_fee_target.effective_address(), + token.to_owned(), validated_fee_amount, - )); - } - _ => { - return Err(Error::Other( - "Token mismatch between shielding input and the \ - corresponding frontend masp fee" - .to_string(), - )); - } + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; + + // Add the extra shielding source and target for the masp + // frontend sustainability fee + transfer_data.targets.push(( + sus_fee_target.to_owned(), + token.to_owned(), + validated_fee_amount, + )); + } + + // Merge the extra shielding source for the masp frontend sustainability + // fee + for (source, token, amount) in extra_transfer { + transfer_data.sources.push((source, token, amount)); } } @@ -4533,11 +4565,38 @@ pub async fn gen_ibc_shielding_transfer( .await; let (extra_target, source_amount) = match &args.frontend_sus_fee { - Some((target, amount)) => { + Some((target, percentage)) => { + let sus_fee_amt = namada_token::Amount::from_uint( + validated_amount + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10( + POS_DECIMAL_PRECISION as _, + ), + ) + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation" + .to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; // Validate the amount given - let validated_fee_amount = - validate_amount(context, amount.to_owned(), &token, false) - .await?; + let validated_fee_amount = validate_amount( + context, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + validated_amount.denom(), + )), + &token, + false, + ) + .await?; let source_amount = checked!(validated_amount + validated_fee_amount)?; diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 307518d7d29..21b94dc99a3 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -2788,11 +2788,6 @@ fn transfer_on_chain( Ok(()) } -enum MaspFrontendFee<'alias> { - Transparent(&'alias str), - Shielded(&'alias str), -} - #[allow(clippy::too_many_arguments)] fn transfer( test: &Test, @@ -2808,7 +2803,7 @@ fn transfer( expected_err: Option<&str>, ibc_memo: Option<&str>, gen_refund_target: bool, - frontend_sus_fee: Option, + frontend_sus_fee: Option<&str>, ) -> Result { let rpc = get_actor_rpc(test, Who::Validator(0)); @@ -2877,16 +2872,8 @@ fn transfer( } if let Some(target) = frontend_sus_fee { - match target { - MaspFrontendFee::Transparent(address) => { - tx_args.push("--test-frontend-sus-fee"); - tx_args.push(address); - } - MaspFrontendFee::Shielded(payment_address) => { - tx_args.push("--test-frontend-sus-fee-shielded"); - tx_args.push(payment_address); - } - } + tx_args.push("--test-frontend-sus-fee"); + tx_args.push(target); } let mut client = run!(test, Bin::Client, tx_args, Some(300))?; @@ -3913,8 +3900,8 @@ fn frontend_sus_fee() -> Result<()> { COSMOS_USER, MASP.to_string(), COSMOS_COIN, - // 1 extra token for the frontend sus fee - 101, + // 10 extra tokens for the frontend sus fee + 110, &port_id_gaia, &channel_id_gaia, Some(Either::Left(shielding_data_path)), @@ -3930,8 +3917,9 @@ fn frontend_sus_fee() -> Result<()> { let ibc_denom_on_namada = format!("{port_id_namada}/{channel_id_namada}/{COSMOS_COIN}"); check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 100)?; - check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 1)?; - check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 899)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 10)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 0)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 890)?; // Unshielding transfer 10 samoleans from Namada to Gaia with transparent // frontend fee @@ -3951,7 +3939,7 @@ fn frontend_sus_fee() -> Result<()> { None, None, true, - Some(MaspFrontendFee::Transparent(ESTER)), + Some(ESTER), )?; wait_for_packet_relay( &hermes_dir, @@ -3960,13 +3948,12 @@ fn frontend_sus_fee() -> Result<()> { &test, )?; check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 89)?; - check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 1)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 10)?; check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; - check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 909)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 900)?; // Unshielding transfer 10 samoleans from Namada to Gaia with shielded // frontend fee - let gaia_receiver = find_cosmos_address(&test_gaia, COSMOS_USER)?; transfer( &test, A_SPENDING_KEY, @@ -3982,7 +3969,7 @@ fn frontend_sus_fee() -> Result<()> { None, None, true, - Some(MaspFrontendFee::Shielded(AC_PAYMENT_ADDRESS)), + Some(AC_PAYMENT_ADDRESS), )?; wait_for_packet_relay( &hermes_dir, @@ -3991,9 +3978,9 @@ fn frontend_sus_fee() -> Result<()> { &test, )?; check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 78)?; - check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 2)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 11)?; check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; - check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 919)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 910)?; Ok(()) } diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 967eea43bef..11884de6b40 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -10234,7 +10234,7 @@ fn frontend_sus_fee() -> Result<()> { NAM, "--amount", "10", - "--test-frontend-sus-fee-shielded", + "--test-frontend-sus-fee", AC_PAYMENT_ADDRESS, "--signing-keys", ALBERT_KEY, @@ -10317,8 +10317,11 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 1")); - // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 1 NAM to - // a transparent address owned by the frontend provider + // FIXME: add a test with a non-native token + // FIXME: add test with masp fee unshielding (also for the ibc case) and + // check that the amount unshielded for fees is not subject to this frontend + // fee Test sus fee when unshielding. Send 9 NAM from PA to Albert and + // 0.9 NAM to a transparent address owned by the frontend provider let captured = CapturedOutput::of(|| { run( &node, @@ -10345,8 +10348,8 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 1 NAM to - // a shielded address owned by the frontend provider + // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 0.9 NAM + // to a shielded address owned by the frontend provider let captured = CapturedOutput::of(|| { run( &node, @@ -10361,7 +10364,7 @@ fn frontend_sus_fee() -> Result<()> { NAM, "--amount", "9", - "--test-frontend-sus-fee-shielded", + "--test-frontend-sus-fee", AC_PAYMENT_ADDRESS, "--signing-keys", ALBERT_KEY, @@ -10404,7 +10407,7 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0")); + assert!(captured.contains("nam: 0.2")); // Assert NAM balance at the transparent frontend is 2 let captured = CapturedOutput::of(|| { @@ -10423,7 +10426,7 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 2")); + assert!(captured.contains("nam: 1.9")); // Assert NAM balance at the shielded frontend is 2 let captured = CapturedOutput::of(|| { @@ -10442,7 +10445,7 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 2")); + assert!(captured.contains("nam: 1.9")); Ok(()) } From 24ba5ebf718477b95a457208e220877b94731d01 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 8 Sep 2025 12:09:03 +0200 Subject: [PATCH 12/18] More masp frontened fee tests (cherry picked from commit 718b77e6f2e6230e9584c444294a30da9321b492) --- crates/tests/src/e2e/ibc_tests.rs | 75 +++++ crates/tests/src/integration/masp.rs | 439 ++++++++++++++++++--------- 2 files changed, 363 insertions(+), 151 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 21b94dc99a3..24a6651a50c 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -147,6 +147,7 @@ fn ibc_transfers() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -226,6 +227,7 @@ fn ibc_transfers() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -309,6 +311,7 @@ fn ibc_transfers() -> Result<()> { None, true, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -364,6 +367,7 @@ fn ibc_transfers() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -394,6 +398,7 @@ fn ibc_transfers() -> Result<()> { None, false, None, + None, )?; // wait for the timeout sleep(10); @@ -428,6 +433,7 @@ fn ibc_transfers() -> Result<()> { None, true, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -460,6 +466,7 @@ fn ibc_transfers() -> Result<()> { None, true, None, + None, )?; // wait for the timeout sleep(10); @@ -621,6 +628,7 @@ fn ibc_nft_transfers() -> Result<()> { None, false, None, + None, )?; clear_packet(&hermes_dir, &port_id_namada, &channel_id_namada, &test)?; check_balance(&test, &namada_receiver, &ibc_trace_on_namada, 0)?; @@ -685,6 +693,7 @@ fn ibc_nft_transfers() -> Result<()> { None, true, None, + None, )?; clear_packet(&hermes_dir, &port_id_namada, &channel_id_namada, &test)?; check_shielded_balance(&test, AB_VIEWING_KEY, &ibc_trace_on_namada, 0)?; @@ -1165,6 +1174,7 @@ fn ibc_rate_limit() -> Result<()> { None, false, None, + None, )?; // Transfer 1 NAM from Namada to Gaia again will fail @@ -1186,6 +1196,7 @@ fn ibc_rate_limit() -> Result<()> { None, false, None, + None, )?; // wait for the next epoch @@ -1211,6 +1222,7 @@ fn ibc_rate_limit() -> Result<()> { None, false, None, + None, )?; // wait for the next epoch @@ -1322,6 +1334,7 @@ fn ibc_unlimited_channel() -> Result<()> { None, false, None, + None, )?; // Proposal on Namada @@ -1381,6 +1394,7 @@ fn ibc_unlimited_channel() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -1713,6 +1727,7 @@ fn ibc_pfm_happy_flows() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( @@ -2011,6 +2026,7 @@ fn ibc_pfm_unhappy_flows() -> Result<()> { None, false, None, + None, )?; wait_for_packet_relay( @@ -2376,6 +2392,7 @@ fn ibc_shielded_recv_middleware_happy_flow() -> Result<()> { Some(&memo), true, None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -2470,6 +2487,7 @@ fn ibc_shielded_recv_middleware_unhappy_flow() -> Result<()> { Some(&memo), false, None, + None, )?; wait_for_packet_relay(&hermes_dir, &port_id_gaia, &channel_id_gaia, &test)?; @@ -2729,6 +2747,7 @@ fn try_invalid_transfers( None, false, None, + None, )?; // invalid channel @@ -2747,6 +2766,7 @@ fn try_invalid_transfers( None, false, None, + None, )?; Ok(()) @@ -2804,6 +2824,7 @@ fn transfer( ibc_memo: Option<&str>, gen_refund_target: bool, frontend_sus_fee: Option<&str>, + gas_token: Option<&str>, ) -> Result { let rpc = get_actor_rpc(test, Who::Validator(0)); @@ -2830,6 +2851,10 @@ fn transfer( &rpc, ]); + if let Some(token) = gas_token { + tx_args.extend_from_slice(&["--gas-token", token]); + } + if let Some(ibc_memo) = ibc_memo { tx_args.extend_from_slice(&["--ibc-memo", ibc_memo]); } @@ -3940,6 +3965,7 @@ fn frontend_sus_fee() -> Result<()> { None, true, Some(ESTER), + None, )?; wait_for_packet_relay( &hermes_dir, @@ -3970,6 +3996,7 @@ fn frontend_sus_fee() -> Result<()> { None, true, Some(AC_PAYMENT_ADDRESS), + None, )?; wait_for_packet_relay( &hermes_dir, @@ -3982,6 +4009,53 @@ fn frontend_sus_fee() -> Result<()> { check_balance(&test, ESTER, &ibc_denom_on_namada, 1)?; check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 910)?; + // Shield 50 nams + transfer_on_chain( + &test, + "shield", + ALBERT_KEY, + AA_PAYMENT_ADDRESS, + NAM, + 50, + ALBERT_KEY, + &[], + )?; + check_shielded_balance(&test, AA_VIEWING_KEY, NAM, 50)?; + check_balance(&test, ESTER, NAM, 1000000)?; + // Unshielding transfer 10 samoleans from Namada to Gaia with transparent + // frontend fee. Also pay gas fees via the masp and verify that this amount + // is not subject to frontend masp fees (no recursive fees) + transfer( + &test, + A_SPENDING_KEY, + &gaia_receiver, + &ibc_denom_on_namada, + // An extra token will be added to this amount as a frontend masp fee + 10, + Some(BERTHA_KEY), + &port_id_namada, + &channel_id_namada, + None, + None, + None, + None, + true, + Some(ESTER), + Some(NAM), + )?; + wait_for_packet_relay( + &hermes_dir, + &port_id_namada, + &channel_id_namada, + &test, + )?; + check_shielded_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 67)?; + check_shielded_balance(&test, AC_VIEWING_KEY, &ibc_denom_on_namada, 11)?; + check_shielded_balance(&test, AC_VIEWING_KEY, NAM, 0)?; + check_balance(&test, ESTER, &ibc_denom_on_namada, 2)?; + check_balance(&test, ESTER, NAM, 1000000)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 920)?; + Ok(()) } @@ -4109,6 +4183,7 @@ fn osmosis_xcs() -> Result<()> { None, false, None, + None, )?; // Transfer Samoleans from Gaia transfer_from_cosmos( diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 11884de6b40..d085cc4bd97 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -10182,7 +10182,14 @@ fn frontend_sus_fee() -> Result<()> { 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 (mut node, _services) = setup::initialize_genesis(|mut genesis| { + // Whitelist BTC for gas payment + genesis.parameters.parameters.minimum_gas_price.insert( + "btc".into(), + DenominatedAmount::new(1.into(), token::Denomination(5)), + ); + genesis + })?; // Wait till epoch boundary node.next_masp_epoch(); @@ -10190,166 +10197,275 @@ fn frontend_sus_fee() -> Result<()> { let (frontend_alias, _frontend_key) = make_temp_account(&node, validator_one_rpc, "Frontend", NAM, 0)?; - // Test sus fee when shielding. Send 10 NAM from Albert to PA and 1 NAM to a - // transparent address owned by the frontend provider - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "10", - "--test-frontend-sus-fee", - frontend_alias, - "--signing-keys", - ALBERT_KEY, - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - - // Test sus fee when shielding. Send 10 NAM from Albert to PA and 1 NAM to a - // shielded address owned by the frontend provider - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "10", - "--test-frontend-sus-fee", - AC_PAYMENT_ADDRESS, - "--signing-keys", - ALBERT_KEY, - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); + for token in [NAM, BTC] { + // Test sus fee when shielding. Send 20 tokens from Albert to PA and 3 + // tokens to a transparent address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + token, + "--amount", + "20", + "--test-frontend-sus-fee", + frontend_alias, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; + // Test sus fee when shielding. Send 10 tokens from Albert to PA and 1 + // token to a shielded address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + token, + "--amount", + "20", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Assert NAM balance at VK(A) is 20 - let captured = CapturedOutput::of(|| { + // sync the shielded context run( &node, Bin::Client, vec![ - "balance", - "--owner", + "shielded-sync", + "--viewing-keys", AA_VIEWING_KEY, - "--token", - NAM, + AC_VIEWING_KEY, "--node", validator_one_rpc, ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 20")); + )?; - // Assert NAM balance at the transparent frontend is 1 - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - frontend_alias, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1")); + // Assert token balance at VK(A) is 40 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 40", token.to_lowercase()))); - // Assert NAM balance at the shielded frontend is 1 - let captured = CapturedOutput::of(|| { + // Assert token balance at the transparent frontend is 2 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + frontend_alias, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 2", token.to_lowercase()))); + + // Assert token balance at the shielded frontend is 2 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AC_VIEWING_KEY, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 2", token.to_lowercase()))); + } + + for token in [NAM, BTC] { + // Test sus fee when unshielding. Send 9 tokens from PA to Albert and + // 0.9 tokens to a transparent address owned by the frontend provider + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + ALBERT, + "--token", + token, + "--amount", + "9", + "--test-frontend-sus-fee", + frontend_alias, + "--signing-keys", + ALBERT_KEY, + "--gas-limit", + "60000", + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Test sus fee when unshielding. Send 8 tokens from PA to Albert and + // 0.8 tokens to a shielded address owned by the frontend provider. Also + // pay gas fees via the masp in this case and check that the frontend + // fees does not account for the fee unshielding amount (no recursive + // fees) + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + ALBERT, + "--token", + token, + "--amount", + "8", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--gas-token", + token, + "--gas-limit", + "70000", + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // sync the shielded context run( &node, Bin::Client, vec![ - "balance", - "--owner", + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, AC_VIEWING_KEY, - "--token", - NAM, "--node", validator_one_rpc, ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1")); + )?; - // FIXME: add a test with a non-native token - // FIXME: add test with masp fee unshielding (also for the ibc case) and - // check that the amount unshielded for fees is not subject to this frontend - // fee Test sus fee when unshielding. Send 9 NAM from PA to Albert and - // 0.9 NAM to a transparent address owned by the frontend provider - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "unshield", - "--source", - AA_VIEWING_KEY, - "--target", - ALBERT, - "--token", - NAM, - "--amount", - "9", - "--test-frontend-sus-fee", - frontend_alias, - "--signing-keys", - ALBERT_KEY, - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); + // Assert token balance at VK(A) is 20.6 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 20.6", token.to_lowercase()))); + + // Assert token balance at the transparent frontend is 2.9 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + frontend_alias, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 2.9", token.to_lowercase()))); + + // Assert token balance at the shielded frontend is 2.8 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AC_VIEWING_KEY, + "--token", + token, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("{}: 2.8", token.to_lowercase()))); + } - // Test sus fee when unshielding. Send 9 NAM from PA to Albert and 0.9 NAM - // to a shielded address owned by the frontend provider + // Test sus fee when unshielding. Send 10 BTCs from PA to Albert and + // 1 BTC to a shielded address owned by the frontend provider. Also pay gas + // fees via the masp with a different token and check that the frontend fees + // does not account for the fee unshielding amount (no recursive fees) let captured = CapturedOutput::of(|| { run( &node, @@ -10361,13 +10477,15 @@ fn frontend_sus_fee() -> Result<()> { "--target", ALBERT, "--token", - NAM, + BTC, "--amount", - "9", + "10", "--test-frontend-sus-fee", AC_PAYMENT_ADDRESS, - "--signing-keys", - ALBERT_KEY, + "--gas-token", + NAM, + "--gas-limit", + "100000", "--node", validator_one_rpc, ]), @@ -10390,7 +10508,7 @@ fn frontend_sus_fee() -> Result<()> { ], )?; - // Assert NAM balance at VK(A) is 0 + // Assert BTC balance at VK(A) is 9.6 let captured = CapturedOutput::of(|| { run( &node, @@ -10400,16 +10518,16 @@ fn frontend_sus_fee() -> Result<()> { "--owner", AA_VIEWING_KEY, "--token", - NAM, + BTC, "--node", validator_one_rpc, ], ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.2")); + assert!(captured.contains("btc: 9.6")); - // Assert NAM balance at the transparent frontend is 2 + // Assert NAM balance at VK(A) is 19.6 let captured = CapturedOutput::of(|| { run( &node, @@ -10417,7 +10535,7 @@ fn frontend_sus_fee() -> Result<()> { vec![ "balance", "--owner", - frontend_alias, + AA_VIEWING_KEY, "--token", NAM, "--node", @@ -10426,9 +10544,28 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.9")); + assert!(captured.contains("nam: 19.6")); + + // Assert BTC balance at the shielded frontend is 3.8 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AC_VIEWING_KEY, + "--token", + BTC, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("btc: 3.8")); - // Assert NAM balance at the shielded frontend is 2 + // Assert NAM balance at the shielded frontend is 2.8 let captured = CapturedOutput::of(|| { run( &node, @@ -10445,7 +10582,7 @@ fn frontend_sus_fee() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.9")); + assert!(captured.contains("nam: 2.8")); Ok(()) } From 7f7a42cb49624f1b56f7e50bf98417ae7f1294db Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 8 Sep 2025 12:40:12 +0200 Subject: [PATCH 13/18] Refactors masp frontend fee handling in the sdk (cherry picked from commit 8070560882c819a74cf5d82bbeafb99d76abd2e8) --- crates/sdk/src/tx.rs | 136 +++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 84 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index e9f35b83216..b55d5c8363a 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -3710,40 +3710,12 @@ pub async fn build_shielding_transfer( data = data .debit(source.to_owned(), token.to_owned(), validated_amount) .ok_or(Error::Other("Combined transfer overflows".to_string()))?; - } - - for args::TxShieldedTarget { - target, - token, - amount, - } in &args.targets - { - // Validate the amount given - let validated_amount = - validate_amount(context, amount.to_owned(), token, args.tx.force) - .await?; - transfer_data.targets.push(( - TransferTarget::PaymentAddress(target.to_owned()), - token.to_owned(), - validated_amount, - )); - - data = data - .credit(MASP, token.to_owned(), validated_amount) - .ok_or(Error::Other("Combined transfer overflows".to_string()))?; - } - if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { - let mut extra_transfer: Vec<( - Address, - Address, - Address, - DenominatedAmount, - )> = Default::default(); - // Transfer the frontend fee, take the fee percentage from every source - for (account, denominated_amt) in &data.sources { + // Transfer the frontend fee (if required), take the fee percentage from + // every source + if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { let sus_fee_amt = namada_token::Amount::from_uint( - denominated_amt + validated_amount .amount() .raw_amount() .checked_mul_div( @@ -3767,41 +3739,58 @@ pub async fn build_shielding_transfer( context, args::InputAmount::Unvalidated(DenominatedAmount::new( sus_fee_amt, - denominated_amt.denom(), + validated_amount.denom(), )), - &account.token, + token, args.tx.force, ) .await?; - extra_transfer.push(( - account.owner.to_owned(), - sus_fee_target.effective_address(), - account.token.to_owned(), - validated_fee_amount, - )); + data = data + .transfer( + source.to_owned(), + sus_fee_target.effective_address(), + token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; if sus_fee_target.payment_address().is_some() { - // Add the extra shielding source and target for the masp - // frontend sustainability fee + // Add the extra shielding source and target transfer_data.sources.push(( - TransferSource::Address(account.owner.to_owned()), - account.token.to_owned(), + TransferSource::Address(source.to_owned()), + token.to_owned(), validated_fee_amount, )); transfer_data.targets.push(( sus_fee_target.to_owned(), - account.token.to_owned(), + token.to_owned(), validated_fee_amount, )); } } + } - // Merge the extra transfer data with the default one - for (source, target, token, amount) in extra_transfer { - data = data.transfer(source, target, token, amount).ok_or( - Error::Other("Combined transfer overflows".to_string()), - )?; - } + for args::TxShieldedTarget { + target, + token, + amount, + } in &args.targets + { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + transfer_data.targets.push(( + TransferTarget::PaymentAddress(target.to_owned()), + token.to_owned(), + validated_amount, + )); + + data = data + .credit(MASP, token.to_owned(), validated_amount) + .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } let shielded_parts = construct_shielded_parts( @@ -3933,27 +3922,14 @@ pub async fn build_unshielding_transfer( data = data .debit(MASP, token.to_owned(), validated_amount) .ok_or(Error::Other("Combined transfer overflows".to_string()))?; - } - if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { - let mut extra_transfer: Vec<( - TransferSource, - Address, - DenominatedAmount, - )> = Default::default(); - // Transfer the frontend fee, take the fee percentage from every source - // FIXME: wrong this should be computed on the targets. Actually maybe - // not if all the sources are unshielded which might be the case - // FIXME: should we loop on the masp transfer data instead also in the - // other builders? What if we add dummy notes? It doesn't matter since - // we will produce them later not here - // FIXME: actually, we should probably loop on the args.sources, same in - // the other builders - for (source, token, denominated_amt) in &transfer_data.sources { + // Transfer the frontend fee (if required), take the fee percentage from + // every source + if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { // NOTE: The frontend fee should NOT account for the masp fee // payment amount let sus_fee_amt = namada_token::Amount::from_uint( - denominated_amt + validated_amount .amount() .raw_amount() .checked_mul_div( @@ -3977,21 +3953,15 @@ pub async fn build_unshielding_transfer( context, args::InputAmount::Unvalidated(DenominatedAmount::new( sus_fee_amt, - denominated_amt.denom(), + validated_amount.denom(), )), token, args.tx.force, ) .await?; - extra_transfer.push(( - source.to_owned(), - token.to_owned(), - validated_fee_amount, - )); - data = data .transfer( - source.effective_address(), + MASP, sus_fee_target.effective_address(), token.to_owned(), validated_fee_amount, @@ -4000,20 +3970,18 @@ pub async fn build_unshielding_transfer( "Combined transfer overflows".to_string(), ))?; - // Add the extra shielding source and target for the masp - // frontend sustainability fee + // Add the extra unshielding source and target + transfer_data.sources.push(( + TransferSource::ExtendedKey(source.to_owned()), + token.to_owned(), + validated_fee_amount, + )); transfer_data.targets.push(( sus_fee_target.to_owned(), token.to_owned(), validated_fee_amount, )); } - - // Merge the extra shielding source for the masp frontend sustainability - // fee - for (source, token, amount) in extra_transfer { - transfer_data.sources.push((source, token, amount)); - } } // Add masp fee payment if necessary From 9a42e8a076f904d66ba3437270d59bbe8a69e314 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 8 Sep 2025 14:41:20 +0200 Subject: [PATCH 14/18] Adds balance check when shielding with masp frontend fees (cherry picked from commit f0c492591497d80b4784be8545cc32809113a1ee) --- crates/sdk/src/tx.rs | 115 +++++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 47 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index b55d5c8363a..bfd8cef0299 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -3680,6 +3680,60 @@ pub async fn build_shielding_transfer( validate_amount(context, amount.to_owned(), token, args.tx.force) .await?; + // Compute the frontend fee (if required), take the fee percentage from + // every source + let validated_frontend_fee_amt = if let Some(( + sus_fee_target, + percentage, + )) = &args.frontend_sus_fee + { + let sus_fee_amt = namada_token::Amount::from_uint( + validated_amount + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10( + POS_DECIMAL_PRECISION as _, + ), + ) + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation" + .to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; + // Validate the amount given + Some(( + sus_fee_target, + validate_amount( + context, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + validated_amount.denom(), + )), + token, + args.tx.force, + ) + .await?, + )) + } else { + None + }; + let total_input_amt = checked!( + validated_amount + + validated_frontend_fee_amt + .map(|(_, amt)| amt) + .unwrap_or_else(|| DenominatedAmount::new( + namada_token::Amount::zero(), + validated_amount.denom() + )) + )?; + // Check the balance of the source if let Some(updated_balance) = &updated_balance { let check_balance = if &updated_balance.source == source @@ -3693,7 +3747,7 @@ pub async fn build_shielding_transfer( check_balance_too_low_err( token, source, - validated_amount.amount(), + total_input_amt.amount(), check_balance, args.tx.force, context, @@ -3708,54 +3762,12 @@ pub async fn build_shielding_transfer( )); data = data - .debit(source.to_owned(), token.to_owned(), validated_amount) + .debit(source.to_owned(), token.to_owned(), total_input_amt) .ok_or(Error::Other("Combined transfer overflows".to_string()))?; - // Transfer the frontend fee (if required), take the fee percentage from - // every source - if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { - let sus_fee_amt = namada_token::Amount::from_uint( - validated_amount - .amount() - .raw_amount() - .checked_mul_div( - percentage.abs(), - namada_core::uint::Uint::exp10( - POS_DECIMAL_PRECISION as _, - ), - ) - .ok_or_else(|| { - Error::Other( - "Overflow in masp frontend fee computation" - .to_string(), - ) - })? - .0, - 0, - ) - .map_err(|e| Error::Other(e.to_string()))?; - // Validate the amount given - let validated_fee_amount = validate_amount( - context, - args::InputAmount::Unvalidated(DenominatedAmount::new( - sus_fee_amt, - validated_amount.denom(), - )), - token, - args.tx.force, - ) - .await?; - data = data - .transfer( - source.to_owned(), - sus_fee_target.effective_address(), - token.to_owned(), - validated_fee_amount, - ) - .ok_or(Error::Other( - "Combined transfer overflows".to_string(), - ))?; - + if let Some((sus_fee_target, validated_fee_amount)) = + validated_frontend_fee_amt + { if sus_fee_target.payment_address().is_some() { // Add the extra shielding source and target transfer_data.sources.push(( @@ -3769,6 +3781,15 @@ pub async fn build_shielding_transfer( validated_fee_amount, )); } + data = data + .credit( + sus_fee_target.effective_address(), + token.to_owned(), + validated_fee_amount, + ) + .ok_or(Error::Other( + "Combined transfer overflows".to_string(), + ))?; } } From b5ff6d783b4f0e577c807fc636f8834d38bb7f47 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 8 Sep 2025 15:34:32 +0200 Subject: [PATCH 15/18] Tests insufficients masp sus fee balances (cherry picked from commit c5fef2be3543811501e88f6c50edbec3a43f9f76) --- crates/tests/src/e2e/ibc_tests.rs | 27 +++ crates/tests/src/integration/masp.rs | 262 +++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 24a6651a50c..aaf61f6ed3f 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -4056,6 +4056,33 @@ fn frontend_sus_fee() -> Result<()> { check_balance(&test, ESTER, NAM, 1000000)?; check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 920)?; + // Unshielding transfer more samoleans than available with transparent + // frontend fee and shielded gas fees. Verify that client checks prevent + // this + transfer( + &test, + A_SPENDING_KEY, + &gaia_receiver, + &ibc_denom_on_namada, + // An extra 10 tokens will be added to this amount as a frontend masp + // fee + 100, + None, + &port_id_namada, + &channel_id_namada, + None, + None, + Some( + "Failed to construct MASP transaction shielded parts: \ + Insufficient funds: 43 \ + tnam1p5n6vw2v870lnjwu7h0l4humkhlf5d78ay693qmv missing", + ), + None, + true, + Some(ESTER), + None, + )?; + Ok(()) } diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index d085cc4bd97..e2e325dbdbc 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -10586,3 +10586,265 @@ fn frontend_sus_fee() -> Result<()> { Ok(()) } + +// Check that the clients performs balance checks correctly when adding a masp +// frontend sus fee +#[test] +fn frontend_sus_fee_client_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()?; + // Wait till epoch boundary + node.next_masp_epoch(); + + // Initialize source address + let (source, _source_key) = + make_temp_account(&node, validator_one_rpc, "Source", NAM, 100)?; + + // Test a shielding tx where the amount and the transparent masp fee amount + // together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + ALBERT, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "is lower than the amount to be transferred. Amount to transfer is \ + 110.000000 and the balance is 100.000000." + )); + + // Test a shielding tx where the amount and the shielded masp fee amount + // together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "is lower than the amount to be transferred. Amount to transfer is \ + 110.000000 and the balance is 100.000000." + )); + + // Test a shielding tx where the amount, the gas fee amount and the shielded + // masp fee amount together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--signing-keys", + source, + "--gas-limit", + "100000", + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "is lower than the amount to be transferred. Amount to transfer is \ + 110.000000 and the balance is 99.000000." + )); + + // Shield some tokens to the source + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "100", + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 100")); + + // Test an unshielding tx where the amount and the transparent masp fee + // amount together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + source, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + ALBERT, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "Failed to construct MASP transaction shielded parts: Insufficient \ + funds: 10 tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5 missing" + )); + + // Test an unshielding tx where the amount and the shielded masp fee amount + // together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + source, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--signing-keys", + ALBERT_KEY, + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "Failed to construct MASP transaction shielded parts: Insufficient \ + funds: 10 tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5 missing" + )); + + // Test an unshielding tx where the amount, the gas fee unshielding amount + // and the shielded masp fee amount together exceed the source balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "unshield", + "--source", + AA_VIEWING_KEY, + "--target", + source, + "--token", + NAM, + "--amount", + "100", + "--test-frontend-sus-fee", + AC_PAYMENT_ADDRESS, + "--gas-limit", + "100000", + "--node", + validator_one_rpc, + ]), + ) + }); + assert!(captured.result.is_err()); + assert!(captured.contains( + "Failed to construct MASP transaction shielded parts: Insufficient \ + funds: 11 tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5 missing" + )); + + Ok(()) +} From fa5d9bcbbe6d8260729156207bff40559caa378b Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 9 Sep 2025 17:16:16 +0200 Subject: [PATCH 16/18] Hides test cli arguments (cherry picked from commit 2c8247206dc9793697e50bdac35ff4a7862e1e0c) --- crates/apps_lib/src/cli.rs | 52 ++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 265e91e6e5a..32ec3e07b05 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -5023,10 +5023,15 @@ pub mod args { .def() .help(wrap!("The amount to transfer in decimal.")), ) - .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .help(wrap!( + "The optional address of the frontend provider \ + that will take the masp sustainability fee." + )) + .hide(true), + ) } } @@ -5139,10 +5144,15 @@ pub mod args { payment. When not provided the source spending key will \ be used." ))) - .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .help(wrap!( + "The optional address of the frontend provider \ + that will take the masp sustainability fee." + )) + .hide(true), + ) } } @@ -5277,10 +5287,15 @@ pub mod args { payment (if this is a shielded action). When not \ provided the source spending key will be used." ))) - .arg(__TEST_FRONTEND_SUS_FEE.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE + .def() + .help(wrap!( + "The optional address of the frontend provider \ + that will take the masp sustainability fee." + )) + .hide(true), + ) } } @@ -7388,10 +7403,15 @@ pub mod args { .arg(CHANNEL_ID.def().help(wrap!( "The channel ID via which the token is received." ))) - .arg(__TEST_FRONTEND_SUS_FEE_IBC.def().help(wrap!( - "The optional address of the frontend provider that will \ - take the masp sustainability fee." - ))) + .arg( + __TEST_FRONTEND_SUS_FEE_IBC + .def() + .help(wrap!( + "The optional address of the frontend provider \ + that will take the masp sustainability fee." + )) + .hide(true), + ) } } From 669ef0c42dbc5c2c2ca844b9e651bfd0f5af1f7c Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 9 Sep 2025 17:42:30 +0200 Subject: [PATCH 17/18] Refactors computation of the masp sus fee into a function (cherry picked from commit 2757e68d6fa729db0b714989a90147de251cc5cf) --- crates/sdk/src/tx.rs | 167 ++++++++++-------------------- crates/tests/src/e2e/ibc_tests.rs | 4 + 2 files changed, 60 insertions(+), 111 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index bfd8cef0299..6eec4e17925 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2905,33 +2905,10 @@ pub async fn build_ibc_transfer( (&MASP, None) => { // NOTE: The frontend fee should NOT account for the masp fee // payment amount - let sus_fee_amt = namada_token::Amount::from_uint( - validated_amount - .amount() - .raw_amount() - .checked_mul_div( - percentage.abs(), - namada_core::uint::Uint::exp10( - POS_DECIMAL_PRECISION as _, - ), - ) - .ok_or_else(|| { - Error::Other( - "Overflow in masp frontend fee computation" - .to_string(), - ) - })? - .0, - 0, - ) - .map_err(|e| Error::Other(e.to_string()))?; - // Validate the amount given - let validated_fee_amount = validate_amount( + let validated_fee_amount = compute_masp_frontend_sus_fee( context, - args::InputAmount::Unvalidated(DenominatedAmount::new( - sus_fee_amt, - validated_amount.denom(), - )), + &validated_amount, + percentage, &args.token, args.tx.force, ) @@ -3635,6 +3612,45 @@ async fn get_masp_fee_payment_amount( }) } +// Extract the validate amount for the masp frontend sustainability fee +async fn compute_masp_frontend_sus_fee( + context: &impl Namada, + input_amount: &namada_token::DenominatedAmount, + percentage: &namada_core::dec::Dec, + token: &Address, + force: bool, +) -> Result { + let sus_fee_amt = namada_token::Amount::from_uint( + input_amount + .amount() + .raw_amount() + .checked_mul_div( + percentage.abs(), + namada_core::uint::Uint::exp10(POS_DECIMAL_PRECISION as _), + ) + .ok_or_else(|| { + Error::Other( + "Overflow in masp frontend fee computation".to_string(), + ) + })? + .0, + 0, + ) + .map_err(|e| Error::Other(e.to_string()))?; + + // Validate the amount given + validate_amount( + context, + args::InputAmount::Unvalidated(DenominatedAmount::new( + sus_fee_amt, + input_amount.denom(), + )), + token, + force, + ) + .await +} + /// Build a shielding transfer pub async fn build_shielding_transfer( context: &N, @@ -3687,40 +3703,15 @@ pub async fn build_shielding_transfer( percentage, )) = &args.frontend_sus_fee { - let sus_fee_amt = namada_token::Amount::from_uint( - validated_amount - .amount() - .raw_amount() - .checked_mul_div( - percentage.abs(), - namada_core::uint::Uint::exp10( - POS_DECIMAL_PRECISION as _, - ), - ) - .ok_or_else(|| { - Error::Other( - "Overflow in masp frontend fee computation" - .to_string(), - ) - })? - .0, - 0, + let validated_fee_amount = compute_masp_frontend_sus_fee( + context, + &validated_amount, + percentage, + token, + args.tx.force, ) - .map_err(|e| Error::Other(e.to_string()))?; - // Validate the amount given - Some(( - sus_fee_target, - validate_amount( - context, - args::InputAmount::Unvalidated(DenominatedAmount::new( - sus_fee_amt, - validated_amount.denom(), - )), - token, - args.tx.force, - ) - .await?, - )) + .await?; + Some((sus_fee_target, validated_fee_amount)) } else { None }; @@ -3949,33 +3940,10 @@ pub async fn build_unshielding_transfer( if let Some((sus_fee_target, percentage)) = &args.frontend_sus_fee { // NOTE: The frontend fee should NOT account for the masp fee // payment amount - let sus_fee_amt = namada_token::Amount::from_uint( - validated_amount - .amount() - .raw_amount() - .checked_mul_div( - percentage.abs(), - namada_core::uint::Uint::exp10( - POS_DECIMAL_PRECISION as _, - ), - ) - .ok_or_else(|| { - Error::Other( - "Overflow in masp frontend fee computation" - .to_string(), - ) - })? - .0, - 0, - ) - .map_err(|e| Error::Other(e.to_string()))?; - // Validate the amount given - let validated_fee_amount = validate_amount( + let validated_fee_amount = compute_masp_frontend_sus_fee( context, - args::InputAmount::Unvalidated(DenominatedAmount::new( - sus_fee_amt, - validated_amount.denom(), - )), + &validated_amount, + percentage, token, args.tx.force, ) @@ -4555,33 +4523,10 @@ pub async fn gen_ibc_shielding_transfer( let (extra_target, source_amount) = match &args.frontend_sus_fee { Some((target, percentage)) => { - let sus_fee_amt = namada_token::Amount::from_uint( - validated_amount - .amount() - .raw_amount() - .checked_mul_div( - percentage.abs(), - namada_core::uint::Uint::exp10( - POS_DECIMAL_PRECISION as _, - ), - ) - .ok_or_else(|| { - Error::Other( - "Overflow in masp frontend fee computation" - .to_string(), - ) - })? - .0, - 0, - ) - .map_err(|e| Error::Other(e.to_string()))?; - // Validate the amount given - let validated_fee_amount = validate_amount( + let validated_fee_amount = compute_masp_frontend_sus_fee( context, - args::InputAmount::Unvalidated(DenominatedAmount::new( - sus_fee_amt, - validated_amount.denom(), - )), + &validated_amount, + percentage, &token, false, ) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index aaf61f6ed3f..2e5873b4dd6 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -1426,6 +1426,8 @@ fn ibc_unlimited_channel() -> Result<()> { None, None, false, + None, + None, )?; wait_for_packet_relay( &hermes_dir, @@ -1456,6 +1458,8 @@ fn ibc_unlimited_channel() -> Result<()> { None, None, false, + None, + None, )?; // wait for the timeout sleep(10); From a7b976b4d614f5831cabf64c41c1b1f41b4c5211 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 21 Aug 2025 18:40:24 +0200 Subject: [PATCH 18/18] Changelog #4790 (cherry picked from commit a13d611e54463c9992c98b5fece292a49aee2d1a) --- .changelog/unreleased/features/4790-frontend-sus-fees.md | 2 ++ wasm/Cargo.lock | 4 ++-- wasm_for_tests/Cargo.lock | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 .changelog/unreleased/features/4790-frontend-sus-fees.md diff --git a/.changelog/unreleased/features/4790-frontend-sus-fees.md b/.changelog/unreleased/features/4790-frontend-sus-fees.md new file mode 100644 index 00000000000..f54c71cf27a --- /dev/null +++ b/.changelog/unreleased/features/4790-frontend-sus-fees.md @@ -0,0 +1,2 @@ +- Added support for MASP frontend providers sustainability fees. + ([\#4790](https://github.com/anoma/namada/pull/4790)) \ No newline at end of file diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 9b33c689d89..e8c971b4015 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -4537,9 +4537,9 @@ dependencies = [ [[package]] name = "nam-sparse-merkle-tree" -version = "0.3.2-nam.0" +version = "0.3.3-nam.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fae108a0f6aabf789e34d6d447c1184608d1c70c087f957b720042226a47ab63" +checksum = "09548cb2907afb9ee1047b7bbe9601e25b73b69229fca50d49badd97cc190cfb" dependencies = [ "borsh", "cfg-if", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 6574b06d4a6..6efbd8887cb 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2618,9 +2618,9 @@ dependencies = [ [[package]] name = "nam-sparse-merkle-tree" -version = "0.3.2-nam.0" +version = "0.3.3-nam.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fae108a0f6aabf789e34d6d447c1184608d1c70c087f957b720042226a47ab63" +checksum = "09548cb2907afb9ee1047b7bbe9601e25b73b69229fca50d49badd97cc190cfb" dependencies = [ "borsh", "cfg-if",