Skip to content

Commit

Permalink
fix: slashable shares in queue amount
Browse files Browse the repository at this point in the history
  • Loading branch information
8sunyuan committed Nov 19, 2024
1 parent 4799402 commit 9f20ad5
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 22 deletions.
27 changes: 20 additions & 7 deletions src/contracts/core/DelegationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ contract DelegationManager is
/// forgefmt: disable-next-item
(uint256 sharesToDecrement, uint256 sharesToBurn) = SlashingLib.calcSlashedAmount({
operatorShares: operatorShares[operator][strategy],
slashableSharesInQueue: _getSlashableSharesInQueue(operator, strategy, newMaxMagnitude),
slashableSharesInQueue: _getSlashedSharesInQueue(operator, strategy, prevMaxMagnitude, newMaxMagnitude),
prevMaxMagnitude: prevMaxMagnitude,
newMaxMagnitude: newMaxMagnitude
});
Expand Down Expand Up @@ -751,19 +751,26 @@ contract DelegationManager is
_beaconChainSlashingFactor[staker] = bsf;
}

/// @dev Calculate amount of slashable shares that are queued withdrawn from an operator for a strategy.
function _getSlashableSharesInQueue(
/**
* @dev Calculate amount of slashable shares that would be slashed from the queued withdrawals from an operator for a strategy
* given the previous maxMagnitude and the new maxMagnitude.
* Note: To get the total amount of slashable shares in the queue withdrawable, set newMaxMagnitude to 0 and prevMaxMagnitude
* is the current maxMagnitude of the operator.
*/
function _getSlashedSharesInQueue(
address operator,
IStrategy strategy,
uint64 maxMagnitude
uint64 prevMaxMagnitude,
uint64 newMaxMagnitude
) internal view returns (uint256) {
uint256 currCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].latest();
// Note: this will simply return 0 if no history exists, same for latest() as well
uint256 pastCumulativeScaledShares = _cumulativeScaledSharesHistory[operator][strategy].upperLookup({
key: uint32(block.number) - MIN_WITHDRAWAL_DELAY_BLOCKS
});
uint256 slashableShareInQueue =
SlashingLib.scaleSharesForBurning(pastCumulativeScaledShares, currCumulativeScaledShares, maxMagnitude);
uint256 slashableShareInQueue = SlashingLib.scaleSharesForBurning(
pastCumulativeScaledShares, currCumulativeScaledShares, prevMaxMagnitude, newMaxMagnitude
);

return slashableShareInQueue;
}
Expand Down Expand Up @@ -864,7 +871,13 @@ contract DelegationManager is
IStrategy[] memory strategies = new IStrategy[](1);
strategies[0] = strategy;
uint64 maxMagnitude = allocationManager.getMaxMagnitudes(operator, strategies)[0];
return _getSlashableSharesInQueue(operator, strategy, maxMagnitude);
// Return amount of shares slashed if all remaining magnitude were to be slashed
return _getSlashedSharesInQueue({
operator: operator,
strategy: strategy,
prevMaxMagnitude: maxMagnitude,
newMaxMagnitude: 0
});
}

/// @inheritdoc IDelegationManager
Expand Down
5 changes: 3 additions & 2 deletions src/contracts/libraries/SlashingLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ library SlashingLib {
function scaleSharesForBurning(
uint256 pastCumulativeScaledShares,
uint256 currCumulativeScaledShares,
uint64 maxMagnitude
uint64 prevMaxMagnitude,
uint64 newMaxMagnitude
) internal pure returns (uint256) {
return (currCumulativeScaledShares - pastCumulativeScaledShares).mulWad(maxMagnitude);
return (currCumulativeScaledShares - pastCumulativeScaledShares).mulWad(prevMaxMagnitude - newMaxMagnitude);
}

function update(
Expand Down
34 changes: 21 additions & 13 deletions src/test/unit/DelegationUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4097,7 +4097,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
// 1. Randomize operator and staker info
// Operator info
address operator = r.Address();
uint64 newMagnitude = 5e17;
uint64 newMagnitude = 25e16;
// First staker
address staker1 = r.Address();
uint256 shares = r.Uint256(1, MAX_STRATEGY_SHARES);
Expand Down Expand Up @@ -4143,12 +4143,12 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
uint256 operatorSharesBefore = delegationManager.operatorShares(operator, strategyMock);
uint256 queuedSlashableSharesBefore = delegationManager.getSlashableSharesInQueue(operator, strategyMock);

// calculate burned shares, should be halved
// staker2 queue withdraws shares and we roll blocks to after the withdrawal is no longer slashable.
// calculate burned shares, should be 3/4 of the original shares
// staker2 queue withdraws shares
// Therefore amount of shares to burn should be what the staker still has remaining + staker1 shares and then
// divided by 2 since the operator was slashed 50%
uint256 sharesToDecrease = (shares + depositAmount - withdrawAmount) / 2;
uint256 sharesToBurn = sharesToDecrease + (delegationManager.getSlashableSharesInQueue(operator, strategyMock) / 2);
uint256 sharesToDecrease = (shares + depositAmount - withdrawAmount) * 3 / 4;
uint256 sharesToBurn = sharesToDecrease + withdrawAmount * 3 / 4;

// 4. Burn shares
_setOperatorMagnitude(operator, strategyMock, newMagnitude);
Expand All @@ -4168,7 +4168,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
uint256 queuedSlashableSharesAfter = delegationManager.getSlashableSharesInQueue(operator, strategyMock);
uint256 operatorSharesAfter = delegationManager.operatorShares(operator, strategyMock);
assertEq(queuedSlashableSharesBefore, withdrawAmount, "Slashable shares in queue should be full withdraw amount");
assertEq(queuedSlashableSharesAfter, withdrawAmount / 2, "Slashable shares in queue should be half withdraw amount after slashing");
assertEq(queuedSlashableSharesAfter, withdrawAmount / 4, "Slashable shares in queue should be 1/4 withdraw amount after slashing");
assertEq(operatorSharesAfter, operatorSharesBefore - sharesToDecrease, "operator shares should be decreased by sharesToBurn");
}

Expand All @@ -4180,7 +4180,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
// 1. Randomize operator and staker info
// Operator info
address operator = r.Address();
uint64 newMagnitude = 5e17;
uint64 newMagnitude = 25e16;
// Staker and withdrawing amounts
address staker = r.Address();
uint256 depositAmount = r.Uint256(3, MAX_STRATEGY_SHARES);
Expand Down Expand Up @@ -4238,9 +4238,9 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
uint256 queuedSlashableSharesBefore = delegationManager.getSlashableSharesInQueue(operator, strategyMock);

// calculate burned shares, should be halved for both operatorShares and slashable shares in queue
// staker queue withdraws shares twice and both withdrawals should be slashed 50%.
uint256 sharesToDecrease = (depositAmount - withdrawAmount1 - withdrawAmount2) / 2;
uint256 sharesToBurn = sharesToDecrease + (delegationManager.getSlashableSharesInQueue(operator, strategyMock) / 2);
// staker queue withdraws shares twice and both withdrawals should be slashed 75%.
uint256 sharesToDecrease = (depositAmount - withdrawAmount1 - withdrawAmount2) * 3 / 4;
uint256 sharesToBurn = sharesToDecrease + (delegationManager.getSlashableSharesInQueue(operator, strategyMock) * 3 / 4);

// 4. Burn shares
_setOperatorMagnitude(operator, strategyMock, newMagnitude);
Expand All @@ -4260,14 +4260,22 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
uint256 queuedSlashableSharesAfter = delegationManager.getSlashableSharesInQueue(operator, strategyMock);
uint256 operatorSharesAfter = delegationManager.operatorShares(operator, strategyMock);
assertEq(queuedSlashableSharesBefore, (withdrawAmount1 + withdrawAmount2), "Slashable shares in queue should be full withdraw amount");
assertEq(queuedSlashableSharesAfter, (withdrawAmount1 + withdrawAmount2) / 2, "Slashable shares in queue should be half withdraw amount after slashing");
assertEq(queuedSlashableSharesAfter, (withdrawAmount1 + withdrawAmount2) / 4, "Slashable shares in queue should be 1/4 withdraw amount after slashing");
assertEq(operatorSharesAfter, operatorSharesBefore - sharesToDecrease, "operator shares should be decreased by sharesToBurn");
}

/**
* @notice TODO Test burning shares for an operator with slashable queued withdrawals in past MIN_WITHDRAWAL_DELAY_BLOCKS window.
* There exists multiple withdrawals that are slashable but queued with different maxMagnitudes at
* time of queuing.
*
* Test Setup:
* - staker1 deposits, queues withdrawal for some amount,
* - operator slashed 50%
* - staker 2 deposits, queues withdrawal for some amount
* - operator is then slashed another 50%
* slashed amount for staker 1 should be 75% and staker 2 should be 50% where the total
* slashed amount is the sum of both
*/
function testFuzz_burnOperatorShares_MultipleWithdrawalsMultipleSlashings(Randomness r) public {}

Expand All @@ -4280,7 +4288,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests
// 1. Randomize operator and staker info
// Operator info
address operator = r.Address();
uint64 newMagnitude = 5e17;
uint64 newMagnitude = 25e16;
// staker
address staker = r.Address();
uint256 depositAmount = r.Uint256(1, MAX_STRATEGY_SHARES);
Expand Down Expand Up @@ -4345,7 +4353,7 @@ contract DelegationManagerUnitTests_burningShares is DelegationManagerUnitTests

uint256 operatorSharesBefore = delegationManager.operatorShares(operator, strategyMock);

// 4. Burn shares
// 4. Burn 0 shares when new magnitude is set
_setOperatorMagnitude(operator, strategyMock, newMagnitude);
cheats.prank(address(allocationManagerMock));
cheats.expectEmit(true, true, true, true, address(delegationManager));
Expand Down

0 comments on commit 9f20ad5

Please sign in to comment.