Skip to content

Commit

Permalink
feat: introduce convenience Status::verify_active method
Browse files Browse the repository at this point in the history
Similarly to ChannelEnd::verify_not_closed, introduce a convenience
method Status::verify_active which checks whether client’s status is
active and returns an error if it isn’t.  This simplifies whole bunch
of code locations making them more DRY.
  • Loading branch information
mina86 committed Dec 12, 2023
1 parent de2c530 commit 0527245
Show file tree
Hide file tree
Showing 19 changed files with 67 additions and 117 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add ibc::core::client::types::Status::verify_active method.
([#1005](https://github.com/cosmos/ibc-rs/pull/1005))
9 changes: 3 additions & 6 deletions ibc-core/ics02-client/src/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,9 @@ where
// Read client state from the host chain store. The client should already exist.
let client_state = ctx.client_state(&client_id)?;

{
let status = client_state.status(ctx.get_client_validation_context(), &client_id)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state
.status(ctx.get_client_validation_context(), &client_id)?
.verify_active()?;

let client_message = msg.client_message();

Expand Down
9 changes: 3 additions & 6 deletions ibc-core/ics02-client/src/handler/upgrade_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ where
let old_client_state = ctx.client_state(&client_id)?;

// Check if the client is active.
{
let status = old_client_state.status(ctx.get_client_validation_context(), &client_id)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
old_client_state
.status(ctx.get_client_validation_context(), &client_id)?
.verify_active()?;

// Read the latest consensus state from the host chain store.
let old_client_cons_state_path = ClientConsensusStatePath::new(
Expand Down
10 changes: 10 additions & 0 deletions ibc-core/ics02-client/types/src/status.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use core::fmt::{Debug, Display, Formatter};

use crate::error::ClientError;

/// `UpdateKind` represents the 2 ways that a client can be updated
/// in IBC: either through a `MsgUpdateClient`, or a `MsgSubmitMisbehaviour`.
#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -38,6 +40,14 @@ impl Status {
pub fn is_expired(&self) -> bool {
*self == Status::Expired
}

/// Checks whether the status is active; returns `Err` if not.
pub fn verify_active(&self) -> Result<(), ClientError> {
match self.clone() {
Self::Active => Ok(()),
status => Err(ClientError::ClientNotActive { status }),

Check failure on line 48 in ibc-core/ics02-client/types/src/status.rs

View workflow job for this annotation

GitHub Actions / test-stable

mismatched types

Check failure on line 48 in ibc-core/ics02-client/types/src/status.rs

View workflow job for this annotation

GitHub Actions / doc_no_default_features

mismatched types

Check failure on line 48 in ibc-core/ics02-client/types/src/status.rs

View workflow job for this annotation

GitHub Actions / doc_all_features

mismatched types
}
}
}

impl Display for Status {
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics03-connection/src/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,9 @@ where
{
let client_state_of_b_on_a = ctx_a.client_state(vars.client_id_on_a())?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), vars.client_id_on_a())?
.verify_active()?;
client_state_of_b_on_a.validate_proof_height(msg.proofs_height_on_b)?;

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics03-connection/src/handler/conn_open_confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,9 @@ where
{
let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?
.verify_active()?;
client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics03-connection/src/handler/conn_open_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,9 @@ where
// An IBC client running on the local (host) chain should exist.
let client_state_of_b_on_a = ctx_a.client_state(&msg.client_id_on_a)?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), &msg.client_id_on_a)?
.verify_active()?;

if let Some(version) = msg.version {
version.verify_is_supported(&ctx_a.get_compatible_versions())?;
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics03-connection/src/handler/conn_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,9 @@ where
{
let client_state_of_a_on_b = ctx_b.client_state(vars.conn_end_on_b.client_id())?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), &msg.client_id_on_b)?
.verify_active()?;
client_state_of_a_on_b.validate_proof_height(msg.proofs_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/acknowledgement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,9 @@ where
let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;
client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_close_confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,9 @@ where
let client_id_on_b = conn_end_on_b.client_id();
let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?
.verify_active()?;
client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_close_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,9 @@ where

let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;
{
let status =
client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;

Ok(())
}
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,9 @@ where
let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;
client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_open_confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,9 @@ where
let client_id_on_b = conn_end_on_b.client_id();
let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?
.verify_active()?;
client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_open_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,9 @@ where
let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status =
client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;

let conn_version = conn_end_on_a.versions();

Expand Down
11 changes: 4 additions & 7 deletions ibc-core/ics04-channel/src/handler/chan_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,10 @@ where
let client_id_on_b = conn_end_on_b.client_id();
let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?
.verify_active()?;

client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
11 changes: 4 additions & 7 deletions ibc-core/ics04-channel/src/handler/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,10 @@ where
let client_id_on_b = conn_end_on_b.client_id();
let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?;

{
let status = client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_a_on_b
.status(ctx_b.get_client_validation_context(), client_id_on_b)?
.verify_active()?;

client_state_of_a_on_b.validate_proof_height(msg.proof_height_on_a)?;

let client_cons_state_path_on_b = ClientConsensusStatePath::new(
Expand Down
10 changes: 3 additions & 7 deletions ibc-core/ics04-channel/src/handler/send_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,9 @@ pub fn send_packet_validate(

let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status =
client_state_of_b_on_a.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;

let latest_height_on_a = client_state_of_b_on_a.latest_height();

Expand Down
11 changes: 4 additions & 7 deletions ibc-core/ics04-channel/src/handler/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,10 @@ where
let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;

client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;

// check that timeout height or timeout timestamp has passed on the other end
Expand Down
11 changes: 4 additions & 7 deletions ibc-core/ics04-channel/src/handler/timeout_on_close.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,10 @@ where
let client_id_on_a = conn_end_on_a.client_id();
let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?;

{
let status = client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?;
if !status.is_active() {
return Err(ClientError::ClientNotActive { status }.into());
}
}
client_state_of_b_on_a
.status(ctx_a.get_client_validation_context(), client_id_on_a)?
.verify_active()?;

client_state_of_b_on_a.validate_proof_height(msg.proof_height_on_b)?;

let client_cons_state_path_on_a = ClientConsensusStatePath::new(
Expand Down

0 comments on commit 0527245

Please sign in to comment.