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 ibc-core and ibc-clients error types #1317

Merged
merged 50 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
255fa71
Clean up pass of ClientError type
seanchen1991 Aug 15, 2024
49f3892
Clean up pass of ics24 errors
seanchen1991 Aug 15, 2024
61fa8c8
Clean up pass of ics23 errors
seanchen1991 Aug 15, 2024
7ba65d6
Clean up pass of ics07 errors
seanchen1991 Aug 16, 2024
198844e
Clean up ChannelError type
seanchen1991 Aug 16, 2024
629737e
Clean up PacketError type
seanchen1991 Aug 16, 2024
eadf96d
Better organize error variants
seanchen1991 Aug 16, 2024
19a78ce
cargo nightly fmt
seanchen1991 Aug 19, 2024
e86f6e8
Update error variant in integration tests
seanchen1991 Aug 19, 2024
4ed807e
Fix ics23 tests
seanchen1991 Aug 19, 2024
83b8ed8
Fix typo in doc comment
seanchen1991 Aug 19, 2024
a6e6283
Update cw-check Cargo.lock
seanchen1991 Aug 19, 2024
686f612
Clean up ics08 error
seanchen1991 Aug 19, 2024
e6aaed3
Cargo nightly fmt
seanchen1991 Aug 19, 2024
ffff554
Change single-field error variants to tuple structs
seanchen1991 Aug 19, 2024
1b80141
Propogate error variant changes
seanchen1991 Aug 19, 2024
359d0bc
Propogate error variant changes
seanchen1991 Aug 19, 2024
c183e83
Clean up PacketError type
seanchen1991 Aug 19, 2024
5af48fa
Clean up IdentifierError type
seanchen1991 Aug 19, 2024
90d4c6c
Clean up ics26 RouterError
seanchen1991 Aug 20, 2024
4298f9b
Clean up ics03 ConnectionError
seanchen1991 Aug 20, 2024
2c35bce
Fix error variant naming
seanchen1991 Aug 20, 2024
efd61bd
Fix error variant naming
seanchen1991 Aug 20, 2024
8edfca9
Remove redundant Other error variant
seanchen1991 Aug 20, 2024
3780fd9
Clarify tendermint client error naming
seanchen1991 Aug 21, 2024
1c4ca7f
Rename tendermint client Error to TendermintClientError
seanchen1991 Aug 21, 2024
0b20606
Rename wasm light client Error to WasmClientError
seanchen1991 Aug 21, 2024
a5bf42c
Rename ics25-handler Error to HandlerError
seanchen1991 Aug 21, 2024
b4d0854
Remove unused HandlerError
seanchen1991 Aug 21, 2024
cef3082
Remove TendermintClientError::InvalidHeader error for more specific v…
seanchen1991 Aug 21, 2024
c6100e2
Rename LightClientVerifierError variant to FailedToVerifyHeader
seanchen1991 Aug 21, 2024
5d019df
Rename ConsensusStateTimestampGteTrustingPeriod to InsufficientTrusti…
seanchen1991 Aug 21, 2024
c0655fa
Remove redundant ChannelError::FailedToParseSequence variant
seanchen1991 Aug 21, 2024
56a82ea
Rename ChannelError::FailedChannelVerification to FailedProofVerifica…
seanchen1991 Aug 21, 2024
8e1c2c5
Replace incorrect usages of FailedPacketVerification
seanchen1991 Aug 21, 2024
2c9b636
Define From<CommitmentError> for ClientError impl
seanchen1991 Aug 21, 2024
2a3a374
Add IdentifierError::FailedToParse variant
seanchen1991 Aug 21, 2024
364fd11
Replace InvalidPrefix error usage
seanchen1991 Aug 21, 2024
f0db399
Fix doc comment typo
seanchen1991 Aug 21, 2024
65ecf8c
Improve ChannelError variant names
seanchen1991 Aug 22, 2024
4776d70
Add PacketError::InvalidPathPrefix variant
seanchen1991 Aug 22, 2024
123710e
Remove unnecessary PacketError variant
seanchen1991 Aug 22, 2024
d2799c9
Add error variants used by basecoin
seanchen1991 Aug 22, 2024
474909d
Merge branch 'error-handling' of https://github.com/cosmos/ibc-rs int…
seanchen1991 Aug 22, 2024
7e41b3d
nit: remove unnecessary map_err in verify_(non_)membership
Farhad-Shabani Aug 22, 2024
c74af19
imp: define EmptyCommitmentRoot
Farhad-Shabani Aug 22, 2024
f8e70f1
nit: remove MissingHostHeight
Farhad-Shabani Aug 22, 2024
115969c
fix: remove unnecessary map_err(PacketError::Channel)
Farhad-Shabani Aug 22, 2024
c4ffcd2
Remove a TODO
seanchen1991 Aug 23, 2024
86b44f3
Merge branch 'sean/clean-up-error-variants' of https://github.com/cos…
seanchen1991 Aug 23, 2024
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
1 change: 1 addition & 0 deletions ci/cw-check/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ibc-clients/ics07-tendermint/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! Rust). As such, this module also includes some trait implementations that
//! serve to pass through traits implemented on the wrapped `ClientState` type.

use ibc_client_tendermint_types::error::Error;
use ibc_client_tendermint_types::error::TendermintClientError;
use ibc_client_tendermint_types::proto::v1::ClientState as RawTmClientState;
use ibc_client_tendermint_types::ClientState as ClientStateType;
use ibc_core_client::types::error::ClientError;
Expand Down Expand Up @@ -44,7 +44,7 @@ impl ClientState {
impl Protobuf<RawTmClientState> for ClientState {}

impl TryFrom<RawTmClientState> for ClientState {
type Error = Error;
type Error = TendermintClientError;

fn try_from(raw: RawTmClientState) -> Result<Self, Self::Error> {
Ok(Self(ClientStateType::try_from(raw)?))
Expand Down
24 changes: 11 additions & 13 deletions ibc-clients/ics07-tendermint/src/client_state/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
let (upgrade_path_prefix, upgrade_path) = match upgrade_path.len() {
0 => {
return Err(UpgradeClientError::InvalidUpgradePath {
reason: "no upgrade path has been set".to_string(),
description: "no upgrade path has been set".to_string(),

Check warning on line 70 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L70

Added line #L70 was not covered by tests
}
.into());
}
Expand All @@ -78,7 +78,7 @@
),
_ => {
return Err(UpgradeClientError::InvalidUpgradePath {
reason: "upgrade path is too long".to_string(),
description: "upgrade path is too long".to_string(),

Check warning on line 81 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L81

Added line #L81 was not covered by tests
}
.into());
}
Expand Down Expand Up @@ -168,9 +168,7 @@
};

if consensus_state_status(&tm_consensus_state, host_timestamp, trusting_period)?.is_expired() {
return Err(ClientError::ClientNotActive {
status: Status::Expired,
});
return Err(ClientError::InvalidStatus(Status::Expired));
}

Ok(())
Expand Down Expand Up @@ -214,8 +212,8 @@

if latest_height < proof_height {
return Err(ClientError::InvalidProofHeight {
latest_height,
proof_height,
actual: latest_height,
expected: proof_height,
});
}

Expand Down Expand Up @@ -258,7 +256,7 @@
// the upgrade height This condition checks both the revision number and
// the height
if latest_height >= upgraded_tm_client_state_height {
Err(UpgradeClientError::LowUpgradeHeight {
Err(UpgradeClientError::InsufficientUpgradeHeight {

Check warning on line 259 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L259

Added line #L259 was not covered by tests
upgraded_height: upgraded_tm_client_state_height,
client_height: latest_height,
})?
Expand Down Expand Up @@ -301,17 +299,17 @@
value: Vec<u8>,
) -> Result<(), ClientError> {
if prefix.is_empty() {
return Err(ClientError::Ics23Verification(
return Err(ClientError::FailedICS23Verification(

Check warning on line 302 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L302

Added line #L302 was not covered by tests
CommitmentError::EmptyCommitmentPrefix,
));
}

let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]);
let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;
let merkle_proof = MerkleProof::try_from(proof)?;

merkle_proof
.verify_membership::<H>(proof_specs, root.clone().into(), merkle_path, value, 0)
.map_err(ClientError::Ics23Verification)
.map_err(ClientError::FailedICS23Verification)
}

/// Verify that the given value does not belong in the client's merkle proof.
Expand All @@ -327,9 +325,9 @@
path: PathBytes,
) -> Result<(), ClientError> {
let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]);
let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;
let merkle_proof = MerkleProof::try_from(proof)?;

merkle_proof
.verify_non_membership::<H>(proof_specs, root.clone().into(), merkle_path)
.map_err(ClientError::Ics23Verification)
.map_err(ClientError::FailedICS23Verification)
}
9 changes: 3 additions & 6 deletions ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ibc_client_tendermint_types::error::{Error, IntoResult};
use ibc_client_tendermint_types::error::{IntoResult, TendermintClientError};
use ibc_client_tendermint_types::{
ConsensusState as ConsensusStateType, Header as TmHeader, Misbehaviour as TmMisbehaviour,
};
Expand Down Expand Up @@ -101,14 +101,11 @@

let duration_since_consensus_state =
current_timestamp.duration_since(&trusted_timestamp).ok_or(
ClientError::InvalidConsensusStateTimestamp {
time1: trusted_timestamp,
time2: current_timestamp,
},
ClientError::InvalidConsensusStateTimestamp(trusted_timestamp),
)?;

if duration_since_consensus_state >= options.trusting_period {
return Err(Error::ConsensusStateTimestampGteTrustingPeriod {
return Err(TendermintClientError::InsufficientTrustingPeriod {

Check warning on line 108 in ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs#L108

Added line #L108 was not covered by tests
duration_since_consensus_state,
trusting_period: options.trusting_period,
}
Expand Down
10 changes: 5 additions & 5 deletions ibc-clients/ics07-tendermint/src/client_state/update_client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ibc_client_tendermint_types::error::{Error, IntoResult};
use ibc_client_tendermint_types::error::{IntoResult, TendermintClientError};
use ibc_client_tendermint_types::{ConsensusState as ConsensusStateType, Header as TmHeader};
use ibc_core_client::context::{Convertible, ExtClientValidationContext};
use ibc_core_client::types::error::ClientError;
Expand Down Expand Up @@ -63,10 +63,10 @@
.trusted_height
.revision_height()
.try_into()
.map_err(|_| ClientError::ClientSpecific {
description: Error::InvalidHeaderHeight {
height: header.trusted_height.revision_height(),
}
.map_err(|_| ClientError::FailedHeaderVerification {
description: TendermintClientError::InvalidHeaderHeight(
header.trusted_height.revision_height(),
)

Check warning on line 69 in ibc-clients/ics07-tendermint/src/client_state/update_client.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/update_client.rs#L67-L69

Added lines #L67 - L69 were not covered by tests
.to_string(),
})?,
next_validators: &header.trusted_next_validator_set,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,5 +278,5 @@
&& subject_proof_specs == &substitute_proof_specs
&& subject_upgrade_path == &substitute_upgrade_path)
.then_some(())
.ok_or(ClientError::ClientRecoveryStateMismatch)
.ok_or(ClientError::MismatchedClientRecoveryStates)

Check warning on line 281 in ibc-clients/ics07-tendermint/src/client_state/validation.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/validation.rs#L281

Added line #L281 was not covered by tests
}
4 changes: 2 additions & 2 deletions ibc-clients/ics07-tendermint/src/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! implementations that serve to pass through traits implemented on the wrapped
//! `ConsensusState` type.

use ibc_client_tendermint_types::error::Error;
use ibc_client_tendermint_types::error::TendermintClientError;
use ibc_client_tendermint_types::proto::v1::ConsensusState as RawTmConsensusState;
use ibc_client_tendermint_types::ConsensusState as ConsensusStateType;
use ibc_core_client::context::consensus_state::ConsensusState as ConsensusStateTrait;
Expand Down Expand Up @@ -52,7 +52,7 @@ impl From<ConsensusState> for ConsensusStateType {
impl Protobuf<RawTmConsensusState> for ConsensusState {}

impl TryFrom<RawTmConsensusState> for ConsensusState {
type Error = Error;
type Error = TendermintClientError;

fn try_from(raw: RawTmConsensusState) -> Result<Self, Self::Error> {
Ok(Self(ConsensusStateType::try_from(raw)?))
Expand Down
Loading
Loading