Use a temporary variable to cache repetitive storage reads

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

In function transferERC721(), the array value stored in a mapping timelockERC721Keys[nftContract][i] is read three times in three different places within the loop iteration. This consumes a lot of unnecessary gas because SLOADs are expensive. This can be prevented by saving the value timelockERC721Keys[nftContract][i] in a temporary bytes32 variable at the beginning of the iteration and using that instead.

Proof of Concept

Declaration: https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L82

Use 1: https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L505

Use 2: https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L507

Use 3: https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L510

Tools Used

Manual Analysis

Recommended Mitigation Steps

Save the value timelockERC721Keys[nftContract][i] in a temporary bytes32 variable at the beginning of the iteration and using that instead

[ghost]

sponsor confirmed We will be applying this optimization in our next version

[ztcrypto]

path link