From 48f8b46844e7155843efe7789e220db73a86749e Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 31 Oct 2024 14:54:54 +0000 Subject: [PATCH 1/3] 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 d8460a5b5a..8d101e358e 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 fddea0d821..a6713e4fee 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 60bf301990..7128d8a3b0 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 a73d4f639e..c2174546be 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 df6a158a62..b7d1338d61 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 289feb654b..a85336e9a6 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 a658cad918..63c878a25f 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/3] 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 a6713e4fee..25ffff324f 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 7128d8a3b0..eb4e12b604 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/3] 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 0000000000..8e405b32b2 --- /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