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

Parametrize chain type via configuration file #1630

Closed
5 of 9 tasks
adizere opened this issue Dec 1, 2021 · 5 comments
Closed
5 of 9 tasks

Parametrize chain type via configuration file #1630

adizere opened this issue Dec 1, 2021 · 5 comments
Labels
E: non-cosmos External: related to non-Cosmos chains I: CLI Internal: related to the relayer's CLI I: configuration Internal: related to Hermes configuration O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@adizere
Copy link
Member

adizere commented Dec 1, 2021

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

Related to: #1318 and cosmos/ibc-rs#67


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added I: CLI Internal: related to the relayer's CLI O: usability Objective: cause to improve the user experience (UX) and ease using the product I: configuration Internal: related to Hermes configuration E: non-cosmos External: related to non-Cosmos chains labels Dec 1, 2021
@adizere adizere added this to the v1.0.0 milestone Dec 1, 2021
@hu55a1n1
Copy link
Member

hu55a1n1 commented Dec 2, 2021

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.)

@mzabaluev
Copy link
Contributor

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.

@ancazamfir
Copy link
Collaborator

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

@adizere
Copy link
Member Author

adizere commented Dec 6, 2021

For completeness: we deleted the proof_specs here ce01aa9.

@adizere adizere modified the milestones: v1.0.0, v1.2 May 27, 2022
@romac
Copy link
Member

romac commented Oct 12, 2022

I think this was done in #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 romac closed this as completed Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E: non-cosmos External: related to non-Cosmos chains I: CLI Internal: related to the relayer's CLI I: configuration Internal: related to Hermes configuration O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

No branches or pull requests

5 participants