# Proposed fix diffs for 5 Medium findings — DRAWING ONLY

**Status**: code NOT modified. This document shows exactly what each fix would look like as a patch against the public `Deepyield-labs/deepyield-vault` repo at commit `3b20d60` (audit baseline).

Apply via standard PR workflow when ready. Estimated total dev time: **~3.5 hours** including test updates.

---

## M1 — DedicatedVaultMain.sol: tighten closePosition slippage

```diff
--- a/src/DedicatedVaultMain.sol
+++ b/src/DedicatedVaultMain.sol
@@ -247,8 +247,9 @@ contract DedicatedVaultMain is AccessControl, Pausable, ReentrancyGuard {
     function closePosition(uint256 amount0Min, uint256 amount1Min, uint256 deadline)
         external onlyRole(KEEPER_ROLE) nonReentrant
     {
-        // normal keeper close must be slippage-bounded; zero-min only via emergency paths.
-        if (amount0Min == 0 && amount1Min == 0) revert ZeroSlippageNotAllowed();
+        // Both legs must be slippage-bounded — single-sided zero-min would
+        // allow MEV sandwich on the unprotected leg even when position is
+        // skewed. Zero-min only via emergencyClose / closeIfStale.
+        if (amount0Min == 0 || amount1Min == 0) revert ZeroSlippageNotAllowed();
         _close("keeper", amount0Min, amount1Min, deadline);
         lastKeeperAt = block.timestamp;
     }
@@ -260,7 +261,8 @@ contract DedicatedVaultMain is AccessControl, Pausable, ReentrancyGuard {
         uint256 amount0Min, uint256 amount1Min, uint256 closeDeadline,
         uint256 pairedMinOut, uint256 rewardMinOut, uint256 swapDeadline
     ) external onlyRole(KEEPER_ROLE) nonReentrant {
-        if (amount0Min == 0 && amount1Min == 0) revert ZeroSlippageNotAllowed();
+        // Both close legs must be bounded (same rationale as closePosition).
+        if (amount0Min == 0 || amount1Min == 0) revert ZeroSlippageNotAllowed();
         _close("keeper", amount0Min, amount1Min, closeDeadline);
         lastKeeperAt = block.timestamp;
```

---

## M2a — BeefyCLMAdapter.sol: gate setBeefyClm/setRouter on active position

```diff
--- a/src/BeefyCLMAdapter.sol
+++ b/src/BeefyCLMAdapter.sol
@@ -47,6 +47,7 @@ contract BeefyCLMAdapter is IDeepYieldStrategy, AccessControl, Pausable, Reentra
     error ZeroAddress();
     error NotVault();
     error ZeroAmount();
+    error MigrationNotEmpty();
     error InvalidSplitBps();

@@ -103,12 +104,21 @@ contract BeefyCLMAdapter is IDeepYieldStrategy, AccessControl, Pausable, Reentra
     }

+    /// @dev Both setBeefyClm and setRouter swap external dependencies that
+    /// hold or quote our funds. Swapping while we have shares in the OLD
+    /// beefyClm would orphan them (accountedAssets says we have assets,
+    /// new beefyClm.balanceOf returns 0). Require panic/unwind first.
     function setBeefyClm(address newBeefyClm) external onlyRole(ADMIN_ROLE) {
         if (newBeefyClm == address(0)) revert ZeroAddress();
+        if (beefyClm.balanceOf(address(this)) > 0) revert MigrationNotEmpty();
+        if (accountedAssets > 0) revert MigrationNotEmpty();
         beefyClm = IBeefyCLM(newBeefyClm);
     }

     function setRouter(address newRouter) external onlyRole(ADMIN_ROLE) {
         if (newRouter == address(0)) revert ZeroAddress();
+        // Router holds no balance itself but quoteWithdraw drives NAV;
+        // forbid swap while accounting is non-empty to keep NAV consistent.
+        if (accountedAssets > 0) revert MigrationNotEmpty();
         router = IUnderlyingRouter(newRouter);
     }
```

**Migration path** to swap Beefy strategy:
1. `managerWithdrawAll()` — pulls everything, sets `accountedAssets = 0`
2. `setBeefyClm(newAddr)` — passes the gate
3. `deploy(amount)` — fresh start with new Beefy

---

## M2b — DeepYieldStrategyAdapter.sol: gate setUnderlyingVault

```diff
--- a/src/DeepYieldStrategyAdapter.sol
+++ b/src/DeepYieldStrategyAdapter.sol
@@ -40,6 +40,7 @@ contract DeepYieldStrategyAdapter is IDeepYieldStrategy, AccessControl, Pausable
     error ZeroAddress();
     error NotVault();
     error ZeroAmount();
+    error MigrationNotEmpty();
     error InvalidStrategyConfig();

@@ -91,8 +92,15 @@ contract DeepYieldStrategyAdapter is IDeepYieldStrategy, AccessControl, Pausable
         _;
     }

+    /// @dev Swapping the underlying vault while we still hold shares in the
+    /// OLD vault would orphan them (accountedAssets stays positive, new
+    /// underlyingVault.balanceOf returns 0). Require unwind first via
+    /// managerWithdrawAll() or panic().
     function setUnderlyingVault(address newUnderlyingVault) external onlyRole(ADMIN_ROLE) {
         if (newUnderlyingVault == address(0)) revert ZeroAddress();
+        if (underlyingVault.balanceOf(address(this)) > 0) revert MigrationNotEmpty();
+        if (accountedAssets > 0) revert MigrationNotEmpty();
         underlyingVault = IUnderlyingVault(newUnderlyingVault);
     }
```

---

## M5 — PancakeV3MasterchefVenue.sol: emergency unstake fallback

```diff
--- a/src/PancakeV3MasterchefVenue.sol
+++ b/src/PancakeV3MasterchefVenue.sol
@@ -76,6 +76,7 @@ contract PancakeV3MasterchefVenue is IDedicatedVenue, ERC721Holder, ReentrancyGu
     error PositionActive();
     error NoActivePosition();
     error RewardTokenRequired();
     error ZeroAddress();
+    error ForceUnstakeUnavailable();

@@ -136,11 +137,38 @@ contract PancakeV3MasterchefVenue is IDedicatedVenue, ERC721Holder, ReentrancyGu
+    /// @notice EMERGENCY ONLY. Forces unstake from masterchef WITHOUT harvest
+    /// when masterchef is broken (paused, upgraded, etc) and the regular
+    /// close() reverts. Skips CAKE rewards. Controller-only.
+    /// @dev Calls masterchef.withdraw via try; if that also reverts, position
+    /// is genuinely stuck and admin must wait for masterchef to recover.
+    function forceUnstakeSkipHarvest(uint256 positionId)
+        external onlyController nonReentrant
+    {
+        if (positionId == 0 || positionId != activeTokenId) revert NoActivePosition();
+        if (!farmed) revert ForceUnstakeUnavailable();
+        // try harvest the canonical way; on revert, skip CAKE
+        try masterchef.harvest(positionId, address(this)) {} catch {}
+        // try bare withdraw; if that also reverts, position truly stuck
+        try masterchef.withdraw(positionId, address(this)) {
+            // NFT recovered — close() can now proceed via non-farmed branch
+        } catch {
+            revert ForceUnstakeUnavailable();
+        }
+    }
+
     /// @notice Full-close: unstake (+harvest), remove all liquidity (bounded), collect,
```

**Use case**: when `close()` reverts because masterchef is paused/broken:
1. Call `forceUnstakeSkipHarvest()` — sacrifices unclaimed CAKE, recovers NFT
2. Call `close()` — succeeds (no longer triggers farmed branch since NFT already received)

---

## M6 — DeepYieldStrategyAdapter.sol: panic() fee crystallization (Task 1.32 parity)

```diff
--- a/src/DeepYieldStrategyAdapter.sol
+++ b/src/DeepYieldStrategyAdapter.sol
@@ -208,16 +208,38 @@ contract DeepYieldStrategyAdapter is IDeepYieldStrategy, AccessControl, Pausable
+    /// @notice Emergency unwind. Mirrors BeefyCLMAdapter.panic() Task 1.32
+    /// fix: crystallizes protocol fee on realized profit BEFORE forwarding
+    /// to vault. Pre-Task 1.32, panic() bypassed both project and partner
+    /// fee on a profitable emergency exit.
     function panic() external onlyRole(GUARDIAN_ROLE) nonReentrant {
         uint256 shares = underlyingVault.balanceOf(address(this));
         if (shares > 0) {
             underlyingVault.redeem(shares, address(this), address(this));
         }
-        uint256 bal = assetToken.balanceOf(address(this));
-        if (bal > 0) {
-            assetToken.safeTransfer(vault, bal);
+
+        uint256 realized = assetToken.balanceOf(address(this));
+
+        // Crystallize fee on realized profit (Task 1.32 parity with Beefy).
+        // Flat/loss path: feeAssets=0, no event, no transfer.
+        uint256 feeAssets = 0;
+        uint256 grossProfit = 0;
+        if (realized > accountedAssets) {
+            grossProfit = realized - accountedAssets;
+            feeAssets = VaultFeesLib.performanceFee(grossProfit, performanceFeeBps);
+        }
+        if (feeAssets > 0) {
+            assetToken.safeTransfer(treasury, feeAssets);
+        }
+
+        uint256 bal = realized - feeAssets;
+        if (bal > 0) {
+            assetToken.safeTransfer(vault, bal);
         }
         accountedAssets = 0;
         _pause();
     }
```

**Optional enhancement** for full parity with BeefyCLMAdapter:
- Add `event ProtocolFeeRealized(bytes32 indexed source, uint256 feeAssets, uint256 grossProfit)`
- Add `bytes32 public constant SOURCE_PANIC = keccak256("PANIC")`
- Emit `ProtocolFeeRealized(SOURCE_PANIC, feeAssets, grossProfit)` when feeAssets > 0

---

## Test impact

After applying all 5 fixes, run `forge test --fork-url $BSC_RPC`. Expected regressions:

| Fix | Tests that may need updates |
|-----|---|
| M1 | Any `closePosition(0, X)` / `closePosition(X, 0)` calls — must pass both mins > 0 |
| M2a/M2b | Tests that swap `beefyClm`/`router`/`underlyingVault` mid-test — add `managerWithdrawAll` first |
| M5 | New tests for force-unstake path — none should fail |
| M6 | Panic-with-profit tests — different `ProtocolFeeRealized` event + reduced `vault` balance |

If any test fails in an unexpected way, that's a signal the fix is doing something semantically wrong — investigate before merging.

## Verification after fix

1. `forge test --fork-url $BSC_RPC` — all green
2. `aderyn .` — re-run, expect fewer / same findings (no new HIGH)
3. `slither . --solc-args="--via-ir --optimize --optimize-runs 200" --filter-paths "lib/|test/|script/"` — same
4. Manual diff review — verify no scope creep beyond the M1-M6 surface

## Effort estimate

| Fix | Code | Test update | Re-audit | Total |
|-----|:---:|:---:|:---:|:---:|
| M1 | 5m | 10m | 5m | 20m |
| M2a | 15m | 15m | 5m | 35m |
| M2b | 15m | 15m | 5m | 35m |
| M5 | 30m | 30m | 10m | 70m |
| M6 | 20m | 20m | 10m | 50m |
| **Total** | — | — | — | **~3.5 hours** |