From f315185e62b63f718857a1e0edbf961a18866ac5 Mon Sep 17 00:00:00 2001
From: Schlag <89420541+Schlagonia@users.noreply.github.com>
Date: Wed, 28 Jun 2023 12:17:40 -0600
Subject: [PATCH] fix: yaudit fixes (#171)

* fix: comments on profit update

* chore: round losses up

* chore: add receiver check

* build: add internal mint

* fix: simple rounding

* feat: simple transfers

* build: fix withdraw (#13)

* build: fix withdraw

* chore: efficiency

* chore: add max loss

* test: max loss add

* feat: redeem max loss

* chore: rounding and redeem fixes

* test: e2e strategy test

* fix: check for over withdaw

* fix: use real values

* fix: typos

* fix: extra underflow check
---
 contracts/VaultFactory.vy                   |   6 +-
 contracts/VaultV3.vy                        | 180 +++++---
 tests/e2e/test_profitable_strategy_flow.py  |  14 +-
 tests/e2e/test_strategy_withdraw_flow.py    |   7 +-
 tests/unit/vault/test_emergency_shutdown.py |   4 +-
 tests/unit/vault/test_profit_unlocking.py   |  92 ++--
 tests/unit/vault/test_queue_management.py   |   4 +-
 tests/unit/vault/test_shares.py             |   2 +-
 tests/unit/vault/test_strategy_withdraw.py  | 464 +++++++++++++++++++-
 9 files changed, 616 insertions(+), 157 deletions(-)

diff --git a/contracts/VaultFactory.vy b/contracts/VaultFactory.vy
index 2ea35ea8..4f4a79ca 100644
--- a/contracts/VaultFactory.vy
+++ b/contracts/VaultFactory.vy
@@ -14,7 +14,7 @@
     initialized fully on-chain with their init byte code, thus not 
     requiring any delegatecall patterns or post deployment initialization.
     The deployments are done through create2 with a specific `salt` 
-    that is dereived from a combination of the deployers address,
+    that is derived from a combination of the deployers address,
     the underlying asset used, as well as the name and symbol specified.
     Meaning a deployer will not be able to deploy the exact same vault
     twice and will need to use different name and or symbols for vaults
@@ -91,7 +91,7 @@ name: public(String[64])
 default_protocol_fee_config: public(PFConfig)
 # Custom fee to charge for a specific vault or strategy.
 custom_protocol_fee: public(HashMap[address, uint16])
-# Repersents if a custom protocol fee should be used.
+# Represents if a custom protocol fee should be used.
 use_custom_protocol_fee: public(HashMap[address, bool])
 
 @external
@@ -158,7 +158,7 @@ def api_version() -> String[28]:
 def protocol_fee_config() -> PFConfig:
     """
     @notice Called during vault and strategy reports 
-    to retreive the protocol fee to charge and address
+    to retrieve the protocol fee to charge and address
     to receive the fees.
     @return The protocol fee config for the msg sender.
     """
diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy
index eb6ac4d6..214f4386 100644
--- a/contracts/VaultV3.vy
+++ b/contracts/VaultV3.vy
@@ -42,10 +42,12 @@ interface IStrategy:
     def maxDeposit(receiver: address) -> uint256: view
     def maxWithdraw(owner: address) -> uint256: view
     def withdraw(amount: uint256, receiver: address, owner: address) -> uint256: nonpayable
+    def redeem(shares: uint256, receiver: address, owner: address) -> uint256: nonpayable
     def deposit(assets: uint256, receiver: address) -> uint256: nonpayable
     def totalAssets() -> (uint256): view
-    def convertToAssets(shares: uint256) -> (uint256): view
-    def convertToShares(assets: uint256) -> (uint256): view
+    def convertToAssets(shares: uint256) -> uint256: view
+    def convertToShares(assets: uint256) -> uint256: view
+    def previewWithdraw(assets: uint256) -> uint256: view
 
 interface IAccountant:
     def report(strategy: address, gain: uint256, loss: uint256) -> (uint256, uint256): nonpayable
@@ -162,19 +164,19 @@ API_VERSION: constant(String[28]) = "3.0.1-beta"
 
 # ENUMS #
 # Each permissioned function has its own Role.
-# Roles can be combined in any combination or all kept seperate.
+# Roles can be combined in any combination or all kept separate.
 # Follows python Enum patterns so the first Enum == 1 and doubles each time.
 enum Roles:
     ADD_STRATEGY_MANAGER # Can add strategies to the vault.
     REVOKE_STRATEGY_MANAGER # Can remove strategies from the vault.
     FORCE_REVOKE_MANAGER # Can force remove a strategy causing a loss.
-    ACCOUNTANT_MANAGER # Can set the accountant that assesss fees.
+    ACCOUNTANT_MANAGER # Can set the accountant that assess fees.
     QUEUE_MANAGER # Can set the default withdrawal queue.
     REPORTING_MANAGER # Calls report for strategies.
     DEBT_MANAGER # Adds and removes debt from strategies.
     MAX_DEBT_MANAGER # Can set the max debt for a strategy.
     DEPOSIT_LIMIT_MANAGER # Sets deposit limit for the vault.
-    MINIMUM_IDLE_MANAGER # Sets the minimun total idle the vault should keep.
+    MINIMUM_IDLE_MANAGER # Sets the minimum total idle the vault should keep.
     PROFIT_UNLOCK_MANAGER # Sets the profit_max_unlock_time.
     DEBT_PURCHASER # Can purchase bad debt from the vault.
     EMERGENCY_MANAGER # Can shutdown vault in an emergency.
@@ -196,7 +198,7 @@ enum RoleStatusChange:
 ASSET: immutable(ERC20)
 # Based off the `asset` decimals.
 DECIMALS: immutable(uint256)
-# Deployer contract used to retreive the protocol fee config.
+# Deployer contract used to retrieve the protocol fee config.
 FACTORY: public(immutable(address))
 
 # STORAGE #
@@ -416,7 +418,7 @@ def _burn_unlocked_shares():
     # Get the amount of shares that have unlocked
     unlocked_shares: uint256 = self._unlocked_shares()
 
-    # IF 0 theres nothing to do.
+    # IF 0 there's nothing to do.
     if unlocked_shares == 0:
         return
 
@@ -477,56 +479,23 @@ def _convert_to_shares(assets: uint256, rounding: Rounding) -> uint256:
 
     return shares
 
-
 @internal
 def _erc20_safe_approve(token: address, spender: address, amount: uint256):
-    # Used only to send tokens that are not the type managed by this Vault.
-    # HACK: Used to handle non-compliant tokens like USDT
-    response: Bytes[32] = raw_call(
-        token,
-        concat(
-            method_id("approve(address,uint256)"),
-            convert(spender, bytes32),
-            convert(amount, bytes32),
-        ),
-        max_outsize=32,
-    )
-    if len(response) > 0:
-        assert convert(response, bool), "Transfer failed!"
-
+    # Used only to approve tokens that are not the type managed by this Vault.
+    # Used to handle non-compliant tokens like USDT
+    assert ERC20(token).approve(spender, amount, default_return_value=True), "approval failed"
 
 @internal
 def _erc20_safe_transfer_from(token: address, sender: address, receiver: address, amount: uint256):
-    # Used only to send tokens that are not the type managed by this Vault.
-    # HACK: Used to handle non-compliant tokens like USDT
-    response: Bytes[32] = raw_call(
-        token,
-        concat(
-            method_id("transferFrom(address,address,uint256)"),
-            convert(sender, bytes32),
-            convert(receiver, bytes32),
-            convert(amount, bytes32),
-        ),
-        max_outsize=32,
-    )
-    if len(response) > 0:
-        assert convert(response, bool), "Transfer failed!"
+    # Used only to transfer tokens that are not the type managed by this Vault.
+    # Used to handle non-compliant tokens like USDT
+    assert ERC20(token).transferFrom(sender, receiver, amount, default_return_value=True), "transfer failed"
 
 @internal
 def _erc20_safe_transfer(token: address, receiver: address, amount: uint256):
     # Used only to send tokens that are not the type managed by this Vault.
-    # HACK: Used to handle non-compliant tokens like USDT
-    response: Bytes[32] = raw_call(
-        token,
-        concat(
-            method_id("transfer(address,uint256)"),
-            convert(receiver, bytes32),
-            convert(amount, bytes32),
-        ),
-        max_outsize=32,
-    )
-    if len(response) > 0:
-        assert convert(response, bool), "Transfer failed!"
+    # Used to handle non-compliant tokens like USDT
+    assert ERC20(token).transfer(receiver, amount, default_return_value=True), "transfer failed"
 
 @internal
 def _issue_shares(shares: uint256, recipient: address):
@@ -593,9 +562,9 @@ def _max_withdraw(owner: address) -> uint256:
 @internal
 def _deposit(sender: address, recipient: address, assets: uint256) -> uint256:
     """
-    Used for `deposit` and `mint` calls to transfer the amoutn of `asset` to
-    the vault, issue the corresponding shares to the `recipient` and update 
-    all needed vault accounting.
+    Used for `deposit` calls to transfer the amount of `asset` to the vault, 
+    issue the corresponding shares to the `recipient` and update all needed 
+    vault accounting.
     """
     assert self.shutdown == False # dev: shutdown
     assert recipient not in [self, empty(address)], "invalid recipient"
@@ -614,6 +583,32 @@ def _deposit(sender: address, recipient: address, assets: uint256) -> uint256:
     log Deposit(sender, recipient, assets, shares)
     return shares
 
+@internal
+def _mint(sender: address, recipient: address, shares: uint256) -> uint256:
+    """
+    Used for `mint` calls to issue the corresponding shares to the `recipient`,
+    transfer the amount of `asset` to the vault, and update all needed vault 
+    accounting.
+    """
+    assert self.shutdown == False # dev: shutdown
+    assert recipient not in [self, empty(address)], "invalid recipient"
+
+    assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_UP)
+
+    assert assets > 0, "cannot mint zero"
+    assert self._total_assets() + assets <= self.deposit_limit, "exceed deposit limit"
+
+    # Transfer the tokens to the vault first.
+    self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, assets)
+    # Record the change in total assets.
+    self.total_idle += assets
+    
+    # Issue the corresponding shares for assets.
+    self._issue_shares(shares, recipient)
+
+    log Deposit(sender, recipient, assets, shares)
+    return assets
+
 @view
 @internal
 def _assess_share_of_unrealised_losses(strategy: address, assets_needed: uint256) -> uint256:
@@ -635,7 +630,11 @@ def _assess_share_of_unrealised_losses(strategy: address, assets_needed: uint256
     # Users will withdraw assets_to_withdraw divided by loss ratio (strategy_assets / strategy_current_debt - 1),
     # but will only receive assets_to_withdraw.
     # NOTE: If there are unrealised losses, the user will take his share.
-    losses_user_share: uint256 = assets_needed - assets_needed * strategy_assets / strategy_current_debt
+    numerator: uint256 = assets_needed * strategy_assets
+    losses_user_share: uint256 = assets_needed - numerator / strategy_current_debt
+    # Always round up.
+    if numerator % strategy_current_debt != 0:
+        losses_user_share += 1
 
     return losses_user_share
 
@@ -643,12 +642,14 @@ def _assess_share_of_unrealised_losses(strategy: address, assets_needed: uint256
 def _redeem(
     sender: address, 
     receiver: address, 
-    owner: address, 
+    owner: address,
+    assets: uint256,
     shares_to_burn: uint256, 
+    max_loss: uint256,
     strategies: DynArray[address, MAX_QUEUE]
 ) -> uint256:
     """
-    This will attempt to free up the full amount of assets equivalant to
+    This will attempt to free up the full amount of assets equivalent to
     `shares_to_burn` and transfer them to the `receiver`. If the vault does
     not have enough idle funds it will go through any strategies provided by
     either the withdrawer or the queue_manaager to free up enough funds to 
@@ -660,6 +661,8 @@ def _redeem(
     Any losses realized during the withdraw from a strategy will be passed on
     to the user that is redeeming their vault shares.
     """
+    assert receiver != empty(address), "ZERO ADDRESS"
+
     shares: uint256 = shares_to_burn
     shares_balance: uint256 = self.balance_of[owner]
 
@@ -670,7 +673,7 @@ def _redeem(
         self._spend_allowance(owner, sender, shares_to_burn)
 
     # The amount of the underlying token to withdraw.
-    requested_assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_DOWN)
+    requested_assets: uint256 = assets
 
     # load to memory to save gas
     curr_total_idle: uint256 = self.total_idle
@@ -690,7 +693,7 @@ def _redeem(
         # load to memory to save gas
         curr_total_debt: uint256 = self.total_debt
 
-        # Withdraw from strategies only what idle doesnt cover.
+        # Withdraw from strategies only what idle doesn't cover.
         # `assets_needed` is the total amount we need to fill the request.
         assets_needed: uint256 = requested_assets - curr_total_idle
         # `assets_to_withdraw` is the amount to request from the current strategy.
@@ -757,13 +760,32 @@ def _redeem(
                 continue
             
             # WITHDRAW FROM STRATEGY
-            IStrategy(strategy).withdraw(assets_to_withdraw, self, self)
+            # Need to get shares since we use redeem to be able to take on losses.
+            shares_to_withdraw: uint256 = min(
+                # Use previewWithdraw since it should round up.
+                IStrategy(strategy).previewWithdraw(assets_to_withdraw), 
+                # And check against our actual balance.
+                IStrategy(strategy).balanceOf(self)
+            )
+            IStrategy(strategy).redeem(shares_to_withdraw, self, self)
             post_balance: uint256 = ASSET.balanceOf(self)
             
+            # Always check withdrawn against the real amounts.
+            withdrawn: uint256 = post_balance - previous_balance
             loss: uint256 = 0
+            # Check if we redeemed to much.
+            if withdrawn > assets_to_withdraw:
+                # Make sure we don't underlfow in debt updates.
+                if withdrawn > current_debt:
+                    # Can't withdraw more than our debt.
+                    assets_to_withdraw = current_debt
+                else:
+                    # Add the extra to how much we withdrew.
+                    assets_to_withdraw += (withdrawn - assets_to_withdraw)
+
             # If we have not received what we expected, we consider the difference a loss.
-            if(previous_balance + assets_to_withdraw > post_balance):
-                loss = previous_balance + assets_to_withdraw - post_balance
+            elif withdrawn < assets_to_withdraw:
+                loss = assets_to_withdraw - withdrawn
 
             # NOTE: strategy's debt decreases by the full amount but the total idle increases 
             # by the actual amount only (as the difference is considered lost).
@@ -786,7 +808,8 @@ def _redeem(
             # We update the previous_balance variable here to save gas in next iteration.
             previous_balance = post_balance
 
-            # Reduce what we still need.
+            # Reduce what we still need. Safe to use assets_to_withdraw 
+            # here since it has been checked against requested_assets
             assets_needed -= assets_to_withdraw
 
         # If we exhaust the queue and still have insufficient total idle, revert.
@@ -794,6 +817,11 @@ def _redeem(
         # Commit memory to storage.
         self.total_debt = curr_total_debt
 
+    # Check if there is a loss and a non-default value was set.
+    if assets > requested_assets and max_loss < MAX_BPS:
+        # The loss is within the allowed range.
+        assert assets - requested_assets <= assets * max_loss / MAX_BPS, "to much loss"
+
     # First burn the corresponding shares from the redeemer.
     self._burn_shares(shares, owner)
     # Commit memory to storage.
@@ -865,7 +893,7 @@ def _revoke_strategy(strategy: address, force: bool=False):
 @internal
 def _update_debt(strategy: address, target_debt: uint256) -> uint256:
     """
-    The vault will rebalance the debt vs target debt. Target debt must be
+    The vault will re-balance the debt vs target debt. Target debt must be
     smaller or equal to strategy's max_debt. This function will compare the 
     current debt with the target debt and will take funds or deposit new 
     funds to the strategy. 
@@ -1422,7 +1450,7 @@ def revoke_strategy(strategy: address):
 def force_revoke_strategy(strategy: address):
     """
     @notice Force revoke a strategy.
-    @dev The vault will remove the inputed strategy and write off any debt left 
+    @dev The vault will remove the strategy and write off any debt left 
         in it as a loss. This function is a dangerous function as it can force a 
         strategy to take a loss. All possible assets should be removed from the 
         strategy first via update_debt. If a strategy is removed erroneously it 
@@ -1500,9 +1528,7 @@ def mint(shares: uint256, receiver: address) -> uint256:
     @param receiver The address to receive the shares.
     @return The amount of assets deposited.
     """
-    assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_UP)
-    self._deposit(msg.sender, receiver, assets)
-    return assets
+    return self._mint(msg.sender, receiver, shares)
 
 @external
 @nonreentrant("lock")
@@ -1510,18 +1536,21 @@ def withdraw(
     assets: uint256, 
     receiver: address, 
     owner: address, 
+    max_loss: uint256 = 0,
     strategies: DynArray[address, MAX_QUEUE] = []
 ) -> uint256:
     """
     @notice Withdraw an amount of asset to `receiver` burning `owner`s shares.
+    @dev The default behavior is to not allow any loss.
     @param assets The amount of asset to withdraw.
     @param receiver The address to receive the assets.
-    @param owner The address whos shares are being burnt.
+    @param owner The address who's shares are being burnt.
+    @param max_loss Optional amount of acceptable loss in Basis Points.
     @param strategies Optional array of strategies to withdraw from.
     @return The amount of shares actually burnt.
     """
     shares: uint256 = self._convert_to_shares(assets, Rounding.ROUND_UP)
-    self._redeem(msg.sender, receiver, owner, shares, strategies)
+    self._redeem(msg.sender, receiver, owner, assets, shares, max_loss, strategies)
     return shares
 
 @external
@@ -1530,18 +1559,23 @@ def redeem(
     shares: uint256, 
     receiver: address, 
     owner: address, 
+    max_loss: uint256 = MAX_BPS,
     strategies: DynArray[address, MAX_QUEUE] = []
 ) -> uint256:
     """
     @notice Redeems an amount of shares of `owners` shares sending funds to `receiver`.
+    @dev The default behavior is to allow losses to be realized.
     @param shares The amount of shares to burn.
     @param receiver The address to receive the assets.
-    @param owner The address whos shares are being burnt.
+    @param owner The address who's shares are being burnt.
+    @param max_loss Optional amount of acceptable loss in Basis Points.
     @param strategies Optional array of strategies to withdraw from.
     @return The amount of assets actually withdrawn.
     """
-    assets: uint256 = self._redeem(msg.sender, receiver, owner, shares, strategies)
-    return assets
+    assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_DOWN)
+    # Always return the actual amount of assets withdrawn.
+    return self._redeem(msg.sender, receiver, owner, assets, shares, max_loss, strategies)
+
 
 @external
 def approve(spender: address, amount: uint256) -> bool:
@@ -1846,10 +1880,10 @@ def profitUnlockingRate() -> uint256:
 
 @view
 @external
-def lastReport() -> uint256:
+def lastProfitUpdate() -> uint256:
     """
-    @notice The timestamp of the last time protocol fees were charged.
-    @return The last report.
+    @notice The timestamp of the last time shares were locked.
+    @return The last profit update.
     """
     return self.last_profit_update
 
diff --git a/tests/e2e/test_profitable_strategy_flow.py b/tests/e2e/test_profitable_strategy_flow.py
index 2b06c886..cf82f286 100644
--- a/tests/e2e/test_profitable_strategy_flow.py
+++ b/tests/e2e/test_profitable_strategy_flow.py
@@ -78,7 +78,6 @@ def test_profitable_strategy_flow(
 
     assert vault.totalIdle() == deposit_amount
 
-    strategy.totalAssets()
     add_debt_to_strategy(gov, strategy, vault, strategy.totalAssets() + deposit_amount)
 
     assert vault.totalIdle() == 0
@@ -120,6 +119,10 @@ def test_profitable_strategy_flow(
     assert vault.strategies(strategy).current_debt != new_debt
 
     user_1_withdraw = vault.totalIdle()
+    print(
+        f"Unrealized losses 1= {vault.assess_share_of_unrealised_losses(strategy, user_1_withdraw)}"
+    )
+    print(f"Asset balance 1 {asset.balanceOf(vault.address)}")
     vault.withdraw(user_1_withdraw, user_1, user_1, sender=user_1)
 
     assert pytest.approx(0, abs=1) == vault.totalIdle()
@@ -144,25 +147,21 @@ def test_profitable_strategy_flow(
         - user_1_withdraw
     )
 
-    # we need to use strategies param to take assets from strategies
     vault.redeem(
         vault.balanceOf(user_1),
         user_1,
         user_1,
+        0,
         [strategy.address],
         sender=user_1,
     )
 
-    assert vault.totalIdle() == 0
     assert pytest.approx(0, abs=1) == vault.balanceOf(user_1)
 
     assert asset.balanceOf(user_1) > user_1_initial_balance
 
-    vault.redeem(
-        vault.balanceOf(user_2), user_2, user_2, [strategy.address], sender=user_2
-    )
+    vault.redeem(vault.balanceOf(user_2), user_2, user_2, 0, sender=user_2)
 
-    assert vault.totalIdle() == 0
     assert pytest.approx(0, abs=1) == vault.balanceOf(user_2)
     assert asset.balanceOf(user_2) > user_2_initial_balance
 
@@ -177,5 +176,6 @@ def test_profitable_strategy_flow(
     add_debt_to_strategy(gov, strategy, vault, 0)
 
     assert strategy.totalAssets() == 0
+    assert vault.strategies(strategy).current_debt == 0
     vault.revoke_strategy(strategy, sender=gov)
     assert vault.strategies(strategy).activation == 0
diff --git a/tests/e2e/test_strategy_withdraw_flow.py b/tests/e2e/test_strategy_withdraw_flow.py
index 0af2a0b5..7d53e1b0 100644
--- a/tests/e2e/test_strategy_withdraw_flow.py
+++ b/tests/e2e/test_strategy_withdraw_flow.py
@@ -61,6 +61,7 @@ def test_multiple_strategy_withdraw_flow(
         fish_amount // 2,
         fish.address,
         fish.address,
+        0,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -75,7 +76,7 @@ def test_multiple_strategy_withdraw_flow(
     assert asset.balanceOf(locked_strategy) == locked_strategy_debt
 
     # drain remaining total idle as whale
-    vault.withdraw(current_idle, whale.address, whale.address, [], sender=whale)
+    vault.withdraw(current_idle, whale.address, whale.address, sender=whale)
 
     assert asset.balanceOf(whale) == current_idle
     assert vault.totalIdle() == 0
@@ -89,6 +90,7 @@ def test_multiple_strategy_withdraw_flow(
         fish_amount // 2,
         bunny.address,
         fish.address,
+        0,
         [locked_strategy.address],
         sender=fish,
     )
@@ -110,6 +112,7 @@ def test_multiple_strategy_withdraw_flow(
             whale_balance,
             whale.address,
             whale.address,
+            0,
             [liquid_strategy.address],
             sender=whale,
         )
@@ -119,6 +122,7 @@ def test_multiple_strategy_withdraw_flow(
         whale_balance,
         whale.address,
         whale.address,
+        0,
         [s.address for s in strategies],
         sender=whale,
     )
@@ -143,6 +147,7 @@ def test_multiple_strategy_withdraw_flow(
         amount_to_lock,
         whale.address,
         whale.address,
+        0,
         [s.address for s in strategies],
         sender=whale,
     )
diff --git a/tests/unit/vault/test_emergency_shutdown.py b/tests/unit/vault/test_emergency_shutdown.py
index e94d7540..6688cefd 100644
--- a/tests/unit/vault/test_emergency_shutdown.py
+++ b/tests/unit/vault/test_emergency_shutdown.py
@@ -62,9 +62,7 @@ def test_shutdown_cant_deposit_can_withdraw(
 
     assert vault_balance_before == asset.balanceOf(vault)
     gov_balance_before = asset.balanceOf(gov)
-    vault.withdraw(
-        vault.balanceOf(gov.address), gov.address, gov.address, [], sender=gov
-    )
+    vault.withdraw(vault.balanceOf(gov.address), gov.address, gov.address, sender=gov)
     assert asset.balanceOf(gov) == gov_balance_before + vault_balance_before
     assert asset.balanceOf(vault) == 0
 
diff --git a/tests/unit/vault/test_profit_unlocking.py b/tests/unit/vault/test_profit_unlocking.py
index c9dc9588..0c892a13 100644
--- a/tests/unit/vault/test_profit_unlocking.py
+++ b/tests/unit/vault/test_profit_unlocking.py
@@ -125,7 +125,7 @@ def test_gain_no_fees_no_refunds_no_existing_buffer(
     )
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -253,7 +253,7 @@ def test_gain_no_fees_with_refunds_no_buffer(
     assert vault.strategies(strategy).current_debt == 0
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -268,7 +268,7 @@ def test_gain_no_fees_with_refunds_no_buffer(
     # Accountant redeems shares
     with reverts("no shares to redeem"):
         vault.redeem(
-            vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
+            vault.balanceOf(accountant), accountant, accountant, sender=accountant
         )
 
 
@@ -405,7 +405,7 @@ def test_gain_no_fees_with_refunds_with_buffer(
     total_shares = vault.totalSupply()
 
     # Fish redeems shares
-    vault.redeem(shares_fish, fish, fish, [], sender=fish)
+    vault.redeem(shares_fish, fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -423,7 +423,7 @@ def test_gain_no_fees_with_refunds_with_buffer(
     # Accountant redeems shares
     with reverts("no shares to redeem"):
         vault.redeem(
-            vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
+            vault.balanceOf(accountant), accountant, accountant, sender=accountant
         )
 
 
@@ -516,7 +516,7 @@ def test_gain_no_fees_no_refunds_with_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -605,7 +605,7 @@ def test_gain_fees_no_refunds_no_existing_buffer(
     )
 
     # Fish redeems shares
-    tx = vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    tx = vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
     withdraw_assets = list(tx.decode_logs(vault.Withdraw))[0].assets
     withdrawn_diff = int(
         amount
@@ -630,9 +630,7 @@ def test_gain_fees_no_refunds_no_existing_buffer(
     assert asset.balanceOf(fish) < fish_amount + first_profit
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault, total_debt=0, total_idle=0, total_assets=0, total_supply=0
@@ -721,7 +719,7 @@ def test_gain_fees_refunds_no_existing_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, vault.totalAssets() / vault.balanceOf(accountant))
     check_vault_totals(
@@ -750,9 +748,7 @@ def test_gain_fees_refunds_no_existing_buffer(
     )
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
     check_vault_totals(
         vault, total_debt=0, total_idle=0, total_assets=0, total_supply=0
     )
@@ -898,7 +894,7 @@ def test_gain_fees_with_refunds_with_buffer(
     assert vault.strategies(strategy).current_debt == 0
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     check_vault_totals(
         vault,
@@ -914,9 +910,7 @@ def test_gain_fees_with_refunds_with_buffer(
     ) + second_profit * (1 + refund_ratio / MAX_BPS_ACCOUNTANT)
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
     check_vault_totals(
         vault, total_debt=0, total_idle=0, total_assets=0, total_supply=0
     )
@@ -1077,7 +1071,7 @@ def test_gain_fees_no_refunds_with_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, vault.totalAssets() / vault.balanceOf(accountant))
 
@@ -1095,9 +1089,7 @@ def test_gain_fees_no_refunds_with_buffer(
     assert asset.balanceOf(fish) < fish_amount + first_profit + second_profit
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
     check_vault_totals(
         vault, total_debt=0, total_idle=0, total_assets=0, total_supply=0
     )
@@ -1249,7 +1241,7 @@ def test_gain_fees_no_refunds_not_enough_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     check_vault_totals(
         vault,
@@ -1260,9 +1252,7 @@ def test_gain_fees_no_refunds_not_enough_buffer(
     )
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
     check_vault_totals(
         vault, total_debt=0, total_idle=0, total_assets=0, total_supply=0
     )
@@ -1334,7 +1324,7 @@ def test_loss_no_fees_no_refunds_no_existing_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -1392,7 +1382,7 @@ def test_loss_fees_no_refunds_no_existing_buffer(
     add_debt_to_strategy(gov, strategy, vault, 0)
 
     # Fish redeems shares
-    tx = vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    tx = vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     check_vault_totals(
         vault,
@@ -1405,9 +1395,7 @@ def test_loss_fees_no_refunds_no_existing_buffer(
     assert asset.balanceOf(fish) < fish_amount - first_loss
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault,
@@ -1490,7 +1478,7 @@ def test_loss_no_fees_refunds_no_existing_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -1623,7 +1611,7 @@ def test_loss_no_fees_with_refunds_with_buffer(
     assert vault.strategies(strategy).current_debt == 0
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -1643,7 +1631,7 @@ def test_loss_no_fees_with_refunds_with_buffer(
     # Accountant redeems shares
     with reverts("no shares to redeem"):
         vault.redeem(
-            vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
+            vault.balanceOf(accountant), accountant, accountant, sender=accountant
         )
 
 
@@ -1767,7 +1755,7 @@ def test_loss_no_fees_no_refunds_with_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -1899,7 +1887,7 @@ def test_loss_fees_no_refunds_with_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     check_vault_totals(
         vault,
@@ -1916,9 +1904,7 @@ def test_loss_fees_no_refunds_with_buffer(
     assert asset.balanceOf(fish) > fish_amount
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault,
@@ -2030,7 +2016,7 @@ def test_loss_no_fees_no_refunds_with_not_enough_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
 
@@ -2168,7 +2154,7 @@ def test_loss_fees_no_refunds_with_not_enough_buffer(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     check_vault_totals(
         vault,
@@ -2182,9 +2168,7 @@ def test_loss_fees_no_refunds_with_not_enough_buffer(
     assert asset.balanceOf(fish) < fish_amount + first_profit - first_loss
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault,
@@ -2262,7 +2246,7 @@ def test_loss_fees_refunds(
     )
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 0.99)
     check_vault_totals(
@@ -2278,9 +2262,7 @@ def test_loss_fees_refunds(
     )
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault,
@@ -2418,7 +2400,7 @@ def test_loss_fees_refunds_with_buffer(
     assert vault.strategies(strategy).current_debt == 0
 
     # Fish redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert pytest.approx(vault.totalSupply(), 1e-4) == total_fees + total_second_fees
 
@@ -2428,9 +2410,7 @@ def test_loss_fees_refunds_with_buffer(
     assert asset.balanceOf(fish) > fish_amount
 
     # Accountant redeems shares
-    vault.redeem(
-        vault.balanceOf(accountant), accountant, accountant, [], sender=accountant
-    )
+    vault.redeem(vault.balanceOf(accountant), accountant, accountant, sender=accountant)
 
     check_vault_totals(
         vault,
@@ -2565,7 +2545,7 @@ def test_increase_profit_max_period__no_change(
     )
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -2634,7 +2614,7 @@ def test_decrease_profit_max_period__no_change(
     )
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -2739,7 +2719,7 @@ def test_increase_profit_max_period__next_report_works(
     )
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
@@ -2844,7 +2824,7 @@ def test_decrease_profit_max_period__next_report_works(
     )
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert_price_per_share(vault, 1.0)
     check_vault_totals(
diff --git a/tests/unit/vault/test_queue_management.py b/tests/unit/vault/test_queue_management.py
index 9cdfdd6c..c9244f54 100644
--- a/tests/unit/vault/test_queue_management.py
+++ b/tests/unit/vault/test_queue_management.py
@@ -135,6 +135,7 @@ def test_withdraw__queue__with_inactive_strategy__reverts(
             shares,
             fish.address,
             fish.address,
+            0,
             strategies,
             sender=fish,
         )
@@ -170,7 +171,7 @@ def test_withdraw__queue__with_liquid_strategy__withdraws(
     add_strategy_to_vault(gov, strategy, vault)
     add_debt_to_strategy(gov, strategy, vault, amount)
 
-    tx = vault.withdraw(shares, fish.address, fish.address, strategies, sender=fish)
+    tx = vault.withdraw(shares, fish.address, fish.address, 0, strategies, sender=fish)
     event = list(tx.decode_logs(vault.Withdraw))
 
     assert len(event) >= 1
@@ -223,6 +224,7 @@ def test_withdraw__queue__with_inactive_strategy__reverts(
             shares,
             fish.address,
             fish.address,
+            0,
             [s.address for s in strategies],
             sender=fish,
         )
diff --git a/tests/unit/vault/test_shares.py b/tests/unit/vault/test_shares.py
index 34b86efe..a45c4693 100644
--- a/tests/unit/vault/test_shares.py
+++ b/tests/unit/vault/test_shares.py
@@ -477,7 +477,7 @@ def test__mint_shares_with_zero_total_supply_positive_assets(
     )  # there are more shares than deposits (due to profit unlock)
 
     # User redeems shares
-    vault.redeem(vault.balanceOf(fish), fish, fish, [], sender=fish)
+    vault.redeem(vault.balanceOf(fish), fish, fish, sender=fish)
 
     assert vault.totalSupply() > 0
 
diff --git a/tests/unit/vault/test_strategy_withdraw.py b/tests/unit/vault/test_strategy_withdraw.py
index 6a54e672..6d45158a 100644
--- a/tests/unit/vault/test_strategy_withdraw.py
+++ b/tests/unit/vault/test_strategy_withdraw.py
@@ -21,6 +21,7 @@ def test_withdraw__with_inactive_strategy__reverts(
     strategy = create_strategy(vault)
     inactive_strategy = create_strategy(vault)
     strategies = [inactive_strategy]
+    max_loss = 0
 
     vault.set_role(
         gov.address,
@@ -36,6 +37,7 @@ def test_withdraw__with_inactive_strategy__reverts(
             shares,
             fish.address,
             fish.address,
+            max_loss,
             [s.address for s in strategies],
             sender=fish,
         )
@@ -57,6 +59,7 @@ def test_withdraw__with_liquid_strategy__withdraws(
     shares = amount
     strategy = create_strategy(vault)
     strategies = [strategy]
+    max_loss = 0
 
     vault.set_role(
         gov.address,
@@ -68,7 +71,12 @@ def test_withdraw__with_liquid_strategy__withdraws(
     add_debt_to_strategy(gov, strategy, vault, amount)
 
     tx = vault.withdraw(
-        shares, fish.address, fish.address, [s.address for s in strategies], sender=fish
+        shares,
+        fish.address,
+        fish.address,
+        max_loss,
+        [s.address for s in strategies],
+        sender=fish,
     )
     event = list(tx.decode_logs(vault.Withdraw))
 
@@ -111,6 +119,7 @@ def test_withdraw__with_multiple_liquid_strategies__withdraws(
     first_strategy = create_strategy(vault)
     second_strategy = create_strategy(vault)
     strategies = [first_strategy, second_strategy]
+    max_loss = 0
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -126,7 +135,12 @@ def test_withdraw__with_multiple_liquid_strategies__withdraws(
         add_debt_to_strategy(gov, strategy, vault, amount_per_strategy)
 
     tx = vault.withdraw(
-        shares, fish.address, fish.address, [s.address for s in strategies], sender=fish
+        shares,
+        fish.address,
+        fish.address,
+        max_loss,
+        [s.address for s in strategies],
+        sender=fish,
     )
     event = list(tx.decode_logs(vault.Withdraw))
 
@@ -175,6 +189,7 @@ def test_withdraw__locked_funds_with_locked_and_liquid_strategy__reverts(
     liquid_strategy = create_strategy(vault)
     locked_strategy = create_locked_strategy(vault)
     strategies = [locked_strategy, liquid_strategy]
+    max_loss = 0
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -197,6 +212,7 @@ def test_withdraw__locked_funds_with_locked_and_liquid_strategy__reverts(
             amount_to_withdraw,
             fish.address,
             fish.address,
+            max_loss,
             [s.address for s in strategies],
             sender=fish,
         )
@@ -223,6 +239,7 @@ def test_withdraw__with_locked_and_liquid_strategy__withdraws(
     liquid_strategy = create_strategy(vault)
     locked_strategy = create_locked_strategy(vault)
     strategies = [locked_strategy, liquid_strategy]
+    max_loss = 0
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -241,7 +258,12 @@ def test_withdraw__with_locked_and_liquid_strategy__withdraws(
     locked_strategy.setLockedFunds(amount_to_lock, DAY, sender=gov)
 
     tx = vault.withdraw(
-        shares, fish.address, fish.address, [s.address for s in strategies], sender=fish
+        shares,
+        fish.address,
+        fish.address,
+        max_loss,
+        [s.address for s in strategies],
+        sender=fish,
     )
     event = list(tx.decode_logs(vault.Withdraw))
 
@@ -273,6 +295,52 @@ def test_withdraw__with_locked_and_liquid_strategy__withdraws(
     assert asset.balanceOf(fish) == amount_to_withdraw
 
 
+def test_withdraw__with_lossy_strategy__no_max_loss__reverts(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount
+    amount_to_lose = amount_per_strategy // 2  # loss only half of strategy
+    amount_to_withdraw = amount  # withdraw full deposit
+    shares = amount
+    lossy_strategy = create_lossy_strategy(vault)
+    max_loss = 0
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    add_strategy_to_vault(gov, lossy_strategy, vault)
+    add_debt_to_strategy(gov, lossy_strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
+
+    with ape.reverts("to much loss"):
+        vault.withdraw(
+            amount_to_withdraw,
+            fish.address,
+            fish.address,
+            max_loss,
+            [lossy_strategy.address],
+            sender=fish,
+        )
+
+
 def test_withdraw__with_lossy_strategy__withdraws_less_than_deposited(
     gov,
     fish,
@@ -291,6 +359,7 @@ def test_withdraw__with_lossy_strategy__withdraws_less_than_deposited(
     amount_to_withdraw = amount  # withdraw full deposit
     shares = amount
     lossy_strategy = create_lossy_strategy(vault)
+    max_loss = 5_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -311,6 +380,76 @@ def test_withdraw__with_lossy_strategy__withdraws_less_than_deposited(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
+        [lossy_strategy.address],
+        sender=fish,
+    )
+    event = list(tx.decode_logs(vault.Withdraw))
+
+    assert len(event) >= 1
+    n = len(event) - 1
+    assert event[n].sender == fish
+    assert event[n].receiver == fish
+    assert event[n].owner == fish
+    assert event[n].shares == shares
+    assert event[n].assets == amount_to_withdraw - amount_to_lose
+
+    event = list(tx.decode_logs(vault.DebtUpdated))
+
+    assert len(event) == 1
+    assert event[0].strategy == lossy_strategy.address
+    assert event[0].current_debt == amount_per_strategy
+    assert event[0].new_debt == 0
+
+    assert vault.totalAssets() == 0
+    assert vault.totalSupply() == 0
+    assert vault.totalIdle() == 0
+    assert vault.totalDebt() == 0
+    assert asset.balanceOf(vault) == 0
+    assert asset.balanceOf(lossy_strategy) == 0
+    assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
+
+
+def test_redeem__with_lossy_strategy__withdraws_less_than_deposited(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount
+    amount_to_lose = amount_per_strategy // 2  # loss only half of strategy
+    amount_to_withdraw = amount  # withdraw full deposit
+    shares = amount
+    max_loss = 10_000
+    lossy_strategy = create_lossy_strategy(vault)
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    add_strategy_to_vault(gov, lossy_strategy, vault)
+    add_debt_to_strategy(gov, lossy_strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
+
+    tx = vault.redeem(
+        amount_to_withdraw,
+        fish.address,
+        fish.address,
+        max_loss,
         [lossy_strategy.address],
         sender=fish,
     )
@@ -358,6 +497,7 @@ def test_withdraw__with_full_loss_strategy__withdraws_none(
     amount_to_withdraw = amount  # withdraw full deposit
     shares = amount
     lossy_strategy = create_lossy_strategy(vault)
+    max_loss = 10_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -378,6 +518,76 @@ def test_withdraw__with_full_loss_strategy__withdraws_none(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
+        [lossy_strategy.address],
+        sender=fish,
+    )
+    event = list(tx.decode_logs(vault.Withdraw))
+
+    assert len(event) >= 1
+    n = len(event) - 1
+    assert event[n].sender == fish
+    assert event[n].receiver == fish
+    assert event[n].owner == fish
+    assert event[n].shares == shares
+    assert event[n].assets == amount_to_withdraw - amount_to_lose
+
+    event = list(tx.decode_logs(vault.DebtUpdated))
+
+    assert len(event) == 1
+    assert event[0].strategy == lossy_strategy.address
+    assert event[0].current_debt == amount_per_strategy
+    assert event[0].new_debt == 0
+
+    assert vault.totalAssets() == 0
+    assert vault.totalSupply() == 0
+    assert vault.totalIdle() == 0
+    assert vault.totalDebt() == 0
+    assert asset.balanceOf(vault) == 0
+    assert asset.balanceOf(lossy_strategy) == 0
+    assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
+
+
+def test_redeem__with_full_loss_strategy__withdraws_none(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount
+    amount_to_lose = amount_per_strategy  # loss all of strategy
+    amount_to_withdraw = amount  # withdraw full deposit
+    shares = amount
+    lossy_strategy = create_lossy_strategy(vault)
+    max_loss = 10_000
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    add_strategy_to_vault(gov, lossy_strategy, vault)
+    add_debt_to_strategy(gov, lossy_strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
+
+    tx = vault.redeem(
+        amount_to_withdraw,
+        fish.address,
+        fish.address,
+        max_loss,
         [lossy_strategy.address],
         sender=fish,
     )
@@ -428,6 +638,7 @@ def test_withdraw__with_lossy_and_liquid_strategy__withdraws_less_than_deposited
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 2_500
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -449,6 +660,7 @@ def test_withdraw__with_lossy_and_liquid_strategy__withdraws_less_than_deposited
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -482,7 +694,7 @@ def test_withdraw__with_lossy_and_liquid_strategy__withdraws_less_than_deposited
     assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
 
 
-def test_withdraw__with_full_lossy_and_liquid_strategy__withdraws_less_than_deposited(
+def test_redeem__with_full_lossy_and_liquid_strategy__withdraws_less_than_deposited(
     gov,
     fish,
     fish_amount,
@@ -503,6 +715,7 @@ def test_withdraw__with_full_lossy_and_liquid_strategy__withdraws_less_than_depo
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 10_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -520,10 +733,11 @@ def test_withdraw__with_full_lossy_and_liquid_strategy__withdraws_less_than_depo
     # lose half of assets in lossy strategy
     lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
 
-    tx = vault.withdraw(
+    tx = vault.redeem(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -578,6 +792,7 @@ def test_withdraw__with_liquid_and_lossy_strategy__withdraws_less_than_deposited
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [liquid_strategy, lossy_strategy]
+    max_loss = 2_500
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -599,6 +814,7 @@ def test_withdraw__with_liquid_and_lossy_strategy__withdraws_less_than_deposited
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -632,7 +848,7 @@ def test_withdraw__with_liquid_and_lossy_strategy__withdraws_less_than_deposited
     assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
 
 
-def test_withdraw__with_liquid_and_full_lossy_strategy__withdraws_less_than_deposited(
+def test_redeem__with_liquid_and_full_lossy_strategy__withdraws_less_than_deposited(
     gov,
     fish,
     fish_amount,
@@ -653,6 +869,7 @@ def test_withdraw__with_liquid_and_full_lossy_strategy__withdraws_less_than_depo
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [liquid_strategy, lossy_strategy]
+    max_loss = 10_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -670,10 +887,11 @@ def test_withdraw__with_liquid_and_full_lossy_strategy__withdraws_less_than_depo
     # lose half of assets in lossy strategy
     lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
 
-    tx = vault.withdraw(
+    tx = vault.redeem(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -707,6 +925,56 @@ def test_withdraw__with_liquid_and_full_lossy_strategy__withdraws_less_than_depo
     assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
 
 
+def test_withdraw__with_liquid_and_lossy_strategy_that_losses_while_withdrawing__no_max_loss__reverts(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_strategy,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount // 2  # deposit half of amount per strategy
+    amount_to_lose = amount_per_strategy // 2  # loss only half of strategy
+    amount_to_withdraw = amount  # withdraw full deposit
+    shares = amount
+    liquid_strategy = create_strategy(vault)
+    lossy_strategy = create_lossy_strategy(vault)
+    strategies = [liquid_strategy, lossy_strategy]
+    max_loss = 0
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    for strategy in strategies:
+        add_strategy_to_vault(gov, strategy, vault)
+        add_debt_to_strategy(gov, strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setWithdrawingLoss(amount_to_lose, sender=gov)
+
+    with ape.reverts("to much loss"):
+        tx = vault.withdraw(
+            amount_to_withdraw,
+            fish.address,
+            fish.address,
+            max_loss,
+            [s.address for s in strategies],
+            sender=fish,
+        )
+
+
 def test_withdraw__with_liquid_and_lossy_strategy_that_losses_while_withdrawing__withdraws_less_than_deposited(
     gov,
     fish,
@@ -728,6 +996,7 @@ def test_withdraw__with_liquid_and_lossy_strategy_that_losses_while_withdrawing_
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [liquid_strategy, lossy_strategy]
+    max_loss = 2_500
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -749,6 +1018,7 @@ def test_withdraw__with_liquid_and_lossy_strategy_that_losses_while_withdrawing_
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -782,7 +1052,7 @@ def test_withdraw__with_liquid_and_lossy_strategy_that_losses_while_withdrawing_
     assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
 
 
-def test_withdraw__half_of_assets_from_lossy_strategy_that_losses_while_withdrawing__withdraws_less_than_deposited(
+def test_redeem__half_of_assets_from_lossy_strategy_that_losses_while_withdrawing__withdraws_less_than_deposited(
     gov,
     fish,
     fish_amount,
@@ -803,6 +1073,7 @@ def test_withdraw__half_of_assets_from_lossy_strategy_that_losses_while_withdraw
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 10_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -820,10 +1091,11 @@ def test_withdraw__half_of_assets_from_lossy_strategy_that_losses_while_withdraw
     # lose half of assets in lossy strategy
     lossy_strategy.setWithdrawingLoss(amount_to_lose, sender=gov)
 
-    tx = vault.withdraw(
+    tx = vault.redeem(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -856,6 +1128,108 @@ def test_withdraw__half_of_assets_from_lossy_strategy_that_losses_while_withdraw
     assert asset.balanceOf(fish) == amount_to_withdraw - amount_to_lose
 
 
+def test_redeem__half_of_assets_from_lossy_strategy_that_losses_while_withdrawing__custom_max_loss__reverts(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_strategy,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount // 2  # deposit half of amount per strategy
+    amount_to_lose = amount_per_strategy // 4  # loss only quarter of strategy
+    amount_to_withdraw = amount // 2  # withdraw half deposit
+    shares = amount
+    liquid_strategy = create_strategy(vault)
+    lossy_strategy = create_lossy_strategy(vault)
+    strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 0
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    for strategy in strategies:
+        add_strategy_to_vault(gov, strategy, vault)
+        add_debt_to_strategy(gov, strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setWithdrawingLoss(amount_to_lose, sender=gov)
+
+    with ape.reverts("to much loss"):
+        vault.redeem(
+            amount_to_withdraw,
+            fish.address,
+            fish.address,
+            max_loss,
+            [s.address for s in strategies],
+            sender=fish,
+        )
+
+
+def test_withdraw__half_of_strategy_assets_from_lossy_strategy_with_unrealised_losses__no_max_fee__reverts(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_strategy,
+    create_lossy_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount // 2  # deposit half of amount per strategy
+    amount_to_lose = amount_per_strategy // 2  # loss only half of strategy
+    amount_to_withdraw = (
+        amount // 4
+    )  # withdraw a quarter deposit (half of strategy debt)
+    shares = amount
+    liquid_strategy = create_strategy(vault)
+    lossy_strategy = create_lossy_strategy(vault)
+    strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 0
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    for strategy in strategies:
+        add_strategy_to_vault(gov, strategy, vault)
+        add_debt_to_strategy(gov, strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    lossy_strategy.setLoss(gov, amount_to_lose, sender=gov)
+
+    with ape.reverts("to much loss"):
+        tx = vault.withdraw(
+            amount_to_withdraw,
+            fish.address,
+            fish.address,
+            max_loss,
+            [s.address for s in strategies],
+            sender=fish,
+        )
+
+
 def test_withdraw__half_of_strategy_assets_from_lossy_strategy_with_unrealised_losses__withdraws_less_than_deposited(
     gov,
     fish,
@@ -879,6 +1253,7 @@ def test_withdraw__half_of_strategy_assets_from_lossy_strategy_with_unrealised_l
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_lossy_strategy(vault)
     strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 5_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -900,6 +1275,7 @@ def test_withdraw__half_of_strategy_assets_from_lossy_strategy_with_unrealised_l
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -936,7 +1312,7 @@ def test_withdraw__half_of_strategy_assets_from_lossy_strategy_with_unrealised_l
     assert vault.balanceOf(fish) == amount - amount_to_withdraw
 
 
-def test_withdraw__half_of_strategy_assets_from_locked_lossy_strategy_with_unrealised_losses__withdraws_less_than_deposited(
+def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unrealised_losses__withdraws_less_than_deposited(
     gov,
     fish,
     fish_amount,
@@ -960,6 +1336,7 @@ def test_withdraw__half_of_strategy_assets_from_locked_lossy_strategy_with_unrea
     liquid_strategy = create_strategy(vault)
     lossy_strategy = create_locked_strategy(vault)
     strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 10_000
 
     # deposit assets to vault
     user_deposit(fish, vault, asset, amount)
@@ -979,10 +1356,11 @@ def test_withdraw__half_of_strategy_assets_from_locked_lossy_strategy_with_unrea
     # Lock half the remaining funds.
     lossy_strategy.setLockedFunds(amount_to_lock, DAY, sender=gov)
 
-    tx = vault.withdraw(
+    tx = vault.redeem(
         amount_to_withdraw,
         fish.address,
         fish.address,
+        max_loss,
         [s.address for s in strategies],
         sender=fish,
     )
@@ -1032,6 +1410,61 @@ def test_withdraw__half_of_strategy_assets_from_locked_lossy_strategy_with_unrea
     assert vault.balanceOf(fish) == amount - amount_to_withdraw
 
 
+def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unrealised_losses__custom_max_loss__reverts(
+    gov,
+    fish,
+    fish_amount,
+    asset,
+    create_vault,
+    create_strategy,
+    create_locked_strategy,
+    user_deposit,
+    add_strategy_to_vault,
+    add_debt_to_strategy,
+):
+    vault = create_vault(asset)
+    amount = fish_amount
+    amount_per_strategy = amount // 2  # deposit half of amount per strategy
+    amount_to_lose = amount_per_strategy // 2  # loss only half of strategy
+    amount_to_lock = amount_to_lose * 9 // 10  # Lock 90% of whats remaining
+    amount_to_withdraw = (
+        amount // 4
+    )  # withdraw a quarter deposit (half of strategy debt)
+    shares = amount
+    liquid_strategy = create_strategy(vault)
+    lossy_strategy = create_locked_strategy(vault)
+    strategies = [lossy_strategy, liquid_strategy]
+    max_loss = 0
+
+    # deposit assets to vault
+    user_deposit(fish, vault, asset, amount)
+
+    # set up strategies
+    vault.set_role(
+        gov.address,
+        ROLES.ADD_STRATEGY_MANAGER | ROLES.DEBT_MANAGER | ROLES.MAX_DEBT_MANAGER,
+        sender=gov,
+    )
+    for strategy in strategies:
+        add_strategy_to_vault(gov, strategy, vault)
+        add_debt_to_strategy(gov, strategy, vault, amount_per_strategy)
+
+    # lose half of assets in lossy strategy
+    asset.transfer(gov, amount_to_lose, sender=lossy_strategy)
+    # Lock half the remaining funds.
+    lossy_strategy.setLockedFunds(amount_to_lock, DAY, sender=gov)
+
+    with ape.reverts("to much loss"):
+        vault.redeem(
+            amount_to_withdraw,
+            fish.address,
+            fish.address,
+            max_loss,
+            [s.address for s in strategies],
+            sender=fish,
+        )
+
+
 def test_withdraw__with_multiple_liquid_strategies_more_assets_than_debt__withdraws(
     gov,
     fish,
@@ -1051,6 +1484,8 @@ def test_withdraw__with_multiple_liquid_strategies_more_assets_than_debt__withdr
     first_strategy = create_strategy(vault)
     second_strategy = create_strategy(vault)
     strategies = [first_strategy, second_strategy]
+    max_loss = 0
+
     profit = (
         amount_per_strategy + 1
     )  # enough so that it could serve a full withdraw with the profit
@@ -1071,7 +1506,12 @@ def test_withdraw__with_multiple_liquid_strategies_more_assets_than_debt__withdr
     asset.transfer(first_strategy, profit, sender=gov)
 
     tx = vault.withdraw(
-        shares, fish.address, fish.address, [s.address for s in strategies], sender=fish
+        shares,
+        fish.address,
+        fish.address,
+        max_loss,
+        [s.address for s in strategies],
+        sender=fish,
     )
     event = list(tx.decode_logs(vault.Withdraw))