modules: Change *Reader traits to return Result instead of Option

[yito88]

Crate

ibc

Summary

The functions of the module readers in contexts (e.g. ClientReader) can return Result instead of Option.

Problem Definition

Most of the readers get a module or state from the storage. These operations could fail, e.g. DB failure. A reader function returns just None in this case for now. It looks difficult to handle the failure.

For example, when initializing a connection, ClientReader returns None due to a temporary DB error. It can be done by just retrying the initialization. If the reader returns None because the corresponding client state doesn't exist, we need to create a client at first. If Result returns, we can handle the failure more clearly.

Proposal

The functions of the module readers return Result instead of Option

Acceptance Criteria

Readers returns Result

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[yito88]

I think we need discussion because this proposal affects many IBC handlers. What do you think?

[adizere]

Your proposal is sound. Our current approach using return values consisting of an Option<..> was good enough so far, but it's probably overly simplistic in practice.

Would you like to take a stab at starting to implement the necessary modifications for a simple Context, e.g., ClientReader? Glad to help reviewing and merging that. This will probably require that we also define the specific kind of errors that this context can return (see here for an example of error definitions).

[yito88]

This is a question. I'd like to know how to add errors for other failures to the error list.

I tried to add a read failure with std error. It looks wrong. I want to give any error to this error, e.g. my storage error. What should I do?

        ReadFailure
            [ std::error::Error ]
            | e | {
                format_args!("Reading an object failed with reason: {}", e.to_string())
            },

[romac]

Try this:

        ReadFailure
            [ DisplayOnly<Box<dyn std::error::Error + Send + Sync>> ]
            | e | {
                format_args!("Reading an object failed, reason: {}", e)
            },