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

Add HostError contexts to module-level error types #1345

Merged
merged 16 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Revert "Migrate some ics03 handlers to return HostErrors"
This reverts commit f48a1a6.
  • Loading branch information
seanchen1991 committed Sep 17, 2024
commit 52165f75a0ea29e0ef0b22538978115bd48ecad3
48 changes: 21 additions & 27 deletions ibc-core/ics03-connection/src/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

use ibc_core_client::context::prelude::*;
use ibc_core_client::types::error::ClientError;
use ibc_core_connection_types::error::ConnectionError;
use ibc_core_connection_types::events::OpenAck;
use ibc_core_connection_types::msgs::MsgConnectionOpenAck;
use ibc_core_connection_types::{ConnectionEnd, Counterparty, State};
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::identifiers::ClientId;
use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath, Path};
use ibc_core_host::{ExecutionContext, ValidationContext};
Expand All @@ -16,7 +17,7 @@ use ibc_primitives::ToVec;

use crate::handler::{pack_host_consensus_state, unpack_host_client_state};

pub fn validate<Ctx>(ctx_a: &Ctx, msg: MsgConnectionOpenAck) -> Result<(), HostError>
pub fn validate<Ctx>(ctx_a: &Ctx, msg: MsgConnectionOpenAck) -> Result<(), HandlerError>
where
Ctx: ValidationContext,
<Ctx::HostClientState as TryFrom<Any>>::Error: Into<ClientError>,
Expand All @@ -29,7 +30,7 @@ fn validate_impl<Ctx>(
ctx_a: &Ctx,
msg: &MsgConnectionOpenAck,
vars: &LocalVars,
) -> Result<(), HostError>
) -> Result<(), HandlerError>
where
Ctx: ValidationContext,
<Ctx::HostClientState as TryFrom<Any>>::Error: Into<ClientError>,
Expand All @@ -39,10 +40,11 @@ where
let host_height = ctx_a.host_height()?;

if msg.consensus_height_of_a_on_b > host_height {
return Err(HostError::invalid_state(format!(
"insufficient consensus height {}; must be at least {}",
host_height, msg.consensus_height_of_a_on_b
)));
return Err(ConnectionError::InsufficientConsensusHeight {
target_height: msg.consensus_height_of_a_on_b,
current_height: host_height,
}
.into());
}

let client_val_ctx_a = ctx_a.get_client_validation_context();
Expand All @@ -55,26 +57,19 @@ where
ctx_a.validate_self_client(client_state_of_a_on_b)?;

msg.version
.verify_is_supported(vars.conn_end_on_a.versions())
.map_err(HostError::failed_to_verify)?;
.verify_is_supported(vars.conn_end_on_a.versions())?;

vars.conn_end_on_a
.verify_state_matches(&State::Init)
.map_err(HostError::failed_to_verify)?;
vars.conn_end_on_a.verify_state_matches(&State::Init)?;

// Proof verification.
{
let client_state_of_b_on_a = client_val_ctx_a.client_state(vars.client_id_on_a())?;

client_state_of_b_on_a
.status(client_val_ctx_a, vars.client_id_on_a())
.map_err(|e| HostError::invalid_state(format!("failed to fetch client status: {e}")))?
.verify_is_active()
.map_err(HostError::failed_to_verify)?;
.status(client_val_ctx_a, vars.client_id_on_a())?
.verify_is_active()?;

client_state_of_b_on_a
.validate_proof_height(msg.proofs_height_on_b)
.map_err(HostError::failed_to_verify)?;
client_state_of_b_on_a.validate_proof_height(msg.proofs_height_on_b)?;

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
vars.client_id_on_a().clone(),
Expand All @@ -99,8 +94,7 @@ where
),
vec![msg.version.clone()],
vars.conn_end_on_a.delay_period(),
)
.map_err(HostError::invalid_state)?;
)?;

client_state_of_b_on_a
.verify_membership(
Expand All @@ -110,7 +104,7 @@ where
Path::Connection(ConnectionPath::new(&msg.conn_id_on_b)),
expected_conn_end_on_b.encode_vec(),
)
.map_err(HostError::failed_to_verify)?;
.map_err(ConnectionError::FailedToVerifyClient)?;
}

client_state_of_b_on_a
Expand All @@ -121,7 +115,7 @@ where
Path::ClientState(ClientStatePath::new(vars.client_id_on_b().clone())),
msg.client_state_of_a_on_b.to_vec(),
)
.map_err(HostError::failed_to_verify)?;
.map_err(ConnectionError::FailedToVerifyClient)?;

let expected_consensus_state_of_a_on_b =
ctx_a.host_consensus_state(&msg.consensus_height_of_a_on_b)?;
Expand All @@ -143,13 +137,13 @@ where
Path::ClientConsensusState(client_cons_state_path_on_b),
stored_consensus_state_of_a_on_b.to_vec(),
)
.map_err(HostError::failed_to_verify)?;
.map_err(ConnectionError::FailedToVerifyClient)?;
}

Ok(())
}

pub fn execute<Ctx>(ctx_a: &mut Ctx, msg: MsgConnectionOpenAck) -> Result<(), HostError>
pub fn execute<Ctx>(ctx_a: &mut Ctx, msg: MsgConnectionOpenAck) -> Result<(), HandlerError>
where
Ctx: ExecutionContext,
{
Expand All @@ -161,7 +155,7 @@ fn execute_impl<Ctx>(
ctx_a: &mut Ctx,
msg: MsgConnectionOpenAck,
vars: LocalVars,
) -> Result<(), HostError>
) -> Result<(), HandlerError>
where
Ctx: ExecutionContext,
{
Expand Down Expand Up @@ -199,7 +193,7 @@ struct LocalVars {
}

impl LocalVars {
fn new<Ctx>(ctx_a: &Ctx, msg: &MsgConnectionOpenAck) -> Result<Self, HostError>
fn new<Ctx>(ctx_a: &Ctx, msg: &MsgConnectionOpenAck) -> Result<Self, HandlerError>
where
Ctx: ValidationContext,
{
Expand Down
29 changes: 15 additions & 14 deletions ibc-core/ics03-connection/src/handler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use ibc_core_client::types::error::ClientError;
use ibc_core_handler_types::error::HandlerError;
#[cfg(feature = "wasm-client")]
use ibc_core_host::types::{error::HostError, identifiers::ClientId};
use ibc_primitives::prelude::*;
use ibc_core_host::types::error::DecodingError;
use ibc_core_host::types::identifiers::ClientId;
use ibc_primitives::proto::Any;

pub mod conn_open_ack;
Expand All @@ -17,37 +18,37 @@ pub mod conn_open_try;
pub(crate) fn unpack_host_client_state<CS>(
value: Any,
host_client_id_at_counterparty: &ClientId,
) -> Result<CS, HostError>
) -> Result<CS, HandlerError>
where
CS: TryFrom<Any>,
<CS as TryFrom<Any>>::Error: Into<ClientError>,
{
#[cfg(feature = "wasm-client")]
if host_client_id_at_counterparty.is_wasm_client_id() {
use ibc_client_wasm_types::client_state::ClientState as WasmClientState;
use ibc_core_connection_types::error::ConnectionError;
use ibc_primitives::prelude::ToString;
use prost::Message;

let wasm_client_state =
WasmClientState::try_from(value).map_err(HostError::invalid_state)?;
let wasm_client_state = WasmClientState::try_from(value).map_err(|e| {
HandlerError::Connection(ConnectionError::InvalidClientState {
description: e.to_string(),
})
})?;

let any_client_state = <Any as Message>::decode(wasm_client_state.data.as_slice())
.map_err(|e| {
HostError::invalid_state(format!("failed to decode wasm client state: {e}"))
})?;
.map_err(|e| ConnectionError::Decoding(DecodingError::Prost(e)))?;

Ok(CS::try_from(any_client_state)
.map_err(|_| HostError::invalid_state("failed to unpack wasm host client state"))?)
Ok(CS::try_from(any_client_state).map_err(Into::<ClientError>::into)?)
} else {
Ok(CS::try_from(value)
.map_err(|_| HostError::invalid_state("failed to unpack host client state"))?)
Ok(CS::try_from(value).map_err(Into::<ClientError>::into)?)
}

#[cfg(not(feature = "wasm-client"))]
{
// this avoids lint warning for unused variable.
let _ = host_client_id_at_counterparty;
Ok(CS::try_from(value)
.map_err(|_| HostError::invalid_state("failed to unpack host client state"))?)
Ok(CS::try_from(value).map_err(Into::<ClientError>::into)?)
}
}

Expand Down
8 changes: 0 additions & 8 deletions ibc-core/ics24-host/types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ pub enum HostError {
FailedToStore { description: String },
/// failed to retrieve from store: `{description}`
FailedToRetrieve { description: String },
/// failed to verify state: `{description}`
FailedToVerify { description: String },
/// other error: `{description}`
Other { description: String },
}
Expand Down Expand Up @@ -50,12 +48,6 @@ impl HostError {
description: description.to_string(),
}
}

pub fn failed_to_verify<T: ToString>(description: T) -> Self {
Self::FailedToVerify {
description: description.to_string(),
}
}
}

/// Errors that arise when parsing identifiers.
Expand Down
2 changes: 1 addition & 1 deletion ibc-core/ics25-handler/src/entrypoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ where
MsgEnvelope::Connection(msg) => match msg {
ConnectionMsg::OpenInit(msg) => conn_open_init::validate(ctx, msg),
ConnectionMsg::OpenTry(msg) => conn_open_try::validate(ctx, msg),
ConnectionMsg::OpenAck(msg) => Ok(conn_open_ack::validate(ctx, msg)?),
ConnectionMsg::OpenAck(msg) => conn_open_ack::validate(ctx, msg),
ConnectionMsg::OpenConfirm(msg) => conn_open_confirm::validate(ctx, &msg),
},
MsgEnvelope::Channel(msg) => {
Expand Down