Omniscia Moby Audit

VaultUtils Code Style Findings

VaultUtils Code Style Findings

VUS-01C: Ineffectual Usage of Safe Arithmetics

Description:

The linked mathematical operation is guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

contracts/VaultUtils.sol
128if (isNakedCall && (strikePricesWithPrecision[0] < _settlePrice)) {
129    isItm = true;
130    intrinsicUsd = _settlePrice - strikePricesWithPrecision[0];
131}

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation (a95db4124c4689f421fc3fd505ffb91173355034):

All arithmetic operations referenced by the exhibit have been safely wrapped in unchecked code blocks.

VUS-02C: Inefficient Array Copy

Description:

The referenced loop will instantiate a new array and copy a subset of elements from a different array from index 0 onwards.

Example:

contracts/VaultUtils.sol
83function getWhitelistedTokens() public override view returns (address[] memory) {
84    uint256 length = IVault(vault).whitelistedTokensLength();
85    address[] memory whitelistedTokens = new address[](length);
86

87    uint256 counter = 0;        
88

89    for (uint256 i = 0; i < length; i++) {
90        address token = IVault(vault).whitelistedTokens(i);
91        bool isWhitelisted = IVault(vault).isWhitelistedToken(token);
92

93        if (isWhitelisted) {
94            whitelistedTokens[counter] = token;
95            counter++;
96        }
97    }
98

99    address[] memory finalWhitelistedTokens = new address[](counter);
100

101    for (uint256 i = 0; i < counter; i++) {
102        finalWhitelistedTokens[i] = whitelistedTokens[i];
103    }
104

105    return finalWhitelistedTokens;
106}

Recommendation:

We advise the whitelistedTokens array to be mutated directly and specifically its length to be reduced via an assembly block which is considered a memory safe statement.

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The assembly based array length mutation has been applied as advised, greatly optimizing the function's gas cost.

VUS-03C: Inefficient Array Shifts

Description:

The referenced array shifts will be performed regardless of the indexes being swapped.

Example:

contracts/VaultUtils.sol
548while (i < _expiries.length) {
549    if (_expiries[i] <= currentTime) {
550        _expiriesToSettle.push(_expiries[i]);
551        _expiries[i] = _expiries[_expiries.length - 1];
552        _expiries.pop();
553    } else {
554        i++;
555    }
556}

Recommendation:

We advise the referenced statements to be performed solely when the index being assigned to is different than the index being removed.

Alleviation (a95db4124c4689f421fc3fd505ffb91173355034):

The VaultUtils::_removeExpiry function will adequately ignore the shift operation if the index and lastIndex are equal, optimizing the code's gas cost as advised.

VUS-04C: Inefficient mapping Lookups

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

contracts/VaultUtils.sol
279function notifyPendingAmount(PriceType _priceType, address _token, uint256 _pendingAmount) external override onlyVault  {
280    uint256 leftover = _priceType == IVaultUtils.PriceType.MP ? IVault(vault).pendingMpAmounts(_token) : IVault(vault).pendingRpAmounts(_token);
281    releaseRate[_priceType][_token] = (_pendingAmount + leftover) / releaseDuration[_priceType][_token];
282

283    lastUpdateTime[_priceType][_token] = block.timestamp;
284    periodFinish[_priceType][_token] = block.timestamp + releaseDuration[_priceType][_token];
285}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation (a95db4124c4689f421fc3fd505ffb91173355034):

All referenced inefficient mapping lookups have been optimized to the greatest extent possible, significantly reducing the gas cost of the functions the statements were located in.

VUS-05C: Loop Iterator Optimizations

Description:

The linked for loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics (post-0.8.X).

Example:

contracts/VaultUtils.sol
89for (uint256 i = 0; i < length; i++) {

Recommendation:

We advise the increment / decrement operations to be performed in an unchecked code block as the last statement within each for loop to optimize their execution cost.

Alleviation (a95db4124c4689f421fc3fd505ffb91173355034):

The iterator optimization has been applied to the referenced for loops as advised.

VUS-06C: Redundant Code Repetition

Description:

The referenced functions perform the exact same statements albeit using different mapping entries as well as invoking different functions.

Example:

contracts/VaultUtils.sol
425function settleSelfOriginatedPositions(uint256 _expiry, uint256 _endLength) external onlyKeeper returns (bool) {
426    require(isPositionAtSelfExpirySettled[_expiry] == false, "Vault: position already settled");
427

428    uint256 index = optionTokensAtSelfExpiryStart[_expiry]; // 0, 1, 2
429    uint256 length = optionTokensAtSelfExpiry[_expiry].length; // 3
430

431    if (index == length) {
432        _removeExpiryFromArray(_expiry, true, true);
433        isPositionAtSelfExpirySettled[_expiry] = true;
434        return true;
435    }
436    
437    if (_endLength > length) {
438        _endLength = length;
439    }
440

441    while(index < _endLength) {
442        IVaultUtils.OptionToken memory token = optionTokensAtSelfExpiry[_expiry][index];
443

444        address quotetoken = _getQuoteTokenForSettlement(token.optionTokenId);
445        address[] memory path = new address[](1);
446        path[0] = quotetoken;
447

448        uint16 underlyingAssetIndex = Utils.getUnderlyingAssetIndexByOptionTokenId(token.optionTokenId);
449        
450        bool shouldContinue = true;
451

452        try IVault(vault).settleVaultPosition(
453            path,
454            underlyingAssetIndex,
455            token.optionTokenId
456        ) returns (bool isSettled) {
457            if (isSettled) IVault(vault).directPoolDeposit(quotetoken);
458        } catch Error(string memory reason) {
459            shouldContinue = false;
460

461            emit CatchError(reason);
462        }
463

464        if (shouldContinue) {
465            delete optionTokensAtSelfExpiry[_expiry][index];
466            index++;
467        } else {
468            break;
469        }
470    }
471

472    optionTokensAtSelfExpiryStart[_expiry] = index;
473

474    if (index == length) {
475        _removeExpiryFromArray(_expiry, true, true);
476        isPositionAtSelfExpirySettled[_expiry] = true;
477        emit VaultPositionSettled(address(vault), _expiry);
478    }
479

480    return true;
481}
482

483function settleExternalOriginatedPositions(uint256 _expiry, uint256 _endLength) external onlyKeeper returns (bool) {
484    require(isPositionAtExternalExpirySettled[_expiry] == false, "Vault: position already settled");
485

486    uint256 index = optionTokensAtExternalExpiryStart[_expiry]; // 0, 1, 2
487    uint256 length = optionTokensAtExternalExpiry[_expiry].length; // 3
488

489    if (index == length) {
490        _removeExpiryFromArray(_expiry, false, true);
491        isPositionAtExternalExpirySettled[_expiry] = true;
492        return true;
493    }
494    
495    if (_endLength > length) {
496        _endLength = length;
497    }
498

499    while(index < _endLength) {
500        IVaultUtils.OptionToken memory token = optionTokensAtExternalExpiry[_expiry][index];
501

502        address quotetoken = _getQuoteTokenForSettlement(token.optionTokenId);
503        address[] memory path = new address[](1);
504        path[0] = quotetoken;
505

506        uint16 underlyingAssetIndex = Utils.getUnderlyingAssetIndexByOptionTokenId(token.optionTokenId);
507

508        bool shouldContinue = true;
509

510        try IVault(vault).settleVaultPosition(
511            path,
512            underlyingAssetIndex,
513            token.optionTokenId
514        ) returns (bool isSettled) {
515            if (isSettled) IVault(vault).directPoolDeposit(quotetoken);
516        } catch {
517            shouldContinue = false;
518        }
519

520        if (shouldContinue) {
521            delete optionTokensAtExternalExpiry[_expiry][index];
522            index++;
523        } else {
524            break;
525        }
526    }
527

528    optionTokensAtExternalExpiryStart[_expiry] = index;
529

530    if (index == length) {
531        _removeExpiryFromArray(_expiry, false, true);
532        isPositionAtExternalExpirySettled[_expiry] = true;
533        emit VaultPendingPositionSettled(address(vault), _expiry);
534    }
535

536    return true;
537}

Recommendation:

We advise the statements to be relocated to a single internal function that is underscore-prefixed (_), permitting it to be invoked with the relevant mapping and potentially function arguments to execute as the original implementations did.

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The Moby team evaluated this optimization and opted to not apply it as it would be too complex to incorporate.

VUS-07C: Redundant Parenthesis Statements

Description:

The referenced statements are redundantly wrapped in parenthesis' (()).

Example:

contracts/VaultUtils.sol
128if (isNakedCall && (strikePricesWithPrecision[0] < _settlePrice)) {

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The redundant parenthesis in the referenced statements have been safely omitted.

Prev

VaultPriceFeed.sol (VPF-C)

Next

ViewAggregator.sol (VAR-C)

On this page