VTVL contest - leosathya's results

[L-01] Use of Floating Pragma

Use of Floating(^) Pragma in Contract file VariableSupplyERC20Token.sol

There is 1 instance of this issue:

** File : token/VariableSupplyERC20Token.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/token/VariableSupplyERC20Token.sol#L2

Recommended Mitigation

Contracts should be deployed using the same compiler version with which they have been tested. Locking the pragma (for e.g. by not using ^ in pragma solidity 0.8.14) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs.

[L-02] setAdmin() Should be a 2 step process

There is 1 instance of this issue:

The critical procedures like Admin change should always be a two step process.

**File : ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/AccessProtected.sol#L39-L44

Proof Of Concept

It would be much better if we use a 2 step process for critical address change In a smartContract, like here we can create 2 functions setAdmin() and updateAdmin(). Here setAdmin() function keeps new_address in storage rather than modifying current Admin address. Then updateAdmin() functions checks whether msg.sender == new_address or not which means new Admin signs transaction and verifies himself as the new Admin, after then Admin storage will updated with new_address.

Recommended Mitigation

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

[L-03] Division before Multiplication

Performing multiplication before division is generally better to avoid loss of precision because Solidity integer division might truncate.

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L169

Recommended Mitigation

prefer multiplication before division. i.e Instead (a/b)c use (ac)/b this gives more accurate result

[N-01] Use msg.sender instead of _msgSender()

_msgSender() is a replacement for msg.sender. For regular transactions it returns msg.sender and for meta transactions it can be used to return the end user (rather than the relayer).

There are 12 instances of this issue:

** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L364 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L367 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L371 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L388 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L391 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L407 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L410 ** File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L450 ** File : AccessProtected.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/AccessProtected.sol#L17 ** File : AccessProtected.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/AccessProtected.sol#L18 ** File : AccessProtected.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/AccessProtected.sol#L25 ** File : token/FullPremintERC20Token.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/token/FullPremintERC20Token.sol#L12 ** File : token/VariableSupplyERC20Token.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/token/VariableSupplyERC20Token.sol#L32

Recommended Mitigation

It is recommended to use msg.sender instead of _msgSender(), If the corresponding contract is not dealing with meta transaction, or don't have any meta transaction implementation.

[G-01] NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

Here Uints initiaziled with 0, which is not necessary cause its by default value is zero

There is 2 instance of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L27 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L148

Recommended Mitigation

Instead of initializing default leave them as it is. For ex : Instead of doing uint x = 0; leave them as it is uint x;

both are same, second one is more gas optimized.

[G-02] INSTEAD OF USING DUPLICATE REQUIRE(), LOGIC SHOULD ENCLOSED INSIDE A MODIFIER, AND USED WHENEVER REQUIRED

The require condition require(address(_tokenAddress) != address(0), "INVALID_ADDRESS"); used multiple times

There is 2 Instances of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L82 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L255

Recommended Mitigation

This require() statement should enclosed inside a modifier and should used in

[G-03] >= COSTS LESS GAS THAN >

There is 4 instance of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L154 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L166 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L374 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L426 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L449

Recommended Mitigation

Instead of using < try to use <= whenever possible

[G-04] <x> = <x> + <y> is more gas efficient than use of <x> += <y>

In many instances <x> += <y> This type of syntax are used, Those can optimized to previous one i.e <x> = <x> + <y>

There are 6 instances of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L161 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L179 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L301 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L381 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L383 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L433

[G-05] INSTEAD OF USING && INSIDE REQUIRE(), IT SHOULD BE SPLIT INTO INDIVIDUALS. THIS IS MORE GAS EFFICIENT

        require(
		_startTimestamps.length == length &&
		_linearVestAmounts.length == length &&
		_cliffAmounts.length == length,
		"ARRAY_LENGTH_MISMATCH"
	);

TO require(_startTimestamps.length ,"ARRAY_LENGTH_MISMATCH"); require(_linearVestAmounts.length ,"ARRAY_LENGTH_MISMATCH"); require(_cliffAmounts.length ,"ARRAY_LENGTH_MISMATCH");

There are 2 instances of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L344-L350 **File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L270-L278

[G-06] ] SHOULD OPTIMIZED FOR LOOP

There is 1 instance of this issue:

**File : VTVLVesting.sol ** => https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L353-L355

Recommended Mitigation

. Should not initialize uint with default value i.e uint i=0 TO uint i; . Should use ++i instead i++ . Should uncheck i++