[ICS24] Enhancements and fixes for ChainId handling and validation

[Farhad-Shabani]

Problem Statement

Upon reviewing our existing design around ChainId the following issues/suggestions were brought up, some of which were raised thanks to @mina86 in #698, #721, and #725.

• Utilize existing identifier validator functions for both character and length checks, ofc with some customization, given that:

  • (bug)ChainIds like -1 and comsos hub-1 (with whitespace) are considered valid!
  • (bug) The ChainId's length check for the new Tendermint client creation is incorrect. It shouldn't allow lengths greater than MaxChainIdLen - 20 (considering u64::MAX length), as it can lead to an overflow.

• Redundant is_epoch_format call by from_string in ChainId creating

• Use revision_number instead of version to get naming consistent with other places (Looking at Height and HeightTimeout) and taking ibc-go as the reference implementation.

• Unclear why invalid identifiers create a default ChainId with version = 0. This should be documented or completely disallow creating ChainId with strings not in format. This way conversions from a string can fail, and the error of FromStr can't be infallible.

• Remove default implementation for ChainId. We can’t assume anything about that.

Highlight

Taking ICS-24 as our reference for identifier validation (As it is so for other identifiers as well), basically, we should not enforce stricter checks on the identifier's format, and leave it up to users if needed. Even though we may allow creating odd ChainId like chainA-a-1 or --chainA-1,

Some related issues in ibc-go

• Retrieve Epoch from ChainID cosmos-sdk#7280
• Revision and client identifier regex formats accept newlines ibc-go#686