From 951c33c22ae82797b519f5e4d2b7ddff75e8d402 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Mon, 29 Sep 2025 14:43:35 +0200 Subject: [PATCH 01/16] feat: add master vault --- .../libraries/vault/IMasterVault.sol | 6 + .../libraries/vault/IMasterVaultFactory.sol | 13 + .../libraries/vault/MasterVault.sol | 255 ++++++++++++++++++ .../libraries/vault/MasterVaultFactory.sol | 69 +++++ contracts/tokenbridge/test/MockSubVault.sol | 18 ++ .../libraries/vault/MasterVault.t.sol | 187 +++++++++++++ .../libraries/vault/MasterVaultFactory.t.sol | 72 +++++ 7 files changed, 620 insertions(+) create mode 100644 contracts/tokenbridge/libraries/vault/IMasterVault.sol create mode 100644 contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol create mode 100644 contracts/tokenbridge/libraries/vault/MasterVault.sol create mode 100644 contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol create mode 100644 contracts/tokenbridge/test/MockSubVault.sol create mode 100644 test-foundry/libraries/vault/MasterVault.t.sol create mode 100644 test-foundry/libraries/vault/MasterVaultFactory.t.sol diff --git a/contracts/tokenbridge/libraries/vault/IMasterVault.sol b/contracts/tokenbridge/libraries/vault/IMasterVault.sol new file mode 100644 index 000000000..6ea8c6255 --- /dev/null +++ b/contracts/tokenbridge/libraries/vault/IMasterVault.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +interface IMasterVault { + function setSubVault(address subVault) external; +} \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol new file mode 100644 index 000000000..513a80007 --- /dev/null +++ b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +interface IMasterVaultFactory { + event VaultDeployed(address indexed token, address indexed vault); + event SubVaultSet(address indexed masterVault, address indexed subVault); + + function initialize(address _owner) external; + function deployVault(address token) external returns (address vault); + function calculateVaultAddress(address token) external view returns (address); + function getVault(address token) external returns (address); + function setSubVault(address masterVault, address subVault) external; +} \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol new file mode 100644 index 000000000..ea3614677 --- /dev/null +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -0,0 +1,255 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; +import { IERC20, ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; +import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +contract MasterVault is ERC4626, Ownable { + using SafeERC20 for IERC20; + using Math for uint256; + + error TooFewSharesReceived(); + error TooManySharesBurned(); + error TooManyAssetsDeposited(); + error TooFewAssetsReceived(); + error SubVaultAlreadySet(); + error SubVaultCannotBeZeroAddress(); + error MustHaveSupplyBeforeSettingSubVault(); + error SubVaultAssetMismatch(); + error SubVaultExchangeRateTooLow(); + error NoExistingSubVault(); + error MustHaveSupplyBeforeSwitchingSubVault(); + error NewSubVaultExchangeRateTooLow(); + + // todo: avoid inflation, rounding, other common 4626 vulns + // we may need a minimum asset or master share amount when setting subvaults (bc of exchange rate calc) + ERC4626 public subVault; + + // how many subVault shares one MV2 share can be redeemed for + // initially 1 to 1 + // constant per subvault + // changes when subvault is set + uint256 public subVaultExchRateWad = 1e18; + + // note: the performance fee can be avoided if the underlying strategy can be sandwiched (eg ETH to wstETH dex swap) + // maybe a simpler and more robust implementation would be for the owner to adjust the subVaultExchRateWad directly + // this would also avoid the need for totalPrincipal tracking + // however, this would require more trust in the owner + uint256 public performanceFeeBps; // in basis points, e.g. 200 = 2% | todo a way to set this + uint256 totalPrincipal; // total assets deposited, used to calculate profit + + event SubvaultChanged(address indexed oldSubvault, address indexed newSubvault); + + constructor(IERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol) ERC4626(_asset) Ownable() {} + + function deposit(uint256 assets, address receiver, uint256 minSharesMinted) public returns (uint256) { + uint256 shares = super.deposit(assets, receiver); + if (shares < minSharesMinted) revert TooFewSharesReceived(); + return shares; + } + + function withdraw(uint256 assets, address receiver, address _owner, uint256 maxSharesBurned) public returns (uint256) { + uint256 shares = super.withdraw(assets, receiver, _owner); + if (shares > maxSharesBurned) revert TooManySharesBurned(); + return shares; + } + + function mint(uint256 shares, address receiver, uint256 maxAssetsDeposited) public returns (uint256) { + uint256 assets = super.mint(shares, receiver); + if (assets > maxAssetsDeposited) revert TooManyAssetsDeposited(); + return assets; + } + + function redeem(uint256 shares, address receiver, address _owner, uint256 minAssetsReceived) public returns (uint256) { + uint256 assets = super.redeem(shares, receiver, _owner); + if (assets < minAssetsReceived) revert TooFewAssetsReceived(); + return assets; + } + + /// @notice Set a subvault. Can only be called if there is not already a subvault set. + /// @param _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault. + /// @param minSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit. + function setSubVault(ERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyOwner { + if (address(subVault) != address(0)) revert SubVaultAlreadySet(); + _setSubVault(_subVault, minSubVaultExchRateWad); + } + + /// @notice Revokes the current subvault, moving all assets back to MasterVault + /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from subvault to outstanding MasterVault shares + function revokeSubVault(uint256 minAssetExchRateWad) external onlyOwner { + _revokeSubVault(minAssetExchRateWad); + } + + function _setSubVault(ERC4626 _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)); + + uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalSupply(), Math.Rounding.Down); + if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); + subVaultExchRateWad = _subVaultExchRateWad; + + subVault = _subVault; + + emit SubvaultChanged(address(0), address(_subVault)); + } + + function _revokeSubVault(uint256 minAssetExchRateWad) internal { + ERC4626 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, Math.Rounding.Down); + if (effectiveAssetExchRateWad < minAssetExchRateWad) revert TooFewAssetsReceived(); + + IERC20(asset()).safeApprove(address(oldSubVault), 0); + subVault = ERC4626(address(0)); + subVaultExchRateWad = 1e18; + + emit SubvaultChanged(address(oldSubVault), address(0)); + } + + /// @notice Switches to a new subvault or revokes current subvault if newSubVault is zero address + /// @param newSubVault The new subvault to switch to, or zero address to revoke current subvault + /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from old subvault to outstanding MasterVault shares + /// @param minNewSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit + function switchSubVault(ERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyOwner { + _revokeSubVault(minAssetExchRateWad); + + if (address(newSubVault) != address(0)) { + _setSubVault(newSubVault, minNewSubVaultExchRateWad); + } + } + + function masterSharesToSubShares(uint256 masterShares, Math.Rounding rounding) public view returns (uint256) { + return masterShares.mulDiv(subVaultExchRateWad, 1e18, rounding); + } + + function subSharesToMasterShares(uint256 subShares, Math.Rounding rounding) public view returns (uint256) { + return subShares.mulDiv(1e18, subVaultExchRateWad, rounding); + } + + /** @dev See {IERC4626-totalAssets}. */ + function totalAssets() public view virtual override returns (uint256) { + ERC4626 _subVault = subVault; + if (address(_subVault) == address(0)) { + return super.totalAssets(); + } + return _subVault.convertToAssets(_subVault.balanceOf(address(this))); + } + + /** @dev See {IERC4626-maxDeposit}. */ + function maxDeposit(address) public view virtual override returns (uint256) { + if (address(subVault) == address(0)) { + return type(uint256).max; + } + return subVault.maxDeposit(address(this)); + } + + /** @dev See {IERC4626-maxMint}. */ + function maxMint(address) public view virtual override returns (uint256) { + uint256 subShares = subVault.maxMint(address(this)); + if (subShares == type(uint256).max) { + return type(uint256).max; + } + return subSharesToMasterShares(subShares, Math.Rounding.Down); + } + + /** + * @dev Internal conversion function (from assets to shares) with support for rounding direction. + * + * Will revert if assets > 0, totalSupply > 0 and totalAssets = 0. That corresponds to a case where any asset + * would represent an infinite amount of shares. + */ + function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual override returns (uint256 shares) { + ERC4626 _subVault = subVault; + if (address(_subVault) == address(0)) { + return super._convertToShares(assets, rounding); + } + uint256 subShares = rounding == Math.Rounding.Up ? _subVault.previewWithdraw(assets) : _subVault.previewDeposit(assets); + return subSharesToMasterShares(subShares, rounding); + } + + /** + * @dev Internal conversion function (from shares to assets) with support for rounding direction. + */ + function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual override returns (uint256 assets) { + ERC4626 _subVault = subVault; + if (address(_subVault) == address(0)) { + return super._convertToAssets(shares, rounding); + } + uint256 subShares = masterSharesToSubShares(shares, rounding); + return rounding == Math.Rounding.Up ? _subVault.previewMint(subShares) : _subVault.previewRedeem(subShares); + } + + function totalProfit() public view returns (uint256) { + uint256 _totalAssets = totalAssets(); + return _totalAssets > totalPrincipal ? _totalAssets - totalPrincipal : 0; + } + + /** + * @dev Deposit/mint common workflow. + */ + 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)); + } + } + + /** + * @dev Withdraw/redeem common workflow. + */ + 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); + } +} \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol new file mode 100644 index 000000000..4ee008b05 --- /dev/null +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 + +pragma solidity ^0.8.0; + +import "@openzeppelin/contracts/utils/Create2.sol"; +import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import "./IMasterVault.sol"; +import "./IMasterVaultFactory.sol"; +import "./MasterVault.sol"; + +contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { + + error ZeroAddress(); + + function initialize(address _owner) public initializer { + _transferOwnership(_owner); + } + + function deployVault(address token) public returns (address vault) { + if (token == address(0)) { + revert ZeroAddress(); + } + + IERC20Metadata tokenMetadata = IERC20Metadata(token); + string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); + string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); + + bytes memory bytecode = abi.encodePacked( + type(MasterVault).creationCode, + abi.encode(token, name, symbol) + ); + + vault = Create2.deploy(0, bytes32(0), bytecode); + + emit VaultDeployed(token, vault); + } + + function calculateVaultAddress(address token) public view returns (address) { + IERC20Metadata tokenMetadata = IERC20Metadata(token); + string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); + string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); + + bytes32 bytecodeHash = keccak256( + abi.encodePacked( + type(MasterVault).creationCode, + abi.encode(token, name, symbol) + ) + ); + return Create2.computeAddress(bytes32(0), bytecodeHash); + } + + function getVault(address token) external returns (address) { + address vault = calculateVaultAddress(token); + if (vault.code.length == 0) { + return deployVault(token); + } + return vault; + } + + // todo: consider a method to enable bridge owner to transfer specific master vault ownership to new address + function setSubVault( + address masterVault, + address subVault + ) external onlyOwner { + IMasterVault(masterVault).setSubVault(subVault); + emit SubVaultSet(masterVault, subVault); + } +} \ No newline at end of file diff --git a/contracts/tokenbridge/test/MockSubVault.sol b/contracts/tokenbridge/test/MockSubVault.sol new file mode 100644 index 000000000..411edb61b --- /dev/null +++ b/contracts/tokenbridge/test/MockSubVault.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +contract MockSubVault is ERC4626 { + constructor( + IERC20 _asset, + string memory _name, + string memory _symbol + ) ERC20(_name, _symbol) ERC4626(_asset) {} + + function totalAssets() public view override returns (uint256) { + return IERC20(asset()).balanceOf(address(this)); + } +} \ No newline at end of file diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol new file mode 100644 index 000000000..037cdbf3b --- /dev/null +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +import { Test } from "forge-std/Test.sol"; +import { MasterVault } from "../../../contracts/tokenbridge/libraries/vault/MasterVault.sol"; +import { TestERC20 } from "../../../contracts/tokenbridge/test/TestERC20.sol"; +import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +contract MasterVaultTest is Test { + MasterVault public vault; + TestERC20 public token; + + event SubvaultChanged(address indexed oldSubvault, address indexed newSubvault); + + address public user = address(0x1); + string public name = "Master Test Token"; + string public symbol = "mTST"; + + function setUp() public { + token = new TestERC20(); + vault = new MasterVault(IERC20(address(token)), name, symbol); + } + + function test_initialize() public { + assertEq(address(vault.asset()), address(token), "Invalid asset"); + assertEq(vault.name(), name, "Invalid name"); + assertEq(vault.symbol(), symbol, "Invalid symbol"); + assertEq(vault.decimals(), token.decimals(), "Invalid decimals"); + assertEq(vault.totalSupply(), 0, "Invalid initial supply"); + assertEq(vault.totalAssets(), 0, "Invalid initial assets"); + assertEq(address(vault.subVault()), address(0), "SubVault should be zero initially"); + } + + function test_WithoutSubvault_deposit() public { + assertEq(address(vault.subVault()), address(0), "SubVault should be zero initially"); + + // user deposit 500 tokens to vault + // by this test expec: + //- user to receive 500 shares + //- total shares supply to increase by 500 + //- total assets to increase by 500 + + uint256 minShares = 0; + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + + token.approve(address(vault), depositAmount); + + uint256 sharesBefore = vault.balanceOf(user); + uint256 totalSupplyBefore = vault.totalSupply(); + uint256 totalAssetsBefore = vault.totalAssets(); + + uint256 shares = vault.deposit(depositAmount, user, minShares); + + assertEq(vault.balanceOf(user), sharesBefore + shares, "Invalid user balance"); + assertEq(vault.totalSupply(), totalSupplyBefore + shares, "Invalid total supply"); + assertEq(vault.totalAssets(), totalAssetsBefore + depositAmount, "Invalid total assets"); + assertEq(token.balanceOf(user), 0, "User tokens should be transferred"); + + vm.stopPrank(); + } + + function test_deposit_RevertTooFewSharesReceived() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + uint256 minShares = depositAmount * 2; // Unrealistic requirement + + token.approve(address(vault), depositAmount); + + vm.expectRevert(MasterVault.TooFewSharesReceived.selector); + vault.deposit(depositAmount, user, minShares); + + vm.stopPrank(); + } + + function test_setSubvault() public { + MockSubVault subVault = new MockSubVault( + IERC20(address(token)), + "Sub Vault Token", + "svTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + assertEq(address(vault.subVault()), address(0), "SubVault should be zero initially"); + assertEq(vault.totalAssets(), depositAmount, "Total assets should equal deposit"); + + uint256 minSubVaultExchRateWad = 1e18; + + vm.expectEmit(true, true, false, false); + emit SubvaultChanged(address(0), address(subVault)); + + vault.setSubVault(subVault, minSubVaultExchRateWad); + + assertEq(address(vault.subVault()), address(subVault), "SubVault should be set"); + assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should be 1:1 initially"); + assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); + assertEq(subVault.balanceOf(address(vault)), depositAmount, "SubVault should have received assets"); + } + + function test_switchSubvault() public { + MockSubVault oldSubVault = new MockSubVault( + IERC20(address(token)), + "Old Sub Vault", + "osvTST" + ); + + MockSubVault newSubVault = new MockSubVault( + IERC20(address(token)), + "New Sub Vault", + "nsvTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + vault.setSubVault(oldSubVault, 1e18); + + assertEq(address(vault.subVault()), address(oldSubVault), "Old subvault should be set"); + assertEq(oldSubVault.balanceOf(address(vault)), depositAmount, "Old subvault should have assets"); + assertEq(newSubVault.balanceOf(address(vault)), 0, "New subvault should have no assets initially"); + + uint256 minAssetExchRateWad = 1e18; + uint256 minNewSubVaultExchRateWad = 1e18; + + vm.expectEmit(true, true, false, false); + emit SubvaultChanged(address(oldSubVault), address(0)); + vm.expectEmit(true, true, false, false); + emit SubvaultChanged(address(0), address(newSubVault)); + + vault.switchSubVault(newSubVault, minAssetExchRateWad, minNewSubVaultExchRateWad); + + assertEq(address(vault.subVault()), address(newSubVault), "New subvault should be set"); + assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should remain 1:1"); + assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); + assertEq(oldSubVault.balanceOf(address(vault)), 0, "Old subvault should have no assets"); + assertEq(newSubVault.balanceOf(address(vault)), depositAmount, "New subvault should have received assets"); + } + + function test_revokeSubvault() public { + MockSubVault subVault = new MockSubVault( + IERC20(address(token)), + "Sub Vault Token", + "svTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + vault.setSubVault(subVault, 1e18); + + assertEq(address(vault.subVault()), address(subVault), "SubVault should be set"); + assertEq(subVault.balanceOf(address(vault)), depositAmount, "SubVault should have assets"); + assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should be 1:1"); + + uint256 minAssetExchRateWad = 1e18; + + vm.expectEmit(true, true, false, false); + emit SubvaultChanged(address(subVault), address(0)); + + vault.revokeSubVault(minAssetExchRateWad); + + assertEq(address(vault.subVault()), address(0), "SubVault should be revoked"); + assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should reset to 1:1"); + assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); + assertEq(subVault.balanceOf(address(vault)), 0, "SubVault should have no assets"); + assertEq(token.balanceOf(address(vault)), depositAmount, "MasterVault should have assets directly"); + } + +} diff --git a/test-foundry/libraries/vault/MasterVaultFactory.t.sol b/test-foundry/libraries/vault/MasterVaultFactory.t.sol new file mode 100644 index 000000000..69d31106a --- /dev/null +++ b/test-foundry/libraries/vault/MasterVaultFactory.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; +import {MasterVaultFactory} from "../../../contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol"; +import {MasterVault} from "../../../contracts/tokenbridge/libraries/vault/MasterVault.sol"; +import {TestERC20} from "../../../contracts/tokenbridge/test/TestERC20.sol"; + +contract MasterVaultFactoryTest is Test { + MasterVaultFactory public factory; + TestERC20 public token; + + address public owner = address(0x1); + address public user = address(0x2); + + event VaultDeployed(address indexed token, address indexed vault); + + function setUp() public { + token = new TestERC20(); + factory = new MasterVaultFactory(); + + vm.prank(owner); + factory.initialize(owner); + } + + function test_initialize() public { + assertEq(factory.owner(), owner, "Invalid owner"); + } + + function test_deployVault() public { + address expectedVault = factory.calculateVaultAddress(address(token)); + + vm.expectEmit(true, true, false, false); + emit VaultDeployed(address(token), expectedVault); + + address deployedVault = factory.deployVault(address(token)); + + assertEq(deployedVault, expectedVault, "Vault address mismatch"); + assertTrue(deployedVault.code.length > 0, "Vault not deployed"); + + MasterVault vault = MasterVault(deployedVault); + assertEq(address(vault.asset()), address(token), "Invalid vault asset"); + assertEq(vault.owner(), address(factory), "Invalid vault owner"); + } + + function test_deployVault_RevertZeroAddress() public { + vm.expectRevert(MasterVaultFactory.ZeroAddress.selector); + factory.deployVault(address(0)); + } + + function test_getVault_DeploysIfNotExists() public { + address expectedVault = factory.calculateVaultAddress(address(token)); + address vault = factory.getVault(address(token)); + + assertEq(vault, expectedVault, "Vault address mismatch"); + assertTrue(vault.code.length > 0, "Vault not deployed"); + } + + function test_getVault_ReturnsExistingVault() public { + address vault1 = factory.getVault(address(token)); + address vault2 = factory.getVault(address(token)); + + assertEq(vault1, vault2, "Should return same vault"); + } + + function test_calculateVaultAddress() public { + address calculatedAddress = factory.calculateVaultAddress(address(token)); + address deployedVault = factory.deployVault(address(token)); + + assertEq(calculatedAddress, deployedVault, "Address calculation incorrect"); + } +} \ No newline at end of file From fc89964a509b2f5e48be15dd8687360010d97d29 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Mon, 29 Sep 2025 19:47:30 +0200 Subject: [PATCH 02/16] permission withdraw performence fees --- .../libraries/vault/MasterVault.sol | 63 +++++++++------ .../libraries/vault/MasterVault.t.sol | 78 +++++++++++++++++++ 2 files changed, 117 insertions(+), 24 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index ea3614677..1dbeaa063 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -24,6 +24,8 @@ contract MasterVault is ERC4626, Ownable { error NoExistingSubVault(); error MustHaveSupplyBeforeSwitchingSubVault(); error NewSubVaultExchangeRateTooLow(); + error BeneficiaryNotSet(); + error PerformanceFeeDisabled(); // todo: avoid inflation, rounding, other common 4626 vulns // we may need a minimum asset or master share amount when setting subvaults (bc of exchange rate calc) @@ -39,10 +41,13 @@ contract MasterVault is ERC4626, Ownable { // maybe a simpler and more robust implementation would be for the owner to adjust the subVaultExchRateWad directly // this would also avoid the need for totalPrincipal tracking // however, this would require more trust in the owner - uint256 public performanceFeeBps; // in basis points, e.g. 200 = 2% | todo a way to set this + bool public enablePerformanceFee; + address public beneficiary; uint256 totalPrincipal; // total assets deposited, used to calculate profit event SubvaultChanged(address indexed oldSubvault, address indexed newSubvault); + event PerformanceFeeToggled(bool enabled); + event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary); constructor(IERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol) ERC4626(_asset) Ownable() {} @@ -137,6 +142,37 @@ contract MasterVault is ERC4626, Ownable { return subShares.mulDiv(1e18, subVaultExchRateWad, rounding); } + /// @notice Toggle performance fee collection on/off + /// @param enabled True to enable performance fees, false to disable + function setPerformanceFee(bool enabled) external onlyOwner { + enablePerformanceFee = enabled; + emit PerformanceFeeToggled(enabled); + } + + /// @notice Set the beneficiary address for performance fees + /// @param newBeneficiary Address to receive performance fees, zero address defaults to owner + function setBeneficiary(address newBeneficiary) external onlyOwner { + address oldBeneficiary = beneficiary; + beneficiary = newBeneficiary; + emit BeneficiaryUpdated(oldBeneficiary, newBeneficiary); + } + + /// @notice Withdraw all accumulated performance fees to beneficiary + /// @dev Only callable by owner when performance fees are enabled + 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); + } + } + /** @dev See {IERC4626-totalAssets}. */ function totalAssets() public view virtual override returns (uint256) { ERC4626 _subVault = subVault; @@ -222,34 +258,13 @@ contract MasterVault is ERC4626, Ownable { uint256 assets, uint256 shares ) internal virtual override { + totalPrincipal -= assets; + 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); } } \ No newline at end of file diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index 037cdbf3b..b5070e92b 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -184,4 +184,82 @@ contract MasterVaultTest is Test { assertEq(token.balanceOf(address(vault)), depositAmount, "MasterVault should have assets directly"); } + function test_WithoutSubvault_withdraw() public { + uint256 maxSharesBurned = type(uint256).max; + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + + uint256 withdrawAmount = depositAmount / 2; + uint256 userSharesBefore = vault.balanceOf(user); + uint256 totalSupplyBefore = vault.totalSupply(); + uint256 totalAssetsBefore = vault.totalAssets(); + + uint256 shares = vault.withdraw(withdrawAmount, user, user, maxSharesBurned); + + assertEq(vault.balanceOf(user), userSharesBefore - shares, "User shares should decrease"); + assertEq(vault.totalSupply(), totalSupplyBefore - shares, "Total supply should decrease"); + assertEq(vault.totalAssets(), totalAssetsBefore - withdrawAmount, "Total assets should decrease"); + assertEq(token.balanceOf(user), withdrawAmount, "User should receive withdrawn assets"); + assertEq(token.balanceOf(address(vault)), depositAmount - withdrawAmount, "Vault should have remaining assets"); + + vm.stopPrank(); + } + + function test_WithSubvault_withdraw() public { + MockSubVault subVault = new MockSubVault( + IERC20(address(token)), + "Sub Vault Token", + "svTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 firstDepositAmount = token.balanceOf(user); + token.approve(address(vault), firstDepositAmount); + vault.deposit(firstDepositAmount, user, 0); + vm.stopPrank(); + + vault.setSubVault(subVault, 1e18); + + uint256 withdrawAmount = firstDepositAmount / 2; + uint256 maxSharesBurned = type(uint256).max; + + vm.startPrank(user); + uint256 userSharesBefore = vault.balanceOf(user); + uint256 totalSupplyBefore = vault.totalSupply(); + uint256 totalAssetsBefore = vault.totalAssets(); + uint256 subVaultSharesBefore = subVault.balanceOf(address(vault)); + + uint256 shares = vault.withdraw(withdrawAmount, user, user, maxSharesBurned); + + assertEq(vault.balanceOf(user), userSharesBefore - shares, "User shares should decrease"); + assertEq(vault.totalSupply(), totalSupplyBefore - shares, "Total supply should decrease"); + assertEq(vault.totalAssets(), totalAssetsBefore - withdrawAmount, "Total assets should decrease"); + assertEq(token.balanceOf(user), withdrawAmount, "User should receive withdrawn assets"); + assertLt(subVault.balanceOf(address(vault)), subVaultSharesBefore, "SubVault shares should decrease"); + + token.mint(); + uint256 secondDepositAmount = token.balanceOf(user) - withdrawAmount; + token.approve(address(vault), secondDepositAmount); + vault.deposit(secondDepositAmount, user, 0); + + vault.balanceOf(user); + uint256 finalTotalAssets = vault.totalAssets(); + subVault.balanceOf(address(vault)); + + vault.withdraw(finalTotalAssets, user, user, type(uint256).max); + + assertEq(vault.balanceOf(user), 0, "User should have no shares left"); + assertEq(vault.totalSupply(), 0, "Total supply should be zero"); + assertEq(vault.totalAssets(), 0, "Total assets should be zero"); + assertEq(token.balanceOf(user), firstDepositAmount + secondDepositAmount, "User should have all original tokens"); + assertEq(subVault.balanceOf(address(vault)), 0, "SubVault should have no shares left"); + + vm.stopPrank(); + } + } From 7922536aa59c93deb736601e8ad9a49c2999d6a9 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Wed, 1 Oct 2025 15:42:06 +0200 Subject: [PATCH 03/16] feat: make master vault beacon upgradable --- .../libraries/vault/IMasterVault.sol | 2 +- .../libraries/vault/IMasterVaultFactory.sol | 2 +- .../libraries/vault/MasterVault.sol | 78 +++++++++++-------- .../libraries/vault/MasterVaultFactory.sol | 50 +++++++----- .../libraries/vault/MasterVault.t.sol | 17 +++- 5 files changed, 94 insertions(+), 55 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/IMasterVault.sol b/contracts/tokenbridge/libraries/vault/IMasterVault.sol index 6ea8c6255..91cfb6c50 100644 --- a/contracts/tokenbridge/libraries/vault/IMasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/IMasterVault.sol @@ -2,5 +2,5 @@ pragma solidity ^0.8.0; interface IMasterVault { - function setSubVault(address subVault) external; + function setSubVault(address subVault, uint256 minSubVaultExchRateWad) external; } \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol index 513a80007..7ac654fca 100644 --- a/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol @@ -9,5 +9,5 @@ interface IMasterVaultFactory { function deployVault(address token) external returns (address vault); function calculateVaultAddress(address token) external view returns (address); function getVault(address token) external returns (address); - function setSubVault(address masterVault, address subVault) external; + function setSubVault(address masterVault, address subVault, uint256 minSubVaultExchRateWad) external; } \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 1dbeaa063..b55978d21 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -1,16 +1,20 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; -import { IERC20, ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; +import {ERC4626Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol"; +import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; +import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; +import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; -import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {MathUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/math/MathUpgradeable.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -contract MasterVault is ERC4626, Ownable { +contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { using SafeERC20 for IERC20; - using Math for uint256; + using MathUpgradeable for uint256; error TooFewSharesReceived(); error TooManySharesBurned(); @@ -29,13 +33,13 @@ contract MasterVault is ERC4626, Ownable { // todo: avoid inflation, rounding, other common 4626 vulns // we may need a minimum asset or master share amount when setting subvaults (bc of exchange rate calc) - ERC4626 public subVault; + IERC4626 public subVault; // how many subVault shares one MV2 share can be redeemed for // initially 1 to 1 // constant per subvault // changes when subvault is set - uint256 public subVaultExchRateWad = 1e18; + uint256 public subVaultExchRateWad; // note: the performance fee can be avoided if the underlying strategy can be sandwiched (eg ETH to wstETH dex swap) // maybe a simpler and more robust implementation would be for the owner to adjust the subVaultExchRateWad directly @@ -49,16 +53,27 @@ contract MasterVault is ERC4626, Ownable { event PerformanceFeeToggled(bool enabled); event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary); - constructor(IERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol) ERC4626(_asset) Ownable() {} + function vaultInit(IERC20 _asset, string memory _name, string memory _symbol, address _owner) external initializer { + require(address(_asset) != address(0), "INVALID_ASSET"); + require(_owner != address(0), "INVALID_OWNER"); + + __ERC20_init(_name, _symbol); + __ERC4626_init(IERC20Upgradeable(address(_asset))); + __Ownable_init(); + _transferOwnership(_owner); + + subVaultExchRateWad = 1e18; + } + function deposit(uint256 assets, address receiver, uint256 minSharesMinted) public returns (uint256) { - uint256 shares = super.deposit(assets, receiver); + uint256 shares = deposit(assets, receiver); if (shares < minSharesMinted) revert TooFewSharesReceived(); return shares; } function withdraw(uint256 assets, address receiver, address _owner, uint256 maxSharesBurned) public returns (uint256) { - uint256 shares = super.withdraw(assets, receiver, _owner); + uint256 shares = withdraw(assets, receiver, _owner); if (shares > maxSharesBurned) revert TooManySharesBurned(); return shares; } @@ -78,7 +93,7 @@ contract MasterVault is ERC4626, Ownable { /// @notice Set a subvault. Can only be called if there is not already a subvault set. /// @param _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault. /// @param minSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit. - function setSubVault(ERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyOwner { + function setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyOwner { if (address(subVault) != address(0)) revert SubVaultAlreadySet(); _setSubVault(_subVault, minSubVaultExchRateWad); } @@ -89,7 +104,7 @@ contract MasterVault is ERC4626, Ownable { _revokeSubVault(minAssetExchRateWad); } - function _setSubVault(ERC4626 _subVault, uint256 minSubVaultExchRateWad) internal { + 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(); @@ -97,7 +112,7 @@ contract MasterVault is ERC4626, Ownable { IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); uint256 subShares = _subVault.deposit(totalAssets(), address(this)); - uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalSupply(), Math.Rounding.Down); + uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalSupply(), MathUpgradeable.Rounding.Down); if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); subVaultExchRateWad = _subVaultExchRateWad; @@ -107,16 +122,16 @@ contract MasterVault is ERC4626, Ownable { } function _revokeSubVault(uint256 minAssetExchRateWad) internal { - ERC4626 oldSubVault = subVault; + 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, Math.Rounding.Down); + uint256 effectiveAssetExchRateWad = assetReceived.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); if (effectiveAssetExchRateWad < minAssetExchRateWad) revert TooFewAssetsReceived(); IERC20(asset()).safeApprove(address(oldSubVault), 0); - subVault = ERC4626(address(0)); + subVault = IERC4626(address(0)); subVaultExchRateWad = 1e18; emit SubvaultChanged(address(oldSubVault), address(0)); @@ -126,7 +141,7 @@ contract MasterVault is ERC4626, Ownable { /// @param newSubVault The new subvault to switch to, or zero address to revoke current subvault /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from old subvault to outstanding MasterVault shares /// @param minNewSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit - function switchSubVault(ERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyOwner { + function switchSubVault(IERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyOwner { _revokeSubVault(minAssetExchRateWad); if (address(newSubVault) != address(0)) { @@ -134,11 +149,11 @@ contract MasterVault is ERC4626, Ownable { } } - function masterSharesToSubShares(uint256 masterShares, Math.Rounding rounding) public view returns (uint256) { + function masterSharesToSubShares(uint256 masterShares, MathUpgradeable.Rounding rounding) public view returns (uint256) { return masterShares.mulDiv(subVaultExchRateWad, 1e18, rounding); } - function subSharesToMasterShares(uint256 subShares, Math.Rounding rounding) public view returns (uint256) { + function subSharesToMasterShares(uint256 subShares, MathUpgradeable.Rounding rounding) public view returns (uint256) { return subShares.mulDiv(1e18, subVaultExchRateWad, rounding); } @@ -165,7 +180,7 @@ contract MasterVault is ERC4626, Ownable { uint256 totalProfits = totalProfit(); if (totalProfits > 0) { - ERC4626 _subVault = subVault; + IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { _subVault.withdraw(totalProfits, address(this), address(this)); } @@ -175,7 +190,7 @@ contract MasterVault is ERC4626, Ownable { /** @dev See {IERC4626-totalAssets}. */ function totalAssets() public view virtual override returns (uint256) { - ERC4626 _subVault = subVault; + IERC4626 _subVault = subVault; if (address(_subVault) == address(0)) { return super.totalAssets(); } @@ -196,7 +211,7 @@ contract MasterVault is ERC4626, Ownable { if (subShares == type(uint256).max) { return type(uint256).max; } - return subSharesToMasterShares(subShares, Math.Rounding.Down); + return subSharesToMasterShares(subShares, MathUpgradeable.Rounding.Down); } /** @@ -205,25 +220,25 @@ contract MasterVault is ERC4626, Ownable { * Will revert if assets > 0, totalSupply > 0 and totalAssets = 0. That corresponds to a case where any asset * would represent an infinite amount of shares. */ - function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual override returns (uint256 shares) { - ERC4626 _subVault = subVault; + function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) { + IERC4626 _subVault = subVault; if (address(_subVault) == address(0)) { return super._convertToShares(assets, rounding); } - uint256 subShares = rounding == Math.Rounding.Up ? _subVault.previewWithdraw(assets) : _subVault.previewDeposit(assets); + uint256 subShares = rounding == MathUpgradeable.Rounding.Up ? _subVault.previewWithdraw(assets) : _subVault.previewDeposit(assets); return subSharesToMasterShares(subShares, rounding); } /** * @dev Internal conversion function (from shares to assets) with support for rounding direction. */ - function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual override returns (uint256 assets) { - ERC4626 _subVault = subVault; + function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) { + IERC4626 _subVault = subVault; if (address(_subVault) == address(0)) { return super._convertToAssets(shares, rounding); } uint256 subShares = masterSharesToSubShares(shares, rounding); - return rounding == Math.Rounding.Up ? _subVault.previewMint(subShares) : _subVault.previewRedeem(subShares); + return rounding == MathUpgradeable.Rounding.Up ? _subVault.previewMint(subShares) : _subVault.previewRedeem(subShares); } function totalProfit() public view returns (uint256) { @@ -241,8 +256,9 @@ contract MasterVault is ERC4626, Ownable { uint256 shares ) internal virtual override { super._deposit(caller, receiver, assets, shares); + totalPrincipal += assets; - ERC4626 _subVault = subVault; + IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { _subVault.deposit(assets, address(this)); } @@ -260,7 +276,7 @@ contract MasterVault is ERC4626, Ownable { ) internal virtual override { totalPrincipal -= assets; - ERC4626 _subVault = subVault; + IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { _subVault.withdraw(assets, address(this), address(this)); } diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol index 4ee008b05..ee39b93e7 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -5,49 +5,56 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/utils/Create2.sol"; import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "../ClonableBeaconProxy.sol"; import "./IMasterVault.sol"; import "./IMasterVaultFactory.sol"; import "./MasterVault.sol"; contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { - error ZeroAddress(); + error BeaconNotDeployed(); + + UpgradeableBeacon public beacon; + BeaconProxyFactory public beaconProxyFactory; function initialize(address _owner) public initializer { _transferOwnership(_owner); + + MasterVault masterVaultImplementation = new MasterVault(); + beacon = new UpgradeableBeacon(address(masterVaultImplementation)); + beaconProxyFactory = new BeaconProxyFactory(); + beaconProxyFactory.initialize(address(beacon)); + beacon.transferOwnership(address(this)); } function deployVault(address token) public returns (address vault) { if (token == address(0)) { revert ZeroAddress(); } + if (address(beaconProxyFactory) == address(0)) { + revert BeaconNotDeployed(); + } + + bytes32 userSalt = _getUserSalt(token); + vault = beaconProxyFactory.createProxy(userSalt); IERC20Metadata tokenMetadata = IERC20Metadata(token); string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); - bytes memory bytecode = abi.encodePacked( - type(MasterVault).creationCode, - abi.encode(token, name, symbol) - ); - - vault = Create2.deploy(0, bytes32(0), bytecode); + MasterVault(vault).vaultInit(IERC20(token), name, symbol, address(this)); emit VaultDeployed(token, vault); } - function calculateVaultAddress(address token) public view returns (address) { - IERC20Metadata tokenMetadata = IERC20Metadata(token); - string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); - string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); + function _getUserSalt(address token) internal pure returns (bytes32) { + return keccak256(abi.encode(token)); + } - bytes32 bytecodeHash = keccak256( - abi.encodePacked( - type(MasterVault).creationCode, - abi.encode(token, name, symbol) - ) - ); - return Create2.computeAddress(bytes32(0), bytecodeHash); + function calculateVaultAddress(address token) public view returns (address) { + bytes32 userSalt = _getUserSalt(token); + return beaconProxyFactory.calculateExpectedAddress(address(this), userSalt); } function getVault(address token) external returns (address) { @@ -61,9 +68,10 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { // todo: consider a method to enable bridge owner to transfer specific master vault ownership to new address function setSubVault( address masterVault, - address subVault + address subVault, + uint256 minSubVaultExchRateWad ) external onlyOwner { - IMasterVault(masterVault).setSubVault(subVault); + IMasterVault(masterVault).setSubVault(subVault, minSubVaultExchRateWad); emit SubVaultSet(masterVault, subVault); } -} \ No newline at end of file +} diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index b5070e92b..9032e744d 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -6,10 +6,14 @@ import { MasterVault } from "../../../contracts/tokenbridge/libraries/vault/Mast import { TestERC20 } from "../../../contracts/tokenbridge/test/TestERC20.sol"; import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { UpgradeableBeacon } from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import { BeaconProxyFactory, ClonableBeaconProxy } from "../../../contracts/tokenbridge/libraries/ClonableBeaconProxy.sol"; contract MasterVaultTest is Test { MasterVault public vault; TestERC20 public token; + UpgradeableBeacon public beacon; + BeaconProxyFactory public beaconProxyFactory; event SubvaultChanged(address indexed oldSubvault, address indexed newSubvault); @@ -19,7 +23,18 @@ contract MasterVaultTest is Test { function setUp() public { token = new TestERC20(); - vault = new MasterVault(IERC20(address(token)), name, symbol); + + MasterVault implementation = new MasterVault(); + beacon = new UpgradeableBeacon(address(implementation)); + + beaconProxyFactory = new BeaconProxyFactory(); + beaconProxyFactory.initialize(address(beacon)); + + bytes32 salt = keccak256("test"); + address proxyAddress = beaconProxyFactory.createProxy(salt); + vault = MasterVault(proxyAddress); + + vault.vaultInit(IERC20(address(token)), name, symbol, address(this)); } function test_initialize() public { From 6331e7a255a677cd1e2239fe0f60dfd2a4dadbeb Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Fri, 3 Oct 2025 11:43:33 +0200 Subject: [PATCH 04/16] fixup! feat: make master vault beacon upgradable --- .../libraries/vault/MasterVaultFactory.sol | 6 ++- .../libraries/vault/MasterVault.t.sol | 20 +++++++++ .../libraries/vault/MasterVaultFactory.t.sol | 42 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol index ee39b93e7..bde4d51d6 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -25,14 +25,16 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { beacon = new UpgradeableBeacon(address(masterVaultImplementation)); beaconProxyFactory = new BeaconProxyFactory(); beaconProxyFactory.initialize(address(beacon)); - beacon.transferOwnership(address(this)); + beacon.transferOwnership(_owner); } function deployVault(address token) public returns (address vault) { if (token == address(0)) { revert ZeroAddress(); } - if (address(beaconProxyFactory) == address(0)) { + if ( + address(beaconProxyFactory) == address(0) && beaconProxyFactory.beacon() == address(0) + ) { revert BeaconNotDeployed(); } diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index 9032e744d..71a7299cc 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -277,4 +277,24 @@ contract MasterVaultTest is Test { vm.stopPrank(); } + function test_beaconUpgrade() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + address oldImplementation = beacon.implementation(); + assertEq(oldImplementation, address(beacon.implementation()), "Should have initial implementation"); + + MasterVault newImplementation = new MasterVault(); + beacon.upgradeTo(address(newImplementation)); + + assertEq(beacon.implementation(), address(newImplementation), "Beacon should point to new implementation"); + assertTrue(beacon.implementation() != oldImplementation, "Implementation should have changed"); + + assertEq(vault.name(), name, "Name should remain after upgrade"); + } + } diff --git a/test-foundry/libraries/vault/MasterVaultFactory.t.sol b/test-foundry/libraries/vault/MasterVaultFactory.t.sol index 69d31106a..224e6de2f 100644 --- a/test-foundry/libraries/vault/MasterVaultFactory.t.sol +++ b/test-foundry/libraries/vault/MasterVaultFactory.t.sol @@ -5,6 +5,7 @@ import {Test} from "forge-std/Test.sol"; import {MasterVaultFactory} from "../../../contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol"; import {MasterVault} from "../../../contracts/tokenbridge/libraries/vault/MasterVault.sol"; import {TestERC20} from "../../../contracts/tokenbridge/test/TestERC20.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; contract MasterVaultFactoryTest is Test { MasterVaultFactory public factory; @@ -69,4 +70,45 @@ contract MasterVaultFactoryTest is Test { assertEq(calculatedAddress, deployedVault, "Address calculation incorrect"); } + + function test_beaconOwnership() public { + assertEq(factory.beacon().owner(), owner, "Beacon owner should be the factory owner"); + } + + function test_ownerCanUpgradeBeacon() public { + MasterVault newImplementation = new MasterVault(); + + UpgradeableBeacon beacon = factory.beacon(); + vm.prank(owner); + beacon.upgradeTo(address(newImplementation)); + + assertEq(factory.beacon().implementation(), address(newImplementation), "Beacon implementation should be updated"); + } + + function test_nonOwnerCannotUpgradeBeacon() public { + MasterVault newImplementation = new MasterVault(); + + UpgradeableBeacon beacon = factory.beacon(); + vm.prank(user); + vm.expectRevert("Ownable: caller is not the owner"); + beacon.upgradeTo(address(newImplementation)); + } + + function test_beaconUpgradeAffectsAllVaults() public { + address vault1 = factory.deployVault(address(token)); + + TestERC20 token2 = new TestERC20(); + address vault2 = factory.deployVault(address(token2)); + + MasterVault newImplementation = new MasterVault(); + + UpgradeableBeacon beacon = factory.beacon(); + vm.prank(owner); + beacon.upgradeTo(address(newImplementation)); + + assertEq(factory.beacon().implementation(), address(newImplementation), "Beacon should point to new implementation"); + + MasterVault(vault1).owner(); + MasterVault(vault2).owner(); + } } \ No newline at end of file From 5632d3d67be2146fa95af97b15f4377b9f18d1d4 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Fri, 3 Oct 2025 12:35:21 +0200 Subject: [PATCH 05/16] satisify slither --- .../libraries/vault/MasterVault.sol | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index b55978d21..8f3fd2840 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -30,6 +30,11 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { error NewSubVaultExchangeRateTooLow(); error BeneficiaryNotSet(); error PerformanceFeeDisabled(); + error NoSharesRedeemed(); + error NoSubvaultShares(); + error NoSharesBurned(); + error InvalidAsset(); + error InvalidOwner(); // todo: avoid inflation, rounding, other common 4626 vulns // we may need a minimum asset or master share amount when setting subvaults (bc of exchange rate calc) @@ -54,8 +59,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary); function vaultInit(IERC20 _asset, string memory _name, string memory _symbol, address _owner) external initializer { - require(address(_asset) != address(0), "INVALID_ASSET"); - require(_owner != address(0), "INVALID_OWNER"); + if (address(_asset) == address(0)) revert InvalidAsset(); + if (_owner == address(0)) revert InvalidOwner(); __ERC20_init(_name, _symbol); __ERC4626_init(IERC20Upgradeable(address(_asset))); @@ -109,15 +114,18 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { if (totalSupply() == 0) revert MustHaveSupplyBeforeSettingSubVault(); if (address(_subVault.asset()) != address(asset())) revert SubVaultAssetMismatch(); + uint256 _totalAssets = totalAssets(); + uint256 _totalSupply = totalSupply(); + + subVault = _subVault; + IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); - uint256 subShares = _subVault.deposit(totalAssets(), address(this)); + uint256 subShares = _subVault.deposit(_totalAssets, address(this)); - uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalSupply(), MathUpgradeable.Rounding.Down); + uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); subVaultExchRateWad = _subVaultExchRateWad; - subVault = _subVault; - emit SubvaultChanged(address(0), address(_subVault)); } @@ -126,14 +134,17 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { 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(); + uint256 maxWithdrawAmount = oldSubVault.maxWithdraw(address(this)); - IERC20(asset()).safeApprove(address(oldSubVault), 0); subVault = IERC4626(address(0)); subVaultExchRateWad = 1e18; + uint256 assetReceived = oldSubVault.withdraw(maxWithdrawAmount, address(this), address(this)); + IERC20(asset()).safeApprove(address(oldSubVault), 0); + + uint256 effectiveAssetExchRateWad = assetReceived.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); + if (effectiveAssetExchRateWad < minAssetExchRateWad) revert TooFewAssetsReceived(); + emit SubvaultChanged(address(oldSubVault), address(0)); } @@ -182,7 +193,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { if (totalProfits > 0) { IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - _subVault.withdraw(totalProfits, address(this), address(this)); + uint256 sharesRedeemed = _subVault.withdraw(totalProfits, address(this), address(this)); + if (sharesRedeemed == 0) revert NoSharesRedeemed(); } IERC20(asset()).safeTransfer(beneficiary, totalProfits); } @@ -260,7 +272,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { totalPrincipal += assets; IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - _subVault.deposit(assets, address(this)); + uint256 subShares = _subVault.deposit(assets, address(this)); + if (subShares == 0) revert NoSubvaultShares(); } } @@ -278,7 +291,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - _subVault.withdraw(assets, address(this), address(this)); + uint256 sharesBurned = _subVault.withdraw(assets, address(this), address(this)); + if (sharesBurned == 0) revert NoSharesBurned(); } super._withdraw(caller, receiver, _owner, assets, shares); From 2b8f151a43a14fe4a75d41ebe425951b99e0319f Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Thu, 16 Oct 2025 09:58:46 +0200 Subject: [PATCH 06/16] remove unnecessary checks --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 8f3fd2840..b641d40bf 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -193,8 +193,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { if (totalProfits > 0) { IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - uint256 sharesRedeemed = _subVault.withdraw(totalProfits, address(this), address(this)); - if (sharesRedeemed == 0) revert NoSharesRedeemed(); + _subVault.withdraw(totalProfits, address(this), address(this)); } IERC20(asset()).safeTransfer(beneficiary, totalProfits); } @@ -272,8 +271,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { totalPrincipal += assets; IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - uint256 subShares = _subVault.deposit(assets, address(this)); - if (subShares == 0) revert NoSubvaultShares(); + _subVault.deposit(assets, address(this)); } } @@ -291,8 +289,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { IERC4626 _subVault = subVault; if (address(_subVault) != address(0)) { - uint256 sharesBurned = _subVault.withdraw(assets, address(this), address(this)); - if (sharesBurned == 0) revert NoSharesBurned(); + _subVault.withdraw(assets, address(this), address(this)); } super._withdraw(caller, receiver, _owner, assets, shares); From 53486041587812cde0f4cbadf78d4f472ed4ee51 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Thu, 16 Oct 2025 09:59:56 +0200 Subject: [PATCH 07/16] fixup! feat: make master vault beacon upgradable --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index b641d40bf..a37e0957b 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -64,7 +64,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { __ERC20_init(_name, _symbol); __ERC4626_init(IERC20Upgradeable(address(_asset))); - __Ownable_init(); _transferOwnership(_owner); subVaultExchRateWad = 1e18; From 3a24b45ddf0bc96322ca2b4993fc819b4d6519a5 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Thu, 16 Oct 2025 11:11:18 +0200 Subject: [PATCH 08/16] fix: set subvault in between depsoit and _subVaultExchRateWad calculation --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index a37e0957b..6ad4ca00f 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -113,15 +113,12 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { if (totalSupply() == 0) revert MustHaveSupplyBeforeSettingSubVault(); if (address(_subVault.asset()) != address(asset())) revert SubVaultAssetMismatch(); - uint256 _totalAssets = totalAssets(); - uint256 _totalSupply = totalSupply(); + IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); + uint256 subShares = _subVault.deposit(totalAssets(), address(this)); subVault = _subVault; - IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); - uint256 subShares = _subVault.deposit(_totalAssets, address(this)); - - uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); + uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalAssets(), MathUpgradeable.Rounding.Down); if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); subVaultExchRateWad = _subVaultExchRateWad; From db0feb15c464aa349bfe89f5ed88530b6f8c648f Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Sun, 19 Oct 2025 19:43:21 +0200 Subject: [PATCH 09/16] fixup! feat: make master vault beacon upgradable --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 6ad4ca00f..f3d7d7359 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -130,17 +130,14 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { if (address(oldSubVault) == address(0)) revert NoExistingSubVault(); uint256 _totalSupply = totalSupply(); - uint256 maxWithdrawAmount = oldSubVault.maxWithdraw(address(this)); + 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; - uint256 assetReceived = oldSubVault.withdraw(maxWithdrawAmount, address(this), address(this)); - IERC20(asset()).safeApprove(address(oldSubVault), 0); - - uint256 effectiveAssetExchRateWad = assetReceived.mulDiv(1e18, _totalSupply, MathUpgradeable.Rounding.Down); - if (effectiveAssetExchRateWad < minAssetExchRateWad) revert TooFewAssetsReceived(); - emit SubvaultChanged(address(oldSubVault), address(0)); } From 1e0c2671a6cb28a27d4448958e6a00a45404c141 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Sun, 19 Oct 2025 19:45:30 +0200 Subject: [PATCH 10/16] fixup! feat: make master vault beacon upgradable --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index f3d7d7359..a763a5182 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -30,9 +30,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { error NewSubVaultExchangeRateTooLow(); error BeneficiaryNotSet(); error PerformanceFeeDisabled(); - error NoSharesRedeemed(); - error NoSubvaultShares(); - error NoSharesBurned(); error InvalidAsset(); error InvalidOwner(); From 4aad0ddffc2d7098633469bccf9f229372b48082 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Sun, 19 Oct 2025 19:51:53 +0200 Subject: [PATCH 11/16] fix: remove not used beacon address from storage --- .../libraries/vault/MasterVaultFactory.sol | 3 +-- .../libraries/vault/MasterVaultFactory.t.sol | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol index bde4d51d6..180f3c1c2 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -15,14 +15,13 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { error ZeroAddress(); error BeaconNotDeployed(); - UpgradeableBeacon public beacon; BeaconProxyFactory public beaconProxyFactory; function initialize(address _owner) public initializer { _transferOwnership(_owner); MasterVault masterVaultImplementation = new MasterVault(); - beacon = new UpgradeableBeacon(address(masterVaultImplementation)); + UpgradeableBeacon beacon = new UpgradeableBeacon(address(masterVaultImplementation)); beaconProxyFactory = new BeaconProxyFactory(); beaconProxyFactory.initialize(address(beacon)); beacon.transferOwnership(_owner); diff --git a/test-foundry/libraries/vault/MasterVaultFactory.t.sol b/test-foundry/libraries/vault/MasterVaultFactory.t.sol index 224e6de2f..268b593a8 100644 --- a/test-foundry/libraries/vault/MasterVaultFactory.t.sol +++ b/test-foundry/libraries/vault/MasterVaultFactory.t.sol @@ -72,23 +72,23 @@ contract MasterVaultFactoryTest is Test { } function test_beaconOwnership() public { - assertEq(factory.beacon().owner(), owner, "Beacon owner should be the factory owner"); + assertEq(UpgradeableBeacon(factory.beaconProxyFactory().beacon()).owner(), owner, "Beacon owner should be the factory owner"); } function test_ownerCanUpgradeBeacon() public { MasterVault newImplementation = new MasterVault(); - UpgradeableBeacon beacon = factory.beacon(); + UpgradeableBeacon beacon = UpgradeableBeacon(factory.beaconProxyFactory().beacon()); vm.prank(owner); beacon.upgradeTo(address(newImplementation)); - assertEq(factory.beacon().implementation(), address(newImplementation), "Beacon implementation should be updated"); + assertEq(UpgradeableBeacon(factory.beaconProxyFactory().beacon()).implementation(), address(newImplementation), "Beacon implementation should be updated"); } function test_nonOwnerCannotUpgradeBeacon() public { MasterVault newImplementation = new MasterVault(); - UpgradeableBeacon beacon = factory.beacon(); + UpgradeableBeacon beacon = UpgradeableBeacon(factory.beaconProxyFactory().beacon()); vm.prank(user); vm.expectRevert("Ownable: caller is not the owner"); beacon.upgradeTo(address(newImplementation)); @@ -102,11 +102,11 @@ contract MasterVaultFactoryTest is Test { MasterVault newImplementation = new MasterVault(); - UpgradeableBeacon beacon = factory.beacon(); + UpgradeableBeacon beacon = UpgradeableBeacon(factory.beaconProxyFactory().beacon()); vm.prank(owner); beacon.upgradeTo(address(newImplementation)); - assertEq(factory.beacon().implementation(), address(newImplementation), "Beacon should point to new implementation"); + assertEq(UpgradeableBeacon(factory.beaconProxyFactory().beacon()).implementation(), address(newImplementation), "Beacon should point to new implementation"); MasterVault(vault1).owner(); MasterVault(vault2).owner(); From 9ff2c39ad4335b44c37504438c9c594d3c808f11 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Sun, 19 Oct 2025 19:53:16 +0200 Subject: [PATCH 12/16] fixup! feat: make master vault beacon upgradable --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 2 +- contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol | 2 +- test-foundry/libraries/vault/MasterVault.t.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index a763a5182..0c1ab34c4 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -55,7 +55,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { event PerformanceFeeToggled(bool enabled); event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary); - function vaultInit(IERC20 _asset, string memory _name, string memory _symbol, address _owner) external initializer { + function initialize(IERC20 _asset, string memory _name, string memory _symbol, address _owner) external initializer { if (address(_asset) == address(0)) revert InvalidAsset(); if (_owner == address(0)) revert InvalidOwner(); diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol index 180f3c1c2..24a19fb81 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -44,7 +44,7 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); - MasterVault(vault).vaultInit(IERC20(token), name, symbol, address(this)); + MasterVault(vault).initialize(IERC20(token), name, symbol, address(this)); emit VaultDeployed(token, vault); } diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index 71a7299cc..84a628e32 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -34,7 +34,7 @@ contract MasterVaultTest is Test { address proxyAddress = beaconProxyFactory.createProxy(salt); vault = MasterVault(proxyAddress); - vault.vaultInit(IERC20(address(token)), name, symbol, address(this)); + vault.initialize(IERC20(address(token)), name, symbol, address(this)); } function test_initialize() public { From cd1a25d9727da66491799b873ec6b3d7cfa385ae Mon Sep 17 00:00:00 2001 From: Wael Date: Fri, 7 Nov 2025 17:35:42 +0100 Subject: [PATCH 13/16] feat: access control roles (#132) * feat: access control roles * remove OwnableUpgradeable * add note about permissionless fee manager --------- Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com> --- .../libraries/vault/IMasterVaultFactory.sol | 2 - .../libraries/vault/MasterVault.sol | 50 +++- .../libraries/vault/MasterVaultFactory.sol | 19 +- .../libraries/vault/MasterVault.t.sol | 216 ++++++++++++++++++ .../libraries/vault/MasterVaultFactory.t.sol | 6 +- 5 files changed, 261 insertions(+), 32 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol index 7ac654fca..2a7617abc 100644 --- a/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/IMasterVaultFactory.sol @@ -3,11 +3,9 @@ pragma solidity ^0.8.0; interface IMasterVaultFactory { event VaultDeployed(address indexed token, address indexed vault); - event SubVaultSet(address indexed masterVault, address indexed subVault); function initialize(address _owner) external; function deployVault(address token) external returns (address vault); function calculateVaultAddress(address token) external view returns (address); function getVault(address token) external returns (address); - function setSubVault(address masterVault, address subVault, uint256 minSubVaultExchRateWad) external; } \ No newline at end of file diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 0c1ab34c4..4f6c39b64 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -6,16 +6,21 @@ import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; -import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; +import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import {MathUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/math/MathUpgradeable.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { +contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradeable, PausableUpgradeable { using SafeERC20 for IERC20; using MathUpgradeable for uint256; + bytes32 public constant VAULT_MANAGER_ROLE = keccak256("VAULT_MANAGER_ROLE"); + bytes32 public constant FEE_MANAGER_ROLE = keccak256("FEE_MANAGER_ROLE"); + bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); + error TooFewSharesReceived(); error TooManySharesBurned(); error TooManyAssetsDeposited(); @@ -61,7 +66,17 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { __ERC20_init(_name, _symbol); __ERC4626_init(IERC20Upgradeable(address(_asset))); - _transferOwnership(_owner); + __AccessControl_init(); + __Pausable_init(); + + _setRoleAdmin(VAULT_MANAGER_ROLE, DEFAULT_ADMIN_ROLE); + _setRoleAdmin(FEE_MANAGER_ROLE, DEFAULT_ADMIN_ROLE); + _setRoleAdmin(PAUSER_ROLE, DEFAULT_ADMIN_ROLE); + + _grantRole(DEFAULT_ADMIN_ROLE, _owner); + _grantRole(VAULT_MANAGER_ROLE, _owner); + _grantRole(FEE_MANAGER_ROLE, _owner); // todo: consider permissionless by default + _grantRole(PAUSER_ROLE, _owner); subVaultExchRateWad = 1e18; } @@ -94,14 +109,14 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { /// @notice Set a subvault. Can only be called if there is not already a subvault set. /// @param _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault. /// @param minSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit. - function setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyOwner { + function setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { if (address(subVault) != address(0)) revert SubVaultAlreadySet(); _setSubVault(_subVault, minSubVaultExchRateWad); } /// @notice Revokes the current subvault, moving all assets back to MasterVault /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from subvault to outstanding MasterVault shares - function revokeSubVault(uint256 minAssetExchRateWad) external onlyOwner { + function revokeSubVault(uint256 minAssetExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { _revokeSubVault(minAssetExchRateWad); } @@ -142,7 +157,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { /// @param newSubVault The new subvault to switch to, or zero address to revoke current subvault /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from old subvault to outstanding MasterVault shares /// @param minNewSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit - function switchSubVault(IERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyOwner { + function switchSubVault(IERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { _revokeSubVault(minAssetExchRateWad); if (address(newSubVault) != address(0)) { @@ -160,22 +175,22 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { /// @notice Toggle performance fee collection on/off /// @param enabled True to enable performance fees, false to disable - function setPerformanceFee(bool enabled) external onlyOwner { + function setPerformanceFee(bool enabled) external onlyRole(VAULT_MANAGER_ROLE) { enablePerformanceFee = enabled; emit PerformanceFeeToggled(enabled); } /// @notice Set the beneficiary address for performance fees /// @param newBeneficiary Address to receive performance fees, zero address defaults to owner - function setBeneficiary(address newBeneficiary) external onlyOwner { + function setBeneficiary(address newBeneficiary) external onlyRole(FEE_MANAGER_ROLE) { address oldBeneficiary = beneficiary; beneficiary = newBeneficiary; emit BeneficiaryUpdated(oldBeneficiary, newBeneficiary); } /// @notice Withdraw all accumulated performance fees to beneficiary - /// @dev Only callable by owner when performance fees are enabled - function withdrawPerformanceFees() external onlyOwner { + /// @dev Only callable by fee manager when performance fees are enabled + function withdrawPerformanceFees() external onlyRole(FEE_MANAGER_ROLE) { if (!enablePerformanceFee) revert PerformanceFeeDisabled(); if (beneficiary == address(0)) revert BeneficiaryNotSet(); @@ -189,6 +204,14 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { } } + function pause() external onlyRole(PAUSER_ROLE) { + _pause(); + } + + function unpause() external onlyRole(PAUSER_ROLE) { + _unpause(); + } + /** @dev See {IERC4626-totalAssets}. */ function totalAssets() public view virtual override returns (uint256) { IERC4626 _subVault = subVault; @@ -208,6 +231,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { /** @dev See {IERC4626-maxMint}. */ function maxMint(address) public view virtual override returns (uint256) { + if (address(subVault) == address(0)) { + return type(uint256).max; + } uint256 subShares = subVault.maxMint(address(this)); if (subShares == type(uint256).max) { return type(uint256).max; @@ -255,7 +281,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { address receiver, uint256 assets, uint256 shares - ) internal virtual override { + ) internal virtual override whenNotPaused { super._deposit(caller, receiver, assets, shares); totalPrincipal += assets; @@ -274,7 +300,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, OwnableUpgradeable { address _owner, uint256 assets, uint256 shares - ) internal virtual override { + ) internal virtual override whenNotPaused { totalPrincipal -= assets; IERC4626 _subVault = subVault; diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol index 24a19fb81..0259c84d4 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/utils/Create2.sol"; -import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; import "../ClonableBeaconProxy.sol"; @@ -11,15 +10,15 @@ import "./IMasterVault.sol"; import "./IMasterVaultFactory.sol"; import "./MasterVault.sol"; -contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { +contract MasterVaultFactory is IMasterVaultFactory, Initializable { error ZeroAddress(); error BeaconNotDeployed(); BeaconProxyFactory public beaconProxyFactory; + address public owner; function initialize(address _owner) public initializer { - _transferOwnership(_owner); - + owner = _owner; MasterVault masterVaultImplementation = new MasterVault(); UpgradeableBeacon beacon = new UpgradeableBeacon(address(masterVaultImplementation)); beaconProxyFactory = new BeaconProxyFactory(); @@ -44,7 +43,7 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { string memory name = string(abi.encodePacked("Master ", tokenMetadata.name())); string memory symbol = string(abi.encodePacked("m", tokenMetadata.symbol())); - MasterVault(vault).initialize(IERC20(token), name, symbol, address(this)); + MasterVault(vault).initialize(IERC20(token), name, symbol, owner); emit VaultDeployed(token, vault); } @@ -65,14 +64,4 @@ contract MasterVaultFactory is IMasterVaultFactory, OwnableUpgradeable { } return vault; } - - // todo: consider a method to enable bridge owner to transfer specific master vault ownership to new address - function setSubVault( - address masterVault, - address subVault, - uint256 minSubVaultExchRateWad - ) external onlyOwner { - IMasterVault(masterVault).setSubVault(subVault, minSubVaultExchRateWad); - emit SubVaultSet(masterVault, subVault); - } } diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index 84a628e32..f78c4f2c2 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -8,6 +8,7 @@ import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.s import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { UpgradeableBeacon } from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; import { BeaconProxyFactory, ClonableBeaconProxy } from "../../../contracts/tokenbridge/libraries/ClonableBeaconProxy.sol"; +import { IAccessControl } from "@openzeppelin/contracts/access/IAccessControl.sol"; contract MasterVaultTest is Test { MasterVault public vault; @@ -45,6 +46,10 @@ contract MasterVaultTest is Test { assertEq(vault.totalSupply(), 0, "Invalid initial supply"); assertEq(vault.totalAssets(), 0, "Invalid initial assets"); assertEq(address(vault.subVault()), address(0), "SubVault should be zero initially"); + + assertTrue(vault.hasRole(vault.DEFAULT_ADMIN_ROLE(), address(this)), "Should have DEFAULT_ADMIN_ROLE"); + assertTrue(vault.hasRole(vault.VAULT_MANAGER_ROLE(), address(this)), "Should have VAULT_MANAGER_ROLE"); + assertTrue(vault.hasRole(vault.FEE_MANAGER_ROLE(), address(this)), "Should have FEE_MANAGER_ROLE"); } function test_WithoutSubvault_deposit() public { @@ -297,4 +302,215 @@ contract MasterVaultTest is Test { assertEq(vault.name(), name, "Name should remain after upgrade"); } + function test_setSubVault_revert_NotVaultManager() public { + MockSubVault subVault = new MockSubVault( + IERC20(address(token)), + "Sub Vault Token", + "svTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + + vm.expectRevert(); + vault.setSubVault(subVault, 1e18); + + vm.stopPrank(); + } + + function test_setBeneficiary_revert_NotFeeManager() public { + address newBeneficiary = address(0x999); + + vm.prank(user); + vm.expectRevert(); + vault.setBeneficiary(newBeneficiary); + } + + function test_withdrawPerformanceFees_revert_NotFeeManager() public { + vm.prank(user); + vm.expectRevert(); + vault.withdrawPerformanceFees(); + } + + function test_roleAdmin() public { + address vaultManager = address(0x1111); + address feeManager = address(0x2222); + + vault.grantRole(vault.VAULT_MANAGER_ROLE(), vaultManager); + vault.grantRole(vault.FEE_MANAGER_ROLE(), feeManager); + + assertTrue(vault.hasRole(vault.VAULT_MANAGER_ROLE(), vaultManager), "Should have VAULT_MANAGER_ROLE"); + assertTrue(vault.hasRole(vault.FEE_MANAGER_ROLE(), feeManager), "Should have FEE_MANAGER_ROLE"); + + vault.revokeRole(vault.VAULT_MANAGER_ROLE(), vaultManager); + assertFalse(vault.hasRole(vault.VAULT_MANAGER_ROLE(), vaultManager), "Should not have VAULT_MANAGER_ROLE"); + } + + function test_multipleRoleHolders() public { + address vaultManager1 = address(0x1111); + address vaultManager2 = address(0x2222); + + vault.grantRole(vault.VAULT_MANAGER_ROLE(), vaultManager1); + vault.grantRole(vault.VAULT_MANAGER_ROLE(), vaultManager2); + + assertTrue(vault.hasRole(vault.VAULT_MANAGER_ROLE(), vaultManager1), "Manager1 should have VAULT_MANAGER_ROLE"); + assertTrue(vault.hasRole(vault.VAULT_MANAGER_ROLE(), vaultManager2), "Manager2 should have VAULT_MANAGER_ROLE"); + + MockSubVault subVault = new MockSubVault( + IERC20(address(token)), + "Sub Vault Token", + "svTST" + ); + + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + vm.prank(vaultManager1); + vault.setSubVault(subVault, 1e18); + + assertEq(address(vault.subVault()), address(subVault), "SubVault should be set by manager1"); + } + + function test_initialize_pauserRole() public { + assertTrue(vault.hasRole(vault.PAUSER_ROLE(), address(this)), "Should have PAUSER_ROLE"); + assertFalse(vault.paused(), "Should not be paused initially"); + } + + function test_pause() public { + assertFalse(vault.paused(), "Should not be paused initially"); + + vault.pause(); + + assertTrue(vault.paused(), "Should be paused"); + } + + function test_unpause() public { + vault.pause(); + assertTrue(vault.paused(), "Should be paused"); + + vault.unpause(); + + assertFalse(vault.paused(), "Should not be paused"); + } + + function test_pause_revert_NotPauser() public { + vm.prank(user); + vm.expectRevert(); + vault.pause(); + } + + function test_unpause_revert_NotPauser() public { + vault.pause(); + + vm.prank(user); + vm.expectRevert(); + vault.unpause(); + } + + function test_deposit_revert_WhenPaused() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vm.stopPrank(); + + vault.pause(); + + vm.prank(user); + vm.expectRevert("Pausable: paused"); + vault.deposit(depositAmount, user, 0); + } + + function test_withdraw_revert_WhenPaused() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + vault.pause(); + + vm.prank(user); + vm.expectRevert("Pausable: paused"); + vault.withdraw(depositAmount / 2, user, user, type(uint256).max); + } + + function test_mint_revert_WhenPaused() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vm.stopPrank(); + + vault.pause(); + + vm.prank(user); + vm.expectRevert("Pausable: paused"); + vault.mint(100, user, type(uint256).max); + } + + function test_redeem_revert_WhenPaused() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + uint256 shares = vault.deposit(depositAmount, user, 0); + vm.stopPrank(); + + vault.pause(); + + vm.prank(user); + vm.expectRevert("Pausable: paused"); + vault.redeem(shares / 2, user, user, 0); + } + + function test_pauseUnpauseFlow() public { + vm.startPrank(user); + token.mint(); + uint256 depositAmount = token.balanceOf(user); + token.approve(address(vault), depositAmount); + vault.deposit(depositAmount / 2, user, 0); + vm.stopPrank(); + + vault.pause(); + + vm.prank(user); + vm.expectRevert("Pausable: paused"); + vault.deposit(depositAmount / 2, user, 0); + + vault.unpause(); + + vm.prank(user); + vault.deposit(depositAmount / 2, user, 0); + + assertEq(token.balanceOf(user), 0, "All tokens should be deposited"); + } + + function test_multiplePausers() public { + address pauser1 = address(0x3333); + address pauser2 = address(0x4444); + + vault.grantRole(vault.PAUSER_ROLE(), pauser1); + vault.grantRole(vault.PAUSER_ROLE(), pauser2); + + assertTrue(vault.hasRole(vault.PAUSER_ROLE(), pauser1), "Pauser1 should have PAUSER_ROLE"); + assertTrue(vault.hasRole(vault.PAUSER_ROLE(), pauser2), "Pauser2 should have PAUSER_ROLE"); + + vm.prank(pauser1); + vault.pause(); + assertTrue(vault.paused(), "Should be paused by pauser1"); + + vm.prank(pauser2); + vault.unpause(); + assertFalse(vault.paused(), "Should be unpaused by pauser2"); + } + } diff --git a/test-foundry/libraries/vault/MasterVaultFactory.t.sol b/test-foundry/libraries/vault/MasterVaultFactory.t.sol index 268b593a8..48359c390 100644 --- a/test-foundry/libraries/vault/MasterVaultFactory.t.sol +++ b/test-foundry/libraries/vault/MasterVaultFactory.t.sol @@ -41,7 +41,7 @@ contract MasterVaultFactoryTest is Test { MasterVault vault = MasterVault(deployedVault); assertEq(address(vault.asset()), address(token), "Invalid vault asset"); - assertEq(vault.owner(), address(factory), "Invalid vault owner"); + assertTrue(vault.hasRole(vault.DEFAULT_ADMIN_ROLE(), owner), "Factory owner should have DEFAULT_ADMIN_ROLE"); } function test_deployVault_RevertZeroAddress() public { @@ -108,7 +108,7 @@ contract MasterVaultFactoryTest is Test { assertEq(UpgradeableBeacon(factory.beaconProxyFactory().beacon()).implementation(), address(newImplementation), "Beacon should point to new implementation"); - MasterVault(vault1).owner(); - MasterVault(vault2).owner(); + assertTrue(MasterVault(vault1).hasRole(MasterVault(vault1).DEFAULT_ADMIN_ROLE(), owner), "Vault1 should have owner as admin"); + assertTrue(MasterVault(vault2).hasRole(MasterVault(vault2).DEFAULT_ADMIN_ROLE(), owner), "Vault2 should have owner as admin"); } } \ No newline at end of file From 35c2a3cca6b370fb54f55d4aa7525d3062123faf Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Mon, 10 Nov 2025 15:08:57 -0700 Subject: [PATCH 14/16] remove subVaultExchRateWad --- .../libraries/vault/MasterVault.sol | 43 ++++++++----------- .../libraries/vault/MasterVault.t.sol | 4 -- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 4f6c39b64..187da940f 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -42,12 +42,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea // we may need a minimum asset or master share amount when setting subvaults (bc of exchange rate calc) IERC4626 public subVault; - // how many subVault shares one MV2 share can be redeemed for - // initially 1 to 1 - // constant per subvault - // changes when subvault is set - uint256 public subVaultExchRateWad; - // note: the performance fee can be avoided if the underlying strategy can be sandwiched (eg ETH to wstETH dex swap) // maybe a simpler and more robust implementation would be for the owner to adjust the subVaultExchRateWad directly // this would also avoid the need for totalPrincipal tracking @@ -77,8 +71,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea _grantRole(VAULT_MANAGER_ROLE, _owner); _grantRole(FEE_MANAGER_ROLE, _owner); // todo: consider permissionless by default _grantRole(PAUSER_ROLE, _owner); - - subVaultExchRateWad = 1e18; } @@ -110,7 +102,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea /// @param _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault. /// @param minSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit. function setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { - if (address(subVault) != address(0)) revert SubVaultAlreadySet(); _setSubVault(_subVault, minSubVaultExchRateWad); } @@ -121,18 +112,17 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea } 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)); + IERC20 underlyingAsset = IERC20(asset()); + if (address(subVault) != address(0)) revert SubVaultAlreadySet(); + if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch(); subVault = _subVault; - uint256 _subVaultExchRateWad = subShares.mulDiv(1e18, totalAssets(), MathUpgradeable.Rounding.Down); - if (_subVaultExchRateWad < minSubVaultExchRateWad) revert SubVaultExchangeRateTooLow(); - subVaultExchRateWad = _subVaultExchRateWad; + IERC20(asset()).safeApprove(address(_subVault), type(uint256).max); + _subVault.deposit(underlyingAsset.balanceOf(address(this)), address(this)); + + uint256 subVaultExchRateWad = _subVault.balanceOf(address(this)).mulDiv(1e18, totalSupply(), MathUpgradeable.Rounding.Down); + if (subVaultExchRateWad < minSubVaultExchRateWad) revert NewSubVaultExchangeRateTooLow(); emit SubvaultChanged(address(0), address(_subVault)); } @@ -141,14 +131,13 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea 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(); + subVault = IERC4626(address(0)); + oldSubVault.redeem(oldSubVault.balanceOf(address(this)), address(this), address(this)); IERC20(asset()).safeApprove(address(oldSubVault), 0); - subVault = IERC4626(address(0)); - subVaultExchRateWad = 1e18; + + uint256 assetExchRateWad = IERC20(asset()).balanceOf(address(this)).mulDiv(1e18, totalSupply(), MathUpgradeable.Rounding.Down); + if (assetExchRateWad < minAssetExchRateWad) revert SubVaultExchangeRateTooLow(); emit SubvaultChanged(address(oldSubVault), address(0)); } @@ -166,11 +155,13 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea } function masterSharesToSubShares(uint256 masterShares, MathUpgradeable.Rounding rounding) public view returns (uint256) { - return masterShares.mulDiv(subVaultExchRateWad, 1e18, rounding); + // masterShares * totalSubVaultShares / totalMasterShares + return masterShares.mulDiv(subVault.balanceOf(address(this)), totalSupply(), rounding); } function subSharesToMasterShares(uint256 subShares, MathUpgradeable.Rounding rounding) public view returns (uint256) { - return subShares.mulDiv(1e18, subVaultExchRateWad, rounding); + // subShares * totalMasterShares / totalSubVaultShares + return subShares.mulDiv(totalSupply(), subVault.balanceOf(address(this)), rounding); } /// @notice Toggle performance fee collection on/off diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index f78c4f2c2..57b7f93b9 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -122,7 +122,6 @@ contract MasterVaultTest is Test { vault.setSubVault(subVault, minSubVaultExchRateWad); assertEq(address(vault.subVault()), address(subVault), "SubVault should be set"); - assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should be 1:1 initially"); assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); assertEq(subVault.balanceOf(address(vault)), depositAmount, "SubVault should have received assets"); } @@ -164,7 +163,6 @@ contract MasterVaultTest is Test { vault.switchSubVault(newSubVault, minAssetExchRateWad, minNewSubVaultExchRateWad); assertEq(address(vault.subVault()), address(newSubVault), "New subvault should be set"); - assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should remain 1:1"); assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); assertEq(oldSubVault.balanceOf(address(vault)), 0, "Old subvault should have no assets"); assertEq(newSubVault.balanceOf(address(vault)), depositAmount, "New subvault should have received assets"); @@ -188,7 +186,6 @@ contract MasterVaultTest is Test { assertEq(address(vault.subVault()), address(subVault), "SubVault should be set"); assertEq(subVault.balanceOf(address(vault)), depositAmount, "SubVault should have assets"); - assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should be 1:1"); uint256 minAssetExchRateWad = 1e18; @@ -198,7 +195,6 @@ contract MasterVaultTest is Test { vault.revokeSubVault(minAssetExchRateWad); assertEq(address(vault.subVault()), address(0), "SubVault should be revoked"); - assertEq(vault.subVaultExchRateWad(), 1e18, "Exchange rate should reset to 1:1"); assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); assertEq(subVault.balanceOf(address(vault)), 0, "SubVault should have no assets"); assertEq(token.balanceOf(address(vault)), depositAmount, "MasterVault should have assets directly"); From 9b4809c2785f56ade6aebe64fb7c6b2847a6c487 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Mon, 10 Nov 2025 15:13:26 -0700 Subject: [PATCH 15/16] remove unused errors --- contracts/tokenbridge/libraries/vault/MasterVault.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 187da940f..8c43eac5d 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -26,12 +26,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea error TooManyAssetsDeposited(); error TooFewAssetsReceived(); error SubVaultAlreadySet(); - error SubVaultCannotBeZeroAddress(); - error MustHaveSupplyBeforeSettingSubVault(); error SubVaultAssetMismatch(); error SubVaultExchangeRateTooLow(); error NoExistingSubVault(); - error MustHaveSupplyBeforeSwitchingSubVault(); error NewSubVaultExchangeRateTooLow(); error BeneficiaryNotSet(); error PerformanceFeeDisabled(); From a05eddcc659d62c71259984b4e8bc42c6b765106 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Mon, 10 Nov 2025 15:34:42 -0700 Subject: [PATCH 16/16] remove switchSubVault --- .../libraries/vault/MasterVault.sol | 26 ++---------- .../libraries/vault/MasterVault.t.sol | 42 ------------------- 2 files changed, 3 insertions(+), 65 deletions(-) diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol index 8c43eac5d..5897d4286 100644 --- a/contracts/tokenbridge/libraries/vault/MasterVault.sol +++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol @@ -99,16 +99,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea /// @param _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault. /// @param minSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit. function setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { - _setSubVault(_subVault, minSubVaultExchRateWad); - } - - /// @notice Revokes the current subvault, moving all assets back to MasterVault - /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from subvault to outstanding MasterVault shares - function revokeSubVault(uint256 minAssetExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { - _revokeSubVault(minAssetExchRateWad); - } - - function _setSubVault(IERC4626 _subVault, uint256 minSubVaultExchRateWad) internal { IERC20 underlyingAsset = IERC20(asset()); if (address(subVault) != address(0)) revert SubVaultAlreadySet(); if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch(); @@ -124,7 +114,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea emit SubvaultChanged(address(0), address(_subVault)); } - function _revokeSubVault(uint256 minAssetExchRateWad) internal { + /// @notice Revokes the current subvault, moving all assets back to MasterVault + /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from subvault to outstanding MasterVault shares + function revokeSubVault(uint256 minAssetExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { IERC4626 oldSubVault = subVault; if (address(oldSubVault) == address(0)) revert NoExistingSubVault(); @@ -139,18 +131,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea emit SubvaultChanged(address(oldSubVault), address(0)); } - /// @notice Switches to a new subvault or revokes current subvault if newSubVault is zero address - /// @param newSubVault The new subvault to switch to, or zero address to revoke current subvault - /// @param minAssetExchRateWad Minimum acceptable ratio (times 1e18) of assets received from old subvault to outstanding MasterVault shares - /// @param minNewSubVaultExchRateWad Minimum acceptable ratio (times 1e18) of new subvault shares to outstanding MasterVault shares after deposit - function switchSubVault(IERC4626 newSubVault, uint256 minAssetExchRateWad, uint256 minNewSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) { - _revokeSubVault(minAssetExchRateWad); - - if (address(newSubVault) != address(0)) { - _setSubVault(newSubVault, minNewSubVaultExchRateWad); - } - } - function masterSharesToSubShares(uint256 masterShares, MathUpgradeable.Rounding rounding) public view returns (uint256) { // masterShares * totalSubVaultShares / totalMasterShares return masterShares.mulDiv(subVault.balanceOf(address(this)), totalSupply(), rounding); diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol index 57b7f93b9..384ef9a39 100644 --- a/test-foundry/libraries/vault/MasterVault.t.sol +++ b/test-foundry/libraries/vault/MasterVault.t.sol @@ -126,48 +126,6 @@ contract MasterVaultTest is Test { assertEq(subVault.balanceOf(address(vault)), depositAmount, "SubVault should have received assets"); } - function test_switchSubvault() public { - MockSubVault oldSubVault = new MockSubVault( - IERC20(address(token)), - "Old Sub Vault", - "osvTST" - ); - - MockSubVault newSubVault = new MockSubVault( - IERC20(address(token)), - "New Sub Vault", - "nsvTST" - ); - - vm.startPrank(user); - token.mint(); - uint256 depositAmount = token.balanceOf(user); - token.approve(address(vault), depositAmount); - vault.deposit(depositAmount, user, 0); - vm.stopPrank(); - - vault.setSubVault(oldSubVault, 1e18); - - assertEq(address(vault.subVault()), address(oldSubVault), "Old subvault should be set"); - assertEq(oldSubVault.balanceOf(address(vault)), depositAmount, "Old subvault should have assets"); - assertEq(newSubVault.balanceOf(address(vault)), 0, "New subvault should have no assets initially"); - - uint256 minAssetExchRateWad = 1e18; - uint256 minNewSubVaultExchRateWad = 1e18; - - vm.expectEmit(true, true, false, false); - emit SubvaultChanged(address(oldSubVault), address(0)); - vm.expectEmit(true, true, false, false); - emit SubvaultChanged(address(0), address(newSubVault)); - - vault.switchSubVault(newSubVault, minAssetExchRateWad, minNewSubVaultExchRateWad); - - assertEq(address(vault.subVault()), address(newSubVault), "New subvault should be set"); - assertEq(vault.totalAssets(), depositAmount, "Total assets should remain the same"); - assertEq(oldSubVault.balanceOf(address(vault)), 0, "Old subvault should have no assets"); - assertEq(newSubVault.balanceOf(address(vault)), depositAmount, "New subvault should have received assets"); - } - function test_revokeSubvault() public { MockSubVault subVault = new MockSubVault( IERC20(address(token)),