Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acknowledge packets asynchronously #1392

Merged
merged 3 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changelog/unreleased/features/1392-optional-ack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Support asynchronous packet acknowledgements.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Support asynchronous packet acknowledgements.
- [ibc-core] Support asynchronous packet acknowledgements.

([\#1392](https://github.com/cosmos/ibc-rs/pull/1392))
8 changes: 4 additions & 4 deletions ibc-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
106 changes: 103 additions & 3 deletions ibc-core/ics04-channel/src/handler/acknowledgement.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,119 @@
use ibc_core_channel_types::channel::{Counterparty, Order, State as ChannelState};
use ibc_core_channel_types::acknowledgement::Acknowledgement;
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;
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_with_chan_end<ExecCtx>(
ctx_b: &mut ExecCtx,
chan_end_on_b: &ChannelEnd,
packet: &Packet,
) -> Result<(), ChannelError>
where
ExecCtx: ExecutionContext,
{
// `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())?;
}
_ => {}

Check warning on line 44 in ibc-core/ics04-channel/src/handler/acknowledgement.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/acknowledgement.rs#L44

Added line #L44 was not covered by tests
}

Ok(())
}

pub fn commit_packet_sequence_number<ExecCtx>(
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)?;

Check warning on line 58 in ibc-core/ics04-channel/src/handler/acknowledgement.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/acknowledgement.rs#L50-L58

Added lines #L50 - L58 were not covered by tests

commit_packet_sequence_number_with_chan_end(ctx_b, &chan_end_on_b, packet)
}

Check warning on line 61 in ibc-core/ics04-channel/src/handler/acknowledgement.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/acknowledgement.rs#L60-L61

Added lines #L60 - L61 were not covered by tests

pub fn commit_packet_acknowledgment<ExecCtx>(
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_with_chan_end<ExecCtx>(
ctx_b: &mut ExecCtx,
chan_end_on_b: &ChannelEnd,
packet: Packet,
acknowledgement: Acknowledgement,
) -> Result<(), ChannelError>
where
ExecCtx: ExecutionContext,
{
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))?;
Farhad-Shabani marked this conversation as resolved.
Show resolved Hide resolved
ctx_b.emit_ibc_event(event)?;

Ok(())
}

pub fn emit_packet_acknowledgement_event<ExecCtx>(
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)?;

Check warning on line 112 in ibc-core/ics04-channel/src/handler/acknowledgement.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/acknowledgement.rs#L103-L112

Added lines #L103 - L112 were not covered by tests

emit_packet_acknowledgement_event_with_chan_end(ctx_b, &chan_end_on_b, packet, acknowledgement)
}

Check warning on line 115 in ibc-core/ics04-channel/src/handler/acknowledgement.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/acknowledgement.rs#L114-L115

Added lines #L114 - L115 were not covered by tests

pub fn acknowledgement_packet_validate<ValCtx>(
ctx_a: &ValCtx,
module: &dyn Module,
Expand Down
62 changes: 22 additions & 40 deletions ibc-core/ics04-channel/src/handler/recv_packet.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -16,6 +15,11 @@
use ibc_core_router::module::Module;
use ibc_primitives::prelude::*;

use super::acknowledgement::{
commit_packet_acknowledgment, commit_packet_sequence_number_with_chan_end,
emit_packet_acknowledgement_event_with_chan_end,
};

pub fn recv_packet_validate<ValCtx>(ctx_b: &ValCtx, msg: MsgRecvPacket) -> Result<(), ChannelError>
where
ValCtx: ValidationContext,
Expand Down Expand Up @@ -70,42 +74,16 @@
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_with_chan_end(ctx_b, &chan_end_on_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(
Expand All @@ -115,14 +93,18 @@
));
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_with_chan_end(
ctx_b,
&chan_end_on_b,
msg.packet,
acknowledgement,
)?;
}

Check warning on line 105 in ibc-core/ics04-channel/src/handler/recv_packet.rs

View check run for this annotation

Codecov / codecov/patch

ibc-core/ics04-channel/src/handler/recv_packet.rs#L105

Added line #L105 was not covered by tests

// module specific events/logs
for module_event in extras.events {
ctx_b.emit_ibc_event(IbcEvent::Module(module_event))?;
}
Expand Down
24 changes: 23 additions & 1 deletion ibc-core/ics26-routing/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +126 to +147
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Much appreciated!

fn on_recv_packet_execute(
&mut self,
packet: &Packet,
relayer: &Signer,
) -> (ModuleExtras, Acknowledgement);
) -> (ModuleExtras, Option<Acknowledgement>);

fn on_acknowledgement_packet_validate(
&self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@
&mut self,
_packet: &Packet,
_relayer: &Signer,
) -> (ModuleExtras, Acknowledgement) {
) -> (ModuleExtras, Option<Acknowledgement>) {

Check warning on line 67 in ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs#L67

Added line #L67 was not covered by tests
(
ModuleExtras::empty(),
Acknowledgement::try_from(vec![1u8]).expect("Never fails"),
Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")),

Check warning on line 70 in ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-testkit/src/testapp/ibc/applications/nft_transfer/module.rs#L70

Added line #L70 was not covered by tests
)
}

Expand Down
4 changes: 2 additions & 2 deletions ibc-testkit/src/testapp/ibc/applications/transfer/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ impl Module for DummyTransferModule {
&mut self,
_packet: &Packet,
_relayer: &Signer,
) -> (ModuleExtras, Acknowledgement) {
) -> (ModuleExtras, Option<Acknowledgement>) {
sug0 marked this conversation as resolved.
Show resolved Hide resolved
(
ModuleExtras::empty(),
Acknowledgement::try_from(vec![1u8]).expect("Never fails"),
Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")),
)
}

Expand Down
8 changes: 4 additions & 4 deletions ibc-testkit/src/testapp/ibc/core/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,12 @@ mod tests {
&mut self,
_packet: &Packet,
_relayer: &Signer,
) -> (ModuleExtras, Acknowledgement) {
) -> (ModuleExtras, Option<Acknowledgement>) {
self.counter += 1;

(
ModuleExtras::empty(),
Acknowledgement::try_from(vec![1u8]).expect("Never fails"),
Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")),
)
}

Expand Down Expand Up @@ -379,10 +379,10 @@ mod tests {
&mut self,
_packet: &Packet,
_relayer: &Signer,
) -> (ModuleExtras, Acknowledgement) {
) -> (ModuleExtras, Option<Acknowledgement>) {
(
ModuleExtras::empty(),
Acknowledgement::try_from(vec![1u8]).expect("Never fails"),
Some(Acknowledgement::try_from(vec![1u8]).expect("Never fails")),
)
}

Expand Down
Loading