search

code-423n4 / 2022-01-timeswap-findings

2 stars 0 forks source link

Loops can be implemented more efficiently #175

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x0x0x

Vulnerability details

Proof of Concept

Example:


for (uint i = 0; i < arr.length; i++) {

//Operations not effecting the length of the array.

}

Loading length of array costs gas. Therefore, the length should be cached, if the length of the array doesn't change inside the loop. Furthermore, there is no need to assign the initial value 0. This costs extra gas.

Recommended implementation:


uint length = arr.length;

for (uint i; i < length; ++i) {

//Operations not effecting the length of the array.

}

By doing so the length is only loaded once rather than loading it as many times as iterations (Therefore, less gas is spent).

Occurences

./Timeswap-V1-Convenience/contracts/libraries/NFTTokenURIScaffold.sol:201:        for (uint256 i = 0; i < data.length; i++) {
./Timeswap-V1-Convenience/contracts/libraries/PayMath.sol:21:        for (uint256 i; i < ids.length; i++) {
./Timeswap-V1-Core/contracts/TimeswapPair.sol:359:        for (uint256 i; i < ids.length; i++) {
./Timeswap-V1-Core/contracts/test/libraries/ArrayTest.sol:16:        for (uint256 i; i < duesStorage.length; i++) duesStorage.pop;
./Timeswap-V1-Core/contracts/test/libraries/ArrayTest.sol:18:        for (uint256 i; i < dues.length; i++) {
amateur-dev commented 2 years ago

Similar issue reported over here #151 hence, closing this issue