Hermes caching layer MVP

[adizere]

Crate

ibc-relayer

Summary

Add general support for a caching layer in Hermes.

Problem Definition

The motivation for the caching layer is to reduce pressure on full nodes.

Proposal & Acceptance Criteria

• Hermes should cache the static data it is querying from a chain and using most frequently
  • using the profiling feature and the telemetry data may prove helpful to track down the most frequent queries
  • note that there is already a minimal cache implemented, which stores connections, channel, and client identifier data to filter by client trust threshold

---

For Admin Use

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

[ancazamfir]

Great work in a short time! Just one observation, maybe you have discussed this already. I think we also need to maintain/ update the cache via the IBC events.
One problem now for example is that I can create a connection in Init-Try state, start the relayer with channel mode enabled and then separately finish the connection handshake. In this case the relayer will not relay on channel events over that connection because it thinks the connection is not opened. This will not be fixed by the connection TTL as I don't think we have the equivalent of clear packets at channel level (i.e. we don't rescan for unfinished channel handshakes).
The other case is a stale channel that hermes thinks is opened but that has been closed.

I see that the TTL for client state and latest height is relatively small and probably the stale cache is less noticeable. But we should consider fixing this also.

[adizere]

I think we also need to maintain/ update the cache via the IBC events.

We were debating this yesterday, but the architectural implications for making the cache accessible from the websocket (supervisor code) was not clear. It may actually be easy to pass the cache (after it is created, somewhere here) to the supervisor, but we haven't tried. The idea we toyed with was to automatically update the ClientState.latest_height automatically from the websocket upon detecting ClientUpdate events, but there was no time to work it through.

One problem now for example

Is this a problem that was introduced with PR #1932? I think we only cache channels and connections once they are in Open state. I don't fully understand the scenario yet.

The other case is a stale channel that hermes thinks is opened but that has been closed.

We debated the implications of this problem, and reduce the TTL from 10min to 1min for channel ends. Decided that the cost for Hermes to retry submitting packets on that (closed) channel for up to a minute before it finally detects the channel closure is a decent tradeoff for the moment.

[ancazamfir]

We were debating this yesterday, but the architectural implications for making the cache accessible from the websocket (supervisor code) was not clear. It may actually be easy to pass the cache (after it is created, somewhere here) to the supervisor, but we haven't tried. The idea we toyed with was to automatically update the ClientState.latest_height automatically from the websocket upon detecting ClientUpdate events, but there was no time to work it through.

Yes, I think we can get the chain latest_height from NewBlock and ClientState.latest_height from ClientUpdate.

Is this a problem that was introduced with PR #1932? I think we only cache channels and connections once they are in Open state. I don't fully understand the scenario yet.

You are right. Just tested now with an older gaia version and it's fine. I was hitting cosmos/ibc-go#601 not being included in gaia v6.0.3.

The other case is a stale channel that hermes thinks is opened but that has been closed.

We debated the implications of this problem, and reduce the TTL from 10min to 1min for channel ends. Decided that the cost for Hermes to retry submitting packets on that (closed) channel for up to a minute before it finally detects the channel closure is a decent tradeoff for the moment.

Agreed for the short term it's ok. My question was for long term. We get the IBC events anyway and may as well use them for cache updates. We could reduce the queries to proofs only. The drawback is that we need to deal with the websocket reconnects.

[romac]

We get the IBC events anyway and may as well use them for cache updates. We could reduce the queries to proofs only. The drawback is that we need to deal with the websocket reconnects.

Yeah maybe we can invalidate the cache when we get disconnected.