Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Murisi/track undated assets #4303

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/4304-track-undated-assets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Stop the minting of unclaimable shielded rewards for undated assets.
([\#4304](https://github.com/anoma/namada/issues/4304))
6 changes: 5 additions & 1 deletion crates/apps_lib/src/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions crates/core/src/masp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S>(txid: &TxIdInner, s: S) -> Result<S::Ok, S::Error>
Expand Down Expand Up @@ -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(),
Expand Down
78 changes: 70 additions & 8 deletions crates/ibc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,17 @@ where
Ok(accum)
}

/// Internal transfer data extracted from the wrapping IBC transaction
#[derive(Debug)]
pub struct InternalData<Transfer> {
/// The transparent transfer that happens in parallel to IBC processes
pub transparent: Option<Transfer>,
/// The shielded transaction that happens in parallel to IBC processes
pub shielded: Option<MaspTransaction>,
/// IBC tokens that are credited/debited to internal accounts
pub ibc_tokens: BTreeSet<Address>,
}

/// IBC actions to handle IBC operations
#[derive(Debug)]
pub struct IbcActions<'a, C, Params, Token>
Expand Down Expand Up @@ -616,7 +627,7 @@ where
pub fn execute<Transfer: BorshDeserialize>(
&mut self,
tx_data: &[u8],
) -> Result<(Option<Transfer>, Option<MaspTransaction>), Error> {
) -> Result<InternalData<Transfer>, Error> {
let message = decode_message::<Transfer>(tx_data)?;
let result = match message {
IbcMessage::Transfer(msg) => {
Expand All @@ -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 =
Expand All @@ -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::<Result<_, _>>()?;

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())
Expand All @@ -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()?;
Expand Down
38 changes: 37 additions & 1 deletion crates/ibc/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<Vec<String>, 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::<PacketData>(&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::<NftPacketData>(&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()
Expand Down
40 changes: 31 additions & 9 deletions crates/shielded_token/src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S, TransToken>(
storage: &mut S,
token: &Address,
) -> Result<Amount>
where
S: StorageWrite + StorageRead,
TransToken: trans_token::Keys + trans_token::Read<S>,
{
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<S, TransToken>(
Expand Down Expand Up @@ -173,17 +195,21 @@ where
last_locked_dec,
);

// total locked amount in the Shielded pool
let rewardable_tokens_in_masp =
get_masp_dated_balance::<S, TransToken>(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() {
grarco marked this conversation as resolved.
Show resolved Hide resolved
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(|| {
Expand All @@ -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,
Expand Down Expand Up @@ -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::<S, TransToken>(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
Expand Down Expand Up @@ -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::<S, TransToken>(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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions crates/shielded_token/src/masp/shielded_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
async fn precompute_asset_types<C: Client + Sync>(
&mut self,
client: &C,
tokens: Vec<&Address>,
tokens: BTreeSet<&Address>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could just be an impl Iterator<Item=&Address>

Copy link
Collaborator Author

@murisi murisi Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The motivation for this change was to try and avoid doing the same precomputation twice (with the redundant querying and encoding that comes with it) if the input contains duplicates, not that this is actually a serious problem.

) -> Result<(), eyre::Error> {
// To facilitate lookups of human-readable token names
for token in tokens {
Expand Down Expand Up @@ -1777,10 +1777,10 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
.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();
Expand Down
14 changes: 14 additions & 0 deletions crates/shielded_token/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ where
Ok(total_rewards)
}

/// Read the undated balance of the given token in the MASP.
pub fn read_undated_balance<S>(
storage: &S,
token_address: &Address,
) -> Result<token::Amount>
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<S>(storage: &S) -> Result<TokenMap>
where
Expand Down
Loading
Loading