`XDEFIDistribution.sol#lock()` Validation of `amount_` can be done earlier to save gas

[code423n4]

Handle

WatchPug

Vulnerability details

In lock(), checks of amount_ is done in _unlock(), after safeTransferFrom().

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L255

Check if amount_ > 0 earlier can avoid unnecessary code execution when this check failed.

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L92-L97

function lock(uint256 amount_, uint256 duration_, address destination_) external noReenter returns (uint256 tokenId_) {
    // Lock the XDEFI in the contract.
    SafeERC20.safeTransferFrom(IERC20(XDEFI), msg.sender, address(this), amount_);

    // Handle the lock position creation and get the tokenId of the locked position.
    return _lock(amount_, duration_, destination_);
}

The same issue also exists in XDEFIDistribution#lockWithPermit().

Recommendation

Change to:

function lock(uint256 amount_, uint256 duration_, address destination_) external noReenter returns (uint256 tokenId_) {
    require(amount_ != uint256(0) && amount_ <= MAX_TOTAL_XDEFI_SUPPLY, "INVALID_AMOUNT");
    // Lock the XDEFI in the contract.
    SafeERC20.safeTransferFrom(IERC20(XDEFI), msg.sender, address(this), amount_);

    // Handle the lock position creation and get the tokenId of the locked position.
    return _lock(amount_, duration_, destination_);
}

[deluca-mike]

This change would only results in gas saving for sad-path (which most wallets would already prevent the transaction from being broadcasted anyway), and increases gas costs for happy-path. In any case, the check for the amount is being removed in place for a check of units lower down.