Fix maybe issues related to frozen height of tendermint client.

[riversyang]

Description

In the development of near-ibc, we encountered two significant issues concerning the handling of the frozen height in the Tendermint client.

The first issue arises in the try_from function within impl TryFrom<RawTmClientState> for ClientState. When the client is frozen, this function returns an Err, leading to a failure in the decode function of the tendermint_proto crate. Consequently, if the client becomes frozen, it becomes impossible to restore the client state using the decode function from tendermint_proto, rendering the client state irreparable.

The second issue is found in the from function of impl From<ClientState> for RawTmClientState. This function incorrectly sets the frozen height to Some(...) even when the original client state has it as None. This appears to be a straightforward oversight.

The modifications in this pull request have been applied in near-ibc v1.0.2. Given that implementations external to the Tendermint client already check for a frozen client state, the current solution functions effectively. However, I believe it is important to highlight these issues in ibc-rs for further attention and resolution.

---

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).

[Farhad-Shabani]

Actually, The current logic is accurate as it stands:

The first issue arises in the try_from function within impl TryFrom for ClientState.... Consequently, if the client becomes frozen, it becomes impossible to restore the client state using the decode function from tendermint_proto, rendering the client state irreparable.

In terms of not allowing ClientState construction with a frozen height, this is a necessary check, since we shouldn’t allow create/update clients with a frozen_height > 0 set.

For the client recovery, we need additional appropriate mechanism enabling hosts to restore their client states. Not only in frozen height cases but also when a client state expires. (we have this ticket)

By the way, this got me thinking we might take this check out of the TryFrom, and have it in a more appropriate validation method or handlers, so letting users obtain domain ClientState type from proto even with frozen heights, but still, would not let to create or update clients with frozen heights.

The second issue is found in the from function of impl From for RawTmClientState. This function incorrectly sets the frozen height to Some(...) even when the original client state has it as None.

The original logic encodes a domain ClientState with frozen_height = None into a proto type with frozen_height = Some(Height { revision_number: 0, revision_height: 0 }).
In this context, a zero proto height is interpreted as the unfrozen height for the Tendermint clients and is considered equivalent to None. (see ibc-go check as well)

Sorry, the docstrings around aren't clear enough. We should improve it.

[riversyang]

OK, I now fully understand your intentions and rationale. Thank you for the detailed explanation and the insightful feedback. We look forward to future updates and releases.