Skip to content

Commit

Permalink
fix: remove faulty receipt check during recv_packet_validate (#1337)
Browse files Browse the repository at this point in the history
* fix: remove faulty receipt check + improve docs around

* chore: add unclog

* Clarify some comments

---------

Co-authored-by: Sean Chen <seanchen11235@gmail.com>
  • Loading branch information
Farhad-Shabani and seanchen1991 committed Sep 12, 2024
1 parent 4e6c12e commit 25f730d
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- [ibc-core] Remove faulty receipt check during `recv_packet_validate`
([#1336](https://github.com/cosmos/ibc-rs/issues/1336)).
19 changes: 6 additions & 13 deletions ibc-core/ics04-channel/src/handler/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ where
let packet = &msg.packet;
let receipt_path_on_b =
ReceiptPath::new(&packet.port_id_on_b, &packet.chan_id_on_b, packet.seq_on_a);
ctx_b.get_packet_receipt(&receipt_path_on_b).is_ok()
ctx_b.get_packet_receipt(&receipt_path_on_b)?.is_ok()
}
Order::Ordered => {
let seq_recv_path_on_b =
Expand Down Expand Up @@ -248,18 +248,11 @@ where
}
}
Order::Unordered => {
let receipt_path_on_b = ReceiptPath::new(
&msg.packet.port_id_on_a,
&msg.packet.chan_id_on_a,
msg.packet.seq_on_a,
);
let packet_rec = ctx_b.get_packet_receipt(&receipt_path_on_b);
match packet_rec {
Ok(_receipt) => {}
Err(ContextError::PacketError(PacketError::PacketReceiptNotFound { sequence }))
if sequence == msg.packet.seq_on_a => {}
Err(e) => return Err(e),
}
// Note: We don't check for the packet receipt here because another
// relayer may have already relayed the packet. If that's the case,
// we want to avoid failing the transaction and consuming
// unnecessary fees.

// Case where the recvPacket is successful and an
// acknowledgement will be written (not a no-op)
validate_write_acknowledgement(ctx_b, msg)?;
Expand Down
15 changes: 15 additions & 0 deletions ibc-core/ics04-channel/types/src/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ pub enum PacketMsgType {
}

/// Packet receipt, used over unordered channels.
///
/// If the receipt is present in the host's state, it's marked as `Ok`,
/// indicating the packet has already been processed. If the receipt is absent,
/// it's marked as `None`, meaning the packet has not been received.
#[cfg_attr(
feature = "parity-scale-codec",
derive(
Expand All @@ -36,6 +40,17 @@ pub enum PacketMsgType {
#[derive(Clone, Debug)]
pub enum Receipt {
Ok,
None,
}

impl Receipt {
pub fn is_ok(&self) -> bool {
matches!(self, Receipt::Ok)
}

pub fn is_none(&self) -> bool {
matches!(self, Receipt::None)
}
}

impl core::fmt::Display for PacketMsgType {
Expand Down
9 changes: 8 additions & 1 deletion ibc-core/ics24-host/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,14 @@ pub trait ValidationContext {
commitment_path: &CommitmentPath,
) -> Result<PacketCommitment, ContextError>;

/// Returns the packet receipt for the given store path
/// Returns the packet receipt for the given store path. This receipt is
/// used to acknowledge the successful processing of a received packet, and
/// must not be pruned.
///
/// If the receipt is present in the host's state, return `Receipt::Ok`,
/// indicating the packet has already been processed. If the receipt is
/// absent, return `Receipt::None`, indicating the packet has not been
/// received.
fn get_packet_receipt(&self, receipt_path: &ReceiptPath) -> Result<Receipt, ContextError>;

/// Returns the packet acknowledgement for the given store path
Expand Down
3 changes: 1 addition & 2 deletions ibc-query/src/core/channel/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,8 @@ where
{
let receipt_path = ReceiptPath::new(&request.port_id, &request.channel_id, request.sequence);

// Receipt only has one enum
// Unreceived packets are not stored
let packet_receipt_data = ibc_ctx.get_packet_receipt(&receipt_path);
let packet_receipt_data = ibc_ctx.get_packet_receipt(&receipt_path)?;

let proof_height = match request.query_height {
Some(height) => height,
Expand Down
20 changes: 7 additions & 13 deletions ibc-testkit/src/testapp/ibc/core/core_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,20 +211,14 @@ where
}

fn get_packet_receipt(&self, receipt_path: &ReceiptPath) -> Result<Receipt, ContextError> {
Ok(self
if self
.packet_receipt_store
.is_path_set(
StoreHeight::Pending,
&ReceiptPath::new(
&receipt_path.port_id,
&receipt_path.channel_id,
receipt_path.sequence,
),
)
.then_some(Receipt::Ok)
.ok_or(PacketError::PacketReceiptNotFound {
sequence: receipt_path.sequence,
})?)
.is_path_set(StoreHeight::Pending, receipt_path)
{
Ok(Receipt::Ok)
} else {
Ok(Receipt::None)
}
}

fn get_packet_acknowledgement(
Expand Down
3 changes: 3 additions & 0 deletions tests-integration/tests/core/ics04_channel/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ fn recv_packet_validate_happy_path(fixture: Fixture) {
packet.seq_on_a,
);

// Note: For unordered channels, there's no need to set a packet receipt.
// The validation will pass whether the receipt exists or not.

let msg_envelope = MsgEnvelope::from(PacketMsg::from(msg));

let res = validate(&context.ibc_store, &router, msg_envelope);
Expand Down

0 comments on commit 25f730d

Please sign in to comment.