hermes icon indicating copy to clipboard operation
hermes copied to clipboard

Parametrize chain type via configuration file

Open adizere opened this issue 3 years ago • 4 comments

Crate

ibc-relayer and ibc-relayer-cli

Summary

We're adapting the relayer and ibc modules to support interoperability with non-SDK chains. In this context, we may want to allow operators to select the type of chain which they are configuring.

For example,

[[chains]]
id = 'ibc-1'
# ...

would turn into

[[chains]]
id = 'ibc-1'
type = 'cosmos' # can be 'cosmos' | 'substrate' | 'mock' | 'basecoin' ...
# ...

Problem Definition

I think there are two problems we're having at the moment.

  1. For all Hermes CLI commands, we hardcode the type of chain runtime to be "CosmosSdkChain".

https://github.com/informalsystems/ibc-rs/blob/02776c879b203510f9d1b38b9f643639221b163c/relayer-cli/src/cli_utils.rs#L69-L70

This will have to change, and it's not clear yet how to parametrize it dynamically. Specifying the chain type in the config.toml could be a solution.

  1. Relayer operators define the type of proofs directly in the config.toml:

https://github.com/informalsystems/ibc-rs/blob/02776c879b203510f9d1b38b9f643639221b163c/config.toml#L228-L232

The proof specification that a network uses is a low-level concern, yet it is very important when creating / manipulating IBC clients and the corresponding proofs. It's unlikely that relayer operators know how to configure the proof specs, and unclear if this should be a configuration option, since the proof type is a property of the chain type.

Acceptance Criteria

  • [x] #2240
    • provides initial support for different chain types
  • [x] add support to parametrize the chain type that is spawned throughout all Hermes CLI commands
  • [x] remove the proof_spec option in the config.toml with a higher level option called type (or similar name)
    • done in https://github.com/informalsystems/ibc-rs/commit/ce01aa9b4db0ca706b392b77f2c0251d496ed00f
  • [ ] Add support to query proof specs from a chain (the specs that should be used upon creating a new client)
    • depends on https://github.com/cosmos/ibc-go/issues/962

Related to: #1318 and #1561


For Admin Use

  • [X] Not duplicate issue
  • [X] Appropriate labels applied
  • [ ] Appropriate milestone (priority) applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned

adizere avatar Dec 01 '21 14:12 adizere

I really like the idea of simplifying the config with chain types. 👍 So IIUC, these chain types are like templates with predefined values for certain config options, right? Just want to point out that some chain types might require additional input from users for config options that are specific to that chain, e.g. Ethermint's address_type.proto_type. So we should keep this in mind. (We might also want to look at ibc-rs forks that have been modified to add more of such chain specific config opts.)

hu55a1n1 avatar Dec 02 '21 07:12 hu55a1n1

some chain types might require additional input from users for config options that are specific to that chain

Just thinking aloud and I never tried it, but this might be solved by adding an extension point in the config, which would produce/accept arbitrary bags of serde wrapped into Box<Any> that will be delegated to chain plugins to handle.

mzabaluev avatar Dec 02 '21 09:12 mzabaluev

I don't think proof-spec should be in the config, definitely not initially. Can we keep them hardcoded for now?

ancazamfir avatar Dec 06 '21 15:12 ancazamfir

For completeness: we deleted the proof_specs here https://github.com/informalsystems/ibc-rs/pull/1629/commits/ce01aa9b4db0ca706b392b77f2c0251d496ed00f.

adizere avatar Dec 06 '21 17:12 adizere

I think this was done in https://github.com/informalsystems/hermes/pull/2228, although we still use the ProofSpecs defined in the config or default to the Cosmos proof specs if not specified, rather than a hardcoded value per chain type. Might be worth opening a new issue to follow up on this.

@adizere Feel free to re-open if you'd rather keep discussing here.

romac avatar Oct 12 '22 12:10 romac