ICS3: Inconsistencies in the spec and code of Connection Handshake

[istoilkovska]

Surfaced from Informal Systems IBC Audit of cosmos-sdk hash cosmos/cosmos-sdk@fe8a891.

• [ ] In connOpenInit() and connOpenTry(), there is no error handling in the spec when calling addConnectionToClient(). In the code, the transaction is aborted if the client does not exist.

• [ ] In the code of connOpenTry(), but not in the spec:

  • the client parameters are validated (by calling ValidateSelfClient())
  • getting a consensus state from a given height aborts on error
  • there is error handling when picking a version
  • the client state is verified, in addition to the client consensus state

• [ ] in the code of connOpenAck(), but not in the spec:

  • there is error handling when getting the connection end by ID from the provable store
  • the client parameters are validated (by calling ValidateSelfClient)
  • getting a consensus state from a given height aborts on error
  • the client state is verified, in addition to the client consensus state

• [ ] in the spec, when handling a ConnOpenAck datagram in case the connection end is in INIT, it is checked whether the version field of the datagram is in the list of versions stored in the connection end. In the code, it is checked whether the version field from the datagram is in the list of supported versions, not if it is in the list of versions stored in the connection end

• [ ] in the code of ConnOpenConfirm(), but not in the spec:

  • there is error handling when getting the connection end by ID from the provable store