From dc2c6aba9700502159ddf0c3eb14533eb0c8d663 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Thu, 11 Dec 2025 13:41:59 -0500
Subject: [PATCH 01/27] wip

---
 .../libraries/vault/MasterVault.sol           | 140 ++++++++----------
 .../libraries/vault/MasterVault.t.sol         |   4 +-
 .../libraries/vault/MasterVaultAttack.t.sol   |   2 +-
 .../libraries/vault/MasterVaultCore.t.sol     |   1 -
 .../libraries/vault/MasterVaultFee.t.sol      |   4 +-
 5 files changed, 65 insertions(+), 86 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 121562c90..b014a9de0 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -50,6 +50,8 @@ 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;
 
+    uint256 targetAllocationWad;
+
     /// @notice Flag indicating if performance fee is enabled
     bool public enablePerformanceFee;
 
@@ -81,7 +83,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         _grantRole(VAULT_MANAGER_ROLE, _owner);
         _grantRole(PAUSER_ROLE, _owner);
 
-        _pause(); 
+        // todo: deploy initial subvault
     }
 
     function distributePerformanceFee() external whenNotPaused {
@@ -102,46 +104,22 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit PerformanceFeesWithdrawn(beneficiary, profit);
     }
 
+    error NonZeroTargetAllocation(uint256 targetAllocationWad);
+
     /// @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 onlyRole(VAULT_MANAGER_ROLE) {
+    function setSubVault(IERC4626 _subVault) external onlyRole(VAULT_MANAGER_ROLE) {
         IERC20 underlyingAsset = IERC20(asset());
-        if (address(subVault) != address(0)) revert SubVaultAlreadySet();
         if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch();
+        if (targetAllocationWad != 0) revert NonZeroTargetAllocation(targetAllocationWad);
 
+        address oldSubVault = address(subVault);
         subVault = _subVault;
 
+        if (oldSubVault != address(0)) IERC20(asset()).safeApprove(address(oldSubVault), 0);
         IERC20(asset()).safeApprove(address(_subVault), type(uint256).max);
-        _subVault.deposit(underlyingAsset.balanceOf(address(this)), address(this));
-
-        uint256 supply = totalSupply();
-        if (supply > 0) {
-            uint256 subVaultExchRateWad = _subVault.balanceOf(address(this)).mulDiv(1e18, supply, MathUpgradeable.Rounding.Down);
-            if (subVaultExchRateWad < minSubVaultExchRateWad) revert NewSubVaultExchangeRateTooLow();
-        }
-
-        emit SubvaultChanged(address(0), address(_subVault));
-    }
-
-    /// @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();
-
-        subVault = IERC4626(address(0));
-
-        oldSubVault.redeem(oldSubVault.balanceOf(address(this)), address(this), address(this));
-        IERC20(asset()).safeApprove(address(oldSubVault), 0);
-
-        uint256 supply = totalSupply();
-        if (supply > 0) {
-            uint256 assetExchRateWad = IERC20(asset()).balanceOf(address(this)).mulDiv(1e18, supply, MathUpgradeable.Rounding.Down);
-            if (assetExchRateWad < minAssetExchRateWad) revert SubVaultExchangeRateTooLow();
-        }
 
-        emit SubvaultChanged(address(oldSubVault), address(0));
+        emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
     /// @notice Toggle performance fee collection on/off
@@ -207,15 +185,16 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         return __totalAssets > totalPrincipal ? __totalAssets - totalPrincipal : 0;
     }
 
+    function totalProfitInIdleAssets(MathUpgradeable.Rounding rounding) public view returns (uint256) {
+        return totalProfit(rounding).mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
+    }
+
     function totalProfitInSubVaultShares(MathUpgradeable.Rounding rounding) public view returns (uint256) {
-        if (address(subVault) == address(0)) {
-            revert("Subvault not set");
-        }
         uint256 profitAssets = totalProfit(rounding);
         if (profitAssets == 0) {
             return 0;
         }
-        return _assetsToSubVaultShares(profitAssets, rounding);
+        return _assetsToSubVaultShares(profitAssets.mulDiv(targetAllocationWad, 1e18, rounding), rounding);
     }
 
     /**
@@ -225,16 +204,12 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address caller,
         address receiver,
         uint256 assets,
-        uint256 shares
-    ) internal virtual override whenNotPaused {
-        super._deposit(caller, receiver, assets, shares);
-
+        uint256 shares,
+        uint256 assetsToDeposit
+    ) internal whenNotPaused {
+        _deposit(caller, receiver, assets, shares);
         if (enablePerformanceFee) totalPrincipal += assets;
-
-        IERC4626 _subVault = subVault;
-        if (address(_subVault) != address(0)) {
-           _subVault.deposit(assets, address(this));
-        }
+        subVault.deposit(assetsToDeposit, address(this));
     }
 
     /**
@@ -258,12 +233,10 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     }
 
     function _totalAssets(MathUpgradeable.Rounding rounding) internal view returns (uint256) {
-        if (address(subVault) == address(0)) {
-            return IERC20(asset()).balanceOf(address(this));
-        }
-        return _subVaultSharesToAssets(subVault.balanceOf(address(this)), rounding);
+        return IERC20(asset()).balanceOf(address(this)) + _subVaultSharesToAssets(subVault.balanceOf(address(this)), rounding);
     }
 
+    // todo: question: will this drift over time? i don't think so but worth checking and testing for
     /**
      * @dev Internal conversion function (from assets to shares) with support for rounding direction.
      *
@@ -271,63 +244,70 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * would represent an infinite amount of shares.
      */
     function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) {
-        uint256 supply = totalSupply();
-
-        if (address(subVault) == address(0)) {
-            uint256 effectiveTotalAssets = enablePerformanceFee ? _min(totalAssets(), totalPrincipal) : totalAssets();
-
-            if (supply == 0 || effectiveTotalAssets == 0) {
-                return assets;  
-            }
+        (,,shares) = _convertToShares2(assets, rounding);
+    }
 
-            return supply.mulDiv(assets, effectiveTotalAssets, rounding);
-        }
+    function _convertToShares2(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 sharesFromIdle, uint256 sharesFromSubVault, uint256 sharesFromBoth) {
+        uint256 supply = totalSupply();
 
+        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
         uint256 totalSubShares = subVault.balanceOf(address(this));
 
         if (enablePerformanceFee) {
-            // since we use totalSubShares in the denominator of the final calculation,
+            // since we use totalSubShares and totalIdle in the denominators of the final calculation,
             // and we are subtracting profit from it, we should use the same rounding direction for profit
             totalSubShares -= totalProfitInSubVaultShares(_flipRounding(rounding));
+            totalIdle -= totalProfitInIdleAssets(_flipRounding(rounding));
         }
 
-        uint256 subShares = _assetsToSubVaultShares(assets, rounding);
+        // figure out how much assets should be deposited to subvault vs kept idle
+        // same rounding direction since they are used in the numerators of the final calculation
+        uint256 assetsForIdle = assets.mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
+        uint256 assetsForSubVault = assets.mulDiv(targetAllocationWad, 1e18, rounding);
 
-        if (supply == 0 || totalSubShares == 0) {
-            return subShares;  
-        }
+        // figure out how many shares would be issued according to each portion
+        sharesFromIdle = assetsForIdle.mulDiv(supply, totalIdle, rounding);
+        sharesFromSubVault = _assetsToSubVaultShares(assetsForSubVault, rounding).mulDiv(supply, totalSubShares, rounding);
 
-        return supply.mulDiv(subShares, totalSubShares, rounding);
+        // take the min if rounding down, max if rounding up
+        sharesFromBoth = rounding == MathUpgradeable.Rounding.Down
+            ? MathUpgradeable.min(sharesFromIdle, sharesFromSubVault)
+            : MathUpgradeable.max(sharesFromIdle, sharesFromSubVault);
     }
 
     /**
      * @dev Internal conversion function (from shares to assets) with support for rounding direction.
      */
     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) {
-        uint256 _totalSupply = totalSupply();
-
-        if(_totalSupply == 0) {
-            return shares;
-        }
+        (,, assets) = _convertToAssets2(shares, rounding);
+    }
 
-        // if we have no subvault, we just do normal pro-rata calculation
-        if (address(subVault) == address(0)) {
-            uint256 effectiveTotalAssets = enablePerformanceFee ? _min(totalAssets(), totalPrincipal) : totalAssets();
-            return effectiveTotalAssets.mulDiv(shares, _totalSupply, rounding);
-        }
+    function _convertToAssets2(uint256 shares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assetsFromIdle, uint256 assetsFromSubVault, uint256 assetsFromBoth) {
+        uint256 supply = totalSupply();
 
+        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
         uint256 totalSubShares = subVault.balanceOf(address(this));
 
         if (enablePerformanceFee) {
-            // since we use totalSubShares in the numerator of the final calculation,
+            // since we use totalSubShares and totalIdle in the numerators of the final calculation,
             // and we are subtracting profit from it, we should use the opposite rounding direction for profit
             totalSubShares -= totalProfitInSubVaultShares(_flipRounding(rounding));
+            totalIdle -= totalProfitInIdleAssets(_flipRounding(rounding));
         }
-        
-        // totalSubShares * shares / totalMasterShares
-        uint256 subShares = totalSubShares.mulDiv(shares, _totalSupply, rounding);
 
-        return _subVaultSharesToAssets(subShares, rounding);
+        // figure out how many shares should be burned for subvault shares vs idle
+        // same rounding direction since they are used in the numerators of the final calculation (todo: confirm rounding direction)
+        uint256 sharesForIdle = shares.mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
+        uint256 sharesForSubVault = shares.mulDiv(targetAllocationWad, 1e18, rounding);
+
+        // figure out how much assets would be received according to each portion
+        assetsFromIdle = sharesForIdle.mulDiv(totalIdle, supply, rounding);
+        assetsFromSubVault = _subVaultSharesToAssets(sharesForSubVault.mulDiv(totalSubShares, supply, rounding), rounding);
+        
+        // take the min if rounding down, max if rounding up
+        assetsFromBoth = rounding == MathUpgradeable.Rounding.Down
+            ? MathUpgradeable.min(assetsFromIdle, assetsFromSubVault)
+            : MathUpgradeable.max(assetsFromIdle, assetsFromSubVault);
     }
 
     function _assetsToSubVaultShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 subShares) {
diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol
index 42563d9c2..9d545c521 100644
--- a/test-foundry/libraries/vault/MasterVault.t.sol
+++ b/test-foundry/libraries/vault/MasterVault.t.sol
@@ -120,7 +120,7 @@ contract MasterVaultTestWithSubvaultFresh is MasterVaultTest {
     function setUp() public override {
         super.setUp();
         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
-        vault.setSubVault(IERC4626(address(_subvault)), 0);
+        vault.setSubVault(IERC4626(address(_subvault)));
     }
 }
 
@@ -144,6 +144,6 @@ contract MasterVaultTestWithSubvaultHoldingAssets is MasterVaultTest {
             "subvault should be initiated with shares = _initAmount"
         );
 
-        vault.setSubVault(IERC4626(address(_subvault)), 0);
+        vault.setSubVault(IERC4626(address(_subvault)));
     }
 }
diff --git a/test-foundry/libraries/vault/MasterVaultAttack.t.sol b/test-foundry/libraries/vault/MasterVaultAttack.t.sol
index 29d9c3109..3523dfaa3 100644
--- a/test-foundry/libraries/vault/MasterVaultAttack.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultAttack.t.sol
@@ -11,7 +11,7 @@ contract MasterVaultTestWithSubvaultFresh is MasterVaultTest {
     function setUp() public override {
         super.setUp();
         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
-        vault.setSubVault(IERC4626(address(_subvault)), 0);
+        vault.setSubVault(IERC4626(address(_subvault)));
     }
 }
 
diff --git a/test-foundry/libraries/vault/MasterVaultCore.t.sol b/test-foundry/libraries/vault/MasterVaultCore.t.sol
index 5fd882359..93862d27f 100644
--- a/test-foundry/libraries/vault/MasterVaultCore.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultCore.t.sol
@@ -40,6 +40,5 @@ contract MasterVaultCoreTest is Test {
         vault = MasterVault(proxyAddress);
 
         vault.initialize(IERC20(address(token)), name, symbol, address(this));
-        vault.unpause();
     }
 }
diff --git a/test-foundry/libraries/vault/MasterVaultFee.t.sol b/test-foundry/libraries/vault/MasterVaultFee.t.sol
index 768e661e0..14a56c0b6 100644
--- a/test-foundry/libraries/vault/MasterVaultFee.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultFee.t.sol
@@ -251,7 +251,7 @@ contract MasterVaultFeeTestWithSubvaultFresh is MasterVaultFeeTest {
     function setUp() public override {
         super.setUp();
         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
-        vault.setSubVault(IERC4626(address(_subvault)), 0);
+        vault.setSubVault(IERC4626(address(_subvault)));
     }
 }
 
@@ -275,6 +275,6 @@ contract MasterVaultFeeTestWithSubvaultHoldingAssets is MasterVaultFeeTest {
             "subvault should be initiated with shares = _initAmount"
         );
 
-        vault.setSubVault(IERC4626(address(_subvault)), 0);
+        vault.setSubVault(IERC4626(address(_subvault)));
     }
 }

From a3376869f774b3e85f14bef5841f21cba29d68e1 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Fri, 12 Dec 2025 11:28:45 -0500
Subject: [PATCH 02/27] wip: conversion and deposit/withdraw functions look
 okay

---
 .../libraries/vault/MasterVault.sol           | 92 ++++++++++++++-----
 1 file changed, 69 insertions(+), 23 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index b014a9de0..ce0cfa3ca 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -197,6 +197,58 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         return _assetsToSubVaultShares(profitAssets.mulDiv(targetAllocationWad, 1e18, rounding), rounding);
     }
 
+    /** @dev See {IERC4626-deposit}. */
+    function deposit(uint256 assets, address receiver) public virtual override returns (uint256) {
+        require(assets <= maxDeposit(receiver), "ERC4626: deposit more than max");
+
+        (uint256 shares, uint256 assetsFromSubVault) = _convertToSharesDetailed(assets, MathUpgradeable.Rounding.Down);
+        _deposit(_msgSender(), receiver, assets, shares, assetsFromSubVault);
+
+        return shares;
+    }
+
+    /** @dev See {IERC4626-mint}.
+     *
+     * As opposed to {deposit}, minting is allowed even if the vault is in a state where the price of a share is zero.
+     * In this case, the shares will be minted without requiring any assets to be deposited.
+     */
+    function mint(uint256 shares, address receiver) public virtual override returns (uint256) {
+        require(shares <= maxMint(receiver), "ERC4626: mint more than max");
+
+        (uint256 assets, uint256 assetsFromSubVault) = _convertToAssetsDetailed(shares, MathUpgradeable.Rounding.Up);
+        _deposit(_msgSender(), receiver, assets, shares, assetsFromSubVault);
+
+        return assets;
+    }
+
+    /** @dev See {IERC4626-withdraw}. */
+    function withdraw(
+        uint256 assets,
+        address receiver,
+        address owner
+    ) public virtual override returns (uint256) {
+        require(assets <= maxWithdraw(owner), "ERC4626: withdraw more than max");
+
+        (uint256 shares, uint256 assetsFromSubVault) = _convertToSharesDetailed(assets, MathUpgradeable.Rounding.Up);
+        _withdraw(_msgSender(), receiver, owner, assets, shares, assetsFromSubVault);
+
+        return shares;
+    }
+
+    /** @dev See {IERC4626-redeem}. */
+    function redeem(
+        uint256 shares,
+        address receiver,
+        address owner
+    ) public virtual override returns (uint256) {
+        require(shares <= maxRedeem(owner), "ERC4626: redeem more than max");
+
+        (uint256 assets, uint256 assetsFromSubVault) = _convertToAssetsDetailed(shares, MathUpgradeable.Rounding.Down);
+        _withdraw(_msgSender(), receiver, owner, assets, shares, assetsFromSubVault);
+
+        return assets;
+    }
+
     /**
      * @dev Deposit/mint common workflow.
      */
@@ -220,16 +272,12 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address receiver,
         address _owner,
         uint256 assets,
-        uint256 shares
-    ) internal virtual override whenNotPaused {
+        uint256 shares,
+        uint256 assetsToWithdraw
+    ) internal whenNotPaused {
         if (enablePerformanceFee) totalPrincipal -= assets;
-
-        IERC4626 _subVault = subVault;
-        if (address(_subVault) != address(0)) {
-            _subVault.withdraw(assets, address(this), address(this));
-        }
-
-        super._withdraw(caller, receiver, _owner, assets, shares);
+        subVault.withdraw(assetsToWithdraw, address(this), address(this));
+        _withdraw(caller, receiver, _owner, assets, shares);
     }
 
     function _totalAssets(MathUpgradeable.Rounding rounding) internal view returns (uint256) {
@@ -244,10 +292,10 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * would represent an infinite amount of shares.
      */
     function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) {
-        (,,shares) = _convertToShares2(assets, rounding);
+        (shares,) = _convertToSharesDetailed(assets, rounding);
     }
 
-    function _convertToShares2(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 sharesFromIdle, uint256 sharesFromSubVault, uint256 sharesFromBoth) {
+    function _convertToSharesDetailed(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 shares, uint256 assetsForSubVault) {
         uint256 supply = totalSupply();
 
         uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
@@ -263,14 +311,14 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         // figure out how much assets should be deposited to subvault vs kept idle
         // same rounding direction since they are used in the numerators of the final calculation
         uint256 assetsForIdle = assets.mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
-        uint256 assetsForSubVault = assets.mulDiv(targetAllocationWad, 1e18, rounding);
+        assetsForSubVault = assets.mulDiv(targetAllocationWad, 1e18, rounding);
 
         // figure out how many shares would be issued according to each portion
-        sharesFromIdle = assetsForIdle.mulDiv(supply, totalIdle, rounding);
-        sharesFromSubVault = _assetsToSubVaultShares(assetsForSubVault, rounding).mulDiv(supply, totalSubShares, rounding);
+        uint256 sharesFromIdle = assetsForIdle.mulDiv(supply, totalIdle, rounding);
+        uint256 sharesFromSubVault = _assetsToSubVaultShares(assetsForSubVault, rounding).mulDiv(supply, totalSubShares, rounding);
 
         // take the min if rounding down, max if rounding up
-        sharesFromBoth = rounding == MathUpgradeable.Rounding.Down
+        shares = rounding == MathUpgradeable.Rounding.Down
             ? MathUpgradeable.min(sharesFromIdle, sharesFromSubVault)
             : MathUpgradeable.max(sharesFromIdle, sharesFromSubVault);
     }
@@ -279,10 +327,10 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * @dev Internal conversion function (from shares to assets) with support for rounding direction.
      */
     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) {
-        (,, assets) = _convertToAssets2(shares, rounding);
+        (assets,) = _convertToAssetsDetailed(shares, rounding);
     }
 
-    function _convertToAssets2(uint256 shares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assetsFromIdle, uint256 assetsFromSubVault, uint256 assetsFromBoth) {
+    function _convertToAssetsDetailed(uint256 shares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assets, uint256 assetsFromSubVault) {
         uint256 supply = totalSupply();
 
         uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
@@ -301,13 +349,11 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         uint256 sharesForSubVault = shares.mulDiv(targetAllocationWad, 1e18, rounding);
 
         // figure out how much assets would be received according to each portion
-        assetsFromIdle = sharesForIdle.mulDiv(totalIdle, supply, rounding);
+        uint256 assetsFromIdle = sharesForIdle.mulDiv(totalIdle, supply, rounding);
         assetsFromSubVault = _subVaultSharesToAssets(sharesForSubVault.mulDiv(totalSubShares, supply, rounding), rounding);
-        
-        // take the min if rounding down, max if rounding up
-        assetsFromBoth = rounding == MathUpgradeable.Rounding.Down
-            ? MathUpgradeable.min(assetsFromIdle, assetsFromSubVault)
-            : MathUpgradeable.max(assetsFromIdle, assetsFromSubVault);
+
+        // total it up
+        assets = assetsFromIdle + assetsFromSubVault;
     }
 
     function _assetsToSubVaultShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 subShares) {

From 545d96dfd0f81a1308239aadabfcc24454fe7fa9 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Fri, 12 Dec 2025 14:08:43 -0500
Subject: [PATCH 03/27] settargetAllocationWad

---
 .../libraries/vault/MasterVault.sol           | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index ce0cfa3ca..3efb84f2b 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -50,7 +50,7 @@ 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;
 
-    uint256 targetAllocationWad;
+    uint256 public targetAllocationWad;
 
     /// @notice Flag indicating if performance fee is enabled
     bool public enablePerformanceFee;
@@ -122,6 +122,26 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
+    function setTargetAllocationWad(uint256 _targetAllocationWad) external onlyRole(VAULT_MANAGER_ROLE) {
+        require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
+        
+        int256 allocationDelta = int256(_targetAllocationWad) - int256(targetAllocationWad);
+        require(allocationDelta != 0, "Allocation unchanged");
+
+        int256 idleDelta = int256(totalAssets()) * allocationDelta / 1e18;
+
+        if (idleDelta > 0) {
+            // move assets into subvault
+            subVault.deposit(uint256(idleDelta), address(this));
+        }
+        else if (idleDelta < 0) {
+            // move assets out of subvault
+            subVault.withdraw(uint256(-idleDelta), address(this), address(this));
+        }
+
+        targetAllocationWad = _targetAllocationWad;
+    }
+
     /// @notice Toggle performance fee collection on/off
     /// @param enabled True to enable performance fees, false to disable
     function setPerformanceFee(bool enabled) external onlyRole(VAULT_MANAGER_ROLE) {

From ec195223ce122c104500c0bcceed719cfa772028 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Fri, 12 Dec 2025 14:34:06 -0500
Subject: [PATCH 04/27] deploy with initial vault

---
 .../libraries/vault/MasterVault.sol           |  9 ++---
 .../libraries/vault/MasterVaultFactory.sol    | 39 ++++++++++++-------
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 3efb84f2b..9478791b1 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -67,12 +67,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary);
     event PerformanceFeesWithdrawn(address indexed beneficiary, uint256 amount);
 
-    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();
-
+    function initialize(IERC4626 _subVault, string memory _name, string memory _symbol, address _owner) external initializer {
         __ERC20_init(_name, _symbol);
-        __ERC4626_init(IERC20Upgradeable(address(_asset)));
+        __ERC4626_init(IERC20Upgradeable(_subVault.asset()));
         __AccessControl_init();
         __Pausable_init();
 
@@ -83,7 +80,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         _grantRole(VAULT_MANAGER_ROLE, _owner);
         _grantRole(PAUSER_ROLE, _owner);
 
-        // todo: deploy initial subvault
+        subVault = _subVault;
     }
 
     function distributePerformanceFee() external whenNotPaused {
diff --git a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol
index 0259c84d4..87e63c1ae 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol
@@ -2,14 +2,19 @@
 
 pragma solidity ^0.8.0;
 
-import "@openzeppelin/contracts/utils/Create2.sol";
 import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
+import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
 import "../ClonableBeaconProxy.sol";
 import "./IMasterVault.sol";
 import "./IMasterVaultFactory.sol";
 import "./MasterVault.sol";
 
+contract DefaultSubVault is ERC4626 {
+    constructor(address token) ERC4626(IERC20(token)) ERC20("Default SubVault", "DSV") {}
+}
+
+// todo: slim down this contract
 contract MasterVaultFactory is IMasterVaultFactory, Initializable {
     error ZeroAddress();
     error BeaconNotDeployed();
@@ -27,23 +32,13 @@ contract MasterVaultFactory is IMasterVaultFactory, Initializable {
     }
 
     function deployVault(address token) public returns (address vault) {
-        if (token == address(0)) {
-            revert ZeroAddress();
-        }
-        if (
-            address(beaconProxyFactory) == address(0) && beaconProxyFactory.beacon() == 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()));
+        string memory name = string(abi.encodePacked("Master ", _tryGetTokenName(token)));
+        string memory symbol = string(abi.encodePacked("m", _tryGetTokenSymbol(token)));
 
-        MasterVault(vault).initialize(IERC20(token), name, symbol, owner);
+        MasterVault(vault).initialize(new DefaultSubVault(token), name, symbol, owner);
 
         emit VaultDeployed(token, vault);
     }
@@ -64,4 +59,20 @@ contract MasterVaultFactory is IMasterVaultFactory, Initializable {
         }
         return vault;
     }
+
+    function _tryGetTokenName(address token) internal view returns (string memory) {
+        try IERC20Metadata(token).name() returns (string memory name) {
+            return name;
+        } catch {
+            return "";
+        }
+    }
+
+    function _tryGetTokenSymbol(address token) internal view returns (string memory) {
+        try IERC20Metadata(token).symbol() returns (string memory symbol) {
+            return symbol;
+        } catch {
+            return "";
+        }
+    }
 }

From 69345c1ca7d63a34f8ed211c89d4496d390ce8da Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Fri, 12 Dec 2025 14:38:50 -0500
Subject: [PATCH 05/27] use factory in core test setup

---
 .../libraries/vault/MasterVaultCore.t.sol     | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/test-foundry/libraries/vault/MasterVaultCore.t.sol b/test-foundry/libraries/vault/MasterVaultCore.t.sol
index 93862d27f..dea2ff62e 100644
--- a/test-foundry/libraries/vault/MasterVaultCore.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultCore.t.sol
@@ -3,6 +3,7 @@ pragma solidity ^0.8.0;
 
 import { Test } from "forge-std/Test.sol";
 import { MasterVault } from "../../../contracts/tokenbridge/libraries/vault/MasterVault.sol";
+import { MasterVaultFactory } from "../../../contracts/tokenbridge/libraries/vault/MasterVaultFactory.sol";
 import { TestERC20 } from "../../../contracts/tokenbridge/test/TestERC20.sol";
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { UpgradeableBeacon } from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
@@ -13,10 +14,9 @@ import {
 import { IAccessControl } from "@openzeppelin/contracts/access/IAccessControl.sol";
 
 contract MasterVaultCoreTest is Test {
+    MasterVaultFactory public factory;
     MasterVault public vault;
     TestERC20 public token;
-    UpgradeableBeacon public beacon;
-    BeaconProxyFactory public beaconProxyFactory;
 
     address public user = address(0x1);
     string public name = "Master Test Token";
@@ -27,18 +27,9 @@ contract MasterVaultCoreTest is Test {
     }
 
     function setUp() public virtual {
+        factory = new MasterVaultFactory();
+        factory.initialize(address(this));
         token = new TestERC20();
-
-        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.initialize(IERC20(address(token)), name, symbol, address(this));
+        vault = MasterVault(factory.deployVault(address(token)));
     }
 }

From 05fe9e4446cf67cb9ed2da6cb346aad8d4dadeb6 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Mon, 15 Dec 2025 10:05:11 -0500
Subject: [PATCH 06/27] slippage tolerance on setTargetAllocationWad

---
 .../tokenbridge/libraries/vault/MasterVault.sol  | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 9478791b1..880d76fdb 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -38,9 +38,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
 
     error SubVaultAlreadySet();
     error SubVaultAssetMismatch();
-    error SubVaultExchangeRateTooLow();
     error NoExistingSubVault();
-    error NewSubVaultExchangeRateTooLow();
+    error SubVaultExchangeRateTooLow(int256 required, int256 actual);
     error PerformanceFeeDisabled();
     error BeneficiaryNotSet();
     error InvalidAsset();
@@ -119,21 +118,28 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
-    function setTargetAllocationWad(uint256 _targetAllocationWad) external onlyRole(VAULT_MANAGER_ROLE) {
+    function setTargetAllocationWad(uint256 _targetAllocationWad, int256 minSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) {
         require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
         
         int256 allocationDelta = int256(_targetAllocationWad) - int256(targetAllocationWad);
         require(allocationDelta != 0, "Allocation unchanged");
 
         int256 idleDelta = int256(totalAssets()) * allocationDelta / 1e18;
+        int256 subVaultExchRateWad;
 
         if (idleDelta > 0) {
             // move assets into subvault
-            subVault.deposit(uint256(idleDelta), address(this));
+            uint256 shares = subVault.deposit(uint256(idleDelta), address(this));
+            subVaultExchRateWad = int256(uint256(idleDelta).mulDiv(1e18, shares, MathUpgradeable.Rounding.Down));
         }
         else if (idleDelta < 0) {
             // move assets out of subvault
-            subVault.withdraw(uint256(-idleDelta), address(this), address(this));
+            uint256 shares = subVault.withdraw(uint256(-idleDelta), address(this), address(this));
+            subVaultExchRateWad = int256(uint256(-idleDelta).mulDiv(1e18, shares, MathUpgradeable.Rounding.Up));
+        }
+
+        if (subVaultExchRateWad < minSubVaultExchRateWad) {
+            revert SubVaultExchangeRateTooLow(minSubVaultExchRateWad, subVaultExchRateWad);
         }
 
         targetAllocationWad = _targetAllocationWad;

From 3fa9992de7a998f66187ba13294e9acd9c0a43bb Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Mon, 15 Dec 2025 16:05:34 -0500
Subject: [PATCH 07/27] big simplify

---
 .../libraries/vault/MasterVault.sol           | 210 +++++-------------
 1 file changed, 52 insertions(+), 158 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 880d76fdb..b7a5a4e91 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -91,12 +91,19 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         uint256 profit = totalProfit(MathUpgradeable.Rounding.Down);
         if (profit == 0) return;
 
-        if (address(subVault) != address(0)) {
-            subVault.redeem(totalProfitInSubVaultShares(MathUpgradeable.Rounding.Down), beneficiary, address(this));
-        } else {
-            IERC20(asset()).safeTransfer(beneficiary, profit);
+        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
+        if (totalIdle > 0) {
+            uint256 amountToTransfer = profit <= totalIdle ? profit : totalIdle;
+            IERC20(asset()).safeTransfer(beneficiary, amountToTransfer);
+            profit -= amountToTransfer;
+        }
+
+        if (profit > 0) {
+            subVault.withdraw(profit, beneficiary, address(this));
         }
 
+        rebalance();
+
         emit PerformanceFeesWithdrawn(beneficiary, profit);
     }
 
@@ -118,30 +125,33 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
-    function setTargetAllocationWad(uint256 _targetAllocationWad, int256 minSubVaultExchRateWad) external onlyRole(VAULT_MANAGER_ROLE) {
-        require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
+    function rebalance() public {
+        // todo: handle 0 and 100 special cases if needed
+        uint256 totalAssetsUp = _totalAssets(MathUpgradeable.Rounding.Up);
+        uint256 totalAssetsDown = _totalAssets(MathUpgradeable.Rounding.Down);
+        uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
+        uint256 idleTargetDown = totalAssetsDown.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Down);
+        uint256 idleBalance = IERC20(asset()).balanceOf(address(this));
         
-        int256 allocationDelta = int256(_targetAllocationWad) - int256(targetAllocationWad);
-        require(allocationDelta != 0, "Allocation unchanged");
-
-        int256 idleDelta = int256(totalAssets()) * allocationDelta / 1e18;
-        int256 subVaultExchRateWad;
-
-        if (idleDelta > 0) {
-            // move assets into subvault
-            uint256 shares = subVault.deposit(uint256(idleDelta), address(this));
-            subVaultExchRateWad = int256(uint256(idleDelta).mulDiv(1e18, shares, MathUpgradeable.Rounding.Down));
-        }
-        else if (idleDelta < 0) {
-            // move assets out of subvault
-            uint256 shares = subVault.withdraw(uint256(-idleDelta), address(this), address(this));
-            subVaultExchRateWad = int256(uint256(-idleDelta).mulDiv(1e18, shares, MathUpgradeable.Rounding.Up));
+        if (idleTargetDown <= idleBalance && idleBalance <= idleTargetUp) {
+            return;
         }
 
-        if (subVaultExchRateWad < minSubVaultExchRateWad) {
-            revert SubVaultExchangeRateTooLow(minSubVaultExchRateWad, subVaultExchRateWad);
+        if (idleBalance < idleTargetDown) {
+            // we need to withdraw from subvault
+            uint256 assetsToWithdraw = idleTargetDown - idleBalance;
+            subVault.withdraw(assetsToWithdraw, address(this), address(this));
         }
+        else {
+            // we need to deposit into subvault
+            uint256 assetsToDeposit = idleBalance - idleTargetUp;
+            subVault.deposit(assetsToDeposit, address(this));
+        }
+    }
 
+    function setTargetAllocationWad(uint256 _targetAllocationWad) external onlyRole(VAULT_MANAGER_ROLE) {
+        require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
+        require(targetAllocationWad != _targetAllocationWad, "Allocation unchanged");
         targetAllocationWad = _targetAllocationWad;
     }
 
@@ -208,70 +218,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         return __totalAssets > totalPrincipal ? __totalAssets - totalPrincipal : 0;
     }
 
-    function totalProfitInIdleAssets(MathUpgradeable.Rounding rounding) public view returns (uint256) {
-        return totalProfit(rounding).mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
-    }
-
-    function totalProfitInSubVaultShares(MathUpgradeable.Rounding rounding) public view returns (uint256) {
-        uint256 profitAssets = totalProfit(rounding);
-        if (profitAssets == 0) {
-            return 0;
-        }
-        return _assetsToSubVaultShares(profitAssets.mulDiv(targetAllocationWad, 1e18, rounding), rounding);
-    }
-
-    /** @dev See {IERC4626-deposit}. */
-    function deposit(uint256 assets, address receiver) public virtual override returns (uint256) {
-        require(assets <= maxDeposit(receiver), "ERC4626: deposit more than max");
-
-        (uint256 shares, uint256 assetsFromSubVault) = _convertToSharesDetailed(assets, MathUpgradeable.Rounding.Down);
-        _deposit(_msgSender(), receiver, assets, shares, assetsFromSubVault);
-
-        return shares;
-    }
-
-    /** @dev See {IERC4626-mint}.
-     *
-     * As opposed to {deposit}, minting is allowed even if the vault is in a state where the price of a share is zero.
-     * In this case, the shares will be minted without requiring any assets to be deposited.
-     */
-    function mint(uint256 shares, address receiver) public virtual override returns (uint256) {
-        require(shares <= maxMint(receiver), "ERC4626: mint more than max");
-
-        (uint256 assets, uint256 assetsFromSubVault) = _convertToAssetsDetailed(shares, MathUpgradeable.Rounding.Up);
-        _deposit(_msgSender(), receiver, assets, shares, assetsFromSubVault);
-
-        return assets;
-    }
-
-    /** @dev See {IERC4626-withdraw}. */
-    function withdraw(
-        uint256 assets,
-        address receiver,
-        address owner
-    ) public virtual override returns (uint256) {
-        require(assets <= maxWithdraw(owner), "ERC4626: withdraw more than max");
-
-        (uint256 shares, uint256 assetsFromSubVault) = _convertToSharesDetailed(assets, MathUpgradeable.Rounding.Up);
-        _withdraw(_msgSender(), receiver, owner, assets, shares, assetsFromSubVault);
-
-        return shares;
-    }
-
-    /** @dev See {IERC4626-redeem}. */
-    function redeem(
-        uint256 shares,
-        address receiver,
-        address owner
-    ) public virtual override returns (uint256) {
-        require(shares <= maxRedeem(owner), "ERC4626: redeem more than max");
-
-        (uint256 assets, uint256 assetsFromSubVault) = _convertToAssetsDetailed(shares, MathUpgradeable.Rounding.Down);
-        _withdraw(_msgSender(), receiver, owner, assets, shares, assetsFromSubVault);
-
-        return assets;
-    }
-
     /**
      * @dev Deposit/mint common workflow.
      */
@@ -279,12 +225,11 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address caller,
         address receiver,
         uint256 assets,
-        uint256 shares,
-        uint256 assetsToDeposit
-    ) internal whenNotPaused {
-        _deposit(caller, receiver, assets, shares);
+        uint256 shares
+    ) internal override whenNotPaused {
+        super._deposit(caller, receiver, assets, shares);
         if (enablePerformanceFee) totalPrincipal += assets;
-        subVault.deposit(assetsToDeposit, address(this));
+        rebalance();
     }
 
     /**
@@ -295,19 +240,22 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address receiver,
         address _owner,
         uint256 assets,
-        uint256 shares,
-        uint256 assetsToWithdraw
-    ) internal whenNotPaused {
+        uint256 shares
+    ) internal override whenNotPaused {
         if (enablePerformanceFee) totalPrincipal -= assets;
-        subVault.withdraw(assetsToWithdraw, address(this), address(this));
-        _withdraw(caller, receiver, _owner, assets, shares);
+        uint256 idleAssets = IERC20(asset()).balanceOf(address(this));
+        if (idleAssets < assets) {
+            uint256 assetsToWithdraw = assets - idleAssets;
+            subVault.withdraw(assetsToWithdraw, address(this), address(this));
+        }
+        super._withdraw(caller, receiver, _owner, assets, shares);
+        rebalance();
     }
 
     function _totalAssets(MathUpgradeable.Rounding rounding) internal view returns (uint256) {
         return IERC20(asset()).balanceOf(address(this)) + _subVaultSharesToAssets(subVault.balanceOf(address(this)), rounding);
     }
 
-    // todo: question: will this drift over time? i don't think so but worth checking and testing for
     /**
      * @dev Internal conversion function (from assets to shares) with support for rounding direction.
      *
@@ -315,82 +263,28 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * would represent an infinite amount of shares.
      */
     function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) {
-        (shares,) = _convertToSharesDetailed(assets, rounding);
-    }
-
-    function _convertToSharesDetailed(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 shares, uint256 assetsForSubVault) {
-        uint256 supply = totalSupply();
-
-        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
-        uint256 totalSubShares = subVault.balanceOf(address(this));
-
+        uint256 __totalAssets = _totalAssets(_flipRounding(rounding));
         if (enablePerformanceFee) {
-            // since we use totalSubShares and totalIdle in the denominators of the final calculation,
-            // and we are subtracting profit from it, we should use the same rounding direction for profit
-            totalSubShares -= totalProfitInSubVaultShares(_flipRounding(rounding));
-            totalIdle -= totalProfitInIdleAssets(_flipRounding(rounding));
+            __totalAssets -= totalProfit(rounding);
         }
-
-        // figure out how much assets should be deposited to subvault vs kept idle
-        // same rounding direction since they are used in the numerators of the final calculation
-        uint256 assetsForIdle = assets.mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
-        assetsForSubVault = assets.mulDiv(targetAllocationWad, 1e18, rounding);
-
-        // figure out how many shares would be issued according to each portion
-        uint256 sharesFromIdle = assetsForIdle.mulDiv(supply, totalIdle, rounding);
-        uint256 sharesFromSubVault = _assetsToSubVaultShares(assetsForSubVault, rounding).mulDiv(supply, totalSubShares, rounding);
-
-        // take the min if rounding down, max if rounding up
-        shares = rounding == MathUpgradeable.Rounding.Down
-            ? MathUpgradeable.min(sharesFromIdle, sharesFromSubVault)
-            : MathUpgradeable.max(sharesFromIdle, sharesFromSubVault);
+        return assets.mulDiv(totalSupply(), __totalAssets, rounding);
     }
 
     /**
      * @dev Internal conversion function (from shares to assets) with support for rounding direction.
      */
     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) {
-        (assets,) = _convertToAssetsDetailed(shares, rounding);
-    }
-
-    function _convertToAssetsDetailed(uint256 shares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assets, uint256 assetsFromSubVault) {
-        uint256 supply = totalSupply();
-
-        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
-        uint256 totalSubShares = subVault.balanceOf(address(this));
-
+        uint256 __totalAssets = _totalAssets(rounding);
         if (enablePerformanceFee) {
-            // since we use totalSubShares and totalIdle in the numerators of the final calculation,
-            // and we are subtracting profit from it, we should use the opposite rounding direction for profit
-            totalSubShares -= totalProfitInSubVaultShares(_flipRounding(rounding));
-            totalIdle -= totalProfitInIdleAssets(_flipRounding(rounding));
+            __totalAssets -= totalProfit(_flipRounding(rounding));
         }
-
-        // figure out how many shares should be burned for subvault shares vs idle
-        // same rounding direction since they are used in the numerators of the final calculation (todo: confirm rounding direction)
-        uint256 sharesForIdle = shares.mulDiv(1e18 - targetAllocationWad, 1e18, rounding);
-        uint256 sharesForSubVault = shares.mulDiv(targetAllocationWad, 1e18, rounding);
-
-        // figure out how much assets would be received according to each portion
-        uint256 assetsFromIdle = sharesForIdle.mulDiv(totalIdle, supply, rounding);
-        assetsFromSubVault = _subVaultSharesToAssets(sharesForSubVault.mulDiv(totalSubShares, supply, rounding), rounding);
-
-        // total it up
-        assets = assetsFromIdle + assetsFromSubVault;
-    }
-
-    function _assetsToSubVaultShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view returns (uint256 subShares) {
-        return rounding == MathUpgradeable.Rounding.Up ? subVault.previewWithdraw(assets) : subVault.previewDeposit(assets);
+        return shares.mulDiv(__totalAssets, totalSupply(), rounding);
     }
 
     function _subVaultSharesToAssets(uint256 subShares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assets) {
         return rounding == MathUpgradeable.Rounding.Up ? subVault.previewMint(subShares) : subVault.previewRedeem(subShares);
     }
 
-    function _min(uint256 a, uint256 b) internal pure returns (uint256) {
-        return a <= b ? a : b;
-    }
-
     function _flipRounding(MathUpgradeable.Rounding rounding) internal pure returns (MathUpgradeable.Rounding) {
         return rounding == MathUpgradeable.Rounding.Up ? MathUpgradeable.Rounding.Down : MathUpgradeable.Rounding.Up;
     }

From 948036067b16970737d2116318f878baee2a675f Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Mon, 15 Dec 2025 16:32:43 -0500
Subject: [PATCH 08/27] WIP: mastervault donation attack mitigation (#142)

* wip: dead shares and fix div by zero

* doc

* add one
---
 .../libraries/vault/MasterVault.sol           |  27 +++-
 .../libraries/vault/MasterVault.t.sol         | 121 ++++++++----------
 .../libraries/vault/MasterVaultAttack.t.sol   |   4 +-
 .../libraries/vault/MasterVaultCore.t.sol     |   2 +-
 4 files changed, 82 insertions(+), 72 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index b7a5a4e91..40d455975 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -36,6 +36,11 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     /// @notice Pauser role can pause/unpause deposits and withdrawals (todo: pause should pause EVERYTHING)
     bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
 
+    /// @notice Extra decimals added to the ERC20 decimals of the underlying asset to determine the decimals of the MasterVault
+    /// @dev    This is done to mitigate the "first depositor" problem described in the OpenZeppelin ERC4626 documentation.
+    ///         See https://docs.openzeppelin.com/contracts/5.x/erc4626 for more details on the mitigation.
+    uint8 public constant EXTRA_DECIMALS = 18;
+
     error SubVaultAlreadySet();
     error SubVaultAssetMismatch();
     error NoExistingSubVault();
@@ -69,6 +74,10 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     function initialize(IERC4626 _subVault, string memory _name, string memory _symbol, address _owner) external initializer {
         __ERC20_init(_name, _symbol);
         __ERC4626_init(IERC20Upgradeable(_subVault.asset()));
+
+        // call decimals() to ensure underlying has reasonable decimals and we won't have overflow
+        decimals();
+
         __AccessControl_init();
         __Pausable_init();
 
@@ -79,8 +88,18 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         _grantRole(VAULT_MANAGER_ROLE, _owner);
         _grantRole(PAUSER_ROLE, _owner);
 
+        // mint some dead shares to avoid first depositor issues
+        // for more information on the mitigation: 
+        // https://web.archive.org/web/20250609034056/https://docs.openzeppelin.com/contracts/4.x/erc4626#fees
+        _mint(address(1), 10 ** EXTRA_DECIMALS);
+
         subVault = _subVault;
     }
+    
+    /// @dev Overridden to add EXTRA_DECIMALS to the underlying asset decimals
+    function decimals() public view override returns (uint8) {
+        return super.decimals() + EXTRA_DECIMALS;
+    }
 
     function distributePerformanceFee() external whenNotPaused {
         if (!enablePerformanceFee) revert PerformanceFeeDisabled();
@@ -263,7 +282,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * would represent an infinite amount of shares.
      */
     function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) {
-        uint256 __totalAssets = _totalAssets(_flipRounding(rounding));
+        // we add one as part of the first deposit mitigation
+        // see for details: https://docs.openzeppelin.com/contracts/5.x/erc4626
+        uint256 __totalAssets = _totalAssets(_flipRounding(rounding)) + 1;
         if (enablePerformanceFee) {
             __totalAssets -= totalProfit(rounding);
         }
@@ -274,7 +295,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
      * @dev Internal conversion function (from shares to assets) with support for rounding direction.
      */
     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) {
-        uint256 __totalAssets = _totalAssets(rounding);
+        // we add one as part of the first deposit mitigation
+        // see for details: https://docs.openzeppelin.com/contracts/5.x/erc4626
+        uint256 __totalAssets = _totalAssets(rounding) + 1;
         if (enablePerformanceFee) {
             __totalAssets -= totalProfit(_flipRounding(rounding));
         }
diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol
index 9d545c521..403762ffd 100644
--- a/test-foundry/libraries/vault/MasterVault.t.sol
+++ b/test-foundry/libraries/vault/MasterVault.t.sol
@@ -6,69 +6,56 @@ import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.s
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { IERC4626 } from "@openzeppelin/contracts/interfaces/IERC4626.sol";
 
-contract MasterVaultTest is MasterVaultCoreTest {
+contract MasterVaultFirstDepositTest is MasterVaultCoreTest {
     // first deposit
     function test_deposit() public {
-        address _assetsHoldingVault = address(vault.subVault()) == address(0)
-            ? address(vault)
-            : address(vault.subVault());
-        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(_assetsHoldingVault);
+        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(address(vault));
 
         vm.startPrank(user);
         token.mint();
+
         uint256 depositAmount = 100;
+        uint256 expectedShares = depositAmount * 10**vault.EXTRA_DECIMALS();
+        uint256 deadShares = 10**vault.EXTRA_DECIMALS();
 
         token.approve(address(vault), depositAmount);
 
         uint256 shares = vault.deposit(depositAmount, user);
 
-        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(_assetsHoldingVault);
+        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(address(vault));
         uint256 diff = _assetsHoldingVaultBalanceAfter - _assetsHoldingVaultBalanceBefore;
 
-        assertEq(vault.balanceOf(user), shares, "User should receive shares");
+        assertEq(shares, expectedShares, "Shares minted should equal deposit amount at a rate of 1^extra_decimals");
+        assertEq(vault.balanceOf(user), expectedShares, "User should receive shares");
         assertEq(vault.totalAssets(), depositAmount, "Vault should hold deposited assets");
-        assertEq(vault.totalSupply(), shares, "Total supply should equal shares minted");
-
+        assertEq(vault.totalSupply(), shares + deadShares, "Total supply should equal shares minted plus dead shares");
         assertEq(diff, depositAmount, "Vault should increase holding of assets");
-        assertGt(token.balanceOf(_assetsHoldingVault), 0, "Vault should hold the tokens");
-
-        assertEq(vault.totalSupply(), diff, "First deposit should be at a rate of 1");
-
         vm.stopPrank();
     }
 
     // first mint
     function test_mint() public {
-        address _assetsHoldingVault = address(vault.subVault()) == address(0)
-            ? address(vault)
-            : address(vault.subVault());
-
-        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(_assetsHoldingVault);
+        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(address(vault));
 
         vm.startPrank(user);
         token.mint();
-        uint256 sharesToMint = 100;
-
-        token.approve(address(vault), type(uint256).max);
 
-        //   assertEq(1, vault.totalAssets(), "First mint should be at a rate of 1"); // 0
-        // assertEq(1, vault.totalSupply(), "First mint should be at a rate of 1");   // 0
+        uint256 sharesToMint = 100 * 10**vault.EXTRA_DECIMALS();
+        uint256 deadShares = 10**vault.EXTRA_DECIMALS();
 
+        token.approve(address(vault), type(uint256).max);
 
         uint256 assetsCost = vault.mint(sharesToMint, user);
+        uint256 expectedAssetsCost = 100;
 
-        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(_assetsHoldingVault);
-
-        assertEq(vault.balanceOf(user), sharesToMint, "User should receive requested shares");
-        assertEq(vault.totalSupply(), sharesToMint, "Total supply should equal shares minted");
-        assertEq(vault.totalAssets(), assetsCost, "Vault should hold the assets deposited");
-        assertEq(
-            _assetsHoldingVaultBalanceAfter - _assetsHoldingVaultBalanceBefore,
-            assetsCost,
-            "Vault should hold the tokens"
-        );
+        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(address(vault));
+        uint256 diff = _assetsHoldingVaultBalanceAfter - _assetsHoldingVaultBalanceBefore;
 
-        assertEq(vault.totalSupply(), vault.totalAssets(), "First mint should be at a rate of 1");
+        assertEq(assetsCost, expectedAssetsCost, "Assets spent should equal mint amount at a rate of 1^extra_decimals");
+        assertEq(vault.balanceOf(user), sharesToMint, "User should receive shares");
+        assertEq(vault.totalAssets(), expectedAssetsCost, "Vault should hold deposited assets");
+        assertEq(vault.totalSupply(), sharesToMint + deadShares, "Total supply should equal shares minted plus dead shares");
+        assertEq(diff, expectedAssetsCost, "Vault should increase holding of assets");
         vm.stopPrank();
     }
 
@@ -87,7 +74,7 @@ contract MasterVaultTest is MasterVaultCoreTest {
         assertEq(vault.balanceOf(user), 0, "User should have no shares left");
         assertEq(token.balanceOf(user), depositAmount, "User should receive all withdrawn tokens");
         assertEq(vault.totalAssets(), 0, "Vault should have no assets left");
-        assertEq(vault.totalSupply(), 0, "Total supply should be zero");
+        assertEq(vault.totalSupply(), 10**vault.EXTRA_DECIMALS(), "Total supply should be only dead shares");
         assertEq(token.balanceOf(address(vault)), 0, "Vault should have no tokens left");
         assertEq(sharesRedeemed, userSharesBefore, "All shares should be redeemed");
 
@@ -108,7 +95,7 @@ contract MasterVaultTest is MasterVaultCoreTest {
         assertEq(vault.balanceOf(user), 0, "User should have no shares left");
         assertEq(token.balanceOf(user), depositAmount, "User should receive all assets back");
         assertEq(vault.totalAssets(), 0, "Vault should have no assets left");
-        assertEq(vault.totalSupply(), 0, "Total supply should be zero");
+        assertEq(vault.totalSupply(), 10**vault.EXTRA_DECIMALS(), "Total supply should be only dead shares");
         assertEq(token.balanceOf(address(vault)), 0, "Vault should have no tokens left");
         assertEq(assetsReceived, depositAmount, "All assets should be received");
 
@@ -116,34 +103,34 @@ contract MasterVaultTest is MasterVaultCoreTest {
     }
 }
 
-contract MasterVaultTestWithSubvaultFresh is MasterVaultTest {
-    function setUp() public override {
-        super.setUp();
-        MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
-        vault.setSubVault(IERC4626(address(_subvault)));
-    }
-}
-
-contract MasterVaultTestWithSubvaultHoldingAssets is MasterVaultTest {
-    function setUp() public override {
-        super.setUp();
-
-        MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
-        uint256 _initAmount = 97659743;
-        token.mint(_initAmount);
-        token.approve(address(_subvault), _initAmount);
-        _subvault.deposit(_initAmount, address(this));
-        assertEq(
-            _initAmount,
-            _subvault.totalAssets(),
-            "subvault should be initiated with assets = _initAmount"
-        );
-        assertEq(
-            _initAmount,
-            _subvault.totalSupply(),
-            "subvault should be initiated with shares = _initAmount"
-        );
-
-        vault.setSubVault(IERC4626(address(_subvault)));
-    }
-}
+// contract MasterVaultTestWithSubvaultFresh is MasterVaultTest {
+//     function setUp() public override {
+//         super.setUp();
+//         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
+//         vault.setSubVault(IERC4626(address(_subvault)));
+//     }
+// }
+
+// contract MasterVaultTestWithSubvaultHoldingAssets is MasterVaultTest {
+//     function setUp() public override {
+//         super.setUp();
+
+//         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
+//         uint256 _initAmount = 97659743;
+//         token.mint(_initAmount);
+//         token.approve(address(_subvault), _initAmount);
+//         _subvault.deposit(_initAmount, address(this));
+//         assertEq(
+//             _initAmount,
+//             _subvault.totalAssets(),
+//             "subvault should be initiated with assets = _initAmount"
+//         );
+//         assertEq(
+//             _initAmount,
+//             _subvault.totalSupply(),
+//             "subvault should be initiated with shares = _initAmount"
+//         );
+
+//         vault.setSubVault(IERC4626(address(_subvault)));
+//     }
+// }
diff --git a/test-foundry/libraries/vault/MasterVaultAttack.t.sol b/test-foundry/libraries/vault/MasterVaultAttack.t.sol
index 3523dfaa3..03c768f23 100644
--- a/test-foundry/libraries/vault/MasterVaultAttack.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultAttack.t.sol
@@ -2,12 +2,12 @@
 pragma solidity ^0.8.0;
 
 import "forge-std/console2.sol";
-import { MasterVaultTest } from "./MasterVault.t.sol";
+import { MasterVaultCoreTest } from "./MasterVaultCore.t.sol";
 import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.sol";
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { IERC4626 } from "@openzeppelin/contracts/interfaces/IERC4626.sol";
 
-contract MasterVaultTestWithSubvaultFresh is MasterVaultTest {
+contract MasterVaultTestWithSubvaultFresh is MasterVaultCoreTest {
     function setUp() public override {
         super.setUp();
         MockSubVault _subvault = new MockSubVault(IERC20(address(token)), "TestSubvault", "TSV");
diff --git a/test-foundry/libraries/vault/MasterVaultCore.t.sol b/test-foundry/libraries/vault/MasterVaultCore.t.sol
index dea2ff62e..fc5549578 100644
--- a/test-foundry/libraries/vault/MasterVaultCore.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultCore.t.sol
@@ -18,7 +18,7 @@ contract MasterVaultCoreTest is Test {
     MasterVault public vault;
     TestERC20 public token;
 
-    address public user = address(0x1);
+    address public user = vm.addr(1);
     string public name = "Master Test Token";
     string public symbol = "mTST";
 

From cf3d27b1294552235b0398dce2219602e27bef4d Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Tue, 16 Dec 2025 16:10:11 -0500
Subject: [PATCH 09/27] initial approval and small refactor

---
 .../tokenbridge/libraries/vault/MasterVault.sol | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 40d455975..4c5dd5148 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -93,6 +93,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         // https://web.archive.org/web/20250609034056/https://docs.openzeppelin.com/contracts/4.x/erc4626#fees
         _mint(address(1), 10 ** EXTRA_DECIMALS);
 
+        IERC20(asset()).safeApprove(address(_subVault), type(uint256).max);
+
         subVault = _subVault;
     }
     
@@ -185,6 +187,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
             totalPrincipal = _totalAssets(MathUpgradeable.Rounding.Up);
         }
         else {
+            // todo: we need to distribute here
             totalPrincipal = 0;
         }
 
@@ -284,11 +287,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     function _convertToShares(uint256 assets, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 shares) {
         // we add one as part of the first deposit mitigation
         // see for details: https://docs.openzeppelin.com/contracts/5.x/erc4626
-        uint256 __totalAssets = _totalAssets(_flipRounding(rounding)) + 1;
-        if (enablePerformanceFee) {
-            __totalAssets -= totalProfit(rounding);
-        }
-        return assets.mulDiv(totalSupply(), __totalAssets, rounding);
+        return assets.mulDiv(totalSupply(), _totalAssetsLessProfit(_flipRounding(rounding)) + 1, rounding);
     }
 
     /**
@@ -297,11 +296,15 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual override returns (uint256 assets) {
         // we add one as part of the first deposit mitigation
         // see for details: https://docs.openzeppelin.com/contracts/5.x/erc4626
-        uint256 __totalAssets = _totalAssets(rounding) + 1;
+        return shares.mulDiv(_totalAssetsLessProfit(rounding) + 1, totalSupply(), rounding);
+    }
+
+    function _totalAssetsLessProfit(MathUpgradeable.Rounding rounding) internal view returns (uint256) {
+        uint256 __totalAssets = _totalAssets(rounding);
         if (enablePerformanceFee) {
             __totalAssets -= totalProfit(_flipRounding(rounding));
         }
-        return shares.mulDiv(__totalAssets, totalSupply(), rounding);
+        return __totalAssets;
     }
 
     function _subVaultSharesToAssets(uint256 subShares, MathUpgradeable.Rounding rounding) internal view returns (uint256 assets) {

From e2bdd276ce751d43f678730739f7441f13bba848 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Tue, 16 Dec 2025 16:14:25 -0500
Subject: [PATCH 10/27] tests

---
 .../libraries/vault/MasterVault.t.sol         | 194 +++++++++++-------
 .../libraries/vault/MasterVaultCore.t.sol     |   3 +-
 2 files changed, 123 insertions(+), 74 deletions(-)

diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol
index 403762ffd..ba90270a0 100644
--- a/test-foundry/libraries/vault/MasterVault.t.sol
+++ b/test-foundry/libraries/vault/MasterVault.t.sol
@@ -5,101 +5,149 @@ import { MasterVaultCoreTest } from "./MasterVaultCore.t.sol";
 import { MockSubVault } from "../../../contracts/tokenbridge/test/MockSubVault.sol";
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { IERC4626 } from "@openzeppelin/contracts/interfaces/IERC4626.sol";
+import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
+import {console2} from "forge-std/console2.sol";
 
 contract MasterVaultFirstDepositTest is MasterVaultCoreTest {
-    // first deposit
-    function test_deposit() public {
-        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(address(vault));
+    using Math for uint256;
+
+    uint256 constant FRESH_STATE_PLACEHOLDER = uint256(keccak256("FRESH_STATE_PLACEHOLDER"));
+    uint256 constant DEAD_SHARES = 10**18;
+
+    struct State {
+        uint256 userShares;
+        uint256 masterVaultTotalAssets;
+        uint256 masterVaultTotalSupply;
+        uint256 masterVaultTokenBalance;
+        uint256 masterVaultSubVaultShareBalance;
+        uint256 subVaultTotalAssets;
+        uint256 subVaultTotalSupply;
+        uint256 subVaultTokenBalance;
+    }
 
+    // first deposit
+    function test_deposit(uint96 _depositAmount) public {
+        uint256 depositAmount = _depositAmount;
         vm.startPrank(user);
-        token.mint();
-
-        uint256 depositAmount = 100;
-        uint256 expectedShares = depositAmount * 10**vault.EXTRA_DECIMALS();
-        uint256 deadShares = 10**vault.EXTRA_DECIMALS();
-
+        token.mint(depositAmount);
         token.approve(address(vault), depositAmount);
-
         uint256 shares = vault.deposit(depositAmount, user);
-
-        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(address(vault));
-        uint256 diff = _assetsHoldingVaultBalanceAfter - _assetsHoldingVaultBalanceBefore;
-
-        assertEq(shares, expectedShares, "Shares minted should equal deposit amount at a rate of 1^extra_decimals");
-        assertEq(vault.balanceOf(user), expectedShares, "User should receive shares");
-        assertEq(vault.totalAssets(), depositAmount, "Vault should hold deposited assets");
-        assertEq(vault.totalSupply(), shares + deadShares, "Total supply should equal shares minted plus dead shares");
-        assertEq(diff, depositAmount, "Vault should increase holding of assets");
         vm.stopPrank();
+        _checkState(State({
+            userShares: depositAmount * DEAD_SHARES,
+            masterVaultTotalAssets: depositAmount,
+            masterVaultTotalSupply: (1 + depositAmount) * DEAD_SHARES,
+            masterVaultTokenBalance: depositAmount,
+            masterVaultSubVaultShareBalance: 0,
+            subVaultTotalAssets: 0,
+            subVaultTotalSupply: 0,
+            subVaultTokenBalance: 0
+        }));
+        assertEq(shares, depositAmount * DEAD_SHARES, "shares mismatch deposit return value");
     }
 
-    // first mint
-    function test_mint() public {
-        uint256 _assetsHoldingVaultBalanceBefore = token.balanceOf(address(vault));
-
+    function test_mint(uint96 _mintAmount) public {
+        uint256 mintAmount = _mintAmount;
         vm.startPrank(user);
-        token.mint();
-
-        uint256 sharesToMint = 100 * 10**vault.EXTRA_DECIMALS();
-        uint256 deadShares = 10**vault.EXTRA_DECIMALS();
-
-        token.approve(address(vault), type(uint256).max);
-
-        uint256 assetsCost = vault.mint(sharesToMint, user);
-        uint256 expectedAssetsCost = 100;
-
-        uint256 _assetsHoldingVaultBalanceAfter = token.balanceOf(address(vault));
-        uint256 diff = _assetsHoldingVaultBalanceAfter - _assetsHoldingVaultBalanceBefore;
-
-        assertEq(assetsCost, expectedAssetsCost, "Assets spent should equal mint amount at a rate of 1^extra_decimals");
-        assertEq(vault.balanceOf(user), sharesToMint, "User should receive shares");
-        assertEq(vault.totalAssets(), expectedAssetsCost, "Vault should hold deposited assets");
-        assertEq(vault.totalSupply(), sharesToMint + deadShares, "Total supply should equal shares minted plus dead shares");
-        assertEq(diff, expectedAssetsCost, "Vault should increase holding of assets");
+        token.mint(mintAmount);
+        token.approve(address(vault), mintAmount);
+        uint256 assets = vault.mint(mintAmount, user);
         vm.stopPrank();
+        _checkState(State({
+            userShares: mintAmount,
+            masterVaultTotalAssets: mintAmount.ceilDiv(1e18),
+            masterVaultTotalSupply: mintAmount + DEAD_SHARES,
+            masterVaultTokenBalance: mintAmount.ceilDiv(1e18),
+            masterVaultSubVaultShareBalance: 0,
+            subVaultTotalAssets: 0,
+            subVaultTotalSupply: 0,
+            subVaultTokenBalance: 0
+        }));
+        assertEq(assets, mintAmount.ceilDiv(1e18), "assets mismatch mint return value");
     }
 
-    function test_withdraw() public {
+    function test_withdraw(uint96 _firstDeposit, uint96 _withdrawAmount) public {
+        uint256 firstDeposit = _firstDeposit;
+        uint256 withdrawAmount = _withdrawAmount;
+        vm.assume(withdrawAmount <= firstDeposit);
+        test_deposit(_firstDeposit);
         vm.startPrank(user);
-        token.mint();
-        uint256 depositAmount = token.balanceOf(user);
-        token.approve(address(vault), depositAmount);
-        vault.deposit(depositAmount, user);
-
-        uint256 userSharesBefore = vault.balanceOf(user);
-        uint256 withdrawAmount = depositAmount; // withdraw all assets
-
         uint256 sharesRedeemed = vault.withdraw(withdrawAmount, user, user);
-
-        assertEq(vault.balanceOf(user), 0, "User should have no shares left");
-        assertEq(token.balanceOf(user), depositAmount, "User should receive all withdrawn tokens");
-        assertEq(vault.totalAssets(), 0, "Vault should have no assets left");
-        assertEq(vault.totalSupply(), 10**vault.EXTRA_DECIMALS(), "Total supply should be only dead shares");
-        assertEq(token.balanceOf(address(vault)), 0, "Vault should have no tokens left");
-        assertEq(sharesRedeemed, userSharesBefore, "All shares should be redeemed");
-
         vm.stopPrank();
+        _checkState(State({
+            userShares: (firstDeposit - withdrawAmount) * DEAD_SHARES,
+            masterVaultTotalAssets: firstDeposit - withdrawAmount,
+            masterVaultTotalSupply: (1 + firstDeposit - withdrawAmount) * DEAD_SHARES,
+            masterVaultTokenBalance: firstDeposit - withdrawAmount,
+            masterVaultSubVaultShareBalance: 0,
+            subVaultTotalAssets: 0,
+            subVaultTotalSupply: 0,
+            subVaultTokenBalance: 0
+        }));
+        assertEq(sharesRedeemed, withdrawAmount * DEAD_SHARES, "sharesRedeemed mismatch withdraw return value");
     }
 
-    function test_redeem() public {
-        vm.startPrank(user);
-        token.mint();
-        uint256 depositAmount = token.balanceOf(user);
-        token.approve(address(vault), depositAmount);
-        uint256 shares = vault.deposit(depositAmount, user);
+    function testFoo() public {
+        test_redeem(79228162514264337593543950335, 79228162514264337593543950332);
+    }
 
-        uint256 sharesToRedeem = shares; // redeem all shares
+    function test_redeem(uint96 _firstMint, uint96 _redeemAmount) public {
+        uint256 firstMint = _firstMint;
+        uint256 redeemAmount = _redeemAmount;
+        vm.assume(redeemAmount <= firstMint);
+        test_mint(_firstMint);
+        State memory beforeState = _getState();
+        vm.startPrank(user);
+        uint256 assets = vault.redeem(redeemAmount, user, user);
+        uint256 expectedAssets = (1 + beforeState.masterVaultTotalAssets) * redeemAmount / (beforeState.masterVaultTotalSupply);
+        vm.stopPrank();
+        _checkState(State({
+            userShares: beforeState.userShares - redeemAmount,
+            masterVaultTotalAssets: beforeState.masterVaultTotalAssets - expectedAssets,
+            masterVaultTotalSupply: beforeState.masterVaultTotalSupply - redeemAmount,
+            masterVaultTokenBalance: beforeState.masterVaultTokenBalance - expectedAssets,
+            masterVaultSubVaultShareBalance: 0,
+            subVaultTotalAssets: 0,
+            subVaultTotalSupply: 0,
+            subVaultTokenBalance: 0
+        }));
+        assertEq(assets, expectedAssets, "assets mismatch redeem return value");
+    }
 
-        uint256 assetsReceived = vault.redeem(sharesToRedeem, user, user);
+    function _checkState(State memory expectedState) internal {
+        assertEq(expectedState.userShares, vault.balanceOf(user), "userShares mismatch");
+        assertEq(expectedState.masterVaultTotalAssets, vault.totalAssets(), "masterVaultTotalAssets mismatch");
+        assertEq(expectedState.masterVaultTotalSupply, vault.totalSupply(), "masterVaultTotalSupply mismatch");
+        assertEq(expectedState.masterVaultTokenBalance, token.balanceOf(address(vault)), "masterVaultTokenBalance mismatch");
+        assertEq(expectedState.masterVaultSubVaultShareBalance, vault.subVault().balanceOf(address(vault)), "masterVaultSubVaultShareBalance mismatch");
+        assertEq(expectedState.subVaultTotalAssets, vault.subVault().totalAssets(), "subVaultTotalAssets mismatch");
+        assertEq(expectedState.subVaultTotalSupply, vault.subVault().totalSupply(), "subVaultTotalSupply mismatch");
+        assertEq(expectedState.subVaultTokenBalance, token.balanceOf(address(vault.subVault())), "subVaultTokenBalance mismatch");
+    }
 
-        assertEq(vault.balanceOf(user), 0, "User should have no shares left");
-        assertEq(token.balanceOf(user), depositAmount, "User should receive all assets back");
-        assertEq(vault.totalAssets(), 0, "Vault should have no assets left");
-        assertEq(vault.totalSupply(), 10**vault.EXTRA_DECIMALS(), "Total supply should be only dead shares");
-        assertEq(token.balanceOf(address(vault)), 0, "Vault should have no tokens left");
-        assertEq(assetsReceived, depositAmount, "All assets should be received");
+    function _getState() internal view returns (State memory) {
+        return State({
+            userShares: vault.balanceOf(user),
+            masterVaultTotalAssets: vault.totalAssets(),
+            masterVaultTotalSupply: vault.totalSupply(),
+            masterVaultTokenBalance: token.balanceOf(address(vault)),
+            masterVaultSubVaultShareBalance: vault.subVault().balanceOf(address(vault)),
+            subVaultTotalAssets: vault.subVault().totalAssets(),
+            subVaultTotalSupply: vault.subVault().totalSupply(),
+            subVaultTokenBalance: token.balanceOf(address(vault.subVault()))
+        });
+    }
 
-        vm.stopPrank();
+    function _logState(string memory label, State memory state) internal view {
+        console2.log(label);
+        console2.log(" userShares:", state.userShares);
+        console2.log(" masterVaultTotalAssets:", state.masterVaultTotalAssets);
+        console2.log(" masterVaultTotalSupply:", state.masterVaultTotalSupply);
+        console2.log(" masterVaultTokenBalance:", state.masterVaultTokenBalance);
+        console2.log(" masterVaultSubVaultShareBalance:", state.masterVaultSubVaultShareBalance);
+        console2.log(" subVaultTotalAssets:", state.subVaultTotalAssets);
+        console2.log(" subVaultTotalSupply:", state.subVaultTotalSupply);
+        console2.log(" subVaultTokenBalance:", state.subVaultTokenBalance);
     }
 }
 
diff --git a/test-foundry/libraries/vault/MasterVaultCore.t.sol b/test-foundry/libraries/vault/MasterVaultCore.t.sol
index fc5549578..06d08938e 100644
--- a/test-foundry/libraries/vault/MasterVaultCore.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultCore.t.sol
@@ -19,6 +19,7 @@ contract MasterVaultCoreTest is Test {
     TestERC20 public token;
 
     address public user = vm.addr(1);
+    address public admin = vm.addr(2);
     string public name = "Master Test Token";
     string public symbol = "mTST";
 
@@ -28,7 +29,7 @@ contract MasterVaultCoreTest is Test {
 
     function setUp() public virtual {
         factory = new MasterVaultFactory();
-        factory.initialize(address(this));
+        factory.initialize(admin);
         token = new TestERC20();
         vault = MasterVault(factory.deployVault(address(token)));
     }

From 6b5e74036ada4ac5a939a8eb0f1662a59b8924f9 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Tue, 16 Dec 2025 16:29:49 -0500
Subject: [PATCH 11/27] fix PerformanceFeesWithdrawn event

---
 .../tokenbridge/libraries/vault/MasterVault.sol | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 4c5dd5148..9c0db33b5 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -69,7 +69,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     event SubvaultChanged(address indexed oldSubvault, address indexed newSubvault);
     event PerformanceFeeToggled(bool enabled);
     event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary);
-    event PerformanceFeesWithdrawn(address indexed beneficiary, uint256 amount);
+    event PerformanceFeesWithdrawn(address indexed beneficiary, uint256 amountTransferred, uint256 amountWithdrawn);
 
     function initialize(IERC4626 _subVault, string memory _name, string memory _symbol, address _owner) external initializer {
         __ERC20_init(_name, _symbol);
@@ -113,19 +113,20 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         if (profit == 0) return;
 
         uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
-        if (totalIdle > 0) {
-            uint256 amountToTransfer = profit <= totalIdle ? profit : totalIdle;
+        
+        uint256 amountToTransfer = profit <= totalIdle ? profit : totalIdle;
+        uint256 amountToWithdraw = profit - amountToTransfer;
+        
+        if (amountToTransfer > 0) {
             IERC20(asset()).safeTransfer(beneficiary, amountToTransfer);
-            profit -= amountToTransfer;
         }
-
-        if (profit > 0) {
-            subVault.withdraw(profit, beneficiary, address(this));
+        if (amountToWithdraw > 0) {
+            subVault.withdraw(amountToWithdraw, beneficiary, address(this));
         }
 
         rebalance();
 
-        emit PerformanceFeesWithdrawn(beneficiary, profit);
+        emit PerformanceFeesWithdrawn(beneficiary, amountToTransfer, amountToWithdraw);
     }
 
     error NonZeroTargetAllocation(uint256 targetAllocationWad);

From 71237b7a98708ab8b09c5c1d9a8d8ec4f9201292 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Tue, 16 Dec 2025 16:30:24 -0500
Subject: [PATCH 12/27] move event

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 9c0db33b5..36da84443 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -49,6 +49,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     error BeneficiaryNotSet();
     error InvalidAsset();
     error InvalidOwner();
+    error NonZeroTargetAllocation(uint256 targetAllocationWad);
 
     // 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)
@@ -129,8 +130,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit PerformanceFeesWithdrawn(beneficiary, amountToTransfer, amountToWithdraw);
     }
 
-    error NonZeroTargetAllocation(uint256 targetAllocationWad);
-
     /// @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.
     function setSubVault(IERC4626 _subVault) external onlyRole(VAULT_MANAGER_ROLE) {

From ba014527cecd59154c8886f21f310378cbb8caef Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 10:28:02 -0500
Subject: [PATCH 13/27] rebalancing does not count profit

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 36da84443..d5119a3ed 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -148,8 +148,8 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
 
     function rebalance() public {
         // todo: handle 0 and 100 special cases if needed
-        uint256 totalAssetsUp = _totalAssets(MathUpgradeable.Rounding.Up);
-        uint256 totalAssetsDown = _totalAssets(MathUpgradeable.Rounding.Down);
+        uint256 totalAssetsUp = _totalAssetsLessProfit(MathUpgradeable.Rounding.Up);
+        uint256 totalAssetsDown = _totalAssetsLessProfit(MathUpgradeable.Rounding.Down);
         uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
         uint256 idleTargetDown = totalAssetsDown.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Down);
         uint256 idleBalance = IERC20(asset()).balanceOf(address(this));

From 79819e8ce9a797ec5694a683ddf967eebcf1be78 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 10:48:56 -0500
Subject: [PATCH 14/27] distribute fees before disabling

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index d5119a3ed..813110ac8 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -104,7 +104,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         return super.decimals() + EXTRA_DECIMALS;
     }
 
-    function distributePerformanceFee() external whenNotPaused {
+    function distributePerformanceFee() public whenNotPaused {
         if (!enablePerformanceFee) revert PerformanceFeeDisabled();
         if (beneficiary == address(0)) {
             revert BeneficiaryNotSet();
@@ -147,7 +147,6 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     }
 
     function rebalance() public {
-        // todo: handle 0 and 100 special cases if needed
         uint256 totalAssetsUp = _totalAssetsLessProfit(MathUpgradeable.Rounding.Up);
         uint256 totalAssetsDown = _totalAssetsLessProfit(MathUpgradeable.Rounding.Down);
         uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
@@ -179,18 +178,18 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     /// @notice Toggle performance fee collection on/off
     /// @param enabled True to enable performance fees, false to disable
     function setPerformanceFee(bool enabled) external onlyRole(VAULT_MANAGER_ROLE) {
-        enablePerformanceFee = enabled;
-
         // reset totalPrincipal to current totalAssets when enabling performance fee
         // this prevents a sudden large profit
         if (enabled) {
             totalPrincipal = _totalAssets(MathUpgradeable.Rounding.Up);
         }
         else {
-            // todo: we need to distribute here
+            distributePerformanceFee();
             totalPrincipal = 0;
         }
 
+        enablePerformanceFee = enabled;
+
         emit PerformanceFeeToggled(enabled);
     }
 

From 83a59663e1d9cfe13a9774350170306567820758 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:02:38 -0500
Subject: [PATCH 15/27] fix maxMint

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 813110ac8..2e72c0dd5 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -224,14 +224,9 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
 
     // /** @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;
-        }
-        return totalSupply().mulDiv(subShares, subVault.balanceOf(address(this)), MathUpgradeable.Rounding.Down); // todo: check rounding direction
+        uint256 assets = _subVaultSharesToAssets(subShares, MathUpgradeable.Rounding.Down);
+        return _convertToShares(assets, MathUpgradeable.Rounding.Down);
     }
 
     function totalProfit(MathUpgradeable.Rounding rounding) public view returns (uint256) {

From 281bf389803b25915b1680e5448acf4f6f85a4e1 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:04:38 -0500
Subject: [PATCH 16/27] fix outdated comment

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 2e72c0dd5..8ac9b7f2c 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -194,7 +194,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     }
 
     /// @notice Set the beneficiary address for performance fees
-    /// @param newBeneficiary Address to receive performance fees, zero address defaults to owner
+    /// @param newBeneficiary Address to receive performance fees
     function setBeneficiary(address newBeneficiary) external onlyRole(VAULT_MANAGER_ROLE) {
         address oldBeneficiary = beneficiary;
         beneficiary = newBeneficiary;

From b49255e6466590a003802191330a9ea25f5f7561 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:12:46 -0500
Subject: [PATCH 17/27] 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 8ac9b7f2c..60a9d4ceb 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -41,10 +41,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
     ///         See https://docs.openzeppelin.com/contracts/5.x/erc4626 for more details on the mitigation.
     uint8 public constant EXTRA_DECIMALS = 18;
 
-    error SubVaultAlreadySet();
     error SubVaultAssetMismatch();
-    error NoExistingSubVault();
-    error SubVaultExchangeRateTooLow(int256 required, int256 actual);
     error PerformanceFeeDisabled();
     error BeneficiaryNotSet();
     error InvalidAsset();

From aba0f9226caca96ce00d4adfdfcb3a2bc2436361 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:24:00 -0500
Subject: [PATCH 18/27] nonReentrant

---
 .../libraries/vault/MasterVault.sol           | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 60a9d4ceb..e12171396 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -12,6 +12,7 @@ import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Ini
 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";
+import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 
 // todo: should we have an arbitrary call function for the vault manager to do stuff with the subvault? like queue withdrawals etc
 
@@ -26,7 +27,7 @@ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol
 ///         - It must be able to handle arbitrarily large deposits and withdrawals
 ///         - Deposit size or withdrawal size must not affect the exchange rate (i.e. no slippage)
 ///         - convertToAssets and convertToShares must not be manipulable
-contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradeable, PausableUpgradeable {
+contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgradeable, AccessControlUpgradeable, PausableUpgradeable {
     using SafeERC20 for IERC20;
     using MathUpgradeable for uint256;
 
@@ -101,7 +102,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         return super.decimals() + EXTRA_DECIMALS;
     }
 
-    function distributePerformanceFee() public whenNotPaused {
+    function distributePerformanceFee() public whenNotPaused nonReentrant {
         if (!enablePerformanceFee) revert PerformanceFeeDisabled();
         if (beneficiary == address(0)) {
             revert BeneficiaryNotSet();
@@ -129,7 +130,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
 
     /// @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.
-    function setSubVault(IERC4626 _subVault) external onlyRole(VAULT_MANAGER_ROLE) {
+    function setSubVault(IERC4626 _subVault) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         IERC20 underlyingAsset = IERC20(asset());
         if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch();
         if (targetAllocationWad != 0) revert NonZeroTargetAllocation(targetAllocationWad);
@@ -143,7 +144,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
-    function rebalance() public {
+    function rebalance() public whenNotPaused nonReentrant {
         uint256 totalAssetsUp = _totalAssetsLessProfit(MathUpgradeable.Rounding.Up);
         uint256 totalAssetsDown = _totalAssetsLessProfit(MathUpgradeable.Rounding.Down);
         uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
@@ -166,15 +167,17 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         }
     }
 
-    function setTargetAllocationWad(uint256 _targetAllocationWad) external onlyRole(VAULT_MANAGER_ROLE) {
+    function setTargetAllocationWad(uint256 _targetAllocationWad) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
         require(targetAllocationWad != _targetAllocationWad, "Allocation unchanged");
         targetAllocationWad = _targetAllocationWad;
     }
 
     /// @notice Toggle performance fee collection on/off
+    /// @dev    Not explicitly marked nonReentrant because distributePerformanceFee is called within
+    ///         this function and is nonReentrant itself.
     /// @param enabled True to enable performance fees, false to disable
-    function setPerformanceFee(bool enabled) external onlyRole(VAULT_MANAGER_ROLE) {
+    function setPerformanceFee(bool enabled) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         // reset totalPrincipal to current totalAssets when enabling performance fee
         // this prevents a sudden large profit
         if (enabled) {
@@ -192,7 +195,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
 
     /// @notice Set the beneficiary address for performance fees
     /// @param newBeneficiary Address to receive performance fees
-    function setBeneficiary(address newBeneficiary) external onlyRole(VAULT_MANAGER_ROLE) {
+    function setBeneficiary(address newBeneficiary) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         address oldBeneficiary = beneficiary;
         beneficiary = newBeneficiary;
         emit BeneficiaryUpdated(oldBeneficiary, newBeneficiary);
@@ -239,7 +242,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address receiver,
         uint256 assets,
         uint256 shares
-    ) internal override whenNotPaused {
+    ) internal override whenNotPaused nonReentrant {
         super._deposit(caller, receiver, assets, shares);
         if (enablePerformanceFee) totalPrincipal += assets;
         rebalance();
@@ -254,7 +257,7 @@ contract MasterVault is Initializable, ERC4626Upgradeable, AccessControlUpgradea
         address _owner,
         uint256 assets,
         uint256 shares
-    ) internal override whenNotPaused {
+    ) internal override whenNotPaused nonReentrant {
         if (enablePerformanceFee) totalPrincipal -= assets;
         uint256 idleAssets = IERC20(asset()).balanceOf(address(this));
         if (idleAssets < assets) {

From 234efe733420c15ebdf8ca716a6bd518b378c1f9 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:31:37 -0500
Subject: [PATCH 19/27] handle max in maxMint

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index e12171396..1a74d9a0c 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -225,6 +225,9 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
     // /** @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;
+        }
         uint256 assets = _subVaultSharesToAssets(subShares, MathUpgradeable.Rounding.Down);
         return _convertToShares(assets, MathUpgradeable.Rounding.Down);
     }

From 45e1d001bdedc671ba6c506dae212e633652b584 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:33:24 -0500
Subject: [PATCH 20/27] sanity check we have no sub shares when switching

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 1a74d9a0c..81bd1fd3e 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -48,6 +48,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
     error InvalidAsset();
     error InvalidOwner();
     error NonZeroTargetAllocation(uint256 targetAllocationWad);
+    error NonZeroSubVaultShares(uint256 subVaultShares);
 
     // 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)
@@ -133,8 +134,15 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
     function setSubVault(IERC4626 _subVault) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         IERC20 underlyingAsset = IERC20(asset());
         if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch();
+
+        // we ensure target allocation is zero, therefore the master vault holds no subvault shares
         if (targetAllocationWad != 0) revert NonZeroTargetAllocation(targetAllocationWad);
 
+        // sanity check to ensure we have zero subvault shares before changing
+        if (subVault.balanceOf(address(this)) != 0) {
+            revert NonZeroSubVaultShares(subVault.balanceOf(address(this)));
+        }
+
         address oldSubVault = address(subVault);
         subVault = _subVault;
 

From a023fa2c361d5e7adeac77c8dc7d37d6badbe9b9 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:34:40 -0500
Subject: [PATCH 21/27] init reentrancy guard

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 81bd1fd3e..98ee5736a 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -78,6 +78,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
         // call decimals() to ensure underlying has reasonable decimals and we won't have overflow
         decimals();
 
+        __ReentrancyGuard_init();
         __AccessControl_init();
         __Pausable_init();
 

From b0cf3e0c52bc5728ee1ea7aa919a07acd9d4a2e3 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:36:14 -0500
Subject: [PATCH 22/27] rebalance after setting target

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 98ee5736a..905db087a 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -180,6 +180,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
         require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
         require(targetAllocationWad != _targetAllocationWad, "Allocation unchanged");
         targetAllocationWad = _targetAllocationWad;
+        rebalance();
     }
 
     /// @notice Toggle performance fee collection on/off

From 519198bbc09de3d58f9ba38dc016eec2e750392f Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:40:38 -0500
Subject: [PATCH 23/27] update docs

---
 .../tokenbridge/libraries/vault/MasterVault.sol      | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 905db087a..139c1ef74 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -17,16 +17,14 @@ import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/se
 // todo: should we have an arbitrary call function for the vault manager to do stuff with the subvault? like queue withdrawals etc
 
 /// @notice MasterVault is an ERC4626 metavault that deposits assets to an admin defined subVault.
-/// @dev    If a subVault is not set, MasterVault shares entitle holders to a pro-rata share of the underlying held by the MasterVault.
-///         If a subVault is set, MasterVault shares entitle holders to a pro-rata share of subVault shares held by the MasterVault.
-///         On deposit to the MasterVault, if there is a subVault set, the assets are immediately deposited into the subVault.
-///         On withdraw from the MasterVault, if there is a subVault set, a pro rata amount of subvault shares are redeemed.
-///         On deposit and withdraw, if there is no subVault set, assets are moved to/from the MasterVault itself.
+/// @dev    The MasterVault keeps some fraction of assets idle and deposits the rest into the subVault to earn yield.
+///         A 100% performance fee can be enabled/disabled by the vault manager, and are collected on demand.
+///         The MasterVault mitigates the "first depositor" problem by adding 18 decimals to the underlying asset.
+///         i.e. if the underlying asset has 6 decimals, the MasterVault will have 24 decimals.
 ///
 ///         For a subVault to be compatible with the MasterVault, it must adhere to the following:
-///         - It must be able to handle arbitrarily large deposits and withdrawals
-///         - Deposit size or withdrawal size must not affect the exchange rate (i.e. no slippage)
 ///         - convertToAssets and convertToShares must not be manipulable
+///         - must not have deposit / withdrawal fees (todo: verify this requirement is necessary)
 contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgradeable, AccessControlUpgradeable, PausableUpgradeable {
     using SafeERC20 for IERC20;
     using MathUpgradeable for uint256;

From 101268aeb4094341ea43c2240fd61562945defcd Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 12:48:29 -0500
Subject: [PATCH 24/27] update outdated comment

---
 contracts/tokenbridge/libraries/vault/MasterVault.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 139c1ef74..7f9f27e5e 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -128,7 +128,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
         emit PerformanceFeesWithdrawn(beneficiary, amountToTransfer, amountToWithdraw);
     }
 
-    /// @notice Set a subvault. Can only be called if there is not already a subvault set.
+    /// @notice Set a new subvault
     /// @param  _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault.
     function setSubVault(IERC4626 _subVault) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
         IERC20 underlyingAsset = IERC20(asset());

From b8f2c2ff981a8a4a9b5c1f7ea532e5fa333a9e00 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 13:26:19 -0500
Subject: [PATCH 25/27] fix nonReentrant modifier placement

---
 .../libraries/vault/MasterVault.sol           | 126 ++++++++++--------
 1 file changed, 67 insertions(+), 59 deletions(-)

diff --git a/contracts/tokenbridge/libraries/vault/MasterVault.sol b/contracts/tokenbridge/libraries/vault/MasterVault.sol
index 7f9f27e5e..9e0893e5c 100644
--- a/contracts/tokenbridge/libraries/vault/MasterVault.sol
+++ b/contracts/tokenbridge/libraries/vault/MasterVault.sol
@@ -96,41 +96,18 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
 
         subVault = _subVault;
     }
-    
-    /// @dev Overridden to add EXTRA_DECIMALS to the underlying asset decimals
-    function decimals() public view override returns (uint8) {
-        return super.decimals() + EXTRA_DECIMALS;
-    }
-
-    function distributePerformanceFee() public whenNotPaused nonReentrant {
-        if (!enablePerformanceFee) revert PerformanceFeeDisabled();
-        if (beneficiary == address(0)) {
-            revert BeneficiaryNotSet();
-        }
-
-        uint256 profit = totalProfit(MathUpgradeable.Rounding.Down);
-        if (profit == 0) return;
-
-        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
-        
-        uint256 amountToTransfer = profit <= totalIdle ? profit : totalIdle;
-        uint256 amountToWithdraw = profit - amountToTransfer;
-        
-        if (amountToTransfer > 0) {
-            IERC20(asset()).safeTransfer(beneficiary, amountToTransfer);
-        }
-        if (amountToWithdraw > 0) {
-            subVault.withdraw(amountToWithdraw, beneficiary, address(this));
-        }
 
-        rebalance();
+    function rebalance() external whenNotPaused nonReentrant {
+        _rebalance();
+    }
 
-        emit PerformanceFeesWithdrawn(beneficiary, amountToTransfer, amountToWithdraw);
+    function distributePerformanceFee() external whenNotPaused nonReentrant {
+        _distributePerformanceFee();
     }
 
     /// @notice Set a new subvault
     /// @param  _subVault The subvault to set. Must be an ERC4626 vault with the same asset as this MasterVault.
-    function setSubVault(IERC4626 _subVault) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
+    function setSubVault(IERC4626 _subVault) external whenNotPaused nonReentrant onlyRole(VAULT_MANAGER_ROLE) {
         IERC20 underlyingAsset = IERC20(asset());
         if (address(_subVault.asset()) != address(underlyingAsset)) revert SubVaultAssetMismatch();
 
@@ -151,48 +128,25 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
         emit SubvaultChanged(oldSubVault, address(_subVault));
     }
 
-    function rebalance() public whenNotPaused nonReentrant {
-        uint256 totalAssetsUp = _totalAssetsLessProfit(MathUpgradeable.Rounding.Up);
-        uint256 totalAssetsDown = _totalAssetsLessProfit(MathUpgradeable.Rounding.Down);
-        uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
-        uint256 idleTargetDown = totalAssetsDown.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Down);
-        uint256 idleBalance = IERC20(asset()).balanceOf(address(this));
-        
-        if (idleTargetDown <= idleBalance && idleBalance <= idleTargetUp) {
-            return;
-        }
-
-        if (idleBalance < idleTargetDown) {
-            // we need to withdraw from subvault
-            uint256 assetsToWithdraw = idleTargetDown - idleBalance;
-            subVault.withdraw(assetsToWithdraw, address(this), address(this));
-        }
-        else {
-            // we need to deposit into subvault
-            uint256 assetsToDeposit = idleBalance - idleTargetUp;
-            subVault.deposit(assetsToDeposit, address(this));
-        }
-    }
-
-    function setTargetAllocationWad(uint256 _targetAllocationWad) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
+    function setTargetAllocationWad(uint256 _targetAllocationWad) external whenNotPaused nonReentrant onlyRole(VAULT_MANAGER_ROLE) {
         require(_targetAllocationWad <= 1e18, "Target allocation must be <= 100%");
         require(targetAllocationWad != _targetAllocationWad, "Allocation unchanged");
         targetAllocationWad = _targetAllocationWad;
-        rebalance();
+        _rebalance();
     }
 
     /// @notice Toggle performance fee collection on/off
     /// @dev    Not explicitly marked nonReentrant because distributePerformanceFee is called within
     ///         this function and is nonReentrant itself.
     /// @param enabled True to enable performance fees, false to disable
-    function setPerformanceFee(bool enabled) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
+    function setPerformanceFee(bool enabled) external whenNotPaused nonReentrant onlyRole(VAULT_MANAGER_ROLE) {
         // reset totalPrincipal to current totalAssets when enabling performance fee
         // this prevents a sudden large profit
         if (enabled) {
             totalPrincipal = _totalAssets(MathUpgradeable.Rounding.Up);
         }
         else {
-            distributePerformanceFee();
+            _distributePerformanceFee();
             totalPrincipal = 0;
         }
 
@@ -203,7 +157,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
 
     /// @notice Set the beneficiary address for performance fees
     /// @param newBeneficiary Address to receive performance fees
-    function setBeneficiary(address newBeneficiary) external whenNotPaused onlyRole(VAULT_MANAGER_ROLE) {
+    function setBeneficiary(address newBeneficiary) external whenNotPaused nonReentrant onlyRole(VAULT_MANAGER_ROLE) {
         address oldBeneficiary = beneficiary;
         beneficiary = newBeneficiary;
         emit BeneficiaryUpdated(oldBeneficiary, newBeneficiary);
@@ -216,6 +170,11 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
     function unpause() external onlyRole(PAUSER_ROLE) {
         _unpause();
     }
+    
+    /// @dev Overridden to add EXTRA_DECIMALS to the underlying asset decimals
+    function decimals() public view override returns (uint8) {
+        return super.decimals() + EXTRA_DECIMALS;
+    }
 
     /** @dev See {IERC4626-totalAssets}. */
     function totalAssets() public view virtual override returns (uint256) {
@@ -245,6 +204,55 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
         return __totalAssets > totalPrincipal ? __totalAssets - totalPrincipal : 0;
     }
 
+    function _rebalance() internal {
+        uint256 totalAssetsUp = _totalAssetsLessProfit(MathUpgradeable.Rounding.Up);
+        uint256 totalAssetsDown = _totalAssetsLessProfit(MathUpgradeable.Rounding.Down);
+        uint256 idleTargetUp = totalAssetsUp.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Up);
+        uint256 idleTargetDown = totalAssetsDown.mulDiv(1e18 - targetAllocationWad, 1e18, MathUpgradeable.Rounding.Down);
+        uint256 idleBalance = IERC20(asset()).balanceOf(address(this));
+        
+        if (idleTargetDown <= idleBalance && idleBalance <= idleTargetUp) {
+            return;
+        }
+
+        if (idleBalance < idleTargetDown) {
+            // we need to withdraw from subvault
+            uint256 assetsToWithdraw = idleTargetDown - idleBalance;
+            subVault.withdraw(assetsToWithdraw, address(this), address(this));
+        }
+        else {
+            // we need to deposit into subvault
+            uint256 assetsToDeposit = idleBalance - idleTargetUp;
+            subVault.deposit(assetsToDeposit, address(this));
+        }
+    }
+
+    function _distributePerformanceFee() internal {
+        if (!enablePerformanceFee) revert PerformanceFeeDisabled();
+        if (beneficiary == address(0)) {
+            revert BeneficiaryNotSet();
+        }
+
+        uint256 profit = totalProfit(MathUpgradeable.Rounding.Down);
+        if (profit == 0) return;
+
+        uint256 totalIdle = IERC20(asset()).balanceOf(address(this));
+        
+        uint256 amountToTransfer = profit <= totalIdle ? profit : totalIdle;
+        uint256 amountToWithdraw = profit - amountToTransfer;
+        
+        if (amountToTransfer > 0) {
+            IERC20(asset()).safeTransfer(beneficiary, amountToTransfer);
+        }
+        if (amountToWithdraw > 0) {
+            subVault.withdraw(amountToWithdraw, beneficiary, address(this));
+        }
+
+        _rebalance();
+
+        emit PerformanceFeesWithdrawn(beneficiary, amountToTransfer, amountToWithdraw);
+    }
+
     /**
      * @dev Deposit/mint common workflow.
      */
@@ -256,7 +264,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
     ) internal override whenNotPaused nonReentrant {
         super._deposit(caller, receiver, assets, shares);
         if (enablePerformanceFee) totalPrincipal += assets;
-        rebalance();
+        _rebalance();
     }
 
     /**
@@ -276,7 +284,7 @@ contract MasterVault is Initializable, ReentrancyGuardUpgradeable, ERC4626Upgrad
             subVault.withdraw(assetsToWithdraw, address(this), address(this));
         }
         super._withdraw(caller, receiver, _owner, assets, shares);
-        rebalance();
+        _rebalance();
     }
 
     function _totalAssets(MathUpgradeable.Rounding rounding) internal view returns (uint256) {

From b601580292ad0641681e99b9fa0e0fe65a1010c3 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Wed, 17 Dec 2025 13:26:33 -0500
Subject: [PATCH 26/27] remove testFoo

---
 test-foundry/libraries/vault/MasterVault.t.sol | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/test-foundry/libraries/vault/MasterVault.t.sol b/test-foundry/libraries/vault/MasterVault.t.sol
index ba90270a0..639289253 100644
--- a/test-foundry/libraries/vault/MasterVault.t.sol
+++ b/test-foundry/libraries/vault/MasterVault.t.sol
@@ -87,10 +87,6 @@ contract MasterVaultFirstDepositTest is MasterVaultCoreTest {
         assertEq(sharesRedeemed, withdrawAmount * DEAD_SHARES, "sharesRedeemed mismatch withdraw return value");
     }
 
-    function testFoo() public {
-        test_redeem(79228162514264337593543950335, 79228162514264337593543950332);
-    }
-
     function test_redeem(uint96 _firstMint, uint96 _redeemAmount) public {
         uint256 firstMint = _firstMint;
         uint256 redeemAmount = _redeemAmount;

From 15a1121a1397e969787073b83289f8a279a880a1 Mon Sep 17 00:00:00 2001
From: Henry <11198460+godzillaba@users.noreply.github.com>
Date: Tue, 23 Dec 2025 11:26:22 -0600
Subject: [PATCH 27/27] fix tests

---
 .../libraries/vault/MasterVaultCore.t.sol     |  3 +--
 .../libraries/vault/MasterVaultFactory.t.sol  |  2 +-
 .../libraries/vault/MasterVaultFee.t.sol      | 22 +++++++++++++------
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/test-foundry/libraries/vault/MasterVaultCore.t.sol b/test-foundry/libraries/vault/MasterVaultCore.t.sol
index 06d08938e..fc5549578 100644
--- a/test-foundry/libraries/vault/MasterVaultCore.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultCore.t.sol
@@ -19,7 +19,6 @@ contract MasterVaultCoreTest is Test {
     TestERC20 public token;
 
     address public user = vm.addr(1);
-    address public admin = vm.addr(2);
     string public name = "Master Test Token";
     string public symbol = "mTST";
 
@@ -29,7 +28,7 @@ contract MasterVaultCoreTest is Test {
 
     function setUp() public virtual {
         factory = new MasterVaultFactory();
-        factory.initialize(admin);
+        factory.initialize(address(this));
         token = new TestERC20();
         vault = MasterVault(factory.deployVault(address(token)));
     }
diff --git a/test-foundry/libraries/vault/MasterVaultFactory.t.sol b/test-foundry/libraries/vault/MasterVaultFactory.t.sol
index 48359c390..ec7e65fa4 100644
--- a/test-foundry/libraries/vault/MasterVaultFactory.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultFactory.t.sol
@@ -45,7 +45,7 @@ contract MasterVaultFactoryTest is Test {
     }
 
     function test_deployVault_RevertZeroAddress() public {
-        vm.expectRevert(MasterVaultFactory.ZeroAddress.selector);
+        vm.expectRevert();
         factory.deployVault(address(0));
     }
 
diff --git a/test-foundry/libraries/vault/MasterVaultFee.t.sol b/test-foundry/libraries/vault/MasterVaultFee.t.sol
index 14a56c0b6..71d34aa88 100644
--- a/test-foundry/libraries/vault/MasterVaultFee.t.sol
+++ b/test-foundry/libraries/vault/MasterVaultFee.t.sol
@@ -19,10 +19,18 @@ contract MasterVaultFeeTest is MasterVaultCoreTest {
         assertTrue(vault.enablePerformanceFee(), "Performance fee should be enabled");
     }
 
-    function test_setPerformanceFee_disable() public {
+    function test_cannotDisableWithoutBeneficiarySet() public {
         vault.setPerformanceFee(true);
         assertTrue(vault.enablePerformanceFee(), "Performance fee should be enabled");
 
+        vm.expectRevert(MasterVault.BeneficiaryNotSet.selector);
+        vault.setPerformanceFee(false);
+    }
+
+    function test_setPerformanceFee_disable() public {
+        vault.setPerformanceFee(true);
+        assertTrue(vault.enablePerformanceFee(), "Performance fee should be enabled");
+        vault.setBeneficiary(beneficiaryAddress);
         vault.setPerformanceFee(false);
 
         assertFalse(vault.enablePerformanceFee(), "Performance fee should be disabled");
@@ -35,6 +43,8 @@ contract MasterVaultFeeTest is MasterVaultCoreTest {
     }
 
     function test_setPerformanceFee_emitsEvent() public {
+        vault.setBeneficiary(beneficiaryAddress);
+
         vm.expectEmit(true, true, true, true);
         emit PerformanceFeeToggled(true);
         vault.setPerformanceFee(true);
@@ -189,12 +199,10 @@ contract MasterVaultFeeTest is MasterVaultCoreTest {
     }
 
     function test_withdrawPerformanceFees_VaultDoubleInAssets() public {
-        vault.setPerformanceFee(true);
         vault.setBeneficiary(beneficiaryAddress);
+        vault.setPerformanceFee(true);
 
-        address _assetsHoldingVault = address(vault.subVault()) == address(0)
-            ? address(vault)
-            : address(vault.subVault());
+        address _assetsHoldingVault = address(vault); // since allocation is 0
 
         vm.startPrank(user);
         token.mint();
@@ -227,7 +235,7 @@ contract MasterVaultFeeTest is MasterVaultCoreTest {
         uint256 beneficiaryBalanceBefore = token.balanceOf(beneficiaryAddress);
 
         vm.expectEmit(true, true, true, true);
-        emit PerformanceFeesWithdrawn(beneficiaryAddress, depositAmount);
+        emit PerformanceFeesWithdrawn(beneficiaryAddress, depositAmount, 0);
         vault.distributePerformanceFee();
 
         assertEq(
@@ -244,7 +252,7 @@ contract MasterVaultFeeTest is MasterVaultCoreTest {
 
     event PerformanceFeeToggled(bool enabled);
     event BeneficiaryUpdated(address indexed oldBeneficiary, address indexed newBeneficiary);
-    event PerformanceFeesWithdrawn(address indexed beneficiary, uint256 amount);
+    event PerformanceFeesWithdrawn(address indexed beneficiary, uint256 amountTransferred, uint256 amountWithdrawn);
 }
 
 contract MasterVaultFeeTestWithSubvaultFresh is MasterVaultFeeTest {