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

IBC ConnectionOpenAck fails for solo machine client #119

Closed
4 tasks
devashishdxt opened this issue Apr 13, 2021 · 3 comments · Fixed by #120 or #153
Closed
4 tasks

IBC ConnectionOpenAck fails for solo machine client #119

devashishdxt opened this issue Apr 13, 2021 · 3 comments · Fixed by #120 or #153
Assignees
Labels
06-solomachine type: bug Something isn't working as expected
Milestone

Comments

@devashishdxt
Copy link

Summary of Bug

After creating an IBC solo machine client, the connection handshake fails at ConnectionOpenAck.

For a solo machine, in MsgConnectionOpenAck, we send proof_height, proof_try, proof_client and proof_consensus. On cosmos-sdk side, this function is called to verify all the details of the message. For solo machine, proof_try contains signed connection data, proof_client contains signed client state of tendermint client on solo machine, proof_consensus contains signed consensus state of tendermint client on solo machine and proof_height is the height of the solo machine at which these proofs were created.

Each of these proofs are verified by cosmos chain in these functions:

All these functions are also supplied with proof_height which are verified against the sequence stored in solo machine client state on cosmos chain. But, all the above three functions increment the sequence stored in solo machine client state. here, here and here.

The verification goes fine for the first verification function which is called, i.e., VerifyConnectionState (which increments the sequence in client state). Verification fails in VerifyClientState here because VerifyConnectionState changes the sequence/revision height in client state.

Version

v0.42.4

Steps to Reproduce

Here is the code:

To run the solo machine setup:

  • Start IBC enabled cosmos-sdk chain (with replaced custom cosmos-sdk fork).
  • Start solo machine server cargo run -- start (If you're running cosmos chain locally, all the defaults are set and you don't have to provide any values when starting)
    Next, send AddChainRequest (use bloom RPC to send grpc requests). The defaults are set in a way that you don't have to provide any values if you're running the chain locally.

Finally, to establish connection, call grpc Connect. This will create a client, connection on both cosmos-sdk and solo machine and fails when sending acknowledgement of connection to cosmos-sdk.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@colin-axner colin-axner transferred this issue from cosmos/cosmos-sdk Apr 14, 2021
@colin-axner
Copy link
Contributor

colin-axner commented Apr 14, 2021

Thanks for opening the issue and providing a lot of information! We need to change the solo machine verify functions to be a pointer so that incrementing the sequence can be referenced in the call to the client state verify function

@colin-axner colin-axner added the type: bug Something isn't working as expected label Apr 14, 2021
@colin-axner colin-axner self-assigned this Apr 14, 2021
@colin-axner colin-axner added this to the 1.0.0 milestone Apr 14, 2021
@colin-axner
Copy link
Contributor

We won't be backporting this, so the fix won't be included until the first release of the IBC module on this repository (3-4 weeks)

@AdityaSripal
Copy link
Member

Thanks for the writeup!

@colin-axner colin-axner reopened this Apr 21, 2021
@colin-axner colin-axner modified the milestones: 1.0.0, 1.0.1 Apr 21, 2021
@crodriguezvega crodriguezvega modified the milestones: 1.1.0, 1.0.0 Sep 17, 2021
faddat referenced this issue in notional-labs/ibc-go Mar 1, 2022
…ts and the last week of work

* Add order to path end and make gen path reuse existing identifiers

* Fixed query.go

Removed misplaced return in case of both `UnmarshalBinaryLengthPrefixed` and `UnmarshalBinaryBare` failing. 
Previously if `UnmarshalBinaryLengthPrefixed` failed but `UnmarshalBinaryBare` did not, `nil, qClntConsStateErr(err)` would have still be returned instead of going on with the normal code execution

* Fix packet acks! 🎉 💃 🚀 🌔

* Update to sdk master

* feat: initial cut at rly tx pkt

* fix: get past MsgSendPacket encoding problem

* feat: get working with SwingSet

* fix: send absolute timeouts

* feat: also start each relayer after link succeeds

* feat: allow multiple paths for the same connection

This works, modulo bugs.

* Fix a couple of small issues

* Add three-chainz, debugging for case

* Update to capabilites fix

* WIP scripts cleanup

* update to latest sdk and fix issues

* Update relayer/pathEnd.go

Co-Authored-By: Segue <[email protected]>

* Merge PR #119: feat: add agoric-solo to start client machines for nchainz

* Enforce relayer channels and ports in streaming relayer and unrelayed

* Update to latest SDK

Co-authored-by: Riccardo Montagnin <[email protected]>
Co-authored-by: Michael FIG <[email protected]>
Co-authored-by: Segue <[email protected]>
Co-authored-by: Michael FIG <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
06-solomachine type: bug Something isn't working as expected
Projects
None yet
4 participants