code-423n4 / 2022-04-phuture-findings

0 stars 0 forks source link

Gas Optimizations #89

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Report

Table of Contents

Caching storage variables in memory to save gas

IMPACT

Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.

In particular, in for loops, when using the length of a storage array as the condition being checked after each loop, caching the array length in memory can yield significant gas savings if the array length is high

PROOF OF CONCEPT

Instances include:

IndexLogic.sol

scope: mint()

IndexLogic.sol:32
IndexLogic.sol:35
IndexLogic.sol:40
IndexLogic.sol:61
IndexLogic.sol:39
IndexLogic.sol:60
IndexLogic.sol:47
IndexLogic.sol:63

scope: burn()

IndexLogic.sol:103
IndexLogic.sol:110
IndexLogic.sol:123
IndexLogic.sol:127
IndexLogic.sol:125
IndexLogic.sol:131

ManagedIndexReweightingLogic.sol

scope: reweight()

ManagedIndexReweightingLogic.sol:32
ManagedIndexReweightingLogic.sol:37
ManagedIndexReweightingLogic.sol:45
ManagedIndexReweightingLogic.sol:62
ManagedIndexReweightingLogic.sol:38
ManagedIndexReweightingLogic.sol:40
ManagedIndexReweightingLogic.sol:76
ManagedIndexReweightingLogic.sol:40

TopNMarketCapReweightingLogic.sol

scope: reweight()

TopNMarketCapReweightingLogic.sol:35
TopNMarketCapReweightingLogic.sol:47
TopNMarketCapReweightingLogic.sol:67
TopNMarketCapReweightingLogic.sol:37
TopNMarketCapReweightingLogic.sol:39
TopNMarketCapReweightingLogic.sol:54
TopNMarketCapReweightingLogic.sol:105

TrackedIndexReweightingLogic.sol

scope: reweight()

TrackedIndexReweightingLogic.sol:29
TrackedIndexReweightingLogic.sol:30
TrackedIndexReweightingLogic.sol:38
TrackedIndexReweightingLogic.sol:63
TrackedIndexReweightingLogic.sol:37
TrackedIndexReweightingLogic.sol:66
TrackedIndexReweightingLogic.sol:41
TrackedIndexReweightingLogic.sol:70

UniswapV2PathPriceOracle.sol

scope: refreshedAssetPerBaseInUQ()

UniswapV2PathPriceOracle.sol:34

scope: lastAssetPerBaseInUQ()

UniswapV2PathPriceOracle.sol:49

vToken.sol

scope: _transferAsset()

vToken.sol:218
vToken.sol:219

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Comparisons with zero for unsigned integers

IMPACT

>0 is less gas efficient than !0 if you enable the optimizer at 10k AND you’re in a require statement. Detailed explanation with the opcodes here

PROOF OF CONCEPT

Instances include:

IndexLogic.sol

IndexLogic.sol:76
IndexLogic.sol:98

TOOLS USED

Manual Analysis

MITIGATION

Replace > 0 with !0

Comparison Operators

IMPACT

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =.

Using strict comparison operators hence saves gas

PROOF OF CONCEPT

Instances include:

ManagedIndexReweightingLogic.sol

ManagedIndexReweightingLogic.sol:32

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:24

UniswapV2PriceOracle.sol

UniswapV2PriceOracle.sol:65

TOOLS USED

Manual Analysis

MITIGATION

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable

example:

-_updatedAssets.length <= IIndexRegistry(registry).maxComponents();
+_updatedAssets.length < IIndexRegistry(registry).maxComponents() + 1;

When the comparison is with a constant storage variable, you can also do the increment in the storage variable declaration

example:

-if (timeElapsed >= MIN_UPDATE_INTERVAL);
+if (timeElapsed > MIN_UPDATE_INTERVAL_MINUS_ONE);

and

-uint private constant MIN_UPDATE_INTERVAL = 24 hours;
+uint private constant MIN_UPDATE_INTERVAL_MINUS_ONE = 24 hours - 1;

However in this case, the 1 second difference is negligible compared to the value of MIN_UPDATE_INTERVAL, so the following is the best option:

-if (timeElapsed >= MIN_UPDATE_INTERVAL);
+if (timeElapsed > MIN_UPDATE_INTERVAL);

and MIN_UPDATE_INTERVAL remains unchanged

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

BaseIndex.sol

BaseIndex.sol:34
BaseIndex.sol:49
BaseIndex.sol:65

ChainlinkPriceOracle.sol

ChainlinkPriceOracle.sol:51
ChainlinkPriceOracle.sol:61
ChainlinkPriceOracle.sol:62
ChainlinkPriceOracle.sol:86

IndexLogic.sol

IndexLogic.sol:40
IndexLogic.sol:76
IndexLogic.sol:98

ManagedIndex.sol

ManagedIndex.sol:28
ManagedIndex.sol:44
ManagedIndex.sol:54

ManagedIndexReweightingLogic.sol

ManagedIndexReweightingLogic.sol:29
ManagedIndexReweightingLogic.sol:52
ManagedIndexReweightingLogic.sol:58
ManagedIndexReweightingLogic.sol:62
ManagedIndexReweightingLogic.sol:85
ManagedIndexReweightingLogic.sol:104

PhuturePriceOracle.sol

PhuturePriceOracle.sol:46
PhuturePriceOracle.sol:47
PhuturePriceOracle.sol:56
PhuturePriceOracle.sol:63
PhuturePriceOracle.sol:83
PhuturePriceOracle.sol:93

TopNMarketCapIndex.sol

TopNMarketCapIndex.sol:45
TopNMarketCapIndex.sol:55
TopNMarketCapIndex.sol:73

TopNMarketCapReweightingLogic.sol

TopNMarketCapReweightingLogic.sol:67

TrackedIndex.sol

TrackedIndex.sol:30
TrackedIndex.sol:63

TrackedIndexReweightingLogic.sol

TrackedIndexReweightingLogic.sol:38

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:24
UniswapV2PathPriceOracle.sol:25

UniswapV2PriceOracle.sol

UniswapV2PriceOracle.sol:46
UniswapV2PriceOracle.sol:83

vToken.sol

vToken.sol:59
vToken.sol:60
vToken.sol:71

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance, in TopNMarketCapIndex.sol:

Replace

require(msg.sender == factory, "TopNMarketCapIndex: FORBIDDEN");

with

if (msg.sender != factory) {
        revert IsNotFactory(msg.sender);
}

and define the custom error in the contract

error IsNotFactory(address _address);

Dead code

IMPACT

Functions that are not used should be removed as they increase the gas cost during deployment

PROOF OF CONCEPT

Instances include:

IndexLibrary.sol

IndexLibrary.sol:24:  //amountInAsset() is never used in the contracts.

TOOLS USED

Manual Analysis

MITIGATION

Remove amountInAsset()

Default value initialization

IMPACT

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

PROOF OF CONCEPT

Instances include:

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:34
UniswapV2PathPriceOracle.sol:49

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments.

PROOF OF CONCEPT

Instances include:

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:34
UniswapV2PathPriceOracle.sol:49

FullMath.sol

UniswapV2PathPriceOracle.sol:124

TOOLS USED

Manual Analysis

MITIGATION

change i++ to ++i in UniswapV2PathPriceOracle.sol.

change result++ to ++result in FullMath.sol

Require instead of &&

IMPACT

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

PROOF OF CONCEPT

Instances include:

ManagedIndexReweightingLogic.sol

ManagedIndexReweightingLogic:29:
require(
            _updatedAssets.length > 1 &&
                _updatedWeights.length == _updatedAssets.length &&
                _updatedAssets.length <= IIndexRegistry(registry).maxComponents(),
            "ManagedIndex: INVALID"
        );

TOOLS USED

Manual Analysis

MITIGATION

require(_updatedAssets.length > 1);
require(_updatedWeights.length == _updatedAssets.length);
require(_updatedAssets.length <= IIndexRegistry(registry).maxComponents(),"ManagedIndex: INVALID");

Unchecked arithmetic

IMPACT

The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.

if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas

PROOF OF CONCEPT

Instances include:

BaseIndex.sol

BaseIndex.sol:78: i bounded by _assets.length, overflow check unnecessary

IndexLogic.sol

IndexLogic.sol:39: i bounded by assets.length, overflow check unnecessary
IndexLogic.sol:60: i bounded by inactiveAssets.length, overflow check unnecessary
IndexLogic.sol:102: i bounded by assets.length, overflow check unnecessary
IndexLogic.sol:125: i bounded by assets.length+inactiveAssets.length, overflow check unnecessary

ManagedIndex.sol

ManagedIndex.sol:30: i bounded by _assets.length, overflow check unnecessary

ManagedIndexReweightingLogic.sol

ManagedIndexReweightingLogic.sol:38: i bounded by assets.length, overflow check unnecessary
ManagedIndexReweightingLogic.sol:50: i bounded by _updatedAssets.length, overflow check unnecessary
ManagedIndexReweightingLogic.sol:96: i bounded by _inactiveAssets.length, overflow check unnecessary

TopNMarketCapIndex.sol

TopNMarketCapIndex.sol:48: i bounded by _assets.length, overflow check unnecessary
TopNMarketCapIndex.sol:49: i bounded by _assets.length_ underflow check unnecessary

TopNMarketCapReweightingLogic.sol

TopNMarketCapReweightingLogic.sol:37: i bounded by assets.length, overflow check unnecessary
TopNMarketCapReweightingLogic.sol:51: i bounded by diff.assetCount, overflow check unnecessary
TopNMarketCapReweightingLogic.sol:104: i bounded by _inactiveAssets.length, overflow check unnecessary

TrackedIndex.sol

TrackedIndex.sol:35: i bounded by _assets.length, overflow check unnecessary

TrackedIndexReweightingLogic.sol

TrackedIndexReweightingLogic.sol:37: i bounded by assets.length, overflow check unnecessary
TrackedIndexReweightingLogic.sol:66: i bounded by assets.length, overflow check unnecessary

UniswapV2PathPriceOracle.sol

UniswapV2PathPriceOracle.sol:34: i bounded by path.length, overflow check unnecessary
UniswapV2PathPriceOracle.sol:49: i bounded by path.length, overflow check unnecessary

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block

Unnecessary computation

IMPACT

Inlining computation can save gas

PROOF OF CONCEPT

Instances include:

IndexLogic.sol

IndexLogic.sol:56:
uint balanceInBase = lastBalanceInAsset.mulDiv(FixedPoint112.Q112, assetPerBaseInUQ);
lastAssetBalanceInBase += balanceInBase;

TOOLS USED

Manual Analysis

MITIGATION

lastAssetBalanceInBase += lastBalanceInAsset.mulDiv(FixedPoint112.Q112, assetPerBaseInUQ);

Unnecessary check

IMPACT

we can remove if statements where unnecessary to save gas.

PROOF OF CONCEPT

BaseIndex.sol

BaseIndex.sol:51:
if (_minAmountInBase < minAmountInBase) {
  minAmountInBase = _minAmountInBase;
}

line 38, minAmountInBase = type(uint).max, hence the condition check line 51 is redundant.

TOOLS USED

Manual Analysis

MITIGATION

Remove the if condition check

minAmountInBase = _minAmountInBase;