feat: add master vault #126
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| function _deposit( | ||
| address caller, | ||
| address receiver, | ||
| uint256 assets, | ||
| uint256 shares | ||
| ) internal virtual override { | ||
| super._deposit(caller, receiver, assets, shares); | ||
| totalPrincipal += assets; | ||
| ERC4626 _subVault = subVault; | ||
| if (address(_subVault) != address(0)) { | ||
| _subVault.deposit(assets, address(this)); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
| function _withdraw( | ||
| address caller, | ||
| address receiver, | ||
| address _owner, | ||
| uint256 assets, | ||
| uint256 shares | ||
| ) internal virtual override { | ||
| ERC4626 _subVault = subVault; | ||
| if (address(_subVault) != address(0)) { | ||
| _subVault.withdraw(assets, address(this), address(this)); | ||
| } | ||
|
|
||
| ////// PERF FEE STUFF ////// | ||
| // determine profit portion and principal portion of assets | ||
| uint256 _totalProfit = totalProfit(); | ||
| // use shares because they are rounded up vs assets which are rounded down | ||
| uint256 profitPortion = shares.mulDiv(_totalProfit, totalSupply(), Math.Rounding.Up); | ||
| uint256 principalPortion = assets - profitPortion; | ||
|
|
||
| // subtract principal portion from totalPrincipal | ||
| totalPrincipal -= principalPortion; | ||
|
|
||
| // send fee to owner (todo should be a separate beneficiary addr set by owner) | ||
| if (performanceFeeBps > 0 && profitPortion > 0) { | ||
| uint256 fee = profitPortion.mulDiv(performanceFeeBps, 10000, Math.Rounding.Up); | ||
| // send fee to owner | ||
| IERC20(asset()).safeTransfer(owner(), fee); | ||
|
|
||
| // note subtraction | ||
| assets -= fee; | ||
| } | ||
|
|
||
| ////// END PERF FEE STUFF ////// | ||
|
|
||
| // call super._withdraw with remaining assets | ||
| super._withdraw(caller, receiver, _owner, assets, shares); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
| function withdrawPerformanceFees() external onlyOwner { | ||
| if (!enablePerformanceFee) revert PerformanceFeeDisabled(); | ||
| if (beneficiary == address(0)) revert BeneficiaryNotSet(); | ||
|
|
||
| uint256 totalProfits = totalProfit(); | ||
| if (totalProfits > 0) { | ||
| ERC4626 _subVault = subVault; | ||
| if (address(_subVault) != address(0)) { | ||
| _subVault.withdraw(totalProfits, address(this), address(this)); | ||
| } | ||
| IERC20(asset()).safeTransfer(beneficiary, totalProfits); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
waelsy123
force-pushed
the
wa/master-vault-isolated
branch
from
590a713 to
fc89964
Compare
| if (address(oldSubVault) == address(0)) revert NoExistingSubVault(); | ||
|
|
||
| uint256 _totalSupply = totalSupply(); | ||
| uint256 assetReceived = oldSubVault.withdraw(oldSubVault.maxWithdraw(address(this)), address(this), address(this)); |
There was a problem hiding this comment.
there is an edge case here - the subvault may not have enough liquidity to serve this big withdrawal all at once.
we probably need to make switching vaults more robust to those liquidity constaints.
the same could be said about depositing to the new vault, it could be such a large deposit that slippage starts to become a serious issue
There was a problem hiding this comment.
we could do check whether maxWithdraw will return same amount of what master vault actually own
Make master vault upgradable
| function _setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) internal { | ||
| if (address(_subVault) == address(0)) revert SubVaultCannotBeZeroAddress(); | ||
| if (totalSupply() == 0) revert MustHaveSupplyBeforeSettingSubVault(); | ||
| if (address(_subVault.asset()) != address(asset())) revert SubVaultAssetMismatch(); | ||
|
|
||
| IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); | ||
| uint256 subShares = _subVault.deposit(totalAssets(), address(this)); | ||
|
|
||
| subVault = _subVault; | ||
|
|
||
| uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalAssets(), MathUpgradeable.Rounding.Down); | ||
| if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); | ||
| subVaultExchRateWad = _subVaultExchRateWad; | ||
|
|
||
| emit SubvaultChanged(address(0), address(_subVault)); | ||
| } |
Check warning
Code scanning / Slither
Dangerous strict equalities Medium
| function _setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) internal { | ||
| if (address(_subVault) == address(0)) revert SubVaultCannotBeZeroAddress(); | ||
| if (totalSupply() == 0) revert MustHaveSupplyBeforeSettingSubVault(); | ||
| if (address(_subVault.asset()) != address(asset())) revert SubVaultAssetMismatch(); | ||
|
|
||
| IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); | ||
| uint256 subShares = _subVault.deposit(totalAssets(), address(this)); | ||
|
|
||
| subVault = _subVault; | ||
|
|
||
| uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalAssets(), MathUpgradeable.Rounding.Down); | ||
| if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); | ||
| subVaultExchRateWad = _subVaultExchRateWad; | ||
|
|
||
| emit SubvaultChanged(address(0), address(_subVault)); | ||
| } |
Check warning
Code scanning / Slither
Reentrancy vulnerabilities Medium
| function _revokeSubVault(uint256 minAssetExchRateWad) internal { | ||
| IERC4626 oldSubVault = subVault; | ||
| if (address(oldSubVault) == address(0)) revert NoExistingSubVault(); | ||
|
|
||
| uint256 _totalSupply = totalSupply(); | ||
| uint256 assetReceived = oldSubVault.withdraw(oldSubVault.maxWithdraw(address(this)), address(this), address(this)); | ||
| uint256 effectiveAssetExchRateWad = assetReceived.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); | ||
| if (effectiveAssetExchRateWad < minAssetExchRateWad) revert TooFewAssetsReceived(); | ||
|
|
||
| IERC20(asset()).safeApprove(address(oldSubVault), 0); | ||
| subVault = IERC4626(address(0)); | ||
| subVaultExchRateWad = 1e18; | ||
|
|
||
| emit SubvaultChanged(address(oldSubVault), address(0)); | ||
| } |
Check warning
Code scanning / Slither
Reentrancy vulnerabilities Medium
Sherlock AI FindingsThe automated tool identified the following potential security issues in the codebase. Please review the details for each issue in the linked dashboard.
Next Steps: Review the linked issues in the dashboard and address high-severity bugs first. Contact the team if you need assistance. Full report available at: https://ai.sherlock.xyz/runs/989379e2-ffde-4590-953e-34652307e2a0 |
This isolate the implementation for the master vault and its related factory and subvault.