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

Cannot configure multiple chains with the same chain_id #1353

Open
5 tasks
michaelfig opened this issue Sep 15, 2021 · 6 comments
Open
5 tasks

Cannot configure multiple chains with the same chain_id #1353

michaelfig opened this issue Sep 15, 2021 · 6 comments
Labels
A: question Admin: further information is requested O: new-feature Objective: cause to add a new feature or support O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@michaelfig
Copy link

Crate

ibc-relayer (hermes)

Summary of Bug

The .hermes/config.toml file cannot express two different chains with the same chainId.

Version

v0.7.0

Steps to Reproduce

  1. create two [[chain]] entries that have the same id = 'agoricstage-14' field, but different RPC servers, etc.
  2. run hermes config validate
  3. results:
$ hermes config validate
Sep 14 20:37:10.378  INFO using default configuration from '/Users/michael/.hermes/config.toml'
error: hermes fatal error: config error: 
   0: config file has duplicate entry for the chain with id agoricstage-14

Location:
   /Users/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/flex-error-0.4.2/src/tracer_impl/eyre.rs:10

Backtrace omitted.
Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Acceptance Criteria

Chain configuration should provide some other field (possibly defaulting to the id field's value) that can be used to disambiguate two different chains that happen to have the same chainID.

That new field should be the name used for the entries in the .hermes/keys directory, and any other persisted state, so that the two chains with the same chainID can function independently.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@michaelfig
Copy link
Author

Another solution could be to introduce a chain_id value that defaults to the id value, but can specify a different chainID than the id unique key.

@adizere
Copy link
Member

adizere commented Sep 15, 2021

Hi Michael, thanks for detailing the issue!

Hermes uses the chain-id from config.toml extensively in CLIs. So for instance when a user runs hermes query clients cosmoshub-4, Hermes will uniquely map this query to the full node configured in the (unique) entry called cosmoshub-4 from the config.toml. Having two of these entries will make it ambiguous, and it will no longer be possible to express CLIs in the way we currently do. Does this rationale make sense?

If you explain your use-case in more detail, maybe we can find a workaround.

@adizere adizere added the A: question Admin: further information is requested label Sep 15, 2021
@michaelfig
Copy link
Author

Hi Adi,

Having two of these entries will make it ambiguous, and it will no longer be possible to express CLIs in the way we currently do. Does this rationale make sense?

I mean the id should default to the chain_id, but it should be possible to supply a different id using the same chain_id. Then users use the id value to interact with Hermes, even if the chain_id values are different.

If you explain your use-case in more detail, maybe we can find a workaround.

Will do:

If a chain has a chain_id that some other chain uses, how does Hermes know which of the chains is the one in the config.toml? If Hermes has a way to do that (such as comparing light client proofs), then I don't see why having two different entries with the same chain_id would be a problem.

If Hermes cannot distinguish between the two chains, then that seems like a (potentially security) bug. It's not robust to assume that all chain_ids are unique.

My use case is if there is are non-unique chain_ids, I don't want to force one of the chains to hard-fork to a different chain_id just to be able to relay the different chains. Having two separate Hermes instances with different configs would work around this, but then there's not a single command to use to manage the relayer. And both instances need similar disambiguation support to know which of the shared chain_ids they are supposed to relay.

@romac romac changed the title Hermes configured chains' chainID should not be assumed to be unique Cannot configure multiple chains with the same chain_id Sep 16, 2021
@adizere
Copy link
Member

adizere commented Sep 16, 2021

Thanks for the additional details, Michael, in particular the use-case description is very helpful.

Changing the the way Hermes uses the config.toml would involve some non-trivial changes in the way we do logging/telemetrics and potentially the runtime.

How important and urgent is this requirement for non-unique chain identifiers, Michael?

We will check with other Hermes users and see if there is value in this change (even beyond what we discussed here). We are mostly focusing on trying to keep up with bug reports and general robustness issues, but you are raising an important user experience problem here.

@michaelfig
Copy link
Author

How important and urgent is this requirement for non-unique chain identifiers, Michael?

For me, it is of medium importance (something that should eventually be addressed) but not urgent. I just saw this when trying to understand the meaning of the id field.

Thanks for listening, and please triage as you see fit.

@adizere
Copy link
Member

adizere commented Sep 17, 2021

Adding here some meeting notes after discussing with Cephalopod, which has potentially similar requirements but coming from a different angle.

There may be some value in allowing the same chain to appear twice in the config.toml,

  • where the chain is represented via two different full nodes, and more importantly: different wallet address
  • concrete use-case for production: helpful to distinguish e.g., between relaying for Osmosis UI and relaying for Emeris UI -- we may use different full nodes, different channels, different wallets, among the same two chains.

Two caveats:

  1. maybe better would be to group together in the same chain entries (though they may have diff. wallets or full node addresses which would be used as ingress points on the same source chain) within the same ``[[chains]]` entry, towards reducing the number of entries that the relayer operator has to keep track, i.e., grouping of multiple ingress resources per-chain

  2. each different ingress resource for a chain should probably have channel filtering enabled, so that the relayer won't attempt to reuse the same channel

Just to emphasize that the notes above here refer to allowing same chain having different contact points as opposed to different chains with the same identifier. Though the fix/consequence may be the same. This needs more thinking through, with concrete user-flows.

@adizere adizere added this to the 11.2021 milestone Sep 17, 2021
@adizere adizere added O: new-feature Objective: cause to add a new feature or support O: usability Objective: cause to improve the user experience (UX) and ease using the product labels Nov 2, 2021
@adizere adizere modified the milestones: 11.2021, 02.2022, Backlog Nov 2, 2021
@adizere adizere modified the milestones: Backlog, v2 May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: question Admin: further information is requested O: new-feature Objective: cause to add a new feature or support O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

No branches or pull requests

2 participants