From 48f8b46844e7155843efe7789e220db73a86749e Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 31 Oct 2024 14:54:54 +0000 Subject: [PATCH 1/6] feat: support optional acks --- ibc-core/README.md | 8 +- .../src/handler/acknowledgement.rs | 80 ++++++++++++++++++- .../ics04-channel/src/handler/recv_packet.rs | 56 ++++--------- ibc-core/ics26-routing/src/module.rs | 24 +++++- .../ibc/applications/nft_transfer/module.rs | 4 +- .../ibc/applications/transfer/module.rs | 4 +- ibc-testkit/src/testapp/ibc/core/types.rs | 8 +- 7 files changed, 129 insertions(+), 55 deletions(-) diff --git a/ibc-core/README.md b/ibc-core/README.md index d8460a5b5..8d101e358 100644 --- a/ibc-core/README.md +++ b/ibc-core/README.md @@ -111,11 +111,11 @@ asynchronously](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channe This allows modules to receive the packet, but only applying the changes at a later time (after which they would write the acknowledgement). -We currently force applications to process the packets as part of -`onRecvPacket()`. If you need asynchronous acknowledgements for your -application, please open an issue. +Our implementation supports the same semantics, as part of the `on_recv_packet_execute` +method of [`Module`]. The documentation of this method explains how packets should be +acknowledged out of sync. -Note that this still makes us 100% compatible with `ibc-go`. +[`Module`]: https://github.com/cosmos/ibc-rs/blob/main/ibc-core/ics26-routing/src/module.rs ## Contributing diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index fddea0d82..a6713e4fe 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -1,19 +1,95 @@ +use ibc_core_channel_types::acknowledgement::Acknowledgement; use ibc_core_channel_types::channel::{Counterparty, Order, State as ChannelState}; use ibc_core_channel_types::commitment::{compute_ack_commitment, compute_packet_commitment}; use ibc_core_channel_types::error::ChannelError; -use ibc_core_channel_types::events::AcknowledgePacket; +use ibc_core_channel_types::events::{AcknowledgePacket, WriteAcknowledgement}; use ibc_core_channel_types::msgs::MsgAcknowledgement; +use ibc_core_channel_types::packet::{Packet, Receipt}; use ibc_core_client::context::prelude::*; use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, SeqAckPath, + AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, + SeqAckPath, SeqRecvPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; +pub fn commit_packet_sequence_number( + ctx_b: &mut ExecCtx, + packet: &Packet, +) -> Result<(), ChannelError> +where + ExecCtx: ExecutionContext, +{ + let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; + + // `recvPacket` core handler state changes + match chan_end_on_b.ordering { + Order::Unordered => { + let receipt_path_on_b = ReceiptPath { + port_id: packet.port_id_on_b.clone(), + channel_id: packet.chan_id_on_b.clone(), + sequence: packet.seq_on_a, + }; + + ctx_b.store_packet_receipt(&receipt_path_on_b, Receipt::Ok)?; + } + Order::Ordered => { + let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; + ctx_b.store_next_sequence_recv(&seq_recv_path_on_b, next_seq_recv.increment())?; + } + _ => {} + } + + Ok(()) +} + +pub fn commit_packet_acknowledgment( + ctx_b: &mut ExecCtx, + packet: &Packet, + acknowledgement: &Acknowledgement, +) -> Result<(), ChannelError> +where + ExecCtx: ExecutionContext, +{ + let ack_path_on_b = AckPath::new(&packet.port_id_on_b, &packet.chan_id_on_b, packet.seq_on_a); + + // `writeAcknowledgement` handler state changes + ctx_b.store_packet_acknowledgement(&ack_path_on_b, compute_ack_commitment(acknowledgement))?; + + Ok(()) +} + +pub fn emit_packet_acknowledgement_event( + ctx_b: &mut ExecCtx, + packet: Packet, + acknowledgement: Acknowledgement, +) -> Result<(), ChannelError> +where + ExecCtx: ExecutionContext, +{ + let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; + let conn_id_on_b = &chan_end_on_b.connection_hops()[0]; + + ctx_b.log_message("success: packet write acknowledgement".to_string())?; + + let event = IbcEvent::WriteAcknowledgement(WriteAcknowledgement::new( + packet, + acknowledgement, + conn_id_on_b.clone(), + )); + ctx_b.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; + ctx_b.emit_ibc_event(event)?; + + Ok(()) +} + pub fn acknowledgement_packet_validate( ctx_a: &ValCtx, module: &dyn Module, diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 60bf30199..7128d8a3b 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -1,9 +1,8 @@ use ibc_core_channel_types::channel::{Counterparty, Order, State as ChannelState}; -use ibc_core_channel_types::commitment::{compute_ack_commitment, compute_packet_commitment}; +use ibc_core_channel_types::commitment::compute_packet_commitment; use ibc_core_channel_types::error::ChannelError; -use ibc_core_channel_types::events::{ReceivePacket, WriteAcknowledgement}; +use ibc_core_channel_types::events::ReceivePacket; use ibc_core_channel_types::msgs::MsgRecvPacket; -use ibc_core_channel_types::packet::Receipt; use ibc_core_client::context::prelude::*; use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_connection::types::State as ConnectionState; @@ -16,6 +15,10 @@ use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; +use super::acknowledgement::{ + commit_packet_acknowledgment, commit_packet_sequence_number, emit_packet_acknowledgement_event, +}; + pub fn recv_packet_validate(ctx_b: &ValCtx, msg: MsgRecvPacket) -> Result<(), ChannelError> where ValCtx: ValidationContext, @@ -70,42 +73,16 @@ where let (extras, acknowledgement) = module.on_recv_packet_execute(&msg.packet, &msg.signer); // state changes - { - // `recvPacket` core handler state changes - match chan_end_on_b.ordering { - Order::Unordered => { - let receipt_path_on_b = ReceiptPath { - port_id: msg.packet.port_id_on_b.clone(), - channel_id: msg.packet.chan_id_on_b.clone(), - sequence: msg.packet.seq_on_a, - }; + commit_packet_sequence_number(ctx_b, &msg.packet)?; - ctx_b.store_packet_receipt(&receipt_path_on_b, Receipt::Ok)?; - } - Order::Ordered => { - let seq_recv_path_on_b = - SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); - let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; - ctx_b.store_next_sequence_recv(&seq_recv_path_on_b, next_seq_recv.increment())?; - } - _ => {} - } - let ack_path_on_b = AckPath::new( - &msg.packet.port_id_on_b, - &msg.packet.chan_id_on_b, - msg.packet.seq_on_a, - ); - // `writeAcknowledgement` handler state changes - ctx_b.store_packet_acknowledgement( - &ack_path_on_b, - compute_ack_commitment(&acknowledgement), - )?; + if let Some(acknowledgement) = acknowledgement.as_ref() { + commit_packet_acknowledgment(ctx_b, &msg.packet, acknowledgement)?; } // emit events and logs { + // receive packet events/logs ctx_b.log_message("success: packet receive".to_string())?; - ctx_b.log_message("success: packet write acknowledgement".to_string())?; let conn_id_on_b = &chan_end_on_b.connection_hops()[0]; let event = IbcEvent::ReceivePacket(ReceivePacket::new( @@ -115,14 +92,13 @@ where )); ctx_b.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; ctx_b.emit_ibc_event(event)?; - let event = IbcEvent::WriteAcknowledgement(WriteAcknowledgement::new( - msg.packet, - acknowledgement, - conn_id_on_b.clone(), - )); - ctx_b.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; - ctx_b.emit_ibc_event(event)?; + // write ack events/logs + if let Some(acknowledgement) = acknowledgement { + emit_packet_acknowledgement_event(ctx_b, msg.packet, acknowledgement)?; + } + + // module specific events/logs for module_event in extras.events { ctx_b.emit_ibc_event(IbcEvent::Module(module_event))?; } diff --git a/ibc-core/ics26-routing/src/module.rs b/ibc-core/ics26-routing/src/module.rs index a73d4f639..c2174546b 100644 --- a/ibc-core/ics26-routing/src/module.rs +++ b/ibc-core/ics26-routing/src/module.rs @@ -123,11 +123,33 @@ pub trait Module: Debug { // if any error occurs, than an "error acknowledgement" // must be returned + /// ICS-26 `onRecvPacket` callback implementation. + /// + /// # Note on optional acknowledgements + /// + /// Acknowledgements can be committed asynchronously, hence + /// the `Option` type. In general, acknowledgements should + /// be committed to storage, accompanied by an ack event, + /// as soon as a packet is received. This will be done + /// automatically as long as `Some(ack)` is returned from + /// this callback. However, in some cases, such as when + /// implementing a multiple hop packet delivery protocol, + /// a packet can only be acknowledged after it has reached + /// the last hop. + /// + /// ## Committing a packet asynchronously + /// + /// Event emission and state updates for packet acknowledgements + /// can be performed asynchronously using [`emit_packet_acknowledgement_event`] + /// and [`commit_packet_acknowledgment`], respectively. + /// + /// [`commit_packet_acknowledgment`]: ../../channel/handler/fn.commit_packet_acknowledgment.html + /// [`emit_packet_acknowledgement_event`]: ../../channel/handler/fn.emit_packet_acknowledgement_event.html fn on_recv_packet_execute( &mut self, packet: &Packet, relayer: &Signer, - ) -> (ModuleExtras, Acknowledgement); + ) -> (ModuleExtras, Option); fn on_acknowledgement_packet_validate( &self, diff --git a/ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs b/ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs index df6a158a6..b7d1338d6 100644 --- a/ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs +++ b/ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs @@ -64,10 +64,10 @@ impl Module for DummyNftTransferModule { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> (ModuleExtras, Acknowledgement) { + ) -> (ModuleExtras, Option) { ( ModuleExtras::empty(), - Acknowledgement::try_from(vec![1u8]).expect("Never fails"), + Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")), ) } diff --git a/ibc-testkit/src/testapp/ibc/applications/transfer/module.rs b/ibc-testkit/src/testapp/ibc/applications/transfer/module.rs index 289feb654..a85336e9a 100644 --- a/ibc-testkit/src/testapp/ibc/applications/transfer/module.rs +++ b/ibc-testkit/src/testapp/ibc/applications/transfer/module.rs @@ -64,10 +64,10 @@ impl Module for DummyTransferModule { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> (ModuleExtras, Acknowledgement) { + ) -> (ModuleExtras, Option) { ( ModuleExtras::empty(), - Acknowledgement::try_from(vec![1u8]).expect("Never fails"), + Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")), ) } diff --git a/ibc-testkit/src/testapp/ibc/core/types.rs b/ibc-testkit/src/testapp/ibc/core/types.rs index a658cad91..63c878a25 100644 --- a/ibc-testkit/src/testapp/ibc/core/types.rs +++ b/ibc-testkit/src/testapp/ibc/core/types.rs @@ -279,12 +279,12 @@ mod tests { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> (ModuleExtras, Acknowledgement) { + ) -> (ModuleExtras, Option) { self.counter += 1; ( ModuleExtras::empty(), - Acknowledgement::try_from(vec![1u8]).expect("Never fails"), + Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")), ) } @@ -379,10 +379,10 @@ mod tests { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> (ModuleExtras, Acknowledgement) { + ) -> (ModuleExtras, Option) { ( ModuleExtras::empty(), - Acknowledgement::try_from(vec![1u8]).expect("Never fails"), + Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")), ) } From 23a7c4662fc175891b8b5a63f611590c952851ac Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Fri, 17 Jan 2025 10:16:10 +0000 Subject: [PATCH 2/6] refactor: add chan end variants of ack commit fns --- .../src/handler/acknowledgement.rs | 40 +++++++++++++++---- .../ics04-channel/src/handler/recv_packet.rs | 12 ++++-- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index a6713e4fe..25ffff324 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -1,5 +1,5 @@ use ibc_core_channel_types::acknowledgement::Acknowledgement; -use ibc_core_channel_types::channel::{Counterparty, Order, State as ChannelState}; +use ibc_core_channel_types::channel::{ChannelEnd, Counterparty, Order, State as ChannelState}; use ibc_core_channel_types::commitment::{compute_ack_commitment, compute_packet_commitment}; use ibc_core_channel_types::error::ChannelError; use ibc_core_channel_types::events::{AcknowledgePacket, WriteAcknowledgement}; @@ -17,16 +17,14 @@ use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; -pub fn commit_packet_sequence_number( +pub fn commit_packet_sequence_number_with_chan_end( ctx_b: &mut ExecCtx, + chan_end_on_b: &ChannelEnd, packet: &Packet, ) -> Result<(), ChannelError> where ExecCtx: ExecutionContext, { - let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); - let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; - // `recvPacket` core handler state changes match chan_end_on_b.ordering { Order::Unordered => { @@ -49,6 +47,19 @@ where Ok(()) } +pub fn commit_packet_sequence_number( + ctx_b: &mut ExecCtx, + packet: &Packet, +) -> Result<(), ChannelError> +where + ExecCtx: ExecutionContext, +{ + let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; + + commit_packet_sequence_number_with_chan_end(ctx_b, &chan_end_on_b, packet) +} + pub fn commit_packet_acknowledgment( ctx_b: &mut ExecCtx, packet: &Packet, @@ -65,16 +76,15 @@ where Ok(()) } -pub fn emit_packet_acknowledgement_event( +pub fn emit_packet_acknowledgement_event_with_chan_end( ctx_b: &mut ExecCtx, + chan_end_on_b: &ChannelEnd, packet: Packet, acknowledgement: Acknowledgement, ) -> Result<(), ChannelError> where ExecCtx: ExecutionContext, { - let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); - let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; let conn_id_on_b = &chan_end_on_b.connection_hops()[0]; ctx_b.log_message("success: packet write acknowledgement".to_string())?; @@ -90,6 +100,20 @@ where Ok(()) } +pub fn emit_packet_acknowledgement_event( + ctx_b: &mut ExecCtx, + packet: Packet, + acknowledgement: Acknowledgement, +) -> Result<(), ChannelError> +where + ExecCtx: ExecutionContext, +{ + let chan_end_path_on_b = ChannelEndPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let chan_end_on_b = ctx_b.channel_end(&chan_end_path_on_b)?; + + emit_packet_acknowledgement_event_with_chan_end(ctx_b, &chan_end_on_b, packet, acknowledgement) +} + pub fn acknowledgement_packet_validate( ctx_a: &ValCtx, module: &dyn Module, diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 7128d8a3b..eb4e12b60 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -16,7 +16,8 @@ use ibc_core_router::module::Module; use ibc_primitives::prelude::*; use super::acknowledgement::{ - commit_packet_acknowledgment, commit_packet_sequence_number, emit_packet_acknowledgement_event, + commit_packet_acknowledgment, commit_packet_sequence_number_with_chan_end, + emit_packet_acknowledgement_event_with_chan_end, }; pub fn recv_packet_validate(ctx_b: &ValCtx, msg: MsgRecvPacket) -> Result<(), ChannelError> @@ -73,7 +74,7 @@ where let (extras, acknowledgement) = module.on_recv_packet_execute(&msg.packet, &msg.signer); // state changes - commit_packet_sequence_number(ctx_b, &msg.packet)?; + commit_packet_sequence_number_with_chan_end(ctx_b, &chan_end_on_b, &msg.packet)?; if let Some(acknowledgement) = acknowledgement.as_ref() { commit_packet_acknowledgment(ctx_b, &msg.packet, acknowledgement)?; @@ -95,7 +96,12 @@ where // write ack events/logs if let Some(acknowledgement) = acknowledgement { - emit_packet_acknowledgement_event(ctx_b, msg.packet, acknowledgement)?; + emit_packet_acknowledgement_event_with_chan_end( + ctx_b, + &chan_end_on_b, + msg.packet, + acknowledgement, + )?; } // module specific events/logs From 2adeb6b9133680a5dd60d0cffaadd95c8799619a Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Wed, 15 Jan 2025 10:30:06 +0000 Subject: [PATCH 3/6] chore: changelog for #1392 --- .changelog/unreleased/features/1392-optional-ack.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/1392-optional-ack.md diff --git a/.changelog/unreleased/features/1392-optional-ack.md b/.changelog/unreleased/features/1392-optional-ack.md new file mode 100644 index 000000000..8e405b32b --- /dev/null +++ b/.changelog/unreleased/features/1392-optional-ack.md @@ -0,0 +1,2 @@ +- Support asynchronous packet acknowledgements. + ([\#1392](https://github.com/cosmos/ibc-rs/pull/1392)) \ No newline at end of file From 0c083bbb3a289149ae3879fc0c6feb93e6f2f55d Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 2 Dec 2024 13:54:48 +0100 Subject: [PATCH 4/6] feat: convert signers with context --- ibc-apps/ics20-transfer/src/context.rs | 6 +++++- ibc-apps/ics20-transfer/src/handler/mod.rs | 16 ++++++---------- .../src/handler/on_recv_packet.rs | 14 ++++++++------ .../src/handler/send_transfer.rs | 18 ++++++------------ .../ibc/applications/transfer/context.rs | 4 ++++ 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/ibc-apps/ics20-transfer/src/context.rs b/ibc-apps/ics20-transfer/src/context.rs index 2917c9fe9..ca7611dcd 100644 --- a/ibc-apps/ics20-transfer/src/context.rs +++ b/ibc-apps/ics20-transfer/src/context.rs @@ -8,7 +8,11 @@ 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 account id. + fn account_id_from_signer(&self, signer: &Signer) -> Option; /// 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..918958792 100644 --- a/ibc-apps/ics20-transfer/src/handler/mod.rs +++ b/ibc-apps/ics20-transfer/src/handler/mod.rs @@ -17,11 +17,9 @@ 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 + .account_id_from_signer(&data.sender) + .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( packet.port_id_on_a.clone(), @@ -48,11 +46,9 @@ 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 + .account_id_from_signer(&data.sender) + .ok_or(TokenTransferError::FailedToParseAccount)?; 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..9fa942a90 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,14 @@ 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 + .account_id_from_signer(&data.receiver) + .ok_or_else(|| { + ( + ModuleExtras::empty(), + TokenTransferError::FailedToParseAccount, + ) + })?; 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..6752430f6 100644 --- a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs @@ -56,12 +56,9 @@ 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 + .account_id_from_signer(&msg.packet_data.sender) + .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( msg.port_id_on_a.clone(), @@ -129,12 +126,9 @@ 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 + .account_id_from_signer(&msg.packet_data.sender) + .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( msg.port_id_on_a.clone(), diff --git a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs index d4a9677ef..b2e2d1c6e 100644 --- a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs +++ b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs @@ -9,6 +9,10 @@ use super::types::DummyTransferModule; impl TokenTransferValidationContext for DummyTransferModule { type AccountId = Signer; + fn account_id_from_signer(&self, signer: &Signer) -> Option { + Some(signer.clone()) + } + fn get_port(&self) -> Result { Ok(PortId::transfer()) } From 6c8b880f484e75e673b2e1d8e194a3edd2f93c7b Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 2 Dec 2024 15:39:07 +0100 Subject: [PATCH 5/6] feat: split account ids into sender/receiver --- ibc-apps/ics20-transfer/src/context.rs | 7 +++++-- ibc-apps/ics20-transfer/src/handler/mod.rs | 4 ++-- ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs | 2 +- ibc-apps/ics20-transfer/src/handler/send_transfer.rs | 4 ++-- .../src/testapp/ibc/applications/transfer/context.rs | 6 +++++- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/ibc-apps/ics20-transfer/src/context.rs b/ibc-apps/ics20-transfer/src/context.rs index ca7611dcd..6fce5225b 100644 --- a/ibc-apps/ics20-transfer/src/context.rs +++ b/ibc-apps/ics20-transfer/src/context.rs @@ -11,8 +11,11 @@ pub trait TokenTransferValidationContext { /// Native chain account id. type AccountId; - /// Attempt to convert a [`Signer`] to a native chain account id. - fn account_id_from_signer(&self, signer: &Signer) -> Option; + /// Attempt to convert a [`Signer`] to a native chain sender account. + fn sender_account_from_signer(&self, signer: &Signer) -> Option; + + /// Attempt to convert a [`Signer`] to a native chain receiver account. + fn receiver_account_from_signer(&self, signer: &Signer) -> Option; /// 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 918958792..48d5215ca 100644 --- a/ibc-apps/ics20-transfer/src/handler/mod.rs +++ b/ibc-apps/ics20-transfer/src/handler/mod.rs @@ -18,7 +18,7 @@ pub fn refund_packet_token_execute( data: &PacketData, ) -> Result<(), TokenTransferError> { let sender = ctx_a - .account_id_from_signer(&data.sender) + .sender_account_from_signer(&data.sender) .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( @@ -47,7 +47,7 @@ pub fn refund_packet_token_validate( data: &PacketData, ) -> Result<(), TokenTransferError> { let sender = ctx_a - .account_id_from_signer(&data.sender) + .sender_account_from_signer(&data.sender) .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( 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 9fa942a90..d7e022292 100644 --- a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs @@ -24,7 +24,7 @@ pub fn process_recv_packet_execute( .map_err(|err| (ModuleExtras::empty(), err.into()))?; let receiver_account = ctx_b - .account_id_from_signer(&data.receiver) + .receiver_account_from_signer(&data.receiver) .ok_or_else(|| { ( 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 6752430f6..0c4b1c7e9 100644 --- a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs @@ -57,7 +57,7 @@ where let token = &msg.packet_data.token; let sender = token_ctx_a - .account_id_from_signer(&msg.packet_data.sender) + .sender_account_from_signer(&msg.packet_data.sender) .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( @@ -127,7 +127,7 @@ where let token = &msg.packet_data.token; let sender = token_ctx_a - .account_id_from_signer(&msg.packet_data.sender) + .sender_account_from_signer(&msg.packet_data.sender) .ok_or(TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( diff --git a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs index b2e2d1c6e..86a06dc23 100644 --- a/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs +++ b/ibc-testkit/src/testapp/ibc/applications/transfer/context.rs @@ -9,7 +9,11 @@ use super::types::DummyTransferModule; impl TokenTransferValidationContext for DummyTransferModule { type AccountId = Signer; - fn account_id_from_signer(&self, signer: &Signer) -> Option { + fn sender_account_from_signer(&self, signer: &Signer) -> Option { + Some(signer.clone()) + } + + fn receiver_account_from_signer(&self, signer: &Signer) -> Option { Some(signer.clone()) } From 12ffb4bd01e962b86e2db00b10ef60921a9b8500 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Fri, 17 Jan 2025 10:40:41 +0000 Subject: [PATCH 6/6] chore: changelog for #1393 --- .changelog/unreleased/features/1393-signer-parsing-from-ctx.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/features/1393-signer-parsing-from-ctx.md diff --git a/.changelog/unreleased/features/1393-signer-parsing-from-ctx.md b/.changelog/unreleased/features/1393-signer-parsing-from-ctx.md new file mode 100644 index 000000000..49e2da9bd --- /dev/null +++ b/.changelog/unreleased/features/1393-signer-parsing-from-ctx.md @@ -0,0 +1,3 @@ +- 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