Description

In order for a pool owner to withdraw their assets, the contract must loop through all issued options, even expired ones. However, as the number of options written increases, the gas cost of the loop also increases. Eventually, the cost can become too high, preventing the owner from withdrawing their assets.

Impact

In order to withdraw, the availableBalance is checked:

function availableBalance() view public override returns(uint256) {
    uint256 balance = token.balanceOf(address(this));
    uint256[] memory optionIds = getOptionIds(); // grows in size per option written
    for (uint256 i = 0; i < optionIds.length; i++) {
        WasabiStructs.OptionData memory optionData = getOptionData(optionIds[i]);
        if (optionData.optionType == WasabiStructs.OptionType.PUT && isValid(optionIds[i])) {
            balance -= optionData.strikePrice;
        }
    }
    return balance;
}

The code currently iterates through every option issued, but it does not set a limit on the number of options that may need to be looped through. This risks a denial of service, leaving pool owners unable to access their funds. As it currently stands, there is no way to remove expired options from the enumerable set of options.

Recommendations

We recommend including a public clearExpiredOptions function that iterates through options of a specified index range and deletes them from the enumerable set if they are expired. Perhaps even creating some sort of incentive mechanism to encourage users to use this function may be beneficial.

Remediation

This issue has been acknowledged by Wasabi, and a fix was implemented in commit eea7c164↗.