HedgeVault Manual Review Findings

HVT-01M: Unused Code

Type	Severity	Location
Logical Fault		HedgeVault.sol:L91-L94

Description:

The HedgeVault::onlyController modifier remains unused within the codebase, rendering the controller notion a contract variable that can be maintained yet is not utilized.

Example:

Copy
91modifier onlyController() {
92    if (msg.sender != controller) revert OnlyController();
93    _;
94}

Recommendation:

We advise the controller variable's purpose to be clearly defined and the relevant modifier to either be omitted or properly used in the code.

Alleviation (eb8a76b31db4a988b7437048e5866795f978ff91):

The referenced unused code has been safely omitted, optimizing the code's bytecode size.

HVT-02M: Inexistent Reflection of Pause State in All EIP-4626 Limits

Type	Severity	Location
Standard Conformity		HedgeVault.sol: • I-1: L267 • I-2: L276

Description:

Per the EIP-4626 standard, all inflow / outflow limitations should reflect a value of 0 if such functionality is temporarily unavailable (i.e. in case it is paused).

The HedgeVault does not accommodate for such scenarios thereby not complying with the standard.

Impact:

The inflow / outflow EIP-4626 limits of the HedgeVault do not reflect the contract's paused states causing it to deviate from the EIP-4626 standard.

Example:

Copy
261/**
262 * @notice Updates the withdrawal pause status
263 * @param paused New pause status
264 */
265
266function setWithdrawalsPaused(bool paused) external override onlyRole(DEFAULT_ADMIN_ROLE) {
267    withdrawalsPaused = paused;
268    emit WithdrawalsStatusUpdated(paused);
269}
270/**
271 * @notice Updates the deposit pause status
272 * @param paused New pause status
273 */
274
275function setDepositsPaused(bool paused) external override onlyRole(DEFAULT_ADMIN_ROLE) {
276    depositsPaused = paused;
277    emit DepositsStatusUpdated(paused);
278}

Recommendation:

We advise the relevant functions to be properly overridden and the pause state of each functionality in the contract to be evaluated, yielding appropriate limits for each corresponding EIP-4626 interaction.

Alleviation (eb8a76b31db4a988b7437048e5866795f978ff91):

The relevant EIP-4626 functions have been properly overridden to yield 0 limits if the deposit or withdrawal mechanisms are paused in each respective function, addressing this exhibit in full.

HVT-03M: Improper Enforcement of Total Deposit Limits

Type	Severity	Location
Standard Conformity		HedgeVault.sol: • I-1: L129-L131 • I-2: L147-L149

Description:

Per the EIP-4626 standard, the deposit and mint functions should allow values up to the IERC4626::maxDeposit and IERC4626::maxMint function results.

The HedgeVault does not override these functions thereby breaching the standard and enforcing custom limitations via the deposit and mint functions directly.

Impact:

Integrators of the HedgeVault will fail to interact with it in certain circumstances as the deposit limit of the vault is not properly reflected in EIP-4626 functions using MUST terminology.

Example:

Copy
123/**
124 * @notice Deposits tokens into the vault
125 * @dev Updates internal accounting of total assets
126 */
127function deposit(uint256 assets, address receiver) public override(ERC4626Upgradeable, IHedgeVault) nonReentrant returns (uint256) {
128    if (depositsPaused) revert DepositsArePaused();
129    if (_totalDeposits + assets > maxTotalDeposit) {
130        revert MaxTotalDepositExceeded();
131    }
132
133    uint256 shares = super.deposit(assets, receiver);
134    _totalAssets += assets;
135    _totalDeposits += assets;
136    return shares;
137}
138/**
139 * @notice Mints vault shares
140 * @dev Updates internal accounting of total assets
141 */
142
143function mint(uint256 shares, address receiver) public override(ERC4626Upgradeable, IHedgeVault) nonReentrant returns (uint256) {
144    if (depositsPaused) revert DepositsArePaused();
145
146    uint256 assets = previewMint(shares);
147    if (_totalDeposits + assets > maxTotalDeposit) {
148        revert MaxTotalDepositExceeded();
149    }
150
151    uint256 actualAssets = super.mint(shares, receiver);
152    _totalAssets += actualAssets;
153    _totalDeposits += actualAssets;
154
155    return actualAssets;
156}

Recommendation:

We advise those functions to be properly overridden, permitting the deposit limits imposed on the HedgeVault::deposit and HedgeVault::mint functions to be omitted and ensuring the contract is properly compatible with the EIP-4626 standard.

Alleviation (eb8a76b31d):

While the relevant EIP-4626 max limits have been properly configured, the HedgeVault::deposit and HedgeVault::mint functions continue to locally impose those limits inefficiently given that they are already imposed by the ERC4626Upgradeable implementation.

We advise the maxTotalDeposit limitation enforcement to be removed locally, further optimizing the code's gas costs.

Alleviation (af82504254):

The maxTotalDeposit local limitations have been removed as advised, optimizing the code further and thus rendering this exhibit fully alleviated.

HVT-04M: Inexistent Reflection of Strategy Funds in Withdrawal / Redemption Limits

Type	Severity	Location
Standard Conformity		HedgeVault.sol:L213

Description:

Per the EIP-4626 standard, the withdraw and redeem functions should allow values up to the IERC4626::maxWithdraw and IERC4626::maxRedeem function results.

The HedgeVault does not override these functions thereby breaching the standard as a withdrawal / redemption will fail for funds that are currently in trading.

Impact:

Integrators of the HedgeVault might attempt to withdraw funds that are currently being traded and thus might consider the vault to be misbehaving as it is not compliant with the EIP-4626 standard.

Example:

Copy
203/**
204 * @notice Allows controller to take funds for a trader
205 * @param trader Address of the trader
206 * @param amount Amount to take
207 */
208function fundStrategy(address trader, uint256 amount) external override onlyRole(SAY_TRADER_ROLE) {
209    // Check if enough available funds
210    uint256 availableFunds = _totalAssets;
211    if (amount > availableFunds) revert InsufficientAvailableFunds();
212
213    fundsInTrading += amount;
214    IERC20(asset()).safeTransfer(trader, amount);
215
216    emit StrategyFunded(trader, amount);
217}

Recommendation:

We advise the relevant functions to be overridden and the fundsInTrading to be accommodated for, ensuring that up to _totalAssets - fundsInTrading can be withdrawn / redeemed at any given point in time.

Alleviation (eb8a76b31d):

The Plutus DAO team evaluated this exhibit and stated that the system's intended usage is to have its withdrawals set to a paused state whenever funds are extracted from the system through the HedgeVault::fundStrategy function.

We believe that such a trait should be enforced by the code itself and we advise this to be done so by ensuring that the system's withdrawals are paused whenever fundsInTrading is non-zero in addition to the manual configuration flag.

Alleviation (af82504254):

The code has applied our recommendation in a highly standardized way by updating the IERC4626::maxWithdraw and IERC4626::maxRedeem functions to yield 0 if the fundsInTrading are non-zero, ensuring that withdrawals / redemptions are paused and properly signaled as such to external observers if there are any funds in trading.