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

Failed to send localhost ibc transfer #3568

Closed
3 tasks
jtieri opened this issue May 8, 2023 · 6 comments · Fixed by #3587
Closed
3 tasks

Failed to send localhost ibc transfer #3568

jtieri opened this issue May 8, 2023 · 6 comments · Fixed by #3587
Assignees
Labels
09-localhost type: bug Something isn't working as expected
Milestone

Comments

@jtieri
Copy link
Member

jtieri commented May 8, 2023

Summary of Bug

We got localhost channel handshakes working with the go-relayer and I started to test ics-20 transfers but the cmd to initiate the transfer fails every time.

The cmd being executed:

simd tx ibc-transfer transfer transfer channel-0 cosmos1cdu2wky305myqfgpr5v47nuwkhrldhhycvfzal 1000stake --from cosmos1aw37tvhsvugtxz674smtxzkez9nlyyp305k3dg --gas-prices 0.0stake --gas-adjustment 1.2 --keyring-backend test --output json -y --home /var/cosmos-chain/simd --node tcp://chain-a-val-0-TestLocalhostIBC:26657 --chain-id chain-a

Error logs:

Error:  Received unexpected error:
        send ibc transfer: exit code 1:  Error: rpc error: code = NotFound desc = rpc error: code = NotFound desc = client-id: 09-localhost: consensus state not found: key not found

I'm using the ibc-go-simd docker image from GHCR built from the main branch.

Looks like the error is returned from here:

clientState, found := k.clientKeeper.GetClientState(ctx, connectionEnd.GetClientID())
if !found {
return 0, clienttypes.ErrConsensusStateNotFound
}

Expected Behaviour

Version

89ecf25

Steps to Reproduce

I have prepared a test case in the relayer repo here:
https://github.com/cosmos/relayer/blob/fb9b953cbc81e6c01774c719adbe35f220f809f8/interchaintest/localhost_client_test.go#L21


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega crodriguezvega added type: bug Something isn't working as expected 09-localhost labels May 12, 2023
@crodriguezvega crodriguezvega added this to the v7.1.0 milestone May 12, 2023
@crodriguezvega
Copy link
Contributor

Thank you for reporting this error, @jtieri. We will look into it, but I just wanted to share that we have an e2e test that makes an ICS20 transfer over the localhost client and it has been passing for us. The test doesn't use the golang relayer though; we broadcast all the handshake messages manually. But maybe we could do a bit of quick refactoring and plugin the relayer if you have a tag that we can use?

@damiannolan
Copy link
Member

damiannolan commented May 15, 2023

Hey @jtieri, I spent some time looking into this today. It looks like it was due to this absoluteTimeouts flag which does a consensus state lookup within the CLI handler itself and the error was being returned from here. I exec'd into the container while using a sleep in your testcase and noticed I wasn't even getting tx confirmation when trying to perform the CLI manually.

I modified the cmd to use something like this:

 simd tx ibc-transfer transfer transfer channel-0 cosmos1t5u0jfg3ljsjrh2m9e47d4ny2hea7eehxrzdgd 1000stake --from faucet --home /var/cosmos-chain/simd --chain-id chain-a --keyring-backend test --absolute-timeouts --packet-timeout-timestamp 9999999999999999999

And following that I can see a packet commitment in state:

root@chain-a-val-0-TestLocalhostIBC:/# simd q ibc channel packet-commitments transfer channel-0
commitments:
- channel_id: channel-0
  data: Dn0itB74YSMoUjShG/YuFfgVSIWUcFbKgEC1E8XKFQs=
  port_id: transfer
  sequence: "1"
height:
  revision_height: "794"
  revision_number: "0"
pagination:
  next_key: null
  total: "0"

I think we'll need to adapt the transfer CLI to accommodate the localhost client, as it doesn't use consensus states.
Nice catch, also the error you originally linked is incorrect so we can fix that too! 😆

@jtieri
Copy link
Member Author

jtieri commented May 15, 2023

Oh interesting. The error happens before the MsgTransfer is broadcasted so I don't think the issue should have to do with the relayer specifically. I took a look at the current main branch and it looked like everything was wired up correctly to use the simd Docker images for testing but perhaps I'm missing something. Would definitely appreciate y'all taking a look though!

I just created a branch on GH and there should be an accompanying Docker image now here

@jtieri
Copy link
Member Author

jtieri commented May 15, 2023

Hey @jtieri, I spent some time looking into this today. It looks like it was due to this absoluteTimeouts flag which does a consensus state lookup within the CLI handler itself and the error was being returned from here. I exec'd into the container while using a sleep in your testcase and noticed I wasn't even getting tx confirmation when trying to perform the CLI manually.

I modified the cmd to use something like this:

 simd tx ibc-transfer transfer transfer channel-0 cosmos1t5u0jfg3ljsjrh2m9e47d4ny2hea7eehxrzdgd 1000stake --from faucet --home /var/cosmos-chain/simd --chain-id chain-a --keyring-backend test --absolute-timeouts --packet-timeout-timestamp 9999999999999999999

And following that I can see a packet commitment in state:

root@chain-a-val-0-TestLocalhostIBC:/# simd q ibc channel packet-commitments transfer channel-0
commitments:
- channel_id: channel-0
  data: Dn0itB74YSMoUjShG/YuFfgVSIWUcFbKgEC1E8XKFQs=
  port_id: transfer
  sequence: "1"
height:
  revision_height: "794"
  revision_number: "0"
pagination:
  next_key: null
  total: "0"

I think we'll need to adapt the transfer CLI to accommodate the localhost client, as it doesn't use consensus states. Nice catch, also the error you originally linked is incorrect so we can fix that too! 😆

Ooops ignore my previous response, I typed up a reply and sent it without refreshing the page haha

Ahh thanks so much @damiannolan!! 🙏

@damiannolan
Copy link
Member

Nice! We have an issue (#3197) to pull in a relayer tag for e2e tests so we can pick that up soon

@jtieri
Copy link
Member Author

jtieri commented May 15, 2023

Awesome, I just confirmed that with your suggestions localhost ics-20 transfers are working with the relayer.
Going to add a few more test cases and then we will get that work merged into the main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
09-localhost type: bug Something isn't working as expected
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants