refactor: small cleanup (#959)

refactor small cleanup

chore: `forge fmt`

fix: `getQueuedWithdrawals` + test

fix: add constructor back

test: `totalQueued` > `withdrawal.strategies.length`

test(wip): `completeQueuedWithdrawals`

currently failing

fix: effectBlock

test(wip): @8sunyuan patch

fix: one flaky test

fix: second flaky test

Author: 0xClandestine

src/contracts/core/AllocationManager.sol

@@ -63,6 +63,7 @@ contract AllocationManager is
         // Check that the operator set exists and the operator is registered to it
         OperatorSet memory operatorSet = OperatorSet(avs, params.operatorSetId);
         bool isOperatorSlashable = _isOperatorSlashable(params.operator, operatorSet);
+        require(params.strategies.length == params.wadsToSlash.length, InputArrayLengthMismatch());
         require(_operatorSets[operatorSet.avs].contains(operatorSet.id), InvalidOperatorSet());
         require(isOperatorSlashable, OperatorNotSlashable());
 

src/contracts/core/DelegationManager.sol

@@ -964,7 +964,7 @@ contract DelegationManager is
             for (uint256 j; j < withdrawals[i].strategies.length; ++j) {
                 shares[i][j] = SlashingLib.scaleForCompleteWithdrawal({
                     scaledShares: withdrawals[i].scaledShares[j],
-                    slashingFactor: slashingFactors[i]
+                    slashingFactor: slashingFactors[j]
                 });
             }
         }

src/contracts/interfaces/IAllocationManager.sol

@@ -4,7 +4,6 @@ pragma solidity >=0.5.0;
 import {OperatorSet} from "../libraries/OperatorSetLib.sol";
 import "./IPauserRegistry.sol";
 import "./IStrategy.sol";
-import "./ISignatureUtils.sol";
 import "./IAVSRegistrar.sol";
 
 interface IAllocationManagerErrors {
@@ -213,7 +212,7 @@ interface IAllocationManagerEvents is IAllocationManagerTypes {
     event StrategyRemovedFromOperatorSet(OperatorSet operatorSet, IStrategy strategy);
 }
 
-interface IAllocationManager is ISignatureUtils, IAllocationManagerErrors, IAllocationManagerEvents {
+interface IAllocationManager is IAllocationManagerErrors, IAllocationManagerEvents {
     /**
      * @dev Initializes the initial owner and paused status.
      */

src/test/unit/AllocationManagerUnit.t.sol

@@ -613,6 +613,15 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests
         allocationManager.slashOperator(defaultAVS, _randSlashingParams(random().Address(), 0));
     }
 
+    function test_revert_InputArrayLengthMismatch() public {
+        SlashingParams memory slashingParams = _randSlashingParams(defaultOperator, 0);
+        slashingParams.strategies = slashingParams.strategies.setLength(2);
+
+        cheats.prank(defaultAVS);
+        cheats.expectRevert(InputArrayLengthMismatch.selector);
+        allocationManager.slashOperator(defaultAVS, slashingParams);
+    }
+
     function test_revert_StrategiesMustBeInAscendingOrder() public {
         IStrategy[] memory strategies = new IStrategy[](3);
         strategies[0] = IStrategy(address(3));

src/test/unit/DelegationUnit.t.sol

@@ -68,7 +68,6 @@ contract DelegationManagerUnitTests is EigenLayerUnitTestSetup, IDelegationManag
     // Helper to use in storage
     DepositScalingFactor dsf;
     uint256 stakerDSF;
-    uint256 slashingFactor;
 
     /// @notice mappings used to handle duplicate entries in fuzzed address array input
     mapping(address => uint256) public totalSharesForStrategyInArray;
@@ -6556,6 +6555,87 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage
         assertEq(operatorSharesAfter, operatorSharesBefore + withdrawalAmount, "operator shares not increased correctly");
         assertFalse(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be completed and marked false now");
     }
+
+    function testFuzz_completeQueuedWithdrawals_OutOfOrderBlocking(Randomness r) public {
+        uint256 totalDepositShares = r.Uint256(4, 100 ether);
+        uint256 depositSharesPerWithdrawal = totalDepositShares / 4;
+
+        _registerOperatorWithBaseDetails(defaultOperator);
+        strategyManagerMock.addDeposit(defaultStaker, strategyMock, totalDepositShares);
+        _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator);
+
+        QueuedWithdrawalParams[] memory queuedParams = new QueuedWithdrawalParams[](4);
+        Withdrawal[] memory withdrawals = new Withdrawal[](4);
+        
+        uint256 startBlock = block.number;
+
+        uint256 nonce = delegationManager.cumulativeWithdrawalsQueued(defaultStaker);
+        for (uint256 i; i < 4; ++i) {
+            cheats.roll(startBlock + i);
+            (
+                QueuedWithdrawalParams[] memory params, 
+                Withdrawal memory withdrawal,
+            ) = _setUpQueueWithdrawalsSingleStrat(
+                defaultStaker, 
+                defaultStaker, 
+                strategyMock, 
+                depositSharesPerWithdrawal
+            );
+            withdrawal.nonce = nonce;
+            nonce += 1;
+
+            (queuedParams[i], withdrawals[i]) = (params[0], withdrawal);
+        }
+
+        uint256 delay = delegationManager.minWithdrawalDelayBlocks();
+
+        cheats.startPrank(defaultStaker);
+        cheats.roll(startBlock);
+        
+        delegationManager.queueWithdrawals(queuedParams[0].toArray());
+        cheats.roll(startBlock + 1);
+        delegationManager.queueWithdrawals(queuedParams[1].toArray());
+        
+        (
+            Withdrawal[] memory firstWithdrawals, 
+            uint256[][] memory firstShares
+        ) = delegationManager.getQueuedWithdrawals(defaultStaker);
+
+        cheats.roll(startBlock + 2);
+        delegationManager.queueWithdrawals(queuedParams[2].toArray());
+        cheats.roll(startBlock + 3);
+        delegationManager.queueWithdrawals(queuedParams[3].toArray());
+
+        IERC20[][] memory tokens = new IERC20[][](2);
+        bool[] memory receiveAsTokens = new bool[](2);
+        for (uint256 i; i < 2; ++i) {
+            tokens[i] = strategyMock.underlyingToken().toArray();
+        }
+
+        bytes32 root1 = delegationManager.calculateWithdrawalRoot(withdrawals[0]);
+        bytes32 root2 = delegationManager.calculateWithdrawalRoot(withdrawals[1]);
+        
+        bytes32 root1_view = delegationManager.calculateWithdrawalRoot(firstWithdrawals[0]);
+        bytes32 root2_view = delegationManager.calculateWithdrawalRoot(firstWithdrawals[1]);
+
+        assertEq(
+            root1, root1_view,
+            "withdrawal root should be the same"
+        );
+
+        assertEq(
+            root2, root2_view,
+            "withdrawal root should be the same"
+        );
+
+        cheats.roll(startBlock + delay + 2);
+        delegationManager.completeQueuedWithdrawals(firstWithdrawals, tokens, true.toArray(2));
+        
+        // Throws `WithdrawalNotQueued`.
+        cheats.roll(startBlock + delay + 3);
+        delegationManager.completeQueuedWithdrawals(withdrawals[2].toArray(), tokens, true.toArray());
+        cheats.stopPrank();
+    }
 }
 
 contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests {
@@ -8549,6 +8629,121 @@ contract DelegationManagerUnitTests_ConvertToDepositShares is DelegationManagerU
 
 contract DelegationManagerUnitTests_getQueuedWithdrawals is DelegationManagerUnitTests {
     using ArrayLib for *;
+    using SlashingLib for *;
+
+    function _withdrawalRoot(Withdrawal memory withdrawal) internal pure returns (bytes32) {
+        return keccak256(abi.encode(withdrawal));
+    }
+
+    function test_getQueuedWithdrawals_Correctness(Randomness r) public rand(r) {
+        uint256 numStrategies = r.Uint256(2, 8);
+        uint256[] memory depositShares = r.Uint256Array({
+            len: numStrategies, 
+            min: 2, 
+            max: 100 ether
+        });
+
+        IStrategy[] memory strategies = _deployAndDepositIntoStrategies(defaultStaker, depositShares, false);
+        _registerOperatorWithBaseDetails(defaultOperator);
+        _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator);
+
+        for (uint256 i; i < numStrategies; ++i) {
+            uint256 newStakerShares = depositShares[i] / 2;
+            _setOperatorMagnitude(defaultOperator, strategies[i], 0.5 ether);
+            cheats.prank(address(allocationManagerMock));
+            delegationManager.burnOperatorShares(defaultOperator, strategies[i], WAD, 0.5 ether);
+            uint256 afterSlash = delegationManager.operatorShares(defaultOperator, strategies[i]);
+            assertApproxEqAbs(afterSlash, newStakerShares, 1, "bad operator shares after slash");
+        }
+
+        // Queue withdrawals.
+        (
+            QueuedWithdrawalParams[] memory queuedWithdrawalParams,
+            Withdrawal memory withdrawal,
+            bytes32 withdrawalRoot
+        ) = _setUpQueueWithdrawals({
+            staker: defaultStaker,
+            withdrawer: defaultStaker,
+            strategies: strategies,
+            depositWithdrawalAmounts: depositShares
+        });
+
+        cheats.prank(defaultStaker);
+        delegationManager.queueWithdrawals(queuedWithdrawalParams);
+        
+        // Get queued withdrawals.
+        (Withdrawal[] memory withdrawals, uint256[][] memory shares) = delegationManager.getQueuedWithdrawals(defaultStaker);
+        // Checks
+        for (uint256 i; i < strategies.length; ++i) {
+            uint256 newStakerShares = depositShares[i] / 2;
+            assertApproxEqAbs(shares[0][i], newStakerShares, 1, "staker shares should be decreased by half +- 1");
+        }
+        
+        assertEq(_withdrawalRoot(withdrawal), _withdrawalRoot(withdrawals[0]), "_withdrawalRoot(withdrawal) != _withdrawalRoot(withdrawals[0])");
+        assertEq(_withdrawalRoot(withdrawal), withdrawalRoot, "_withdrawalRoot(withdrawal) != withdrawalRoot");
+    }
+
+    function test_getQueuedWithdrawals_TotalQueuedGreaterThanTotalStrategies(
+        Randomness r
+    ) public rand(r) {
+        uint256 totalDepositShares = r.Uint256(2, 100 ether);
+
+        _registerOperatorWithBaseDetails(defaultOperator);
+        strategyManagerMock.addDeposit(defaultStaker, strategyMock, totalDepositShares);
+        _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator);
+        
+        uint256 newStakerShares = totalDepositShares / 2;
+        _setOperatorMagnitude(defaultOperator, strategyMock, 0.5 ether);
+        cheats.prank(address(allocationManagerMock));
+        delegationManager.burnOperatorShares(defaultOperator, strategyMock, WAD, 0.5 ether);
+        uint256 afterSlash = delegationManager.operatorShares(defaultOperator, strategyMock);
+        assertApproxEqAbs(afterSlash, newStakerShares, 1, "bad operator shares after slash");
+
+        // Queue withdrawals.
+        (
+            QueuedWithdrawalParams[] memory queuedWithdrawalParams0,
+            Withdrawal memory withdrawal0,
+            bytes32 withdrawalRoot0
+        ) = _setUpQueueWithdrawalsSingleStrat({
+            staker: defaultStaker,
+            withdrawer: defaultStaker,
+            strategy: strategyMock,
+            depositSharesToWithdraw: totalDepositShares / 2
+        });
+
+        cheats.prank(defaultStaker);
+        delegationManager.queueWithdrawals(queuedWithdrawalParams0);
+
+        (
+            QueuedWithdrawalParams[] memory queuedWithdrawalParams1,
+            Withdrawal memory withdrawal1,
+            bytes32 withdrawalRoot1
+        ) = _setUpQueueWithdrawalsSingleStrat({
+            staker: defaultStaker,
+            withdrawer: defaultStaker,
+            strategy: strategyMock,
+            depositSharesToWithdraw:  totalDepositShares / 2
+        });
+
+        cheats.prank(defaultStaker);
+        delegationManager.queueWithdrawals(queuedWithdrawalParams1);
+
+        // Get queued withdrawals.
+        (Withdrawal[] memory withdrawals, uint256[][] memory shares) = delegationManager.getQueuedWithdrawals(defaultStaker);
+
+        // Sanity
+        assertEq(withdrawals.length, 2, "withdrawal.length != 2");
+        assertEq(withdrawals[0].strategies.length, 1, "withdrawals[0].strategies.length != 1");
+        assertEq(withdrawals[1].strategies.length, 1, "withdrawals[1].strategies.length != 1");
+
+        // Checks
+        assertApproxEqAbs(shares[0][0], newStakerShares / 2, 1, "shares[0][0] != newStakerShares");
+        assertApproxEqAbs(shares[1][0], newStakerShares / 2, 1, "shares[1][0] != newStakerShares");
+        assertEq(_withdrawalRoot(withdrawal0), _withdrawalRoot(withdrawals[0]), "withdrawal0 != withdrawals[0]");
+        assertEq(_withdrawalRoot(withdrawal1), _withdrawalRoot(withdrawals[1]), "withdrawal1 != withdrawals[1]");
+        assertEq(_withdrawalRoot(withdrawal0), withdrawalRoot0, "_withdrawalRoot(withdrawal0) != withdrawalRoot0");
+        assertEq(_withdrawalRoot(withdrawal1), withdrawalRoot1, "_withdrawalRoot(withdrawal1) != withdrawalRoot1");
+    }
 
     /**
      * @notice Assert that the shares returned in the view function `getQueuedWithdrawals` are unaffected from a

src/test/utils/ArrayLib.sol

@@ -152,6 +152,13 @@ library ArrayLib {
         array[0] = withdrawal;
     }
 
+    function toArray(
+        IDelegationManagerTypes.QueuedWithdrawalParams memory queuedWithdrawalParams
+    ) internal pure returns (IDelegationManagerTypes.QueuedWithdrawalParams[] memory array) {
+        array = new IDelegationManagerTypes.QueuedWithdrawalParams[](1);
+        array[0] = queuedWithdrawalParams;
+    }
+
     /// -----------------------------------------------------------------------
     /// Length Updates
     /// -----------------------------------------------------------------------
@@ -236,6 +243,16 @@ library ArrayLib {
         return array;
     }
 
+    function setLength(
+        IDelegationManagerTypes.Withdrawal[] memory array,
+        uint256 len
+    ) internal pure returns (IDelegationManagerTypes.Withdrawal[] memory) {
+        assembly {
+            mstore(array, len)
+        }
+        return array;
+    }
+
     /// -----------------------------------------------------------------------
     /// Contains
     /// -----------------------------------------------------------------------
@@ -310,6 +327,26 @@ library ArrayLib {
         return false;
     }
 
+    function contains(
+        OperatorSet[] memory array,
+        OperatorSet memory x
+    ) internal pure returns (bool) {
+        for (uint256 i; i < array.length; ++i) {
+            if (keccak256(abi.encode(array[i])) == keccak256(abi.encode(x))) return true;
+        }
+        return false;
+    }
+
+    function contains(
+        IDelegationManagerTypes.Withdrawal[] memory array,
+        IDelegationManagerTypes.Withdrawal memory x
+    ) internal pure returns (bool) {
+        for (uint256 i; i < array.length; ++i) {
+            if (keccak256(abi.encode(array[i])) == keccak256(abi.encode(x))) return true;
+        }
+        return false;
+    }
+
     /// -----------------------------------------------------------------------
     /// indexOf
     /// -----------------------------------------------------------------------
@@ -384,6 +421,26 @@ library ArrayLib {
         return type(uint256).max;
     }
 
+    function indexOf(
+        OperatorSet[] memory array,
+        OperatorSet memory x
+    ) internal pure returns (uint256) {
+        for (uint256 i; i < array.length; ++i) {
+            if (keccak256(abi.encode(array[i])) == keccak256(abi.encode(x))) return i;
+        }
+        return type(uint256).max;
+    }
+
+    function indexOf(
+        IDelegationManagerTypes.Withdrawal[] memory array,
+        IDelegationManagerTypes.Withdrawal memory x
+    ) internal pure returns (uint256) {
+        for (uint256 i; i < array.length; ++i) {
+            if (keccak256(abi.encode(array[i])) == keccak256(abi.encode(x))) return i;
+        }
+        return type(uint256).max;
+    }
+
     /// -----------------------------------------------------------------------
     /// Sorting
     /// -----------------------------------------------------------------------