From a5f9fbf236cb26db3a76cde20ba43382de2d1ba4 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 21 Jan 2025 14:34:18 +0000 Subject: [PATCH] Signer parsing from context (#1393) * feat: convert signers with context * chore: changelog for #1393 --- .../1393-signer-parsing-from-ctx.md | 3 +++ ibc-apps/ics20-transfer/src/context.rs | 9 ++++++++- ibc-apps/ics20-transfer/src/handler/mod.rs | 12 ++---------- .../ics20-transfer/src/handler/on_recv_packet.rs | 9 +++------ .../ics20-transfer/src/handler/send_transfer.rs | 14 ++------------ ibc-apps/ics20-transfer/types/src/error.rs | 2 -- ibc-apps/ics721-nft-transfer/src/context.rs | 9 ++++++++- ibc-apps/ics721-nft-transfer/src/handler/mod.rs | 12 ++---------- .../src/handler/on_recv_packet.rs | 9 +++------ .../src/handler/send_transfer.rs | 14 ++------------ ibc-apps/ics721-nft-transfer/types/src/error.rs | 2 -- .../ibc/applications/nft_transfer/context.rs | 8 ++++++++ .../testapp/ibc/applications/transfer/context.rs | 8 ++++++++ 13 files changed, 49 insertions(+), 62 deletions(-) create mode 100644 .changelog/unreleased/breaking-changes/1393-signer-parsing-from-ctx.md diff --git a/.changelog/unreleased/breaking-changes/1393-signer-parsing-from-ctx.md b/.changelog/unreleased/breaking-changes/1393-signer-parsing-from-ctx.md new file mode 100644 index 000000000..6567d8249 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/1393-signer-parsing-from-ctx.md @@ -0,0 +1,3 @@ +- [ibc-apps] Replace the `TryFrom` bound on `AccountId` with new + context methods, with the aim of contextually parsing `Signer` instances. + ([\#1393](https://github.com/cosmos/ibc-rs/pull/1393)) \ No newline at end of file diff --git a/ibc-apps/ics20-transfer/src/context.rs b/ibc-apps/ics20-transfer/src/context.rs index 2917c9fe9..8960c8adc 100644 --- a/ibc-apps/ics20-transfer/src/context.rs +++ b/ibc-apps/ics20-transfer/src/context.rs @@ -8,7 +8,14 @@ use ibc_core::primitives::Signer; /// Methods required in token transfer validation, to be implemented by the host pub trait TokenTransferValidationContext { - type AccountId: TryFrom; + /// Native chain account id. + type AccountId; + + /// Attempt to convert a [`Signer`] to a native chain sender account. + fn sender_account(&self, sender: &Signer) -> Result; + + /// Attempt to convert a [`Signer`] to a native chain receiver account. + fn receiver_account(&self, receiver: &Signer) -> Result; /// get_port returns the portID for the transfer module. fn get_port(&self) -> Result; diff --git a/ibc-apps/ics20-transfer/src/handler/mod.rs b/ibc-apps/ics20-transfer/src/handler/mod.rs index f1cd65967..eede4ff5a 100644 --- a/ibc-apps/ics20-transfer/src/handler/mod.rs +++ b/ibc-apps/ics20-transfer/src/handler/mod.rs @@ -17,11 +17,7 @@ pub fn refund_packet_token_execute( packet: &Packet, data: &PacketData, ) -> Result<(), TokenTransferError> { - let sender = data - .sender - .clone() - .try_into() - .map_err(|_| TokenTransferError::FailedToParseAccount)?; + let sender = ctx_a.sender_account(&data.sender)?; if is_sender_chain_source( packet.port_id_on_a.clone(), @@ -48,11 +44,7 @@ pub fn refund_packet_token_validate( packet: &Packet, data: &PacketData, ) -> Result<(), TokenTransferError> { - let sender = data - .sender - .clone() - .try_into() - .map_err(|_| TokenTransferError::FailedToParseAccount)?; + let sender = ctx_a.sender_account(&data.sender)?; if is_sender_chain_source( packet.port_id_on_a.clone(), 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 76e22b3b6..a20d904a9 100644 --- a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs @@ -23,12 +23,9 @@ pub fn process_recv_packet_execute( .can_receive_coins() .map_err(|err| (ModuleExtras::empty(), err.into()))?; - let receiver_account = data.receiver.clone().try_into().map_err(|_| { - ( - ModuleExtras::empty(), - TokenTransferError::FailedToParseAccount, - ) - })?; + let receiver_account = ctx_b + .receiver_account(&data.receiver) + .map_err(|err| (ModuleExtras::empty(), err.into()))?; let extras = if is_receiver_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs index 39d40ea46..24369adc5 100644 --- a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs @@ -56,12 +56,7 @@ where let token = &msg.packet_data.token; - let sender: TokenCtx::AccountId = msg - .packet_data - .sender - .clone() - .try_into() - .map_err(|_| TokenTransferError::FailedToParseAccount)?; + let sender = token_ctx_a.sender_account(&msg.packet_data.sender)?; if is_sender_chain_source( msg.port_id_on_a.clone(), @@ -129,12 +124,7 @@ where let token = &msg.packet_data.token; - let sender = msg - .packet_data - .sender - .clone() - .try_into() - .map_err(|_| TokenTransferError::FailedToParseAccount)?; + let sender = token_ctx_a.sender_account(&msg.packet_data.sender)?; if is_sender_chain_source( msg.port_id_on_a.clone(), diff --git a/ibc-apps/ics20-transfer/types/src/error.rs b/ibc-apps/ics20-transfer/types/src/error.rs index 3c2058598..605588074 100644 --- a/ibc-apps/ics20-transfer/types/src/error.rs +++ b/ibc-apps/ics20-transfer/types/src/error.rs @@ -30,8 +30,6 @@ pub enum TokenTransferError { FailedToDeserializePacketData, /// failed to deserialize acknowledgement FailedToDeserializeAck, - /// failed to parse account - FailedToParseAccount, } #[cfg(feature = "std")] diff --git a/ibc-apps/ics721-nft-transfer/src/context.rs b/ibc-apps/ics721-nft-transfer/src/context.rs index 201d342cb..8f1da87c4 100644 --- a/ibc-apps/ics721-nft-transfer/src/context.rs +++ b/ibc-apps/ics721-nft-transfer/src/context.rs @@ -36,10 +36,17 @@ pub trait NftClassContext { /// Read-only methods required in NFT transfer validation context. pub trait NftTransferValidationContext { - type AccountId: TryFrom + PartialEq; + /// Native chain account id. + type AccountId: PartialEq; type Nft: NftContext; type NftClass: NftClassContext; + /// Attempt to convert a [`Signer`] to a native chain sender account. + fn sender_account(&self, sender: &Signer) -> Result; + + /// Attempt to convert a [`Signer`] to a native chain receiver account. + fn receiver_account(&self, receiver: &Signer) -> Result; + /// get_port returns the portID for the transfer module. fn get_port(&self) -> Result; diff --git a/ibc-apps/ics721-nft-transfer/src/handler/mod.rs b/ibc-apps/ics721-nft-transfer/src/handler/mod.rs index 661533958..41bf1251b 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/mod.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/mod.rs @@ -17,11 +17,7 @@ pub fn refund_packet_nft_execute( packet: &Packet, data: &PacketData, ) -> Result<(), NftTransferError> { - let sender = data - .sender - .clone() - .try_into() - .map_err(|_| NftTransferError::FailedToParseAccount)?; + let sender = ctx_a.sender_account(&data.sender)?; if is_sender_chain_source( packet.port_id_on_a.clone(), @@ -58,11 +54,7 @@ pub fn refund_packet_nft_validate( packet: &Packet, data: &PacketData, ) -> Result<(), NftTransferError> { - let sender = data - .sender - .clone() - .try_into() - .map_err(|_| NftTransferError::FailedToParseAccount)?; + let sender = ctx_a.sender_account(&data.sender)?; if is_sender_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs b/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs index 59cae4ddb..5827803e6 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs @@ -25,12 +25,9 @@ where .can_receive_nft() .map_err(|err| (ModuleExtras::empty(), err.into()))?; - let receiver_account = data.receiver.clone().try_into().map_err(|_| { - ( - ModuleExtras::empty(), - NftTransferError::FailedToParseAccount, - ) - })?; + let receiver_account = ctx_b + .receiver_account(&data.receiver) + .map_err(|err| (ModuleExtras::empty(), err.into()))?; let extras = if is_receiver_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs b/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs index c156f75f2..1a2072de9 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs @@ -56,12 +56,7 @@ where let seq_send_path_on_a = SeqSendPath::new(&msg.port_id_on_a, &msg.chan_id_on_a); let sequence = send_packet_ctx_a.get_next_sequence_send(&seq_send_path_on_a)?; - let sender: TransferCtx::AccountId = msg - .packet_data - .sender - .clone() - .try_into() - .map_err(|_| NftTransferError::FailedToParseAccount)?; + let sender: TransferCtx::AccountId = transfer_ctx.sender_account(&msg.packet_data.sender)?; let mut packet_data = msg.packet_data; let class_id = &packet_data.class_id; @@ -159,12 +154,7 @@ where let seq_send_path_on_a = SeqSendPath::new(&msg.port_id_on_a, &msg.chan_id_on_a); let sequence = send_packet_ctx_a.get_next_sequence_send(&seq_send_path_on_a)?; - let sender = msg - .packet_data - .sender - .clone() - .try_into() - .map_err(|_| NftTransferError::FailedToParseAccount)?; + let sender = transfer_ctx.sender_account(&msg.packet_data.sender)?; let mut packet_data = msg.packet_data; let class_id = &packet_data.class_id; diff --git a/ibc-apps/ics721-nft-transfer/types/src/error.rs b/ibc-apps/ics721-nft-transfer/types/src/error.rs index 9335f4c2f..c295c54cd 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/error.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/error.rs @@ -33,8 +33,6 @@ pub enum NftTransferError { FailedToDeserializePacketData, /// failed to deserialize acknowledgement FailedToDeserializeAck, - /// failed to parse account ID - FailedToParseAccount, /// invalid channel state: cannot be closed InvalidClosedChannel, } diff --git a/ibc-testkit/src/testapp/ibc/applications/nft_transfer/context.rs b/ibc-testkit/src/testapp/ibc/applications/nft_transfer/context.rs index 84838d846..d4b33d20c 100644 --- a/ibc-testkit/src/testapp/ibc/applications/nft_transfer/context.rs +++ b/ibc-testkit/src/testapp/ibc/applications/nft_transfer/context.rs @@ -48,6 +48,14 @@ impl NftTransferValidationContext for DummyNftTransferModule { type Nft = DummyNft; type NftClass = DummyNftClass; + fn sender_account(&self, sender: &Signer) -> Result { + Ok(sender.clone()) + } + + fn receiver_account(&self, receiver: &Signer) -> Result { + Ok(receiver.clone()) + } + fn get_port(&self) -> Result { Ok(PortId::transfer()) } diff --git a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs index d4a9677ef..578646fee 100644 --- a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs +++ b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs @@ -9,6 +9,14 @@ use super::types::DummyTransferModule; impl TokenTransferValidationContext for DummyTransferModule { type AccountId = Signer; + fn sender_account(&self, sender: &Signer) -> Result { + Ok(sender.clone()) + } + + fn receiver_account(&self, receiver: &Signer) -> Result { + Ok(receiver.clone()) + } + fn get_port(&self) -> Result { Ok(PortId::transfer()) }