Unit test review: Interoperability utils

[ricott1]

utils.spec.ts

In general, there is no 1-to-1 correspondence with the LIP, which is fine, but makes checking the tests a bit more tricky.

I think we are missing tests for verifyCertificateSignature, verifyValidatorsUpdate, and verifyPartnerChainOutboxRoot (maybe covered by checkInboxUpdateValidity).

• checkLivenessRequirementFirstCCU: I find the name misleading, as this test refers to registered chains but apply to all CCUs, not only the first. I would update the function name.
• (REMOVED)checkValidCertificateLiveness: it looks to me that this function is never called. I also think there could be a bug, as the check and error message don't match (I think the check should not be LIVENESS_LIMIT / 2 but just LIVENESS_LIMIT.
• (UNUSED)checkCertificateValidity: Missing tests on timestamp and validatorsHash properties against values currently stored in the chainAccount.
• (UNUSED)checkValidatorsHashWithCertificate: Here I lost a bit the correspondence with the LIP, but maybe we should check that if the certificate certifies a new validators hash, than at least the validatorsUpdate must be non -mepty or the certificate threshold must differ from the current one.
• (REMOVED)checkInboxUpdateValidity:
  • some data is invalid (hash values with incorrect length, non-exisiting crossChainCommandID property in ccms)
  • missing minReturnFeePerByte property in channel data
  • check that inbox bitmap and siblingHashes are either both empty or both non-empty
• calculateNewActiveValidators: I already reviewed these recently

[ricott1]

• checkCertificateTimestamp: Could add a check for equal timestamp (should fail)
• checkValidatorsHashWithCertificate: several tests are run depending on the value of certificate threshold compared to 0, but the relevant value is the current certificate threshold stored in the state. Also, not sure where the function checkValidatorsHashWithCertificate is used in the code base