Unnecessary `SLOAD` Operations

[code423n4]

Handle

leastwood

Vulnerability details

Impact

There are a number instances where costly SLOAD operations are performed. Instead of parsing storage variables into their respective functions, they can be parsed as memory and have the return value written to storage once.

Proof of Concept

https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/TwabLib.sol#L164-L170 https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/ObservationLib.sol#L49-L95 https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/TwabLib.sol#L145-L157 https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/TwabLib.sol#L191-L232 https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/TwabLib.sol#L242-L282 https://github.com/pooltogether/v4-core/blob/master/contracts/libraries/TwabLib.sol#L300-L345

Tools Used

Manual code review

Recommended Mitigation Steps

One of the more computationally intensive functions, ObservationLib.binarySearch(), takes a list of observations as a storage variable and performs several SLOAD operations. The cost can be heavily reduced by parsing the observations list into memory where instead the more cost effective MLOAD operation is performed. There are a number of instances where this can be further improved upon in TwabLib.sol and ObservationLib.sol.

[asselstine]

This report seems to suggest that we load the twabs array into memory; but the twabs array will be very large.

I think the warden doesn't understand just how big this array will get

[GalloDaSballo]

The sponsor disputes, the lack of evidence and details in the finding don't help the case. Invalid