VTVL contest - delfin454000's results

Missing @param statements

FullPremintERC20Token.sol: L8-14

contract FullPremintERC20Token is ERC20 {
    // uint constant _initialSupply = 100 * (10**18);
    constructor(string memory name_, string memory symbol_, uint256 supply_) ERC20(name_, symbol_) {
        require(supply_ > 0, "NO_ZERO_MINT");
        _mint(_msgSender(), supply_);
    }
}

@param statements missing for name_ and symbol_

---

---

Long single line comments

In theory, comments over 79 characters should wrap using multi-line comment syntax. Even if somewhat longer comments are acceptable and a scroll bar is provided, there are cases where very long comments interfere with readability. Below are five instances of extra-long comments whose readability could be improved by wrapping, as shown:

---

VTVLVesting.sol: L235

    @notice Permission-unchecked version of claim creation (no onlyAdmin). Actual logic for create claim, to be run within either createClaim or createClaimBatch.

Suggestion:

    @notice Permission-unchecked version of claim creation (no onlyAdmin). Actual
      logic for create claim, to be run within either createClaim or createClaimBatch.

---

The following line occurs twice:

VTVLVesting.sol: L241

VTVLVesting.sol: L313

    @param _releaseIntervalSecs - The release interval for the linear vesting. If this is, for example, 60, that means that the linearly vested amount gets released every 60 seconds.

Suggestion:

    @param _releaseIntervalSecs - The release interval for the linear vesting. 
      If this is, for example, 60, that means that the linearly vested amount 
      gets released every 60 seconds.

---

VTVLVesting.sol: L260

        // -> Conclusion: we want to allow this, for founders that might have forgotten to add some users, or to avoid issues with transactions not going through because of discoordination between block.timestamp and sender's local time

Suggestion:

        // -> Conclusion: we want to allow this, for founders that might have forgotten
        //   to add some users, or to avoid issues with transactions not going through 
        //   because of discoordination between block.timestamp and sender's local time.

---

VTVLVesting.sol: L269

        // Both or neither of _cliffReleaseTimestamp and _cliffAmount must be set. If cliff is set, _cliffReleaseTimestamp must be before or at the _startTimestamp

Suggestion:

        // Both or neither of _cliffReleaseTimestamp and _cliffAmount must be set. If cliff
        //   is set, _cliffReleaseTimestamp must be before or at the _startTimestamp.

---

VTVLVesting.sol: L357

        // No need for separate emit, since createClaim will emit for each claim (and this function is merely a convenience/gas-saver for multiple claims creation)

Suggestion:

        // No need for separate emit, since createClaim will emit for each claim (and this
        //   function is merely a convenience/gas-saver for multiple claims creation).

---

---

Comments concerning unfinished work or open items should be addressed

Comments that contain TODOs or other language referring to open items should be addressed and modified or removed. Below are two instances:

---

VariableSupplyERC20Token.sol: L26

        // Should we allow this?

---

VTVLVesting.sol: L266

        // Potential TODO: sanity check, if _linearVestAmount == 0, should we perhaps force that start and end ts are the same?

---

---

Use of exponentiation

For readability, scientific notation for large multiples of ten is preferable to using exponentiation

---

FullPremintERC20Token.sol: L9

    // uint constant _initialSupply = 100 * (10**18);

Suggestion: Change 10**18 to 1e18

---

---

Avoid use of '&&' within a require function

Splitting into separate require() statements saves gas VTVLVesting.sol: L344-351

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

Recommendation:

        require(_startTimestamps.length == length, "ARRAY_LENGTH_MISMATCH";
        require(_endTimestamps.length == length, "ARRAY_LENGTH_MISMATCH";
        require(_cliffReleaseTimestamps.length == length, "ARRAY_LENGTH_MISMATCH";
        require(_releaseIntervalsSecs.length == length, "ARRAY_LENGTH_MISMATCH";
        require(_linearVestAmounts.length == length, "ARRAY_LENGTH_MISMATCH";
        require(_cliffAmounts.length == length, "ARRAY_LENGTH_MISMATCH";

---

---

Use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)

!= 0 should be used where possible since > 0 costs more gas

FullPremintERC20Token.sol: L8-14

contract FullPremintERC20Token is ERC20 {
    // uint constant _initialSupply = 100 * (10**18);
    constructor(string memory name_, string memory symbol_, uint256 supply_) ERC20(name_, symbol_) {
        require(supply_ > 0, "NO_ZERO_MINT");
        _mint(_msgSender(), supply_);
    }
}

Recommendation: Change supply_ > 0 to supply != 0

---

---

State variables should not be initialized to their default values

Initializing uint variables to their default value of 0 is unnecessary and costs gas

---

VTVLVesting.sol: L27

    uint112 public numTokensReservedForVesting = 0;

Change to:

    uint112 public numTokensReservedForVesting;

---

VTVLVesting.sol: L148

        uint112 vestAmt = 0;

Change to:

        uint112 vestAmt;

---

---

Use ++i instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i

---

VTVLVesting.sol: L353-355

        for (uint256 i = 0; i < length; i++) {
            _createClaimUnchecked(_recipients[i], _startTimestamps[i], _endTimestamps[i], _cliffReleaseTimestamps[i], _releaseIntervalsSecs[i], _linearVestAmounts[i], _cliffAmounts[i]);
        }

Suggestion:

        for (uint256 i = 0; i < length; ++i) {
            _createClaimUnchecked(_recipients[i], _startTimestamps[i], _endTimestamps[i], _cliffReleaseTimestamps[i], _releaseIntervalsSecs[i], _linearVestAmounts[i], _cliffAmounts[i]);
        }

---

---

Inline a function/modifier that’s only used once

It costs less gas to inline the code of functions that are only called once. Doing so saves the cost of allocating the function selector, and the cost of the jump when the function is called.

---

VTVLVesting.sol: L123-140

    modifier hasNoClaim(address _recipient) {
        Claim storage _claim = claims[_recipient];
        // Start timestamp != 0 is a sufficient condition for a claim to exist
        // This is because we only ever add claims (or modify startTs) in the createClaim function 
        // Which requires that its input startTimestamp be nonzero
        // So therefore, a zero value for this indicates the claim does not exist.
        require(_claim.startTimestamp == 0, "CLAIM_ALREADY_EXISTS");
        
        // We don't even need to check for active to be unset, since this function only 
        // determines that a claim hasn't been set
        // require(_claim.isActive == false, "CLAIM_ALREADY_EXISTS");
    
        // Further checks aren't necessary (to save gas), as they're done at creation time (createClaim)
        // require(_claim.endTimestamp == 0, "CLAIM_ALREADY_EXISTS");
        // require(_claim.linearVestAmount + _claim.cliffAmount == 0, "CLAIM_ALREADY_EXISTS");
        // require(_claim.amountWithdrawn == 0, "CLAIM_ALREADY_EXISTS");
        _;
    }

Since hasNoClaim() is used only once in this contract (in function _createClaimUnchecked()) as shown below, it should be inlined to save gas

VTVLVesting.sol: L245-253

    function _createClaimUnchecked(
            address _recipient, 
            uint40 _startTimestamp, 
            uint40 _endTimestamp, 
            uint40 _cliffReleaseTimestamp, 
            uint40 _releaseIntervalSecs, 
            uint112 _linearVestAmount, 
            uint112 _cliffAmount
                ) private  hasNoClaim(_recipient) {

---

---