diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 6c368ddd61..90a699c379 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -4661,16 +4661,13 @@ pub mod args { }); } - let gas_spending_keys = self - .gas_spending_keys - .iter() - .map(|key| chain_ctx.get_cached(key)) - .collect(); + let gas_spending_key = + self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); Ok(TxShieldedTransfer:: { tx, data, - gas_spending_keys, + gas_spending_key, disposable_signing_key: self.disposable_signing_key, tx_code_path: self.tx_code_path.to_path_buf(), }) @@ -4691,16 +4688,13 @@ pub mod args { token, amount, }]; - let mut gas_spending_keys = vec![]; - if let Some(key) = GAS_SPENDING_KEY.parse(matches) { - gas_spending_keys.push(key); - } + let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let disposable_gas_payer = DISPOSABLE_SIGNING_KEY.parse(matches); Self { tx, data, - gas_spending_keys, + gas_spending_key, disposable_signing_key: disposable_gas_payer, tx_code_path, } @@ -4830,16 +4824,13 @@ pub mod args { amount: transfer_data.amount, }); } - let gas_spending_keys = self - .gas_spending_keys - .iter() - .map(|key| chain_ctx.get_cached(key)) - .collect(); + let gas_spending_key = + self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); Ok(TxUnshieldingTransfer:: { tx, data, - gas_spending_keys, + gas_spending_key, disposable_signing_key: self.disposable_signing_key, source: chain_ctx.get_cached(&self.source), tx_code_path: self.tx_code_path.to_path_buf(), @@ -4860,17 +4851,14 @@ pub mod args { token, amount, }]; - let mut gas_spending_keys = vec![]; - if let Some(key) = GAS_SPENDING_KEY.parse(matches) { - gas_spending_keys.push(key); - } + let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let disposable_signing_key = DISPOSABLE_SIGNING_KEY.parse(matches); Self { tx, source, data, - gas_spending_keys, + gas_spending_key, disposable_signing_key, tx_code_path, } @@ -4919,11 +4907,8 @@ pub mod args { ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let gas_spending_keys = self - .gas_spending_keys - .iter() - .map(|key| chain_ctx.get_cached(key)) - .collect(); + let gas_spending_key = + self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); Ok(TxIbcTransfer:: { tx, @@ -4938,7 +4923,7 @@ pub mod args { refund_target: chain_ctx.get_opt(&self.refund_target), ibc_shielding_data: self.ibc_shielding_data, ibc_memo: self.ibc_memo, - gas_spending_keys, + gas_spending_key, disposable_signing_key: self.disposable_signing_key, tx_code_path: self.tx_code_path.to_path_buf(), }) @@ -4965,10 +4950,7 @@ pub mod args { .expect("Failed to decode IBC shielding data") }); let ibc_memo = IBC_MEMO.parse(matches); - let mut gas_spending_keys = vec![]; - if let Some(key) = GAS_SPENDING_KEY.parse(matches) { - gas_spending_keys.push(key); - } + let gas_spending_key = GAS_SPENDING_KEY.parse(matches); let disposable_signing_key = DISPOSABLE_SIGNING_KEY.parse(matches); let tx_code_path = PathBuf::from(TX_IBC_WASM); Self { @@ -4984,7 +4966,7 @@ pub mod args { refund_target, ibc_shielding_data, ibc_memo, - gas_spending_keys, + gas_spending_key, disposable_signing_key, tx_code_path, } diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index c4f82a8c6a..4d727b3913 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -195,11 +195,9 @@ impl AssetData { /// Give this pre-asset type the given epoch if already has an epoch. Return /// the replaced value. - pub fn redate(&mut self, to: MaspEpoch) -> Option { + pub fn redate(&mut self, to: MaspEpoch) { if self.epoch.is_some() { - self.epoch.replace(to) - } else { - None + self.epoch = Some(to); } } @@ -1121,14 +1119,12 @@ mod test { assert!(data.epoch.is_none()); let epoch_0 = MaspEpoch::new(3); - let old = data.redate(epoch_0); - assert!(old.is_none()); + data.redate(epoch_0); assert!(data.epoch.is_none()); data.epoch = Some(epoch_0); let epoch_1 = MaspEpoch::new(5); - let old = data.redate(epoch_1); - assert_eq!(old, Some(epoch_0)); + data.redate(epoch_1); assert_eq!(data.epoch, Some(epoch_1)); } diff --git a/crates/core/src/token.rs b/crates/core/src/token.rs index 1071b1d0ae..ca8fb14a6c 100644 --- a/crates/core/src/token.rs +++ b/crates/core/src/token.rs @@ -56,6 +56,11 @@ pub const NATIVE_SCALE: u64 = 1_000_000; pub type Change = I256; impl Amount { + /// Iterate over all words in this [`Amount`]. + pub fn iter_words(self) -> impl Iterator { + self.raw.0.into_iter() + } + /// Convert a [`u64`] to an [`Amount`]. pub const fn from_u64(x: u64) -> Self { Self { @@ -905,6 +910,14 @@ pub enum MaspDigitPos { Three, } +impl From for MaspDigitPos { + fn from(denom: usize) -> Self { + u8::try_from(denom) + .expect("Possible MASP denominations must be between 0 and 3") + .into() + } +} + impl From for MaspDigitPos { fn from(denom: u8) -> Self { match denom { diff --git a/crates/core/src/uint.rs b/crates/core/src/uint.rs index aa260a52f3..1239d5748e 100644 --- a/crates/core/src/uint.rs +++ b/crates/core/src/uint.rs @@ -546,6 +546,8 @@ impl FromStr for I256 { } impl I256 { + const N_WORDS: usize = 4; + /// Compute the two's complement of a number. pub fn negate(&self) -> Option { let (uint, overflow) = self.0.negate(); @@ -614,33 +616,30 @@ impl I256 { Self(MAX_SIGNED_VALUE) } - /// Attempt to convert a MASP-denominated integer to an I256 - /// using the given denomination. + /// Given a u128 and [`MaspDigitPos`], construct the corresponding + /// amount. pub fn from_masp_denominated( - value: impl Into, + val: i128, denom: MaspDigitPos, - ) -> Result { - let value = value.into(); - let is_negative = value < 0; - let value = value.unsigned_abs(); - let mut result = [0u64; 4]; - result[denom as usize] = u64::try_from(value) - .map_err(|_e| AmountParseError::PrecisionOverflow)?; - let result = Uint(result); - if result <= MAX_SIGNED_VALUE { - if is_negative { - let (inner, overflow) = result.negate(); - if overflow { - Err(AmountParseError::InvalidRange) - } else { - Ok(Self(inner)) - } + ) -> Result>::Error> { + let abs = val.unsigned_abs(); + #[allow(clippy::cast_possible_truncation)] + let lo = abs as u64; + let hi = (abs >> 64) as u64; + let lo_pos = denom as usize; + #[allow(clippy::arithmetic_side_effects)] + let hi_pos = lo_pos + 1; + let mut raw = [0u64; Self::N_WORDS]; + raw[lo_pos] = lo; + raw[hi_pos] = hi; + i64::try_from(raw[Self::N_WORDS - 1]).map(|_| { + let res = Self(Uint(raw)); + if val.is_negative() { + res.checked_neg().unwrap() } else { - Ok(Self(result).canonical()) + res } - } else { - Err(AmountParseError::InvalidRange) - } + }) } /// Multiply by a decimal [`Dec`] with the result rounded up. Checks for @@ -777,6 +776,23 @@ impl CheckedAdd for I256 { } } +// NOTE: This is here only because MASP requires it for `ValueSum` subtraction +impl CheckedSub for &I256 { + type Output = I256; + + fn checked_sub(self, rhs: Self) -> Option { + self.checked_sub(*rhs) + } +} + +impl CheckedSub for I256 { + type Output = I256; + + fn checked_sub(self, rhs: Self) -> Option { + I256::checked_sub(&self, rhs) + } +} + // NOTE: This is here only because num_traits::CheckedAdd requires it impl std::ops::Add for I256 { type Output = Self; diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 93643a0ced..782e7225c0 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -356,7 +356,7 @@ pub struct TxShieldedTransfer { /// Transfer-specific data pub data: Vec>, /// Optional additional keys for gas payment - pub gas_spending_keys: Vec, + pub gas_spending_key: Option, /// Generate an ephemeral signing key to be used only once to sign the /// wrapper tx pub disposable_signing_key: bool, @@ -453,7 +453,7 @@ pub struct TxUnshieldingTransfer { /// Transfer-specific data pub data: Vec>, /// Optional additional keys for gas payment - pub gas_spending_keys: Vec, + pub gas_spending_key: Option, /// Generate an ephemeral signing key to be used only once to sign the /// wrapper tx pub disposable_signing_key: bool, @@ -499,7 +499,7 @@ pub struct TxIbcTransfer { /// Memo for IBC transfer packet pub ibc_memo: Option, /// Optional additional keys for gas payment - pub gas_spending_keys: Vec, + pub gas_spending_key: Option, /// Generate an ephemeral signing key to be used only once to sign the /// wrapper tx pub disposable_signing_key: bool, @@ -591,12 +591,9 @@ impl TxIbcTransfer { } /// Gas spending keys - pub fn gas_spending_keys( - self, - gas_spending_keys: Vec, - ) -> Self { + pub fn gas_spending_keys(self, gas_spending_key: C::SpendingKey) -> Self { Self { - gas_spending_keys, + gas_spending_key: Some(gas_spending_key), ..self } } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 061deacf79..dcffe3e530 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -173,12 +173,12 @@ pub trait Namada: NamadaIo { fn new_shielded_transfer( &self, data: Vec, - gas_spending_keys: Vec, + gas_spending_key: Option, disposable_signing_key: bool, ) -> args::TxShieldedTransfer { args::TxShieldedTransfer { data, - gas_spending_keys, + gas_spending_key, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), disposable_signing_key, tx: self.tx_builder(), @@ -206,13 +206,13 @@ pub trait Namada: NamadaIo { &self, source: ExtendedSpendingKey, data: Vec, - gas_spending_keys: Vec, + gas_spending_key: Option, disposable_signing_key: bool, ) -> args::TxUnshieldingTransfer { args::TxUnshieldingTransfer { source, data, - gas_spending_keys, + gas_spending_key, disposable_signing_key, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), @@ -318,7 +318,7 @@ pub trait Namada: NamadaIo { refund_target: None, ibc_shielding_data: None, ibc_memo: None, - gas_spending_keys: Default::default(), + gas_spending_key: Default::default(), tx: self.tx_builder(), tx_code_path: PathBuf::from(TX_IBC_WASM), } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 114eb64262..2d7d9b3230 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2620,7 +2620,7 @@ pub async fn build_ibc_transfer( &args.tx, fee_per_gas_unit, &signing_data.fee_payer, - args.gas_spending_keys.clone(), + args.gas_spending_key, ) .await?; if let Some(fee_data) = &masp_fee_data { @@ -3086,7 +3086,7 @@ pub async fn build_shielded_transfer( &args.tx, fee_per_gas_unit, &signing_data.fee_payer, - args.gas_spending_keys.clone(), + args.gas_spending_key, ) .await?; if let Some(fee_data) = &masp_fee_data { @@ -3159,7 +3159,7 @@ async fn get_masp_fee_payment_amount( args: &args::Tx, fee_amount: DenominatedAmount, fee_payer: &common::PublicKey, - gas_spending_keys: Vec, + gas_spending_key: Option, ) -> Result> { let fee_payer_address = Address::from(fee_payer); let balance_key = balance_key(&args.fee_token, &fee_payer_address); @@ -3174,7 +3174,7 @@ async fn get_masp_fee_payment_amount( Ok(match total_fee.checked_sub(balance) { Some(diff) if !diff.is_zero() => Some(MaspFeeData { - sources: gas_spending_keys, + source: gas_spending_key, target: fee_payer_address, token: args.fee_token.clone(), amount: DenominatedAmount::new(diff, fee_amount.denom()), @@ -3374,7 +3374,7 @@ pub async fn build_unshielding_transfer( &args.tx, fee_per_gas_unit, &signing_data.fee_payer, - args.gas_spending_keys.clone(), + args.gas_spending_key, ) .await?; if let Some(fee_data) = &masp_fee_data { diff --git a/crates/shielded_token/src/masp.rs b/crates/shielded_token/src/masp.rs index cc1fc79fbb..848b2dd372 100644 --- a/crates/shielded_token/src/masp.rs +++ b/crates/shielded_token/src/masp.rs @@ -11,7 +11,6 @@ use std::collections::BTreeMap; use std::fmt::Debug; use borsh::{BorshDeserialize, BorshSerialize}; -use itertools::Itertools; use masp_primitives::asset_type::AssetType; #[cfg(feature = "mainnet")] use masp_primitives::consensus::MainNetwork as Network; @@ -43,7 +42,6 @@ use namada_migrations::*; use namada_tx::IndexedTx; use rand_core::{CryptoRng, RngCore}; pub use shielded_wallet::ShieldedWallet; -use smooth_operator::checked; use thiserror::Error; pub use crate::masp::shielded_sync::dispatcher::{Dispatcher, DispatcherCache}; @@ -81,7 +79,7 @@ pub struct ShieldedTransfer { #[allow(missing_docs)] #[derive(Debug)] pub struct MaspFeeData { - pub sources: Vec, + pub source: Option, pub target: Address, pub token: Address, pub amount: token::DenominatedAmount, @@ -105,9 +103,9 @@ pub struct MaspSourceTransferData { } /// The data for a masp transfer relative to a given target -#[derive(Hash, Eq, PartialEq)] -struct MaspTargetTransferData { - source: TransferSource, +#[derive(Debug, Hash, Eq, PartialEq)] +pub struct MaspTargetTransferData { + source: Option, target: TransferTarget, token: Address, } @@ -118,21 +116,16 @@ struct MaspTargetTransferData { pub struct MaspDataLog { pub source: Option, pub token: Address, - pub amount: token::DenominatedAmount, + pub amount: token::Amount, } #[allow(missing_docs)] pub struct MaspTxReorderedData { - source_data: HashMap, - target_data: HashMap, + source_data: HashMap, + target_data: HashMap, denoms: HashMap, } -/// Data about the unspent amounts for any given shielded source coming from the -/// spent notes in their posses that have been added to the builder. Can be used -/// to either pay fees or to return a change -pub type Changes = HashMap; - /// Shielded pool data for a token #[allow(missing_docs)] #[derive(Debug, BorshSerialize, BorshDeserialize, BorshDeserializer)] @@ -246,66 +239,6 @@ pub fn find_valid_diversifier( (diversifier, g_d) } -/// Determine if using the current note would actually bring us closer to our -/// target. Returns the unused amounts (change) of delta if any -pub fn is_amount_required( - src: I128Sum, - dest: I128Sum, - normed_delta: I128Sum, - opt_delta: Option, -) -> Option { - let mut changes = None; - let gap = dest.clone() - src; - - for (asset_type, value) in gap.components() { - if *value > 0 && normed_delta[asset_type] > 0 { - #[allow(clippy::disallowed_methods)] - let signed_change_amt = - checked!(normed_delta[asset_type] - *value).unwrap_or_default(); - let unsigned_change_amt = if signed_change_amt > 0 { - signed_change_amt - } else { - // Even if there's no change we still need to set the return - // value of this function to be Some so that the caller sees - // that this note should be used - 0 - }; - - let change_amt = I128Sum::from_nonnegative( - asset_type.to_owned(), - unsigned_change_amt, - ) - .expect("Change is guaranteed to be non-negative"); - changes = changes - .map(|prev| prev + change_amt.clone()) - .or(Some(change_amt)); - } - } - - // Because of the way conversions are computed, we need an extra step here - // if the token is not the native one - if let Some(delta) = opt_delta { - // Only if this note is going to be used, handle the assets in delta - // (not normalized) that are not part of dest - changes = changes.map(|mut chngs| { - for (delta_asset_type, delta_amt) in delta.components() { - if !dest.asset_types().contains(delta_asset_type) { - let rmng = I128Sum::from_nonnegative( - delta_asset_type.to_owned(), - *delta_amt, - ) - .expect("Change is guaranteed to be non-negative"); - chngs += rmng; - } - } - - chngs - }); - } - - changes -} - /// a masp change #[derive(BorshSerialize, BorshDeserialize, BorshDeserializer, Debug, Clone)] pub struct MaspChange { diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index 470e197995..8c214f151e 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -1,5 +1,4 @@ //! The shielded wallet implementation -use std::cmp::Ordering; use std::collections::{btree_map, BTreeMap, BTreeSet}; use eyre::eyre; @@ -36,9 +35,7 @@ use namada_core::masp::{ }; use namada_core::task_env::TaskEnvironment; use namada_core::time::{DateTimeUtc, DurationSecs}; -use namada_core::token::{ - Amount, Change, DenominatedAmount, Denomination, MaspDigitPos, -}; +use namada_core::token::{Amount, Change, Denomination, MaspDigitPos}; use namada_io::client::Client; use namada_io::{ display_line, edisplay_line, Io, MaybeSend, MaybeSync, NamadaIo, @@ -51,12 +48,11 @@ use rand_core::{OsRng, SeedableRng}; use crate::masp::utils::MaspClient; use crate::masp::{ - cloned_pair, is_amount_required, to_viewing_key, Changes, - ContextSyncStatus, Conversions, MaspAmount, MaspDataLog, MaspFeeData, - MaspSourceTransferData, MaspTargetTransferData, MaspTransferData, - MaspTxReorderedData, NoteIndex, ShieldedSyncConfig, ShieldedTransfer, - ShieldedUtils, SpentNotesTracker, TransferErr, WalletMap, WitnessMap, - NETWORK, + cloned_pair, to_viewing_key, ContextSyncStatus, Conversions, MaspAmount, + MaspDataLog, MaspFeeData, MaspSourceTransferData, MaspTargetTransferData, + MaspTransferData, MaspTxReorderedData, NoteIndex, ShieldedSyncConfig, + ShieldedTransfer, ShieldedUtils, SpentNotesTracker, TransferErr, WalletMap, + WitnessMap, NETWORK, }; #[cfg(any(test, feature = "testing"))] use crate::masp::{testing, ENV_VAR_MASP_TEST_SEED}; @@ -319,8 +315,6 @@ impl ShieldedWallet { usage: &mut i128, input: &mut I128Sum, output: &mut I128Sum, - normed_asset_type: AssetType, - normed_output: &mut I128Sum, ) -> Result<(), eyre::Error> { // we do not need to convert negative values if value <= 0 { @@ -344,14 +338,11 @@ impl ShieldedWallet { // Forget about the trace amount left over because we cannot // realize its value let trace = I128Sum::from_pair(asset_type, value % threshold); - let normed_trace = - I128Sum::from_pair(normed_asset_type, value % threshold); // Record how much more of the given conversion has been used *usage += required; // Apply the conversions to input and move the trace amount to output *input += conv * required - trace.clone(); *output += trace; - *normed_output += normed_trace; Ok(()) } @@ -537,11 +528,9 @@ pub trait ShieldedApi: mut input: I128Sum, target_epoch: MaspEpoch, mut conversions: Conversions, - ) -> Result<(I128Sum, I128Sum, Conversions), eyre::Error> { + ) -> Result<(I128Sum, Conversions), eyre::Error> { // Where we will store our exchanged value let mut output = I128Sum::zero(); - // Where we will store our normed exchanged value - let mut normed_output = I128Sum::zero(); // Repeatedly exchange assets until it is no longer possible while let Some((asset_type, value)) = input.components().next().map(cloned_pair) @@ -549,37 +538,18 @@ pub trait ShieldedApi: // Get the equivalent to the current asset in the target epoch and // note whether this equivalent chronologically comes after the // current asset - let (target_asset_type, forward_conversion) = self + let target_asset_type = self .decode_asset_type(client, asset_type) .await .map(|mut pre_asset_type| { - let old_epoch = pre_asset_type.redate(target_epoch); + pre_asset_type.redate(target_epoch); pre_asset_type .encode() - .map(|asset_type| { - ( - asset_type, - old_epoch.map_or(false, |epoch| { - target_epoch >= epoch - }), - ) - }) .map_err(|_| eyre!("unable to create asset type",)) }) .transpose()? - .unwrap_or((asset_type, false)); + .unwrap_or(asset_type); let at_target_asset_type = target_asset_type == asset_type; - let trace_asset_type = if forward_conversion { - // If we are doing a forward conversion, then we can assume that - // the trace left over in the older epoch has at least a 1-to-1 - // conversion to the newer epoch. - target_asset_type - } else { - // If we are not doing a forward conversion, then we cannot - // lower bound what the asset type will be worth in the target - // asset type. So leave the asset type fixed. - asset_type - }; // Fetch and store the required conversions self.query_allowed_conversion( client, @@ -608,8 +578,6 @@ pub trait ShieldedApi: usage, &mut input, &mut output, - trace_asset_type, - &mut normed_output, ) .await?; } else if let (Some((conv, _wit, usage)), false) = ( @@ -632,8 +600,6 @@ pub trait ShieldedApi: usage, &mut input, &mut output, - trace_asset_type, - &mut normed_output, ) .await?; } else { @@ -641,11 +607,10 @@ pub trait ShieldedApi: // output. let comp = input.project(asset_type); output += comp.clone(); - normed_output += comp.clone(); input -= comp; } } - Ok((output, normed_output, conversions)) + Ok((output, conversions)) } /// Compute the total unspent notes associated with the viewing key in the @@ -679,6 +644,45 @@ pub trait ShieldedApi: } } + /// Determine if using the current note would actually bring us closer to + /// our target. Returns the contribution of the current note to the + /// target if so. + #[allow(async_fn_in_trait)] + async fn is_amount_required( + &mut self, + client: &(impl Client + Sync), + src: ValueSum<(MaspDigitPos, Address), Change>, + dest: ValueSum<(MaspDigitPos, Address), Change>, + delta: I128Sum, + ) -> Option> { + // If the delta causes any regression, then do not use it + #[allow(clippy::neg_cmp_op_on_partial_ord)] + if !(delta >= I128Sum::zero()) { + return None; + } + let gap = dest.clone() - src; + let (decoded_delta, _rem_delta) = self.decode_sum(client, delta).await; + for ((_, asset_data), value) in decoded_delta.components() { + // Check that this component of the delta helps close the gap + if *value > 0 + && gap + .get(&(asset_data.position, asset_data.token.clone())) + .is_positive() + { + // Convert the delta into Namada amounts + let mut converted_delta = ValueSum::zero(); + for ((_, asset_data), value) in decoded_delta.components() { + converted_delta += ValueSum::from_pair( + (asset_data.position, asset_data.token.clone()), + Change::from(*value), + ); + } + return Some(converted_delta); + } + } + None + } + /// Collect enough unspent notes in this context to exceed the given amount /// of the specified asset type. Return the total value accumulated plus /// notes and the corresponding diversifiers/merkle paths that were used to @@ -690,10 +694,8 @@ pub trait ShieldedApi: context: &impl NamadaIo, spent_notes: &mut SpentNotesTracker, sk: namada_core::masp::ExtendedSpendingKey, - is_native_token: bool, - target: I128Sum, + target: ValueSum<(MaspDigitPos, Address), Change>, target_epoch: MaspEpoch, - changes: &mut Changes, ) -> Result< ( I128Sum, @@ -707,8 +709,8 @@ pub trait ShieldedApi: // transaction to allow people to fetch less often // Establish connection with which to do exchange rate queries let mut conversions = BTreeMap::new(); - let mut val_acc = I128Sum::zero(); - let mut normed_val_acc = I128Sum::zero(); + let mut namada_acc = ValueSum::zero(); + let mut masp_acc = I128Sum::zero(); let mut notes = Vec::new(); // Retrieve the notes that can be spent by this key @@ -723,7 +725,7 @@ pub trait ShieldedApi: } // No more transaction inputs are required once we have met // the target amount - if normed_val_acc >= target { + if namada_acc >= target { break; } // Spent notes from the shielded context (i.e. from previous @@ -740,7 +742,7 @@ pub trait ShieldedApi: // The amount contributed by this note before conversion let pre_contr = I128Sum::from_pair(note.asset_type, i128::from(note.value)); - let (contr, normed_contr, proposed_convs) = self + let (contr, proposed_convs) = self .compute_exchanged_amount( context.client(), context.io(), @@ -750,28 +752,20 @@ pub trait ShieldedApi: ) .await?; - let opt_delta = if is_native_token { - None - } else { - Some(contr.clone()) - }; // Use this note only if it brings us closer to our target - if let Some(change) = is_amount_required( - normed_val_acc.clone(), - target.clone(), - normed_contr.clone(), - opt_delta, - ) { + if let Some(namada_contr) = self + .is_amount_required( + context.client(), + namada_acc.clone(), + target.clone(), + contr.clone(), + ) + .await + { // Be sure to record the conversions used in computing // accumulated value - val_acc += contr; - normed_val_acc += normed_contr; - - // Update the changes - changes - .entry(sk) - .and_modify(|amt| *amt += &change) - .or_insert(change); + masp_acc += contr; + namada_acc += namada_contr; // Commit the conversions that were used to exchange conversions = proposed_convs; @@ -799,7 +793,7 @@ pub trait ShieldedApi: } } } - Ok((val_acc, notes, conversions)) + Ok((masp_acc, notes, conversions)) } /// Convert an amount whose units are AssetTypes to one whose units are @@ -879,17 +873,20 @@ pub trait ShieldedApi: &mut self, client: &C, amt: I128Sum, - ) -> ValueSum<(AssetType, AssetData), i128> { + ) -> (ValueSum<(AssetType, AssetData), i128>, I128Sum) { let mut res = ValueSum::zero(); + let mut rem = ValueSum::zero(); for (asset_type, val) in amt.components() { // Decode the asset type if let Some(decoded) = self.decode_asset_type(client, *asset_type).await { res += ValueSum::from_pair((*asset_type, decoded), *val); + } else { + rem += ValueSum::from_pair(*asset_type, *val); } } - res + (res, rem) } /// Make shielded components to embed within a Transfer object. If no @@ -912,9 +909,6 @@ pub trait ShieldedApi: let epoch = Self::query_masp_epoch(context.client()) .await .map_err(|e| TransferErr::General(e.to_string()))?; - let native_token = Self::query_native_token(context.client()) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; // Try to get a seed from env var, if any. #[allow(unused_mut)] let mut rng = StdRng::from_rng(OsRng).unwrap(); @@ -998,13 +992,13 @@ pub trait ShieldedApi: source_data, target_data, mut denoms, - }) = Self::reorder_data_for_masp_transfer(context, data).await? + }) = Self::reorder_data_for_masp_transfer(context, data, fee_data) + .await? else { // No shielded components are needed when neither source nor // destination are shielded return Ok(None); }; - let mut changes = Changes::default(); for (MaspSourceTransferData { source, token }, amount) in &source_data { self.add_inputs( @@ -1016,8 +1010,6 @@ pub trait ShieldedApi: epoch, &denoms, &mut notes_tracker, - &mut changes, - *token == native_token, ) .await?; } @@ -1039,37 +1031,21 @@ pub trait ShieldedApi: token, amount, epoch, - &denoms, - ) - .await?; - } - - // Collect the fees if needed - if let Some(MaspFeeData { - sources, - target, - token, - amount, - }) = fee_data - { - self.add_fees( - context, - &mut builder, - &source_data, - sources, - &target, - &token, - &amount, - epoch, &mut denoms, - &mut notes_tracker, - &mut changes, ) .await?; } - // Finally, add outputs representing the change from this payment. - Self::add_changes(&mut builder, changes)?; + // Final safety check on the value balance to verify that the + // transaction is balanced + if !builder.value_balance().is_zero() { + return Result::Err(TransferErr::Build { + error: builder::Error::InsufficientFunds( + builder.value_balance(), + ), + data: None, + }); + } let builder_clone = builder.clone().map_builder(WalletMap); // Build and return the constructed transaction @@ -1100,6 +1076,25 @@ pub trait ShieldedApi: })) } + /// Either get the denomination from the cache or query it + #[allow(async_fn_in_trait)] + async fn get_denom( + client: &(impl Client + Sync), + denoms: &mut HashMap, + token: &Address, + ) -> Result { + if let Some(denom) = denoms.get(token) { + Ok(*denom) + } else if let Some(denom) = Self::query_denom(client, token).await { + denoms.insert(token.clone(), denom); + Ok(denom) + } else { + Err(TransferErr::General(format!( + "denomination for token {token}" + ))) + } + } + /// Group all the information for every source/token and target/token /// couple, and extract the denominations for all the tokens involved /// (expect the one involved in the fees if needed). This step is @@ -1110,13 +1105,42 @@ pub trait ShieldedApi: async fn reorder_data_for_masp_transfer( context: &impl NamadaIo, data: Vec, + fee_data: Option, ) -> Result, TransferErr> { - let mut source_data = - HashMap::::new(); - let mut target_data = - HashMap::::new(); + let mut source_data = HashMap::::new(); + let mut target_data = HashMap::::new(); let mut denoms = HashMap::new(); + // If present, add the fee data to the rest of the transfer data + if let Some(fee_data) = fee_data { + let denom = + Self::get_denom(context.client(), &mut denoms, &fee_data.token) + .await?; + let amount = fee_data + .amount + .increase_precision(denom) + .map_err(|e| TransferErr::General(e.to_string()))? + .amount(); + if let Some(source) = fee_data.source { + source_data.insert( + MaspSourceTransferData { + source: TransferSource::ExtendedSpendingKey(source), + token: fee_data.token.clone(), + }, + amount, + ); + } + target_data.insert( + MaspTargetTransferData { + source: fee_data.source.map(|source| { + TransferSource::ExtendedSpendingKey(source) + }), + target: TransferTarget::Address(fee_data.target), + token: fee_data.token, + }, + amount, + ); + } for MaspTransferData { source, target, @@ -1132,17 +1156,12 @@ pub trait ShieldedApi: return Ok(None); } - if denoms.get(&token).is_none() { - if let Some(denom) = - Self::query_denom(context.client(), &token).await - { - denoms.insert(token.clone(), denom); - } else { - return Err(TransferErr::General(format!( - "denomination for token {token}" - ))); - }; - } + let denom = + Self::get_denom(context.client(), &mut denoms, &token).await?; + let amount = amount + .increase_precision(denom) + .map_err(|e| TransferErr::General(e.to_string()))? + .amount(); let key = MaspSourceTransferData { source: source.clone(), @@ -1159,7 +1178,7 @@ pub trait ShieldedApi: } let key = MaspTargetTransferData { - source, + source: Some(source), target, token, }; @@ -1181,6 +1200,50 @@ pub trait ShieldedApi: })) } + /// Computes added_amt - required_amt taking care of denominations and asset + /// decodings. Error out if required_amt is not less than added_amt. + #[allow(async_fn_in_trait)] + async fn compute_change( + &mut self, + client: &(impl Client + Sync), + added_amt: I128Sum, + mut required_amt: ValueSum<(MaspDigitPos, Address), Change>, + ) -> Result { + // Compute the amount of change due to the sender. + let (decoded_amount, mut change) = + self.decode_sum(client, added_amt.clone()).await; + for ((asset_type, asset_data), value) in decoded_amount.components() { + // Get current component of the required amount + let req = required_amt + .get(&(asset_data.position, asset_data.token.clone())); + // Compute how much this decoded component covers of the requirement + let covered = std::cmp::min( + i128::try_from(req).expect("Masp digit value overflow"), + *value, + ); + // Record how far in excess of the requirement we are. This is + // change. + change += ValueSum::from_pair(*asset_type, value - covered); + // Denominate the cover and decrease the required amount accordingly + let covered = + Change::from_masp_denominated(covered, asset_data.position) + .map_err(|e| TransferErr::General(e.to_string()))?; + required_amt -= ValueSum::from_pair( + (asset_data.position, asset_data.token.clone()), + covered, + ); + } + // Error out if the required amount was not covered by the added amount + if !required_amt.is_zero() { + // TODO: zero should be replaced by required_amt in the error + return Result::Err(TransferErr::Build { + error: builder::Error::InsufficientFunds(I128Sum::zero()), + data: None, + }); + } + Ok(change) + } + /// Add the necessary transaction inputs to the builder. #[allow(async_fn_in_trait)] #[allow(clippy::too_many_arguments)] @@ -1190,57 +1253,102 @@ pub trait ShieldedApi: builder: &mut Builder, source: &TransferSource, token: &Address, - amount: &DenominatedAmount, + amount: &Amount, epoch: MaspEpoch, denoms: &HashMap, notes_tracker: &mut SpentNotesTracker, - changes: &mut Changes, - is_native_token: bool, ) -> Result, TransferErr> { // We want to fund our transaction solely from supplied spending key let spending_key = source.spending_key(); // Now we build up the transaction within this object - // Convert transaction amount into MASP types - // Ok to unwrap because we've already seen the token before, the - // denomination must be there + // Compute transparent output asset types in case they are required and + // save them to facilitate decodings. let denom = denoms.get(token).unwrap(); - let (asset_types, masp_amount) = { - // Do the actual conversion to an asset type - let amount = self - .convert_namada_amount_to_masp( - context.client(), - epoch, - token, - denom.to_owned(), - amount.amount(), - ) + let mut transparent_asset_types = Vec::new(); + for digit in MaspDigitPos::iter() { + let mut pre_asset_type = AssetData { + epoch: Some(epoch), + token: token.clone(), + denom: *denom, + position: digit, + }; + let asset_type = self + .get_asset_type(context.client(), &mut pre_asset_type) .await .map_err(|e| TransferErr::General(e.to_string()))?; - // Make sure to save any decodings of the asset types used so - // that balance queries involving them are - // successful - let _ = self.save().await; - amount - }; + transparent_asset_types.push((digit, asset_type)); + } + let _ = self.save().await; // If there are shielded inputs let added_amt = if let Some(sk) = spending_key { + // Compute the target amount to spend from the + // input token address and namada amount. We + // collect all words from the namada amount + // whose values are non-zero, and pair them + // with their corresponding masp digit position. + let required_amt = amount + .iter_words() + .enumerate() + .filter_map(|(masp_digit_pos, value)| { + if value != 0 { + Some(( + MaspDigitPos::from(masp_digit_pos), + Change::from(value), + )) + } else { + None + } + }) + .fold( + ValueSum::zero(), + |mut accum, (masp_digit_pos, value)| { + accum += ValueSum::from_pair( + (masp_digit_pos, token.clone()), + value, + ); + accum + }, + ); // Locate unspent notes that can help us meet the transaction // amount - let (added_amount, unspent_notes, used_convs) = self + let (added_amt, unspent_notes, used_convs) = self .collect_unspent_notes( context, notes_tracker, sk, - is_native_token, - I128Sum::from_sum(masp_amount), + required_amt.clone(), epoch, - changes, ) .await .map_err(|e| TransferErr::General(e.to_string()))?; + + // Compute the change that needs to go back to the sender + let change = self + .compute_change( + context.client(), + added_amt.clone(), + required_amt, + ) + .await?; + // Commit the computed change back to the sender. + for (asset_type, value) in change.components() { + builder + .add_sapling_output( + Some(MaspExtendedSpendingKey::from(sk).expsk.ovk), + MaspExtendedSpendingKey::from(sk).default_address().1, + *asset_type, + *value as u64, + MemoBytes::empty(), + ) + .map_err(|e| TransferErr::Build { + error: builder::Error::SaplingBuild(e), + data: None, + })?; + } + // Commit the notes found to our transaction for (diversifier, note, merkle_path) in unspent_notes { builder @@ -1271,7 +1379,7 @@ pub trait ShieldedApi: } } - Some(added_amount) + Some(added_amt) } else { // We add a dummy UTXO to our transaction, but only the source // of the parent Transfer object is used to @@ -1285,15 +1393,13 @@ pub trait ShieldedApi: })? .taddress(); - for (digit, asset_type) in - MaspDigitPos::iter().zip(asset_types.iter()) - { - let amount_part = digit.denominate(&amount.amount()); + for (digit, asset_type) in transparent_asset_types { + let amount_part = digit.denominate(amount); // Skip adding an input if its value is 0 if amount_part != 0 { builder .add_transparent_input(TxOut { - asset_type: *asset_type, + asset_type, value: amount_part, address: script, }) @@ -1317,30 +1423,31 @@ pub trait ShieldedApi: &mut self, context: &impl NamadaIo, builder: &mut Builder, - source: TransferSource, + source: Option, target: &TransferTarget, token: Address, - amount: DenominatedAmount, + amount: Amount, epoch: MaspEpoch, - denoms: &HashMap, + denoms: &mut HashMap, ) -> Result<(), TransferErr> { // Anotate the asset type in the value balance with its decoding in // order to facilitate cross-epoch computations - let value_balance = self + let (value_balance, rem_balance) = self .decode_sum(context.client(), builder.value_balance()) .await; + assert!( + rem_balance.is_zero(), + "no undecodable asset types should remain at this point", + ); let payment_address = target.payment_address(); // This indicates how many more assets need to be sent to the // receiver in order to satisfy the requested transfer // amount. - let mut rem_amount = amount.amount().raw_amount().0; - - // Ok to unwrap cause we've already seen the token before, the - // denomination must be there - let denom = denoms.get(&token).unwrap(); + let mut rem_amount = amount.raw_amount().0; + let denom = Self::get_denom(context.client(), denoms, &token).await?; // Now handle the outputs of this transaction // Loop through the value balance components and see which // ones can be given to the receiver @@ -1349,7 +1456,7 @@ pub trait ShieldedApi: // Only asset types with the correct token can contribute. But // there must be a demonstrated need for it. if decoded.token == token - && &decoded.denom == denom + && decoded.denom == denom && decoded.epoch.map_or(true, |vbal_epoch| vbal_epoch <= epoch) && *rem_amount > 0 { @@ -1362,9 +1469,11 @@ pub trait ShieldedApi: let contr = std::cmp::min(u128::from(*rem_amount), val) as u64; // If we are sending to a shielded address, we need the outgoing // viewing key in the following computations. - let ovk_opt = source - .spending_key() - .map(|x| MaspExtendedSpendingKey::from(x).expsk.ovk); + let ovk_opt = source.clone().and_then(|source| { + source + .spending_key() + .map(|x| MaspExtendedSpendingKey::from(x).expsk.ovk) + }); // Make transaction output tied to the current token, // denomination, and epoch. if let Some(pa) = payment_address { @@ -1415,7 +1524,7 @@ pub trait ShieldedApi: epoch, &token, denom.to_owned(), - amount.amount(), + amount, ) .await .map_err(|e| TransferErr::General(e.to_string()))?; @@ -1435,7 +1544,7 @@ pub trait ShieldedApi: return Result::Err(TransferErr::Build { error: builder::Error::InsufficientFunds(shortfall), data: Some(MaspDataLog { - source: Some(source), + source, token, amount, }), @@ -1445,297 +1554,6 @@ pub trait ShieldedApi: Ok(()) } - /// Add the necessary note to include a masp fee payment in the transaction. - /// Funds are gathered in the following order: - /// - /// 1. From the residual values of the already included spend notes (i.e. - /// changes) - /// 2. From new spend notes of the transaction's sources - /// 3. From new spend notes of the optional gas spending keys - #[allow(clippy::too_many_arguments)] - #[allow(async_fn_in_trait)] - async fn add_fees( - &mut self, - context: &impl NamadaIo, - builder: &mut Builder, - source_data: &HashMap, - sources: Vec, - target: &Address, - token: &Address, - amount: &DenominatedAmount, - epoch: MaspEpoch, - denoms: &mut HashMap, - notes_tracker: &mut SpentNotesTracker, - changes: &mut Changes, - ) -> Result<(), TransferErr> { - if denoms.get(token).is_none() { - if let Some(denom) = - Self::query_denom(context.client(), token).await - { - denoms.insert(token.to_owned(), denom); - } else { - return Err(TransferErr::General(format!( - "denomination for token {token}" - ))); - }; - } - let native_token = Self::query_native_token(context.client()) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - let raw_amount = amount.amount().raw_amount().0; - let (asset_types, _) = { - // Do the actual conversion to an asset type - let (asset_types, amount) = self - .convert_namada_amount_to_masp( - context.client(), - epoch, - token, - // Safe to unwrap - denoms.get(token).unwrap().to_owned(), - amount.amount(), - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - // Make sure to save any decodings of the asset types used so - // that balance queries involving them are - // successful - let _ = self.save().await; - (asset_types, amount) - }; - - let mut fees = I128Sum::zero(); - // Convert the shortfall into a I128Sum - for (asset_type, val) in asset_types.iter().zip(raw_amount) { - fees += I128Sum::from_nonnegative(*asset_type, val.into()) - .map_err(|()| { - TransferErr::General( - "Fee amount is expected expected to be non-negative" - .to_string(), - ) - })?; - } - - // 1. Try to use the change to pay fees - let mut temp_changes = Changes::default(); - - for (sp, changes) in changes.iter() { - for (asset_type, change) in changes.components() { - for (_, fee_amt) in fees - .clone() - .components() - .filter(|(axt, _)| *axt == asset_type) - { - // Get the minimum between the available change and - // the due fee - let output_amt = I128Sum::from_nonnegative( - asset_type.to_owned(), - *change.min(fee_amt), - ) - .map_err(|()| { - TransferErr::General( - "Fee amount is expected to be non-negative" - .to_string(), - ) - })?; - let denominated_output_amt = self - .convert_masp_amount_to_namada( - context.client(), - // Safe to unwrap - denoms.get(token).unwrap().to_owned(), - output_amt.clone(), - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - - self.add_outputs( - context, - builder, - TransferSource::ExtendedSpendingKey(sp.to_owned()), - &TransferTarget::Address(target.clone()), - token.clone(), - denominated_output_amt, - epoch, - denoms, - ) - .await?; - - fees -= &output_amt; - // Update the changes - temp_changes - .entry(*sp) - .and_modify(|amt| *amt += &output_amt) - .or_insert(output_amt); - } - } - - if fees.is_zero() { - break; - } - } - - // Decrease the changes by the amounts used for fee payment - for (sp, temp_changes) in temp_changes.iter() { - for (asset_type, temp_change) in temp_changes.components() { - let output_amt = I128Sum::from_nonnegative( - asset_type.to_owned(), - *temp_change, - ) - .map_err(|()| { - TransferErr::General( - "Fee amount is expected expected to be non-negative" - .to_string(), - ) - })?; - - // Entry is guaranteed to be in the map - changes.entry(*sp).and_modify(|amt| *amt -= &output_amt); - } - } - - if !fees.is_zero() { - // 2. Look for unused spent notes of the sources and the optional - // gas spending keys (sources first) - for fee_source in - source_data.iter().map(|(src, _)| src.source.clone()).chain( - sources - .into_iter() - .map(TransferSource::ExtendedSpendingKey), - ) - { - for (asset_type, fee_amt) in fees.clone().components() { - let input_amt = I128Sum::from_nonnegative( - asset_type.to_owned(), - *fee_amt, - ) - .map_err(|()| { - TransferErr::General( - "Fee amount is expected expected to be \ - non-negative" - .to_string(), - ) - })?; - let denominated_fee = self - .convert_masp_amount_to_namada( - context.client(), - // Safe to unwrap - denoms.get(token).unwrap().to_owned(), - input_amt.clone(), - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - - let Some(found_amt) = self - .add_inputs( - context, - builder, - &fee_source, - token, - &denominated_fee, - epoch, - denoms, - notes_tracker, - changes, - *token == native_token, - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))? - else { - continue; - }; - // Pick the minimum between the due fee and the amount found - let output_amt = match found_amt.partial_cmp(&input_amt) { - None | Some(Ordering::Less) => found_amt, - _ => input_amt.clone(), - }; - let denom_amt = self - .convert_masp_amount_to_namada( - context.client(), - // Safe to unwrap - denoms.get(token).unwrap().to_owned(), - output_amt.clone(), - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - - self.add_outputs( - context, - builder, - fee_source.clone(), - &TransferTarget::Address(target.clone()), - token.clone(), - denom_amt, - epoch, - denoms, - ) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - - fees -= &output_amt; - } - - if fees.is_zero() { - break; - } - } - } - - if !fees.is_zero() { - return Result::Err(TransferErr::Build { - error: builder::Error::InsufficientFunds(fees), - data: Some(MaspDataLog { - source: None, - token: token.to_owned(), - amount: *amount, - }), - }); - } - - Ok(()) - } - - /// Consumes the changes and adds them back to the original sources to - /// balance the transaction. This function has to be called after - /// `add_fees` because we might have some change coming from there too - #[allow(clippy::result_large_err)] - #[allow(async_fn_in_trait)] - fn add_changes( - builder: &mut Builder, - changes: Changes, - ) -> Result<(), TransferErr> { - for (sp, changes) in changes.into_iter() { - for (asset_type, amt) in changes.components() { - if let Ordering::Greater = amt.cmp(&0) { - let sk = MaspExtendedSpendingKey::from(sp.to_owned()); - // Send the change in this asset type back to the sender - builder - .add_sapling_output( - Some(sk.expsk.ovk), - sk.default_address().1, - *asset_type, - *amt as u64, - MemoBytes::empty(), - ) - .map_err(|e| TransferErr::Build { - error: builder::Error::SaplingBuild(e), - data: None, - })?; - } - } - } - - // Final safety check on the value balance to verify that the - // transaction is balanced - let value_balance = builder.value_balance(); - if !value_balance.is_zero() { - return Result::Err(TransferErr::Build { - error: builder::Error::InsufficientFunds(value_balance), - data: None, - }); - } - - Ok(()) - } - /// Get the asset type with the given epoch, token, and denomination. If it /// does not exist in the protocol, then remove the timestamp. Make sure to /// store the derived AssetType so that future decoding is possible. @@ -1794,27 +1612,6 @@ pub trait ShieldedApi: amount, )) } - - /// Convert MASP amount to Namada equivalent - #[allow(async_fn_in_trait)] - async fn convert_masp_amount_to_namada( - &mut self, - client: &C, - denom: Denomination, - amt: I128Sum, - ) -> Result { - let mut amount = Amount::zero(); - let value_sum = self.decode_sum(client, amt).await; - - for ((_, decoded), val) in value_sum.components() { - let positioned_amt = - Amount::from_masp_denominated_i128(*val, decoded.position) - .unwrap_or_default(); - amount = checked!(amount + positioned_amt)?; - } - - Ok(DenominatedAmount::new(amount, denom)) - } } impl> diff --git a/crates/shielded_token/src/storage.rs b/crates/shielded_token/src/storage.rs index 50aab32021..b0c5e1597b 100644 --- a/crates/shielded_token/src/storage.rs +++ b/crates/shielded_token/src/storage.rs @@ -1,5 +1,6 @@ use namada_core::address::{self, Address}; use namada_core::arith::checked; +use namada_core::masp::TokenMap; use namada_core::token; use namada_core::token::Amount; use namada_core::uint::Uint; @@ -77,3 +78,21 @@ where storage.read(&total_rewards_key)?.unwrap_or_default(); Ok(total_rewards) } + +/// Read the masp token map. +pub fn read_token_map(storage: &S) -> Result +where + S: StorageRead, +{ + let token_map_key = masp_token_map_key(); + Ok(storage.read(&token_map_key)?.unwrap_or_default()) +} + +/// Write a new masp token map. +pub fn write_token_map(storage: &mut S, token_map: TokenMap) -> Result<()> +where + S: StorageWrite, +{ + let token_map_key = masp_token_map_key(); + storage.write(&token_map_key, token_map) +} diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 75c9a95802..f53f523e4b 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -6,6 +6,7 @@ use color_eyre::owo_colors::OwoColorize; use namada_apps_lib::wallet::defaults::{ albert_keypair, bertha_keypair, christel_keypair, }; +use namada_core::address::Address; use namada_core::dec::Dec; use namada_core::masp::TokenMap; use namada_node::shell::testing::client::run; @@ -17,12 +18,12 @@ use namada_sdk::signing::SigningTxData; use namada_sdk::state::{StorageRead, StorageWrite}; use namada_sdk::time::DateTimeUtc; use namada_sdk::token::storage_key::masp_token_map_key; -use namada_sdk::token::{self, DenominatedAmount}; +use namada_sdk::token::{self, Amount, DenominatedAmount}; use namada_sdk::tx::Tx; use namada_sdk::{tx, DEFAULT_GAS_LIMIT}; use test_log::test; -use super::setup; +use super::{helpers, setup}; use crate::e2e::setup::constants::{ AA_PAYMENT_ADDRESS, AA_VIEWING_KEY, AB_PAYMENT_ADDRESS, AB_VIEWING_KEY, AC_PAYMENT_ADDRESS, AC_VIEWING_KEY, ALBERT, ALBERT_KEY, A_SPENDING_KEY, @@ -31,6 +32,483 @@ use crate::e2e::setup::constants::{ }; use crate::strings::TX_APPLIED_SUCCESS; +/// Enable masp rewards after some token had already been shielded. +#[test] +fn enable_rewards_after_shielding() -> Result<()> { + // Dummy validator rpc address + const RPC: &str = "http://127.0.0.1:26567"; + + // We will mint tokens with this address + const TEST_TOKEN_ADDR: &str = + "tnam1q9382etwdaekg6tpwdkkzar0wd5ku6r0wvu5ukqd"; + let test_token_addr: Address = TEST_TOKEN_ADDR.parse().unwrap(); + + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + // Boot up a mock node + let (mut node, _services) = setup::setup()?; + + // Initialize the test token + token::write_denom( + &mut node.shell.lock().unwrap().state, + &test_token_addr, + 0u8.into(), + )?; + + // Give Bertha some test tokens + let bertha_addr = helpers::find_address(&node, BERTHA)?; + token::credit_tokens( + &mut node.shell.lock().unwrap().state, + &test_token_addr, + &bertha_addr, + Amount::from_u64(1_000_000_000u64), + )?; + + // Commit test token changes to a new block + node.finalize_and_commit(None); + assert_eq!( + token::read_total_supply( + &node.shell.lock().unwrap().state, + &test_token_addr, + )?, + Amount::from_u64(1_000_000_000u64), + ); + + // Shield 1_000_000 test tokens + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + BERTHA, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + TEST_TOKEN_ADDR, + "--amount", + "1000000", + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch the note we just created containing test tokens + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check the test token balance corresponds 1:1 to what + // we shielded + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + TEST_TOKEN_ADDR, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("{TEST_TOKEN_ADDR}: 1000000"))); + + // Skip a couple of masp epochs + for _ in 0..3 { + node.next_masp_epoch(); + } + + // The balance shouldn't have changed + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + TEST_TOKEN_ADDR, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("{TEST_TOKEN_ADDR}: 1000000"))); + + // Check that our NAM balance is null + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains("nam: 0")); + + // Let us now start minting NAM rewards for any + // test tokens in the shielded pool + token::write_params( + &Some(token::ShieldedParams { + max_reward_rate: Dec::from_str("9999999999").unwrap(), + kp_gain_nom: Dec::from_str("9999999999").unwrap(), + kd_gain_nom: Dec::from_str("9999999999").unwrap(), + locked_amount_target: 999999999u64, + }), + &mut node.shell.lock().unwrap().state, + &test_token_addr, + &0u8.into(), + )?; + let mut token_map = + token::read_token_map(&node.shell.lock().unwrap().state)?; + token_map.insert("TEST".to_owned(), test_token_addr.clone()); + token::write_token_map(&mut node.shell.lock().unwrap().state, token_map)?; + node.finalize_and_commit(None); + + // Unshield and reshield some test tokens, such that they + // are now tagged with a masp epoch + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "unshield", + "--source", + A_SPENDING_KEY, + "--target", + BERTHA, + "--token", + TEST_TOKEN_ADDR, + "--amount", + "1000000", + "--signing-keys", + BERTHA_KEY, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch the latest test token notes + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check that the balance is now 0 + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + TEST_TOKEN_ADDR, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("{TEST_TOKEN_ADDR}: 0"))); + + // + node.next_masp_epoch(); + + // Reshield + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + BERTHA, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + TEST_TOKEN_ADDR, + "--amount", + "1000000", + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch the latest test token notes + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check that we have some shielded test tokens once more + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + TEST_TOKEN_ADDR, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("{TEST_TOKEN_ADDR}: 1000000"))); + + // Skip a couple of masp epochs + for _ in 0..3 { + node.next_masp_epoch(); + } + + // Assert that we have minted NAM rewards + const EXPECTED_REWARDS: u128 = 6427858447239330; + const HALF_OF_EXPECTED_REWARDS: u128 = EXPECTED_REWARDS / 2; + + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("nam: {EXPECTED_REWARDS}"))); + + // Unshield half of the rewards + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "unshield", + "--source", + A_SPENDING_KEY, + "--target", + BERTHA, + "--token", + NAM, + "--amount", + &HALF_OF_EXPECTED_REWARDS.to_string(), + "--signing-keys", + BERTHA_KEY, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch latest shielded state + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check that we now have half of the rewards + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("nam: {HALF_OF_EXPECTED_REWARDS}"))); + + // Transfer the other half of the rewards + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "transfer", + "--source", + A_SPENDING_KEY, + "--target", + AB_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + &HALF_OF_EXPECTED_REWARDS.to_string(), + "--signing-keys", + BERTHA_KEY, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch latest shielded state + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check that we now a null NAM balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains("nam: 0")); + + // Unshield half of our test tokens + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "unshield", + "--source", + A_SPENDING_KEY, + "--target", + BERTHA, + "--token", + TEST_TOKEN_ADDR, + "--amount", + "500000", + "--signing-keys", + BERTHA_KEY, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Fetch latest shielded state + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + RPC, + ], + )?; + + // Check test token balance + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + TEST_TOKEN_ADDR, + "--node", + RPC, + ], + ) + }); + assert!(captured.result.is_ok(), "{:?}", captured.result); + assert!(captured.contains(&format!("{TEST_TOKEN_ADDR}: 500000"))); + + Ok(()) +} + /// In this test we verify that users of the MASP receive the correct rewards /// for leaving their assets in the pool for varying periods of time. #[test] @@ -2369,6 +2847,63 @@ fn dynamic_assets() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 0.156705")); + // Unshield the rewards + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "unshield", + "--source", + A_SPENDING_KEY, + "--target", + BERTHA, + "--token", + NAM, + "--amount", + "0.156705", + "--gas-payer", + BERTHA_KEY, + "--ledger-address", + 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", "--node", validator_one_rpc], + )?; + + // Unshield the principal + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "unshield", + "--source", + A_SPENDING_KEY, + "--target", + BERTHA, + "--token", + BTC, + "--amount", + "2", + "--gas-payer", + BERTHA_KEY, + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + Ok(()) } @@ -2620,6 +3155,8 @@ fn masp_fee_payment() -> Result<()> { "10000", "--gas-price", "1", + "--gas-spending-key", + A_SPENDING_KEY, "--disposable-gas-payer", "--ledger-address", validator_one_rpc, @@ -2958,6 +3495,8 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { "1", "--gas-payer", ALBERT_KEY, + "--gas-spending-key", + A_SPENDING_KEY, "--ledger-address", validator_one_rpc, ], @@ -3208,7 +3747,7 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0")); + assert!(captured.contains("nam: 1000")); let captured = CapturedOutput::of(|| { run( @@ -3226,7 +3765,7 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1000")); + assert!(captured.contains("nam: 0")); let captured = CapturedOutput::of(|| { run( @@ -3517,7 +4056,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("btc: 0")); + assert!(captured.contains("btc: 1000")); let captured = CapturedOutput::of(|| { run( @@ -3535,7 +4074,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("btc: 1000")); + assert!(captured.contains("btc: 0")); Ok(()) }