telescope icon indicating copy to clipboard operation
telescope copied to clipboard

wasm/MsgStoreCode incorrect Amino encoding

Open noahsaso opened this issue 1 year ago • 3 comments

I noticed two issues when trying to Amino encode wasm/MsgStoreCode, one that's indicative of an error that needs to be fixed everywhere, and one that essentially means that autogenerating Amino types from protobuf files is impossible (rip).

MsgStoreCode uses AccessConfig, which has an AccessType enum. The AccessConfig encoding is the source of the issue.

For AccessConfig (in cosmwasm/wasm/v1/types.ts), the amino encoders need to change as follows:

   fromAmino(object: AccessConfigAmino): AccessConfig {
     const message = createBaseAccessConfig();
     if (object.permission !== undefined && object.permission !== null) {
       message.permission = accessTypeFromJSON(object.permission);
     }
     message.addresses = object.addresses?.map(e => e) || [];
     return message;
   },

   toAmino(message: AccessConfig, useInterfaces: boolean = false): AccessConfigAmino {
     const obj: any = {};
-    obj.permission = message.permission;
-    if (message.addresses) {
+    obj.permission = accessTypeToJSON(message.permission);
+    if (message.addresses?.length) {
       obj.addresses = message.addresses.map(e => e);
-    } else {
-      obj.addresses = [];
     }
     return obj;
   },

Notice how:

  • toAmino is not using accessTypeToJSON (relevant in the next section)
  • message.addresses should only be added to the Amino encoding if it's non-empty. this is because the cosmwasm/wasm/v1/types.proto protobuf has repeated string addresses = 3, and it does NOT have (amino.dont_omitempty) = true. for comparison, MsgSubmitProposal in cosmos/gov/v1/tx.proto has a repeated initial_deposit field WITH (amino.dont_omitempty) = true, meaning that it should include an empty array ([]) if there is no deposit included. i believe this holds true everywhere—if a repeated field does NOT have (amino.dont_omitempty) = true, it should only be included in toAmino if it has at least one element.

I believe this omit repeated issue shows up everywhere and can be fixed easily.

Amino types cannot be generated from protobufs alone

Additionally, the accessTypeToJSON and accessTypeFromJSON helper functions themselves are incorrect. The correct functions are:

 export function accessTypeFromJSON(object: any): AccessType {
   switch (object) {
     case 0:
-    case "ACCESS_TYPE_UNSPECIFIED":
+    case "Unspecified":
       return AccessType.ACCESS_TYPE_UNSPECIFIED;
     case 1:
-    case "ACCESS_TYPE_NOBODY":
+    case "Nobody":
       return AccessType.ACCESS_TYPE_NOBODY;
     case 3:
-    case "ACCESS_TYPE_EVERYBODY":
+    case "Everybody":
       return AccessType.ACCESS_TYPE_EVERYBODY;
     case 4:
-    case "ACCESS_TYPE_ANY_OF_ADDRESSES":
+    case "AnyOfAddresses":
       return AccessType.ACCESS_TYPE_ANY_OF_ADDRESSES;
     case -1:
     case "UNRECOGNIZED":
     default:
       return AccessType.UNRECOGNIZED;
   }
 }

 export function accessTypeToJSON(object: AccessType): string {
   switch (object) {
     case AccessType.ACCESS_TYPE_UNSPECIFIED:
-      return "ACCESS_TYPE_UNSPECIFIED";
+      return "Unspecified";
     case AccessType.ACCESS_TYPE_NOBODY:
-      return "ACCESS_TYPE_NOBODY";
+      return "Nobody";
     case AccessType.ACCESS_TYPE_EVERYBODY:
-      return "ACCESS_TYPE_EVERYBODY";
+      return "Everybody";
     case AccessType.ACCESS_TYPE_ANY_OF_ADDRESSES:
-      return "ACCESS_TYPE_ANY_OF_ADDRESSES";
+      return "AnyOfAddresses";
     case AccessType.UNRECOGNIZED:
     default:
       return "UNRECOGNIZED";
   }
 }

This one is tricky because the real strings for enums are not stored in the protobuf files, instead living in the Go code directly. This is found in x/wasm/types/params.go:

func (a AccessType) String() string {
	switch a {
	case AccessTypeNobody:
		return "Nobody"
	case AccessTypeEverybody:
		return "Everybody"
	case AccessTypeAnyOfAddresses:
		return "AnyOfAddresses"
	}
	return "Unspecified"
}

And digging deeper, we find this other enum string encoding for VoteOption in x/gov/types/v1/gov.pb.go:

func (x VoteOption) String() string {
	return proto.EnumName(VoteOption_name, int32(x))
}

...which corresponds to the correct encoders for MsgVote in the gov module, which uses the uppercase enum names.

This reveals that Cosmos SDK modules have no standard for how enums are encoded, and it's up to the module author to decide how to encode an enum. Thus any attempt to autogenerate Amino encoders from protobuf files will fail, and instead it needs to actually parse and construct these types from Go code.

noahsaso avatar Jun 12 '24 17:06 noahsaso

#625 #624

pyramation avatar Jun 13 '24 06:06 pyramation

Hi,

For your first issue, it has been fixed in Telescope. Now the Telescope applies accessTypeFromJSON and accessTypeToJSON to the object.permission.

  fromAmino(object: AccessConfigAmino): AccessConfig {
    const message = createBaseAccessConfig();
    if (object.permission !== undefined && object.permission !== null) {
      message.permission = object.permission;
    }
    if (object.address !== undefined && object.address !== null) {
      message.address = object.address;
    }
    return message;
  },
  toAmino(message: AccessConfig): AccessConfigAmino {
    const obj: any = {};
    obj.permission = message.permission === 0 ? undefined : message.permission;
    obj.address = message.address === "" ? undefined : message.address;
    return obj;
  },

For your second issue, as Dan posted above #624 #625 , now we added two options to let user to customized the enum value. First enable the enums.useCustomNames, set it to true. Then you can customized the value for specific AccessType like this:

{
    "prototypes": {
        "patch": {
            "cosmwasm/wasm/v1/types.proto": [
                {
                    "op": "replace",
                    "path": "@/AccessType/valuesOptions/ACCESS_TYPE_UNSPECIFIED/(gogoproto.enumvalue_customname)",
                    "value": "Unspecified"
                },
            ]
        }
    }
}

The detail would be in Readme: https://github.com/hyperweb-io/telescope/blob/main/README.md#enums-options https://github.com/hyperweb-io/telescope/blob/main/README.md#json-patch-protos

If you have further question or new issue, please tell us.

NorOldBurden avatar May 30 '25 08:05 NorOldBurden

Hi,

For your first issue, it has been fixed in Telescope. Now the Telescope applies accessTypeFromJSON and accessTypeToJSON to the object.permission.

  fromAmino(object: AccessConfigAmino): AccessConfig {
    const message = createBaseAccessConfig();
    if (object.permission !== undefined && object.permission !== null) {
      message.permission = object.permission;
    }
    if (object.address !== undefined && object.address !== null) {
      message.address = object.address;
    }
    return message;
  },
  toAmino(message: AccessConfig): AccessConfigAmino {
    const obj: any = {};
    obj.permission = message.permission === 0 ? undefined : message.permission;
    obj.address = message.address === "" ? undefined : message.address;
    return obj;
  },

For your second issue, as Dan posted above #624 #625 , now we added two options to let user to customized the enum value. First enable the enums.useCustomNames, set it to true. Then you can customized the value for specific AccessType like this:

{
    "prototypes": {
        "patch": {
            "cosmwasm/wasm/v1/types.proto": [
                {
                    "op": "replace",
                    "path": "@/AccessType/valuesOptions/ACCESS_TYPE_UNSPECIFIED/(gogoproto.enumvalue_customname)",
                    "value": "Unspecified"
                },
            ]
        }
    }
}

The detail would be in Readme: https://github.com/hyperweb-io/telescope/blob/main/README.md#enums-options https://github.com/hyperweb-io/telescope/blob/main/README.md#json-patch-protos

If you have further question or new issue, please tell us.

hmm, i'm not sure the first issue is fixed. when using v1.12.17, i get the same as before for cosmwasm/wasm/v1/types.ts:

  fromAmino(object: AccessConfigAmino): AccessConfig {
    const message = createBaseAccessConfig();
    if (object.permission !== undefined && object.permission !== null) {
      message.permission = object.permission;
    }
    message.addresses = object.addresses?.map(e => e) || [];
    return message;
  },
  toAmino(message: AccessConfig, useInterfaces: boolean = false): AccessConfigAmino {
    const obj: any = {};
    obj.permission = message.permission === 0 ? undefined : message.permission;
    if (message.addresses) {
      obj.addresses = message.addresses.map(e => e);
    } else {
      obj.addresses = message.addresses;
    }
    return obj;
  },

which actually removed accessTypeFromJSON from fromAmino, and didn't affect toAmino at all.

noahsaso avatar May 31 '25 07:05 noahsaso