cosmos / ibc-rs

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

Investigate if store functions should return a `Result` #381

Open plafer opened 1 year ago

plafer commented 1 year ago

We currently return early if one of the writes to the store fails (e.g. here). ibc-go assumes writes never fail (e.g. here).

We should think whether we want to also assume that writes never fail, or if not, what to do when a write fails.

romac commented 1 year ago

Any store backed by some kind of I/O device (file system, network, memory in some specific cases, etc) can and likely will fail at some point. Therefore I believe we should reflect this at the type level and deal with those failures accordingly (eg. by simply bubbling them up to the host application).

What do you think?

plafer commented 1 year ago

Right, I thought some more about this since yesterday, and I think it's easily solvable if we document that "if validate/execute/deliver() returns an error (any top-level function), all state changes need to be reverted". This is what we do in basecoin-rs, and I think an undocumented assumption that we make everywhere in ibc-rs (which makes sense but I still keep forgetting from time to time). The fix is probably to just clearly document this requirement.