cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
197 stars 79 forks source link

Clean up error variants #1316

Closed seanchen1991 closed 1 month ago

seanchen1991 commented 1 month ago

Feature Summary

As specified in #1310, part of improving ibc-rs's error handling involves cleaning up old and redundant error variants, as well as consolidating unnecessary fields under error variants.

Some specific problems that need to be addressed include the following:

  1. The fields contained in error enum variants are not formatted consistently. For example, error variants that contain a string description might have this field named description, or it might be named reason; string descriptions are not named consistently across error variant fields.
  2. Error variant field formatting is also not consistent. A single error variant should have no more than 2 fields and adhere to the following formatting based on the exact number of fields:
    • Single-element variants should either be encoded as a struct containing a string field named "description" that provides more context on why a certain error was produced, or a tuple-struct that points to some state that the error variant applies to. It should be clear from the name of the error variant how the un-named tuple-struct element relates to the error.
    • Two-element variants should generally be formatted as an "actual" field and an "expected" field for capturing state mismatches that cause errors.
  3. Error variants should all be classified into one of six classes, as illustrated in the following snippet:
    /// All the possible error codes that error variants can encapsulate.
    pub enum Code {
    /// cannot be empty
    Empty,
    /// something is missing
    Missing,
    /// already exists
    AlreadyExists,
    /// has invalid state
    Invalid,
    /// state mismatch
    Mismatch
    /// NotAllowed
    NotAllowed,
    /// this action is not supported
    NotSupported,
    }

    To this end, all error variants should adhere to one of these six classifications. Variants that make use of an Other class should be consolidated as best as possible into one of these classifications.

Proposal

As part of cleaning up ibc-rs's error variants, we should address each of the above concerns:

  1. Any error variant that contains a string description should have that description field be renamed to description if it is not already named as such.
  2. Ensure that error variant fields adhere to the format outlined above.
  3. Consolidate error variants such that they all clearly fall into one of the six specified error classifications. This will involve consolidating many errors that make use of an Other classification.
### Tasks
- [ ] https://github.com/cosmos/ibc-rs/pull/1317
- [ ] https://github.com/cosmos/ibc-rs/pull/1318
- [ ] https://github.com/cosmos/ibc-rs/pull/1321