search

code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

`XDEFIDistribution.sol#_updateXDEFIBalance()` Avoiding unnecessary storage writes can save gas #151

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Storage writes (SSTORE) to distributableXDEFI may not be needed when previousDistributableXDEFI == currentDistributableXDEFI, therefore the code can be reorganized to save gas from unnecessary storage writes.

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

function _updateXDEFIBalance() internal returns (int256 newFundsTokenBalance_) {
    uint256 previousDistributableXDEFI = distributableXDEFI;
    uint256 currentDistributableXDEFI = distributableXDEFI = IERC20(XDEFI).balanceOf(address(this)) - totalDepositedXDEFI;

    return _toInt256Safe(currentDistributableXDEFI) - _toInt256Safe(previousDistributableXDEFI);
}

Recommendation

Change to:

function _updateXDEFIBalance() internal returns (int256 newFundsTokenBalance_) {
    uint256 previousDistributableXDEFI = distributableXDEFI;
    uint256 currentDistributableXDEFI = IERC20(XDEFI).balanceOf(address(this)) - totalDepositedXDEFI;

    newFundsTokenBalance_ = _toInt256Safe(currentDistributableXDEFI) - _toInt256Safe(previousDistributableXDEFI);
    if (newFundsTokenBalance_ != 0) {
        distributableXDEFI = currentDistributableXDEFI;
    }
}
deluca-mike commented 2 years ago

Agreed. This is being changed to:

   function _updateDistributableXDEFI() internal returns (int256 changeInDistributableXDEFI_) {
        uint256 xdefiBalance = IERC20(xdefi).balanceOf(address(this));
        uint256 previousDistributableXDEFI = distributableXDEFI;

        unchecked {
            uint256 currentDistributableXDEFI = xdefiBalance > totalDepositedXDEFI ? xdefiBalance - totalDepositedXDEFI : uint256(0);

            if (currentDistributableXDEFI == previousDistributableXDEFI) return int256(0);

            // NOTE: Max XDEFI is 240M (with 18 decimals), so this can never over or underflow.
            changeInDistributableXDEFI_ = _toInt256Safe(distributableXDEFI = currentDistributableXDEFI) - _toInt256Safe(previousDistributableXDEFI);
        }
    }
deluca-mike commented 2 years ago

In release candidate contract, _updateXDEFIBalance has been more aptly renamed to _updateDistributableXDEFI, where distributableXDEFI is not written if currentDistributableXDEFI == previousDistributableXDEFI.

Ivshti commented 2 years ago

seems to be resolved, valid finding