Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slashing types incompatibility between v45 provider and v47 consumer #1092

Closed
MSalopek opened this issue Jun 28, 2023 · 2 comments · Fixed by #1093
Closed

slashing types incompatibility between v45 provider and v47 consumer #1092

MSalopek opened this issue Jun 28, 2023 · 2 comments · Fixed by #1093
Assignees
Labels
scope: cosmos-sdk Integration with Cosmos SDK type: bug Issues that need priority attention -- something isn't working

Comments

@MSalopek
Copy link
Contributor

MSalopek commented Jun 28, 2023

Problem

During testing it was discovered that an incompatibility exists between ICS <= v2.0.0 and ICS v3 release candidates.

The incompatibility arises from the fact that ICS v3 uses cosmos-sdk slashing proto definitions, whereas ICS <= v2 uses custom proto definitions.

For ICS v3 this is the Infraction enum used:

// Infraction indicates the infraction a validator commited.
enum Infraction {
  // UNSPECIFIED defines an empty infraction.
  INFRACTION_UNSPECIFIED = 0;
  // DOUBLE_SIGN defines a validator that double-signs a block.
  INFRACTION_DOUBLE_SIGN = 1;
  // DOWNTIME defines a validator that missed signing too many blocks.
  INFRACTION_DOWNTIME = 2;
}

ICS <= v2 uses a slightly different definition:

// InfractionType indicates the infraction type a validator commited.
enum InfractionType {
  option (gogoproto.goproto_enum_prefix) = false;

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

This difference makes the ICS v2 provider (cosmos-sdk v45, ibc-go v4) unable to process slash packets coming from ICS v3-rc consumer (cosmos-sdk v47, ibc-go v7).

The changes are not state breaking since the underlying structure stored on chain is the same (both are INT). When sending packets, the structures get marshalled into different JSON strings causing an error when the slashes are processed on the provider. The provider cannot unmarshal the Slash packet Infraction enum.

ICS <= v2 (cosmos-sdk v45, ibc-go v4):

{
    "type": "CONSUMER_PACKET_TYPE_SLASH",
    "slashPacketData": {
        "validator": {
            "address": "",
            "power": "1"
        },
        "valset_update_id": "1",
        "infraction": "INFRACTION_TYPE_DOWNTIME"
    }
}

ICS v3-rc (cosmos-sdk v47, ibc-go v7):

{
    "type": "CONSUMER_PACKET_TYPE_SLASH",
    "slashPacketData": {
        "validator": {
            "address": "",
            "power": "1"
        },
        "valset_update_id": "1",
        "infraction": "INFRACTION_DOWNTIME"
    }
}

Closing criteria

Address the issue by conforming to the infraction proto/json type used by the earlier versions of ICS so the protocol communication over the wire is not hindered.

Possible solutions:

Problem details

Details available on cosmos-sdk:

@MSalopek MSalopek added type: bug Issues that need priority attention -- something isn't working scope: cosmos-sdk Integration with Cosmos SDK ccv-core labels Jun 28, 2023
@MSalopek MSalopek self-assigned this Jun 28, 2023
@shaspitz
Copy link
Contributor

This all looks great, thanks! Quick comment that from my knowledge the test was ran with gaia v10 which references ICS v1.1.0-multiden, not v2. Also the v1 proto definitions still come from sdk (45 branch), it's just a different proto definition there

@MSalopek
Copy link
Contributor Author

Updated parts of the issue that did not state that the compatibility must be maintainer for ICS <= v2. That includes both v1 and v2. Thanks for pointing it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: cosmos-sdk Integration with Cosmos SDK type: bug Issues that need priority attention -- something isn't working
Projects
Status: ✅ Done
2 participants