Skip to content

Commit

Permalink
Migrate some ics03 handlers to return HostErrors
Browse files Browse the repository at this point in the history
  • Loading branch information
seanchen1991 committed Sep 17, 2024
1 parent 800f4b0 commit f48a1a6
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 37 deletions.
48 changes: 27 additions & 21 deletions ibc-core/ics03-connection/src/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@

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

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

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

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

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

// 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())?
.verify_is_active()?;
.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)?;

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

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
vars.client_id_on_a().clone(),
Expand All @@ -94,7 +99,8 @@ 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 @@ -104,7 +110,7 @@ where
Path::Connection(ConnectionPath::new(&msg.conn_id_on_b)),
expected_conn_end_on_b.encode_vec(),
)
.map_err(ConnectionError::FailedToVerifyClient)?;
.map_err(HostError::failed_to_verify)?;
}

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

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

Ok(())
}

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

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

pub mod conn_open_ack;
Expand All @@ -18,37 +17,37 @@ pub mod conn_open_try;
pub(crate) fn unpack_host_client_state<CS>(
value: Any,
host_client_id_at_counterparty: &ClientId,

Check failure on line 19 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std panic conflict

cannot find type `ClientId` in this scope

Check failure on line 19 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check Features

cannot find type `ClientId` in this scope

Check failure on line 19 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std substrate support

cannot find type `ClientId` in this scope
) -> Result<CS, HandlerError>
) -> Result<CS, HostError>

Check failure on line 20 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std panic conflict

cannot find type `HostError` in this scope

Check failure on line 20 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check Features

cannot find type `HostError` in this scope

Check failure on line 20 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std substrate support

cannot find type `HostError` in this scope
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(|e| {
HandlerError::Connection(ConnectionError::InvalidClientState {
description: e.to_string(),
})
})?;
let wasm_client_state =
WasmClientState::try_from(value).map_err(HostError::invalid_state)?;

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

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

#[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(Into::<ClientError>::into)?)
Ok(CS::try_from(value)
.map_err(|_| HostError::invalid_state("failed to unpack host client state"))?)

Check failure on line 50 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std panic conflict

failed to resolve: use of undeclared type `HostError`

Check failure on line 50 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check Features

failed to resolve: use of undeclared type `HostError`

Check failure on line 50 in ibc-core/ics03-connection/src/handler/mod.rs

View workflow job for this annotation

GitHub Actions / Check no_std substrate support

failed to resolve: use of undeclared type `HostError`
}
}

Expand Down
8 changes: 8 additions & 0 deletions ibc-core/ics24-host/types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ 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 @@ -48,6 +50,12 @@ 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) => conn_open_ack::validate(ctx, msg),
ConnectionMsg::OpenAck(msg) => Ok(conn_open_ack::validate(ctx, msg)?),
ConnectionMsg::OpenConfirm(msg) => conn_open_confirm::validate(ctx, &msg),
},
MsgEnvelope::Channel(msg) => {
Expand Down

0 comments on commit f48a1a6

Please sign in to comment.