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

0 stars 0 forks source link

Unnecessary `SLOAD`s in `Swap.sweepFees()` #76

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

These lines in Swap.sweepFees() perfom SLOADs operations for the same feeRecipient value:

require(
    feeRecipient != address(0),
    "Swap::withdrawAccruedFees: feeRecipient is not initialized"
);
IERC20(tokens[i]).safeTransfer(feeRecipient, balance);
emit FeesSwept(tokens[i], balance, feeRecipient);
feeRecipient.transfer(address(this).balance);
emit FeesSwept(address(0), address(this).balance, feeRecipient);

Impact

Storage reads are much more expensive than reading local variables.

Tool Used

Manual code review.

Recommended Mitigation Steps

Load feeRecipient from storage once, cache it in a local variable and them use this local variable instead of loading this value from storage again.

Shadowfiend commented 2 years ago

Duplicate of #18.