diff --git a/.changelog/unreleased/bug-fixes/4304-track-undated-assets.md b/.changelog/unreleased/bug-fixes/4304-track-undated-assets.md new file mode 100644 index 0000000000..f7ebb78d16 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/4304-track-undated-assets.md @@ -0,0 +1,2 @@ +- Stop the minting of unclaimable shielded rewards for undated assets. + ([\#4304](https://github.com/anoma/namada/issues/4304)) \ No newline at end of file diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 3fd82b5754..96b7e0468a 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -488,8 +488,12 @@ async fn query_shielded_balance( { let mut shielded = context.shielded_mut().await; let _ = shielded.load().await; + // Precompute asset types to increase chances of success in decoding + let token_map = context.wallet().await.get_addresses(); + let mut tokens: BTreeSet<_> = token_map.values().collect(); + tokens.insert(&token); let _ = shielded - .precompute_asset_types(context.client(), vec![&token]) + .precompute_asset_types(context.client(), tokens) .await; // Save the update state so that future fetches can be short-circuited let _ = shielded.save().await; diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index b74f8d1b90..78fd744a86 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -28,7 +28,7 @@ use crate::string_encoding::{ self, MASP_EXT_FULL_VIEWING_KEY_HRP, MASP_EXT_SPENDING_KEY_HRP, MASP_PAYMENT_ADDRESS_HRP, }; -use crate::token::{Denomination, MaspDigitPos}; +use crate::token::{Denomination, MaspDigitPos, NATIVE_MAX_DECIMAL_PLACES}; /// Serialize the given TxId pub fn serialize_txid(txid: &TxIdInner, s: S) -> Result @@ -249,11 +249,15 @@ pub fn encode_asset_type( .encode() } -/// Encode the assets that are used for masp rewards +/// Encode the assets that are used for masp rewards. The address supplied to +/// this function must be that of the native token. pub fn encode_reward_asset_types( native_token: &Address, ) -> Result<[AssetType; 4], std::io::Error> { - use crate::token::{MaspDigitPos, NATIVE_MAX_DECIMAL_PLACES}; + // Construct MASP asset type for rewards. Always deflate and timestamp + // reward tokens with the zeroth epoch to minimize the number of convert + // notes clients have to use. This trick works under the assumption that + // reward tokens will then be reinflated back to the current epoch. Ok([ encode_asset_type( native_token.clone(), diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 1b376a1fda..dddff458c7 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -571,6 +571,17 @@ where Ok(accum) } +/// Internal transfer data extracted from the wrapping IBC transaction +#[derive(Debug)] +pub struct InternalData { + /// The transparent transfer that happens in parallel to IBC processes + pub transparent: Option, + /// The shielded transaction that happens in parallel to IBC processes + pub shielded: Option, + /// IBC tokens that are credited/debited to internal accounts + pub ibc_tokens: BTreeSet
, +} + /// IBC actions to handle IBC operations #[derive(Debug)] pub struct IbcActions<'a, C, Params, Token> @@ -616,7 +627,7 @@ where pub fn execute( &mut self, tx_data: &[u8], - ) -> Result<(Option, Option), Error> { + ) -> Result, Error> { let message = decode_message::(tx_data)?; let result = match message { IbcMessage::Transfer(msg) => { @@ -642,13 +653,22 @@ where if msg.transfer.is_some() { token_transfer_ctx.enable_shielded_transfer(); } + // Record the token credited/debited in this transfer + let denom = msg.message.packet_data.token.denom.to_string(); + let token = convert_to_address(denom) + .into_storage_result() + .map_err(Error::Storage)?; send_transfer_execute( &mut self.ctx, &mut token_transfer_ctx, msg.message, ) .map_err(Error::TokenTransfer)?; - Ok((msg.transfer, None)) + Ok(InternalData { + transparent: msg.transfer, + shielded: None, + ibc_tokens: [token].into(), + }) } IbcMessage::NftTransfer(msg) => { let mut nft_transfer_ctx = @@ -671,13 +691,34 @@ where ) })?, ); + // Record the tokens credited/debited in this NFT transfer + let tokens = msg + .message + .packet_data + .token_ids + .0 + .iter() + .map(|token_id| { + convert_to_address(ibc_trace_for_nft( + &msg.message.packet_data.class_id, + token_id, + )) + .into_storage_result() + .map_err(Error::Storage) + }) + .collect::>()?; + send_nft_transfer_execute( &mut self.ctx, &mut nft_transfer_ctx, msg.message, ) .map_err(Error::NftTransfer)?; - Ok((msg.transfer, None)) + Ok(InternalData { + transparent: msg.transfer, + shielded: None, + ibc_tokens: tokens, + }) } IbcMessage::Envelope(envelope) => { if let Some(verifier) = get_envelope_verifier(envelope.as_ref()) @@ -695,24 +736,45 @@ where .map_err(|e| Error::Handler(Box::new(e)))?; // Extract MASP tx from the memo in the packet if needed - let masp_tx = match &*envelope { + let (masp_tx, tokens) = match &*envelope { MsgEnvelope::Packet(PacketMsg::Recv(msg)) if self .is_receiving_success(msg)? .is_some_and(|ack_succ| ack_succ) => { - extract_masp_tx_from_packet(&msg.packet) + let ibc_traces = extract_traces_from_recv_msg(msg) + .map_err(StorageError::new) + .map_err(Error::Storage)?; + let mut tokens = BTreeSet::new(); + for ibc_trace in ibc_traces { + // Get the received token + let token = received_ibc_token( + ibc_trace, + &msg.packet.port_id_on_a, + &msg.packet.chan_id_on_a, + &msg.packet.port_id_on_b, + &msg.packet.chan_id_on_b, + ) + .into_storage_result() + .map_err(Error::Storage)?; + tokens.insert(token); + } + (extract_masp_tx_from_packet(&msg.packet), tokens) } #[cfg(is_apple_silicon)] MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { // NOTE: This is unneeded but wasm compilation error // happened if deleted on macOS with Apple Silicon let _ = extract_masp_tx_from_packet(&msg.packet); - None + (None, BTreeSet::new()) } - _ => None, + _ => (None, BTreeSet::new()), }; - Ok((None, masp_tx)) + Ok(InternalData { + transparent: None, + shielded: masp_tx, + ibc_tokens: tokens, + }) } }; self.insert_verifiers()?; diff --git a/crates/ibc/src/msg.rs b/crates/ibc/src/msg.rs index c5d75407df..77527bda17 100644 --- a/crates/ibc/src/msg.rs +++ b/crates/ibc/src/msg.rs @@ -11,7 +11,9 @@ use ibc::apps::nft_transfer::types::PORT_ID_STR as NFT_PORT_ID_STR; use ibc::apps::transfer::types::msgs::transfer::MsgTransfer as IbcMsgTransfer; use ibc::apps::transfer::types::packet::PacketData; use ibc::apps::transfer::types::PORT_ID_STR as FT_PORT_ID_STR; -use ibc::core::channel::types::msgs::PacketMsg; +use ibc::core::channel::types::msgs::{ + MsgRecvPacket as IbcMsgRecvPacket, PacketMsg, +}; use ibc::core::channel::types::packet::Packet; use ibc::core::handler::types::msgs::MsgEnvelope; use ibc::core::host::types::identifiers::PortId; @@ -21,6 +23,8 @@ use namada_core::borsh::BorshSerializeExt; use namada_core::string_encoding::StringEncoded; use serde::{Deserialize, Serialize}; +use crate::trace; + trait Sealed {} /// Marker trait that denotes whether an IBC memo is valid @@ -327,6 +331,38 @@ fn extract_memo_from_packet( } } +/// Extract the traces used in a receive packet message +pub fn extract_traces_from_recv_msg( + msg: &IbcMsgRecvPacket, +) -> Result, serde_json::Error> { + match msg.packet.port_id_on_b.as_str() { + FT_PORT_ID_STR => { + // Record the token credited/debited in this + // transfer + let packet_data = + serde_json::from_slice::(&msg.packet.data)?; + let ibc_denom = packet_data.token.denom.to_string(); + Ok([ibc_denom].into()) + } + NFT_PORT_ID_STR => { + // Record the tokens credited/debited in this NFT + // transfer + let packet_data = + serde_json::from_slice::(&msg.packet.data)?; + let ibc_traces: Vec<_> = packet_data + .token_ids + .0 + .iter() + .map(|token_id| { + trace::ibc_trace_for_nft(&packet_data.class_id, token_id) + }) + .collect(); + Ok(ibc_traces) + } + _ => Ok(vec![]), + } +} + /// Get IBC memo string from MASP transaction for receiving pub fn convert_masp_tx_to_ibc_memo(transaction: &MaspTransaction) -> String { IbcShieldingData(transaction.clone()).into() diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 69a472f025..91c1ebe3c4 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -104,6 +104,28 @@ where Ok((checked!(10u128 ^ precision_denom)?, denomination)) } +/// Get the balance of the given token at the MASP address that is eligble to +/// receive rewards. +fn get_masp_dated_balance( + storage: &mut S, + token: &Address, +) -> Result +where + S: StorageWrite + StorageRead, + TransToken: trans_token::Keys + trans_token::Read, +{ + use crate::read_undated_balance; + let masp_addr = MASP; + + // total locked amount in the Shielded pool + let total_tokens_in_masp = + TransToken::read_balance(storage, token, &masp_addr)?; + // Since dated and undated tokens are stored together in the pool, subtract + // the latter to get the dated balance + let masp_undated_balance = read_undated_balance(storage, token)?; + Ok(checked!(total_tokens_in_masp - masp_undated_balance)?) +} + /// Compute the MASP rewards by applying the PD-controller to the genesis /// parameters and the last inflation and last locked rewards ratio values. pub fn calculate_masp_rewards( @@ -173,17 +195,21 @@ where last_locked_dec, ); + // total locked amount in the Shielded pool + let rewardable_tokens_in_masp = + get_masp_dated_balance::(storage, token)?; + // inflation-per-token = inflation / locked tokens = n/PRECISION // ∴ n = (inflation * PRECISION) / locked tokens // Since we must put the notes in a compatible format with the // note format, we must make the inflation amount discrete. - let noterized_inflation = if total_tokens_in_masp.is_zero() { + let noterized_inflation = if rewardable_tokens_in_masp.is_zero() { 0u128 } else { inflation .checked_mul_div( Uint::from(precision), - total_tokens_in_masp.raw_amount(), + rewardable_tokens_in_masp.raw_amount(), ) .and_then(|x| x.0.try_into().ok()) .unwrap_or_else(|| { @@ -198,7 +224,7 @@ where }; let inflation_amount = Amount::from_uint( checked!( - total_tokens_in_masp.raw_amount() / precision.into() + rewardable_tokens_in_masp.raw_amount() / precision.into() * Uint::from(noterized_inflation) )?, 0, @@ -339,7 +365,7 @@ where ); } // Dispense a transparent reward in parallel to the shielded rewards - let addr_bal = TransToken::read_balance(storage, token, &MASP)?; + let addr_bal = get_masp_dated_balance::(storage, token)?; // The reward for each reward.1 units of the current asset // is reward.0 units of the reward token let native_reward = addr_bal @@ -461,7 +487,7 @@ where ); } // Dispense a transparent reward in parallel to the shielded rewards - let addr_bal = TransToken::read_balance(storage, token, &MASP)?; + let addr_bal = get_masp_dated_balance::(storage, token)?; // The reward for each reward.1 units of the current asset // is reward.0 units of the reward token *total_reward = total_reward @@ -523,10 +549,6 @@ where // The total transparent value of the rewards being distributed let mut total_reward = Amount::zero(); - // Construct MASP asset type for rewards. Always deflate and timestamp - // reward tokens with the zeroth epoch to minimize the number of convert - // notes clients have to use. This trick works under the assumption that - // reward tokens will then be reinflated back to the current epoch. let reward_assets = encode_reward_asset_types(&native_token).into_storage_result()?; // Conversions from the previous to current asset for each address diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index 333a5e5a5f..539b40a8e5 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -437,7 +437,7 @@ pub trait ShieldedApi: async fn precompute_asset_types( &mut self, client: &C, - tokens: Vec<&Address>, + tokens: BTreeSet<&Address>, ) -> Result<(), eyre::Error> { // To facilitate lookups of human-readable token names for token in tokens { @@ -1777,10 +1777,10 @@ pub trait ShieldedApi: .encode() .map_err(|_| eyre!("unable to create asset type"))?; if self.decode_asset_type(client, asset_type).await.is_none() { - // If we fail to decode the epoched asset type, then remove the + // If we fail to decode the dated asset type, then remove the // epoch tracing::debug!( - "Failed to decode epoched asset type, undating it: {:#?}", + "Failed to decode dated asset type, undating it: {:#?}", decoded ); *decoded = decoded.clone().undate(); diff --git a/crates/shielded_token/src/storage.rs b/crates/shielded_token/src/storage.rs index b0c5e1597b..c79bf20dd9 100644 --- a/crates/shielded_token/src/storage.rs +++ b/crates/shielded_token/src/storage.rs @@ -79,6 +79,20 @@ where Ok(total_rewards) } +/// Read the undated balance of the given token in the MASP. +pub fn read_undated_balance( + storage: &S, + token_address: &Address, +) -> Result +where + S: StorageRead, +{ + let undated_balance_key = masp_undated_balance_key(token_address); + let undated_balance: token::Amount = + storage.read(&undated_balance_key)?.unwrap_or_default(); + Ok(undated_balance) +} + /// Read the masp token map. pub fn read_token_map(storage: &S) -> Result where diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index f79d5865f0..c6cd05d1cd 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -1,5 +1,7 @@ //! Shielded token storage keys +use std::str::FromStr; + use masp_primitives::bls12_381::Scalar; use masp_primitives::sapling::Nullifier; use namada_core::address::{self, Address}; @@ -11,6 +13,8 @@ use namada_systems::trans_token; const BALANCE_STORAGE_KEY: &str = "balance"; /// Key segment prefix for the nullifiers pub const MASP_NULLIFIERS_KEY: &str = "nullifiers"; +/// The key for the masp reward balance +pub const MASP_UNDATED_BALANCE_KEY: &str = "undated_balance"; /// Key segment prefix for the note commitment merkle tree pub const MASP_NOTE_COMMITMENT_TREE_KEY: &str = "commitment_tree"; /// Key segment prefix for the note commitment anchor @@ -85,6 +89,27 @@ pub fn masp_last_inflation_key( .with_segment(MASP_LAST_INFLATION_KEY.to_owned()) } +/// Check if the given storage key is a masp reward balance key +pub fn is_masp_undated_balance_key(key: &storage::Key) -> Option
{ + match &key.segments[..] { + [ + DbKeySeg::AddressSeg(address::MASP), + DbKeySeg::StringSeg(prefix), + DbKeySeg::StringSeg(token), + ] if prefix == MASP_UNDATED_BALANCE_KEY => { + Address::from_str(token).ok() + } + _ => None, + } +} + +/// Obtain the storage key for the undated balance of a token +pub fn masp_undated_balance_key(token_address: &Address) -> storage::Key { + storage::Key::from(address::MASP.to_db_key()) + .with_segment(MASP_UNDATED_BALANCE_KEY.to_owned()) + .with_segment(token_address.to_string().to_db_key()) +} + /// Check if the given storage key is MASP transparent balance key pub fn is_masp_balance_key(key: &storage::Key) -> bool { matches!( @@ -115,12 +140,11 @@ pub fn is_masp_transfer_key(key: &storage::Key) -> bool { { true } - [ - DbKeySeg::AddressSeg(addr), - DbKeySeg::StringSeg(key), - DbKeySeg::StringSeg(_nullifier), - ] if *addr == address::MASP && key == MASP_NULLIFIERS_KEY => true, - _ => is_masp_balance_key(key), + _ => { + is_masp_nullifier_key(key) + || is_masp_balance_key(key) + || is_masp_undated_balance_key(key).is_some() + } } } @@ -129,7 +153,7 @@ pub fn is_masp_nullifier_key(key: &storage::Key) -> bool { matches!(&key.segments[..], [DbKeySeg::AddressSeg(addr), DbKeySeg::StringSeg(prefix), - .. + DbKeySeg::StringSeg(_nullifier), ] if *addr == address::MASP && prefix == MASP_NULLIFIERS_KEY) } diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index d04e05a632..b94390e654 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -31,8 +31,9 @@ use namada_vp_env::{Error, Result, VpEnv}; use crate::storage_key::{ is_masp_key, is_masp_nullifier_key, is_masp_token_map_key, - is_masp_transfer_key, masp_commitment_anchor_key, masp_commitment_tree_key, - masp_convert_anchor_key, masp_nullifier_key, + is_masp_transfer_key, is_masp_undated_balance_key, + masp_commitment_anchor_key, masp_commitment_tree_key, + masp_convert_anchor_key, masp_nullifier_key, masp_undated_balance_key, }; use crate::validation::verify_shielded_tx; @@ -44,16 +45,35 @@ pub struct MaspVp<'ctx, CTX, Params, Gov, Ibc, TransToken, Transfer> { } // Balances changed by a transaction -#[derive(Default, Debug, Clone)] +#[derive(Debug, Clone)] struct ChangedBalances { - // Maps asset types to their decodings - tokens: BTreeMap, + // Maps undated asset types to their decodings + undated_tokens: + BTreeMap, // Map between MASP transparent address and Namada types decoder: BTreeMap, // Balances before the tx pre: BTreeMap>, // Balances after the tx post: BTreeMap>, + // Undated MASP balances before the tx + undated_pre: ValueSum, + // Undated MASP balances after the tx + undated_post: ValueSum, +} + +// Default is manually implemented due to imperfect derive +impl Default for ChangedBalances { + fn default() -> Self { + Self { + undated_tokens: Default::default(), + decoder: Default::default(), + pre: Default::default(), + post: Default::default(), + undated_pre: ValueSum::zero(), + undated_post: ValueSum::zero(), + } + } } impl<'view, 'ctx: 'view, CTX, Params, Gov, Ibc, TransToken, Transfer> @@ -195,6 +215,41 @@ where Ok(()) } + // Store the undated balances before and after this tx is applied. + fn apply_undated_balances( + ctx: &'ctx CTX, + keys_changed: &BTreeSet, + mut result: ChangedBalances, + ) -> Result { + // Record the undated balances of the keys that changed + for token in keys_changed.iter().filter_map(is_masp_undated_balance_key) + { + // Read and store the undated balance before this tx is applied + let pre_undated_balance: Amount = ctx + .read_pre(&masp_undated_balance_key(&token))? + .unwrap_or_default(); + // Attach the token type to the undated balance + let pre_undated_balance = + ValueSum::from_pair(token.clone(), pre_undated_balance); + // Now finally record the undated balance + result.undated_pre = + checked!(result.undated_pre.clone() + &pre_undated_balance) + .map_err(Error::new)?; + // Read and store the undated balance after this tx is applied + let post_undated_balance: Amount = ctx + .read_post(&masp_undated_balance_key(&token))? + .unwrap_or_default(); + // Attach the token type to the undated balance + let post_undated_balance = + ValueSum::from_pair(token, post_undated_balance); + // Now finally record the undated balance + result.undated_post = + checked!(result.undated_post.clone() + &post_undated_balance) + .map_err(Error::new)?; + } + Ok(result) + } + // Check that a transaction carrying output descriptions correctly updates // the tree and anchor in storage fn valid_note_commitment_update( @@ -302,7 +357,7 @@ where "No denomination found in storage for the given token", )?; // Record the token without an epoch to facilitate later decoding - unepoched_tokens(token, denom, &mut result.tokens)?; + undated_tokens(token, denom, &mut result.undated_tokens)?; let counterpart_balance_key = TransToken::balance_key(token, counterpart); let pre_balance: Amount = @@ -351,11 +406,15 @@ where .filter_map(TransToken::is_any_token_balance_key); // Apply the balance changes to the changed balances structure - let mut changed_balances = counterparts_balances + let changed_balances = counterparts_balances .try_fold(ChangedBalances::default(), |acc, account| { Self::apply_balance_change(ctx, acc, account) })?; + // Apply the undated balances to the changed balances structure + let mut changed_balances = + Self::apply_undated_balances(ctx, keys_changed, changed_balances)?; + let ibc_addr = TAddrData::Addr(address::IBC); // Enable decoding the IBC address hash changed_balances @@ -364,10 +423,12 @@ where // Note the balance changes they imply let ChangedBalances { - tokens, + undated_tokens, decoder, pre, post, + undated_pre, + undated_post, } = changed_balances; let ibc::ChangedBalances { decoder, pre, post } = Ibc::apply_ibc_packet::( @@ -377,10 +438,12 @@ where keys_changed, )?; Ok(ChangedBalances { - tokens, + undated_tokens, decoder, pre, post, + undated_pre, + undated_post, }) } @@ -455,9 +518,11 @@ where .post .get(&masp_address_hash) .unwrap_or(&zero), + &changed_balances.undated_pre, + &changed_balances.undated_post, &shielded_tx.sapling_value_balance(), masp_epoch, - &changed_balances.tokens, + &changed_balances.undated_tokens, conversion_state, )?; @@ -606,7 +671,7 @@ where } // Make a map to help recognize asset types lacking an epoch -fn unepoched_tokens( +fn undated_tokens( token: &Address, denom: token::Denomination, tokens: &mut BTreeMap< @@ -664,16 +729,19 @@ fn validate_transparent_input( })?; } // Maybe the asset type has no attached epoch - None if changed_balances.tokens.contains_key(&vin.asset_type) => { + None if changed_balances + .undated_tokens + .contains_key(&vin.asset_type) => + { let (token, denom, digit) = - &changed_balances.tokens[&vin.asset_type]; - // Determine what the asset type would be if it were epoched - let epoched_asset_type = + &changed_balances.undated_tokens[&vin.asset_type]; + // Determine what the asset type would be if it were dated + let dated_asset_type = encode_asset_type(token.clone(), *denom, *digit, Some(epoch)) .wrap_err("unable to create asset type")?; - if conversion_state.assets.contains_key(&epoched_asset_type) { - // If such an epoched asset type is available in the - // conversion tree, then we must reject the unepoched + if conversion_state.assets.contains_key(&dated_asset_type) { + // If such a dated asset type is available in the + // conversion tree, then we must reject the undated // variant let error = Error::new_const("epoch is missing from asset type"); @@ -745,10 +813,13 @@ fn validate_transparent_output( })?; } // Maybe the asset type has no attached epoch - None if changed_balances.tokens.contains_key(&out.asset_type) => { + None if changed_balances + .undated_tokens + .contains_key(&out.asset_type) => + { // Otherwise note the contribution to this transparent output let (token, _denom, digit) = - &changed_balances.tokens[&out.asset_type]; + &changed_balances.undated_tokens[&out.asset_type]; let amount = token::Amount::from_masp_denominated(out.value, *digit); *bal_ref = bal_ref @@ -847,15 +918,20 @@ fn apply_balance_component( // Verify that the pre balance - the Sapling value balance = the post balance // using the decodings in tokens and conversion_state for assistance. +#[allow(clippy::too_many_arguments)] fn verify_sapling_balancing_value( pre: &ValueSum, post: &ValueSum, + undated_pre: &ValueSum, + undated_post: &ValueSum, sapling_value_balance: &I128Sum, target_epoch: MaspEpoch, tokens: &BTreeMap, conversion_state: &ConversionState, ) -> Result<()> { let mut acc = ValueSum::::from_sum(post.clone()); + let mut undated_acc = + ValueSum::::from_sum(undated_post.clone()); for (asset_type, val) in sapling_value_balance.components() { // Only assets with at most the target timestamp count match conversion_state.assets.get(asset_type) { @@ -871,6 +947,13 @@ fn verify_sapling_balancing_value( let (token, _denom, digit) = &tokens[asset_type]; acc = apply_balance_component(&acc, *val, *digit, token.clone())?; + // Additionally record separately the undated changes + undated_acc = apply_balance_component( + &undated_acc, + *val, + *digit, + token.clone(), + )?; } _ => { let error = Error::new_const("Unable to decode asset type"); @@ -879,14 +962,21 @@ fn verify_sapling_balancing_value( } } } - if acc == ValueSum::from_sum(pre.clone()) { - Ok(()) - } else { + if acc != ValueSum::from_sum(pre.clone()) { let error = Error::new_const( "MASP balance change not equal to Sapling value balance", ); tracing::debug!("{error}"); Err(error) + } else if undated_acc != ValueSum::from_sum(undated_pre.clone()) { + let error = Error::new_const( + "MASP undated balance change not equal to undated Sapling value \ + balance", + ); + tracing::debug!("{error}"); + return Err(error); + } else { + Ok(()) } } diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 2a81df25ce..fb9a00f9b7 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3564,7 +3564,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.06262")); + assert!(captured.contains("nam: 0.063")); // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { @@ -3643,7 +3643,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.15655")); + assert!(captured.contains("nam: 0.189")); { // Stop decoding and distributing shielded rewards for BTC in next epoch @@ -3709,7 +3709,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.15655")); + assert!(captured.contains("nam: 0.189")); // Wait till epoch boundary node.next_masp_epoch(); @@ -3755,7 +3755,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.15655")); + assert!(captured.contains("nam: 0.189")); { // Start distributing shielded rewards for NAM in next epoch @@ -3814,7 +3814,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.156705")); + assert!(captured.contains("nam: 0.189567")); // Unshield the rewards let captured = CapturedOutput::of(|| { @@ -3830,7 +3830,7 @@ fn dynamic_assets() -> Result<()> { "--token", NAM, "--amount", - "0.156705", + "0.189567", "--gas-payer", BERTHA_KEY, "--ledger-address", @@ -4987,6 +4987,8 @@ fn identical_output_descriptions() -> Result<()> { "1000", "--gas-payer", bradley_alias.as_ref(), + "--gas-limit", + "60000", "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs index 38e8fa81c8..859b150de8 100644 --- a/crates/token/src/tx.rs +++ b/crates/token/src/tx.rs @@ -1,13 +1,20 @@ //! Token transaction use std::borrow::Cow; -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; +use namada_core::arith::CheckedSub; use namada_core::collections::HashSet; -use namada_core::masp; +use namada_core::masp::encode_asset_type; +use namada_core::masp_primitives::transaction::Transaction; +use namada_core::token::MaspDigitPos; +use namada_core::uint::I320; +use namada_core::{masp, token}; use namada_events::EmitEvents; -use namada_shielded_token::{utils, MaspTxId}; +use namada_shielded_token::storage_key::masp_undated_balance_key; +use namada_shielded_token::{read_undated_balance, utils, MaspTxId}; use namada_storage::{Error, OptionExt, ResultExt}; +use namada_trans_token::read_denom; pub use namada_trans_token::tx::transfer; use namada_tx::action::{self, Action, MaspAction}; use namada_tx::BatchedTx; @@ -26,12 +33,12 @@ where ENV: TxEnv + EmitEvents + action::Write, { // Effect the transparent multi transfer(s) - let debited_accounts = + let (debited_accounts, tokens) = if let Some(transparent) = transfers.transparent_part() { apply_transparent_transfers(env, transparent, event_desc) .wrap_err("Transparent token transfer failed")? } else { - HashSet::new() + (HashSet::new(), HashSet::new()) }; // Apply the shielded transfer if there is a link to one @@ -40,6 +47,7 @@ where env, masp_section_ref, debited_accounts, + tokens, tx_data, ) .wrap_err("Shielded token transfer failed")?; @@ -56,16 +64,93 @@ pub fn apply_transparent_transfers( env: &mut ENV, transfers: TransparentTransfersRef<'_>, event_desc: Cow<'static, str>, -) -> Result> +) -> Result<(HashSet
, HashSet
)> where ENV: TxEnv + EmitEvents, { let sources = transfers.sources(); let targets = transfers.targets(); - let debited_accounts = namada_trans_token::tx::multi_transfer( + let (debited_accounts, tokens) = namada_trans_token::tx::multi_transfer( env, sources, targets, event_desc, )?; - Ok(debited_accounts) + Ok((debited_accounts, tokens)) +} + +/// Update the undated balance keys to reflect the net changes implied by the +/// given shielded transaction. +/// +/// This function takes the set of token addresses impacted by the transaction +/// in order to help it decode the asset types in its value balance. +pub fn update_undated_balances( + env: &mut ENV, + shielded: &Transaction, + tokens: HashSet
, +) -> Result<()> +where + ENV: TxEnv + EmitEvents + action::Write, +{ + // Record undated balance changes + let mut undated_balances = BTreeMap::new(); + let mut undated_asset_types = BTreeMap::new(); + let asset_type_err = + |err| Error::new_alloc(format!("unable to create asset type: {err}")); + // First construct a map to decode asset types + for token in tokens { + let Some(denom) = read_denom(env, &token)? else { + // Ignore asset type if denomination cannot be read + continue; + }; + // Read the undated balance of the current token + let undated_balance = read_undated_balance(env, &token)?; + // Save the undated balance in a map for updating + undated_balances.insert(token.clone(), I320::from(undated_balance)); + // Store the encoding for each digit + for digit in MaspDigitPos::iter() { + let atype = encode_asset_type(token.clone(), denom, digit, None) + .map_err(asset_type_err)?; + undated_asset_types.insert(atype, (token.clone(), denom, digit)); + } + } + // Update the undated balances with the Sapling value balance + for (asset_type, val) in shielded.sapling_value_balance().components() { + let Some((token, _denom, digit)) = undated_asset_types.get(asset_type) + else { + // Assume that token cannot be decoded because it's dated + continue; + }; + // Retrieve the current undated balance + let undated_balance = + undated_balances.get_mut(token).ok_or_else(|| { + Error::new_alloc(format!( + "unable to retrieve undated balance for {token}" + )) + })?; + // Construct the undated balance change from value and digit + let change = + I320::from_masp_denominated(*val, *digit).map_err(|_| { + Error::new_alloc(format!( + "overflow in undated balance for {token}" + )) + })?; + // Update the undated balance + *undated_balance = + undated_balance.checked_sub(change).ok_or_else(|| { + Error::new_alloc(format!( + "overflow in undated balance for {token}" + )) + })?; + } + // Now commit to the updated balances to storage + for (token, balance) in undated_balances { + let undated_balance_key = masp_undated_balance_key(&token); + // Convert the undated balance back into an Amount + let balance: token::Amount = balance.try_into().map_err(|_| { + Error::new_alloc(format!("overflow in undated balance for {token}")) + })?; + // Save the unddated balance back into storage + env.write(&undated_balance_key, balance)?; + } + Ok(()) } /// Apply a shielded transfer @@ -73,6 +158,7 @@ pub fn apply_shielded_transfer( env: &mut ENV, masp_section_ref: MaspTxId, debited_accounts: HashSet
, + tokens: HashSet
, tx_data: &BatchedTx, ) -> Result<()> where @@ -94,6 +180,7 @@ where env.push_action(Action::Masp(MaspAction::MaspSectionRef( masp_section_ref, )))?; + update_undated_balances(env, &shielded, tokens)?; // Extract the debited accounts for the masp part of the transfer and // push the relative actions let vin_addresses = diff --git a/crates/trans_token/src/tx.rs b/crates/trans_token/src/tx.rs index bfcd51f700..76396cd445 100644 --- a/crates/trans_token/src/tx.rs +++ b/crates/trans_token/src/tx.rs @@ -50,13 +50,14 @@ impl CreditOrDebit for BTreeMap<(Address, Address), Amount> { /// /// Returns an `Err` if any source has insufficient balance or if the transfer /// to any destination would overflow (This can only happen if the total supply -/// doesn't fit in `token::Amount`). Returns the set of debited accounts. +/// doesn't fit in `token::Amount`). Returns a pair comprising the set of +/// debited accounts and the set of tokens debited and credited by the transfer. pub fn multi_transfer( env: &mut ENV, sources: impl CreditOrDebit, targets: impl CreditOrDebit, event_desc: Cow<'static, str>, -) -> Result> +) -> Result<(HashSet
, HashSet
)> where ENV: TxEnv + EmitEvents, { @@ -73,7 +74,11 @@ where }; // Apply the balance change for each account in turn let mut any_balance_changed = false; + // To store all the tokens used in the transfer + let mut tokens = HashSet::new(); for ref account @ (ref owner, ref token) in accounts { + // Record the encountered tokens + tokens.insert(token.clone()); let overflow_err = || { Error::new_alloc(format!( "The transfer would overflow balance of {owner}" @@ -109,7 +114,7 @@ where } if !any_balance_changed { - return Ok(debited_accounts); + return Ok((debited_accounts, tokens)); } let mut evt_sources = BTreeMap::new(); @@ -166,7 +171,7 @@ where }, }); - Ok(debited_accounts) + Ok((debited_accounts, tokens)) } #[derive(Debug, Clone)] @@ -276,7 +281,7 @@ mod test { let targets = BTreeMap::from_iter([((dest.clone(), token.clone()), amount)]); - let debited_accounts = + let (debited_accounts, _token) = multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); if amount.is_zero() { @@ -528,7 +533,7 @@ mod test { ((p1.clone(), token2.clone()), amount3), ]); - let debited_accounts = + let (debited_accounts, _token) = multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); // p2 is not debited as it received more of token1 than it spent @@ -602,7 +607,7 @@ mod test { let targets = BTreeMap::from_iter([((addr.clone(), token.clone()), pre_balance)]); - let debited_accounts = + let (debited_accounts, _token) = multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); // No account has been debited @@ -649,7 +654,7 @@ mod test { let targets = BTreeMap::from_iter([((src.clone(), token.clone()), amount)]); - let debited_accounts = + let (debited_accounts, _token) = multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); // No account has been debited diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index b77ef598df..cf383bb98d 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -1,6 +1,7 @@ //! Shielded and transparent tokens related functions use namada_core::collections::HashSet; +use namada_core::masp_primitives::transaction::Transaction; #[cfg(any(test, feature = "testing"))] pub use namada_token::testing; pub use namada_token::tx::apply_shielded_transfer; @@ -36,6 +37,19 @@ pub fn multi_transfer( namada_token::tx::multi_transfer(ctx, transfers, tx_data, EVENT_DESC.into()) } +/// Update the undated balance keys to reflect the net changes implied by the +/// given shielded transaction. +/// +/// This function takes the set of token addresses impacted by the transaction +/// in order to help it decode the asset types in its value balance. +pub fn update_undated_balances( + ctx: &mut Ctx, + shielded: &Transaction, + tokens: HashSet
, +) -> Result<()> { + namada_token::tx::update_undated_balances(ctx, shielded, tokens) +} + /// Transfer tokens from `sources` to `targets` and submit a transfer event. /// /// Returns an `Err` if any source has insufficient balance or if the transfer @@ -44,7 +58,7 @@ pub fn multi_transfer( pub fn apply_transparent_transfers( ctx: &mut Ctx, transfers: TransparentTransfersRef<'_>, -) -> Result> { +) -> Result<(HashSet
, HashSet
)> { namada_token::tx::apply_transparent_transfers( ctx, transfers, diff --git a/wasm/tx_ibc/src/lib.rs b/wasm/tx_ibc/src/lib.rs index 985445978f..01ca4f9733 100644 --- a/wasm/tx_ibc/src/lib.rs +++ b/wasm/tx_ibc/src/lib.rs @@ -8,21 +8,26 @@ use namada_tx_prelude::*; #[transaction] fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { let data = ctx.get_tx_data(&tx_data)?; - let (transfer, masp_tx) = ibc::ibc_actions(ctx) + let data = ibc::ibc_actions(ctx) .execute::(&data) .into_storage_result()?; - let masp_section_ref = if let Some(transfers) = transfer { - if let Some(transparent) = transfers.transparent_part() { - let _debited_accounts = - token::apply_transparent_transfers(ctx, transparent) - .wrap_err("Transparent token transfer failed")?; - } + let (masp_section_ref, mut token_addrs) = + if let Some(transfers) = data.transparent { + let (_debited_accounts, tokens) = + if let Some(transparent) = transfers.transparent_part() { + token::apply_transparent_transfers(ctx, transparent) + .wrap_err("Transparent token transfer failed")? + } else { + Default::default() + }; - transfers.shielded_section_hash - } else { - None - }; + (transfers.shielded_section_hash, tokens) + } else { + (None, Default::default()) + }; + + token_addrs.extend(data.ibc_tokens); let shielded = if let Some(masp_section_ref) = masp_section_ref { Some( @@ -38,7 +43,7 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { })?, ) } else { - masp_tx + data.shielded }; if let Some(shielded) = shielded { token::utils::handle_masp_tx(ctx, &shielded) @@ -52,6 +57,7 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { } else { ctx.push_action(Action::IbcShielding)?; } + token::update_undated_balances(ctx, &shielded, token_addrs)?; } Ok(())