VTVL contest - V_B's results

1. allVestingRecipients function fail

As we see allVestingRecipients function returns all the vestingRecipients array. Especially, the vestingRecipients array will be read from storage and later copied into memory. In the case of big enought number of the elements of this array this function will use big amount of gas, as it uses at least 2100 gas for reading corresponding storage slot for each recipient (actually it can use more gas that can fit into block gas limit in case when there is at least ~1500 recipients, because 1500*2100 > 30M).

It is reasonable to create an external view function that gives ability to read only part of that array.

2. ERC20 with permit

It is considered good practice to use ERC20 standard modification that contains permit function.

3. wrong hasNoClaim modifier description

hasNoClaim modifier's description states "@notice Modifier which is opposite hasActiveClaim". But actually it is not the opposite for the hasActiveClaim modifier, because hasActiveClaim also checks that recipient's claim is active but not that recipient have any claim.

4. wrong NOTHING_TO_WITHDRAW require description

There is the following check in the withdraw function.

// Make sure we didn't already withdraw more that we're allowed.
require(allowance > usrClaim.amountWithdrawn, "NOTHING_TO_WITHDRAW");

But the description above the require statement is wrong --- the right one is "Make sure we didn't already withdraw more or equal to what we're allowed..

5. redundant check in hasActiveClaim modifier

There is the following check in the hasActiveClaim modifier:

require(_claim.startTimestamp > 0, "NO_ACTIVE_CLAIM");

It is redundant statement, because below there is the following check:

require(_claim.isActive == true, "NO_ACTIVE_CLAIM");

In case the second check do not revert the first one will also pass.

1. calldata for parameters instead of memory in createClaimsBatch

It is reasonable to use calldata instead of memory for input arrays in createClaimsBatch function (which is external) to reduce gas consumption and make the code more clear.

2. redundant check in hasActiveClaim modifier

There is the following check in the hasActiveClaim modifier:

require(_claim.startTimestamp > 0, "NO_ACTIVE_CLAIM");

It is redundant statement, because below there is the following check:

require(_claim.isActive == true, "NO_ACTIVE_CLAIM");

In case the second check do not revert the first one will also pass.

3. hashing the Claim struct

It is reasonable to hash the Claim structs and store only the hash instead of storing all of them in the storage. Later users will be able to prove that provided struct data corresponds to the real one by checking the corresponding hash. This will substantially reduce gas consumption.