Omniscia Moby Audit

VaultUtils Static Analysis Findings

VaultUtils Static Analysis Findings

VUS-01S: Illegible Numeric Value Representations

TypeSeverityLocation
Code StyleVaultUtils.sol:L15, L17

Description:

The linked representations of numeric literals are sub-optimally represented decreasing the legibility of the codebase.

Example:

contracts/VaultUtils.sol
15uint256 public constant BASIS_POINTS_DIVISOR = 10000; // 100%

Recommendation:

To properly illustrate each value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The referenced value literals have been updated in their representation to 100_00, and 5_00 respectively in accordance with the recommendation's underscore style, addressing this exhibit.

VUS-02S: Inexistent Event Emissions

Description:

The linked functions adjust sensitive contract variables yet do not emit an event for it.

Example:

contracts/VaultUtils.sol
274function setReleaseDuration(address _token, uint256 _releaseDuration, PriceType _priceType) public override onlyKeeper {
275 releaseDuration[_priceType][_token] = _releaseDuration;
276}

Recommendation:

We advise an event to be declared and correspondingly emitted for each function to ensure off-chain processes can properly react to this system adjustment.

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The SetFees, and SetReferral events were introduced to the codebase and are correspondingly emitted in the VaultUtils::setFees, and VaultUtils::setReferral functions respectively, addressing this exhibit in full.

VUS-03S: Literal Equality of bool Variables

Description:

The linked bool comparisons are performed between variables and bool literals.

Example:

contracts/VaultUtils.sol
426require(isPositionAtSelfExpirySettled[_expiry] == false, "Vault: position already settled");

Recommendation:

We advise each bool variable to be utilized directly either in its negated (!) or original form.

Alleviation (b02fae335f):

Only the first of the two direct bool comparisons has been addressed, rendering this exhibit partially alleviated.

Alleviation (a8720219a6):

The second bool comparison has been corrected as well using the bool in its negated form directly.

As such, we consider this exhibit fully addressed.

VUS-04S: Inexistent Sanitization of Input Address

Description:

The linked function accepts an address argument yet does not properly sanitize it.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

contracts/VaultUtils.sol
52function initialize(
53 address _vault,
54 IOptionsAuthority _authority
55) external initializer {
56 __Ownable_init();
57 __AuthorityUtil_init__(_authority);
58
59 vault = _vault;
60
61 feeBasisPoints[FeeType.OpenPositionFee] = 3; // 0.03%
62 feeBasisPoints[FeeType.ClosePositionFee] = 3; // 0.03%
63 feeBasisPoints[FeeType.SettlePositionFee] = 2; // 0.02%
64 feeBasisPoints[FeeType.TaxFee] = 60 ; // 0.6%
65 feeBasisPoints[FeeType.StableTaxFee] = 20; // 0.2%
66 feeBasisPoints[FeeType.MintBurnFee] = 30; // 0.3%
67 feeBasisPoints[FeeType.SwapFee] = 30; // 0.3%
68 feeBasisPoints[FeeType.StableSwapFee] = 20; // 0.2%
69
70 hasDynamicFees = true;
71}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that the address specified is non-zero.

Alleviation (b02fae335f62cc1f5f4236fb4d982ad16a32bd26):

The input _vault address argument of the VaultUtils::initialize function is adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.