Unit Test Review: Interoperability commands

[gkoumout]

Unit test review for all interoperability commands (including Base CCU command specs which contains many functions used in command verification/execution):

NOTE: The review was done on Feb 24, therefore in case the file gets updated after that, plz make sure that you are checking the appropriate version (so that line numbers are valid..). Since several modifications happened since then, a link to the version considered at the time of review is added. Nevertheless, I might have missed some versions, so I suggest being careful to consider the corresponding version while addressing the unit test reviews.

Register Sidechain Command

Version at time of review

• Missing test for execute: Check that after fee.payFee is called, the function Token.initializeEscrowAccount() is called (this was probably fixed in this PR).
• Missing test for execute: Before test of line 583, there should be a test checking that function addToOutbox is called.

Register Mainchain Command

Version at time of review

• Missing test for verify(): Check that trs.params.signature is valid (i.e., has length BLS_SIGNATURE_LENGTH).

Mainchain CCU command

Version at time of review

• Verify() missing check: The test of line 353 covers the case that first CCU does not contain an inbox Update, but contains a certificate. It would be good to check that if after that 2nd CCU contains an inbox update, everything works properly (liveness is checked as in test of line 330 - ideally create two tests, one where certificate is older than LIVENESS_LIMIT / 2 and one that is newer)

• execute() missing test: Check that it calls afterExecuteCommon (add test after line 487)

Sidechain CCU command

Version at time of review

• Test of line 387, I guess you mean "sending chain is the mainchain" instead of receiving chain.
• Missing test for execute: Function afterExecuteCommon should be called.

base_cross_chain_update_command.spec.ts

Version at time of review

• line 312. It would be good to add an extra test where blsKeysUpdate is empty but validatorsUpdate.bftWeightsUpdate and bftWeightsUpdateBitmap are non empty and then again the verifyValidatorsUpdate function should be called.

• Test of line 533 is relevant only in case the ccm.receivingChain is the mainchain, should not have the same output if ccm.receivingChain is a sidechain. I suggest having two tests, to make sure that both cases are treated appopriately.

• Missing tests for afterExecuteCommon: After tests of lines 826, 833, it would be good to add tests for the cases i) non-empty certificate but empty inbox update ( updateCertificate should be called and updatePartnerChainOutboxRoot should not be called) and ii) both certificate and inbox update are non-empty (both updateCertificate and updatePartnerChainOutboxRoot should be called).

• Missing test for apply(): After test of line 1023, it should be checked that in case execute does NOT fail, then the event with name "EVENT_NAME_CCM_PROCESSED" is emitted with result CCM_PROCESSED_RESULT_APPLIED.

• Missing Tests for bounce: In case status is ok and fee is higher than minFee, the following checks should be performed: i) Emit the EVENT_NAME_CCM_PROCESSED with result CCM_PROCESSED_RESULT_BOUNCED ii) If ccm.fee is updated correctly depending on the value of newCCMStatus (test of line 1273 assumes that ccm.fee -= minimumFee , which happens only if newCCMStatus != CCM_STATUS_CODE_FAILED_CCM; there should be an extra test for the case newCCMStatus == CCM_STATUS_CODE_FAILED_CCM.

Liveness Termination Command

Version at time of review

• Add check in verify about the schema validation (chainID should be valid).
• Line 126: This test should be reverted. The verify() function throws error when the chain is Live; when it is not live, verification is successful. So, the description should be "should return error if the chain is live" and in line 127 call mockResolvedValue(true).

State Recovery Command

Version at time of review

• Missing test for verify(): check validity of params according to schema.

Message Recovery Initialization Command

Version at time of review

• Missing test for verify(): check validity of params according to schema.

State Recovery Initialization Command

Version at time of review

• Line 230: Add test for the case that chain is the mainchain (here tests only case it is ownchain)
• line 249: "but the sidechain violates the liveness requirement" should be replaced by "and the sidechain does not violate the liveness requirement": To get an error, the chain should not violate liveness. Make sure the test details are appropriately set to test this case.

Message Recovery Command

Version at time of review

• Tests of lines 594, 633: I am not sure that it is checked that recoveredCCMs array is correctly updated.

[sitetester]

executeCommon should match LIP #9009

[sitetester]

CCU part still needs to be verified