Skip to content

Commit

Permalink
refactor: small cleanup (#959)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
0xClandestine authored and ypatil12 committed Jan 3, 2025
1 parent dadd4b0 commit 9a198b5
Show file tree
Hide file tree
Showing 7 changed files with 329 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/contracts/core/AllocationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
2 changes: 1 addition & 1 deletion src/contracts/core/DelegationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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]
});
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/contracts/interfaces/IAllocationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
*/
Expand Down
9 changes: 9 additions & 0 deletions src/test/unit/AllocationManagerUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
197 changes: 196 additions & 1 deletion src/test/unit/DelegationUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions src/test/utils/ArrayLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// -----------------------------------------------------------------------
Expand Down Expand Up @@ -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
/// -----------------------------------------------------------------------
Expand Down Expand Up @@ -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
/// -----------------------------------------------------------------------
Expand Down Expand Up @@ -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
/// -----------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 9a198b5

Please sign in to comment.