Infraction enum changed in cosmos-sdk v0.47.x with respect to v0.45.x-ics

[MSalopek]

Summary of Bug

Changes required for the ICS protocol were introduced to the cosmos-sdk v0.47.0.
Previously, the changes were only available in cosmos-sdk v0.45.X-ics releases.

The Infraction and InfractionTypes enums available in cosmos-sdk v0.45.X-ics and cosmos-sdk v0.47.x are not 100% compatible.

The issue:

When storing the Infraction type the issue will not be manifested, because the data is usually stored as proto bytes and the underlying enum values are the same between versions.

However, when marshalling the protos into JSON (using ModuleCdc.MustMarshalJSON()) the types are not compatible due to how JSON marshalling works in cosmos-sdk.
When marshalling the Infraction from v47 to JSON we get the following structure:

{
    ...
    "infraction": "INFRACTION_TYPE_DOWNTIME"  // enum INT value == 1
}

When marshalling the InfractionType from v45 we get the following output:

{
    ...
    "infraction": "INFRACTION_DOWNTIME" // this is a different string in JSON, but the proto enum INT value == 1
}

Generated proto enum value maps are different

v47

// Enum value maps for Infraction.
var (
	Infraction_name = map[int32]string{
		0: "INFRACTION_UNSPECIFIED",
		1: "INFRACTION_DOUBLE_SIGN",
		2: "INFRACTION_DOWNTIME",
	}
	Infraction_value = map[string]int32{
		"INFRACTION_UNSPECIFIED": 0,
		"INFRACTION_DOUBLE_SIGN": 1,
		"INFRACTION_DOWNTIME":    2,
	}
)

v45

var InfractionType_name = map[int32]string{
	0: "INFRACTION_TYPE_UNSPECIFIED",
	1: "INFRACTION_TYPE_DOUBLE_SIGN",
	2: "INFRACTION_TYPE_DOWNTIME",
}

var InfractionType_value = map[string]int32{
	"INFRACTION_TYPE_UNSPECIFIED": 0,
	"INFRACTION_TYPE_DOUBLE_SIGN": 1,
	"INFRACTION_TYPE_DOWNTIME":    2,
}

The issue with parsing the Infraction can manifest when 2 chains running different versions of cosmos-sdk are communicating over IBC.
In ICS it manifests because a SlashPacket can be sent between chains that use different cosmos-sdk versions.

Context & Example:

When we send data packets containing infractions in v45 vs v47 we get the following:

// cosmos-sdk v45 proto marshalled to JSON
{
    "type": "CONSUMER_PACKET_TYPE_SLASH",
    "slashPacketData": {
        "validator": {
            "address": "",
            "power": "1"
        },
        "valset_update_id": "1",
        "infraction": "INFRACTION_TYPE_DOWNTIME"  // enum INT value == 1
    }
}

// cosmos-sdk v45 proto marshalled to JSON
{
    "type": "CONSUMER_PACKET_TYPE_SLASH",
    "slashPacketData": {
        "validator": {
            "address": "",
            "power": "1"
        },
        "valset_update_id": "1",
        // this is a different string, but the proto enum INT is 1
        "infraction": "INFRACTION_DOWNTIME"
    }
}

Due to how JSON Marshal/Unmarshal works in cosmos-sdk, these 2 types are not compatible. One type cannot be parsed into another because the enum tags don't match.

v45

Last ICS version
https://github.com/cosmos/cosmos-sdk/blob/v0.45.16-ics/proto/cosmos/staking/v1beta1/staking.proto#L356

First ICS version
https://github.com/cosmos/cosmos-sdk/blob/v0.45.11-ics/proto/cosmos/staking/v1beta1/staking.proto#L356

The InfractionType is unchanged

v46

Not affected, InfractionType is not available.

v47

https://github.com/cosmos/cosmos-sdk/blob/v0.47.3/proto/cosmos/staking/v1beta1/staking.proto#L392C18-L392C18

InfractionType got renamed to Infraction.
Enum tags were renamed:

enum Infraction {
  INFRACTION_TYPE_UNSPECIFIED --> INFRACTION_DOWNTIME
  INFRACTION_TYPE_DOUBLE_SIGN --> INFRACTION_TYPE_DOUBLE_SIGN
  INFRACTION_TYPE_DOWNTIME --> INFRACTION_TYPE_DOWNTIME
}

Unfortunately the enum tag is used in JSON format, instead of the enum value.

Possible solution

Change the Infraction enum in such a way that it is the same "over the wire" between different cosmos-sdk versions.

enum Infraction {
  option (gogoproto.goproto_enum_prefix) = false;

  // UNSPECIFIED defines an empty infraction.
  INFRACTION_TYPE_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "Infraction_INFRACTION_UNSPECIFIED"];
  // DOUBLE_SIGN defines a validator that double-signs a block.
  INFRACTION_TYPE_DOUBLE_SIGN = 1 [(gogoproto.enumvalue_customname) = "Infraction_INFRACTION_DOUBLE_SIGN"];
  // DOWNTIME defines a validator that missed signing too many blocks.
  INFRACTION_TYPE_DOWNTIME = 2 [(gogoproto.enumvalue_customname) = "Infraction_INFRACTION_DOWNTIME"];
}

The changes should not be state breaking. As far as we can tell, the issue is isolated to sending the types "over the wire" since the strings are different in JSON format.

Temporary solution implemented in ICS

We have temporarily created shims that ensure that v45 and v47 chains remain compatible when parsing data that contains Infraction.

Version

https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.3 (v0.47.x)

Steps to Reproduce

[tac0turtle]

i wouldnt consider this a bug since the pr that added this went through review cycles and this wasnt brought up, since this is already part of a release im inclined to leave it as is.

I dont think TYPE is needed here as well. I wish we caught this in the review cycles but for 0.47 it will need to stay as is in the repository

[tac0turtle]

discussed and closed the pr. we can close this issue now