Clean up QueryError and RelayerError types (#1321)

* Clean up ibc-query QueryError type

* Clean up ibc-testkit RelayerError

* Clean up ibc-testkit RelayerError

* Change wording of a doc comment

* Remove RelayerError

ibc-query/src/core/channel/query.rs

@@ -47,7 +47,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::ChannelEnd(channel_end_path.clone()))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for channel end path {channel_end_path:?}"
             ))
         })?;
@@ -119,7 +119,7 @@ where
         .first()
         .map(|connection_id| ibc_ctx.connection_end(connection_id))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Channel {} does not have a connection",
                 request.channel_id
             ))
@@ -140,7 +140,7 @@ where
             &Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for client state path: {:?}",
                 connection_end.client_id()
             ))
@@ -172,7 +172,7 @@ where
         .first()
         .map(|connection_id| ibc_ctx.connection_end(connection_id))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Channel {} does not have a connection",
                 request.channel_id
             ))
@@ -198,7 +198,7 @@ where
             &Path::ClientConsensusState(consensus_path.clone()),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for client consensus state path: {consensus_path:?}"
             ))
         })?;
@@ -233,7 +233,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::Commitment(commitment_path.clone()))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for packet commitment path: {commitment_path:?}"
             ))
         })?;
@@ -291,7 +291,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::Receipt(receipt_path.clone()))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for packet receipt path: {receipt_path:?}"
             ))
         })?;
@@ -325,7 +325,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::Ack(acknowledgement_path.clone()))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for packet acknowledgement path: {acknowledgement_path:?}"
             ))
         })?;
@@ -431,7 +431,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::SeqSend(next_seq_send_path))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Next sequence send proof not found for channel {}",
                 request.channel_id
             ))
@@ -464,7 +464,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::SeqRecv(next_seq_recv_path))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Next sequence receive proof not found for channel {}",
                 request.channel_id
             ))

ibc-query/src/core/client/query.rs

@@ -50,7 +50,7 @@ where
             &Path::ClientState(ClientStatePath::new(client_id.clone())),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for client state path: {client_id:?}"
             ))
         })?;
@@ -109,7 +109,7 @@ where
             .into_iter()
             .max_by_key(|&(h, _)| h)
             .ok_or_else(|| {
-                QueryError::proof_not_found(format!(
+                QueryError::missing_proof(format!(
                     "No consensus state found for client: {client_id:?}"
                 ))
             })?
@@ -130,7 +130,7 @@ where
             )),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for consensus state path: {client_id:?}"
             ))
         })?;
@@ -242,7 +242,7 @@ where
             &Path::UpgradeClientState(upgraded_client_state_path),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for upgraded client state at: {proof_height:?}"
             ))
         })?;
@@ -301,7 +301,7 @@ where
             &Path::UpgradeConsensusState(upgraded_consensus_state_path),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for upgraded consensus state at: {proof_height:?}"
             ))
         })?;

ibc-query/src/core/connection/query.rs

@@ -41,7 +41,7 @@ where
             &Path::Connection(ConnectionPath::new(&request.connection_id)),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for connection path: {:?}",
                 request.connection_id
             ))
@@ -92,7 +92,7 @@ where
             &Path::ClientConnection(ClientConnectionPath::new(request.client_id.clone())),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for client connection path: {:?}",
                 request.client_id
             ))
@@ -130,7 +130,7 @@ where
             &Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())),
         )
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for client state path: {:?}",
                 connection_end.client_id()
             ))
@@ -172,7 +172,7 @@ where
     let proof = ibc_ctx
         .get_proof(proof_height, &Path::ClientConsensusState(consensus_path))
         .ok_or_else(|| {
-            QueryError::proof_not_found(format!(
+            QueryError::missing_proof(format!(
                 "Proof not found for consensus state path: {:?}",
                 connection_end.client_id()
             ))

ibc-query/src/error.rs

@@ -8,21 +8,24 @@ use ibc::core::handler::types::error::ContextError;
 use ibc::core::host::types::error::IdentifierError;
 use tonic::Status;
 
+/// The main error type of the ibc-query crate. This type mainly
+/// serves to surface lower-level errors that occur when executing
+/// ibc-query's codepaths.
 #[derive(Debug, Display)]
 pub enum QueryError {
-    /// Context error: {0}
+    /// context error: `{0}`
     ContextError(ContextError),
-    /// Identifier error: {0}
+    /// identifier error: `{0}`
     IdentifierError(IdentifierError),
-    /// Proof not found: {0}
-    ProofNotFound(String),
-    /// Missing field: {0}
+    /// missing proof: `{0}`
+    MissingProof(String),
+    /// missing field: `{0}`
     MissingField(String),
 }
 
 impl QueryError {
-    pub fn proof_not_found<T: ToString>(description: T) -> Self {
-        Self::ProofNotFound(description.to_string())
+    pub fn missing_proof<T: ToString>(description: T) -> Self {
+        Self::MissingProof(description.to_string())
     }
 
     pub fn missing_field<T: ToString>(description: T) -> Self {
@@ -35,7 +38,7 @@ impl From<QueryError> for Status {
         match e {
             QueryError::ContextError(ctx_err) => Self::internal(ctx_err.to_string()),
             QueryError::IdentifierError(id_err) => Self::internal(id_err.to_string()),
-            QueryError::ProofNotFound(description) => Self::not_found(description),
+            QueryError::MissingProof(description) => Self::not_found(description),
             QueryError::MissingField(description) => Self::invalid_argument(description),
         }
     }

ibc-testkit/src/context.rs

@@ -25,7 +25,6 @@ use ibc::primitives::Timestamp;
 use super::testapp::ibc::core::types::{LightClientState, MockIbcStore};
 use crate::fixtures::core::context::TestContextConfig;
 use crate::hosts::{HostClientState, MockHost, TendermintHost, TestBlock, TestHeader, TestHost};
-use crate::relayer::error::RelayerError;
 use crate::testapp::ibc::clients::{AnyClientState, AnyConsensusState};
 use crate::testapp::ibc::core::router::MockRouter;
 use crate::testapp::ibc::core::types::DEFAULT_BLOCK_TIME_SECS;
@@ -480,9 +479,9 @@ where
     /// A datagram passes from the relayer to the IBC module (on host chain).
     /// Alternative method to `Ics18Context::send` that does not exercise any serialization.
     /// Used in testing the Ics18 algorithms, hence this may return an Ics18Error.
-    pub fn deliver(&mut self, msg: MsgEnvelope) -> Result<(), RelayerError> {
-        self.dispatch(msg)
-            .map_err(RelayerError::TransactionFailed)?;
+    pub fn deliver(&mut self, msg: MsgEnvelope) -> Result<(), ContextError> {
+        self.dispatch(msg)?;
+
         // Create a new block.
         self.advance_block_height();
         Ok(())

ibc-testkit/src/relayer/mod.rs

@@ -1,4 +1,3 @@
 pub mod context;
-pub mod error;
 pub mod integration;
 pub mod utils;

tests-integration/tests/core/ics02_client/update_client.rs

@@ -32,7 +32,6 @@ use ibc_testkit::hosts::tendermint::BlockParams;
 use ibc_testkit::hosts::{
     HostClientState, MockHost, TendermintHost, TestBlock, TestHeader, TestHost,
 };
-use ibc_testkit::relayer::error::RelayerError;
 use ibc_testkit::testapp::ibc::clients::mock::client_state::{
     client_type as mock_client_type, MockClientState,
 };
@@ -1527,7 +1526,7 @@ pub(crate) fn build_client_update_datagram<H: TestHeader, Dst: TestHost>(
     dest: &TestContext<Dst>,
     client_id: &ClientId,
     src_header: &H,
-) -> Result<ClientMsg, RelayerError>
+) -> Option<ClientMsg>
 where
     HostClientState<Dst>: ClientStateValidation<DefaultIbcStore>,
 {
@@ -1536,23 +1535,15 @@ where
     let dest_client_latest_height = dest.light_client_latest_height(client_id);
 
     if src_header.height() == dest_client_latest_height {
-        return Err(RelayerError::ClientAlreadyUpToDate {
-            client_id: client_id.clone(),
-            source_height: src_header.height(),
-            destination_height: dest_client_latest_height,
-        });
+        return None;
     };
 
     if dest_client_latest_height > src_header.height() {
-        return Err(RelayerError::ClientAtHigherHeight {
-            client_id: client_id.clone(),
-            source_height: src_header.height(),
-            destination_height: dest_client_latest_height,
-        });
+        return None;
     };
 
     // Client on destination chain can be updated.
-    Ok(ClientMsg::UpdateClient(MsgUpdateClient {
+    Some(ClientMsg::UpdateClient(MsgUpdateClient {
         client_id: client_id.clone(),
         client_message: src_header.clone().into(),
         signer: dummy_account_id(),
@@ -1615,9 +1606,9 @@ fn client_update_ping_pong() {
         );
 
         assert!(
-                client_msg_b_res.is_ok(),
-                "create_client_update failed for context destination {ctx_b:?}, error: {client_msg_b_res:?}",
-            );
+            client_msg_b_res.is_some(),
+            "create_client_update failed for context destination {ctx_b:?}",
+        );
 
         let client_msg_b = client_msg_b_res.unwrap();
 
@@ -1653,9 +1644,9 @@ fn client_update_ping_pong() {
             build_client_update_datagram(&ctx_a, &client_on_a_for_b, &b_latest_header);
 
         assert!(
-                client_msg_a_res.is_ok(),
-                "create_client_update failed for context destination {ctx_a:?}, error: {client_msg_a_res:?}",
-            );
+            client_msg_a_res.is_some(),
+            "create_client_update failed for context destination {ctx_a:?}",
+        );
 
         let client_msg_a = client_msg_a_res.unwrap();