Ideas for improvements

[hu55a1n1]

CI

cross

cross provides “Zero setup” cross compilation and “cross testing” of Rust crates using docker/podman.

• This could be used to test if the code is actually building & passing tests on all supported platforms before release. See list of supported targets here.
• Supporting (and testing for) Windows (mingw) should also be pretty straight-forward.

criterion.rs

Statistics-driven benchmarking library for Rust

• Might be a good time to start thinking about optimizations and benchmarking. Some performance improvements and bottlenecks have already been identified, so we could leverage a lib like criterion.rs to measure performance improvements.
• The library also helps identify performance regressions, so we could set a threshold and perform regression testing.
• There's also support for statistics/charts/flamegraphs.

Testing

fail-rs

Fail points for rust

• Fault-injection can be used to test correctness by injecting dynamic failures that might occur rarely.
• This is particularly helpful to make sure that the state of long running programs (daemons) is consistent even in case of failures and to provide guarantees for fault tolerance and recovery.
• Fail points are flexible and can be configured to exhibit a variety of behavior, including panics, early returns, and sleeping.
• I would propose that we add fail-points for all systems calls/functions that are not expected to fail, especially in the daemon code.
• The API documentation has some goood usage examples.

CLI tests

With a sufficient number of users of an API, it does not matter what you promise in the contract: all observable behaviors of your system will be depended on by somebody. - Hyrum's law

• Test CLI rigorously for various options and their combinations.
• Make CLI context aware?

Long-running tests & DSL

• Orchestrate and run end-to-end tests for extended periods of time.
• Test multi-chain and multi-relayer scenarios.
• It might be helpful to define a dedicated DSL (domain specific language)/script/config for bootstrapping and running these tests.

Design and Distribution

actix

• The actor model seems like a good fit for what we're trying to achieve with the supervisor and workers.
• actix also supports combining async/sync actors (i.e. Arbiter v/s SyncArbiter) and actor supervision.

hermes = hermesd + hermesctl

• The hermes binary is overloaded with functionality, so we could split it into a daemon and a controller. I propose using the systemd/systemctl design.
• We could also provide installers, systemd unit files, etc. for easier installation.

UX

• add ibc-rs deps to crate docs -> libssl + cc
• print git commit hash in hermes help
• document config::strategy
• provide option to generate default config?
• Add autocomplete?

Misc

• Since we foresee validator owners running relayers, does it make sense to pursue optimizations in IBC where we assume trusted validators?

[soareschen]

I can share few of my ideas as well:

Nix

Nix provides reproducible build across different language boundaries. Using Nix we will be able to build Hermes, Tendermint, and Cosmos SDK as Nix packages, and create a reproducible environment for the different executables to interact. The build artifacts can be packages as Docker containers, and they can also be deployed directly using NixOps. The Nix build is much more lightweight than Docker build, and users can run the executables directly on the host machine without any tools such as Rust and Go being installed.

Async / Await

The current concurrency model of ibc-rs introduces unnecessary complexity by separating components to run in threads that communicate through messages. This introduces various bottlenecks to the system as the components are processing each message sequentially. We should instead move to an event-driven architecture using async/await, and run each execution flow in their own lightweight async tasks. That way, all events can be processed concurrency with no limit on the underlying OS threads.

Message-passing concurrency is only needed when two concurrent executions need to be synchronized. We may need that in some cases, but I suspect in the common cases, the execution tasks can run independent of each others. So async tasks may result in simpler architecture.

Generic Types

We should make more using of generic types instead of dynamic traits to abstract over the implementation. For example, instead of a function like fn handler(chain1: Box<dyn ChainHandler>, chain2: Box<dyn ChainHandler>), it could instead be written as fn handle<Chain1: ChainHandler, Chain2: ChainHandler>(chain1: Chain1, chain2: Chain2). The chain implementations could then have associated types such as Height, Connection, etc. This would make it less likely to accidentally mix up the values when forwarding them from one chain to another.

Decouple from Cosmos SDK

The current code is pretty tightly coupled to Cosmos SDK, even so with only specific version. The code should allow multiple chain implementations, at least starting with multiple versions of Cosmos SDK. In a truly multichain world, I do not think all blockchains will be running the same software. With the relayer restricted to specific versions of Cosmos SDK, this can significantly affect the ability of chains to evolve their implementations independently. In terms of chain sovereignty, this also gives too much power to the developers of Cosmos SDK, if they can force chains to use specific versions of the code because of the limitations of the relayer.

Using the generic types design earlier, we should be able to implement different types of chains, and ensure that ibc-rs can evolve independently of the chain implementations.

Isolated Unit Tests

It is essential to have end to end testing of the relayer done with actual Cosmos SDK chain. However we are pretty lacking in isolated unit tests that test for the correctness of the relayer independent of the chains. With the current design, it is very difficult to know if a code change would break anything, unless we run the full end to end tests.

Ideally, we should aim for catching most bugs and breaking changes by running only the unit tests. The end to end tests should be reserved for catching obscure scenarios that have not been considered in the unit tests. With the correct design, many of the functionalities of the relayer should be testable without having to spawn up the full Tendermint and Cosmos SDK.

[romac]

Thank you very much for starting this discussion, @hu55a1n1! Some thoughts/comments below:

cross

I think we can do this by just enabling Windows builds in the workflow we use to build and upload Hermes binaries on each release.

criterion

There's definitely a lot of perf to gain across the board, but it's not clear to me if most of it is the type that can be profiled and optimized using microbenchmarking libraries like criterion, as we are currently mostly I/O bound it seems.
We should definitely look into whether that's the case or not.

fail-rs

That's a great idea! I was not aware of this library but setting up our very own Chaos Monkey would definitely be worthwile! Not using Netflix's tool, but fail-rs instead.

CLI tests

We do have some end-to-end tests which make use of the various CLIs, but it's true that we do not test all possible combinations of options. IMHO that's lower priority than the rest as most of those CLIs are not really meant to be used by end-users.

Can you expand on what you mean by "Make CLI context aware"?

Long-running tests & DSL

👍

Actors

Yep, been recently thinking the same. Not clear if we want to go for Actix, or just move towards a more principled actor implementation at the moment. I need to look more into Actix's supervision capabilities.

hermes = hermesd + hermesctl

Makes sense to me, and would be a chance to clean things up on the CLI side, perhaps even move away from Abscissa while we are at it. It's a lot of work though, and not very high priority at the moment but def something we'll eventually want at some point imho.

UX

• add ibc-rs deps to crate docs -> libssl + cc 👍
• print git commit hash in hermes help 👍
• document config::strategy 👍
• provide option to generate default config? 👍
• Add autocomplete? 👍

Few more additions:

• validate the configuration (on load, and via a CLI)
• figure out how to move away from gumdrop and towards clap, as the way the former deals with help messages is very much subpar

[hu55a1n1]

Thank you for your insightful thoughts and analysis of each of the suggestions @romac!

One thing I forgot to mention about cross-rs is that it also allows running tests on targets that include a PowerPC big-endian and WASM platforms (not sure if that's relevant for us, might be more useful for tendermint-rs). I haven't worked much with workflow before, but I like the fact that it has first-class support on GitHub and it definitely looks like a better alternative for Windows builds.

Regarding Make CLI context aware, I was thinking about UX improvements where instead of just printing the errors encountered, we could also provide hints/help regarding that error. e.g. something similar to what apt does with its are you root? message so that the user realizes that the command needs to be invoked with sudo ->

$ apt install something
E: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), are you root?

To me adding a retry mechanism (where the CLI controls the retrying) is also something I would consider as making the CLI context-aware.

[adizere]

Great start! I'm also amazed to see how on many of these points we're converging, though coming from different perspectives.

To make things concrete and follow-up, I would like to propose the following priorities:

1. commit hash in hermes help, as well as in error messages
2. support to generate default config
3. autocomplete -- should be handled by gumdrop/clap
4. generic types
5. config validation -- Tendermint light client: add consensus state pruning on client update ibc-rs#600
6. extend the end-to-end test suite
   • multi-chain and multi-relayer scenarios
   • DSL (domain specific language) or porting the Python suite to Rust

These are ranked by a combination of their simplicity (i.e., the fix/feature is localized, and needs not much context for a successful PR) and value.