Unit Test Review: Random Module

[gkoumout]

NOTE: The review was done on Feb 15, 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..).

method.spec.ts

• Line 295: I don't know where value 1000 came from. I guess it should be bounded by the maximum size of validatorReveals array (I guess implementation needs also to be updated here).
• Tests of lines 322, 344, 366, 388: There was a PR fixing a logic error in getRandomBytes function which does not seem to have been incorporated neither in the implementation nor in unit tests. For the implementation, line 87 of random/utils.ts should be modified to replace "<=" by "<" (also I think it could be implemented more efficiently given that validatorsReveal array is sorted by height..). If this change is done, then for all aforementioned tests cases, numberOfSeeds should be increased by 1 in order to expect the same outcome.
• Tests of lines 322, 344, 366, 388 assume that function bitwiseXOR is correctly implemented. Although it is simple, it would be good to check that it works as intended.
• line 426: I don't see where function generateRandomSeeds is implemented.

Other Comments

getSeedRevealValidity could be implemented more efficiently by making use of the fact that validatorsReveal is ordered according to height (same as in the implementation of isSeedValidInput).

[bobanm]

To make PRs more focused, this issue will focus only on updating the protocol change logic. The changes are covered by PR #8570.

line 426: I don't see where function generateRandomSeeds is implemented.

Indeed, the generateRandomSeeds as the name of test suite is inconsistent, as there is no function with that name. I have renamed the test suite into getRandomBytes from protocol specs.

I've created separate issues for the remaining points from this issue:

Line 295: I don't know where value 1000 came from. I guess it should be bounded by the maximum size of validatorReveals array (I guess implementation needs also to be updated here).

Remove limit check for numberOfSeeds in getRandomSeed() #8574

Tests of lines 322, 344, 366, 388 assume that function bitwiseXOR is correctly implemented. Although it is simple, it would be good to check that it works as intended.

Add unit test for Random module bitwiseXOR() #8568

getSeedRevealValidity could be implemented more efficiently by making use of the fact that validatorsReveal is ordered according to height (same as in the implementation of isSeedValidInput).

Improve performance of Random module getSeedRevealValidity() function #8578