Remaining concerns for upgrade_client implementation

[Farhad-Shabani]

In continuation of #349

Summary

The investigation conducted around the upgrade client implementation (ADR06) left some unanswered questions, primarily arising from the ibc-go implementation approach. Before enabling this feature, these concerns should be addressed:

Details

1. Considering the point made here, though decreasing the unbounding period can be a risky action, there isn't any check for this purpose in ibc-go

2. Upgrading a frozen client or one where the upgrade message is outside the trust period, both are accepted. Is there any case related to these situations in which we should also avoid preserving connections? (at any cost)

3. There have been breaking changes in how client upgrades work in the 02-client-refactor PR. Specifically, in v5.0.1 (the version ibc-rs is tracking), the upgraded ClientState doesn’t get its “custom fields” zero’d, but in main, it does. This is a protocol breaking change, right? So which of the 2 versions should ibc-rs replicate? And even within ibc-go chains, doesn’t that cause problems if e.g. a chain connected to the hub upgrades to the latest ibc-go, but not the hub?

[Farhad-Shabani]

Based on details we received thanks to theibc-go team:

1. There is not check on this because there are instances when chains do want to lower their unbonding period and since client param changes go thru governance, ibc can assume the community is behind this decision. The only hard check is that the unbonding period has to be greater than the trusting period (window for submission of misbehaviour).
   The change in the unbonding period that could break the counter party client would be if the unbonding period updated was lower than the trusting period.
   Also should be noticed by lowering unbonding period there would the smaller window for misbehaviour check and it could be more risky (depending on how active the misbehaviour catching agents through client updates submission)

2. A ClientUpdate of a frozen client can be done through governance and the respective checks for a frozen client and trusting period exist here through status() call

3. the upgrade proposal handler always zeros out custom fields, for example here on v3 It would be the responsibility of the relayer to provide the correct UpgradedClient as part of MsgUpgradeClient submitted to the counterparty in order for verification to succeed (i.e. the clientstate with zeroed out custom fields). The difference in ibc-go/v7 is that ZeroCustomFields() is called here defensively.
   See this issue -> Zero out client state before processing upgraded client proof ibc-go#292