Lodestar-Finance / lodestar-protocol

Houses the code for the Lodestar Finance DeFi protocol.
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

Gas savings report #23

Open maarcweiss opened 1 year ago

maarcweiss commented 1 year ago

TITLE (Gas savings )

ISSUE 1 VARIABLES SHOULD BE IMMUTABLE

https://github.com/LodestarFinance/lodestar-token-fix/blob/8807ec57f88b96421e97b0011833897ac48b4bb6/contracts/TokenFIx.sol#L11-L12

The following variables should be market as immutable because they are initialized in the constructor and can't be changed after:

IERC20 public oldToken;
IERC20 public newToken;

SEVERITY (either high or medium, see the rules)

Gas

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/lodestar-token-fix/blob/8807ec57f88b96421e97b0011833897ac48b4bb6/contracts/TokenFIx.sol#L11-L12

SOLUTION

IERC20 public immutable oldToken; IERC20 public immutable newToken;

ISSUE 2 FOR LOOP OPTIMAZATION

https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/PriceOracleProxyETH.sol#L237

Add an unchecked keyword to the i++ on the for loop

SEVERITY (either high or medium, see the rules)

Gas

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/PriceOracleProxyETH.sol#L237

https://github.com/LodestarFinance/plvglp-oracle/blob/b9e78d6bfb119ee1703a7e97d283a1e1dcf25046/contracts/plvGLPOracle.sol#L99

SOLUTION

  for (uint256 i = 0; i < cTokenAddresses.length;) {
      if (sources[i] != address(0)) {
            require(msg.sender == admin, "Only the admin or guardian can clear the aggregators");
        }
   unchecked{
        i++
       } 
    }

ISSUE 3 DON'T INITIALIZE VARIABLES TO 0

[https://github.com/LodestarFinance/lodestar-token-fix/blob/8807ec57f88b96421e97b0011833897ac48b4bb6/contracts/TokenFIx.sol#L11-L12

      for (uint256 i = 0; i < latestIndexing; i++) {
            sum += HistoricalIndices[i].recordedIndex;
        }

Should not have uint i = 0 instead uint i

SEVERITY (either high or medium, see the rules)

Gas

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/plvglp-oracle/blob/b9e78d6bfb119ee1703a7e97d283a1e1dcf25046/contracts/plvGLPOracle.sol#L92

SOLUTION

for (uint256 i; i < latestIndexing; i++) { sum += HistoricalIndices[i].recordedIndex; }

ISSUE 4 USE REVERT INSTEAD OF REQUIRE

(https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/PriceOracleProxyETH.sol#L219)

 require(_newGlpOracle.isGLPOracle(), "Invalid Contract");

          Instead it should be if(!_newGlpOracle.isGLPOracle()) revert;

SEVERITY (either high or medium, see the rules)

Gas

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/PriceOracleProxyETH.sol#L219

SOLUTION

      if(!_newGlpOracle.isGLPOracle()) revert
LodestarFinance commented 1 year ago

Please provide summary with total gas savings for these proposed changes in terms of contract calls and deployment costs. Please provide relevant sources to substantiate submissions wherever possible.