VTVL contest - imare's results

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L129

Vulnerability details

Currently there is no way to recreate the claim with the same recipient address in case the founder/admin has made a mistake in creating the claim on the first time.

Impact

Clam cannot be recreated if we try to we get CLAIM_ALREADY_EXISTS reject reason. Even if we revoked the claim we get the same rejection.

Recommended Mitigation Steps

0. Admin first calls revokeClaim with the problematic recipient address. With this it takes care of internal tokens calculations (especially important numTokensReservedForVesting).

1. Change existing function _createClaimUnchecked by adding a new parameter that indicates we are about to restore an already used claim. When this parameter is true we don't add recipient to the vestingRecipients list because it is already there. In all other cases when we are normaly creating claims this paramater is set to false (false is inside createClaim and createClaimsBatch).

function _createClaimUnchecked(
...
if (!reinstate) { // if we are in normal create claim operation mode
    vestingRecipients.push(_recipient);
}
emit ClaimCreated(_recipient, _claim); // let everyone know -- in case of restoring claim we can optionally skip this?
}

2. New function for reinstating/restoring the claim that only admins can call:

<pre><code> ... // added new event to signal recepient reuse event ClaimReinstantiated(address indexed _recipient, Claim _claim); ... function reinstateClaim( address _recipient, uint40 _startTimestamp, uint40 _endTimestamp, uint40 _cliffReleaseTimestamp, uint40 _releaseIntervalSecs, uint112 _linearVestAmount, uint112 _cliffAmount) public onlyAdmin { Claim storage _claim = claims[_recipient]; require(_claim.startTimestamp != 0, "CLAIM_DOESNT_EXIST"); require(_claim.isActive == false, "CLAIM_ACTIVE"); // do revoke first delete claims[_recipient]; // just remove it ..revokeClaim did the heavy lifting for us _createClaimUnchecked(_recipient, _startTimestamp, _endTimestamp, _cliffReleaseTimestamp, _releaseIntervalSecs, _linearVestAmount, _cliffAmount, <b>false</b>); <b>emit ClaimReinstated(_recipient, _claim);</b> // let everyone know } </code></pre>

[GAS-01] Redundant zero initalization

• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L27
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L148
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L353

[GAS-02] Use prefix not postfix in loops

• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L353

[GAS-03] > 0 IS LESS EFFICIENT THAN != 0 FOR UNSIGNED INTEGERS

• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L449
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L107
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L256
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L257
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L263
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L272
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L273
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L107

[GAS-04] USE CUSTOM ERRORS RATHER THAN REVERT()/REQUIRE() STRINGS TO SAVE GAS

Custom errors are available from solidity version 0.8.4 and using them leads to cheaper deploy time cost and run time cost.

[GAS-05] FUNCTIONS GUARANTEED TO REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLE

If a function modifier such as onlyAdmin is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L317
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L333
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L398
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L418
• https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L446