Add amino JSON proto annotations

[aaronc]

Summary

Add proto options that specify how protobuf types should be serialized in amino JSON.

Problem Definition

Currently, in amino JSON serialization options are specified by:

• registering structs and their amino type name with the amino codec
• go field tags
• custom amino marshaling methods

This means that clients need to inspect the go code directly to figure out how to generate amino JSON properly. It is also problematic for supporting the google.golang.org/protobuf API because those types won't work with the amino codec. (This is described more fully in #10993 where amino JSON proto annotations were first proposed).

Proposal

Add protobuf options to specify amino JSON serialization options that can be used by:

• clients
• to implement Implement protoreflect based amino JSON encoder #10993

Specifically, we need options for:

• amino type names for protobuf messages (maybe cosmos.msg.v1.legacy_amino_name)
• whether fields have omitempty specified (maybe cosmos.msg.v1.legacy_amino_omitempty)

The existing cosmos_proto.scalar annotation may be sufficient to handle marshaling of custom types but we need to make sure these annotations are applied consistently.

In order for this system to work correctly, there needs to be some validation that options are correctly specified during Msg server registration, similar to the validation described in #13405).

Note: depending on the time frame for SIGN_MODE_TEXTUAL and how quickly we expect it to completely replace amino this issue may or may not be irrelevant.

[alexanderbez]

Doesn't sign mode textual mean that Amino can be completely removed? If so, are these annotations worth it? How long do we see Amino sticking around?

[aaronc]

Doesn't sign mode textual mean that Amino can be completely removed? If so, are these annotations worth it? How long do we see Amino sticking around?

Good point. I meant to put that in as a caveat. I'm not really sure. If it's more that an year to SIGN_MODE_TEXTUAL maybe worth it. If it's six months, probably not.

@AmauryM do you have a roadmap estimate? I did post this with the thought that it will be a while till everything is ready but I'd be happy to hear that it will be sooner.

[alexanderbez]

Ok that's fine. Since adding annotations are relatively low-lift, there's no harm in doing this.

[amaury1093]

Even though Textual gets shipped in the next 6 months (which is my current estimate, Q1 2023), I think we still want to keep amino for a while, for the ecosystem to migrate.

So my preference is still to move on with this protoreflect amino encoder for now.

[pyramation]

hey I'm super curious about this, as I'm building a transpiler to generate amino encoders and would love to collaborate on a standard way to create proto annotations. This seems like a relevant issue, let me know if I'm in the wrong place!

A few examples that we're doing today:

for example, in wasm/v1/tx Telescope is currently reading (gogoproto.customname) = "WASMByteCode" in order to know when it should call toBase64() and fromBase64 in the amino encoders

Another example, we are using (gogoproto.casttype) = "RawContractMessage" for encoding smart contract objects, and using RawContractMessage in order to know when it should call toUtf8() and fromUtf8() in the amino encoders

However, these particular proto options feel less semantic than having a codegen or amino json annotation of sorts. So I was imagining something like this that I could hook into:

  bytes wasm_byte_code = 2 [ (gogoproto.customname) = "WASMByteCode", (codegen.hint) = 'base64' ];

or something like this so it’s it’s own flag that wouldn’t get updated by devs accidentally and break codegen

cc @webmaster128

[aaronc]

So gogoproto options should generally not be inspected by anything other than gogoproto, although maybe it's okay for use cases like yours temporarily. Gogo proto is basically a dead project and we're going to remove the annotations eventually. They're very golang specific and not semantic like you say.

cosmos_proto.scalar may suit your use case, however, and is intended to be a semantic annotation that can be used by tooling.

[amaury1093]

Hey @pyramation, I created a branch to show @aaronc's cosmos_proto.scalar idea: #13466.

// existing Msg
message MsgStoreCode {
   bytes wasm_byte_code = 1 [
        (gogoproto.customname) = "WASMByteCode",
        (cosmos_proto.scalar) = "wasm.v1.WASMByteCode" // <--- New option
    ];
}

// inside new `wasm/v1/scalars.proto` file:

option (cosmos_proto.declare_scalar) = {
    name: "WASMByteCode",
    description: "A coswasm contract byte code",
    field_type: [SCALAR_TYPE_BYTES],
    amino_encoding: "Use base64 to encode this field" // Some human-readable string on how to encode this field.
    // TODO: or should `amino_encoding` be machine-readable?
};

It's basically expanding on your codegen.hint idea, but trying to standardize this. It's imo slightly superior, because it would disallow this type of (wrong) lines:

  bytes wasm_byte_code = 2 [ (gogoproto.customname) = "WASMByteCode", (codegen.hint) = 'utf8' ]; // Should be base64!

because we would standardize (once) in a separate scalars.proto file that WASMByteCode should always be amino-encoded as base64.

[ValarDragon]

I don't understand, why aren't we just adding the simple codegen.hint field?

Happy to rename codegen.hint to cosmos.amino_encoding="base64"

Why did this "declare_scalar" thing come in, what problem is that solving? This as a problem area should be rare, no? And for this example of WasmStoreBytes, the problem is its a one-off, not something to be routinely reused. (If it was it should just be a new proto type that we serialize, no?)

Also Amino_encoding should unquestionably be a simple enum, that is trivially machine readable. The sentence is at most a comment, but is honestly dead simple in almost all situation.

[pyramation]

I think cosmos.amino_encoding or something as simple would be the perfect solution. We can simply add to any field that needs it.

  bytes wasm_byte_code = 2 [ (gogoproto.customname) = "WASMByteCode", (cosmos.amino_encoding) = 'base64' ];

I also didn't realize the complexity of the scalar concept. I thought it was something like the codegen.hint, now realizing it's a way of defining types. I think for codegen, the thing we need is small hints on only a small handful of fields.

[amaury1093]

The rationale for scalar is 1/ adding semantic meaning to string/bytes fields and 2/ having a centralized place for encoding of these field (aka the scalars.proto file).

bytes msg = 5 [ (cosmos.scalar) = "wasm.v1.RawContractMessage" ];

so:

1. people know what these bytes represent
2. in the wasm/v1/scalars.proto, you have the legacy_amino encoding, but also potentially the sign_mode_textual custom encoding (as @webmaster128 was requiring)

Without scalars, we would do

bytes msg = 5 [ (cosmos.amino_encoding) = 'base64', (cosmos.textual) = 'decode_as_json' ];

and if there are N places where we use RawContractMessage, we define encoding N times (+ risk of inconsistency).

But I do agree it's slight more code to retrieve the correct encoding (i.e. need to find the scalars.proto file). If the above is not a strong reason enough, I'm happy to fall back to field-level proto annotations. LMK what you think @pyramation @ValarDragon

[webmaster128]

I also don't understand the scalar concept. Feels like it just creates indirection but I might have missed something.

So in CosmWasm we (ab)use the current messages implementation for embedding JSON messages sent to a contract. A separate Go type RawContractMessage defines the Go implementation and the client has to magically know that the protobuf bytes fields need to be UTF-8 decoded and displayed as a JSON string. This is the same case for both Amino JSON and Textual:

bytes msg = 5 [ (cosmos.amino_encoding) = 'utf8_json', (cosmos.textual) = 'utf8_json' ];

The same applies for four message types MsgInstantiateContract, MsgInstantiateContract2, MsgExecuteContract, MsgMigrateContract as well as proposal types InstantiateContractProposal, MigrateContractProposal, SudoContractProposal, ExecuteContractProposal.

and if there are N places where we use RawContractMessage, we define encoding N times (+ risk of inconsistency).

This does not sound like an issue to me. We do that for custom proto types as well and IMO every Amino JSON and textual renderer schould have a test that ensures a particular sign doc representation.

[robert-zaremba]

Is there any use case for teleascope to deal with Amino? Dapps usually don't do multi sig. cc: @pyramation

[robert-zaremba]

@webmaster128 the idea of cosmos.scalar is to be a metatype. It will be serialized / deserialized as bytes. But as you noted in your example:

bytes msg = 5 [ (cosmos.amino_encoding) = 'utf8_json', (cosmos.textual) = 'utf8_json' ];

you will need to specify these hints for every codec.
Both options will work. So we just need to make a decision. Maybe a simple example will help to validate which one is better?