Platform: Code4rena
Start Date: 04/01/2022
Pot Size: $25,000 USDC
Total HM: 3
Participants: 40
Period: 3 days
Judge: Ivo Georgiev
Total Solo HM: 1
Id: 75
League: ETH
Rank: 18/40
Findings: 1
Award: $190.43
🌟 Selected for report: 3
🚀 Solo Findings: 0
45.1044 USDC - $45.10
OriDabush
The "currentDistributableXDEFI" variable is not used (can use distributableXDEFI instead).
// function before: function _updateXDEFIBalance() internal returns (int256 newFundsTokenBalance_) { uint256 previousDistributableXDEFI = distributableXDEFI; uint256 currentDistributableXDEFI = distributableXDEFI = IERC20(XDEFI).balanceOf(address(this)) - totalDepositedXDEFI; return _toInt256Safe(currentDistributableXDEFI) - _toInt256Safe(previousDistributableXDEFI); } // function after: function _updateXDEFIBalance() internal returns (int256 newFundsTokenBalance_) { uint256 previousDistributableXDEFI = distributableXDEFI; distributableXDEFI = IERC20(XDEFI).balanceOf(address(this)) - totalDepositedXDEFI; return _toInt256Safe(distributableXDEFI) - _toInt256Safe(previousDistributableXDEFI); }
#0 - deluca-mike
2022-01-08T21:53:44Z
Yup, good catch. Will refactor this.
#1 - deluca-mike
2022-01-14T03:58:49Z
Given refactor of _updateDistributableXDEFI
(formerly poorly named _updateXDEFIBalance
) in release candidate contract, currentDistributableXDEFI
is now used: https://github.com/XDeFi-tech/xdefi-distribution/blob/v1.0.0-rc.0/contracts/XDEFIDistribution.sol#L450-L461
#2 - Ivshti
2022-01-16T06:14:26Z
seems to be resoslved
45.1044 USDC - $45.10
OriDabush
Instead of multiplying by _pointsMultiplier, which is 2 ** 128, it is more efficient to shift by 128 (x * (2 ** 128) = x << 128), same for dividing (x / (2 ** 128) = x >> 128)
// line 151 - old _pointsPerUnit += ((newXDEFI * _pointsMultiplier) / totalUnitsCached); // line 151 - new _pointsPerUnit += ((newXDEFI << 128) / totalUnitsCached); // lines 338-344 - old return ( _toUint256Safe( _toInt256Safe(_pointsPerUnit * uint256(units_)) + pointsCorrection_ ) / _pointsMultiplier ) + uint256(depositedXDEFI_); // lines 338-344 - new return ( _toUint256Safe( _toInt256Safe(_pointsPerUnit * uint256(units_)) + pointsCorrection_ ) >> 128 ) + uint256(depositedXDEFI_);
#0 - deluca-mike
2022-01-08T21:40:12Z
Yup, this is valid. Will do.
#1 - deluca-mike
2022-01-14T04:01:53Z
In release candidate contract, _pointsMultiplier
(now POINTS_MULTIPLIER_BITS
) is defined as a number of bits to shift, and shifting is occuring (instead of multiplication) in updateDistribution
, and instead of division in _withdrawableGiven
.
#2 - Ivshti
2022-01-16T06:15:36Z
that's a good one!
🌟 Selected for report: OriDabush
100.2321 USDC - $100.23
OriDabush
Make functions like "_getPoints" and "_getPointsFromTokenId" inline (saves the function call gas, the functions can be left and the inline "call" can reference to them with a comment or something)
function _getPoints(uint256 amount_, uint256 duration_) internal view returns (uint256 points_) { return amount_ * (duration_ + _zeroDurationPointBase); } function _getPointsFromTokenId(uint256 tokenId_) internal pure returns (uint256 points_) { return tokenId_ >> uint256(128); } // old usage: return _getPointsFromTokenId(tokenId_); // new usage: return tokenId_ >> uint256(128);
#0 - deluca-mike
2022-01-08T21:50:56Z
_getPoints
and _getScoreFromTokenId
are each used twice, so it's better for that math to be in one place, even if it is trivial. It increases deploy costs but decrease runtime costs such that the breakeven is about 100 calls, which is a good trade-off. Issue is valid, but we won't implement.