refactor: cleaned up some code

contracts/ics02-client/src/contract.rs

@@ -32,7 +32,6 @@ pub fn instantiate(
 ///
 /// # Errors
 /// Will return an error if the handler returns an error.
-#[allow(clippy::needless_pass_by_value)]
 #[cosmwasm_std::entry_point]
 pub fn execute(
     deps: DepsMut,
@@ -64,11 +63,10 @@ pub fn execute(
 ///
 /// # Errors
 /// Will return an error if the handler returns an error.
-#[allow(clippy::needless_pass_by_value)]
 #[cosmwasm_std::entry_point]
-pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> Result<Binary, ContractError> {
+pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result<Binary, ContractError> {
     match msg {
-        QueryMsg::ClientInfo { client_id } => query::client_info(deps, client_id),
+        QueryMsg::ClientInfo { client_id } => query::client_info(deps, env, client_id),
     }
 }
 
@@ -184,13 +182,13 @@ mod execute {
 }
 
 mod query {
-    use super::{state, Binary, ContractError, Deps};
+    use super::{state, Binary, ContractError, Deps, Env};
 
     use crate::types::msg::query_responses;
 
     /// Returns the address of the client encoded as a JSON binary.
     #[allow(clippy::needless_pass_by_value)]
-    pub fn client_info(deps: Deps, client_id: String) -> Result<Binary, ContractError> {
+    pub fn client_info(deps: Deps, _env: Env, client_id: String) -> Result<Binary, ContractError> {
         let address = state::CLIENTS.load(deps.storage, &client_id)?;
         let counterparty_info = state::COUNTERPARTY.may_load(deps.storage, &client_id)?;
         let creator = state::CREATORS.load(deps.storage, &client_id)?;

contracts/ics26-router/src/contract.rs

@@ -33,7 +33,6 @@ pub fn instantiate(
         &deps.querier,
         &env,
         ics02_client::types::msg::InstantiateMsg {},
-        // TODO: ensure there is no DOS attack vector here
         format!("{}.{}", keys::ICS02_CLIENT_SALT, env.contract.address),
         None::<String>,
         keys::ICS02_CLIENT_SALT,
@@ -74,7 +73,6 @@ pub fn execute(
 #[allow(clippy::needless_pass_by_value)]
 pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result<Response, ContractError> {
     match msg.id {
-        // TODO: Ensure that events are emitted for all the replies.
         keys::reply::ON_RECV_PACKET => {
             reply::write_acknowledgement(deps, env, msg.result, msg.payload)
         }
@@ -149,34 +147,22 @@ mod execute {
             }
         }
 
-        let source_channel = identifiers::ChannelId::from_str(&msg.source_channel)?;
-        let source_port = identifiers::PortId::from_str(&msg.source_port)?;
-        let dest_port = identifiers::PortId::from_str(&msg.dest_port)?;
-        let dest_channel = identifiers::ChannelId::from_str(
-            msg.dest_channel.as_ref().unwrap_or(&counterparty_id),
-        )?;
-
         // Ensure the timeout is valid.
         utils::timeout::validate(&env, &msg.timeout)?;
 
         // Construct the packet.
-        let sequence: identifiers::Sequence = state::helpers::new_sequence_send(
-            deps.storage,
-            source_port.as_str(),
-            source_channel.as_str(),
-        )?
-        .into();
-        let packet = ibc::Packet {
+        let sequence =
+            state::helpers::new_sequence_send(deps.storage, &msg.source_port, &msg.source_channel)?;
+        let packet = ibc::Packet::new(
             sequence,
-            source_channel,
-            source_port,
-            destination_channel: dest_channel,
-            destination_port: dest_port,
-            data: msg.data,
-            timeout: msg.timeout,
-        };
+            &msg.source_channel,
+            &msg.source_port,
+            &counterparty_id,
+            &msg.dest_port,
+            msg.data,
+            msg.timeout,
+        )?;
 
-        // TODO: Ensure it is ok to commit packet and emit events before the callback.
         state::helpers::commit_packet(deps.storage, &packet)?;
 
         let send_packet_event = events::send_packet::success(&packet);
@@ -200,9 +186,8 @@ mod execute {
         info: MessageInfo,
         msg: RecvPacketMsg,
     ) -> Result<Response, ContractError> {
+        msg.packet.validate()?;
         let packet = msg.packet;
-        let proof_commitment = msg.proof_commitment;
-        let proof_height = msg.proof_height;
 
         let ics02_address = state::ICS02_CLIENT_ADDRESS.load(deps.storage)?;
         let ics02_contract = ics02_client::helpers::Ics02ClientContract::new(ics02_address);
@@ -232,10 +217,10 @@ mod execute {
         }
         .to_prefixed_merkle_path(counterparty.merkle_path_prefix)?;
         let verify_membership_msg = VerifyMembershipMsgRaw {
-            proof: proof_commitment.into(),
+            proof: msg.proof_commitment.into(),
             path: counterparty_commitment_path,
             value: packet.to_commitment_vec(),
-            height: proof_height.into(),
+            height: msg.proof_height.into(),
             delay_time_period: 0,
             delay_block_period: 0,
         };
@@ -259,7 +244,6 @@ mod execute {
         )
         .with_payload(reply_payload);
 
-        // TODO: Ensure event emission is reverted if the callback fails.
         Ok(Response::new()
             .add_submessage(recv_packet_callback)
             .add_event(event))
@@ -272,6 +256,7 @@ mod execute {
         info: MessageInfo,
         msg: AcknowledgementMsg,
     ) -> Result<Response, ContractError> {
+        msg.packet.validate()?;
         let packet = msg.packet;
 
         let ics02_address = state::ICS02_CLIENT_ADDRESS.load(deps.storage)?;
@@ -350,6 +335,7 @@ mod execute {
         info: MessageInfo,
         msg: TimeoutMsg,
     ) -> Result<Response, ContractError> {
+        msg.packet.validate()?;
         let packet = msg.packet;
 
         let ics02_address = state::ICS02_CLIENT_ADDRESS.load(deps.storage)?;

e2e/interchaintestv8/chainconfig/chain_config.go

@@ -38,8 +38,8 @@ var DefaultChainSpecs = []*interchaintest.ChainSpec{
 			ChainID: "simd-1",
 			Images: []ibc.DockerImage{
 				{
-					Repository: "ghcr.io/cosmos/ibc-go-simd",    // FOR LOCAL IMAGE USE: Docker Image Name
-					Version:    "serdar-xxx-ibc-lite-e2e-debug", // FOR LOCAL IMAGE USE: Docker Image Tag
+					Repository: "ghcr.io/cosmos/ibc-go-simd", // FOR LOCAL IMAGE USE: Docker Image Name
+					Version:    "ibc-lite",                   // FOR LOCAL IMAGE USE: Docker Image Tag
 					UidGid:     "1025:1025",
 				},
 			},

packages/shared/src/types/ibc.rs

@@ -1,7 +1,10 @@
 //! Defines the IBC types used by the contract.
 
+use std::str::FromStr;
+
 use cosmwasm_schema::cw_serde;
 use cosmwasm_std::{Binary, IbcTimeout};
+use ibc_core_host::types::error::IdentifierError;
 use sha2::Digest;
 
 use super::{error::ContractError, paths::identifiers};
@@ -57,6 +60,46 @@ pub struct Height {
 // }
 
 impl Packet {
+    /// Creates a new validated packet with the given parameters.
+    ///
+    /// # Errors
+    /// Fails if any of the identifiers are invalid.
+    pub fn new(
+        sequence: impl Into<identifiers::Sequence>,
+        source_port: &str,
+        source_channel: &str,
+        destination_port: &str,
+        destination_channel: &str,
+        data: Binary,
+        timeout: IbcTimeout,
+    ) -> Result<Self, IdentifierError> {
+        let source_port = identifiers::PortId::from_str(source_port)?;
+        let source_channel = identifiers::ChannelId::from_str(source_channel)?;
+        let destination_port = identifiers::PortId::from_str(destination_port)?;
+        let destination_channel = identifiers::ChannelId::from_str(destination_channel)?;
+        Ok(Self {
+            sequence: sequence.into(),
+            source_port,
+            source_channel,
+            destination_port,
+            destination_channel,
+            data,
+            timeout,
+        })
+    }
+
+    /// Validates the packet identifiers.
+    ///
+    /// # Errors
+    /// Fails if any of the identifiers are invalid.
+    pub fn validate(&self) -> Result<(), IdentifierError> {
+        self.source_port.validate()?;
+        self.destination_port.validate()?;
+        identifiers::ChannelId::from_str(self.source_channel.as_str())?;
+        identifiers::ChannelId::from_str(self.destination_channel.as_str())?;
+        Ok(())
+    }
+
     /// `to_commitment_bytes` serializes the packet to commitment bytes as per [ibc-lite go implementation](https://github.com/cosmos/ibc-go/blob/2b40562bcd59ce820ddd7d6732940728487cf94e/modules/core/04-channel/types/packet.go#L38)
     #[must_use]
     pub fn to_commitment_vec(&self) -> Vec<u8> {