QA Report

[code423n4]

(Low) feeRate can be modified for existing vaults

feeRate is a parameter that controls the fee applied on exercise. It can be set by the function:

function setFee(uint256 feeRate_) external onlyOwner {
    feeRate = feeRate_; 
} 

So it can be changed by the owner at any time, changing also the fee payed by existing vaults.

Proof of concept

Alice is a trader looking for a delta neutral position on her NFTs. She opens a vault with strike 10 ETH and fee 0.5%. She's happy getting 9.95 ETH if the option is exercised. During the call lifetime the fee gets increased up to 1%. Now Alice will get 9.90 ETH, an amount which may bring her EV negative.

Recommendations

It's suggested to save feeRate into a vault struct during createVault or buyOption. This value can then be used during exercise instead of the global variable.

(Low) Mistake in tokenURI encoding

In the tokenURI string calculation, we need to pass the vault premium. At line L464 there's a getPremium(vault.premiumIndex) but the function actually wants a vaultId:

    /// @notice Get the fixed option premium for a vault
    /// @param vaultId The tokenId of the vault to fetch the premium for
    /// @return premium The premium for the vault
    function getPremium(uint256 vaultId) public view returns (uint256 premium) {
        Vault memory vault = _vaults[vaultId];
        return premiumOptions[vault.premiumIndex];
    }

As a consequence the premium gets mispriced on UI, leading to unpredictable errors.

Recommendations

Change L464 to premiumOptions[vault.premiumIndex].

(Info) Wrong math terminology: strike curve is not "exponential"

Documentation (L400) claims that during dutch auction, strike decreases in time "exponentially". This term is correct colloquially, but it's technically misused since the curve is actually a parabola instead of an exponential.

Suggested removing the adverb or finding a more appropriate one.

(Info) Missing event when changing feeRate

The function setFee doesn't emit an event when changing feeRate. It's generally suggested to add events for every important parameter that can get changed, for easier off-chain monitoring.

[outdoteth]

this can be updated to be medium severity: (Low) feeRate can be modified for existing vaults; https://github.com/code-423n4/2022-05-cally-findings/issues/47