code-423n4 / 2022-01-trader-joe-findings

2 stars 0 forks source link

Check if amount > 0 before token transfer can save gas #238

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-trader-joe/blob/a1579f6453bc4bf9fb0db9c627beaa41135438ed/contracts/RocketJoeStaking.sol#L116-L135

function withdraw(uint256 _amount) external {
    UserInfo storage user = userInfo[msg.sender];
    require(
        user.amount >= _amount,
        "RocketJoeStaking: withdraw amount exceeds balance"
    );

    updatePool();

    uint256 pending = (user.amount * accRJoePerShare) /
        PRECISION -
        user.rewardDebt;

    user.amount = user.amount - _amount;
    user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;

    _safeRJoeTransfer(msg.sender, pending);
    joe.safeTransfer(address(msg.sender), _amount);
    emit Withdraw(msg.sender, _amount);
}

Since _amount can be 0. Checking if (_amount != 0) before the transfer can potentially save an external call and the unnecessary gas cost of a 0 token transfer.

When withdraw(0), user.amount = user.amount - _amount; can be skipped, to save storage write and external call of joe.safeTransfer(address(msg.sender), _amount);.

cryptofish7 commented 2 years ago

Duplicate of #317

dmvt commented 2 years ago

This does point to a valid gas optimization. Rewards could be disbursed without sending a corresponding zero transfer. Using an if statement here would have a net positive impact on gas. Even better would be to split into the more common withdraw / withdrawAndClaim pattern.