Add missing validation checks for all IBC message types

[Farhad-Shabani]

Closes: #233

Description

Following table represents a comprehensive list of the validations conducted on IBC domain messages in IBC-go.
It also reflects the current status of these checks and briefly outlines the measures taken to fix any erroneous or missing validation in IBC-rs. Also, it should be noted; to make some checks clearer, a few improvements are made around some of these checks and the relevant errors.

Status: ✅ Existed - ❌ Missing, wrong or partially checked - ⚠️ Not Applicable

Status - Basic Validation in IBC-go	Description / Action
All IBC Messages
❌ Signer must be parsed as a valid address	Fixed in #619
MsgCreateClient
✅ Unpack ClientState	Done through decode_client_state
✅ Validate ClientState	Checked upon constructing a ClientState
❌ Unpack ConsensusState	Added to validate()
⚠️ ClientType match for ClientState and ConsensusState	ConsensusState does not contain ClientType
❌ClientType meet identifier constraints	Fixed in #639
ConsensusState
✅ Root cannot be None	ConsensusState::try_from
❌ CommitmentRoot cannot be empty	Added to ConsensusState::try_from
✅ Next validators hash must be valid	ConsensusState::try_from
✅ Timestamp must be a positive Unix time	ConsensusState::try_from
MsgUpdateClient
✅ Unpack ClientMessage
✅ SignedHeader cannot be nil	TmHeader::try_from
✅ SignedHeader basic validations	SignedHeader::try_from of tendermint-rs
✅ TmHeader cannot be nil	TmHeader::try_from
✅ Header be a Tendermint header	TmHeader::try_from
✅ ValidatorSet cannot be nil	TmHeader::try_from
✅ ValidatorSet is tendermint validator set	TmHeader::try_from
✅ Match height revisions of Header	Moved to validate_basic of TmHeader
✅ TrustedHeight < Header.height() for updates	Moved to validate_basic of TmHeader
❌ Current validators match hashes	Added to validate_basic of TmHeader
✅ Trusted next validators match hashes	Moved to validate_basic of TmHeader
✅ Misbehaviour headers' height > 0	Misbehaviour::try_from
✅ Misbehaviour headers cannot be empty	Misbehaviour::try_from
✅ Misbehaviour trusted vals cannot be empty	Misbehaviour::try_from
❌ Misbehaviour headers' basic validation	Added to validate_basic of Misbehaviour
✅ Misbehaviour headers have same chain Ids	Moved to validate_basic of Misbehaviour
✅ Misbehaviour header1 height > header2 height	Moved to validate_basic of Misbehaviour
❌ Misbehaviour ClientId be valid	Fixed in #639
✅ Misbehaviour valid BlockId, Commit	Header::try_from of tendermint-rs
MsgUpdateClient
✅ Unpack ClientState	TmClientState::try_from
✅ Unpack ConsensusState	TmConsensusState::try_from
⚠️ ClientType match for ClientState and ConsensusState	ConsensusState does not contain ClientType
✅ Proof of upgrade client cannot be empty	CommitmentProofBytes::try_from
✅ Proof of upgrade consensus must be valid	MsgUpgradeClient::try_from
❌ ClientId be valid	Fixed in #639
MsgConnectionOpenInit
❌ ClientIds be valid	Fixed in #639
❌ Counterparty ConnectionId must be empty	Added to try_from, though internally set to None
✅ Counterparty prefix cannot be empty	CommitmentPrefix::try_from
✅ Version cannot be nil	Fixed in #626
✅ Version identifier cannot be blank	Fixed in #626
✅ Version features be valid	Fixed in #626
MsgConnectionOpenTry
❌ ClientIds be valid	Fixed in #639
✅ Counterparty ConnectionId must be valid	Improved ics24_host::identifier
✅ Unpack & Validate ClientState	Done in validate_self_tendermint_client
✅ Non empty counterparty versions	Fixed in #626
✅ Version features be valid	Fixed in #626
✅ Proof of conn_end_on_a cannot be empty	CommitmentProofBytes::try_from
✅ Proof of client & cons states cannot be empty	CommitmentProofBytes::try_from
✅ Consensus height non-zero	Height::try_from
✅ Counterparty prefix cannot be empty	CommitmentPrefix::try_from
MsgConnectionOpenAck
✅ ConnectionId must be valid	Improved ics24_host::identifier
✅ Unpack & Validate ClientState	Done in validate_self_tendermint_client
✅ Validate Version	Fixed in #626
✅ Proof of conn_end_on_b cannot be empty	CommitmentProofBytes::try_from
✅ Proof of client & cons states cannot be empty	CommitmentProofBytes::try_from
✅ Consensus height non-zero	Height::try_from
✅ Counterparty prefix cannot be empty	CommitmentPrefix::try_from
MsgConnectionOpenConfirm
✅ ConnectionId must be valid	Improved ics24_host::identifier
✅ Proof of conn_end_on_a cannot be empty	CommitmentProofBytes::try_from
All Channel Handshake Messages
❌ UNINITIALIZED state is invalid	Fixed in ChannelEnd::try_from
❌ None ordering is invalid	Fixed in ChannelEnd::try_from
✅ Only supports one connection hop	Moved to ChannelEnd::try_from
✅ Each connection hop must have valid conn Id	Improved ics24_host::identifier
✅ Counterparty identifiers must be valid	Improved ics24_host::identifier
MsgChannelOpenInit
✅ PortIds must be valid	PortId::from_str
❌ Channel state must be INIT	Added to try_from, though internally set to INIT
❌ Counterparty ChannelId must be empty	Added to try_from, though internally set to None
MsgChannelOpenTry
✅ PortIds must be valid	PortId::from_str
❌ Channel state must be TRYOPEN	Added to MsgChannelOpenTry::try_from
❌ previous ChannelId must be empty	Added to MsgChannelOpenTry::try_from
✅ Both ChannelIds must be non-empty valid	Improved ics24_host::identifier
✅ Proof of chan_end_on_a cannot be empty	CommitmentProofBytes::try_from
MsgChannelOpenAck & MsgChannelOpenConfirm
MsgChannelCloseInit & MsgChannelCloseConfirm
✅ PortIds must be valid	PortId::from_str
✅ ChannelIds must be valid	Improved ics24_host::identifier
✅ Proof of chan_ends cannot be empty	CommitmentProofBytes::try_from
Packet
✅ PortIds must be valid	PortId::from_str
✅ ChannelIds must be valid	Improved ics24_host::identifier
✅ Packet sequence cannot be 0	Packet::try_from
✅ Packet data bytes cannot be empty	Packet::try_from
❌ Timeout height and timestamp cannot both be 0	Packet::try_from
MsgRecvPacket
✅ PortIds must be valid	PortId::from_str
✅ ChannelIds must be valid	Improved ics24_host::identifier
✅ Commitment proof cannot be empty	CommitmentProofBytes::try_from
MsgAcknowledgement
✅ Acknowledgement proof cannot be empty	CommitmentProofBytes::try_from
✅ Ack bytes cannot be empty	CommitmentProofBytes::try_from
MsgTimeout
❌ Next sequence receive cannot be 0	MsgTimeout::try_from
✅ Unreceived proof cannot be empty	CommitmentProofBytes::try_from
MsgTimeoutOnClose
❌ Next sequence receive cannot be 0	MsgTimeout::try_from
✅ unreceived proof cannot be empty	CommitmentProofBytes::try_from
✅ Proof of closed cparty chan end cannot be empty	CommitmentProofBytes::try_from

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests.
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).
• Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• Reviewed Files changed in the GitHub PR explorer.
• Manually tested (in case integration/unit/mock tests are absent).

[codecov]

Codecov Report

Patch coverage: 74.93% and project coverage change: +0.12 🎉

Comparison is base (dc54640) 72.97% compared to head (7f65497) 73.09%.

❗ Current head 7f65497 differs from pull request most recent head cb71f2c. Consider uploading reports for the commit cb71f2c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #650      +/-   ##
==========================================
+ Coverage   72.97%   73.09%   +0.12%     
==========================================
  Files         127      127              
  Lines       15696    15625      -71     
==========================================
- Hits        11454    11421      -33     
+ Misses       4242     4204      -38     

Impacted Files	Coverage Δ
crates/ibc/src/applications/transfer/error.rs	13.63% <0.00%> (+1.13%)	⬆️
...tes/ibc/src/applications/transfer/msgs/transfer.rs	50.49% <0.00%> (+3.70%)	⬆️
crates/ibc/src/clients/ics07_tendermint/error.rs	19.04% <ø> (ø)
crates/ibc/src/core/context.rs	83.51% <ø> (ø)
crates/ibc/src/core/handler.rs	75.00% <ø> (ø)
crates/ibc/src/core/ics03_connection/error.rs	7.69% <ø> (ø)
crates/ibc/src/mock/client_state.rs	90.17% <ø> (ø)
crates/ibc/src/core/ics03_connection/connection.rs	39.11% <40.74%> (+4.13%)	⬆️
...s/ibc/src/clients/ics07_tendermint/misbehaviour.rs	66.01% <47.36%> (+0.11%)	⬆️
crates/ibc/src/core/ics24_host/identifier.rs	60.37% <52.63%> (+3.29%)	⬆️
... and 45 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.