Vault Static Analysis Findings

VTL-01S: Illegible Numeric Value Representations

Type	Severity	Location
Code Style		Vault.sol:L22, L778, L805

Description:

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

Example:

Copy
22uint256 public constant BASIS_POINTS_DIVISOR = 10000;

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, 50_00, and 50_00 respectively in accordance with the recommendation's underscore style, addressing this exhibit.

VTL-02S: Inexistent Event Emission

Type	Severity	Location
Language Specific		Vault.sol:L111-L113, L119-L121, L123-L125, L127-L129, L131-L133, L145-L159, L161-L187, L189-L202

Description:

The linked function adjusts a sensitive contract variable yet does not emit an event for it.

Example:

Copy
111function setManager(address _manager, bool _isManager) external override onlyAdmin {
112    isManager[_manager] = _isManager;
113}

Recommendation:

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

Alleviation (b02fae335f62cc1f5f4236fb4d982ad16a32bd26):

The Moby team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

VTL-03S: Suboptimal Event Declarations

Type	Severity	Location
Gas Optimization		Vault.sol:L76, L77, L78, L80, L83, L84, L85, L86, L87, L89, L90, L91

Description:

The referenced event declarations do not have any indexed argument or have less than three indexed arguments that are a primitive type.

Example:

Copy
76event BuyUSDG(address account, address token, uint256 tokenAmount, uint256 usdgAmount, uint256 feeBasisPoints);

Recommendation:

Apart from aiding off-chain integrators in consuming and filtering such events, primitive types that are set as indexed will result in a gas optimization due to reduced memory costs. As such, we advise the indexed keyword to be introduced to up to three different primitive types in total optimizing the referenced event declarations.

Alleviation (b02fae335f62cc1f5f4236fb4d982ad16a32bd26):

The indexed keyword has been introduced to all referenced event declarations per our recommendation, addressing this exhibit.

VTL-04S: Deprecated Native Asset Transfer

Type	Severity	Location
Language Specific		Vault.sol:L240

Description:

The linked statement performs a low-level native asset transfer via the transfer function exposed by the address payable data type.

Impact:

As new EIPs such as EIP-2930 are introduced to the blockchain, gas costs can change and the transfer instruction of Solidity specifies a fixed gas stipend that is prone to failure should such changes be integrated to the blockchain the contract is deployed in. A prime example of this behaviour are legacy versions of Gnosis which were susceptible to this issue and would cause native transfers to fail if sent to a new address.

Example:

Copy
240payable(receiver).transfer(_amount);

Recommendation:

We advise alternative ways of transferring assets to be utilized instead, such as OpenZeppelin's Address.sol library and in particular the sendValue method exposed by it. If re-entrancies are desired to be prevented based on gas costs, we instead advise a mechanism to be put in place that either credits an account with a native balance they can withdraw at a secondary transaction or that performs the native asset transfers at the end of the top-level transaction's execution.

Alleviation (a8720219a6a97e10b8d9c6a70c6345747f0fdcb3):

The Vault::emergencyRecovery function that the statement was present in has been commented out of the codebase, rendering this exhibit inapplicable.

VTL-05S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		Vault.sol:L145-L159

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

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:

Copy
145function setContracts(
146    IVaultUtils _vaultUtils,
147    address _optionsMarket,
148    address _positionManager,
149    address _controller,
150    address _vaultPriceFeed,
151    address _usdg
152) external override onlyAdmin {
153    vaultUtils = _vaultUtils;
154    optionsMarket = _optionsMarket;
155    positionManager = _positionManager;
156    controller = _controller;
157    vaultPriceFeed = _vaultPriceFeed;
158    usdg = _usdg;
159}

Recommendation:

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

Alleviation (b02fae335f62cc1f5f4236fb4d982ad16a32bd26):

All input arguments of the Vault::setContracts function are adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.