diff --git a/src/contracts/core/AllocationManager.sol b/src/contracts/core/AllocationManager.sol index 30c3621758..31d2101fa6 100644 --- a/src/contracts/core/AllocationManager.sol +++ b/src/contracts/core/AllocationManager.sol @@ -200,7 +200,9 @@ contract AllocationManager is // the deallocation delay. This magnitude remains slashable until then. deallocationQueue[operator][strategy].pushBack(operatorSet.key()); - allocation.effectBlock = uint32(block.number) + DEALLOCATION_DELAY; + // deallocations are slashable in the window [block.number, block.number + deallocationDelay] + // therefore, the effectBlock is set to the block right after the slashable window + allocation.effectBlock = uint32(block.number) + DEALLOCATION_DELAY + 1; } else { // Deallocation immediately updates/frees magnitude if the operator is not slashable info.encumberedMagnitude = _addInt128(info.encumberedMagnitude, allocation.pendingDiff); @@ -446,7 +448,9 @@ contract AllocationManager is function _isOperatorSlashable(address operator, OperatorSet memory operatorSet) internal view returns (bool) { RegistrationStatus memory status = registrationStatus[operator][operatorSet.key()]; - return status.registered || block.number < status.slashableUntil; + // slashableUntil returns the last block the operator is slashable in so we check for + // less than or equal to + return status.registered || block.number <= status.slashableUntil; } /// @notice returns whether the operator's allocation is slashable in the given operator set diff --git a/src/contracts/core/DelegationManager.sol b/src/contracts/core/DelegationManager.sol index 108d772ceb..d415c3bcac 100644 --- a/src/contracts/core/DelegationManager.sol +++ b/src/contracts/core/DelegationManager.sol @@ -533,7 +533,7 @@ contract DelegationManager is * @dev This function completes a queued withdrawal for a staker. * This will apply any slashing that has occurred since the the withdrawal was queued by multiplying the withdrawal's * scaledShares by the operator's maxMagnitude for each strategy. This ensures that any slashing that has occurred - * during the period the withdrawal was queued until its completable timestamp is applied to the withdrawal amount. + * during the period the withdrawal was queued until its slashableUntil block is applied to the withdrawal amount. * If receiveAsTokens is true, then these shares will be withdrawn as tokens. * If receiveAsTokens is false, then they will be redeposited according to the current operator the staker is delegated to, * and added back to the operator's delegatedShares. @@ -550,8 +550,10 @@ contract DelegationManager is uint256[] memory prevSlashingFactors; { - uint32 completableBlock = withdrawal.startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS; - require(completableBlock <= uint32(block.number), WithdrawalDelayNotElapsed()); + // slashableUntil is block inclusive so we need to check if the current block is strictly greater than the slashableUntil block + // meaning the withdrawal can be completed. + uint32 slashableUntil = withdrawal.startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS; + require(slashableUntil < uint32(block.number), WithdrawalDelayNotElapsed()); // Given the max magnitudes of the operator the staker was originally delegated to, calculate // the slashing factors for each of the withdrawal's strategies. @@ -559,7 +561,7 @@ contract DelegationManager is staker: withdrawal.staker, operator: withdrawal.delegatedTo, strategies: withdrawal.strategies, - blockNumber: completableBlock + blockNumber: slashableUntil }); } @@ -761,9 +763,12 @@ contract DelegationManager is ) internal view returns (uint256) { // Fetch the cumulative scaled shares sitting in the withdrawal queue both now and before // the withdrawal delay. + // NOTE: We want all the shares in the window [block.number - MIN_WITHDRAWAL_DELAY_BLOCKS, block.number] + // as this is all slashable and since prevCumulativeScaledShares is being subtracted from curCumulativeScaledShares + // we do a -1 on the block number to also include (block.number - MIN_WITHDRAWAL_DELAY_BLOCKS) as slashable. uint256 curCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].latest(); uint256 prevCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].upperLookup({ - key: uint32(block.number) - MIN_WITHDRAWAL_DELAY_BLOCKS + key: uint32(block.number) - MIN_WITHDRAWAL_DELAY_BLOCKS - 1 }); // The difference between these values represents the number of scaled shares that entered the @@ -937,7 +942,24 @@ contract DelegationManager is withdrawals[i] = queuedWithdrawals[withdrawalRoots[i]]; shares[i] = new uint256[](withdrawals[i].strategies.length); - uint256[] memory slashingFactors = _getSlashingFactors(staker, operator, withdrawals[i].strategies); + uint32 slashableUntil = withdrawals[i].startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS; + + uint256[] memory slashingFactors; + // If slashableUntil block is in the past, read the slashing factors at that block + // Otherwise read the current slashing factors. Note that if the slashableUntil block is the current block + // or in the future then the slashing factors are still subject to change before the withdrawal is completable + // and the shares withdrawn to be less + if (slashableUntil < uint32(block.number)) { + slashingFactors = _getSlashingFactorsAtBlock({ + staker: staker, + operator: operator, + strategies: withdrawals[i].strategies, + blockNumber: slashableUntil + }); + } else { + slashingFactors = + _getSlashingFactors({staker: staker, operator: operator, strategies: withdrawals[i].strategies}); + } for (uint256 j; j < withdrawals[i].strategies.length; ++j) { shares[i][j] = SlashingLib.scaleForCompleteWithdrawal({ diff --git a/src/test/integration/IntegrationBase.t.sol b/src/test/integration/IntegrationBase.t.sol index 9e0019c51b..63f4c2edb3 100644 --- a/src/test/integration/IntegrationBase.t.sol +++ b/src/test/integration/IntegrationBase.t.sol @@ -1317,7 +1317,7 @@ abstract contract IntegrationBase is IntegrationDeployer { for (uint i = 0; i < withdrawals.length; ++i) { if (withdrawals[i].startBlock > latest) latest = withdrawals[i].startBlock; } - cheats.roll(latest + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(latest + delegationManager.minWithdrawalDelayBlocks() + 1); } /// @dev Rolls forward by the default allocation delay blocks. @@ -1328,7 +1328,7 @@ abstract contract IntegrationBase is IntegrationDeployer { /// @dev Rolls forward by the default deallocation delay blocks. function _rollBlocksForCompleteDeallocation() internal { - cheats.roll(block.number + allocationManager.DEALLOCATION_DELAY()); + cheats.roll(block.number + allocationManager.DEALLOCATION_DELAY() + 1); } /// @dev Uses timewarp modifier to get the operator set strategy allocations at the last snapshot. diff --git a/src/test/unit/AllocationManagerUnit.t.sol b/src/test/unit/AllocationManagerUnit.t.sol index d5a90bc7e8..06bd5dab33 100644 --- a/src/test/unit/AllocationManagerUnit.t.sol +++ b/src/test/unit/AllocationManagerUnit.t.sol @@ -1131,7 +1131,7 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests // Deallocate allocationManager.modifyAllocations(defaultOperator, deallocateParams); - uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); cheats.stopPrank(); uint256 magnitudeAllocated = allocateParams[0].newMagnitudes[0]; @@ -1265,7 +1265,7 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests // Validate event for the deallocation cheats.expectEmit(true, true, true, true, address(allocationManager)); - emit AllocationUpdated(defaultOperator, defaultOperatorSet, strategyMock, 0, uint32(block.number + DEALLOCATION_DELAY)); + emit AllocationUpdated(defaultOperator, defaultOperatorSet, strategyMock, 0, uint32(block.number + DEALLOCATION_DELAY + 1)); // Slash operator for 100% cheats.prank(defaultAVS); @@ -1285,12 +1285,12 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests operator: defaultOperator, operatorSet: defaultOperatorSet, strategy: strategyMock, - expectedAllocation: Allocation({currentMagnitude: 0, pendingDiff: 0, effectBlock: uint32(block.number) + DEALLOCATION_DELAY}), + expectedAllocation: Allocation({currentMagnitude: 0, pendingDiff: 0, effectBlock: uint32(block.number) + DEALLOCATION_DELAY + 1}), expectedMagnitudes: Magnitudes({encumbered: 0, max: 0, allocatable: 0}) }); // Complete deallocation - cheats.roll(uint32(block.number) + DEALLOCATION_DELAY); + cheats.roll(uint32(block.number) + DEALLOCATION_DELAY + 1); allocationManager.clearDeallocationQueue(defaultOperator, defaultStrategies, _maxNumToClear()); // Validate allocatable amount is 0 @@ -1319,7 +1319,7 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests AllocateParams[] memory deallocateParams = _newAllocateParams(defaultOperatorSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, deallocateParams); - uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); // Warp to deallocation effect block cheats.roll(deallocationEffectBlock); @@ -1598,7 +1598,7 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); allocationManager.modifyAllocations(defaultOperator, deallocateParams); - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); cheats.stopPrank(); // Slash operator for some random amount (1% -> 99%). @@ -1664,6 +1664,165 @@ contract AllocationManagerUnitTests_SlashOperator is AllocationManagerUnitTests }) }); } + + /** + * Allocates magnitude to an operator, deallocates all, warps to deallocation effect block and attempts to slash + * Asserts that the operator is not slashed + */ + function test_noFundsSlashedAfterDeallocationDelay() public { + // Allocate all magnitude + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, WAD)); + cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); + + // Deallocate all + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, 0)); + + // Warp to deallocation effect block + cheats.roll(block.number + DEALLOCATION_DELAY + 1); + + // Slash operator for all wad + cheats.prank(defaultAVS); + allocationManager.slashOperator( + defaultAVS, + SlashingParams({ + operator: defaultOperator, + operatorSetId: defaultOperatorSet.id, + strategies: defaultStrategies, + wadsToSlash: WAD.toArrayU256(), + description: "test" + }) + ); + + // Assert that the operator's max magnitude and allocatable magnitude are still WAD + assertEq( + WAD, + allocationManager.getAllocatableMagnitude(defaultOperator, strategyMock), + "Allocatable magnitude should be WAD" + ); + assertEq( + WAD, + allocationManager.getMaxMagnitude(defaultOperator, strategyMock), + "Max magnitude should be WAD" + ); + } + + function testRevert_noFundsSlashedAfterDeregistration() public { + // Allocate all magnitude + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, WAD)); + cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); + + // Deregister operator + DeregisterParams memory deregisterParams = DeregisterParams({ + operator: defaultOperator, + avs: defaultAVS, + operatorSetIds: defaultOperatorSet.id.toArrayU32() + }); + cheats.prank(defaultOperator); + allocationManager.deregisterFromOperatorSets(deregisterParams); + + // Warp to deallocation effect block + cheats.roll(block.number + DEALLOCATION_DELAY + 1); + + // Slash operator for all wad + cheats.expectRevert(OperatorNotSlashable.selector); + cheats.prank(defaultAVS); + allocationManager.slashOperator( + defaultAVS, + SlashingParams({ + operator: defaultOperator, + operatorSetId: defaultOperatorSet.id, + strategies: defaultStrategies, + wadsToSlash: WAD.toArrayU256(), + description: "test" + }) + ); + } + + function test_deallocationSlashedJustBeforeEffectBlock() public { + // Allocate all magnitude + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, WAD)); + cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); + + // Deallocate all + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, 0)); + + // Warp to just before deallocation effect block + cheats.roll(block.number + DEALLOCATION_DELAY); + + // Slash operator for all wad + cheats.prank(defaultAVS); + allocationManager.slashOperator( + defaultAVS, + SlashingParams({ + operator: defaultOperator, + operatorSetId: defaultOperatorSet.id, + strategies: defaultStrategies, + wadsToSlash: WAD.toArrayU256(), + description: "test" + }) + ); + + // Assert that the operator has no max magnitude or allocatable magnitude + assertEq( + 0, + allocationManager.getAllocatableMagnitude(defaultOperator, strategyMock), + "Allocatable magnitude should be 0" + ); + assertEq( + 0, + allocationManager.getMaxMagnitude(defaultOperator, strategyMock), + "Max magnitude should be 0" + ); + } + + function test_deregisteredOperatorSlashableBeforeDelay() public { + // Allocate all magnitude + cheats.prank(defaultOperator); + allocationManager.modifyAllocations(defaultOperator, _newAllocateParams(defaultOperatorSet, WAD)); + cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); + + // Deregister operator + DeregisterParams memory deregisterParams = DeregisterParams({ + operator: defaultOperator, + avs: defaultAVS, + operatorSetIds: defaultOperatorSet.id.toArrayU32() + }); + cheats.prank(defaultOperator); + allocationManager.deregisterFromOperatorSets(deregisterParams); + + // Roll to the slashableUntil at block + cheats.roll(block.number + DEALLOCATION_DELAY); + + // Slash operator for all wad + cheats.prank(defaultAVS); + allocationManager.slashOperator( + defaultAVS, + SlashingParams({ + operator: defaultOperator, + operatorSetId: defaultOperatorSet.id, + strategies: defaultStrategies, + wadsToSlash: WAD.toArrayU256(), + description: "test" + }) + ); + + // Assert that the operator has no max magnitude or allocatable magnitude + assertEq( + 0, + allocationManager.getAllocatableMagnitude(defaultOperator, strategyMock), + "Allocatable magnitude should be 0" + ); + assertEq( + 0, + allocationManager.getMaxMagnitude(defaultOperator, strategyMock), + "Max magnitude should be 0" + ); + } } contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTests { @@ -2190,14 +2349,14 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe operatorSet: defaultOperatorSet, strategy: strategyMock, magnitude: secondMod, - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); // Check storage after dealloc - uint32 effectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 effectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); _checkAllocationStorage({ operator: defaultOperator, @@ -2287,7 +2446,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe // Deallocate AllocateParams[] memory deallocateParams = r.DeallocateParams(allocateParams); - uint32 deallocEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, deallocateParams); @@ -2449,7 +2608,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe operatorSet: deallocSets[i], strategy: strategyMock, magnitude: 0, - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); } @@ -2463,7 +2622,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe ); // Move forward to deallocation completion - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); // Check that we now have sufficient allocatable magnitude assertEq( @@ -2536,14 +2695,14 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe operatorSet: defaultOperatorSet, strategy: strategyMock, magnitude: 0, - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); // Warp to completion and clear deallocation queue - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); allocationManager.clearDeallocationQueue(defaultOperator, defaultStrategies, uint16(1).toArrayU16()); // Check storage @@ -2652,7 +2811,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe operatorSet: deallocateParams[i].operatorSet, strategy: deallocateParams[i].strategies[j], magnitude: deallocateParams[i].newMagnitudes[j], - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); } } @@ -2670,7 +2829,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe expectedAllocation: Allocation({ currentMagnitude: allocateParams[i].newMagnitudes[j], pendingDiff: expectedDiff, - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }), expectedMagnitudes: Magnitudes({ encumbered: allocateParams[i].newMagnitudes[j], @@ -2682,7 +2841,7 @@ contract AllocationManagerUnitTests_ModifyAllocations is AllocationManagerUnitTe } // Warp to deallocation complete block - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); for (uint256 i; i < allocateParams.length; ++i) { for (uint256 j = 0; j < allocateParams[i].strategies.length; j++) { _checkAllocationStorage({ @@ -2799,7 +2958,7 @@ contract AllocationManagerUnitTests_ClearDeallocationQueue is AllocationManagerU operatorSet: defaultOperatorSet, strategy: strategyMock, magnitude: deallocateParams[0].newMagnitudes[0], - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, deallocateParams); @@ -2815,7 +2974,7 @@ contract AllocationManagerUnitTests_ClearDeallocationQueue is AllocationManagerU expectedAllocation: Allocation({ currentMagnitude: allocateParams[0].newMagnitudes[0], pendingDiff: -int128(uint128(allocateParams[0].newMagnitudes[0] - deallocateParams[0].newMagnitudes[0])), - effectBlock: uint32(block.number + DEALLOCATION_DELAY) + effectBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }), expectedMagnitudes: Magnitudes({ encumbered: allocateParams[0].newMagnitudes[0], @@ -2825,7 +2984,7 @@ contract AllocationManagerUnitTests_ClearDeallocationQueue is AllocationManagerU }); // Warp to deallocation complete block - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); // Clear queue _checkClearDeallocationQueueEvents({ @@ -2866,7 +3025,7 @@ contract AllocationManagerUnitTests_ClearDeallocationQueue is AllocationManagerU cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); // Deallocate half from default operator set - uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); AllocateParams[] memory firstDeallocation = _newAllocateParams(defaultOperatorSet, 25e16); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, firstDeallocation); @@ -2950,7 +3109,7 @@ contract AllocationManagerUnitTests_ClearDeallocationQueue is AllocationManagerU assertEq(allocationEffectBlock, allocation.effectBlock, "effect block not correct"); // Deallocate all from opSet1 - uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); AllocateParams[] memory firstDeallocation = _newAllocateParams(defaultOperatorSet, 0); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, firstDeallocation); @@ -3658,11 +3817,11 @@ contract AllocationManagerUnitTests_getSlashableStake is AllocationManagerUnitTe operator: defaultOperator, strategies: defaultStrategies, expectedSlashableStake: DEFAULT_OPERATOR_SHARES.mulWad(secondMod), - futureBlock: uint32(block.number + DEALLOCATION_DELAY) + futureBlock: uint32(block.number + DEALLOCATION_DELAY + 1) }); // Warp to deallocation effect block - cheats.roll(block.number + DEALLOCATION_DELAY); + cheats.roll(block.number + DEALLOCATION_DELAY + 1); // 5. Check slashable stake at the deallocation effect block _checkSlashableStake({ @@ -3702,7 +3861,7 @@ contract AllocationManagerUnitTests_getSlashableStake is AllocationManagerUnitTe // Deallocate allocationManager.modifyAllocations(defaultOperator, deallocateParams); - uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY); + uint32 deallocationEffectBlock = uint32(block.number + DEALLOCATION_DELAY + 1); cheats.stopPrank(); // Check slashable stake after deallocation (still pending; no change) diff --git a/src/test/unit/DelegationUnit.t.sol b/src/test/unit/DelegationUnit.t.sol index 92650a1292..106040d016 100644 --- a/src/test/unit/DelegationUnit.t.sol +++ b/src/test/unit/DelegationUnit.t.sol @@ -124,7 +124,7 @@ contract DelegationManagerUnitTests is EigenLayerUnitTestSetup, IDelegationManag // Roll blocks forward so that block.number - MIN_WITHDRAWAL_DELAY_BLOCKS doesn't revert // in _getSlashableSharesInQueue - cheats.roll(MIN_WITHDRAWAL_DELAY_BLOCKS); + cheats.roll(MIN_WITHDRAWAL_DELAY_BLOCKS + 1); // Exclude delegation manager from fuzzed tests isExcludedFuzzAddress[address(delegationManager)] = true; @@ -4672,7 +4672,7 @@ contract DelegationManagerUnitTests_undelegate is DelegationManagerUnitTests { // complete withdrawal as shares, should add back delegated shares to operator due to delegating again IERC20[] memory tokens = new IERC20[](1); tokens[0] = IERC20(strategyMock.underlyingToken()); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); cheats.prank(defaultStaker); delegationManager.completeQueuedWithdrawal(withdrawal, tokens, false); @@ -4974,7 +4974,7 @@ contract DelegationManagerUnitTests_redelegate is DelegationManagerUnitTests { delegationManager.undelegate(staker); // 4. Delegate to operator again with shares added back { - cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks() + 1); IERC20[] memory strategyTokens = new IERC20[](1); strategyTokens[0] = IERC20(strategyMock.underlyingToken()); IERC20[] memory beaconTokens = new IERC20[](1); @@ -5905,7 +5905,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); // Roll to completion block - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); // resize tokens array IERC20[] memory newTokens = new IERC20[](0); @@ -5956,7 +5956,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); assertTrue(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be pending"); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); cheats.prank(defaultStaker); delegationManager.completeQueuedWithdrawal(withdrawal, tokens, true); assertFalse(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be completed and marked false now"); @@ -5994,12 +5994,54 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage }); // prank as withdrawer address - cheats.roll(withdrawal.startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS - 1); + cheats.roll(withdrawal.startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS); cheats.expectRevert(WithdrawalDelayNotElapsed.selector); cheats.prank(defaultStaker); delegationManager.completeQueuedWithdrawal(withdrawal, tokens, receiveAsTokens); } + /// @notice Verifies that when we complete a withdrawal as shares after a full slash, we revert + function test_revert_fullySlashed() public { + // Register operator + _registerOperatorWithBaseDetails(defaultOperator); + _setOperatorMagnitude(defaultOperator, strategyMock, WAD); + + // Set the staker deposits in the strategies + uint256 depositAmount = 100e18; + strategyManagerMock.addDeposit(defaultStaker, strategyMock, depositAmount); + _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); + + // Queue withdrawal + uint256 withdrawalAmount = depositAmount; + ( + QueuedWithdrawalParams[] memory queuedWithdrawalParams, + Withdrawal memory withdrawal, + bytes32 withdrawalRoot + ) = _setUpQueueWithdrawalsSingleStrat({ + staker: defaultStaker, + withdrawer: defaultStaker, + strategy: strategyMock, + depositSharesToWithdraw: withdrawalAmount + }); + cheats.prank(defaultStaker); + delegationManager.queueWithdrawals(queuedWithdrawalParams); + + // Warp to just before the MIN_WITHDRAWAL_DELAY_BLOCKS + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + + // Slash all of operator's shares + _setOperatorMagnitude(defaultOperator, strategyMock, 0); + cheats.prank(address(allocationManagerMock)); + delegationManager.burnOperatorShares(defaultOperator, strategyMock, WAD, 0); + + // Complete withdrawal as shares and assert that operator has no shares increased + cheats.roll(block.number + 1); + IERC20[] memory tokens = strategyMock.underlyingToken().toArray(); + cheats.expectRevert(FullySlashed.selector); + cheats.prank(defaultStaker); + delegationManager.completeQueuedWithdrawal(withdrawal, tokens, false); + } + /** * Test completing multiple queued withdrawals for a single strategy by passing in the withdrawals */ @@ -6030,7 +6072,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage bool[] memory receiveAsTokensArray = receiveAsTokens.toArray(numWithdrawals); // completeQueuedWithdrawal - cheats.roll(withdrawals[0].startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawals[0].startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawals_expectEmit( CompleteQueuedWithdrawalsEmitStruct({ withdrawals: withdrawals, @@ -6120,7 +6162,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage assertTrue(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be pending"); // completeQueuedWithdrawal - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawal_expectEmit( CompleteQueuedWithdrawalEmitStruct({ withdrawal: withdrawal, @@ -6230,7 +6272,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage { IERC20[] memory tokens = new IERC20[](1); tokens[0] = IERC20(strategyMock.underlyingToken()); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawal_expectEmit( CompleteQueuedWithdrawalEmitStruct({ withdrawal: withdrawal, @@ -6337,7 +6379,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage { IERC20[] memory tokens = new IERC20[](1); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawal_expectEmit( CompleteQueuedWithdrawalEmitStruct({ withdrawal: withdrawal, @@ -6434,7 +6476,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage ) = delegationManager.getWithdrawableShares(defaultStaker, beaconChainETHStrategy.toArray()); uint256 operatorSharesBefore = delegationManager.operatorShares(defaultOperator, beaconChainETHStrategy); IERC20[] memory tokens = new IERC20[](1); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawal_expectEmit( CompleteQueuedWithdrawalEmitStruct({ withdrawal: withdrawal, @@ -6498,7 +6540,7 @@ contract DelegationManagerUnitTests_completeQueuedWithdrawal is DelegationManage strategyManagerMock.setDelegationManager(delegationManager); // completeQueuedWithdrawal - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); _completeQueuedWithdrawal_expectEmit( CompleteQueuedWithdrawalEmitStruct({ withdrawal: withdrawal, @@ -6539,6 +6581,189 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests assertEq(delegationManager.operatorShares(defaultOperator, strategyMock), 0, "shares should not have changed"); } + /// @notice Verifies that shares are burnable for a withdrawal slashed just before the MIN_WITHDRAWAL_DELAY_BLOCKS is hit + function test_sharesBurnableAtMinDelayBlocks() public { + // Register operator + _registerOperatorWithBaseDetails(defaultOperator); + _setOperatorMagnitude(defaultOperator, strategyMock, WAD); + + // Set the staker deposits in the strategies + uint256 depositAmount = 100e18; + strategyManagerMock.addDeposit(defaultStaker, strategyMock, depositAmount); + _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); + + // Queue withdrawal + uint256 withdrawalAmount = depositAmount; + ( + QueuedWithdrawalParams[] memory queuedWithdrawalParams, + Withdrawal memory withdrawal, + bytes32 withdrawalRoot + ) = _setUpQueueWithdrawalsSingleStrat({ + staker: defaultStaker, + withdrawer: defaultStaker, + strategy: strategyMock, + depositSharesToWithdraw: withdrawalAmount + }); + cheats.prank(defaultStaker); + delegationManager.queueWithdrawals(queuedWithdrawalParams); + + // Warp to just before the MIN_WITHDRAWAL_DELAY_BLOCKS + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + + uint256 slashableSharesInQueue = delegationManager.getSlashableSharesInQueue(defaultOperator, strategyMock); + + // Slash all of operator's shares + _setOperatorMagnitude(defaultOperator, strategyMock, 0); + cheats.expectEmit(true, true, true, true, address(delegationManager)); + emit OperatorSharesBurned(defaultOperator, strategyMock, depositAmount); + cheats.prank(address(allocationManagerMock)); + delegationManager.burnOperatorShares(defaultOperator, strategyMock, WAD, 0); + + uint256 slashableSharesInQueueAfter = delegationManager.getSlashableSharesInQueue(defaultOperator, strategyMock); + + // Complete withdrawal as tokens and assert that nothing is returned + cheats.roll(block.number + 1); + IERC20[] memory tokens = strategyMock.underlyingToken().toArray(); + cheats.expectCall( + address(strategyManagerMock), + abi.encodeWithSelector( + IShareManager.withdrawSharesAsTokens.selector, + defaultStaker, + strategyMock, + strategyMock.underlyingToken(), + 0 + ) + ); + cheats.prank(defaultStaker); + delegationManager.completeQueuedWithdrawal(withdrawal, tokens, true); + + assertEq( + slashableSharesInQueue, + depositAmount, + "the withdrawal in queue from block.number - minWithdrawalDelayBlocks should still be included" + ); + assertEq( + slashableSharesInQueueAfter, + 0, + "slashable shares in queue should be 0 after burning" + ); + } + + /// @notice Verifies that shares are NOT burnable for a withdrawal queued just before the MIN_WITHDRAWAL_DELAY_BLOCKS + function test_sharesNotBurnableWhenWithdrawalCompletable() public { + // Register operator + _registerOperatorWithBaseDetails(defaultOperator); + _setOperatorMagnitude(defaultOperator, strategyMock, WAD); + + // Set the staker deposits in the strategies + uint256 depositAmount = 100e18; + strategyManagerMock.addDeposit(defaultStaker, strategyMock, depositAmount); + _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); + + // Queue withdrawal + uint256 withdrawalAmount = depositAmount; + ( + QueuedWithdrawalParams[] memory queuedWithdrawalParams, + Withdrawal memory withdrawal, + bytes32 withdrawalRoot + ) = _setUpQueueWithdrawalsSingleStrat({ + staker: defaultStaker, + withdrawer: defaultStaker, + strategy: strategyMock, + depositSharesToWithdraw: withdrawalAmount + }); + cheats.prank(defaultStaker); + delegationManager.queueWithdrawals(queuedWithdrawalParams); + + // Warp to completion time + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); + uint256 slashableShares = delegationManager.getSlashableSharesInQueue(defaultOperator, strategyMock); + assertEq(slashableShares, 0, "shares should not be slashable"); + + // Slash all of operator's shares + _setOperatorMagnitude(defaultOperator, strategyMock, 0); + cheats.prank(address(allocationManagerMock)); + delegationManager.burnOperatorShares(defaultOperator, strategyMock, WAD, 0); + + // Complete withdrawal as tokens and assert that we call back into teh SM with 100 tokens + IERC20[] memory tokens = strategyMock.underlyingToken().toArray(); + cheats.expectCall( + address(strategyManagerMock), + abi.encodeWithSelector( + IShareManager.withdrawSharesAsTokens.selector, + defaultStaker, + strategyMock, + strategyMock.underlyingToken(), + 100e18 + ) + ); + cheats.prank(defaultStaker); + delegationManager.completeQueuedWithdrawal(withdrawal, tokens, true); + } + + /** + * @notice Queues 5 withdrawals at different blocks. Then, warps such that the first 2 are completable. Validates the slashable shares + */ + function test_slashableSharesInQueue() public { + // Register operator + _registerOperatorWithBaseDetails(defaultOperator); + _setOperatorMagnitude(defaultOperator, strategyMock, WAD); + + // Set the staker deposits in the strategies + uint256 depositAmount = 120e18; + strategyManagerMock.addDeposit(defaultStaker, strategyMock, depositAmount); + _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); + + // Queue 5 withdrawals + uint256 startBlock = block.number; + uint256 withdrawalAmount = depositAmount / 6; + for(uint256 i = 0; i < 5; i++) { + ( + QueuedWithdrawalParams[] memory queuedWithdrawalParams, + Withdrawal memory withdrawal, + bytes32 withdrawalRoot + ) = _setUpQueueWithdrawalsSingleStrat({ + staker: defaultStaker, + withdrawer: defaultStaker, + strategy: strategyMock, + depositSharesToWithdraw: withdrawalAmount + }); + cheats.prank(defaultStaker); + delegationManager.queueWithdrawals(queuedWithdrawalParams); + cheats.roll(startBlock + i + 1); + } + + // Warp to completion time for the first 2 withdrawals + // First withdrawal queued at startBlock. Second queued at startBlock + 1 + cheats.roll(startBlock + 1 + delegationManager.minWithdrawalDelayBlocks() + 1); + + // Get slashable shares + uint256 slashableSharesInQueue = delegationManager.getSlashableSharesInQueue(defaultOperator, strategyMock); + assertEq(slashableSharesInQueue, depositAmount/6 * 3, "slashable shares in queue should be 3/6 of the deposit amount"); + + // Slash all of operator's shares + _setOperatorMagnitude(defaultOperator, strategyMock, 0); + cheats.prank(address(allocationManagerMock)); + cheats.expectEmit(true, true, true, true, address(delegationManager)); + emit OperatorSharesDecreased( + defaultOperator, + address(0), + strategyMock, + depositAmount / 6 // 1 withdrawal not queued so decreased + ); + cheats.expectEmit(true, true, true, true, address(delegationManager)); + emit OperatorSharesBurned( + defaultOperator, + strategyMock, + depositAmount / 6 * 4 // 4 parts are burned + ); + delegationManager.burnOperatorShares(defaultOperator, strategyMock, WAD, 0); + + // Assert slashable shares + slashableSharesInQueue = delegationManager.getSlashableSharesInQueue(defaultOperator, strategyMock); + assertEq(slashableSharesInQueue, 0); + } + /** * @notice Verifies that `DelegationManager.burnOperatorShares` properly decreases the delegated `shares` that the operator * who the `defaultStaker` is delegated to has in the strategies @@ -6743,7 +6968,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests withdrawAmount, "there should be withdrawAmount slashable shares in queue" ); - cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1); } uint256 operatorSharesBefore = delegationManager.operatorShares(operator, strategyMock); @@ -7139,7 +7364,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests "there should be depositAmount slashable shares in queue" ); // Check slashable shares in queue before and when the withdrawal is completable - completableBlock = withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks(); + completableBlock = withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1; IERC20[] memory tokenArray = strategyMock.underlyingToken().toArray(); // 3.2 roll to right before withdrawal is completable, check that slashable shares are still there @@ -8062,7 +8287,7 @@ contract DelegationManagerUnitTests_Lifecycle is DelegationManagerUnitTests { bytes32 withdrawalRoot = keccak256(abi.encode(withdrawal)); assertTrue(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be pending"); - cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks() + 1); cheats.prank(staker); delegationManager.completeQueuedWithdrawal(withdrawal, tokenMock.toArray(), false); @@ -8316,9 +8541,144 @@ contract DelegationManagerUnitTests_ConvertToDepositShares is DelegationManagerU cheats.prank(defaultStaker); delegationManager.queueWithdrawals(queuedWithdrawalParams); - cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks()); + cheats.roll(block.number + delegationManager.minWithdrawalDelayBlocks() + 1); cheats.prank(defaultStaker); delegationManager.completeQueuedWithdrawal(withdrawal, tokenMock.toArray(), false); } } +contract DelegationManagerUnitTests_getQueuedWithdrawals is DelegationManagerUnitTests { + using ArrayLib for *; + + /** + * @notice Assert that the shares returned in the view function `getQueuedWithdrawals` are unaffected from a + * slash that occurs after the withdrawal is completed. Also assert that completing the withdrawal matches the + * expected withdrawn shares from the view function. + * Slashing on the completableBlock of the withdrawal should have no affect on the withdrawn shares. + */ + function test_getQueuedWithdrawals_SlashAfterWithdrawalCompletion(Randomness r) public rand(r) { + uint256 depositAmount = r.Uint256(1, MAX_STRATEGY_SHARES); + + // Deposit Staker + strategyManagerMock.addDeposit(defaultStaker, strategyMock, depositAmount); + + // Register operator and delegate to it + _registerOperatorWithBaseDetails(defaultOperator); + _delegateToOperatorWhoAcceptsAllStakers(defaultStaker, defaultOperator); + + // Queue withdrawal + ( + QueuedWithdrawalParams[] memory queuedWithdrawalParams, + Withdrawal memory withdrawal, + bytes32 withdrawalRoot + ) = _setUpQueueWithdrawalsSingleStrat({ + staker: defaultStaker, + withdrawer: defaultStaker, + strategy: strategyMock, + depositSharesToWithdraw: depositAmount + }); + { + uint256 operatorSharesBeforeQueue = delegationManager.operatorShares(defaultOperator, strategyMock); + cheats.prank(defaultStaker); + delegationManager.queueWithdrawals(queuedWithdrawalParams); + + assertTrue(delegationManager.pendingWithdrawals(withdrawalRoot), "withdrawalRoot should be pending"); + uint256 operatorSharesAfterQueue = delegationManager.operatorShares(defaultOperator, strategyMock); + uint256 sharesWithdrawn = _calcWithdrawableShares({ + depositShares: depositAmount, + depositScalingFactor: uint256(WAD), + slashingFactor: uint256(WAD) + }); + assertEq( + operatorSharesAfterQueue, + operatorSharesBeforeQueue - sharesWithdrawn, + "operator shares should be decreased after queue" + ); + } + + // Slash operator 50% while staker has queued withdrawal + { + uint256 operatorSharesAfterQueue = delegationManager.operatorShares(defaultOperator, strategyMock); + (uint256 sharesToDecrement, ) = _calcSlashedAmount({ + operatorShares: operatorSharesAfterQueue, + prevMaxMagnitude: uint64(WAD), + newMaxMagnitude: 50e16 + }); + _setOperatorMagnitude(defaultOperator, strategyMock, 50e16); + cheats.prank(address(allocationManagerMock)); + delegationManager.burnOperatorShares(defaultOperator, withdrawal.strategies[0], uint64(WAD), 50e16); + uint256 operatorSharesAfterSlash = delegationManager.operatorShares(defaultOperator, strategyMock); + assertEq( + operatorSharesAfterSlash, + operatorSharesAfterQueue - sharesToDecrement, + "operator shares should be decreased after slash" + ); + } + + // Assert that the getQueuedWithdrawals returns shares that are halved as a result of being slashed 50% + { + (Withdrawal[] memory withdrawals, uint256[][] memory shares) = delegationManager.getQueuedWithdrawals(defaultStaker); + assertEq(withdrawals.length, 1, "withdrawals.length != 1"); + assertEq(withdrawals[0].strategies.length, 1, "withdrawals[0].strategies.length != 1"); + assertEq(shares[0][0], depositAmount / 2, "shares[0][0] != depositAmount / 2"); + } + + // Roll blocks to after withdrawal completion + uint32 completableBlock = withdrawal.startBlock + delegationManager.minWithdrawalDelayBlocks() + 1; + cheats.roll(completableBlock); + + // slash operator 50% again + { + uint256 operatorShares = delegationManager.operatorShares(defaultOperator, strategyMock); + (uint256 sharesToDecrement, ) = _calcSlashedAmount({ + operatorShares: operatorShares, + prevMaxMagnitude: 50e16, + newMaxMagnitude: 25e16 + }); + _setOperatorMagnitude(defaultOperator, strategyMock, 25e16); + cheats.prank(address(allocationManagerMock)); + delegationManager.burnOperatorShares(defaultOperator, withdrawal.strategies[0], 50e16, 25e16); + uint256 operatorSharesAfterSecondSlash = delegationManager.operatorShares(defaultOperator, strategyMock); + assertEq( + operatorSharesAfterSecondSlash, + operatorShares - sharesToDecrement, + "operator shares should be decreased after slash" + ); + } + + // Assert that the getQueuedWithdrawals returns shares that are halved as a result of being slashed 50% and hasn't been + // affected by the second slash + uint256 expectedSharesIncrease = depositAmount / 2; + uint256 queuedWithdrawableShares; + { + (Withdrawal[] memory withdrawals, uint256[][] memory shares) = delegationManager.getQueuedWithdrawals(defaultStaker); + queuedWithdrawableShares = shares[0][0]; + assertEq(withdrawals.length, 1, "withdrawals.length != 1"); + assertEq(withdrawals[0].strategies.length, 1, "withdrawals[0].strategies.length != 1"); + assertEq(queuedWithdrawableShares, depositAmount / 2, "queuedWithdrawableShares != withdrawalAmount / 2"); + } + + // Complete queued Withdrawal with shares added back. Since total deposit slashed by 50% and not 75% + (uint256[] memory withdrawableSharesBefore, ) = delegationManager.getWithdrawableShares(defaultStaker, withdrawal.strategies); + cheats.prank(defaultStaker); + delegationManager.completeQueuedWithdrawal(withdrawal, tokenMock.toArray(), false); + (uint256[] memory withdrawableSharesAfter, ) = delegationManager.getWithdrawableShares(defaultStaker, withdrawal.strategies); + + // Added shares + assertEq( + withdrawableSharesAfter[0], + withdrawableSharesBefore[0] + expectedSharesIncrease, + "withdrawableShares should be increased by expectedSharesIncrease" + ); + assertEq( + expectedSharesIncrease, + queuedWithdrawableShares, + "expectedSharesIncrease should be equal to queuedWithdrawableShares" + ); + assertEq( + block.number, + completableBlock, + "block.number should be the completableBlock" + ); + } +}