Support json tx encoding for interchain accounts

[womensrights]

Summary

Currently interchain accounts only has protobuf encoding, json encoding would give more flexibility to using the feature, particularly for CosmWasm and Solidity smart contracts.

Problem Definition

Adding json encoding would improve ICA composability, the feature is currently limited to only protobuf encoding

Proposal

Add json encoding for ICA messages

---

For Admin Use

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

[colin-axner]

Is there an example user flow? Trying to understand whether this is a helper function or for decoding on the host? I'm not sure I follow when this would need to be rlp encoded on the host side within ibc-go

[colin-axner]

Revisiting, here's how to implement this issue for json. Please implement json and rlp support in separate prs.

User flow

json_encode(tx) -> give to controller -> ibc send, relay -> host recv -> decode_json(tx) into []sdk.Msg -> execute tx

json_encoding code will be external to ibc-go (potentially a wasm contract)

Implementation

Add EncodingJSON to metadata.go in the same format as EncodingProtobuf

Add EncodingJSON to the list of supported encodings

Modify SerializeCosmosTx/DeserializeCosmosTx to add an extra parameter for the encoding type (this is an API breaking change; we'll need to deal with it in the backport to v7.1.x by keeping the signature and adding an extra function). It should switch on the encoding type, something similar to:

// DeserializeCosmosTx, pass in full packetData
// *codec.ProtoCodec implements the JSONCodec interface

if _, ok := cdc.(*codec.ProtoCodec); !ok {
        return nil, errorsmod.Wrap(ErrInvalidCodec, "provided codec is not of type %T", codec.ProtoCodec{})
}

var cosmosTx CosmosTx

switch encoding // encoding is a parameter passed to the function
case EncodingProtobuf:
    if err := cdc.Unmarshal(data, &cosmosTx); err != nil {
        return nil, err
    }
    
case EncodingJSON:
    if err := cdc.UnmarshalJSON(data, &cosmosTx); err != nil {
        return nil, err
    }
    
default:
    return nil, errorsmod.Wrap(ErrUnsupportedEncoding, "encoding type %s is not supported", packetData.Encoding)
}

Similar changes can be made to SerializeCosmosTx

Testing

A test should be added to the host submodule which creates a channel using the JSON encoding. It should send a successful json encoded transaction (succeeds) and it should send a proto encoded transaction (fails). An e2e is not necessary in my opinion.

It would be greatly appreciated if someone could test this integration with wasm 🙏

I may have missed something so please ask questions if you run into any issues

[crodriguezvega]

Thanks for the write-up, @colin-axner! I would like to add a couple of things and hear your opinion.

The InterchainAccountPacketData does not contain at the moment any field with the encoding type. Is your proposal to add it to the proto definition of the packet data? If that was the case, then each message sent to the host could potentially be encoded either in JSON or proto, and then the encoding type (which is part of the version string) agreed during the channel handshake would not be really useful (I guess could be marked deprecated). But my question would be: is reasonable to expect that messages will be encoded differently over one given channel? UPDATE: this is not what Colin meant.

If we don't add an extra field to the InterchainAccountPacketData then we could pass to SerializeCosmosTx/DeserializeCosmosTx, instead of the full packet data, an extra argument with the type of encoding. Then in OnRecvPacket on the host submodule, when the messages need to be decoded, we will need to get the encoding type somehow. Maybe there are, at least, two options:

1. To retrieve the version of the channel (using GetAppVersion), un-marshal it to the Metadata type and read the Encoding field. Pro is that we already have the information in state; cons is that we need to do JSON deserialisation before executing every packet data to figure out how to decode the data. UPDATE: this is the preferred option.
2. To store in state the encoding for each ICA channel and retrieve it in OnRecvPacket. Pro is that we don't need to do JSON deserialisation of the channel version in every ICA packet; cons is that it adds extra state.

However, SerializeCosmosTx/DeserializeCosmosTx are both public functions and changing their signature (either to take the full packet data or to add an extra argument for encoding type) will break an API-breaking change, right? Unless we do a work around and create new functions with the new signature and leave these as is.

Maybe we can also make the encoding type an enum, at least in the Go code, but still use a string in the Metadata type, so that we don't break the API here as well.

We also have this helper CLI command on the host submodule to generate the packet data that I think we should also adapt so that it takes an extra parameter for the encoding type. UPDATE: instead of adding an extra argument to the CLI, it would be possible to retrieve the encoding from the version of the channel end.

[webmaster128]

Please note that there is no specification for how Cosmos SDK transactions are serialized to JSON. It is defined by the Cosmos SDK implementation based on the protobuf JSON specs plus a bunch of extra rules that may or may not be consistent. I.e. the JSON serialization can change any time. I would not recommend to assume chain 1 and chain 2 use the same JSON codec as long as such a spec does not exist.