`XDEFIDistribution.sol#relock()` Implementation can be simpler and save some gas

[code423n4]

Handle

WatchPug

Vulnerability details

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

uint256 withdrawAmount = amountUnlocked_ - lockAmount_;

if (withdrawAmount != uint256(0)) {
    // Send the excess XDEFI to the destination, if needed.
    SafeERC20.safeTransfer(IERC20(XDEFI), destination_, withdrawAmount);
}

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

uint256 withdrawAmount = amountUnlocked_ - lockAmount_;

if (withdrawAmount != uint256(0)) {
    // Send the excess XDEFI to the destination, if needed.
    SafeERC20.safeTransfer(IERC20(XDEFI), destination_, withdrawAmount);
}

Recommendation

Change to:

if (amountUnlocked_ > lockAmount_) {
    SafeERC20.safeTransfer(IERC20(XDEFI), destination_, amountUnlocked_ - lockAmount_);
}

• Removed a local variable: withdrawAmount;
• Only do the arithmetic when needed: amountUnlocked_ - lockAmount_.

[deluca-mike]

Yup, this is good. I tested it and it saves deploy and runtime gas even when using all unchecked math.

        amountUnlocked_ = _destroyLockedPosition(msg.sender, tokenId_);

        if (lockAmount_ > amountUnlocked_) revert InsufficientAmountUnlocked();

        newTokenId_ = _createLockedPosition(lockAmount_, duration_, bonusMultiplier_, destination_);

        unchecked {
            if (amountUnlocked_ - lockAmount_ != uint256(0)) {
                IERC20(xdefi).transfer(destination_, amountUnlocked_ - lockAmount_);
            }
        }

        _updateDistributableXDEFI();

[deluca-mike]

That common relock logic has been isolated, in the release candidate contract, into _relock where 418 prevents underflow of 424-426.

[Ivshti]

resolved, valid finding