Clean up ibc-core and ibc-clients error types
#1317
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanchen1991
changed the title
ibc-core error typesibc-core error types
seanchen1991
changed the title
ibc-core error typesibc-core and ibc-clients error types
Farhad-Shabani
requested changes
Aug 20, 2024
…o sean/clean-up-error-variants
Farhad-Shabani
approved these changes
Aug 23, 2024
There was a problem hiding this comment.
This PR itself brought some much-needed improvements to our error handling. Thank you, @seanchen1991. Now it's easier to explore different design approaches to address the remaining concerns.
…mos/ibc-rs into sean/clean-up-error-variants
Farhad-Shabani
added a commit
that referenced
this pull request
Sep 9, 2024
* Clean up pass of ClientError type * Clean up pass of ics24 errors * Clean up pass of ics23 errors * Clean up pass of ics07 errors * Clean up ChannelError type * Clean up PacketError type * Better organize error variants * cargo nightly fmt * Update error variant in integration tests * Fix ics23 tests * Fix typo in doc comment * Update cw-check Cargo.lock * Clean up ics08 error * Cargo nightly fmt * Change single-field error variants to tuple structs * Propogate error variant changes * Propogate error variant changes * Clean up PacketError type * Clean up IdentifierError type * Clean up ics26 RouterError * Clean up ics03 ConnectionError * Fix error variant naming * Fix error variant naming * Remove redundant Other error variant * Clarify tendermint client error naming * Rename tendermint client Error to TendermintClientError * Rename wasm light client Error to WasmClientError * Rename ics25-handler Error to HandlerError * Remove unused HandlerError * Remove TendermintClientError::InvalidHeader error for more specific variant * Rename LightClientVerifierError variant to FailedToVerifyHeader * Rename ConsensusStateTimestampGteTrustingPeriod to InsufficientTrustingPeriod * Remove redundant ChannelError::FailedToParseSequence variant * Rename ChannelError::FailedChannelVerification to FailedProofVerification * Replace incorrect usages of FailedPacketVerification * Define From<CommitmentError> for ClientError impl * Add IdentifierError::FailedToParse variant * Replace InvalidPrefix error usage * Fix doc comment typo * Improve ChannelError variant names * Add PacketError::InvalidPathPrefix variant * Remove unnecessary PacketError variant * Add error variants used by basecoin * nit: remove unnecessary map_err in verify_(non_)membership * imp: define EmptyCommitmentRoot * nit: remove MissingHostHeight * fix: remove unnecessary map_err(PacketError::Channel) * Remove a TODO --------- Co-authored-by: Farhad Shabani <[email protected]>
15 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
* Clean up `ibc-apps` errors (#1318) * Clean up ics20-transfer error * Clean up NftTransfer error * Incorporate PR feedback * Remove redundant TODO comment * Simplify NftTransferError variant * Add TODO reminder * Clean up `ibc-core` and `ibc-clients` error types (#1317) * Clean up pass of ClientError type * Clean up pass of ics24 errors * Clean up pass of ics23 errors * Clean up pass of ics07 errors * Clean up ChannelError type * Clean up PacketError type * Better organize error variants * cargo nightly fmt * Update error variant in integration tests * Fix ics23 tests * Fix typo in doc comment * Update cw-check Cargo.lock * Clean up ics08 error * Cargo nightly fmt * Change single-field error variants to tuple structs * Propogate error variant changes * Propogate error variant changes * Clean up PacketError type * Clean up IdentifierError type * Clean up ics26 RouterError * Clean up ics03 ConnectionError * Fix error variant naming * Fix error variant naming * Remove redundant Other error variant * Clarify tendermint client error naming * Rename tendermint client Error to TendermintClientError * Rename wasm light client Error to WasmClientError * Rename ics25-handler Error to HandlerError * Remove unused HandlerError * Remove TendermintClientError::InvalidHeader error for more specific variant * Rename LightClientVerifierError variant to FailedToVerifyHeader * Rename ConsensusStateTimestampGteTrustingPeriod to InsufficientTrustingPeriod * Remove redundant ChannelError::FailedToParseSequence variant * Rename ChannelError::FailedChannelVerification to FailedProofVerification * Replace incorrect usages of FailedPacketVerification * Define From<CommitmentError> for ClientError impl * Add IdentifierError::FailedToParse variant * Replace InvalidPrefix error usage * Fix doc comment typo * Improve ChannelError variant names * Add PacketError::InvalidPathPrefix variant * Remove unnecessary PacketError variant * Add error variants used by basecoin * nit: remove unnecessary map_err in verify_(non_)membership * imp: define EmptyCommitmentRoot * nit: remove MissingHostHeight * fix: remove unnecessary map_err(PacketError::Channel) * Remove a TODO --------- Co-authored-by: Farhad Shabani <[email protected]> * Clean up `QueryError` and `RelayerError` types (#1321) * Clean up ibc-query QueryError type * Clean up ibc-testkit RelayerError * Clean up ibc-testkit RelayerError * Change wording of a doc comment * Remove RelayerError * Consolidate decoding-related errors into new `DecodingError` type (#1325) * Define DecodingError in ibc-primitives * Utilize DecodingError in ics721 * Start using DecodingError in ClientError * Define StatusError type * Wire up StatusError * Wiring up DecodingError * Add DecodingError to RouterError * Remove unused From impl * Change format of an import * Change format of an import * Change format of an import * Move DecodingError into ics24 * Cargo nightly fmt * Use DecodingError in more places * cargo fmt * cargo fmt * Update cw-check Cargo.lock * Add necessary wasm-client feature attribute * Move InvalidUri error variant back to NftTransferError * Regenerate cw hcheck cargo.lock * Add serde feature attribute * Remove ClientError::InvalidClientIdentifier error variant in favor of DecodingError::InvalidIdentifier * Add derive_more::From on NftTransferError * Stashing changes * Revert "Add derive_more::From on NftTransferError" This reverts commit 234ebee. * Remove RouterError::UnknownMessageTypeUrl * Add derive_more::From on TokenTransferError * Add derive_more to NftTransferError * Remove tendermint-proto dependency from ibc-primitives * Remove StatusError * Remove unnecessary ClientError::Decoding wrappings * Clean up TendermintClientError * Regenerate cw-check cargo.lock * taplo fmt * Apply PR feedback * Use ibc_proto::Error type instead of tendermint_proto::Error * Change FailedToParseType fields to make them more clear * Revert InvalidTrace and InvalidCoin TokenTransferError variants * Remove NftTransferError variants in favor of DecodingError::InvalidJson * cargo fmt * Regenerate cw-check cargo.lock * Separate out error variants that need to be moved to host-relevant errors * Consolidate TendermintClientError decoding errors * Consolidate Connection and Packet decoding errors * Remove WasmClientError * Consolidate Identifier variant naming * Remove HostError annotations in doc comments * Consolidate CommitmentError variants * Consolidate ConnectionError variants * Revert some CommitmentError variants * Change TryFrom<Any> for MsgEnvelope Error type to DecodingError * Remove ConnectionError::Identifier variant in favor of DecodingError::Identifier * Remove ChannelError::Identifier variant in favor of DecodingError::Identifier * Remove PacketError::Identifier variant in favor of DecodingError::Identifier * Revert TokenTransferError FailedToDeserializeAck and FailedToDeserializePacketData * Revert NftTransferError FailedToDeserializeAck and FailedToDeserializePacketData * Revert ics20 and ics721 on_recv_packet_execute impls * Remove additional Identifier error variants * Add TendermintClientError::InsufficientMisbehaviourHeaderHeight variant * Implement From<IdentifierError> for ClientError and ConnectionError * Incorporate PR feedback * Change TryFrom<RawConsensusState> for ConsensusState error to DecodingError * Remove RouterError::Decoding variant * Revert AcknowledgementStatus tests * Cargo fmt * Fix test_ack_de * Fix typo in doc comment * Fix test_ack_de * Fix test_ack_de * fix: revert nft721 on module errors * fix: remove Identifier variant from few more enums * fix: remove unused tendermint-proto dep * chore: update Cargo.lock --------- Co-authored-by: Farhad Shabani <[email protected]> * imp(ibc-apps): migrate to `DecodingError` in `TryFrom` conversions (#1335) * imp: migrate to DecodingError in TryFrom conversions, ibc-apps * imp: apply on TryFrom<RawPacketData> * fix: missing import * fix: apply review comments * imp: move away from From<Infallible> * Define `HostError` type (#1331) * Define DecodingError in ibc-primitives * Utilize DecodingError in ics721 * Start using DecodingError in ClientError * Define StatusError type * Wire up StatusError * Wiring up DecodingError * Add DecodingError to RouterError * Remove unused From impl * Change format of an import * Change format of an import * Change format of an import * Move DecodingError into ics24 * Cargo nightly fmt * Use DecodingError in more places * cargo fmt * cargo fmt * Update cw-check Cargo.lock * Add necessary wasm-client feature attribute * Move InvalidUri error variant back to NftTransferError * Regenerate cw hcheck cargo.lock * Add serde feature attribute * Remove ClientError::InvalidClientIdentifier error variant in favor of DecodingError::InvalidIdentifier * Add derive_more::From on NftTransferError * Stashing changes * Revert "Add derive_more::From on NftTransferError" This reverts commit ea9c83d. * Remove RouterError::UnknownMessageTypeUrl * Add derive_more::From on TokenTransferError * Add derive_more to NftTransferError * Remove tendermint-proto dependency from ibc-primitives * Remove StatusError * Remove unnecessary ClientError::Decoding wrappings * Clean up TendermintClientError * Regenerate cw-check cargo.lock * taplo fmt * Apply PR feedback * Use ibc_proto::Error type instead of tendermint_proto::Error * Change FailedToParseType fields to make them more clear * Revert InvalidTrace and InvalidCoin TokenTransferError variants * Remove NftTransferError variants in favor of DecodingError::InvalidJson * cargo fmt * Regenerate cw-check cargo.lock * Separate out error variants that need to be moved to host-relevant errors * Consolidate TendermintClientError decoding errors * Consolidate Connection and Packet decoding errors * Remove WasmClientError * Consolidate Identifier variant naming * Remove HostError annotations in doc comments * Consolidate CommitmentError variants * Consolidate ConnectionError variants * Revert some CommitmentError variants * Change TryFrom<Any> for MsgEnvelope Error type to DecodingError * Define HostError type * Rename ContextError to HandlerError * Change `pick_version` impl * Update ValidationContext trait methods to return HostError type * Rename HandlerError variants throughout the code base * Remove match arm that is no longer relevant * Change ClientValidationContext trait methods to return HostErrors * Change TokenTransferValidationContext and NftTransferValidationContext trait error types * Updating validation contexts to return HostError types * Map errors to HostError variants * Update ValidateSelfClientContext trait * Update Upgrade validation and execution contexts to return HostErrors * Fix merge conflicts * Fix merge conflicts * Revert execute_upgrade_client_proposal error type to UpgradeClientError * cargo fmt * Remove unused alloc::string::ToString * Import ibc_core::primitives::prelude into handler/mod.rs * cargo fmt * Attempt to fix failing testkit test * Remove RouterError::UnknownPort variant * Remove some uses of ClientError::Other variants * Fix recv_packet_validate_happy_path test * Fix conn_open_ack_no_connection test * cargo fmt * fix conn_open_ack_no_connection test without adding new HostError variant * fix test_create_client_try_from test * Remove unnecessary import * Fix test_create_client_try_from * cargo fmt * fix: remove now unneeded with_packet_receipt * fix: revert unintended ack status changes in ICS-20 * fix: revert unintended ack status changes in ICS-721 * fix: remove unintended tendermint-proto * fix: update Cargo.lock for cw-check * Fix lingering issues from merge conflicts * Fix lingering issues from merge conflicts * Remove From<Infallible> for ClientError impl * cargo fmt * Rename HostError InvalidData and MissingData variants * Define helper functions for HostError * Use newly-define HostError helpers * Remove HostError::AppModule variant * Prune some HostError variants * Consolidate some HostError variants * Remove HostError::NonexistentType variant * Remove HostError::UnexpectedState variant * Consolidate HostError::UnknownResource variant * cargo fmt * Remove unnecessary to_string calls * Revert `pick_version` method's documentation * Remove some more HostError variants * Remove From<DecodingError> for StatusValue impl * Fix AcknowledgementStatus tests * fix: revert errors in ICS721 callbacks * Change references to HandlerError in docs to refer to HostError where appropriate --------- Signed-off-by: Sean Chen <[email protected]> Co-authored-by: Farhad Shabani <[email protected]> * Consolidate `PacketError` and `ChannelError` (#1343) * Merge PacketError variants into ChannelError * cargo nightly fmt * Consolidate ChanelError verification variants * Convert ics04 try_from impls to use DecodingError * Add changelog entry * Incorporate some PR feedback * Remove ChannelError::MissingProof and MissingProofHeight error variants * Add `HostError` contexts to module-level error types (#1345) * Add ConnectionError::Host variant and clean up some other variants * Migrate some ics03 handlers to return HostErrors * Revert "Migrate some ics03 handlers to return HostErrors" This reverts commit f48a1a6. * Add HostError variant to ChannelError * Remove HandlerError::Host variant * Map client handler HostErrors to ClientError::Host * Revert "Map client handler HostErrors to ClientError::Host" This reverts commit 247be8a. * Change ics02-client handlers to return ClientError * Change ics03-connection handlers to return ConnectionError * Change ics04-channel handlers to return ChannelError * Make necessary changes in ics25 dispatch * imp: misc improvements and fixes * imp: use derive_more::From for ChannelError * fix: rename InvalidClientState under ConnectionError * nit: remove redundant import --------- Co-authored-by: Farhad Shabani <[email protected]> * Clean up generic String variants in error types (#1347) * Clean up most instances of ClientError::Other * Rename CommitmentError variants * Clean up some DecodingError calls * Incorporate some PR feedback * Incorporate more PR feedback * A bunch of DecodingError associated type changes * Eliminate a bunch of redundant map_err calls * cargo fmt * Fix ics23 spec tests * Fix a typo * Clean up some error messages * Consolidate MismatchedTypeUrl and MismatchedEventKind variants * Miscellaneous clean up * cargo fmt * Remove unused CommitmentError variants * Clean up errors to match ADR description * More clean up * Add changelog entry * fix: remove now unused HeightError * Clean up TimestampError --------- Co-authored-by: Farhad Shabani <[email protected]> * nitpicks * fix: remove ibc-core-connection-types dep under ICS-04 * fix: propogate errors in refund_packet_nft_execute * fix: revert few errors in ICS-20 module callbacks * imp: use if let for type_url matches * nit: minor improvements * fix: use RouterError in case of ModuleID not found * chore: add changelog * chore: add changelog for #1319 --------- Signed-off-by: Sean Chen <[email protected]> Co-authored-by: Farhad Shabani <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: #1316
Description
PR author checklist:
unclog.docs/).Reviewer checklist:
Files changedin the GitHub PR explorer.