code-423n4 / 2021-11-yaxis-findings

0 stars 0 forks source link

Unnecessary `MLOAD`s in for-each loops #80

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

Vulnerability details

There are many for loops that follows this for-each pattern:

for (uint256 i = 0; i < array.length; i++) {
    // do something with `array[i]`
}

for example: MockdYdXsoloMargin.sol line 27: for (uint256 i = 0; i < _marketIds.length; i++) MockdYdXsoloMargin.sol line48: for (uint256 a = 0; a < accounts.length; a++) MockdYdXsoloMargin.sol line59: for (uint256 i = 0; i < actions.length; i++)

In such for loops, the array.length is read on every iteration, instead of caching it once in a local variable and read it again using the local variable.

Impact

Memory reads are a bit more expensive than reading local variables.

Tool Used

Manual code review.

Recommended Mitigation Steps

Read these values from memory once, cache them in local variables and then read them again using the local variables. For example:

uint256 length = array.length;
for (uint256 i = 0; i < length; i++) {
    // do something with `array[i]`
}
Xuefeng-Zhu commented 2 years ago

out of scope

0xleastwood commented 2 years ago

Agree with sponsor, finding is valid but out of scope.