fix: remove faulty receipt check during recv_packet_validate (#1337)

* fix: remove faulty receipt check + improve docs around

* chore: add unclog

* Clarify some comments

---------

Co-authored-by: Sean Chen <[email protected]>

.changelog/unreleased/bug-fixes/1336-remove-faulty-receipt-check-during-recv-packet-validate.md

@@ -0,0 +1,2 @@
+- [ibc-core] Remove faulty receipt check during `recv_packet_validate`
+  ([#1336](https://github.com/cosmos/ibc-rs/issues/1336)).

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

@@ -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 =
@@ -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)?;

ibc-core/ics04-channel/types/src/packet.rs

@@ -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(
@@ -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 {

ibc-core/ics24-host/src/context.rs

@@ -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

ibc-query/src/core/channel/query.rs

@@ -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,

ibc-testkit/src/testapp/ibc/core/core_ctx.rs

@@ -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(

tests-integration/tests/core/ics04_channel/recv_packet.rs

@@ -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);