Replace TimestampAtHeight with TimeoutElapsed interface on light client module

[colin-axner]

Thank you @benluelo for surfacing this issue and providing great assistance in helping me understand the problem!

Summary

Replace TimestampAtHeight with TimeoutElapsed on the light client module interface

Problem

We have two general problems with the current design:

1. other ecosystems may have heights/timestamps which are decoupled from consensus and execution layers
2. core ibc enforces an understanding of timeouts when it is up to the counterparty to define it and the light client to interpret it

When consensus and execution layers use different height values

The first issue was brought to light by discussion with @benluelo.

As the structure for a blockchain is broken down, it is desirable to separate the consensus layer from the execution layer. This is typically done in L1/L2 frameworks. This results in a height or timestamp for each:

• consensus height
• execution height

Consensus heights are related to the height at which a validator comes to consensus over a blockhas. This is used for proving, verifing membership etc. This is typically the height of the consensus state.

Execution heights are related to timeouts. The question being "would this execution have succeeded or would succeed given the timeout value?". The distinction here being that sometimes execution occurs against a height/timestamp which is not directly correlated to the height of the proof.

Currently, on packet sends and when determining timeouts, core IBC will use either the latest light client height (consensus height) on packet send, or the proof height on timeout to determine if a timeout has elapsed. That is, core IBC is obtaining a consensus height and doing a comparison against an execution height, which makes supporting decoupled consensus/execution layers impossible.

Core IBC making assumptions on timeouts

As we note above, it is difficult for core IBC to define a timeout interpretation which is generic to all chains. In fact, this is not necessary.

Timeouts are used to determine whether a packet can still be received. Conceptually, whether a packet is timed out is defined by the receiving IBC implementation. This means the light client on the sending side should be capable of implementing this interpretation as well.

By decoupling timeout interpretation from core IBC, timeouts can be defined more broadly and chosen by implementations. This would allow us to remove the dreaded "revision number" and condense timeout into a single string (rather than being a "Height" and timestamp).

Proposal

We should remove the TimestampAtHeight and replace it with a TimeoutElapsed interface function:

  TimeoutElapsed(ctx sdk.Context, clientID string, timeout Timeout, consensusHeight clienttypes.Height) bool

In this api, the light client module will be given the ctx and client identifier as well as the execution timeout height/timestamp for a packet and the consensus height at which this packet is being referenced.

The light client module is being asked, "has this timeout (execution height/timestamp) passed at the given consensus height".

In SendPacket:

-       latestTimestamp, err := k.clientKeeper.GetClientTimestampAtHeight(ctx, connectionEnd.ClientId, latestHeight)
-       if err != nil {
-               return 0, err
-       }
-
        // check if packet is timed out on the receiving chain
        timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp())
-       if timeout.Elapsed(latestHeight, latestTimestamp) {
-               return 0, errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight, latestTimestamp), "invalid packet timeout")
+       if lightClientModule.TimeoutElapsed(ctx, connectionEnd.ClientId, timeout, latestHeight) {
+               return 0, errorsmod.Wrap(types.ErrTimeoutElapsed(latestHeight), "invalid packet timeout")
        }

In TimeoutPacket:

-       // check that timeout height or timeout timestamp has passed on the other end
-       proofTimestamp, err := k.clientKeeper.GetClientTimestampAtHeight(ctx, connectionEnd.ClientId, proofHeight)
-       if err != nil {
-               return "", err
-       }
-
        timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp())
-       if !timeout.Elapsed(proofHeight.(clienttypes.Height), proofTimestamp) {
-               return "", errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached")
+       if !lightClientModule.TimeoutElapsed(ctx, connectionEnd.ClientId, timeout, proofHeight.(clienttypes.Height)) {
+               return "", errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height)), "packet timeout not reached")
        }

We likely need a special solution for wasm (maybe wasm for now would do the checks core IBC did before or it would just add the new interface and require contracts to migrate).

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned