There are many Functions loops that follows this for-each pattern:
for (uint256 i = 0; i < weights.length; i++) {
// do something
}
In such for loops, the weights.length is read from Storage on every iteration, instead of caching it once in a local variable and read it again using the local variable.
Storage reads are a bit more expensive than reading local variables.
0xleastwood
commented
2 years ago
Handle
Meta0xNull
Vulnerability details
Impact
There are many Functions loops that follows this for-each pattern: for (uint256 i = 0; i < weights.length; i++) { // do something }
In such for loops, the weights.length is read from Storage on every iteration, instead of caching it once in a local variable and read it again using the local variable.
Storage reads are a bit more expensive than reading local variables.
Proof of Concept
https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L275 https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L282 https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L290
Tools Used
Manual Review
Recommended Mitigation Steps
Read these values from storage once, cache them in local variables and then read them again using the local variables. For example:
uint256 weights_temp = weights; for (uint256 i = 0; i < weights_temp.length; i++) { // do something // Replace weights[i] to weights_temp[i] }