Skip to content

Commit

Permalink
fix for apply_recv_msg
Browse files Browse the repository at this point in the history
  • Loading branch information
@yito88
yito88 committed Jun 19, 2024
1 parent c1d9eed commit 05c584a
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 75 deletions.
8 changes: 2 additions & 6 deletions crates/ibc/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,8 @@ pub fn received_ibc_token(
dest_port_id,
dest_channel_id,
)?;
if ibc_trace.contains('/') {
Ok(storage::ibc_token(ibc_trace))
} else {
Address::decode(ibc_trace)
.map_err(|e| Error::Trace(format!("Invalid base token: {e}")))
}
storage::convert_to_address(ibc_trace)
.map_err(|e| Error::Trace(format!("Invalid base token: {e}")))
}

/// Returns the trace path and the token string if the denom is an IBC
Expand Down
80 changes: 76 additions & 4 deletions crates/ibc/src/storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

use std::str::FromStr;

use ibc::apps::nft_transfer::types::{PrefixedClassId, TokenId};
use ibc::apps::nft_transfer::types::{
PrefixedClassId, TokenId, TracePath as NftTracePath,
};
use ibc::apps::transfer::types::{PrefixedDenom, TracePath};
use ibc::core::client::types::Height;
use ibc::core::host::types::identifiers::{
ChannelId, ClientId, ConnectionId, PortId, Sequence,
Expand Down Expand Up @@ -47,8 +50,8 @@ pub enum Error {
StorageKey(namada_core::storage::Error),
#[error("Invalid Key: {0}")]
InvalidKey(String),
#[error("Port capability error: {0}")]
InvalidPortCapability(String),
#[error("Invalid IBC trace: {0}")]
InvalidIbcTrace(String),
}

/// IBC storage functions result
Expand Down Expand Up @@ -512,7 +515,76 @@ pub fn ibc_token_for_nft(
class_id: &PrefixedClassId,
token_id: &TokenId,
) -> Address {
ibc_token(format!("{class_id}/{token_id}"))
ibc_token(ibc_trace_for_nft(class_id, token_id))
}

/// Obtain the IBC trace from the given NFT class ID and NFT ID
pub fn ibc_trace_for_nft(
class_id: &PrefixedClassId,
token_id: &TokenId,
) -> String {
format!("{class_id}/{token_id}")
}

/// Convert the given IBC trace to [`Address`]
pub fn convert_to_address(ibc_trace: impl AsRef<str>) -> Result<Address> {
// validation
if is_ibc_denom(&ibc_trace).is_none() && is_nft_trace(&ibc_trace).is_none()
{
return Err(Error::InvalidIbcTrace(format!(
"Invalid IBC trace: {}",
ibc_trace.as_ref()
)));
}

if ibc_trace.as_ref().contains('/') {
Ok(ibc_token(ibc_trace.as_ref()))
} else {
Address::decode(ibc_trace.as_ref())
.map_err(|e| Error::InvalidIbcTrace(e.to_string()))
}
}

/// Returns the trace path and the token string if the denom is an IBC
/// denom.
pub fn is_ibc_denom(denom: impl AsRef<str>) -> Option<(TracePath, String)> {
let prefixed_denom = PrefixedDenom::from_str(denom.as_ref()).ok()?;
let base_denom = prefixed_denom.base_denom.to_string();
if prefixed_denom.trace_path.is_empty() || base_denom.contains('/') {
// The denom is just a token or an NFT trace
return None;
}
// The base token isn't decoded because it could be non Namada token
Some((prefixed_denom.trace_path, base_denom))
}

/// Returns the trace path and the token string if the trace is an NFT one
pub fn is_nft_trace(
trace: impl AsRef<str>,
) -> Option<(NftTracePath, String, String)> {
// The trace should be {port}/{channel}/.../{class_id}/{token_id}
if let Some((class_id, token_id)) = trace.as_ref().rsplit_once('/') {
let prefixed_class_id = PrefixedClassId::from_str(class_id).ok()?;
// The base token isn't decoded because it could be non Namada token
Some((
prefixed_class_id.trace_path,
prefixed_class_id.base_class_id.to_string(),
token_id.to_string(),
))
} else {
None
}
}

/// Return true if the source of the given IBC trace is this chain
pub fn is_sender_chain_source(
trace: impl AsRef<str>,
src_port_id: &PortId,
src_channel_id: &ChannelId,
) -> bool {
!trace
.as_ref()
.starts_with(&format!("{src_port_id}/{src_channel_id}"))
}

/// Returns true if the given key is for IBC
Expand Down
122 changes: 65 additions & 57 deletions crates/namada/src/ledger/native_vp/masp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ use namada_core::address::Address;
use namada_core::arith::{checked, CheckedAdd, CheckedSub};
use namada_core::booleans::BoolResultUnitExt;
use namada_core::collections::HashSet;
use namada_core::ibc::apps::nft_transfer::types::packet::PacketData as NftPacketData;
use namada_core::ibc::apps::transfer::types::packet::PacketData;
use namada_core::masp::{addr_taddr, encode_asset_type, ibc_taddr, MaspEpoch};
use namada_core::storage::Key;
use namada_gas::GasMetering;
use namada_governance::storage::is_proposal_accepted;
use namada_ibc::core::channel::types::msgs::MsgRecvPacket as IbcMsgRecvPacket;
use namada_ibc::core::host::types::identifiers::{ChannelId, PortId};
use namada_ibc::storage::ibc_token;
use namada_ibc::core::host::types::identifiers::{ChannelId, PortId, Sequence};
use namada_ibc::storage::{
convert_to_address, ibc_trace_for_nft, is_sender_chain_source,
};
use namada_ibc::IbcMessage;
use namada_sdk::masp::{verify_shielded_tx, TAddrData};
use namada_state::{ConversionState, OptionExt, ResultExt, StateRead};
Expand All @@ -50,10 +53,9 @@ use crate::sdk::ibc::core::channel::types::acknowledgement::AcknowledgementStatu
use crate::sdk::ibc::core::channel::types::commitment::{
compute_ack_commitment, AcknowledgementCommitment,
};
use crate::sdk::ibc::core::channel::types::packet::Packet;
use crate::token;
use crate::token::MaspDigitPos;
use crate::uint::{Uint, I320};
use crate::uint::I320;
use crate::vm::WasmCacheAccess;

#[allow(missing_docs)]
Expand Down Expand Up @@ -340,18 +342,11 @@ where
amount: Amount,
receiver: impl AsRef<str>,
) -> Result<ChangedBalances> {
let token = if ibc_trace.as_ref().contains('/') {
Address::decode(ibc_trace.as_ref()).into_storage_result()?
} else {
ibc_token(ibc_trace.as_ref())
};
let token = convert_to_address(&ibc_trace).into_storage_result()?;
let delta = ValueSum::from_pair(token.clone(), amount);
// If there is a transfer to the IBC account, then deduplicate the
// balance increase since we already accounted for it above
if !ibc_trace
.as_ref()
.starts_with(&format!("{src_port_id}/{src_channel_id}"))
{
if is_sender_chain_source(ibc_trace, src_port_id, src_channel_id) {
let post_entry =
acc.post.entry(addr_taddr(IBC)).or_insert(ValueSum::zero());
*post_entry =
Expand All @@ -371,19 +366,12 @@ where
// Check if IBC message was received successfully in this state transition
fn is_receiving_success(
&self,
packet: &Packet,
keys_changed: &BTreeSet<Key>,
src_port_id: &PortId,
src_channel_id: &ChannelId,
sequence: Sequence,
) -> Result<bool> {
// Ensure that the event corresponds to the current changes to storage
let ack_key = storage::ack_key(
&packet.port_id_on_a,
&packet.chan_id_on_a,
packet.seq_on_a,
);
if !keys_changed.contains(&ack_key) {
// Ignore packet if it was not acknowledged during this state change
return Ok(false);
}
let ack_key = storage::ack_key(src_port_id, src_channel_id, sequence);
// If the receive is a success, then the commitment is unique
let succ_ack_commitment = compute_ack_commitment(
&AcknowledgementStatus::success(ack_success_b64()).into(),
Expand All @@ -403,31 +391,28 @@ where
&self,
mut acc: ChangedBalances,
msg: &IbcMsgRecvPacket,
packet_data: &PacketData,
keys_changed: &BTreeSet<Key>,
ibc_trace: impl AsRef<str>,
amount: Amount,
) -> Result<ChangedBalances> {
// If the transfer was a failure, then enable funds to
// be withdrawn from the IBC internal address
if self.is_receiving_success(&msg.packet, keys_changed)? {
if self.is_receiving_success(
&msg.packet.port_id_on_a,
&msg.packet.chan_id_on_a,
msg.packet.seq_on_a,
)? {
// Mirror how the IBC token is derived in
// gen_ibc_shielded_transfer in the non-refund case
let ibc_denom = self.query_ibc_denom(
packet_data.token.denom.to_string(),
Some(&Address::Internal(InternalAddress::Ibc)),
)?;
let token = namada_ibc::received_ibc_token(
ibc_denom,
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::NativeVpError)?;
let delta = ValueSum::from_pair(
token.clone(),
Amount::from_uint(Uint(*packet_data.token.amount), 0).unwrap(),
);
let delta = ValueSum::from_pair(token.clone(), amount);
// Enable funds to be taken from the IBC internal
// address and be deposited elsewhere
// Required for the IBC internal Address to release
Expand All @@ -445,7 +430,6 @@ where
&self,
mut acc: ChangedBalances,
ibc_msg: &IbcMessage,
keys_changed: &BTreeSet<Key>,
) -> Result<ChangedBalances> {
match ibc_msg {
// This event is emitted on the sender
Expand Down Expand Up @@ -477,9 +461,9 @@ where
let addr = TAddrData::Ibc(receiver.clone());
acc.decoder.insert(ibc_taddr(receiver.clone()), addr);
for token_id in &msg.message.packet_data.token_ids.0 {
let ibc_trace = format!(
"{}/{}",
msg.message.packet_data.class_id, token_id
let ibc_trace = ibc_trace_for_nft(
&msg.message.packet_data.class_id,
token_id,
);
let amount = Amount::from_u64(1);
acc = self.apply_transfer_msg(
Expand All @@ -493,22 +477,46 @@ where
}
}
// This event is emitted on the receiver
IbcMessage::RecvPacket(msg)
if msg.message.packet.port_id_on_b.as_str() == PORT_ID_STR =>
{
let packet_data = serde_json::from_slice::<PacketData>(
&msg.message.packet.data,
)
.map_err(native_vp::Error::new)?;
let receiver = packet_data.receiver.to_string();
let addr = TAddrData::Ibc(receiver.clone());
acc.decoder.insert(ibc_taddr(receiver), addr);
acc = self.apply_recv_msg(
acc,
&msg.message,
&packet_data,
keys_changed,
)?;
IbcMessage::RecvPacket(msg) => {
if msg.message.packet.port_id_on_b.as_str() == PORT_ID_STR {
let packet_data = serde_json::from_slice::<PacketData>(
&msg.message.packet.data,
)
.map_err(native_vp::Error::new)?;
let receiver = packet_data.receiver.to_string();
let addr = TAddrData::Ibc(receiver.clone());
acc.decoder.insert(ibc_taddr(receiver), addr);
let ibc_denom = packet_data.token.denom.to_string();
let amount = packet_data
.token
.amount
.try_into()
.into_storage_result()?;
acc = self.apply_recv_msg(
acc,
&msg.message,
ibc_denom,
amount,
)?;
} else {
let packet_data = serde_json::from_slice::<NftPacketData>(
&msg.message.packet.data,
)
.map_err(native_vp::Error::new)?;
let receiver = packet_data.receiver.to_string();
let addr = TAddrData::Ibc(receiver.clone());
acc.decoder.insert(ibc_taddr(receiver), addr);
for token_id in &packet_data.token_ids.0 {
let ibc_trace =
ibc_trace_for_nft(&packet_data.class_id, token_id);
acc = self.apply_recv_msg(
acc,
&msg.message,
ibc_trace,
Amount::from_u64(1),
)?;
}
}
}
// Ignore all other IBC events
_ => {}
Expand Down Expand Up @@ -584,7 +592,7 @@ where
let changed_balances =
ibc_msgs.iter().try_fold(changed_balances, |acc, ibc_msg| {
// Apply all IBC packets to the changed balances structure
self.apply_ibc_packet(acc, ibc_msg, keys_changed)
self.apply_ibc_packet(acc, ibc_msg)
})?;

Ok(changed_balances)
Expand Down
11 changes: 3 additions & 8 deletions crates/sdk/src/tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use namada_governance::storage::proposal::{
InitProposalData, ProposalType, VoteProposalData,
};
use namada_governance::storage::vote::ProposalVote;
use namada_ibc::storage::{channel_key, ibc_token};
use namada_ibc::storage::{channel_key, convert_to_address};
use namada_ibc::{is_nft_trace, MsgNftTransfer, MsgTransfer};
use namada_proof_of_stake::parameters::{
PosParams, MAX_VALIDATOR_METADATA_LEN,
Expand Down Expand Up @@ -3488,13 +3488,8 @@ pub async fn gen_ibc_shielding_transfer<N: Namada>(
let ibc_denom =
rpc::query_ibc_denom(context, &args.token, Some(&source)).await;
let token = if args.refund {
if ibc_denom.contains('/') {
ibc_token(ibc_denom)
} else {
// the token is a base token
Address::decode(&ibc_denom)
.map_err(|e| Error::Other(format!("Invalid token: {e}")))?
}
convert_to_address(ibc_denom)
.map_err(|e| Error::Other(format!("Invalid token: {e}")))?
} else {
// Need to check the prefix
namada_ibc::received_ibc_token(
Expand Down

0 comments on commit 05c584a

Please sign in to comment.