Merge branch 'yuji/fix-ibc-amount' (#1784)

* origin/yuji/fix-ibc-amount:
  for clippy
  refine code
  add changelog
  for clippy
  fix a test
  add a test for receiving no token
  fix denom update
  for clippy
  fix tests
  fix amount conversion
  IBC transfer for non-native denom

.changelog/unreleased/bug-fixes/1744-fix-ibc-amount.md

@@ -0,0 +1 @@
+- Fix IBC amount handling ([\#1744](https://github.com/anoma/namada/issues/1744))

Cargo.toml

@@ -103,6 +103,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"

apps/src/lib/cli.rs

@@ -3479,7 +3479,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);
@@ -3491,7 +3491,7 @@ pub mod args {
                 source,
                 receiver,
                 token,
-                amount: amount.amount,
+                amount,
                 port_id,
                 channel_id,
                 timeout_height,

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

core/src/ledger/ibc/context/common.rs

@@ -1,11 +1,10 @@
 //! IbcCommonContext implementation for IBC
 
-use borsh::BorshSerialize;
+use borsh::{BorshDeserialize, BorshSerialize};
 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;
@@ -31,7 +30,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,25 +358,92 @@ 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,
+        trace_hash: impl AsRef<str>,
+        denom: impl AsRef<str>,
     ) -> Result<(), ContextError> {
-        let key = storage::ibc_denom_key(trace_hash);
-        let bytes = denom.to_string().try_to_vec().map_err(|e| {
+        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!(
-                    "Encoding the denom failed: Denom {}, error {}",
-                    denom, e
+                    "Reading the IBC denom failed: Key {}",
+                    key
                 ),
             })
         })?;
-        self.write(&key, bytes).map_err(|_| {
+        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(())
+    }
+
+    /// Read the token denom
+    fn read_token_denom(
+        &self,
+        token: &Address,
+    ) -> Result<Option<token::Denomination>, ContextError> {
+        let key = token::denom_key(token);
+        let bytes = self.read(&key).map_err(|_| {
             ContextError::ChannelError(ChannelError::Other {
-                description: format!("Writing the denom failed: Key {}", key),
+                description: format!(
+                    "Reading the token denom failed: Key {}",
+                    key
+                ),
             })
-        })
+        })?;
+        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
+    fn store_token_denom(
+        &mut self,
+        token: &Address,
+    ) -> Result<(), ContextError> {
+        let key = token::denom_key(token);
+        let has_key = self.has_key(&key).map_err(|_| {
+            ContextError::ChannelError(ChannelError::Other {
+                description: format!(
+                    "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(())
     }
 }

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`
@@ -31,6 +31,9 @@ pub trait IbcStorageContext {
     /// Read IBC-related data
     fn read(&self, key: &Key) -> Result<Option<Vec<u8>>, Self::Error>;
 
+    /// Check if the given key is present
+    fn has_key(&self, key: &Key) -> Result<bool, Self::Error>;
+
     /// Read IBC-related data with a prefix
     fn iter_prefix<'iter>(
         &'iter self,
@@ -52,29 +55,35 @@ 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<str>,
+    ) -> Result<Option<IbcEvent>, Self::Error>;
+
     /// Transfer token
     fn transfer_token(
         &mut self,
         src: &Address,
         dest: &Address,
         token: &Address,
-        amount: Amount,
+        amount: DenominatedAmount,
     ) -> Result<(), Self::Error>;
 
     /// Mint token
     fn mint_token(
         &mut self,
         target: &Address,
         token: &Address,
-        amount: Amount,
+        amount: DenominatedAmount,
     ) -> Result<(), Self::Error>;
 
     /// Burn token
     fn burn_token(
         &mut self,
         target: &Address,
         token: &Address,
-        amount: Amount,
+        amount: DenominatedAmount,
     ) -> Result<(), Self::Error>;
 
     /// Get the current height of this chain

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<C> ModuleWrapper for TransferModule<C>
@@ -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,25 +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.try_into().map_err(|_| {
-        TokenTransferError::InvalidCoin {
-            coin: coin.to_string(),
-        }
-    })?;
-
-    Ok((token, amount))
-}
-
 fn into_channel_error(error: TokenTransferError) -> ChannelError {
     ChannelError::AppModule {
         description: error.to_string(),