Heap is incorrectly stores the removed operator ID which can lead to division by zero in deposit/withdrawal flow
An operator's strategy can be reset by the owner calling `setOperatorStrategyCaps` to "0". This action sets the utilization to "0" and removes the operator from the heap. Consequently, this means that the operator has unwound all its strategy shares and can no longer receive any more deposits. However, due to how the heap is organized, if an operator who had funds before is reset to "0", the heap will not successfully remove the operator. As a result, when ordering the heap, a division by "0" will occur, causing the transaction to revert on deposits and withdrawals indefinitely.\\nIn order to break down the issue, let's divide the issue to 2 parts which their combination is the issue itself\\n1- Heap is not removing the removed ID from the heaps storage when the operator is removed\\nWhen the operator is removed, the operator will be removed from the heap as follows:\\n```\\nfunction setOperatorStrategyCap(\\n        RioLRTOperatorRegistryStorageV1.StorageV1 storage s,\\n        uint8 operatorId,\\n        IRioLRTOperatorRegistry.StrategyShareCap memory newShareCap\\n    ) internal {\\n        .\\n        OperatorUtilizationHeap.Data memory utilizationHeap = s.getOperatorUtilizationHeapForStrategy(newShareCap.strategy);\\n        // If the current cap is greater than 0 and the new cap is 0, remove the operator from the strategy.\\n        if (currentShareDetails.cap > 0 && newShareCap.cap == 0) {\\n            // If the operator has allocations, queue them for exit.\\n            if (currentShareDetails.allocation > 0) {\\n                operatorDetails.queueOperatorStrategyExit(operatorId, newShareCap.strategy);\\n            }\\n            // Remove the operator from the utilization heap.\\n            -> utilizationHeap.removeByID(operatorId);\\n        }\\n        .\\n\\n        // Persist the updated heap to the active operators tracking.\\n        -> utilizationHeap.store(s.activeOperatorsByStrategyShareUtilization[newShareCap.strategy]);\\n         .\\n    }\\n```\\n\\n`removeByID` calls the internal `_remove` function which is NOT removes the last element! `self.count` is decreased however, the index is still the previous value of the `self.count`\\n```\\nfunction _remove(Data memory self, uint8 i) internal pure {\\n        self.operators[i] = self.operators[self.count--];\\n    }\\n```\\n\\nFor example, if there are 3 operators as follows: operatorId: 1, utilization: 50% operatorId: 2, utilization: 60% operatorId: 3, utilization: 70% then, the `heap.count` would be 3 and the order would be: 1, 2, 3 in the heap heap.operators[1] = operatorId 1 heap.operators[2] = operatorId 2 heap.operators[3] = operatorId 3\\nif we remove the operator Id 2: `heap.count` = 2 order: 1,3 heap.operators[1] = operatorId 1 heap.operators[2] = operatorId 2 heap.operators[3] = operatorId 0 THIS SHOULD BE "0" since its removed but it is "3" in the current implementation!\\nAs shown here, the operators[3] should be "0" since there isn't any operator3 in the heap anymore but the heap keeps the value and not resets it.\\nHere a test shows the above issue:\\n```\\n// forge test --match-contract OperatorUtilizationHeapTest --match-test test_removingDoesNotUpdatesStoredHeap -vv\\n    function test_removingDoesNotUpdatesStoredHeap() public {\\n        OperatorUtilizationHeap.Data memory heap = OperatorUtilizationHeap.initialize(5);\\n\\n        heap.insert(OperatorUtilizationHeap.Operator({id: 1, utilization: 50}));\\n        heap.store(heapStore);\\n\\n        heap.insert(OperatorUtilizationHeap.Operator({id: 2, utilization: 60}));\\n        heap.store(heapStore);\\n\\n        heap.insert(OperatorUtilizationHeap.Operator({id: 3, utilization: 70}));\\n        heap.store(heapStore);\\n\\n        console.log("Heaps count", heap.count);\\n        console.log("1st", heap.operators[1].id);\\n        console.log("2nd", heap.operators[2].id);\\n        console.log("3rd", heap.operators[3].id);\\n\\n        // remove 2\\n        heap.removeByID(3);\\n        heap.store(heapStore);\\n\\n        console.log("Heaps count", heap.count);\\n        console.log("1st", heap.operators[1].id);\\n        console.log("2nd", heap.operators[2].id);\\n        console.log("3rd", heap.operators[3].id);\\n    }\\n```\\n\\nLogs:\\n2- When the operator cap is reseted the allocations/deallocations will not work due to above heap issue because of division by zero\\nNow, take the above example, we removed the operatorId 3 from the heap by setting its cap to "0". Now, there are only operators 1 and 2 active for that specific strategy. When there are idle funds in the deposit pool before the rebalance call, the excess funds that are not requested as withdrawals will be pushed to EigenLayer as follows:\\n```\\nfunction rebalance(address asset) external checkRebalanceDelayMet(asset) {\\n       .\\n       .\\n        -> (uint256 sharesReceived, bool isDepositCapped) = depositPool().depositBalanceIntoEigenLayer(asset);\\n        .\\n    }\\n```\\n\\n```\\n function depositBalanceIntoEigenLayer(address asset) external onlyCoordinator returns (uint256, bool) {\\n        uint256 amountToDeposit = asset.getSelfBalance();\\n        if (amountToDeposit == 0) return (0, false);\\n        .\\n        .\\n        -> return (OperatorOperations.depositTokenToOperators(operatorRegistry(), asset, strategy, sharesToAllocate), isDepositCapped);\\n    }\\n```\\n\\n```\\nfunction depositTokenToOperators(\\n        IRioLRTOperatorRegistry operatorRegistry,\\n        address token,\\n        address strategy,\\n        uint256 sharesToAllocate\\n    ) internal returns (uint256 sharesReceived) {\\n       -> (uint256 sharesAllocated, IRioLRTOperatorRegistry.OperatorStrategyAllocation[] memory  allocations) = operatorRegistry.allocateStrategyShares(\\n            strategy, sharesToAllocate\\n        );\\n        .\\n        .\\n    }\\n```\\n\\n```\\nfunction allocateStrategyShares(address strategy, uint256 sharesToAllocate) external onlyDepositPool returns (uint256 sharesAllocated, OperatorStrategyAllocation[] memory allocations) {\\n        -> OperatorUtilizationHeap.Data memory heap = s.getOperatorUtilizationHeapForStrategy(strategy);\\n       .\\n       .\\n       .\\n       .\\n    }\\n```\\n\\n```\\nfunction getOperatorUtilizationHeapForStrategy(RioLRTOperatorRegistryStorageV1.StorageV1 storage s, address strategy) internal view returns (OperatorUtilizationHeap.Data memory heap) {\\n        uint8 numActiveOperators = s.activeOperatorCount;\\n        if (numActiveOperators == 0) return OperatorUtilizationHeap.Data(new OperatorUtilizationHeap.Operator[](0), 0);\\n        \\n        heap = OperatorUtilizationHeap.initialize(MAX_ACTIVE_OPERATOR_COUNT);\\n        LibMap.Uint8Map storage operators = s.activeOperatorsByStrategyShareUtilization[strategy];\\n\\n        IRioLRTOperatorRegistry.OperatorShareDetails memory operatorShares;\\n        unchecked {\\n            uint8 i;\\n            for (i = 0; i < numActiveOperators; ++i) {\\n                uint8 operatorId = operators.get(i);\\n\\n                // Non-existent operator ID. We've reached the end of the heap.\\n                if (operatorId == 0) break;\\n\\n                operatorShares = s.operatorDetails[operatorId].shareDetails[strategy];\\n                heap.operators[i + 1] = OperatorUtilizationHeap.Operator({\\n                    id: operatorId,\\n                    -> utilization: operatorShares.allocation.divWad(operatorShares.cap)\\n                });\\n            }\\n            heap.count = i;\\n        }\\n    }\\n```\\n\\nAs we can see in one above code snippet, the `numActiveOperators` is 3. Since the stored heaps last element is not set to "0" it will point to operatorId 3 which has a cap of "0" after the removal. This will make the\\n```\\nutilization: operatorShares.allocation.divWad(operatorShares.cap)\\n```\\n\\npart of the code to perform a division by zero and the function will revert.\\nCoded PoC:\\n```\\n// forge test --match-contract RioLRTOperatorRegistryTest --match-test test_Capped0ValidatorBricksFlow -vv\\n    function test_Capped0ValidatorBricksFlow() public {\\n        // Add 3 operators\\n        addOperatorDelegators(reLST.operatorRegistry, address(reLST.rewardDistributor), 3);\\n\\n        // The caps for each operator is 1000e18, we will delete the id 2 so we need funds there\\n        // any number that is more than 1000 should be ok for that experiement \\n        uint256 AMOUNT = 1002e18;\\n\\n        // Allocate to cbETH strategy.\\n        cbETH.approve(address(reLST.coordinator), type(uint256).max);\\n        uint256 lrtAmount = reLST.coordinator.deposit(CBETH_ADDRESS, AMOUNT);\\n\\n        // Push funds into EigenLayer.\\n        vm.prank(EOA, EOA);\\n        reLST.coordinator.rebalance(CBETH_ADDRESS);\\n\\n        // Build the empty caps\\n        IRioLRTOperatorRegistry.StrategyShareCap[] memory zeroStrategyShareCaps =\\n            new IRioLRTOperatorRegistry.StrategyShareCap[](1);\\n        zeroStrategyShareCaps[0] = IRioLRTOperatorRegistry.StrategyShareCap({strategy: CBETH_STRATEGY, cap: 0});\\n\\n        // Set the caps of CBETH_STRATEGY for operator 2 as "0"\\n        reLST.operatorRegistry.setOperatorStrategyShareCaps(2, zeroStrategyShareCaps);\\n\\n        // Try an another deposit, we expect revert when we do the rebalance\\n        reLST.coordinator.deposit(CBETH_ADDRESS, 10e18);\\n\\n        // Push funds into EigenLayer. Expect revert, due to division by "0"\\n        skip(reETH.coordinator.rebalanceDelay());\\n        vm.startPrank(EOA, EOA);\\n        vm.expectRevert(bytes4(keccak256("DivWadFailed()")));\\n        reLST.coordinator.rebalance(CBETH_ADDRESS);\\n        vm.stopPrank();\\n    }\\n```\\n
When removing from the heap also remove the last element from the heap.\\nI am not sure of this, but this might work\\n```\\nfunction _remove(Data memory self, uint8 i) internal pure {\\n        self.operators[i] = self.operators[--self.count];\\n    }\\n```\\n
Core logic broken, withdrawal/deposits can not be performed.
```\\nfunction setOperatorStrategyCap(\\n        RioLRTOperatorRegistryStorageV1.StorageV1 storage s,\\n        uint8 operatorId,\\n        IRioLRTOperatorRegistry.StrategyShareCap memory newShareCap\\n    ) internal {\\n        .\\n        OperatorUtilizationHeap.Data memory utilizationHeap = s.getOperatorUtilizationHeapForStrategy(newShareCap.strategy);\\n        // If the current cap is greater than 0 and the new cap is 0, remove the operator from the strategy.\\n        if (currentShareDetails.cap > 0 && newShareCap.cap == 0) {\\n            // If the operator has allocations, queue them for exit.\\n            if (currentShareDetails.allocation > 0) {\\n                operatorDetails.queueOperatorStrategyExit(operatorId, newShareCap.strategy);\\n            }\\n            // Remove the operator from the utilization heap.\\n            -> utilizationHeap.removeByID(operatorId);\\n        }\\n        .\\n\\n        // Persist the updated heap to the active operators tracking.\\n        -> utilizationHeap.store(s.activeOperatorsByStrategyShareUtilization[newShareCap.strategy]);\\n         .\\n    }\\n```\\n