From 7922536aa59c93deb736601e8ad9a49c2999d6a9 Mon Sep 17 00:00:00 2001 From: Wael Almattar Date: Wed, 1 Oct 2025 15:42:06 +0200 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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 08/10] 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 09/10] 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 10/10] 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 {