code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

`PrizeDistributor.sol#claim()` Remove redundant check can save gas #41

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

https://github.com/pooltogether/v4-core/blob/055335bf9b09e3f4bbe11a788710dd04d827bf37/contracts/PrizeDistributor.sol#L58-L81

function claim(
    address _user,
    uint32[] calldata _drawIds,
    bytes calldata _data
) external override returns (uint256) {

    uint256 totalPayout;

    (uint256[] memory drawPayouts, ) = drawCalculator.calculate(_user, _drawIds, _data); // neglect the prizeCounts since we are not interested in them here

    for (uint256 payoutIndex = 0; payoutIndex < drawPayouts.length; payoutIndex++) {
        uint32 drawId = _drawIds[payoutIndex];
        uint256 payout = drawPayouts[payoutIndex];
        uint256 oldPayout = _getDrawPayoutBalanceOf(_user, drawId);
        uint256 payoutDiff = 0;

        if (payout > oldPayout) {
            payoutDiff = payout - oldPayout;
            _setDrawPayoutBalanceOf(_user, drawId, payout);
        }

        // helpfully short-circuit, in case the user screwed something up.
        require(payoutDiff > 0, "PrizeDistributor/zero-payout");

Given the funtcion requires payoutDiff > 0, the check if (payout > oldPayout) is redundant.

Removing it will make the code simpler and save some gas.

Recommendation

Change to:

function claim(
    address _user,
    uint32[] calldata _drawIds,
    bytes calldata _data
) external override returns (uint256) {

    uint256 totalPayout;

    (uint256[] memory drawPayouts, ) = drawCalculator.calculate(_user, _drawIds, _data); // neglect the prizeCounts since we are not interested in them here

    for (uint256 payoutIndex = 0; payoutIndex < drawPayouts.length; payoutIndex++) {
        uint32 drawId = _drawIds[payoutIndex];
        uint256 payout = drawPayouts[payoutIndex];
        uint256 oldPayout = _getDrawPayoutBalanceOf(_user, drawId);

        require(payout > oldPayout, "PrizeDistributor/zero-payout");
        uint256 payoutDiff = payout - oldPayout;
        _setDrawPayoutBalanceOf(_user, drawId, payout);
asselstine commented 3 years ago

https://github.com/pooltogether/v4-core/pull/234

GalloDaSballo commented 3 years ago

The sponsor has applied the optimization in a subsequent PR