Cally contest - CertoraInc's results

Low and Non critical bugs

• Mistake on comment - require(dutchAuctionReserveStrike < strikeOptions[dutchAuctionStartingStrikeIndex], "Reserve strike too small"); - this is suppose to be "too big" instead of "too small"
• Fee is not taken from the premium, only from the strike payment, and this might be unwanted (in regular the protocol takes fee from every payment that is done through it)

Gas Optimizations

• Use unchecked in getDutchAuctionStrike (line 417) - it won't underflow because we know that auctionEndTimestamp > block.timestamp.

  uint256 delta = auctionEndTimestamp > block.timestamp ? auctionEndTimestamp - block.timestamp : 0;

• Use 1e36 instead of 1e18 * 1e18 in the getDutchAuctionStrike function (uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);)
• Optimize the getDutchAuctionStrike by splitting it into if statements

These three optimizations gives us these changes: old code: ```sol function getDutchAuctionStrike( uint256 startingStrike, uint32 auctionEndTimestamp, uint256 reserveStrike ) public view returns (uint256 strike) { /* delta = max(auctionEnd - currentTimestamp, 0) progress = delta / auctionDuration auctionStrike = progress^2 * startingStrike strike = max(auctionStrike, reserveStrike) */ uint256 delta = auctionEndTimestamp > block.timestamp ? auctionEndTimestamp - block.timestamp : 0; uint256 progress = (1e18 * delta) / AUCTION_DURATION; uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

    // max(auctionStrike, reserveStrike)
    strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;
}
```

new code:
```sol
function getDutchAuctionStrike(
    uint256 startingStrike,
    uint32 auctionEndTimestamp,
    uint256 reserveStrike
) public view returns (uint256 strike) {
    /*
        delta = max(auctionEnd - currentTimestamp, 0)
        progress = delta / auctionDuration
        auctionStrike = progress^2 * startingStrike
        strike = max(auctionStrike, reserveStrike)
    */
    // this is because `auctionStrike == 0` so `auctionStrike > reserveStrike == false` for every value of reserveStrike
    if (auctionEndTimestamp > block.timestamp) return reserveStrike;

    unchecked {
        uint256 delta = auctionEndTimestamp - block.timestamp; // it won't underflow because of the if statement
    }
    uint256 progress = (1e18 * delta) / AUCTION_DURATION;
    uint256 auctionStrike = (progress * progress * startingStrike) / 1e36; // instead of 1e18 * 1e18

    // max(auctionStrike, reserveStrike)
    strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;
}
```

• Inline the getPremium function to save the gas spent of function call

• Use bitwise operations to check if a number is even or odd (num & 1 instead of num % 2)

• use logic not instead of == false (this can be done twice in the buyOption function and also in the withdraw function)

• save an SLOAD on lines 188-190 by applying this optimization

• old:

  vaultIndex += 2;
  vaultId = vaultIndex;
  _vaults[vaultId] = vault;

  new:

  vaultId = vaultIndex;
  vaultId += 2;
  vaultIndex = vaultId;
  _vaults[vaultId] = vault;

• Put mapping(uint256 => address) private _vaultBeneficiaries; inside the vault struct (it won't change the size of the struct because of the 256 bits padding)

• Use auctionStartTimestamp instead of accessing vault.currentExpiration again in the buyOption function

  uint32 auctionStartTimestamp = vault.currentExpiration;
  require(block.timestamp >= auctionStartTimestamp, "Auction not started");
  
  // set new currentStrike
  vault.currentStrike = getDutchAuctionStrike(
      strikeOptions[vault.dutchAuctionStartingStrikeIndex],
      vault.currentExpiration + AUCTION_DURATION,
      vault.dutchAuctionReserveStrike
  );

• Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc. An even better and gas efficient approach will be to use Solidity Custom Errors instead of revert strings.

• Cache feeRate to save an SLOAD + redundant initialization (variables in solidity are automatically initialized to their default value)

  old:

  uint256 fee = 0;
  if (feeRate > 0) {
      fee = (msg.value * feeRate) / 1e18;
      protocolUnclaimedFees += fee;
  }

  new:

  uint256 fee;
  uint cachedFeeRate = feeRate;
  if (cachedFeeRate > 0) {
      fee = (msg.value * cachedFeeRate) / 1e18;
      protocolUnclaimedFees += fee;
  }