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

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

Merged
merged 21 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 20 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- [ibc-core] Fix proof verification for `PacketTimeout` on a closed channel.
([\#1217](https://github.com/cosmos/ibc-rs/issues/1217))
14 changes: 7 additions & 7 deletions docs/architecture/adr-009-revamp-testkit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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>;
```
Expand Down
2 changes: 1 addition & 1 deletion ibc-core/ics04-channel/src/handler/timeout_on_close.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion ibc-testkit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
4 changes: 2 additions & 2 deletions ibc-testkit/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`].
Expand Down
179 changes: 179 additions & 0 deletions ibc-testkit/src/relayer/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,4 +356,183 @@
signer,
)
}

/// Timeouts a packet from the first context to the second context.
Copy link
Contributor

Choose a reason for hiding this comment

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

This language is a bit ambiguous when it comes to delineating whether a packet sent between the two contexts was expired, or if an actual timeout packet was sent.

The code seems to indicate the latter, in which case we should say "Sends a timeout packet from the first context to the second context."

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you. I updated the comments and the method names.

3abc78e

/// The IBC packet is created by an IBC application on the first context.
pub fn timeout_packet_on_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_on_a(
&mut self.ctx_a,
&mut self.ctx_b,
packet,
client_id_on_a,
client_id_on_b,
signer,
)
}

/// Timeouts a packet from the second context to the first context,
/// because of the channel is closed.
pub fn timeout_packet_on_channel_close_on_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_on_channel_close_on_a(
&mut self.ctx_a,
&mut self.ctx_b,
packet,
client_id_on_a,
client_id_on_b,
signer,
)
}

/// Submit a packet via
/// [`DummyTransferModule`](crate::testapp::ibc::applications::transfer::types::DummyTransferModule)
/// on the first context.
///
/// Requires `serde` feature because of [`ibc::apps::transfer::handler::send_transfer`].
#[cfg(feature = "serde")]
pub fn send_packet_via_dummy_transfer_module_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")

Check warning on line 523 in ibc-testkit/src/relayer/context.rs

View check run for this annotation

Codecov / codecov/patch

ibc-testkit/src/relayer/context.rs#L523

Added line #L523 was not covered by tests
};

// 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(),
}
}
}
Loading
Loading