ICS4: Suggestions for code improvement in channel handshake related code

[milosevic]

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

• Basic validation
  of MsgChanOpenTry contains business logic. The same check is done also here.

• in ChanOpenTry function, found at line
  is the same as found at line,
  so the latter is redundant. As found variable is overwritten several times, renaming of this variable
  is needed to simplify the logic.

• Using "" as a special value
  (see line)
  seems like bad practice. It would probably be better defining it as a constant.

• in function
  it might be better passing connectionId or conectionEnd as it already fetched at the calling point (for example
  see line)
  instead of channel. Then line can be removed.

[crodriguezvega]

Closing the issue since the comments does not seem to be applicable anymore. Besides, the issue refers to the implementation and not to the spec.

• Basic validation
  of MsgChanOpenTry contains business logic. The same check is done also here.

The check of the counterparty channel ID is now only done in ValidateBasic and it checks that it is a valid identifier.

• in ChanOpenTry function, found at line
  is the same as found at line,
  so the latter is redundant. As found variable is overwritten several times, renaming of this variable
  is needed to simplify the logic.

found is only used now once in ChanOpenTry when retrieving the connection end.

• Using "" as a special value
  (see line)
  seems like bad practice. It would probably be better defining it as a constant.

Checking for empty strings is done like this all over the codebase and seems to be pretty standard in Go.

• in function
  it might be better passing connectionId or conectionEnd as it already fetched at the calling point (for example
  see line)
  instead of channel. Then line can be removed.

This code does not exist anymore.

[gjermundgaraba]

This issue was mistakenly reopened during some integration work, so closing again now.