imp(testkit): integration test for packet timeout (#1215)

* use basecoin rev

* rm RevertibleStore and GrowingStore use

* add packet timeout integration test

this verifies non-membership proofs

* reorder imports

* add timeout_packet_on_channel_close_on_a in utils

* add timeout_packet_on_channel_close_on_a in context

* test for packet timeout on channel close

* use correct event type

* validate packet events

* rm dbg

* add send_packet_via_dummy_transfer_module_on_a method

* use the new method

* add changelog entry

* update doc links

* fix link in doc-strings

* fix for serde feature

* basecoin main branch

* fix unused imports

* cargo format

* fix link in doc-strings

* fix method names and comments

.changelog/unreleased/bug-fixes/1217-fix-packet-timeout-on-closed-channel.md

@@ -0,0 +1,2 @@
+- [ibc-core] Fix proof verification for `PacketTimeout` on a closed channel.
+  ([\#1217](https://github.com/cosmos/ibc-rs/issues/1217))

docs/architecture/adr-009-revamp-testkit.md

@@ -304,16 +304,16 @@ The following provides the concrete implementations of the proposed changes:
 #### MockIbcStore
 
 The modified `MockIbcStore` with Merkle store lives at
-[`testapp/ibc/core/types.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/testapp/ibc/core/types.rs#L43-L96).
+[`testapp/ibc/core/types.rs`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/testapp/ibc/core/types.rs#L43-L96).
 
 #### TestHost
 
 The Rust trait lives at
-[`hosts/mod.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/mod.rs#L27).
+[`hosts/mod.rs`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/hosts/mod.rs#L27).
 The `Mock` and `Tendermint` host implementations live in
-[`hosts/mock.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/mock.rs#L30)
+[`hosts/mock.rs`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/hosts/mock.rs#L30)
 and
-[`hosts/tendermint.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/tendermint.rs#L42)
+[`hosts/tendermint.rs`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/hosts/tendermint.rs#L42)
 respectively.
 
 #### Renaming `MockContext` to `StoreGenericTestContext`
@@ -328,14 +328,14 @@ have `Mock` in their name.
 
 #### StoreGenericTestContext
 
-[`StoreGenericTestContext`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/context.rs#L34-L52)
+[`StoreGenericTestContext`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/context.rs#L34-L52)
 is actually what is described as `MockContext` in the ADR. For convenience, we
 defined `TestContext` to have a concrete store implementation -
-[`MockStore`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/context.rs#L55-L56).
+[`MockStore`](https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/src/context.rs#L55-L56).
 
 ```rs
 // A mock store type using basecoin-storage implementations.
-pub type MockStore = RevertibleStore<GrowingStore<InMemoryStore>>;
+pub type MockStore = InMemoryStore;
 
 pub type TestContext<H> = StoreGenericTestContext<MockStore, H>;
 ```

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

@@ -109,7 +109,7 @@ where
         client_state_of_b_on_a
             .verify_membership(
                 prefix_on_b,
-                &msg.proof_unreceived_on_b,
+                &msg.proof_close_on_b,
                 consensus_state_of_b_on_a.root(),
                 Path::ChannelEnd(chan_end_path_on_b),
                 expected_chan_end_on_b.encode_vec(),

ibc-testkit/Cargo.toml

@@ -36,7 +36,7 @@ ibc-client-tendermint-cw = { workspace = true }
 ibc-query                = { workspace = true }
 
 # basecoin dependencies
-basecoin-store = { git = "https://github.com/informalsystems/basecoin-rs", rev = "8496b3f" }
+basecoin-store = { git = "https://github.com/informalsystems/basecoin-rs", rev = "2dd5b95" }
 
 # cosmos dependencies
 tendermint         = { workspace = true }

ibc-testkit/src/context.rs

@@ -2,7 +2,7 @@ use core::fmt::Debug;
 use core::time::Duration;
 
 use basecoin_store::context::ProvableStore;
-use basecoin_store::impls::{GrowingStore, InMemoryStore, RevertibleStore};
+use basecoin_store::impls::InMemoryStore;
 use ibc::core::channel::types::channel::ChannelEnd;
 use ibc::core::channel::types::commitment::PacketCommitment;
 use ibc::core::client::context::client_state::ClientStateValidation;
@@ -53,7 +53,7 @@ where
 }
 
 /// A mock store type using basecoin-storage implementations.
-pub type MockStore = RevertibleStore<GrowingStore<InMemoryStore>>;
+pub type MockStore = InMemoryStore;
 /// A [`StoreGenericTestContext`] using [`MockStore`].
 pub type TestContext<H> = StoreGenericTestContext<MockStore, H>;
 /// A [`StoreGenericTestContext`] using [`MockStore`] and [`MockHost`].

ibc-testkit/src/relayer/context.rs

@@ -306,9 +306,11 @@ where
         )
     }
 
-    /// Sends a packet from the first context to the second context.
+    /// Sends a packet from the first context to the second context by
+    /// submitting on receive packet on the second context.
+    ///
     /// The IBC packet is created by an IBC application on the first context.
-    pub fn send_packet_on_a(&mut self, packet: Packet, signer: Signer) {
+    pub fn submit_packet_on_b(&mut self, packet: Packet, signer: Signer) {
         let conn_id_on_a = self
             .ctx_a
             .ibc_store()
@@ -347,7 +349,7 @@ where
             .client_id()
             .clone();
 
-        TypedRelayerOps::<A, B>::send_packet_on_a(
+        TypedRelayerOps::<A, B>::submit_packet_on_b(
             &mut self.ctx_a,
             &mut self.ctx_b,
             packet,
@@ -356,4 +358,187 @@ where
             signer,
         )
     }
+
+    /// Times out a packet from the first context to the second context by
+    /// waiting for timeout period and then sending timeout packet on first context.
+    ///
+    /// The IBC packet is created by an IBC application on the first context.
+    pub fn timeout_packet_from_a(&mut self, packet: Packet, signer: Signer) {
+        let conn_id_on_a = self
+            .ctx_a
+            .ibc_store()
+            .channel_end(&ChannelEndPath::new(
+                &packet.port_id_on_a,
+                &packet.chan_id_on_a,
+            ))
+            .expect("connection exists")
+            .connection_hops()[0]
+            .clone();
+
+        let conn_id_on_b = self
+            .ctx_b
+            .ibc_store()
+            .channel_end(&ChannelEndPath::new(
+                &packet.port_id_on_b,
+                &packet.chan_id_on_b,
+            ))
+            .expect("connection exists")
+            .connection_hops()[0]
+            .clone();
+
+        let client_id_on_a = self
+            .ctx_a
+            .ibc_store()
+            .connection_end(&conn_id_on_a)
+            .expect("connection exists")
+            .client_id()
+            .clone();
+
+        let client_id_on_b = self
+            .ctx_b
+            .ibc_store()
+            .connection_end(&conn_id_on_b)
+            .expect("connection exists")
+            .client_id()
+            .clone();
+
+        TypedRelayerOps::<A, B>::timeout_packet_from_a(
+            &mut self.ctx_a,
+            &mut self.ctx_b,
+            packet,
+            client_id_on_a,
+            client_id_on_b,
+            signer,
+        )
+    }
+
+    /// Timeouts a packet from the first context on the second context by closing the
+    /// corresponding channel is closed and then sending a timeout packet on the first context.
+    ///
+    /// The IBC packet is created by an IBC application on the first context.
+    pub fn timeout_packet_from_a_on_channel_close(&mut self, packet: Packet, signer: Signer) {
+        let conn_id_on_a = self
+            .ctx_a
+            .ibc_store()
+            .channel_end(&ChannelEndPath::new(
+                &packet.port_id_on_a,
+                &packet.chan_id_on_a,
+            ))
+            .expect("connection exists")
+            .connection_hops()[0]
+            .clone();
+
+        let conn_id_on_b = self
+            .ctx_b
+            .ibc_store()
+            .channel_end(&ChannelEndPath::new(
+                &packet.port_id_on_b,
+                &packet.chan_id_on_b,
+            ))
+            .expect("connection exists")
+            .connection_hops()[0]
+            .clone();
+
+        let client_id_on_a = self
+            .ctx_a
+            .ibc_store()
+            .connection_end(&conn_id_on_a)
+            .expect("connection exists")
+            .client_id()
+            .clone();
+
+        let client_id_on_b = self
+            .ctx_b
+            .ibc_store()
+            .connection_end(&conn_id_on_b)
+            .expect("connection exists")
+            .client_id()
+            .clone();
+
+        TypedRelayerOps::<A, B>::timeout_packet_from_a_on_channel_close(
+            &mut self.ctx_a,
+            &mut self.ctx_b,
+            packet,
+            client_id_on_a,
+            client_id_on_b,
+            signer,
+        )
+    }
+
+    /// Submit a
+    /// [`DummyTransferModule`](crate::testapp::ibc::applications::transfer::types::DummyTransferModule)
+    /// packet on the first context.
+    ///
+    /// Requires `serde` feature because of [`ibc::apps::transfer::handler::send_transfer`].
+    #[cfg(feature = "serde")]
+    pub fn send_dummy_transfer_packet_on_a(
+        &mut self,
+        chan_id_on_a: ChannelId,
+        signer: Signer,
+    ) -> Packet {
+        use ibc::apps::transfer::handler::send_transfer;
+        use ibc::apps::transfer::types::msgs::transfer::MsgTransfer;
+        use ibc::apps::transfer::types::packet::PacketData;
+        use ibc::core::handler::types::events::IbcEvent;
+        use ibc::primitives::Timestamp;
+
+        use crate::testapp::ibc::applications::transfer::types::DummyTransferModule;
+
+        // generate packet for DummyTransferModule
+        let packet_data = PacketData {
+            token: "1000uibc".parse().expect("valid prefixed coin"),
+            sender: signer.clone(),
+            receiver: signer.clone(),
+            memo: "sample memo".into(),
+        };
+
+        // packet with ibc metadata
+        // either height timeout or timestamp timeout must be set
+        let msg = MsgTransfer {
+            port_id_on_a: PortId::transfer(),
+            chan_id_on_a: chan_id_on_a.clone(),
+            packet_data,
+            // setting timeout height to 10 blocks from B's current height.
+            timeout_height_on_b: self.get_ctx_b().latest_height().add(10).into(),
+            // not setting timeout timestamp.
+            timeout_timestamp_on_b: Timestamp::none(),
+        };
+
+        // module creates the send_packet
+        send_transfer(
+            self.get_ctx_a_mut().ibc_store_mut(),
+            &mut DummyTransferModule,
+            msg,
+        )
+        .expect("successfully created send_packet");
+
+        // send_packet wasn't committed, hence produce a block
+        self.get_ctx_a_mut().advance_block_height();
+
+        // retrieve the send_packet event
+        let Some(IbcEvent::SendPacket(send_packet_event)) = self
+            .get_ctx_a()
+            .ibc_store()
+            .events
+            .lock()
+            .iter()
+            .rev()
+            .nth(2)
+            .cloned()
+        else {
+            panic!("unexpected event")
+        };
+
+        // create the IBC packet type
+        Packet {
+            port_id_on_a: send_packet_event.port_id_on_a().clone(),
+            chan_id_on_a: send_packet_event.chan_id_on_a().clone(),
+            seq_on_a: *send_packet_event.seq_on_a(),
+            data: send_packet_event.packet_data().to_vec(),
+            timeout_height_on_b: *send_packet_event.timeout_height_on_b(),
+            timeout_timestamp_on_b: *send_packet_event.timeout_timestamp_on_b(),
+            port_id_on_b: send_packet_event.port_id_on_b().clone(),
+            chan_id_on_b: send_packet_event.chan_id_on_b().clone(),
+        }
+    }
 }