Add HostError contexts to module-level error types (#1345)

* Add ConnectionError::Host variant and clean up some other variants

* Migrate some ics03 handlers to return HostErrors

* Revert "Migrate some ics03 handlers to return HostErrors"

This reverts commit f48a1a6.

* Add HostError variant to ChannelError

* Remove HandlerError::Host variant

* Map client handler HostErrors to ClientError::Host

* Revert "Map client handler HostErrors to ClientError::Host"

This reverts commit 247be8a.

* Change ics02-client handlers to return ClientError

* Change ics03-connection handlers to return ConnectionError

* Change ics04-channel handlers to return ChannelError

* Make necessary changes in ics25 dispatch

* imp: misc improvements and fixes

* imp: use derive_more::From for ChannelError

* fix: rename InvalidClientState under ConnectionError

* nit: remove redundant import

---------

Co-authored-by: Farhad Shabani <[email protected]>

Author: seanchen1991

ibc-apps/ics20-transfer/src/module.rs

@@ -6,7 +6,6 @@ use ibc_core::channel::types::acknowledgement::{Acknowledgement, Acknowledgement
 use ibc_core::channel::types::channel::{Counterparty, Order};
 use ibc_core::channel::types::packet::Packet;
 use ibc_core::channel::types::Version;
-use ibc_core::handler::types::error::HandlerError;
 use ibc_core::host::types::error::DecodingError;
 use ibc_core::host::types::identifiers::{ChannelId, ConnectionId, PortId};
 use ibc_core::primitives::prelude::*;
@@ -42,9 +41,7 @@ pub fn on_chan_open_init_validate(
     }
 
     if !version.is_empty() {
-        version
-            .verify_is_expected(Version::new(VERSION.to_string()))
-            .map_err(HandlerError::from)?;
+        version.verify_is_expected(Version::new(VERSION.to_string()))?;
     }
 
     Ok(())
@@ -78,9 +75,7 @@ pub fn on_chan_open_try_validate(
         });
     }
 
-    counterparty_version
-        .verify_is_expected(Version::new(VERSION.to_string()))
-        .map_err(HandlerError::from)?;
+    counterparty_version.verify_is_expected(Version::new(VERSION.to_string()))?;
 
     Ok(())
 }
@@ -103,9 +98,7 @@ pub fn on_chan_open_ack_validate(
     _channel_id: &ChannelId,
     counterparty_version: &Version,
 ) -> Result<(), TokenTransferError> {
-    counterparty_version
-        .verify_is_expected(Version::new(VERSION.to_string()))
-        .map_err(HandlerError::from)?;
+    counterparty_version.verify_is_expected(Version::new(VERSION.to_string()))?;
 
     Ok(())
 }

ibc-apps/ics20-transfer/types/src/error.rs

@@ -2,17 +2,19 @@
 use displaydoc::Display;
 use ibc_core::channel::types::acknowledgement::StatusValue;
 use ibc_core::channel::types::channel::Order;
-use ibc_core::handler::types::error::HandlerError;
+use ibc_core::channel::types::error::ChannelError;
 use ibc_core::host::types::error::{DecodingError, HostError};
 use ibc_core::host::types::identifiers::{ChannelId, PortId};
 use ibc_core::primitives::prelude::*;
 
-#[derive(Display, Debug)]
+#[derive(Display, Debug, derive_more::From)]
 pub enum TokenTransferError {
     /// host error: `{0}`
-    Handler(HandlerError),
+    Host(HostError),
     /// decoding error: `{0}`
     Decoding(DecodingError),
+    /// channel error: `{0}`
+    Channel(ChannelError),
     /// missing destination channel `{channel_id}` on port `{port_id}`
     MissingDestinationChannel {
         port_id: PortId,
@@ -36,31 +38,14 @@ pub enum TokenTransferError {
 impl std::error::Error for TokenTransferError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match &self {
-            Self::Handler(e) => Some(e),
+            Self::Host(e) => Some(e),
             Self::Decoding(e) => Some(e),
+            Self::Channel(e) => Some(e),
             _ => None,
         }
     }
 }
 
-impl From<HandlerError> for TokenTransferError {
-    fn from(e: HandlerError) -> Self {
-        Self::Handler(e)
-    }
-}
-
-impl From<HostError> for TokenTransferError {
-    fn from(e: HostError) -> Self {
-        Self::Handler(HandlerError::Host(e))
-    }
-}
-
-impl From<DecodingError> for TokenTransferError {
-    fn from(e: DecodingError) -> Self {
-        Self::Decoding(e)
-    }
-}
-
 impl From<TokenTransferError> for StatusValue {
     fn from(e: TokenTransferError) -> Self {
         StatusValue::new(e.to_string()).expect("error message must not be empty")

ibc-apps/ics721-nft-transfer/src/module.rs

@@ -3,7 +3,6 @@ use ibc_core::channel::types::acknowledgement::{Acknowledgement, Acknowledgement
 use ibc_core::channel::types::channel::{Counterparty, Order};
 use ibc_core::channel::types::packet::Packet;
 use ibc_core::channel::types::Version;
-use ibc_core::handler::types::error::HandlerError;
 use ibc_core::host::types::identifiers::{ChannelId, ConnectionId, PortId};
 use ibc_core::primitives::prelude::*;
 use ibc_core::primitives::Signer;
@@ -42,9 +41,7 @@ pub fn on_chan_open_init_validate(
     }
 
     if !version.is_empty() {
-        version
-            .verify_is_expected(Version::new(VERSION.to_string()))
-            .map_err(HandlerError::from)?;
+        version.verify_is_expected(Version::new(VERSION.to_string()))?;
     }
 
     Ok(())
@@ -78,9 +75,7 @@ pub fn on_chan_open_try_validate(
         });
     }
 
-    counterparty_version
-        .verify_is_expected(Version::new(VERSION.to_string()))
-        .map_err(HandlerError::from)?;
+    counterparty_version.verify_is_expected(Version::new(VERSION.to_string()))?;
 
     Ok(())
 }
@@ -103,10 +98,7 @@ pub fn on_chan_open_ack_validate(
     _channel_id: &ChannelId,
     counterparty_version: &Version,
 ) -> Result<(), NftTransferError> {
-    counterparty_version
-        .verify_is_expected(Version::new(VERSION.to_string()))
-        .map_err(HandlerError::from)?;
-
+    counterparty_version.verify_is_expected(Version::new(VERSION.to_string()))?;
     Ok(())
 }
 

ibc-apps/ics721-nft-transfer/types/src/error.rs

@@ -3,15 +3,17 @@ use derive_more::From;
 use displaydoc::Display;
 use ibc_core::channel::types::acknowledgement::StatusValue;
 use ibc_core::channel::types::channel::Order;
-use ibc_core::handler::types::error::HandlerError;
+use ibc_core::channel::types::error::ChannelError;
 use ibc_core::host::types::error::{DecodingError, HostError};
 use ibc_core::host::types::identifiers::{ChannelId, PortId};
 use ibc_core::primitives::prelude::*;
 
 #[derive(Display, Debug, From)]
 pub enum NftTransferError {
-    /// handler error: `{0}`
-    Handler(HandlerError),
+    /// host error: `{0}`
+    Host(HostError),
+    /// channel error: `{0}`
+    Channel(ChannelError),
     /// decoding error: `{0}`
     Decoding(DecodingError),
     /// missing destination channel `{channel_id}` on port `{port_id}`
@@ -41,19 +43,14 @@ pub enum NftTransferError {
 impl std::error::Error for NftTransferError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match &self {
-            Self::Handler(e) => Some(e),
+            Self::Channel(e) => Some(e),
+            Self::Host(e) => Some(e),
             Self::Decoding(e) => Some(e),
             _ => None,
         }
     }
 }
 
-impl From<HostError> for NftTransferError {
-    fn from(e: HostError) -> Self {
-        Self::Handler(HandlerError::Host(e))
-    }
-}
-
 impl From<NftTransferError> for StatusValue {
     fn from(err: NftTransferError) -> Self {
         StatusValue::new(err.to_string()).expect("error message must not be empty")

ibc-core/ics02-client/src/handler/create_client.rs

@@ -5,13 +5,12 @@ use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::events::CreateClient;
 use ibc_core_client_types::msgs::MsgCreateClient;
 use ibc_core_client_types::Status;
-use ibc_core_handler_types::error::HandlerError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
 use ibc_core_host::{ClientStateMut, ClientStateRef, ExecutionContext, ValidationContext};
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
 
-pub fn validate<Ctx>(ctx: &Ctx, msg: MsgCreateClient) -> Result<(), HandlerError>
+pub fn validate<Ctx>(ctx: &Ctx, msg: MsgCreateClient) -> Result<(), ClientError>
 where
     Ctx: ValidationContext,
     <ClientStateRef<Ctx> as TryFrom<Any>>::Error: Into<ClientError>,
@@ -36,21 +35,21 @@ where
     let status = client_state.status(client_val_ctx, &client_id)?;
 
     if status.is_frozen() {
-        return Err(ClientError::UnexpectedStatus(Status::Frozen).into());
+        return Err(ClientError::UnexpectedStatus(Status::Frozen));
     };
 
     let host_timestamp = ctx.host_timestamp()?;
 
     client_state.verify_consensus_state(consensus_state, &host_timestamp)?;
 
     if client_val_ctx.client_state(&client_id).is_ok() {
-        return Err(ClientError::DuplicateClientState(client_id).into());
+        return Err(ClientError::DuplicateClientState(client_id));
     };
 
     Ok(())
 }
 
-pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgCreateClient) -> Result<(), HandlerError>
+pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgCreateClient) -> Result<(), ClientError>
 where
     Ctx: ExecutionContext,
     <ClientStateMut<Ctx> as TryFrom<Any>>::Error: Into<ClientError>,

ibc-core/ics02-client/src/handler/recover_client.rs

@@ -3,15 +3,14 @@
 use ibc_core_client_context::prelude::*;
 use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::msgs::MsgRecoverClient;
-use ibc_core_handler_types::error::HandlerError;
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_core_host::{ExecutionContext, ValidationContext};
 
 /// Performs the validation steps associated with the client recovery process. This
 /// includes validating that the parameters of the subject and substitute clients match,
 /// as well as validating that the substitute client *is* active and that the subject
 /// client is *not* active.
-pub fn validate<Ctx>(ctx: &Ctx, msg: MsgRecoverClient) -> Result<(), HandlerError>
+pub fn validate<Ctx>(ctx: &Ctx, msg: MsgRecoverClient) -> Result<(), ClientError>
 where
     Ctx: ValidationContext,
 {
@@ -33,8 +32,7 @@ where
         return Err(ClientError::NotAllowedClientRecoveryHeights {
             subject_height,
             substitute_height,
-        }
-        .into());
+        });
     }
 
     substitute_client_state
@@ -62,7 +60,7 @@ where
 ///  - copying the substitute client's consensus state as the subject's consensus state
 ///  - setting the subject client's processed height and processed time values to match the substitute client's
 ///  - setting the subject client's latest height, trusting period, and chain ID values to match the substitute client's
-pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgRecoverClient) -> Result<(), HandlerError>
+pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgRecoverClient) -> Result<(), ClientError>
 where
     Ctx: ExecutionContext,
 {

ibc-core/ics02-client/src/handler/update_client.rs

@@ -5,14 +5,13 @@ use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::events::{ClientMisbehaviour, UpdateClient};
 use ibc_core_client_types::msgs::MsgUpdateOrMisbehaviour;
 use ibc_core_client_types::UpdateKind;
-use ibc_core_handler_types::error::HandlerError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
 use ibc_core_host::types::error::HostError;
 use ibc_core_host::{ExecutionContext, ValidationContext};
 use ibc_primitives::prelude::*;
 use ibc_primitives::ToVec;
 
-pub fn validate<Ctx>(ctx: &Ctx, msg: MsgUpdateOrMisbehaviour) -> Result<(), HandlerError>
+pub fn validate<Ctx>(ctx: &Ctx, msg: MsgUpdateOrMisbehaviour) -> Result<(), ClientError>
 where
     Ctx: ValidationContext,
 {
@@ -36,7 +35,7 @@ where
     Ok(())
 }
 
-pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgUpdateOrMisbehaviour) -> Result<(), HandlerError>
+pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgUpdateOrMisbehaviour) -> Result<(), ClientError>
 where
     Ctx: ExecutionContext,
 {
@@ -67,8 +66,7 @@ where
         if !matches!(update_kind, UpdateKind::UpdateClient) {
             return Err(ClientError::FailedMisbehaviourHandling {
                 description: "misbehaviour submitted, but none found".to_string(),
-            }
-            .into());
+            });
         }
 
         let header = client_message;

ibc-core/ics02-client/src/handler/upgrade_client.rs

@@ -1,16 +1,15 @@
 //! Protocol logic specific to processing ICS2 messages of type `MsgUpgradeAnyClient`.
 //!
 use ibc_core_client_context::prelude::*;
+use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::events::UpgradeClient;
 use ibc_core_client_types::msgs::MsgUpgradeClient;
-use ibc_core_handler_types::error::HandlerError;
 use ibc_core_handler_types::events::{IbcEvent, MessageEvent};
-use ibc_core_host::types::error::HostError;
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_core_host::{ExecutionContext, ValidationContext};
 use ibc_primitives::prelude::*;
 
-pub fn validate<Ctx>(ctx: &Ctx, msg: MsgUpgradeClient) -> Result<(), HandlerError>
+pub fn validate<Ctx>(ctx: &Ctx, msg: MsgUpgradeClient) -> Result<(), ClientError>
 where
     Ctx: ValidationContext,
 {
@@ -36,15 +35,7 @@ where
         old_client_state.latest_height().revision_number(),
         old_client_state.latest_height().revision_height(),
     );
-    let old_consensus_state = client_val_ctx
-        .consensus_state(&old_client_cons_state_path)
-        .map_err(|_| {
-            HostError::missing_state(format!(
-                "missing consensus state for client {} at height {}",
-                client_id,
-                old_client_state.latest_height()
-            ))
-        })?;
+    let old_consensus_state = client_val_ctx.consensus_state(&old_client_cons_state_path)?;
 
     // Validate the upgraded client state and consensus state and verify proofs against the root
     old_client_state.verify_upgrade_client(
@@ -58,7 +49,7 @@ where
     Ok(())
 }
 
-pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgUpgradeClient) -> Result<(), HandlerError>
+pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgUpgradeClient) -> Result<(), ClientError>
 where
     Ctx: ExecutionContext,
 {

ibc-core/ics03-connection/src/delay.rs

@@ -2,14 +2,13 @@ use ibc_core_client::context::ClientValidationContext;
 use ibc_core_client::types::Height;
 use ibc_core_connection_types::error::ConnectionError;
 use ibc_core_connection_types::ConnectionEnd;
-use ibc_core_handler_types::error::HandlerError;
 use ibc_core_host::ValidationContext;
 
 pub fn verify_conn_delay_passed<Ctx>(
     ctx: &Ctx,
     packet_proof_height: Height,
     connection_end: &ConnectionEnd,
-) -> Result<(), HandlerError>
+) -> Result<(), ConnectionError>
 where
     Ctx: ValidationContext,
 {
@@ -31,23 +30,19 @@ where
     let earliest_valid_time = (last_client_update.0 + conn_delay_time_period)
         .map_err(ConnectionError::OverflowedTimestamp)?;
     if current_host_time < earliest_valid_time {
-        return Err(HandlerError::Connection(
-            ConnectionError::InsufficientTimeElapsed {
-                current_host_time,
-                earliest_valid_time,
-            },
-        ));
+        return Err(ConnectionError::InsufficientTimeElapsed {
+            current_host_time,
+            earliest_valid_time,
+        });
     }
 
     // Verify that the current host chain height is later than the last client update height
     let earliest_valid_height = last_client_update.1.add(conn_delay_height_period);
     if current_host_height < earliest_valid_height {
-        return Err(HandlerError::Connection(
-            ConnectionError::InsufficientBlocksElapsed {
-                current_host_height,
-                earliest_valid_height,
-            },
-        ));
+        return Err(ConnectionError::InsufficientBlocksElapsed {
+            current_host_height,
+            earliest_valid_height,
+        });
     };
 
     Ok(())