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

Clean up QueryError and RelayerError types #1321

Merged
merged 6 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 10 additions & 10 deletions ibc-query/src/core/channel/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
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!(

Check warning on line 50 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L50 was not covered by tests
"Proof not found for channel end path {channel_end_path:?}"
))
})?;
Expand Down Expand Up @@ -119,7 +119,7 @@
.first()
.map(|connection_id| ibc_ctx.connection_end(connection_id))
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 122 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L122 was not covered by tests
"Channel {} does not have a connection",
request.channel_id
))
Expand All @@ -140,7 +140,7 @@
&Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 143 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L143 was not covered by tests
"Proof not found for client state path: {:?}",
connection_end.client_id()
))
Expand Down Expand Up @@ -172,7 +172,7 @@
.first()
.map(|connection_id| ibc_ctx.connection_end(connection_id))
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 175 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L175 was not covered by tests
"Channel {} does not have a connection",
request.channel_id
))
Expand All @@ -198,7 +198,7 @@
&Path::ClientConsensusState(consensus_path.clone()),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 201 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L201 was not covered by tests
"Proof not found for client consensus state path: {consensus_path:?}"
))
})?;
Expand Down Expand Up @@ -233,7 +233,7 @@
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!(

Check warning on line 236 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L236 was not covered by tests
"Proof not found for packet commitment path: {commitment_path:?}"
))
})?;
Expand Down Expand Up @@ -291,7 +291,7 @@
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!(

Check warning on line 294 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L294 was not covered by tests
"Proof not found for packet receipt path: {receipt_path:?}"
))
})?;
Expand Down Expand Up @@ -325,7 +325,7 @@
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!(

Check warning on line 328 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L328 was not covered by tests
"Proof not found for packet acknowledgement path: {acknowledgement_path:?}"
))
})?;
Expand Down Expand Up @@ -431,7 +431,7 @@
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!(

Check warning on line 434 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L434 was not covered by tests
"Next sequence send proof not found for channel {}",
request.channel_id
))
Expand Down Expand Up @@ -464,7 +464,7 @@
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!(

Check warning on line 467 in ibc-query/src/core/channel/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L467 was not covered by tests
"Next sequence receive proof not found for channel {}",
request.channel_id
))
Expand Down
10 changes: 5 additions & 5 deletions ibc-query/src/core/client/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
&Path::ClientState(ClientStatePath::new(client_id.clone())),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 53 in ibc-query/src/core/client/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L53 was not covered by tests
"Proof not found for client state path: {client_id:?}"
))
})?;
Expand Down Expand Up @@ -109,7 +109,7 @@
.into_iter()
.max_by_key(|&(h, _)| h)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 112 in ibc-query/src/core/client/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L112 was not covered by tests
"No consensus state found for client: {client_id:?}"
))
})?
Expand All @@ -130,7 +130,7 @@
)),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 133 in ibc-query/src/core/client/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L133 was not covered by tests
"Proof not found for consensus state path: {client_id:?}"
))
})?;
Expand Down Expand Up @@ -242,7 +242,7 @@
&Path::UpgradeClientState(upgraded_client_state_path),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 245 in ibc-query/src/core/client/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L245 was not covered by tests
"Proof not found for upgraded client state at: {proof_height:?}"
))
})?;
Expand Down Expand Up @@ -301,7 +301,7 @@
&Path::UpgradeConsensusState(upgraded_consensus_state_path),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 304 in ibc-query/src/core/client/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L304 was not covered by tests
"Proof not found for upgraded consensus state at: {proof_height:?}"
))
})?;
Expand Down
8 changes: 4 additions & 4 deletions ibc-query/src/core/connection/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
&Path::Connection(ConnectionPath::new(&request.connection_id)),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 44 in ibc-query/src/core/connection/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L44 was not covered by tests
"Proof not found for connection path: {:?}",
request.connection_id
))
Expand Down Expand Up @@ -92,7 +92,7 @@
&Path::ClientConnection(ClientConnectionPath::new(request.client_id.clone())),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 95 in ibc-query/src/core/connection/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L95 was not covered by tests
"Proof not found for client connection path: {:?}",
request.client_id
))
Expand Down Expand Up @@ -130,7 +130,7 @@
&Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())),
)
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 133 in ibc-query/src/core/connection/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L133 was not covered by tests
"Proof not found for client state path: {:?}",
connection_end.client_id()
))
Expand Down Expand Up @@ -172,7 +172,7 @@
let proof = ibc_ctx
.get_proof(proof_height, &Path::ClientConsensusState(consensus_path))
.ok_or_else(|| {
QueryError::proof_not_found(format!(
QueryError::missing_proof(format!(

Check warning on line 175 in ibc-query/src/core/connection/query.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L175 was not covered by tests
"Proof not found for consensus state path: {:?}",
connection_end.client_id()
))
Expand Down
19 changes: 11 additions & 8 deletions ibc-query/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,24 @@
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())

Check warning on line 28 in ibc-query/src/error.rs

View check run for this annotation

Codecov / codecov/patch

ibc-query/src/error.rs#L27-L28

Added lines #L27 - L28 were not covered by tests
}

pub fn missing_field<T: ToString>(description: T) -> Self {
Expand All @@ -35,7 +38,7 @@
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),

Check warning on line 41 in ibc-query/src/error.rs

View check run for this annotation

Codecov / codecov/patch

ibc-query/src/error.rs#L41

Added line #L41 was not covered by tests
QueryError::MissingField(description) => Self::invalid_argument(description),
}
}
Expand Down
7 changes: 3 additions & 4 deletions ibc-testkit/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
Expand Down
38 changes: 0 additions & 38 deletions ibc-testkit/src/relayer/error.rs

This file was deleted.

1 change: 0 additions & 1 deletion ibc-testkit/src/relayer/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub mod context;
pub mod error;
pub mod integration;
pub mod utils;
29 changes: 10 additions & 19 deletions tests-integration/tests/core/ics02_client/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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>,
{
Expand All @@ -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(),
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down
Loading