add-paths nil pointer dereference

[michaelfig]

Hi, when I try adding a new path, I get the following error:

$ rly config add-paths nchainz/config/paths/
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4edf3bf]

goroutine 1 [running]:
github.com/cosmos/relayer/relayer.(*Chain).QueryClientState(0xc000f5c240, 0x702, 0x9, 0x40, 0x0)
        /Users/michael/agoric/forks/relayer/relayer/query.go:130 +0x7f
github.com/cosmos/relayer/cmd.(*Config).ValidateClient(0xc000558780, 0xc000f5c240, 0x702, 0xc000f815e0, 0x4, 0xc000f5c240)
        /Users/michael/agoric/forks/relayer/cmd/config.go:551 +0x75
github.com/cosmos/relayer/cmd.(*Config).ValidatePathEnd(0xc000558780, 0xc000f815e0, 0x20, 0x4fe0ae0)
        /Users/michael/agoric/forks/relayer/cmd/config.go:517 +0x1c5
github.com/cosmos/relayer/cmd.(*Config).ValidatePath(0xc000558780, 0xc000fbecc0, 0x51deeb5, 0x1)
        /Users/michael/agoric/forks/relayer/cmd/config.go:479 +0x5c
github.com/cosmos/relayer/cmd.cfgFilesAddPaths(0x7ffeefbff42d, 0x14, 0xc000e5fd50, 0x0, 0x0)
        /Users/michael/agoric/forks/relayer/cmd/config.go:287 +0x4e5
github.com/cosmos/relayer/cmd.configAddPathsCmd.func1(0xc000f1d080, 0xc000f09e30, 0x1, 0x1, 0x0, 0x0)
        /Users/michael/agoric/forks/relayer/cmd/config.go:206 +0x45
github.com/spf13/cobra.(*Command).execute(0xc000f1d080, 0xc000f09e10, 0x1, 0x1, 0xc000f1d080, 0xc000f09e10)
        /Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:850 +0x47c
github.com/spf13/cobra.(*Command).ExecuteC(0xc000f1c2c0, 0x0, 0x0, 0x0)
        /Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:958 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        /Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:895
github.com/cosmos/relayer/cmd.Execute()
        /Users/michael/agoric/forks/relayer/cmd/root.go:116 +0x3d
main.main()
        /Users/michael/agoric/forks/relayer/main.go:21 +0x25

The nchainz/config/paths directory contains only a single file, path-1.json, whose contents are:

{
  "src": {
    "chain-id": "ibc0",
    "client-id": "07-tendermint-0",
    "connection-id": "connection-0",
    "port-id": "port-1",
    "order": "ordered",
    "version": "echovsn"
  },
  "dst": {
    "chain-id": "ibc1",
    "client-id": "07-tendermint-0",
    "connection-id": "connection-0",
    "port-id": "echo",
    "order": "ordered",
    "version": "ics20-1"
  },
  "strategy": {
    "type": "naive"
  }
}

Output from rly config list (note that path-0 is currently being streaming relayed, but I've reproduced the failure even when the relayer is stopped):

global:
  api-listen-addr: :5183
  timeout: 10s
  light-cache-size: 20
chains:
- key: testkey
  chain-id: ibc0
  rpc-addr: http://localhost:26657
  account-prefix: agoric
  gas-adjustment: 1
  gas-prices: ""
  trusting-period: 336h
- key: testkey
  chain-id: ibc1
  rpc-addr: http://localhost:26557
  account-prefix: agoric
  gas-adjustment: 1
  gas-prices: ""
  trusting-period: 336h
paths:
  path-0:
    src:
      chain-id: ibc0
      client-id: 07-tendermint-0
      connection-id: connection-0
      channel-id: channel-0
      port-id: transfer
      order: unordered
      version: ics20-1
    dst:
      chain-id: ibc1
      client-id: 07-tendermint-0
      connection-id: connection-0
      channel-id: channel-0
      port-id: transfer
      order: unordered
      version: ics20-1
    strategy:
      type: naive

[michaelfig]

Turns out the c.PathEnd in c.QueryClientState(height) on relayer/query.go:130 is nil.

If I stop the chains, then I get:

soil:relayer michael$ rly config add-paths nchainz/config/paths/
Error: failed to validate path nchainz/config/paths/path-1.json: chain ibc0 failed path validation: post failed: Post "http://localhost:26657": dial tcp 127.0.0.1:26657: connect: connection refused [/Users/michael/agoric/forks/relayer/cmd/config.go:480]
soil:relayer michael$

[colin-axner]

Thanks for the write up. Will look into it now.

Are you using a custom ICS20 implementation? If I recall correctly, both versions need to be ics20-1

[colin-axner]

Looks like there are multiple issues with path validation. I'll see if I can rework it and post a fix

[colin-axner]

Also, this is not good news, but I don't think the relayer can support what you are trying to do. From my understanding of the relayer architecture, there can only be one chain of a certain chain-id and only one path end for that chain. Obviously this makes the golang relayer pretty limited and this is a completely reasonable request, but the way it is designed prevents an easy fix.

The best I could try to do is rework the Config internals to support multiple Chain instances even if they have the same chain-id. I cannot guarantee this won't lead to more bugs, but it seems like the only viable option atm. The question is what set of parameters distinguish a Chain instance, I guess a combination of the chain-id and the path end.

Is this the last feature you need from the golang relayer? This is starting to enter the territory of complexity that might be better off with a redesign of the relayer

It might take me some time to work through a fix that doesn't break things too much

Edit: the relayer can support this, I found that it already has a hacky fix, ChainsFromPath sets the path end before running the core relayer functions. Seems like a risky mechanism since if you have 2 path ends and you don't call this function, the default path end would be used. This could result in a user sending funds via the wrong channel!

[michaelfig]

Is this the last feature you need from the golang relayer? This is starting to enter the territory of complexity that might be better off with a redesign of the relayer

Unfortunately, things seemed to work properly before the bottom-up validation was implemented. So I see it mainly as a regression, and without this basic use case working (and since the Hermes/Rust relayer is not yet ready) I can't deploy cosmos-sdk v0.41.0 in good faith.

The feature I need is to configure multiple paths for a single connection, and start/stop a separate relayer instance independently for those paths. That was working in the relayer that used cosmos-sdk v0.40.0-rc4.

Edit: the relayer can support this, I found that it already has a hacky fix, ChainsFromPath sets the path end before running the core relayer functions.

Yeah, that was my experience before the validation.

Seems like a risky mechanism since if you have 2 path ends and you don't call this function, the default path end would be used. This could result in a user sending funds via the wrong channel!

Hmm. This should be made better with first-class connections.

Thanks for your help, I'll test it now.