fix(ibc-core): reject packets with no timeouts in `send_packet_valida…

…te` (#1205)

* correct name

* add test for packet with no timeout

* ensure packet timeout is set

* rename var

* fix test compile

* packet without timeout should fail

---------

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

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

@@ -30,6 +30,10 @@ pub fn send_packet_validate(
     ctx_a: &impl SendPacketValidationContext,
     packet: &Packet,
 ) -> Result<(), ContextError> {
+    if !packet.timeout_height_on_b.is_set() && !packet.timeout_timestamp_on_b.is_set() {
+        return Err(ContextError::PacketError(PacketError::MissingTimeout));
+    }
+
     let chan_end_path_on_a = ChannelEndPath::new(&packet.port_id_on_a, &packet.chan_id_on_a);
     let chan_end_on_a = ctx_a.channel_end(&chan_end_path_on_a)?;
 

ibc-testkit/src/fixtures/core/channel/packet.rs

@@ -84,7 +84,7 @@ mod tests {
 
         let proof_height = 10;
         let default_raw_packet = dummy_raw_packet(proof_height, 1000);
-        let raw_packet_no_timeout_or_timestamp = dummy_raw_packet(10, 0);
+        let raw_packet_no_timeout_timestamp = dummy_raw_packet(10, 0);
 
         let mut raw_packet_invalid_timeout_height = dummy_raw_packet(0, 10);
         raw_packet_invalid_timeout_height.timeout_height = Some(RawHeight {
@@ -101,8 +101,8 @@ mod tests {
             Test {
                 // Note: ibc-go currently (July 2022) incorrectly rejects this
                 // case, even though it is allowed in ICS-4.
-                name: "Packet with no timeout of timestamp".to_string(),
-                raw: raw_packet_no_timeout_or_timestamp.clone(),
+                name: "Packet with no timeout timestamp".to_string(),
+                raw: raw_packet_no_timeout_timestamp.clone(),
                 want_pass: true,
             },
             Test {
@@ -225,7 +225,7 @@ mod tests {
                 name: "Missing both timeout height and timestamp".to_string(),
                 raw: RawPacket {
                     timeout_height: None,
-                    ..raw_packet_no_timeout_or_timestamp
+                    ..raw_packet_no_timeout_timestamp
                 },
                 want_pass: false,
             }

ibc-testkit/tests/core/ics04_channel/send_packet.rs

@@ -4,6 +4,7 @@ use core::time::Duration;
 use ibc::core::channel::handler::send_packet;
 use ibc::core::channel::types::channel::{ChannelEnd, Counterparty, Order, State};
 use ibc::core::channel::types::packet::Packet;
+use ibc::core::channel::types::timeout::TimeoutHeight;
 use ibc::core::channel::types::Version;
 use ibc::core::client::types::Height;
 use ibc::core::commitment_types::commitment::CommitmentPrefix;
@@ -84,6 +85,13 @@ fn send_packet_processing() {
 
     let client_height = Height::new(0, client_raw_height).unwrap();
 
+    let packet_with_no_timeout: Packet = {
+        let mut packet: Packet = dummy_raw_packet(10, 10).try_into().unwrap();
+        packet.timeout_height_on_b = TimeoutHeight::no_timeout();
+        packet.timeout_timestamp_on_b = Timestamp::none();
+        packet
+    };
+
     let tests: Vec<Test> = vec![
         Test {
             name: "Processing fails because no channel exists in the context".to_string(),
@@ -136,6 +144,21 @@ fn send_packet_processing() {
             packet: packet_timeout_one_before_client_height,
             want_pass: false,
         },
+        Test {
+            name: "Packet without height and timestamp timeout".to_string(),
+            ctx: context
+                .clone()
+                .with_client_config(
+                    MockClientConfig::builder()
+                        .latest_height(client_height)
+                        .build(),
+                )
+                .with_connection(ConnectionId::zero(), conn_end_on_a.clone())
+                .with_channel(PortId::transfer(), ChannelId::zero(), chan_end_on_a.clone())
+                .with_send_sequence(PortId::transfer(), ChannelId::zero(), 1.into()),
+            packet: packet_with_no_timeout,
+            want_pass: false,
+        },
         Test {
             name: "Packet timeout due to timestamp".to_string(),
             ctx: context

ibc-testkit/tests/core/router.rs

@@ -157,6 +157,7 @@ fn routing_module_and_keepers() {
 
     let msg_transfer_no_timeout_or_timestamp = MsgTransferConfig::builder()
         .packet_data(packet_data.clone())
+        // Timestamp::from_nanoseconds(0) and Timestamp::none() are equivalent
         .timeout_timestamp_on_b(Timestamp::from_nanoseconds(0).unwrap())
         .build();
 
@@ -348,13 +349,13 @@ fn routing_module_and_keepers() {
         Test {
             name: "Transfer message no timeout".to_string(),
             msg: msg_transfer_no_timeout.into(),
-            want_pass: true,
+            want_pass: false,
             state_check: None,
         },
         Test {
             name: "Transfer message no timeout nor timestamp".to_string(),
             msg: msg_transfer_no_timeout_or_timestamp.into(),
-            want_pass: true,
+            want_pass: false,
             state_check: None,
         },
         //ICS04-close channel