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

Support localhost client/connection in conformance test #671

Merged
merged 4 commits into from
Aug 15, 2023
Merged

Support localhost client/connection in conformance test #671

merged 4 commits into from
Aug 15, 2023

Conversation

migueldingli1997
Copy link
Contributor

Closes #670

Quite a quick-and-dirty fix, if I'm being honest. Open to other ways of tackling this.

@migueldingli1997 migueldingli1997 requested a review from a team as a code owner July 17, 2023 12:51
@migueldingli1997
Copy link
Contributor Author

I've just noticed that the scope of this is a bit larger than I expected, since interchaintest also makes use of an old version of the Go relayer, which does not seem to be compatible with the localhost client either.

@boojamya
Copy link
Contributor

Hey @migueldingli1997, this fix looks good to me.
But you're right, we still need a relayer that has localhost support. Without it, I'm seeing this error:

"no concrete type registered for type URL /ibc.lightclients.localhost.v2.ClientState against interface *exported.ClientState"

We plan to cut a relayer release this week.
Then we can update the relayers DefaultContainerVersion in cosmos_relayer.go.

I've confirmed that conformance will pass with a relayer that supports localhost with this matrix file:

{
  "Relayers": ["rly"],

  "ChainSets": [
    [
      {
        "Name": "ibc-go-simd",
        "Version": "v7.2.0"
      },
      {
        "Name": "ibc-go-simd",
        "Version": "v7.2.0"
      }
    ]
  ]
}

Thanks for the PR, we'll get this merged soon.

@gjermundgaraba
Copy link
Contributor

gjermundgaraba commented Aug 8, 2023

Btw: just tested this change with the latest version of rly (2.4.0) and it works for IBC 7.2 with localhost client. Would love to see this merged with the latest version of rly included 🙏

@boojamya boojamya added BACKPORT backport into all maintained branches and removed BACKPORT backport into all maintained branches labels Aug 15, 2023
@boojamya boojamya enabled auto-merge (squash) August 15, 2023 22:42
@boojamya boojamya merged commit f1b0d44 into strangelove-ventures:main Aug 15, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conformance test does not support localhost client
3 participants