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

0 stars 0 forks source link

Using shift operators can save gas #145

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

_pointsMultiplier multiplication and division can be replaced with shift operators to save gas.

The gas cost of << and >> is 3 while * and / costs 5.

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

    uint256 internal constant _pointsMultiplier = uint256(2**128);

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

        _pointsPerUnit += ((newXDEFI * _pointsMultiplier) / totalUnitsCached);

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

    function _withdrawableGiven(uint96 units_, uint88 depositedXDEFI_, int256 pointsCorrection_) internal view returns (uint256 withdrawableXDEFI_) {
        return
            (
                _toUint256Safe(
                    _toInt256Safe(_pointsPerUnit * uint256(units_)) +
                    pointsCorrection_
                ) / _pointsMultiplier
            ) + uint256(depositedXDEFI_);
    }

Recommendation

Change to:

_pointsPerUnit += ((newXDEFI << 128) / totalUnitsCached);
    function _withdrawableGiven(uint96 units_, uint88 depositedXDEFI_, int256 pointsCorrection_) internal view returns (uint256 withdrawableXDEFI_) {
        return
            (
                _toUint256Safe(
                    _toInt256Safe(_pointsPerUnit * uint256(units_)) +
                    pointsCorrection_
                ) >> 128
            ) + uint256(depositedXDEFI_);
    }
deluca-mike commented 2 years ago

Oh, I like this! Will do! Also, in checked and unchecked math, division by zero is always checked, so shifting does away with that too. More gas savings from this than you think!

deluca-mike commented 2 years ago

Duplicate #122