diff --git a/.changelog/unreleased/993-bump-ibc-proto-rs.md b/.changelog/unreleased/993-bump-ibc-proto-rs.md deleted file mode 100644 index bc4fa551d..000000000 --- a/.changelog/unreleased/993-bump-ibc-proto-rs.md +++ /dev/null @@ -1,2 +0,0 @@ -- Bump ibc-proto-rs dependency to v0.39.1. - ([\#993](https://github.com/cosmos/ibc-rs/issues/993)) diff --git a/.changelog/unreleased/breaking-changes/837-split-sent-coins-method copy.md b/.changelog/unreleased/breaking-changes/837-split-sent-coins-method copy.md new file mode 100644 index 000000000..f3877cdbb --- /dev/null +++ b/.changelog/unreleased/breaking-changes/837-split-sent-coins-method copy.md @@ -0,0 +1,5 @@ +- `[ibc-app-transfer]` Refactor `send-coins-*()` methods by breaking them down + into distinct escrow and unescrow methods, enhancing both clarity and + specificity in functionality. + ([\#837](https://github.com/cosmos/ibc-rs/issues/837)) + \ No newline at end of file diff --git a/.changelog/unreleased/breaking-changes/839-add-memo-field-for-token-escrow.md b/.changelog/unreleased/breaking-changes/839-add-memo-field-for-token-escrow.md new file mode 100644 index 000000000..f506d8809 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/839-add-memo-field-for-token-escrow.md @@ -0,0 +1,5 @@ +- `[ibc-app-transfer]` Add `memo` field to `escrow-coins-*()` and + `burn-coins-*()` methods, allowing implementors to pass in arbitrary data + necessary for their use case. + ([\#839](https://github.com/cosmos/ibc-rs/issues/837)) + \ No newline at end of file diff --git a/.changelog/unreleased/breaking-changes/978-identifier-error.md b/.changelog/unreleased/breaking-changes/978-identifier-error.md index cd03b558e..51bc5be68 100644 --- a/.changelog/unreleased/breaking-changes/978-identifier-error.md +++ b/.changelog/unreleased/breaking-changes/978-identifier-error.md @@ -1,2 +1,2 @@ -- Optimize `IdentifierError` variants and make them mutually exclusive. - ([\#978](https://github.com/cosmos/ibc-rs/issues/978)) +- `[ibc-core-host-type]` Optimize `IdentifierError` variants and make them + mutually exclusive. ([\#978](https://github.com/cosmos/ibc-rs/issues/978)) diff --git a/.changelog/unreleased/breaking-changes/993-bump-ibc-proto-rs.md b/.changelog/unreleased/breaking-changes/993-bump-ibc-proto-rs.md new file mode 100644 index 000000000..07c4e5024 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/993-bump-ibc-proto-rs.md @@ -0,0 +1,2 @@ +- `[ibc-data-types]` Bump ibc-proto-rs dependency to v0.39.1. + ([\#993](https://github.com/cosmos/ibc-rs/issues/993)) diff --git a/.changelog/unreleased/breaking-changes/997-minimize-prost-dependency.md b/.changelog/unreleased/breaking-changes/997-minimize-prost-dependency.md index a1e20886d..558b56b31 100644 --- a/.changelog/unreleased/breaking-changes/997-minimize-prost-dependency.md +++ b/.changelog/unreleased/breaking-changes/997-minimize-prost-dependency.md @@ -1,4 +1,4 @@ -- [`ibc`] Minimize `prost` dependency by introducing `ToVec` trait +- `[ibc]` Minimize `prost` dependency by introducing `ToVec` trait - Now `prost` is only imported in `ibc-primitives` crate - Remove error variants originating from `prost` (Breaking change) - Eliminate the need for the `bytes` dependency diff --git a/.changelog/unreleased/improvements/992-more-rigorous-cosmwasm-check.md b/.changelog/unreleased/improvements/992-more-rigorous-cosmwasm-check.md index 7540c8a9d..a4c91b322 100644 --- a/.changelog/unreleased/improvements/992-more-rigorous-cosmwasm-check.md +++ b/.changelog/unreleased/improvements/992-more-rigorous-cosmwasm-check.md @@ -1,3 +1,3 @@ -- [`cw-check`] More rigorous CosmWasm check by upgrading dependencies and +- `[cw-check]` More rigorous CosmWasm check by upgrading dependencies and including `std` and `schema` features for `ibc-core`. ([\#992](https://github.com/cosmos/ibc-rs/pull/992)) diff --git a/ibc-apps/ics20-transfer/src/context.rs b/ibc-apps/ics20-transfer/src/context.rs index 672c87819..c427d4ad6 100644 --- a/ibc-apps/ics20-transfer/src/context.rs +++ b/ibc-apps/ics20-transfer/src/context.rs @@ -1,7 +1,7 @@ //! Defines the main context traits and IBC module callbacks use ibc_app_transfer_types::error::TokenTransferError; -use ibc_app_transfer_types::{PrefixedCoin, PrefixedDenom}; +use ibc_app_transfer_types::{Memo, PrefixedCoin, PrefixedDenom}; use ibc_core::host::types::identifiers::{ChannelId, PortId}; use ibc_core::primitives::prelude::*; use ibc_core::primitives::Signer; @@ -13,24 +13,31 @@ pub trait TokenTransferValidationContext { /// get_port returns the portID for the transfer module. fn get_port(&self) -> Result; - /// Returns the escrow account id for a port and channel combination - fn get_escrow_account( - &self, - port_id: &PortId, - channel_id: &ChannelId, - ) -> Result; - /// Returns Ok() if the host chain supports sending coins. fn can_send_coins(&self) -> Result<(), TokenTransferError>; /// Returns Ok() if the host chain supports receiving coins. fn can_receive_coins(&self) -> Result<(), TokenTransferError>; - /// Validates the sender and receiver accounts and the coin inputs - fn send_coins_validate( + /// Validates that the tokens can be escrowed successfully. + /// + /// `memo` field allows to incorporate additional contextual details in the + /// escrow validation. + fn escrow_coins_validate( &self, from_account: &Self::AccountId, + port_id: &PortId, + channel_id: &ChannelId, + coin: &PrefixedCoin, + memo: &Memo, + ) -> Result<(), TokenTransferError>; + + /// Validates that the tokens can be unescrowed successfully. + fn unescrow_coins_validate( + &self, to_account: &Self::AccountId, + port_id: &PortId, + channel_id: &ChannelId, coin: &PrefixedCoin, ) -> Result<(), TokenTransferError>; @@ -41,11 +48,15 @@ pub trait TokenTransferValidationContext { coin: &PrefixedCoin, ) -> Result<(), TokenTransferError>; - /// Validates the sender account and the coin input + /// Validates the sender account and the coin input before burning. + /// + /// `memo` field allows to incorporate additional contextual details in the + /// burn validation. fn burn_coins_validate( &self, account: &Self::AccountId, coin: &PrefixedCoin, + memo: &Memo, ) -> Result<(), TokenTransferError>; /// Returns a hash of the prefixed denom. @@ -55,27 +66,45 @@ pub trait TokenTransferValidationContext { } } -/// Methods required in token transfer execution, to be implemented by the host +/// Methods required in token transfer execution, to be implemented by the host. pub trait TokenTransferExecutionContext: TokenTransferValidationContext { - /// This function should enable sending ibc fungible tokens from one account to another - fn send_coins_execute( + /// Executes the escrow of the tokens in a user account. + /// + /// `memo` field allows to incorporate additional contextual details in the + /// escrow execution. + fn escrow_coins_execute( &mut self, from_account: &Self::AccountId, + port_id: &PortId, + channel_id: &ChannelId, + coin: &PrefixedCoin, + memo: &Memo, + ) -> Result<(), TokenTransferError>; + + /// Executes the unescrow of the tokens in a user account. + fn unescrow_coins_execute( + &mut self, to_account: &Self::AccountId, + port_id: &PortId, + channel_id: &ChannelId, coin: &PrefixedCoin, ) -> Result<(), TokenTransferError>; - /// This function to enable minting ibc tokens to a user account + /// Executes minting of the tokens in a user account. fn mint_coins_execute( &mut self, account: &Self::AccountId, coin: &PrefixedCoin, ) -> Result<(), TokenTransferError>; - /// This function should enable burning of minted tokens in a user account + /// Executes burning of the tokens in a user account. + /// + /// `memo` field allows to incorporate additional contextual details in the + /// burn execution. fn burn_coins_execute( &mut self, account: &Self::AccountId, coin: &PrefixedCoin, + memo: &Memo, ) -> Result<(), TokenTransferError>; } diff --git a/ibc-apps/ics20-transfer/src/handler/mod.rs b/ibc-apps/ics20-transfer/src/handler/mod.rs index 9689ab629..39dcd373d 100644 --- a/ibc-apps/ics20-transfer/src/handler/mod.rs +++ b/ibc-apps/ics20-transfer/src/handler/mod.rs @@ -27,11 +27,12 @@ pub fn refund_packet_token_execute( packet.chan_id_on_a.clone(), &data.token.denom, ) { - // unescrow tokens back to sender - let escrow_address = - ctx_a.get_escrow_account(&packet.port_id_on_a, &packet.chan_id_on_a)?; - - ctx_a.send_coins_execute(&escrow_address, &sender, &data.token) + ctx_a.unescrow_coins_execute( + &sender, + &packet.port_id_on_a, + &packet.chan_id_on_a, + &data.token, + ) } // mint vouchers back to sender else { @@ -55,10 +56,12 @@ pub fn refund_packet_token_validate( packet.chan_id_on_a.clone(), &data.token.denom, ) { - let escrow_address = - ctx_a.get_escrow_account(&packet.port_id_on_a, &packet.chan_id_on_a)?; - - ctx_a.send_coins_validate(&escrow_address, &sender, &data.token) + ctx_a.unescrow_coins_validate( + &sender, + &packet.port_id_on_a, + &packet.chan_id_on_a, + &data.token, + ) } else { ctx_a.mint_coins_validate(&sender, &data.token) } diff --git a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs index 2baa187f6..193b33196 100644 --- a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs @@ -43,10 +43,6 @@ pub fn process_recv_packet_execute( c }; - let escrow_address = ctx_b - .get_escrow_account(&packet.port_id_on_b, &packet.chan_id_on_b) - .map_err(|token_err| (ModuleExtras::empty(), token_err))?; - // Note: it is correct to do the validation here because `recv_packet()` // works slightly differently. We do not have a // `on_recv_packet_validate()` callback because regardless of whether or @@ -58,11 +54,20 @@ pub fn process_recv_packet_execute( // gets relayed back to the sender so that the escrowed tokens // can be refunded. ctx_b - .send_coins_validate(&escrow_address, &receiver_account, &coin) + .unescrow_coins_validate( + &receiver_account, + &packet.port_id_on_b, + &packet.chan_id_on_b, + &coin, + ) .map_err(|token_err| (ModuleExtras::empty(), token_err))?; - ctx_b - .send_coins_execute(&escrow_address, &receiver_account, &coin) + .unescrow_coins_execute( + &receiver_account, + &packet.port_id_on_b, + &packet.chan_id_on_b, + &coin, + ) .map_err(|token_err| (ModuleExtras::empty(), token_err))?; ModuleExtras::empty() diff --git a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs index 683b96edf..439e5604c 100644 --- a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs @@ -68,11 +68,15 @@ where msg.chan_id_on_a.clone(), &token.denom, ) { - let escrow_address = - token_ctx_a.get_escrow_account(&msg.port_id_on_a, &msg.chan_id_on_a)?; - token_ctx_a.send_coins_validate(&sender, &escrow_address, token)?; + token_ctx_a.escrow_coins_validate( + &sender, + &msg.port_id_on_a, + &msg.chan_id_on_a, + token, + &msg.packet_data.memo, + )?; } else { - token_ctx_a.burn_coins_validate(&sender, token)?; + token_ctx_a.burn_coins_validate(&sender, token, &msg.packet_data.memo)?; } let packet = { @@ -137,11 +141,15 @@ where msg.chan_id_on_a.clone(), &token.denom, ) { - let escrow_address = - token_ctx_a.get_escrow_account(&msg.port_id_on_a, &msg.chan_id_on_a)?; - token_ctx_a.send_coins_execute(&sender, &escrow_address, token)?; + token_ctx_a.escrow_coins_execute( + &sender, + &msg.port_id_on_a, + &msg.chan_id_on_a, + token, + &msg.packet_data.memo, + )?; } else { - token_ctx_a.burn_coins_execute(&sender, token)?; + token_ctx_a.burn_coins_execute(&sender, token, &msg.packet_data.memo)?; } let packet = { diff --git a/ibc-apps/ics20-transfer/types/src/error.rs b/ibc-apps/ics20-transfer/types/src/error.rs index 8737ddf96..f4b06aee5 100644 --- a/ibc-apps/ics20-transfer/types/src/error.rs +++ b/ibc-apps/ics20-transfer/types/src/error.rs @@ -11,12 +11,19 @@ use ibc_core::host::types::identifiers::{ChannelId, PortId}; use ibc_core::primitives::prelude::*; use uint::FromDecStrErr; +use crate::Amount; + #[derive(Display, Debug)] pub enum TokenTransferError { /// context error: `{0}` ContextError(ContextError), /// invalid identifier: `{0}` InvalidIdentifier(IdentifierError), + /// insufficient funds: tried to send `{send_attempt}`, sender only has `{available_funds}` + InsufficientFunds { + send_attempt: Amount, + available_funds: Amount, + }, /// destination channel not found in the counterparty of port_id `{port_id}` and channel_id `{channel_id}` DestinationChannelNotFound { port_id: PortId, @@ -70,6 +77,8 @@ pub enum TokenTransferError { InvalidCoin { coin: String }, /// decoding raw bytes as UTF8 string error: `{0}` Utf8Decode(Utf8Error), + /// other error: `{0}` + Other(String), } #[cfg(feature = "std")] diff --git a/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs b/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs index f100bfb52..06bd48027 100644 --- a/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs +++ b/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs @@ -32,6 +32,7 @@ pub(crate) const TYPE_URL: &str = "/ibc.applications.transfer.v1.MsgTransfer"; feature = "borsh", derive(borsh::BorshSerialize, borsh::BorshDeserialize) )] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct MsgTransfer { /// the port on which the packet will be sent pub port_id_on_a: PortId, diff --git a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs index 2608dcda6..d8876b315 100644 --- a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs +++ b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs @@ -1,10 +1,8 @@ use ibc::apps::transfer::context::{TokenTransferExecutionContext, TokenTransferValidationContext}; use ibc::apps::transfer::types::error::TokenTransferError; -use ibc::apps::transfer::types::PrefixedCoin; +use ibc::apps::transfer::types::{Memo, PrefixedCoin}; use ibc::core::host::types::identifiers::{ChannelId, PortId}; use ibc::core::primitives::Signer; -use ibc::cosmos_host::utils::cosmos_adr028_escrow_address; -use subtle_encoding::bech32; use super::types::DummyTransferModule; @@ -15,15 +13,6 @@ impl TokenTransferValidationContext for DummyTransferModule { Ok(PortId::transfer()) } - fn get_escrow_account( - &self, - port_id: &PortId, - channel_id: &ChannelId, - ) -> Result { - let addr = cosmos_adr028_escrow_address(port_id, channel_id); - Ok(bech32::encode("cosmos", addr).into()) - } - fn can_send_coins(&self) -> Result<(), TokenTransferError> { Ok(()) } @@ -31,11 +20,22 @@ impl TokenTransferValidationContext for DummyTransferModule { fn can_receive_coins(&self) -> Result<(), TokenTransferError> { Ok(()) } - - fn send_coins_validate( + fn escrow_coins_validate( &self, _from_account: &Self::AccountId, + _port_id: &PortId, + _channel_id: &ChannelId, + _coin: &PrefixedCoin, + _memo: &Memo, + ) -> Result<(), TokenTransferError> { + Ok(()) + } + + fn unescrow_coins_validate( + &self, _to_account: &Self::AccountId, + _port_id: &PortId, + _channel_id: &ChannelId, _coin: &PrefixedCoin, ) -> Result<(), TokenTransferError> { Ok(()) @@ -53,16 +53,29 @@ impl TokenTransferValidationContext for DummyTransferModule { &self, _account: &Self::AccountId, _coin: &PrefixedCoin, + _memo: &Memo, ) -> Result<(), TokenTransferError> { Ok(()) } } impl TokenTransferExecutionContext for DummyTransferModule { - fn send_coins_execute( + fn escrow_coins_execute( &mut self, _from_account: &Self::AccountId, + _port_id: &PortId, + _channel_id: &ChannelId, + _coin: &PrefixedCoin, + _memo: &Memo, + ) -> Result<(), TokenTransferError> { + Ok(()) + } + + fn unescrow_coins_execute( + &mut self, _to_account: &Self::AccountId, + _port_id: &PortId, + _channel_id: &ChannelId, _coin: &PrefixedCoin, ) -> Result<(), TokenTransferError> { Ok(()) @@ -80,6 +93,7 @@ impl TokenTransferExecutionContext for DummyTransferModule { &mut self, _account: &Self::AccountId, _coin: &PrefixedCoin, + _memo: &Memo, ) -> Result<(), TokenTransferError> { Ok(()) }