Audit fixes

.github/workflows/coverage.yaml

@@ -12,12 +12,12 @@ jobs:
 
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
         with:
           submodules: recursive
 
       - name: Install foundry
-        uses: foundry-rs/foundry-toolchain@v1
+        uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10
         with:
           version: stable
 
@@ -43,7 +43,7 @@ jobs:
           TEST_USE_FORK_VAULTS: false
 
       - name: Check coverage is updated
-        uses: actions/github-script@v5
+        uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           script: |
@@ -61,7 +61,7 @@ jobs:
 
       - name: Comment on PR
         id: comment
-        uses: actions/github-script@v5
+        uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           script: |

.github/workflows/lint.yaml

@@ -13,10 +13,10 @@ jobs:
 
     steps:
       - name: Check out Git repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
 
       - name: Install Foundry
-        uses: foundry-rs/foundry-toolchain@v1
+        uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10
         with:
           version: stable
 

.github/workflows/slither.yaml

@@ -12,17 +12,17 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
         with:
           submodules: recursive
 
       - name: Install Foundry
-        uses: foundry-rs/foundry-toolchain@v1
+        uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10
         with:
           version: stable
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405
         with:
           python-version: '3.10'
 

.github/workflows/test-fork.yaml

@@ -12,12 +12,12 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
         with:
           submodules: recursive
 
       - name: Install Foundry
-        uses: foundry-rs/foundry-toolchain@v1
+        uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10
         with:
           version: stable
 

.github/workflows/test.yaml

@@ -12,12 +12,12 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
         with:
           submodules: recursive
 
       - name: Install Foundry
-        uses: foundry-rs/foundry-toolchain@v1
+        uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10
         with:
           version: stable
 

contracts/curators/BalancedCurator.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.22;
 
 import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
 import {ISubVaultsCurator} from "../interfaces/ISubVaultsCurator.sol";
+import {IVaultState} from "../interfaces/IVaultState.sol";
 import {Errors} from "../libraries/Errors.sol";
 
 /**
@@ -15,7 +16,7 @@ contract BalancedCurator is ISubVaultsCurator {
     /// @inheritdoc ISubVaultsCurator
     function getDeposits(uint256 assetsToDeposit, address[] calldata subVaults, address ejectingVault)
         external
-        pure
+        view
         override
         returns (Deposit[] memory deposits)
     {
@@ -24,42 +25,78 @@ contract BalancedCurator is ISubVaultsCurator {
         }
 
         uint256 subVaultsCount = subVaults.length;
-        // the deposits should not be made to the vault that is being ejected
-        uint256 depositSubVaultsCount = ejectingVault != address(0) ? subVaultsCount - 1 : subVaultsCount;
-        if (depositSubVaultsCount == 0) {
-            revert Errors.EmptySubVaults();
-        }
-        uint256 amountPerVault = assetsToDeposit / depositSubVaultsCount;
-        uint256 dust = assetsToDeposit % depositSubVaultsCount;
-
-        // distribute assets evenly across sub-vaults
         deposits = new Deposit[](subVaultsCount);
         bool ejectingVaultFound = false;
+
+        // fetch remaining capacities and validate vaults
+        uint256[] memory capacities = new uint256[](subVaultsCount);
+        uint256 depositSubVaultsCount;
         for (uint256 i = 0; i < subVaultsCount;) {
             address subVault = subVaults[i];
             if (subVault == address(0)) {
                 revert Errors.ZeroAddress();
-            } else if (subVault == ejectingVault) {
+            }
+            deposits[i].vault = subVault;
+            if (subVault == ejectingVault) {
                 if (ejectingVaultFound) {
-                    // only one vault can be ejected at a time
                     revert Errors.RepeatedEjectingVault();
                 }
-                deposits[i] = Deposit({vault: subVault, assets: 0});
                 ejectingVaultFound = true;
-            } else if (dust > 0) {
-                deposits[i] = Deposit({vault: subVault, assets: amountPerVault + dust});
-                dust = 0; // only one vault can receive dust
             } else {
-                deposits[i] = Deposit({vault: subVault, assets: amountPerVault});
+                uint256 capacity = IVaultState(subVault).capacity();
+                uint256 totalAssets = IVaultState(subVault).totalAssets();
+                if (capacity > totalAssets) {
+                    capacities[i] = capacity - totalAssets;
+                    depositSubVaultsCount += 1;
+                }
             }
             unchecked {
-                // cannot realistically overflow
                 ++i;
             }
         }
         if (ejectingVault != address(0) && !ejectingVaultFound) {
             revert Errors.EjectingVaultNotFound();
         }
+
+        // distribute assets evenly across sub-vaults, respecting capacities
+        while (assetsToDeposit > 0) {
+            if (depositSubVaultsCount == 0) {
+                revert Errors.EmptySubVaults();
+            }
+            uint256 amountPerVault =
+                assetsToDeposit > depositSubVaultsCount ? assetsToDeposit / depositSubVaultsCount : assetsToDeposit;
+
+            depositSubVaultsCount = 0;
+            for (uint256 i = 0; i < subVaultsCount;) {
+                uint256 subVaultCapacity = capacities[i];
+
+                if (subVaultCapacity == 0) {
+                    unchecked {
+                        ++i;
+                    }
+                    continue;
+                }
+
+                uint256 depositAmount = Math.min(Math.min(subVaultCapacity, amountPerVault), assetsToDeposit);
+
+                deposits[i].assets += depositAmount;
+                assetsToDeposit -= depositAmount;
+                if (assetsToDeposit == 0) {
+                    return deposits;
+                }
+
+                subVaultCapacity -= depositAmount;
+                capacities[i] = subVaultCapacity;
+
+                if (subVaultCapacity > 0) {
+                    depositSubVaultsCount += 1;
+                }
+
+                unchecked {
+                    ++i;
+                }
+            }
+        }
     }
 
     /// @inheritdoc ISubVaultsCurator

contracts/interfaces/ISubVaultsCurator.sol

@@ -37,7 +37,7 @@ interface ISubVaultsCurator {
      */
     function getDeposits(uint256 assetsToDeposit, address[] calldata subVaults, address ejectingVault)
         external
-        pure
+        view
         returns (Deposit[] memory deposits);
 
     /**

contracts/keeper/KeeperRewards.sol

@@ -186,7 +186,6 @@ abstract contract KeeperRewards is KeeperOracles, IKeeperRewards {
             revert Errors.InvalidProof();
         }
 
-        // SLOAD to memory
         Reward storage lastReward = rewards[msg.sender];
         // check whether Vault's nonce is smaller that the current, otherwise it's already harvested
         if (lastReward.nonce >= currentNonce) return (0, 0, false);

contracts/libraries/OsTokenUtils.sol

@@ -46,7 +46,6 @@ library OsTokenUtils {
         IOsTokenVaultController osTokenVaultController,
         RedemptionData memory data
     ) external view returns (uint256 receivedAssets) {
-        // SLOAD to memory
         IOsTokenConfig.Config memory config = osTokenConfig.getConfig(address(this));
 
         // calculate received assets

contracts/nodes/NodesManager.sol

@@ -230,7 +230,7 @@ abstract contract NodesManager is
         uint256 penaltyAssetsDelta = params.cumPenaltyAssets - operatorState.cumPenaltyAssets;
         uint256 totalPenaltyAssets = penaltyAssetsDelta + pendingPenaltyAssets[operator];
         if (totalPenaltyAssets > 0) {
-            totalPenaltyShares = IVaultState(vault).convertToShares(totalPenaltyAssets);
+            totalPenaltyShares = IVaultState(vault).convertToShares(totalPenaltyAssets + 1);
         }
 
         // apply penalty to balance, storing excess as pending if insufficient
@@ -318,8 +318,19 @@ abstract contract NodesManager is
     }
 
     /// @inheritdoc INodesManager
-    function withdrawValidators(bytes calldata validators) external payable override onlyWithdrawalsManager {
+    function withdrawValidators(bytes calldata validators)
+        external
+        payable
+        override
+        nonReentrant
+        onlyWithdrawalsManager
+    {
+        uint256 balanceBefore = address(this).balance - msg.value;
         IVaultValidators(vault).withdrawValidators{value: msg.value}(validators, bytes(""));
+        uint256 surplus = address(this).balance - balanceBefore;
+        if (surplus > 0) {
+            _transferAssets(msg.sender, surplus);
+        }
         emit ValidatorWithdrawalSubmitted(msg.sender);
     }
 
@@ -360,6 +371,9 @@ abstract contract NodesManager is
         address operator = _exitPositions[positionTicket];
         if (operator == address(0)) revert Errors.InvalidTicket();
 
+        // check whether the vault is harvested
+        if (_keeper.isHarvestRequired(vault)) revert Errors.NotHarvested();
+
         // check whether the operator has synced the latest state
         uint128 currentNonce = stateData.currentNonce;
         if (operatorNonces[operator][OperatorNonceType.LastStateUpdate] != currentNonce) {
@@ -413,10 +427,7 @@ abstract contract NodesManager is
                 pendingPenaltyAssets[operator] = 0;
                 exitedAssets -= pendingPenalty;
             }
-        }
-
-        // donate deducted penalty back to the vault
-        if (penaltyDeducted > 0) {
+            // donate deducted penalty back to the vault
             _donateAssets(penaltyDeducted);
         }
 
@@ -436,6 +447,14 @@ abstract contract NodesManager is
     function _deposit(uint256 assets) internal returns (uint256 addedShares) {
         if (assets < minDepositAssets) revert Errors.InvalidAssets();
 
+        // check whether the operator has synced the latest state
+        if (
+            operatorStates[msg.sender].totalAssets > 0
+                && operatorNonces[msg.sender][OperatorNonceType.LastStateUpdate] != stateData.currentNonce
+        ) {
+            revert Errors.NotHarvested();
+        }
+
         // deposit assets to the vault
         uint256 depositShares = _depositToVault(assets);
 
@@ -453,11 +472,10 @@ abstract contract NodesManager is
                 penaltyAssets = IVaultState(vault).convertToAssets(penaltyShares);
                 pendingPenaltyAssets[msg.sender] = pendingPenalty - penaltyAssets;
             }
-        }
-
-        if (penaltyShares > 0) {
-            // donate penalty shares to the vault
-            IVaultState(vault).donateShares(penaltyShares);
+            if (penaltyShares > 0) {
+                // donate penalty shares to the vault
+                IVaultState(vault).donateShares(penaltyShares);
+            }
         }
 
         // update operator's shares balance