From 2fbb9bc14a2157bb36c0728d8fec2c88247817f3 Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 21 Jul 2023 12:38:32 +0200 Subject: [PATCH 01/11] IBC transfer for non-native denom --- Cargo.lock | 1 + Cargo.toml | 1 + apps/src/lib/cli.rs | 4 +- core/Cargo.toml | 1 + core/src/ledger/ibc/context/transfer_mod.rs | 6 +- core/src/types/token.rs | 22 +++--- shared/src/ledger/args.rs | 2 +- shared/src/ledger/tx.rs | 76 ++++++++++----------- tests/src/vm_host_env/ibc.rs | 8 ++- wasm/Cargo.lock | 1 + wasm_for_tests/wasm_source/Cargo.lock | 1 + 11 files changed, 63 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3a069b0956..935e15a1c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4148,6 +4148,7 @@ dependencies = [ "num-traits 0.2.15", "num256", "pretty_assertions", + "primitive-types", "proptest", "prost", "prost-types", diff --git a/Cargo.toml b/Cargo.toml index 2d50daf0b6..ee4539a1a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,6 +96,7 @@ once_cell = "1.8.0" orion = "0.16.0" paste = "1.0.9" pretty_assertions = "0.7.2" +primitive-types = "0.12.1" proptest = "1.2.0" proptest-state-machine = "0.1.0" prost = "0.11.6" diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 1ed2275f2c..6f058d761a 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -3336,7 +3336,7 @@ pub mod args { let source = SOURCE.parse(matches); let receiver = RECEIVER.parse(matches); let token = TOKEN.parse(matches); - let amount = AMOUNT.parse(matches); + let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let port_id = PORT_ID.parse(matches); let channel_id = CHANNEL_ID.parse(matches); let timeout_height = TIMEOUT_HEIGHT.parse(matches); @@ -3348,7 +3348,7 @@ pub mod args { source, receiver, token, - amount: amount.amount, + amount, port_id, channel_id, timeout_height, diff --git a/core/Cargo.toml b/core/Cargo.toml index 7c5addad7c..9401b7c3c0 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -84,6 +84,7 @@ num256.workspace = true num-integer = "0.1.45" num-rational.workspace = true num-traits.workspace = true +primitive-types.workspace = true proptest = {version = "1.2.0", optional = true} prost.workspace = true prost-types.workspace = true diff --git a/core/src/ledger/ibc/context/transfer_mod.rs b/core/src/ledger/ibc/context/transfer_mod.rs index 0684432dab..ac62fbe209 100644 --- a/core/src/ledger/ibc/context/transfer_mod.rs +++ b/core/src/ledger/ibc/context/transfer_mod.rs @@ -558,11 +558,7 @@ fn get_token_amount( _ => storage::ibc_token(coin.denom.to_string()), }; - let amount = coin.amount.try_into().map_err(|_| { - TokenTransferError::InvalidCoin { - coin: coin.to_string(), - } - })?; + let amount = coin.amount.into(); Ok((token, amount)) } diff --git a/core/src/types/token.rs b/core/src/types/token.rs index 6056495cd1..f71d333f52 100644 --- a/core/src/types/token.rs +++ b/core/src/types/token.rs @@ -768,17 +768,17 @@ impl MaspDenom { } } -impl TryFrom for Amount { - type Error = AmountParseError; - - fn try_from(amount: IbcAmount) -> Result { - // TODO: https://github.com/anoma/namada/issues/1089 - // TODO: OVERFLOW CHECK PLEASE (PATCH IBC TO ALLOW GETTING - // IBCAMOUNT::MAX OR SIMILAR) if amount > u64::MAX.into() { - // return Err(AmountParseError::InvalidRange); - //} - DenominatedAmount::from_str(&amount.to_string()) - .map(|a| a.amount * NATIVE_SCALE) +impl From for Amount { + fn from(amount: IbcAmount) -> Self { + Self { + raw: Uint(primitive_types::U256::from(amount).0), + } + } +} + +impl From for IbcAmount { + fn from(amount: DenominatedAmount) -> Self { + primitive_types::U256(amount.amount.raw.0).into() } } diff --git a/shared/src/ledger/args.rs b/shared/src/ledger/args.rs index 2d426d5510..96e6deeaca 100644 --- a/shared/src/ledger/args.rs +++ b/shared/src/ledger/args.rs @@ -150,7 +150,7 @@ pub struct TxIbcTransfer { /// Transferred token addres s pub token: C::Address, /// Transferred token amount - pub amount: token::Amount, + pub amount: InputAmount, /// Port ID pub port_id: PortId, /// Channel ID diff --git a/shared/src/ledger/tx.rs b/shared/src/ledger/tx.rs index aa9050519e..a3ee11a16f 100644 --- a/shared/src/ledger/tx.rs +++ b/shared/src/ledger/tx.rs @@ -1337,72 +1337,72 @@ pub async fn build_pgf_stewards_proposal< /// Submit an IBC transfer pub async fn build_ibc_transfer( client: &C, - args::TxIbcTransfer { - tx: tx_args, - source, - receiver, - token, - amount, - port_id, - channel_id, - timeout_height, - timeout_sec_offset, - memo, - tx_code_path, - }: args::TxIbcTransfer, + mut args: args::TxIbcTransfer, gas_payer: &common::PublicKey, ) -> Result { // Check that the source address exists on chain let source = - source_exists_or_err(source.clone(), tx_args.force, client).await?; + source_exists_or_err(args.source.clone(), args.tx.force, client) + .await?; // We cannot check the receiver + // validate the amount given + let validated_amount = + validate_amount(client, args.amount, &args.token, args.tx.force) + .await + .expect("expected to validate amount"); + let validate_fee = validate_amount( + client, + args.tx.gas_amount, + &args.tx.gas_token, + args.tx.force, + ) + .await + .expect("expected to be able to validate fee"); + + args.amount = InputAmount::Validated(validated_amount); + args.tx.gas_amount = InputAmount::Validated(validate_fee); + // Check source balance - let balance_key = token::balance_key(&token, &source); + let balance_key = token::balance_key(&args.token, &source); check_balance_too_low_err( - &token, + &args.token, &source, - amount, + validated_amount.amount, balance_key, - tx_args.force, + args.tx.force, client, ) .await?; let tx_code_hash = - query_wasm_code_hash(client, tx_code_path.to_str().unwrap()) + query_wasm_code_hash(client, args.tx_code_path.to_str().unwrap()) .await .unwrap(); - let ibc_denom = match &token { + let ibc_denom = match &args.token { Address::Internal(InternalAddress::IbcToken(hash)) => { let ibc_denom_key = ibc_denom_key(hash); rpc::query_storage_value::(client, &ibc_denom_key) .await - .ok_or_else(|| Error::TokenDoesNotExist(token.clone()))? + .ok_or_else(|| Error::TokenDoesNotExist(args.token.clone()))? } - _ => token.to_string(), + _ => args.token.to_string(), }; - let amount = amount - .to_string_native() - .split('.') - .next() - .expect("invalid amount") - .to_string(); let token = PrefixedCoin { denom: ibc_denom.parse().expect("Invalid IBC denom"), - amount: amount.parse().expect("Invalid amount"), + amount: validated_amount.into(), }; let packet_data = PacketData { token, sender: source.to_string().into(), - receiver: receiver.into(), - memo: memo.unwrap_or_default().into(), + receiver: args.receiver.into(), + memo: args.memo.unwrap_or_default().into(), }; // this height should be that of the destination chain, not this chain - let timeout_height = match timeout_height { + let timeout_height = match args.timeout_height { Some(h) => { TimeoutHeight::At(IbcHeight::new(0, h).expect("invalid height")) } @@ -1411,7 +1411,7 @@ pub async fn build_ibc_transfer( let now: crate::tendermint::Time = DateTimeUtc::now().try_into().unwrap(); let now: IbcTimestamp = now.into(); - let timeout_timestamp = if let Some(offset) = timeout_sec_offset { + let timeout_timestamp = if let Some(offset) = args.timeout_sec_offset { (now + Duration::new(offset, 0)).unwrap() } else if timeout_height == TimeoutHeight::Never { // we cannot set 0 to both the height and the timestamp @@ -1421,8 +1421,8 @@ pub async fn build_ibc_transfer( }; let msg = MsgTransfer { - port_id_on_a: port_id, - chan_id_on_a: channel_id, + port_id_on_a: args.port_id, + chan_id_on_a: args.channel_id, packet_data, timeout_height_on_b: timeout_height, timeout_timestamp_on_b: timeout_timestamp, @@ -1433,14 +1433,14 @@ pub async fn build_ibc_transfer( prost::Message::encode(&any_msg, &mut data) .map_err(Error::EncodeFailure)?; - let chain_id = tx_args.chain_id.clone().unwrap(); - let mut tx = Tx::new(chain_id, tx_args.expiration); + let chain_id = args.tx.chain_id.clone().unwrap(); + let mut tx = Tx::new(chain_id, args.tx.expiration); tx.add_code_from_hash(tx_code_hash) .add_serialized_data(data); prepare_tx::( client, - &tx_args, + &args.tx, &mut tx, gas_payer.clone(), #[cfg(not(feature = "mainnet"))] diff --git a/tests/src/vm_host_env/ibc.rs b/tests/src/vm_host_env/ibc.rs index f88be446fe..881d83c956 100644 --- a/tests/src/vm_host_env/ibc.rs +++ b/tests/src/vm_host_env/ibc.rs @@ -80,7 +80,7 @@ use namada::types::storage::{ self, BlockHash, BlockHeight, Epoch, Key, TxIndex, }; use namada::types::time::DurationSecs; -use namada::types::token::{self, Amount}; +use namada::types::token::{self, Amount, DenominatedAmount}; use namada::vm::{wasm, WasmCacheRwAccess}; use namada_test_utils::TestWasms; use namada_tx_prelude::BorshSerialize; @@ -637,6 +637,7 @@ pub fn msg_transfer( denom: String, sender: &Address, ) -> MsgTransfer { + let amount = DenominatedAmount::native(Amount::native_whole(100)); let timestamp = (Timestamp::now() + Duration::from_secs(100)).unwrap(); MsgTransfer { port_id_on_a: port_id, @@ -644,7 +645,7 @@ pub fn msg_transfer( packet_data: PacketData { token: PrefixedCoin { denom: denom.parse().expect("invalid denom"), - amount: 100.into(), + amount: amount.into(), }, sender: sender.to_string().into(), receiver: address::testing::gen_established_address() @@ -689,11 +690,12 @@ pub fn received_packet( token: String, receiver: &Address, ) -> Packet { + let amount = DenominatedAmount::native(Amount::native_whole(100)); let counterparty = dummy_channel_counterparty(); let timestamp = (Timestamp::now() + Duration::from_secs(100)).unwrap(); let coin = PrefixedCoin { denom: token.parse().expect("invalid denom"), - amount: 100.into(), + amount: amount.into(), }; let sender = address::testing::gen_established_address(); let data = PacketData { diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index be65562b6d..6571b24ada 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3422,6 +3422,7 @@ dependencies = [ "num-rational 0.4.1", "num-traits", "num256", + "primitive-types", "proptest", "prost", "prost-types", diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 77b2082a4e..b88985178f 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -3422,6 +3422,7 @@ dependencies = [ "num-rational 0.4.1", "num-traits", "num256", + "primitive-types", "proptest", "prost", "prost-types", From 087e501c4b4fe186c6e9067f88d6b164d62be2a6 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 26 Jul 2023 23:41:41 +0200 Subject: [PATCH 02/11] fix amount conversion --- core/src/ledger/ibc/context/common.rs | 62 ++++++++++++++++--- core/src/ledger/ibc/context/storage.rs | 8 +-- core/src/ledger/ibc/context/transfer_mod.rs | 66 +++++++++++++-------- core/src/ledger/ibc/mod.rs | 31 +++++++++- core/src/ledger/ibc/storage.rs | 32 ---------- core/src/types/token.rs | 8 --- shared/src/ledger/ibc/vp/context.rs | 26 ++++---- shared/src/ledger/tx.rs | 9 ++- tx_prelude/src/ibc.rs | 13 ++-- 9 files changed, 155 insertions(+), 100 deletions(-) diff --git a/core/src/ledger/ibc/context/common.rs b/core/src/ledger/ibc/context/common.rs index c3e9b1d08d..d2dbba0b94 100644 --- a/core/src/ledger/ibc/context/common.rs +++ b/core/src/ledger/ibc/context/common.rs @@ -1,6 +1,6 @@ //! IbcCommonContext implementation for IBC -use borsh::BorshSerialize; +use borsh::{BorshDeserialize, BorshSerialize}; use prost::Message; use sha2::Digest; @@ -31,7 +31,9 @@ use crate::ibc::mock::consensus_state::MockConsensusState; use crate::ibc_proto::google::protobuf::Any; use crate::ibc_proto::protobuf::Protobuf; use crate::ledger::ibc::storage; +use crate::types::address::Address; use crate::types::storage::Key; +use crate::types::token; /// Context to handle typical IBC data pub trait IbcCommonContext: IbcStorageContext { @@ -357,24 +359,70 @@ pub trait IbcCommonContext: IbcStorageContext { }) } - /// Write the denom - fn store_denom( + /// Write the IBC denom + fn store_ibc_denom( &mut self, trace_hash: String, denom: PrefixedDenom, ) -> Result<(), ContextError> { let key = storage::ibc_denom_key(trace_hash); - let bytes = denom.to_string().try_to_vec().map_err(|e| { + let bytes = denom + .to_string() + .try_to_vec() + .expect("encoding shouldn't fail"); + self.write(&key, bytes).map_err(|_| { + ContextError::ChannelError(ChannelError::Other { + description: format!("Writing the denom failed: Key {}", key), + }) + }) + } + + /// Write the IBC denom + fn read_token_denom( + &self, + token: &Address, + ) -> Result, ContextError> { + let key = token::denom_key(token); + let bytes = self.read(&key).map_err(|_| { ContextError::ChannelError(ChannelError::Other { description: format!( - "Encoding the denom failed: Denom {}, error {}", - denom, e + "Reading the token denom failed: Key {}", + key ), }) })?; + match bytes { + Some(b) => { + let denom = + token::Denomination::try_from_slice(&b).map_err(|_| { + ContextError::ChannelError(ChannelError::Other { + description: format!( + "Decoding the token denom failed: Token {}", + token + ), + }) + })?; + Ok(Some(denom)) + } + None => Ok(None), + } + } + + /// Write the IBC denom + fn store_token_denom( + &mut self, + token: &Address, + ) -> Result<(), ContextError> { + let key = token::denom_key(token); + // IBC denomination should be zero for U256 + let denom = token::Denomination::from(0); + let bytes = denom.try_to_vec().expect("encoding shouldn't fail"); self.write(&key, bytes).map_err(|_| { ContextError::ChannelError(ChannelError::Other { - description: format!("Writing the denom failed: Key {}", key), + description: format!( + "Writing the token denom failed: Key {}", + key + ), }) }) } diff --git a/core/src/ledger/ibc/context/storage.rs b/core/src/ledger/ibc/context/storage.rs index 87555990a4..1a52669f20 100644 --- a/core/src/ledger/ibc/context/storage.rs +++ b/core/src/ledger/ibc/context/storage.rs @@ -9,7 +9,7 @@ use crate::ledger::storage_api; use crate::types::address::Address; use crate::types::ibc::IbcEvent; use crate::types::storage::{BlockHeight, Header, Key}; -use crate::types::token::Amount; +use crate::types::token::DenominatedAmount; // This is needed to use `ibc::Handler::Error` with `IbcActions` in // `tx_prelude/src/ibc.rs` @@ -58,7 +58,7 @@ pub trait IbcStorageContext { src: &Address, dest: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error>; /// Mint token @@ -66,7 +66,7 @@ pub trait IbcStorageContext { &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error>; /// Burn token @@ -74,7 +74,7 @@ pub trait IbcStorageContext { &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error>; /// Get the current height of this chain diff --git a/core/src/ledger/ibc/context/transfer_mod.rs b/core/src/ledger/ibc/context/transfer_mod.rs index ac62fbe209..28a24f72e0 100644 --- a/core/src/ledger/ibc/context/transfer_mod.rs +++ b/core/src/ledger/ibc/context/transfer_mod.rs @@ -49,6 +49,7 @@ use crate::ibc::Signer; use crate::ledger::ibc::storage; use crate::types::address::{Address, InternalAddress}; use crate::types::token; +use crate::types::uint::Uint; /// IBC module wrapper for getting the reference of the module pub trait ModuleWrapper: Module { @@ -82,6 +83,40 @@ where pub fn module_id(&self) -> ModuleId { ModuleId::new(MODULE_ID_STR.to_string()) } + + /// Get the token address and the amount from PrefixedCoin. If the base + /// denom is not an address, it returns `IbcToken` + fn get_token_amount( + &self, + coin: &PrefixedCoin, + ) -> Result<(Address, token::DenominatedAmount), TokenTransferError> { + let token = match Address::decode(coin.denom.base_denom.as_str()) { + Ok(token_addr) if coin.denom.trace_path.is_empty() => token_addr, + _ => storage::ibc_token(coin.denom.to_string()), + }; + + // Convert IBC amount to Namada amount for the token + let denom = self + .ctx + .borrow() + .read_token_denom(&token)? + .unwrap_or(token::Denomination(0)); + let uint_amount = Uint(primitive_types::U256::from(coin.amount).0); + let amount = + token::Amount::from_uint(uint_amount, denom).map_err(|e| { + TokenTransferError::ContextError(ContextError::ChannelError( + ChannelError::Other { + description: format!( + "The IBC amount is invalid: Coin {}, Error {}", + coin, e + ), + }, + )) + })?; + let amount = token::DenominatedAmount { amount, denom }; + + Ok((token, amount)) + } } impl ModuleWrapper for TransferModule @@ -443,7 +478,7 @@ where ) -> Result<(), TokenTransferError> { // Assumes that the coin denom is prefixed with "port-id/channel-id" or // has no prefix - let (ibc_token, amount) = get_token_amount(coin)?; + let (ibc_token, amount) = self.get_token_amount(coin)?; self.ctx .borrow_mut() @@ -453,9 +488,7 @@ where ChannelError::Other { description: format!( "Sending a coin failed: from {}, to {}, amount {}", - from, - to, - amount.to_string_native() + from, to, amount, ), }, )) @@ -468,7 +501,7 @@ where coin: &PrefixedCoin, ) -> Result<(), TokenTransferError> { // The trace path of the denom is already updated if receiving the token - let (ibc_token, amount) = get_token_amount(coin)?; + let (ibc_token, amount) = self.get_token_amount(coin)?; self.ctx .borrow_mut() @@ -478,8 +511,7 @@ where ChannelError::Other { description: format!( "Minting a coin failed: account {}, amount {}", - account, - amount.to_string_native() + account, amount, ), }, )) @@ -491,7 +523,7 @@ where account: &Self::AccountId, coin: &PrefixedCoin, ) -> Result<(), TokenTransferError> { - let (ibc_token, amount) = get_token_amount(coin)?; + let (ibc_token, amount) = self.get_token_amount(coin)?; // The burn is "unminting" from the minted balance self.ctx @@ -502,8 +534,7 @@ where ChannelError::Other { description: format!( "Burning a coin failed: account {}, amount {}", - account, - amount.to_string_native() + account, amount, ), }, )) @@ -548,21 +579,6 @@ where } } -/// Get the token address and the amount from PrefixedCoin. If the base denom is -/// not an address, it returns `IbcToken` -fn get_token_amount( - coin: &PrefixedCoin, -) -> Result<(Address, token::Amount), TokenTransferError> { - let token = match Address::decode(coin.denom.base_denom.as_str()) { - Ok(token_addr) if coin.denom.trace_path.is_empty() => token_addr, - _ => storage::ibc_token(coin.denom.to_string()), - }; - - let amount = coin.amount.into(); - - Ok((token, amount)) -} - fn into_channel_error(error: TokenTransferError) -> ChannelError { ChannelError::AppModule { description: error.to_string(), diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 75ff665914..6aa3d9ecc3 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -153,10 +153,35 @@ where let trace_hash = storage::calc_hash(coin.denom.to_string()); self.ctx .borrow_mut() - .store_denom(trace_hash, coin.denom) + .store_ibc_denom(trace_hash, coin.denom.clone()) .map_err(|e| { - Error::Denom(format!("Write the denom failed: {}", e)) - }) + Error::Denom(format!( + "Writing the IBC denom failed: {}", + e + )) + })?; + + let token = storage::ibc_token(coin.denom.to_string()); + let denom = + self.ctx.borrow().read_token_denom(&token).map_err( + |e| { + Error::Denom(format!( + "Reading the token denom failed: {}", + e + )) + }, + )?; + if denom.is_none() { + self.ctx.borrow_mut().store_token_denom(&token).map_err( + |e| { + Error::Denom(format!( + "Writing the token denom failed: {}", + e + )) + }, + )?; + } + Ok(()) } // other messages _ => Ok(()), diff --git a/core/src/ledger/ibc/storage.rs b/core/src/ledger/ibc/storage.rs index 9a80766c47..317aa108bf 100644 --- a/core/src/ledger/ibc/storage.rs +++ b/core/src/ledger/ibc/storage.rs @@ -373,38 +373,6 @@ pub fn ibc_denom_key(token_hash: impl AsRef) -> Key { ibc_key(path).expect("Creating a key for the denom key shouldn't fail") } -/// Token address from the denom string -pub fn token(denom: impl AsRef) -> Result
{ - let token_str = denom.as_ref().split('/').last().ok_or_else(|| { - Error::Denom(format!("No token was specified: {}", denom.as_ref())) - })?; - Address::decode(token_str).map_err(|e| { - Error::Denom(format!( - "Invalid token address: token {}, error {}", - token_str, e - )) - }) -} - -/// Get the hash of IBC token address from the denom string -pub fn token_hash_from_denom(denom: impl AsRef) -> Result> { - let addr = Address::decode(denom.as_ref()).map_err(|e| { - Error::Denom(format!( - "Decoding the denom failed: denom {}, error {}", - denom.as_ref(), - e - )) - })?; - match addr { - Address::Established(_) => Ok(None), - Address::Internal(InternalAddress::IbcToken(h)) => Ok(Some(h)), - _ => Err(Error::Denom(format!( - "Unexpected address was given: {}", - addr - ))), - } -} - /// Hash the denom pub fn calc_hash(denom: impl AsRef) -> String { let mut hasher = Sha256::new(); diff --git a/core/src/types/token.rs b/core/src/types/token.rs index f71d333f52..ac27e0e6d9 100644 --- a/core/src/types/token.rs +++ b/core/src/types/token.rs @@ -768,14 +768,6 @@ impl MaspDenom { } } -impl From for Amount { - fn from(amount: IbcAmount) -> Self { - Self { - raw: Uint(primitive_types::U256::from(amount).0), - } - } -} - impl From for IbcAmount { fn from(amount: DenominatedAmount) -> Self { primitive_types::U256(amount.amount.raw.0).into() diff --git a/shared/src/ledger/ibc/vp/context.rs b/shared/src/ledger/ibc/vp/context.rs index daf2246cbe..7d51914430 100644 --- a/shared/src/ledger/ibc/vp/context.rs +++ b/shared/src/ledger/ibc/vp/context.rs @@ -11,7 +11,7 @@ use namada_core::ledger::storage_api::StorageRead; use namada_core::types::address::{Address, InternalAddress}; use namada_core::types::ibc::IbcEvent; use namada_core::types::storage::{BlockHeight, Header, Key}; -use namada_core::types::token::{self, Amount}; +use namada_core::types::token::{self, Amount, DenominatedAmount}; use super::Error; use crate::ledger::native_vp::CtxPreStorageRead; @@ -118,20 +118,20 @@ where src: &Address, dest: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error> { let src_key = token::balance_key(token, src); let dest_key = token::balance_key(token, dest); let src_bal: Option = self.ctx.read(&src_key).map_err(Error::NativeVpError)?; let mut src_bal = src_bal.expect("The source has no balance"); - src_bal.spend(&amount); + src_bal.spend(&amount.amount); let mut dest_bal: Amount = self .ctx .read(&dest_key) .map_err(Error::NativeVpError)? .unwrap_or_default(); - dest_bal.receive(&amount); + dest_bal.receive(&amount.amount); self.write( &src_key, @@ -147,7 +147,7 @@ where &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error> { let target_key = token::balance_key(token, target); let mut target_bal: Amount = self @@ -155,7 +155,7 @@ where .read(&target_key) .map_err(Error::NativeVpError)? .unwrap_or_default(); - target_bal.receive(&amount); + target_bal.receive(&amount.amount); let minted_key = token::minted_balance_key(token); let mut minted_bal: Amount = self @@ -163,7 +163,7 @@ where .read(&minted_key) .map_err(Error::NativeVpError)? .unwrap_or_default(); - minted_bal.receive(&amount); + minted_bal.receive(&amount.amount); self.write( &target_key, @@ -187,7 +187,7 @@ where &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error> { let target_key = token::balance_key(token, target); let mut target_bal: Amount = self @@ -195,7 +195,7 @@ where .read(&target_key) .map_err(Error::NativeVpError)? .unwrap_or_default(); - target_bal.spend(&amount); + target_bal.spend(&amount.amount); let minted_key = token::minted_balance_key(token); let mut minted_bal: Amount = self @@ -203,7 +203,7 @@ where .read(&minted_key) .map_err(Error::NativeVpError)? .unwrap_or_default(); - minted_bal.spend(&amount); + minted_bal.spend(&amount.amount); self.write( &target_key, @@ -311,7 +311,7 @@ where _src: &Address, _dest: &Address, _token: &Address, - _amount: Amount, + _amount: DenominatedAmount, ) -> Result<(), Self::Error> { unimplemented!("Validation doesn't transfer") } @@ -320,7 +320,7 @@ where &mut self, _target: &Address, _token: &Address, - _amount: Amount, + _amount: DenominatedAmount, ) -> Result<(), Self::Error> { unimplemented!("Validation doesn't mint") } @@ -329,7 +329,7 @@ where &mut self, _target: &Address, _token: &Address, - _amount: Amount, + _amount: DenominatedAmount, ) -> Result<(), Self::Error> { unimplemented!("Validation doesn't burn") } diff --git a/shared/src/ledger/tx.rs b/shared/src/ledger/tx.rs index a3ee11a16f..745314fc75 100644 --- a/shared/src/ledger/tx.rs +++ b/shared/src/ledger/tx.rs @@ -1351,6 +1351,12 @@ pub async fn build_ibc_transfer( validate_amount(client, args.amount, &args.token, args.tx.force) .await .expect("expected to validate amount"); + if validated_amount.canonical().denom.0 != 0 { + return Err(Error::Other(format!( + "The amount for the IBC transfer should be an integer: {}", + validated_amount + ))); + } let validate_fee = validate_amount( client, args.tx.gas_amount, @@ -1392,7 +1398,8 @@ pub async fn build_ibc_transfer( }; let token = PrefixedCoin { denom: ibc_denom.parse().expect("Invalid IBC denom"), - amount: validated_amount.into(), + // Set the IBC amount as an integer + amount: validated_amount.canonical().into(), }; let packet_data = PacketData { token, diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index d15a60a5af..51f4959323 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -12,7 +12,7 @@ use namada_core::ledger::tx_env::TxEnv; use namada_core::types::address::{Address, InternalAddress}; pub use namada_core::types::ibc::IbcEvent; use namada_core::types::storage::{BlockHeight, Header, Key}; -use namada_core::types::token::Amount; +use namada_core::types::token::DenominatedAmount; use crate::token::{burn, mint, transfer}; use crate::{Ctx, KeyValIterator}; @@ -76,9 +76,8 @@ impl IbcStorageContext for Ctx { src: &Address, dest: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> std::result::Result<(), Self::Error> { - let amount = amount.denominated(token, self)?; transfer(self, src, dest, token, amount, &None, &None, &None) } @@ -86,14 +85,14 @@ impl IbcStorageContext for Ctx { &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error> { mint( self, &Address::Internal(InternalAddress::Ibc), target, token, - amount, + amount.amount, ) } @@ -101,9 +100,9 @@ impl IbcStorageContext for Ctx { &mut self, target: &Address, token: &Address, - amount: Amount, + amount: DenominatedAmount, ) -> Result<(), Self::Error> { - burn(self, target, token, amount) + burn(self, target, token, amount.amount) } fn get_height(&self) -> std::result::Result { From dd05a4e591120547e1784b17daf785b2959bf6c0 Mon Sep 17 00:00:00 2001 From: yito88 Date: Thu, 27 Jul 2023 21:24:46 +0200 Subject: [PATCH 03/11] fix tests --- core/src/types/token.rs | 2 +- shared/src/ledger/tx.rs | 2 +- tests/src/e2e/ibc_tests.rs | 94 +++++++++++++++++++++++++++++++----- tests/src/vm_host_env/ibc.rs | 17 +++++-- tests/src/vm_host_env/mod.rs | 41 +++++++++++----- 5 files changed, 124 insertions(+), 32 deletions(-) diff --git a/core/src/types/token.rs b/core/src/types/token.rs index ac27e0e6d9..59d03cef86 100644 --- a/core/src/types/token.rs +++ b/core/src/types/token.rs @@ -770,7 +770,7 @@ impl MaspDenom { impl From for IbcAmount { fn from(amount: DenominatedAmount) -> Self { - primitive_types::U256(amount.amount.raw.0).into() + primitive_types::U256(amount.canonical().amount.raw.0).into() } } diff --git a/shared/src/ledger/tx.rs b/shared/src/ledger/tx.rs index 745314fc75..9732dab631 100644 --- a/shared/src/ledger/tx.rs +++ b/shared/src/ledger/tx.rs @@ -1399,7 +1399,7 @@ pub async fn build_ibc_transfer( let token = PrefixedCoin { denom: ibc_denom.parse().expect("Invalid IBC denom"), // Set the IBC amount as an integer - amount: validated_amount.canonical().into(), + amount: validated_amount.into(), }; let packet_data = PacketData { token, diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index bbc959908f..c65544bb7a 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -160,6 +160,9 @@ fn run_ledger_ibc() -> Result<()> { )?; check_balances(&port_id_b, &channel_id_b, &test_a, &test_b)?; + // Try invalid transfers and they will fail + try_invalid_transfers(&test_a, &test_b, &port_id_a, &channel_id_a)?; + // Transfer 50000 received over IBC on Chain B transfer_received_token(&port_id_b, &channel_id_b, &test_b)?; check_balances_after_non_ibc(&port_id_b, &channel_id_b, &test_b)?; @@ -634,11 +637,12 @@ fn transfer_token( ALBERT, &receiver, NAM, - &Amount::native_whole(100000), + "100000", ALBERT_KEY, port_id_a, channel_id_a, None, + None, false, )?; let events = get_events(test_a, height)?; @@ -688,6 +692,62 @@ fn transfer_token( Ok(()) } +fn try_invalid_transfers( + test_a: &Test, + test_b: &Test, + port_id_a: &PortId, + channel_id_a: &ChannelId, +) -> Result<()> { + let receiver = find_address(test_b, BERTHA)?; + + // invalid amount + transfer( + test_a, + ALBERT, + &receiver, + NAM, + "10.1", + ALBERT_KEY, + port_id_a, + channel_id_a, + None, + Some("The amount for the IBC transfer should be an integer"), + false, + )?; + + // invalid port + transfer( + test_a, + ALBERT, + &receiver, + NAM, + "10", + ALBERT_KEY, + &"port".parse().unwrap(), + &channel_id_a, + None, + Some("Error trying to apply a transaction"), + false, + )?; + + // invalid channel + transfer( + test_a, + ALBERT, + &receiver, + NAM, + "10", + ALBERT_KEY, + port_id_a, + &"channel-42".parse().unwrap(), + None, + Some("Error trying to apply a transaction"), + false, + )?; + + Ok(()) +} + fn transfer_received_token( port_id: &PortId, channel_id: &ChannelId, @@ -735,11 +795,11 @@ fn transfer_back( port_id_b: &PortId, channel_id_b: &ChannelId, ) -> Result<()> { - let nam = find_address(test_b, NAM)?.to_string(); + let token = find_address(test_b, NAM)?.to_string(); let receiver = find_address(test_a, ALBERT)?; // Chain A was the source for the sent token - let denom_raw = format!("{}/{}/{}", port_id_b, channel_id_b, nam); + let denom_raw = format!("{}/{}/{}", port_id_b, channel_id_b, token); let ibc_token = ibc_token(denom_raw).to_string(); // Send a token from Chain B let height = transfer( @@ -747,11 +807,12 @@ fn transfer_back( BERTHA, &receiver, ibc_token, - &Amount::native_whole(50000), + "50000", BERTHA_KEY, port_id_b, channel_id_b, None, + None, false, )?; let events = get_events(test_b, height)?; @@ -809,11 +870,12 @@ fn transfer_timeout( ALBERT, &receiver, NAM, - &Amount::native_whole(100000), + "100000", ALBERT_KEY, port_id_a, channel_id_a, Some(Duration::new(5, 0)), + None, false, )?; let events = get_events(test_a, height)?; @@ -941,17 +1003,17 @@ fn transfer( sender: impl AsRef, receiver: &Address, token: impl AsRef, - amount: &Amount, + amount: impl AsRef, signer: impl AsRef, port_id: &PortId, channel_id: &ChannelId, timeout_sec: Option, + expected_err: Option<&str>, wait_reveal_pk: bool, ) -> Result { let rpc = get_actor_rpc(test, &Who::Validator(0)); let receiver = receiver.to_string(); - let amount = amount.to_string_native(); let channel_id = channel_id.to_string(); let port_id = port_id.to_string(); let mut tx_args = vec![ @@ -965,7 +1027,7 @@ fn transfer( "--token", token.as_ref(), "--amount", - &amount, + amount.as_ref(), "--channel-id", &channel_id, "--port-id", @@ -981,11 +1043,19 @@ fn transfer( } let mut client = run!(test, Bin::Client, tx_args, Some(40))?; - client.exp_string("Transaction applied")?; - if wait_reveal_pk { - client.exp_string("Transaction applied")?; + match expected_err { + Some(err) => { + client.exp_string(err)?; + Ok(0) + } + None => { + client.exp_string("Transaction applied")?; + if wait_reveal_pk { + client.exp_string("Transaction applied")?; + } + check_tx_height(test, &mut client) + } } - check_tx_height(test, &mut client) } fn check_tx_height(test: &Test, client: &mut NamadaCmd) -> Result { diff --git a/tests/src/vm_host_env/ibc.rs b/tests/src/vm_host_env/ibc.rs index 881d83c956..97d5f5e6f4 100644 --- a/tests/src/vm_host_env/ibc.rs +++ b/tests/src/vm_host_env/ibc.rs @@ -88,7 +88,7 @@ use namada_tx_prelude::BorshSerialize; use crate::tx::*; const ADDRESS: Address = Address::Internal(InternalAddress::Ibc); - +pub const ANY_DENOMINATION: u8 = 4; const COMMITMENT_PREFIX: &[u8] = b"ibc"; pub struct TestIbcVp<'a> { @@ -228,14 +228,21 @@ pub fn init_storage() -> (Address, Address) { // initialize a token let token = tx_host_env::ctx().init_account(code_hash).unwrap(); - + let denom_key = token::denom_key(&token); + let token_denom = token::Denomination(ANY_DENOMINATION); // initialize an account let account = tx_host_env::ctx().init_account(code_hash).unwrap(); let key = token::balance_key(&token, &account); - let init_bal = Amount::native_whole(100); - let bytes = init_bal.try_to_vec().expect("encoding failed"); + let init_bal = Amount::from_uint(100, token_denom).unwrap(); tx_host_env::with(|env| { - env.wl_storage.storage.write(&key, &bytes).unwrap(); + env.wl_storage + .storage + .write(&denom_key, &token_denom.try_to_vec().unwrap()) + .unwrap(); + env.wl_storage + .storage + .write(&key, &init_bal.try_to_vec().unwrap()) + .unwrap(); }); // epoch duration diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index 878d93aeef..7144744f83 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -1187,7 +1187,10 @@ mod tests { let balance: Option = tx_host_env::with(|env| { env.wl_storage.read(&balance_key).expect("read error") }); - assert_eq!(balance, Some(Amount::native_whole(0))); + assert_eq!( + balance, + Some(Amount::from_uint(0, ibc::ANY_DENOMINATION).unwrap()) + ); let escrow_key = token::balance_key( &token, &address::Address::Internal(address::InternalAddress::Ibc), @@ -1195,7 +1198,10 @@ mod tests { let escrow: Option = tx_host_env::with(|env| { env.wl_storage.read(&escrow_key).expect("read error") }); - assert_eq!(escrow, Some(Amount::native_whole(100))); + assert_eq!( + escrow, + Some(Amount::from_uint(100, ibc::ANY_DENOMINATION).unwrap()) + ); } #[test] @@ -1221,7 +1227,7 @@ mod tests { let denom = format!("{}/{}/{}", port_id, channel_id, token); let ibc_token = ibc_storage::ibc_token(&denom); let balance_key = token::balance_key(&ibc_token, &sender); - let init_bal = Amount::native_whole(100); + let init_bal = Amount::from_u64(100); writes.insert(balance_key.clone(), init_bal.try_to_vec().unwrap()); let minted_key = token::minted_balance_key(&ibc_token); writes.insert(minted_key.clone(), init_bal.try_to_vec().unwrap()); @@ -1274,11 +1280,11 @@ mod tests { let balance: Option = tx_host_env::with(|env| { env.wl_storage.read(&balance_key).expect("read error") }); - assert_eq!(balance, Some(Amount::native_whole(0))); + assert_eq!(balance, Some(Amount::from_u64(0))); let minted: Option = tx_host_env::with(|env| { env.wl_storage.read(&minted_key).expect("read error") }); - assert_eq!(minted, Some(Amount::native_whole(0))); + assert_eq!(minted, Some(Amount::from_u64(0))); } #[test] @@ -1351,11 +1357,11 @@ mod tests { let balance: Option = tx_host_env::with(|env| { env.wl_storage.read(&key).expect("read error") }); - assert_eq!(balance, Some(Amount::native_whole(100))); + assert_eq!(balance, Some(Amount::from_u64(100))); let minted: Option = tx_host_env::with(|env| { env.wl_storage.read(&minted_key).expect("read error") }); - assert_eq!(minted, Some(Amount::native_whole(100))); + assert_eq!(minted, Some(Amount::from_u64(100))); } #[test] @@ -1390,7 +1396,10 @@ mod tests { &token, &address::Address::Internal(address::InternalAddress::Ibc), ); - let val = Amount::native_whole(100).try_to_vec().unwrap(); + let val = Amount::from_uint(100, ibc::ANY_DENOMINATION) + .unwrap() + .try_to_vec() + .unwrap(); tx_host_env::with(|env| { env.wl_storage .storage @@ -1443,11 +1452,17 @@ mod tests { let balance: Option = tx_host_env::with(|env| { env.wl_storage.read(&key).expect("read error") }); - assert_eq!(balance, Some(Amount::native_whole(200))); + assert_eq!( + balance, + Some(Amount::from_uint(200, ibc::ANY_DENOMINATION).unwrap()) + ); let escrow: Option = tx_host_env::with(|env| { env.wl_storage.read(&escrow_key).expect("read error") }); - assert_eq!(escrow, Some(Amount::native_whole(0))); + assert_eq!( + escrow, + Some(Amount::from_uint(0, ibc::ANY_DENOMINATION).unwrap()) + ); } #[test] @@ -1486,7 +1501,7 @@ mod tests { denom, &address::Address::Internal(address::InternalAddress::Ibc), ); - let val = Amount::native_whole(100).try_to_vec().unwrap(); + let val = Amount::from_u64(100).try_to_vec().unwrap(); tx_host_env::with(|env| { env.wl_storage .storage @@ -1545,11 +1560,11 @@ mod tests { let balance: Option = tx_host_env::with(|env| { env.wl_storage.read(&key).expect("read error") }); - assert_eq!(balance, Some(Amount::native_whole(100))); + assert_eq!(balance, Some(Amount::from_u64(100))); let escrow: Option = tx_host_env::with(|env| { env.wl_storage.read(&escrow_key).expect("read error") }); - assert_eq!(escrow, Some(Amount::native_whole(0))); + assert_eq!(escrow, Some(Amount::from_u64(0))); } #[test] From fac66e9414a746da585f9123d14741b34241aaa2 Mon Sep 17 00:00:00 2001 From: yito88 Date: Thu, 27 Jul 2023 21:59:52 +0200 Subject: [PATCH 04/11] for clippy --- tests/src/e2e/ibc_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index c65544bb7a..d81bdd9af9 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -724,7 +724,7 @@ fn try_invalid_transfers( "10", ALBERT_KEY, &"port".parse().unwrap(), - &channel_id_a, + channel_id_a, None, Some("Error trying to apply a transaction"), false, From e17004fe03d56e5dd5c89e5a5e19ba42c38c145e Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 2 Aug 2023 00:36:08 +0200 Subject: [PATCH 05/11] fix denom update --- core/src/ledger/ibc/context/common.rs | 60 +++++++++++++------ core/src/ledger/ibc/context/storage.rs | 9 +++ core/src/ledger/ibc/mod.rs | 83 ++++++++++++-------------- core/src/ledger/tx_env.rs | 6 ++ shared/src/ledger/ibc/vp/context.rs | 28 +++++++++ shared/src/vm/host_env.rs | 34 +++++++++++ shared/src/vm/wasm/host_env.rs | 1 + tests/src/vm_host_env/tx.rs | 1 + tx_prelude/src/ibc.rs | 11 ++++ tx_prelude/src/lib.rs | 20 +++++++ vm_env/src/lib.rs | 6 ++ 11 files changed, 196 insertions(+), 63 deletions(-) diff --git a/core/src/ledger/ibc/context/common.rs b/core/src/ledger/ibc/context/common.rs index d2dbba0b94..fde2335492 100644 --- a/core/src/ledger/ibc/context/common.rs +++ b/core/src/ledger/ibc/context/common.rs @@ -5,7 +5,6 @@ use prost::Message; use sha2::Digest; use super::storage::IbcStorageContext; -use crate::ibc::applications::transfer::denom::PrefixedDenom; use crate::ibc::clients::ics07_tendermint::client_state::ClientState as TmClientState; use crate::ibc::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::ibc::core::ics02_client::client_state::ClientState; @@ -362,22 +361,36 @@ pub trait IbcCommonContext: IbcStorageContext { /// Write the IBC denom fn store_ibc_denom( &mut self, - trace_hash: String, - denom: PrefixedDenom, + trace_hash: impl AsRef, + denom: impl AsRef, ) -> Result<(), ContextError> { - let key = storage::ibc_denom_key(trace_hash); - let bytes = denom - .to_string() - .try_to_vec() - .expect("encoding shouldn't fail"); - self.write(&key, bytes).map_err(|_| { + let key = storage::ibc_denom_key(trace_hash.as_ref()); + let has_key = self.has_key(&key).map_err(|_| { ContextError::ChannelError(ChannelError::Other { - description: format!("Writing the denom failed: Key {}", key), + description: format!( + "Reading the IBC denom failed: Key {}", + key + ), }) - }) + })?; + if !has_key { + let bytes = denom + .as_ref() + .try_to_vec() + .expect("encoding shouldn't fail"); + self.write(&key, bytes).map_err(|_| { + ContextError::ChannelError(ChannelError::Other { + description: format!( + "Writing the denom failed: Key {}", + key + ), + }) + })?; + } + Ok(()) } - /// Write the IBC denom + /// Read the token denom fn read_token_denom( &self, token: &Address, @@ -414,16 +427,27 @@ pub trait IbcCommonContext: IbcStorageContext { token: &Address, ) -> Result<(), ContextError> { let key = token::denom_key(token); - // IBC denomination should be zero for U256 - let denom = token::Denomination::from(0); - let bytes = denom.try_to_vec().expect("encoding shouldn't fail"); - self.write(&key, bytes).map_err(|_| { + let has_key = self.has_key(&key).map_err(|_| { ContextError::ChannelError(ChannelError::Other { description: format!( - "Writing the token denom failed: Key {}", + "Reading the token denom failed: Key {}", key ), }) - }) + })?; + if !has_key { + // IBC denomination should be zero for U256 + let denom = token::Denomination::from(0); + let bytes = denom.try_to_vec().expect("encoding shouldn't fail"); + self.write(&key, bytes).map_err(|_| { + ContextError::ChannelError(ChannelError::Other { + description: format!( + "Writing the token denom failed: Key {}", + key + ), + }) + })?; + } + Ok(()) } } diff --git a/core/src/ledger/ibc/context/storage.rs b/core/src/ledger/ibc/context/storage.rs index 1a52669f20..2d1c8afdb3 100644 --- a/core/src/ledger/ibc/context/storage.rs +++ b/core/src/ledger/ibc/context/storage.rs @@ -31,6 +31,9 @@ pub trait IbcStorageContext { /// Read IBC-related data fn read(&self, key: &Key) -> Result>, Self::Error>; + /// Check if the given key is present + fn has_key(&self, key: &Key) -> Result; + /// Read IBC-related data with a prefix fn iter_prefix<'iter>( &'iter self, @@ -52,6 +55,12 @@ pub trait IbcStorageContext { /// Emit an IBC event fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), Self::Error>; + /// Get an IBC event + fn get_ibc_event( + &self, + event_type: impl AsRef, + ) -> Result, Self::Error>; + /// Transfer token fn transfer_token( &mut self, diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 6aa3d9ecc3..0c9337bf85 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -15,10 +15,8 @@ pub use context::transfer_mod::{ModuleWrapper, TransferModule}; use prost::Message; use thiserror::Error; -use crate::ibc::applications::transfer::denom::TracePrefix; use crate::ibc::applications::transfer::error::TokenTransferError; use crate::ibc::applications::transfer::msgs::transfer::MsgTransfer; -use crate::ibc::applications::transfer::packet::PacketData; use crate::ibc::applications::transfer::{ send_transfer_execute, send_transfer_validate, }; @@ -136,50 +134,45 @@ where /// Store the denom when transfer with MsgRecvPacket fn store_denom(&mut self, envelope: MsgEnvelope) -> Result<(), Error> { match envelope { - MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { - let data = match serde_json::from_slice::( - &msg.packet.data, - ) { - Ok(data) => data, - // not token transfer data - Err(_) => return Ok(()), - }; - let prefix = TracePrefix::new( - msg.packet.port_id_on_b.clone(), - msg.packet.chan_id_on_b, - ); - let mut coin = data.token; - coin.denom.add_trace_prefix(prefix); - let trace_hash = storage::calc_hash(coin.denom.to_string()); - self.ctx - .borrow_mut() - .store_ibc_denom(trace_hash, coin.denom.clone()) - .map_err(|e| { - Error::Denom(format!( - "Writing the IBC denom failed: {}", - e - )) + MsgEnvelope::Packet(PacketMsg::Recv(_)) => { + let result = self + .ctx + .borrow() + .get_ibc_event("denomination_trace") + .map_err(|_| { + Error::Denom(format!("Reading the IBC event failed",)) })?; - - let token = storage::ibc_token(coin.denom.to_string()); - let denom = - self.ctx.borrow().read_token_denom(&token).map_err( - |e| { - Error::Denom(format!( - "Reading the token denom failed: {}", - e - )) - }, - )?; - if denom.is_none() { - self.ctx.borrow_mut().store_token_denom(&token).map_err( - |e| { - Error::Denom(format!( - "Writing the token denom failed: {}", - e - )) - }, - )?; + if let Some(event) = result { + if let (Some(trace_hash), Some(ibc_denom)) = ( + event.attributes.get("trace_hash"), + event.attributes.get("denom"), + ) { + // If the denomination trace event has the trace hash + // and the IBC denom, a token has been minted. The raw + // IBC denom including the port ID, the channel ID and + // the base token is stored to be restored from the + // trace hash. The amount denomination is also set for + // the minting. + self.ctx + .borrow_mut() + .store_ibc_denom(trace_hash, ibc_denom) + .map_err(|e| { + Error::Denom(format!( + "Writing the IBC denom failed: {}", + e + )) + })?; + let token = storage::ibc_token(ibc_denom); + self.ctx + .borrow_mut() + .store_token_denom(&token) + .map_err(|e| { + Error::Denom(format!( + "Writing the token denom failed: {}", + e + )) + })?; + } } Ok(()) } diff --git a/core/src/ledger/tx_env.rs b/core/src/ledger/tx_env.rs index c53d38d4b2..d10cfdda42 100644 --- a/core/src/ledger/tx_env.rs +++ b/core/src/ledger/tx_env.rs @@ -55,4 +55,10 @@ pub trait TxEnv: StorageRead + StorageWrite { &mut self, event: &IbcEvent, ) -> Result<(), storage_api::Error>; + + /// Get an IBC event with a event type + fn get_ibc_event( + &self, + event_type: impl AsRef, + ) -> Result, storage_api::Error>; } diff --git a/shared/src/ledger/ibc/vp/context.rs b/shared/src/ledger/ibc/vp/context.rs index 7d51914430..5af2afebf7 100644 --- a/shared/src/ledger/ibc/vp/context.rs +++ b/shared/src/ledger/ibc/vp/context.rs @@ -81,6 +81,11 @@ where } } + fn has_key(&self, key: &Key) -> Result { + Ok(self.store.contains_key(key) + || self.ctx.has_key(key).map_err(Error::NativeVpError)?) + } + fn iter_prefix<'iter>( &'iter self, prefix: &Key, @@ -113,6 +118,18 @@ where Ok(()) } + fn get_ibc_event( + &self, + event_type: impl AsRef, + ) -> Result, Self::Error> { + for event in &self.event { + if event.event_type == *event_type.as_ref() { + return Ok(Some(event.clone())); + } + } + Ok(None) + } + fn transfer_token( &mut self, src: &Address, @@ -280,6 +297,10 @@ where self.ctx.read_bytes(key).map_err(Error::NativeVpError) } + fn has_key(&self, key: &Key) -> Result { + self.ctx.has_key(key).map_err(Error::NativeVpError) + } + fn iter_prefix<'iter>( &'iter self, prefix: &Key, @@ -306,6 +327,13 @@ where unimplemented!("Validation doesn't emit an event") } + fn get_ibc_event( + &self, + _event_type: impl AsRef, + ) -> Result, Self::Error> { + unimplemented!("Validation doesn't get an event") + } + fn transfer_token( &mut self, _src: &Address, diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index f290e484ff..5c71529eae 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -1007,6 +1007,40 @@ where tx_add_gas(env, gas) } +/// Getting an IBC event function exposed to the wasm VM Tx environment. +pub fn tx_get_ibc_event( + env: &TxVmEnv, + event_type_ptr: u64, + event_type_len: u64, +) -> TxResult +where + MEM: VmMemory, + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + CA: WasmCacheAccess, +{ + let (event_type, gas) = env + .memory + .read_string(event_type_ptr, event_type_len as _) + .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; + tx_add_gas(env, gas)?; + let write_log = unsafe { env.ctx.write_log.get() }; + for event in write_log.get_ibc_events() { + if event.event_type == event_type { + let value = + event.try_to_vec().map_err(TxRuntimeError::EncodingError)?; + let len: i64 = value + .len() + .try_into() + .map_err(TxRuntimeError::NumConversionError)?; + let result_buffer = unsafe { env.ctx.result_buffer.get() }; + result_buffer.replace(value); + return Ok(len); + } + } + Ok(HostEnvResult::Fail.to_i64()) +} + /// Storage read prior state (before tx execution) function exposed to the wasm /// VM VP environment. It will try to read from the storage. /// diff --git a/shared/src/vm/wasm/host_env.rs b/shared/src/vm/wasm/host_env.rs index 0899110164..c9c67c5bab 100644 --- a/shared/src/vm/wasm/host_env.rs +++ b/shared/src/vm/wasm/host_env.rs @@ -73,6 +73,7 @@ where "namada_tx_update_validity_predicate" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_update_validity_predicate), "namada_tx_init_account" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_init_account), "namada_tx_emit_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_emit_ibc_event), + "namada_tx_get_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_ibc_event), "namada_tx_get_chain_id" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_chain_id), "namada_tx_get_tx_index" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_tx_index), "namada_tx_get_block_height" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_block_height), diff --git a/tests/src/vm_host_env/tx.rs b/tests/src/vm_host_env/tx.rs index a0e88ab802..485a6fb7c1 100644 --- a/tests/src/vm_host_env/tx.rs +++ b/tests/src/vm_host_env/tx.rs @@ -448,6 +448,7 @@ mod native_tx_host_env { result_ptr: u64 )); native_host_fn!(tx_emit_ibc_event(event_ptr: u64, event_len: u64)); + native_host_fn!(tx_get_ibc_event(event_type_ptr: u64, event_type_len: u64) -> i64); native_host_fn!(tx_get_chain_id(result_ptr: u64)); native_host_fn!(tx_get_block_height() -> u64); native_host_fn!(tx_get_tx_index() -> u32); diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index 51f4959323..f7ee8230b1 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -37,6 +37,10 @@ impl IbcStorageContext for Ctx { self.read_bytes(key) } + fn has_key(&self, key: &Key) -> Result { + ::has_key(self, key) + } + fn write( &mut self, key: &Key, @@ -71,6 +75,13 @@ impl IbcStorageContext for Ctx { ::emit_ibc_event(self, &event) } + fn get_ibc_event( + &self, + event_type: impl AsRef, + ) -> Result, Self::Error> { + ::get_ibc_event(self, &event_type) + } + fn transfer_token( &mut self, src: &Address, diff --git a/tx_prelude/src/lib.rs b/tx_prelude/src/lib.rs index cd2f5d5f92..aa2979882b 100644 --- a/tx_prelude/src/lib.rs +++ b/tx_prelude/src/lib.rs @@ -324,4 +324,24 @@ impl TxEnv for Ctx { }; Ok(()) } + + fn get_ibc_event( + &self, + event_type: impl AsRef, + ) -> Result, Error> { + let event_type = event_type.as_ref().to_string(); + let read_result = unsafe { + namada_tx_get_ibc_event( + event_type.as_ptr() as _, + event_type.len() as _, + ) + }; + match read_from_buffer(read_result, namada_tx_result_buffer) { + Some(value) => Ok(Some( + ibc::IbcEvent::try_from_slice(&value[..]) + .expect("The conversion shouldn't fail"), + )), + None => Ok(None), + } + } } diff --git a/vm_env/src/lib.rs b/vm_env/src/lib.rs index f51d92e62e..d5f23b548a 100644 --- a/vm_env/src/lib.rs +++ b/vm_env/src/lib.rs @@ -78,6 +78,12 @@ pub mod tx { // Emit an IBC event pub fn namada_tx_emit_ibc_event(event_ptr: u64, event_len: u64); + // Get an IBC event + pub fn namada_tx_get_ibc_event( + event_type_ptr: u64, + event_type_len: u64, + ) -> i64; + // Get the chain ID pub fn namada_tx_get_chain_id(result_ptr: u64); From ca379419b8de0cb625fa8a0878d3b16abd0523cc Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 2 Aug 2023 12:01:19 +0200 Subject: [PATCH 06/11] add a test for receiving no token --- tests/src/vm_host_env/ibc.rs | 7 +++ tests/src/vm_host_env/mod.rs | 82 ++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/tests/src/vm_host_env/ibc.rs b/tests/src/vm_host_env/ibc.rs index 97d5f5e6f4..59edad5465 100644 --- a/tests/src/vm_host_env/ibc.rs +++ b/tests/src/vm_host_env/ibc.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use namada::ibc::applications::transfer::acknowledgement::TokenTransferAcknowledgement; use namada::ibc::applications::transfer::coin::PrefixedCoin; +use namada::ibc::applications::transfer::error::TokenTransferError; use namada::ibc::applications::transfer::msgs::transfer::MsgTransfer; use namada::ibc::applications::transfer::packet::PacketData; use namada::ibc::applications::transfer::VERSION; @@ -780,3 +781,9 @@ pub fn balance_key_with_ibc_prefix(denom: String, owner: &Address) -> Key { let ibc_token = ibc_token(denom); token::balance_key(&ibc_token, owner) } + +pub fn transfer_ack_with_error() -> TokenTransferAcknowledgement { + TokenTransferAcknowledgement::Error( + TokenTransferError::PacketDataDeserialization.to_string(), + ) +} diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index 7144744f83..dd18ddc1c3 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -18,6 +18,7 @@ pub mod vp; #[cfg(test)] mod tests { + use std::collections::BTreeSet; use std::panic; use itertools::Itertools; @@ -1364,6 +1365,87 @@ mod tests { assert_eq!(minted, Some(Amount::from_u64(100))); } + #[test] + fn test_ibc_receive_no_token() { + // The environment must be initialized first + tx_host_env::init(); + + // Set the initial state before starting transactions + let (token, receiver) = ibc::init_storage(); + let (client_id, _client_state, mut writes) = ibc::prepare_client(); + let (conn_id, conn_writes) = ibc::prepare_opened_connection(&client_id); + writes.extend(conn_writes); + let (port_id, channel_id, channel_writes) = + ibc::prepare_opened_channel(&conn_id, false); + writes.extend(channel_writes); + + writes.into_iter().for_each(|(key, val)| { + tx_host_env::with(|env| { + env.wl_storage + .storage + .write(&key, &val) + .expect("write error"); + }); + }); + + // packet with invalid data + let sequence = ibc::Sequence::from(1); + let mut packet = ibc::received_packet( + port_id.clone(), + channel_id.clone(), + sequence, + token.to_string(), + &receiver, + ); + packet.data = vec![0]; + + // Start a transaction to receive a packet + let msg = ibc::msg_packet_recv(packet); + let mut tx_data = vec![]; + msg.to_any().encode(&mut tx_data).expect("encoding failed"); + + let mut tx = Tx::new(TxType::Raw); + tx.set_code(Code::new(vec![])); + tx.set_data(Data::new(tx_data.clone())); + tx.add_section(Section::Signature(Signature::new( + vec![*tx.code_sechash(), *tx.data_sechash()], + &key::testing::keypair_1(), + ))); + // Receive the packet, but no token is received + tx_host_env::ibc::ibc_actions(tx::ctx()) + .execute(&tx_data) + .expect("receiving the token failed"); + + // Check if the transaction is valid + let env = tx_host_env::take(); + let result = ibc::validate_ibc_vp_from_tx(&env, &tx); + assert!(result.expect("validation failed unexpectedly")); + // Check if the ack has an error due to the invalid packet data + tx_host_env::set(env); + let ack_key = ibc_storage::ack_key(&port_id, &channel_id, sequence); + let ack = tx_host_env::with(|env| { + env.wl_storage + .read_bytes(&ack_key) + .expect("read error") + .unwrap() + }); + let expected_ack = + Hash::sha256(Vec::::from(ibc::transfer_ack_with_error())) + .to_vec(); + assert_eq!(ack, expected_ack); + // Check if only the ack and the receipt are added + let receipt_key = + ibc_storage::receipt_key(&port_id, &channel_id, sequence); + let changed_keys = tx_host_env::with(|env| { + env.wl_storage + .write_log + .verifiers_and_changed_keys(&BTreeSet::new()) + .1 + }); + let expected_changed_keys = BTreeSet::from([ack_key, receipt_key]); + assert_eq!(changed_keys, expected_changed_keys); + } + #[test] fn test_ibc_unescrow_token() { // The environment must be initialized first From c08cc863fab5f87cc28c8a0ba779c59e146ee4a9 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 14 Aug 2023 21:26:54 +0200 Subject: [PATCH 07/11] fix a test --- tests/src/vm_host_env/mod.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index dd18ddc1c3..69772f5b73 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -1370,6 +1370,12 @@ mod tests { // The environment must be initialized first tx_host_env::init(); + let keypair = key::testing::keypair_1(); + let keypairs = vec![keypair.clone()]; + let pks_map = AccountPublicKeysMap::from_iter([ + key::testing::keypair_1().ref_to(), + ]); + // Set the initial state before starting transactions let (token, receiver) = ibc::init_storage(); let (client_id, _client_state, mut writes) = ibc::prepare_client(); @@ -1404,13 +1410,11 @@ mod tests { let mut tx_data = vec![]; msg.to_any().encode(&mut tx_data).expect("encoding failed"); - let mut tx = Tx::new(TxType::Raw); - tx.set_code(Code::new(vec![])); - tx.set_data(Data::new(tx_data.clone())); - tx.add_section(Section::Signature(Signature::new( - vec![*tx.code_sechash(), *tx.data_sechash()], - &key::testing::keypair_1(), - ))); + let mut tx = Tx::new(ChainId::default(), None); + tx.add_code(vec![]) + .add_serialized_data(tx_data.clone()) + .sign_raw(keypairs, pks_map) + .sign_wrapper(keypair); // Receive the packet, but no token is received tx_host_env::ibc::ibc_actions(tx::ctx()) .execute(&tx_data) From 875b2711e898ca30cf3bfb31f2051eca47e46cf7 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 14 Aug 2023 22:03:55 +0200 Subject: [PATCH 08/11] for clippy --- core/src/ledger/ibc/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 0c9337bf85..04db50e3ef 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -140,7 +140,7 @@ where .borrow() .get_ibc_event("denomination_trace") .map_err(|_| { - Error::Denom(format!("Reading the IBC event failed",)) + Error::Denom("Reading the IBC event failed".to_string()) })?; if let Some(event) = result { if let (Some(trace_hash), Some(ibc_denom)) = ( From 95abf84746a37e67509e20143f96abcd6aa682b7 Mon Sep 17 00:00:00 2001 From: yito88 Date: Tue, 15 Aug 2023 16:22:22 +0200 Subject: [PATCH 09/11] add changelog --- .changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md diff --git a/.changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md b/.changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md new file mode 100644 index 0000000000..1533e6bc70 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md @@ -0,0 +1 @@ +- Fix IBC amount handling ([\#1744](https://github.com/anoma/namada/issues/1744)) \ No newline at end of file From 87e153f1c0bd0e88f747fc2eb7c39a66f437dbf7 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 16 Aug 2023 12:37:58 +0200 Subject: [PATCH 10/11] refine code --- core/src/ledger/ibc/context/common.rs | 26 +++++------ core/src/ledger/ibc/mod.rs | 67 ++++++++++++++------------- shared/src/ledger/tx.rs | 7 +-- 3 files changed, 48 insertions(+), 52 deletions(-) diff --git a/core/src/ledger/ibc/context/common.rs b/core/src/ledger/ibc/context/common.rs index fde2335492..5e963e7a5f 100644 --- a/core/src/ledger/ibc/context/common.rs +++ b/core/src/ledger/ibc/context/common.rs @@ -404,21 +404,17 @@ pub trait IbcCommonContext: IbcStorageContext { ), }) })?; - match bytes { - Some(b) => { - let denom = - token::Denomination::try_from_slice(&b).map_err(|_| { - ContextError::ChannelError(ChannelError::Other { - description: format!( - "Decoding the token denom failed: Token {}", - token - ), - }) - })?; - Ok(Some(denom)) - } - None => Ok(None), - } + bytes + .map(|b| token::Denomination::try_from_slice(&b)) + .transpose() + .map_err(|_| { + ContextError::ChannelError(ChannelError::Other { + description: format!( + "Decoding the token denom failed: Token {}", + token + ), + }) + }) } /// Write the IBC denom diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 04db50e3ef..3338ba0b09 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -142,39 +142,42 @@ where .map_err(|_| { Error::Denom("Reading the IBC event failed".to_string()) })?; - if let Some(event) = result { - if let (Some(trace_hash), Some(ibc_denom)) = ( - event.attributes.get("trace_hash"), - event.attributes.get("denom"), - ) { - // If the denomination trace event has the trace hash - // and the IBC denom, a token has been minted. The raw - // IBC denom including the port ID, the channel ID and - // the base token is stored to be restored from the - // trace hash. The amount denomination is also set for - // the minting. - self.ctx - .borrow_mut() - .store_ibc_denom(trace_hash, ibc_denom) - .map_err(|e| { - Error::Denom(format!( - "Writing the IBC denom failed: {}", - e - )) - })?; - let token = storage::ibc_token(ibc_denom); - self.ctx - .borrow_mut() - .store_token_denom(&token) - .map_err(|e| { - Error::Denom(format!( - "Writing the token denom failed: {}", - e - )) - })?; - } + if let Some((trace_hash, ibc_denom)) = result + .as_ref() + .map(|event| { + event + .attributes + .get("trace_hash") + .zip(event.attributes.get("denom")) + }) + .flatten() + { + // If the denomination trace event has the trace hash and + // the IBC denom, a token has been minted. The raw IBC denom + // including the port ID, the channel ID and the base token + // is stored to be restored from the trace hash. The amount + // denomination is also set for the minting. + self.ctx + .borrow_mut() + .store_ibc_denom(trace_hash, ibc_denom) + .map_err(|e| { + Error::Denom(format!( + "Writing the IBC denom failed: {}", + e + )) + })?; + let token = storage::ibc_token(ibc_denom); + self.ctx.borrow_mut().store_token_denom(&token).map_err( + |e| { + Error::Denom(format!( + "Writing the token denom failed: {}", + e + )) + }, + ) + } else { + Ok(()) } - Ok(()) } // other messages _ => Ok(()), diff --git a/shared/src/ledger/tx.rs b/shared/src/ledger/tx.rs index 9732dab631..f6830c0ae7 100644 --- a/shared/src/ledger/tx.rs +++ b/shared/src/ledger/tx.rs @@ -1337,7 +1337,7 @@ pub async fn build_pgf_stewards_proposal< /// Submit an IBC transfer pub async fn build_ibc_transfer( client: &C, - mut args: args::TxIbcTransfer, + args: args::TxIbcTransfer, gas_payer: &common::PublicKey, ) -> Result { // Check that the source address exists on chain @@ -1357,7 +1357,7 @@ pub async fn build_ibc_transfer( validated_amount ))); } - let validate_fee = validate_amount( + validate_amount( client, args.tx.gas_amount, &args.tx.gas_token, @@ -1366,9 +1366,6 @@ pub async fn build_ibc_transfer( .await .expect("expected to be able to validate fee"); - args.amount = InputAmount::Validated(validated_amount); - args.tx.gas_amount = InputAmount::Validated(validate_fee); - // Check source balance let balance_key = token::balance_key(&args.token, &source); From d37fdaf63f52fe85d4b1bb10a9c97e545577c54a Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 16 Aug 2023 17:15:42 +0200 Subject: [PATCH 11/11] for clippy --- core/src/ledger/ibc/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 3338ba0b09..fcabcee745 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -142,15 +142,13 @@ where .map_err(|_| { Error::Denom("Reading the IBC event failed".to_string()) })?; - if let Some((trace_hash, ibc_denom)) = result - .as_ref() - .map(|event| { + if let Some((trace_hash, ibc_denom)) = + result.as_ref().and_then(|event| { event .attributes .get("trace_hash") .zip(event.attributes.get("denom")) }) - .flatten() { // If the denomination trace event has the trace hash and // the IBC denom, a token has been minted. The raw IBC denom