An array's length should be cached to save gas in for-loops

[code423n4]

Handle

Dravee

Vulnerability details

Impact

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Proof of Concept

./NFTXEligibilityManager.sol:85:        for (uint256 i = 0; i < modulesCopy.length; i++) {
./NFTXLPStaking.sol:81:        for (uint256 i = 0; i < vaultIds.length; i++) {
./NFTXLPStaking.sol:206:        for (uint256 i = 0; i < vaultIds.length; i++) {
./NFTXMarketplaceZap.sol:263:    for (uint256 i = 0; i < idsIn.length; i++) {
./NFTXMarketplaceZap.sol:297:    for (uint256 i = 0; i < idsIn.length; i++) {
./NFTXMarketplaceZap.sol:379:    for (uint256 i = 0; i < ids.length; i++) {
./NFTXMarketplaceZap.sol:399:    for (uint256 i = 0; i < ids.length; i++) {
./NFTXMarketplaceZap.sol:414:    for (uint256 i = 0; i < ids.length; i++) {
./NFTXMarketplaceZap.sol:437:    for (uint256 i = 0; i < idsIn.length; i++) {
./NFTXSimpleFeeDistributor.sol:62:    for (uint256 i = 0; i < length; i++) {
./NFTXStakingZap.sol:192:    for (uint256 i = 0; i < tokenIds.length; i++) {
./NFTXStakingZap.sol:203:    for (uint256 i = 0; i < tokenIds.length; i++) {
./NFTXStakingZap.sol:341:    for (uint256 i = 0; i < ids.length; i++) {
./NFTXVaultUpgradeable.sol:267:            for (uint256 i = 0; i < tokenIds.length; i++) {
./NFTXVaultUpgradeable.sol:406:            for (uint256 i = 0; i < tokenIds.length; i++) {
./NFTXVaultUpgradeable.sol:419:            for (uint256 i = 0; i < tokenIds.length; i++) {

Tools Used

VS Code

Recommended Mitigation Steps

Store the array's length in a variable before the for-loop, and use it instead. As an example, it's already done here: https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L362-L364

[dmvt]

duplicate of #198