Custom self client validation for 08-wasm tendermint clients

[damiannolan]

Summary

Wire up 08-wasm simapp with a custom SelfClientValidator, using the changes made in #5836.
Branch off the PR linked above.

We need an implementation of this interface (below), which unwraps the bytes stored in the 08-wasm ClientState to a 07-tendermint ClientState, and performs the same logic as is done here. (same logic for GetSelfConsensusState)

type SelfClientValidator interface {
	GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error)
	ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error
}

Something like the following should allow us to connection two simapps using wasm tendermint contracts:

var _ clienttypes.SelfClientValidator = (*WasmTMSelfClientValidator)(nil)

type WasmTMSelfClientValidator struct {
    cdc codec.BinaryCodec
    tm clientkeeper.TendermintSelfClientValidator
}

func (w WasmTMSelfClientValidator) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) {
    consensusState, err := w.tm.GetSelfConsensusState(ctx, height)
    if err != nil {
         return err
    }

    // encode consensusState to wasm.ConsensusState.Data

    return wasmConsensusState
}

func (w WasmTMSelfClientValidator) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error {
    wasmClientState, ok := clientState.(*wasm.ClientState)
    if !ok {
         return err
    }

    // unmarshal the wasmClientState bytes into tendermint client and call self validation
    var tmClientState *ibctm.ClientState 
    if err := w.cdc.Unmarshal(wasmClientState.Data, tmClientState); err != nil { 
        return err
    }

    return w.tm.ValidateSelfClient(ctx, tmClientState)
}

---

For Admin Use

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

[damiannolan]

It would be nice if we could support both 08-wasm tendermint as well as 07-tendermint (native go) in one implementation.

It would be possible to do for ValidateSelfClient with the current approach:

switch cs := clientState.(type) {
    case *ibctm.ClientState:
        return w.tm.ValidateSelfClient(ctx, cs)
    case *wasm.ClientState:
        // unmarshal cs.Data to tmClientState
        return w.tm.ValidateSelfClient(ctx, tmClientState)
}

But for GetSelfConsensusState we don't have a way to infer whether or not to encode the consensus state as one type or the other. Not ideal.

[crodriguezvega]

Just an idea, but feel free to discard it's no good: could we have a stack of SelfClientValidator implementations instead of just one? For example a chain might want to add client validators for Go Tendermint and Wasm Tendermint and they could stack them, so that ibc-go will go through the stack until it reaches one that doesn't return an error? Although I think this wouldn't solve the problem for GetSelfConsensuState... which Steve solved, by the way, by adding an extra parameter to the function with the client type.

And another idea, what about separating the SelfClientValidator interface in two different interfaces? GetSelfConsensusState is not really doing any validation, so maybe it's better to have it in a separate interface. Although maybe two interfaces with just one function each is to granular...

[damiannolan]

Thanks for digging up that branch @crodriguezvega, was meaning to follow up on that this week.

It's a good point that GetSelfConsensusState does not really perform any validation - maybe this is a good time to rethink the abstraction here as well as the naming - SelfClientValidator was just something we used in the initial PR (#5836) in order to start making progress, and I mentioned in the description that this is probably not ideal naming given the usage of validator terminology in pos consensus as well.
It's possible to also require relayers to provide the ConsensusState as an argument in the msg, as this is just constructed for proof verification of counterparty state in the connection handshake, Aditya suggested this approach in #5815 IIRC.

From the progress made on the 02-client routing work we have also discussed the ability for wasm clients to be able to store their ClientState without the 08-wasm envelope. Where the envelope type is necessary only for routing to the light client module on Initialize. However, I think this would still require the bytes field within the envelope to encoded as protobuf.Any. This isn't necessarily always true at the moment, as the 08-wasm envelope contains bytes data and not protobuf.Any data for the underlying client state.
If we want the client runtime(golang, wasmer) to be completely opaque to the 03-connection layer (for negotiating the handshake etc.) then the encoding needs to be the same.

[chatton]

@crodriguezvega @damiannolan the stack idea is nice, and can actually already be implemented using the same interface. All that would be required is an implementation which contains a []SelfClientValidator whose implementation loops through them until a non error is returned.

Happy to consider changing the interface though if something else makes a little more sense.

[damiannolan]

Going to close this issue as this was meant to track this smaller piece of work as a stepping stone in the greater picture of #5315

I'm not sure if I'd want to make some an opinionated design or api decision solely to support this encoding problem between a native golang 07-tendermint client and a wasm tendermint client. I think that issue can be solved elsewhere.

Let's move any additional discussion to #5315.