search

code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

Gas Optimizations #40

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Preface

Gas optimizations have been benchmarked against the current configuration. This can be done by running

forge snapshot

then, after modifications, run

forge snapshot --diff

G01: Increase solc runs

Description

The number of solc runs used for contract compilation is 1000. This number can be bumped significantly to produce more gas efficient code (max value of 2**32-1). More information can be found in the solidity docs.

Gas Saved

6 to 528

Recommended Mitigation Steps

The maximum number of runs attainable before the max contract size is exceeded is about 5000. While deployment costs will increase, functions will cost less gas to execute.

[default]
solc_version = "0.8.13"
fuzz_runs = 1000
optimizer_runs = 5000
optimizer = true

G02: Redundant TokenType check

Line References

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L171

Description

The type check on the enum TokenType is redundant because Solidity implictly enforces this check.

Gas Saved

66

Recommended Mitigation Steps

Remove the check.

G03: Redundant null ownerOf check

Line References

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L214

Description

The null ownerOf is checked in Solmate already, making this a duplicate check.

Gas Saved

253

Recommended Mitigation Steps

Remove the check. Alternatively, consider overriding the ownerOf() method to remove the null check in Solmate because a number of instances check ownerOf() against msg.sender which is guaranteed to be non-null.

G04: Dutch auction price calculation can be unchecked

Line References

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L417-L419

Description

The dutch auction price calculation can be unchecked because the variables are bounded, where, when the function is called internally, the following are true:

Therefore, we can be sure that there are no overflow in the calculations:

uint256 progress = (1e18 * delta) / AUCTION_DURATION;

means the largest value of the intermediate calculation is 1e18 * 86400

uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

means the largest value of the intermediate calculation is 1e18 * 1e18 * startingStrike = 1e18 * 1e18 * 6765 * 1e18 = 6.765e+57 which is within type(uint256).max

The assumptions made might of course not hold when called externally. Nevertheless, the gas savings seem substantial enough to consider implementing the change.

Gas Saved

382 or 421

Recommended Mitigation Steps

uint256 delta;
uint256 auctionStrike;
unchecked {
  if (auctionEndTimestamp > block.timestamp) delta = auctionEndTimestamp - block.timestamp;
  uint256 progress = (1e18 * delta) / AUCTION_DURATION;
  auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);
}

NG01: Use of IR Optimizer

Description

The new Yul-based optimizer that is shipped with 0.8.13 ”can do much higher-level optimization across functions”. However, when I turned it on together with the increase in solc runs, an increase in gas usage is observed instead.

The optimizer can be turned on by setting via_ir = true in the foundry.toml file.

outdoteth commented 2 years ago

high quality report