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

Hermes uses wrong unbonding period or fails for ICS Consumer chains #3125

Closed
4 of 14 tasks
Tracked by #3198 ...
ancazamfir opened this issue Feb 25, 2023 · 0 comments · Fixed by #3174
Closed
4 of 14 tasks
Tracked by #3198 ...

Hermes uses wrong unbonding period or fails for ICS Consumer chains #3125

ancazamfir opened this issue Feb 25, 2023 · 0 comments · Fixed by #3174
Assignees
Labels
O: interchain-security Objective: related to Interchain Security
Milestone

Comments

@ancazamfir
Copy link
Collaborator

ancazamfir commented Feb 25, 2023

Summary

As it was discovered in #3112, hermes does not properly support ICS Consumer chains. We fixed this temporarily in #3113
However the fix is not ideal as it requires relayer operator knowledge of the correct unbonding_period of the consumer chain. It also does not address the health check for historical_entries parameter.

Problem Definition

summary of discussions with @mpoke @MSalopek @romac

Currently hermes uses the staking parameter query endpoint to get some parameters required for IBC client creation and chain health check.
ICS Consumer chains do not have an SDK staking module that can be queried in the same way as for sovereign chains. The relevant parameters for relaying are the ccvconsumer parameters in genesis which are derived from the governance proposal used to create the consumer chain in the first place. Here is an example (see historical_entries and unbonding_period):

    "ccvconsumer": {
      "params": {
        "enabled": true,
        "blocks_per_distribution_transmission": "1000",
        "distribution_transmission_channel": "",
        "provider_fee_pool_addr_str": "",
        "ccv_timeout_period": "2419200s",
        "transfer_timeout_period": "3600s",
        "consumer_redistribution_fraction": "0.75",
        "historical_entries": "1000",
        "unbonding_period": "1728000s"
      },

For relaying purposes (creating and updating clients, creating connections, etc) we need to use these parameters as they are the ones relevant to block production.
If the consumer chain is also a "democracy", i.e. it has its own native token, the chain will have an additional staking module. This is not the one hermes should use.

Proposal

  • add gRPC endpoint in ICS to retrieve the ccvconsumer parameters. This will be done in the interchain-security, (see issue) and partial blocker to proceed with the final implementation.
  • add ICS protos to ibc-proto-rs. This is a blocker now as we cannot change the SDK version to the custom one that ICS is using
  • use this new gRPC endpoint in hermes. This should be further discussed but high level we need to:
    • remove the unbonding_period parameter from config.toml
    • import the new ICS .proto file and release ibc-proto
    • add a new configuration flag to indicate a chain is consumer (vs. sovereign - default). The name is TBD. An alternative is to derive the chain type based on the Unimplemented return value of the new gRPC
    • call the new gRPC for consumer chains
    • write integration tests for ICS
  • coordinate release with cosmos hub

Acceptance Criteria

Be able to retrieve and use the correct parameters when creating clients such that connections with other chains can be established.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir ancazamfir added A: blocked Admin: blocked by another (internal/external) issue or PR A: okr Admin: tracks an ongoing OKR labels Feb 25, 2023
@romac romac added the O: interchain-security Objective: related to Interchain Security label Mar 7, 2023
@seanchen1991 seanchen1991 removed the A: okr Admin: tracks an ongoing OKR label Mar 15, 2023
@ancazamfir ancazamfir changed the title Hermes support for ICS Consumer chains Hermes uses wrong unbonding period or fails ICS Consumer chains Mar 16, 2023
@ancazamfir ancazamfir changed the title Hermes uses wrong unbonding period or fails ICS Consumer chains Hermes uses wrong unbonding period or fails for ICS Consumer chains Mar 16, 2023
@romac romac added this to the v1.5 milestone Apr 12, 2023
@seanchen1991 seanchen1991 removed the A: blocked Admin: blocked by another (internal/external) issue or PR label Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: interchain-security Objective: related to Interchain Security
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants