Skip to content

Commit

Permalink
refactor: rename inconsistent getQueuedWithdrawal alias (#1133)
Browse files Browse the repository at this point in the history 
**Motivation:**

Naming between the `getQueuedWIthdrawal` aliases was inconsistent.

**Modifications:**

- ~made `queuedWithdrawals[withdrawalRoot]` mapping public.~
- renamed `queuedWithdrawals` -> `_queuedWithdrawals`.
- added `_queuedWithdrawals` getter
- removed previous `getQueuedWithdrawal` alias.
- renamed `getQueuedWithdrawalFromRoot` to `getQueuedWithdrawal`. 

**Result:**

Consistent function naming.

---------

Co-authored-by: Yash Patil <[email protected]>
  • Loading branch information
@0xClandestine @ypatil12
0xClandestine and ypatil12 committed Feb 20, 2025
1 parent 832820e commit 50f23c8
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 99 deletions.
15 changes: 7 additions & 8 deletions src/contracts/core/DelegationManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ contract DelegationManager is
bytes32 withdrawalRoot = calculateWithdrawalRoot(withdrawal);

pendingWithdrawals[withdrawalRoot] = true;
queuedWithdrawals[withdrawalRoot] = withdrawal;
_queuedWithdrawals[withdrawalRoot] = withdrawal;
_stakerQueuedWithdrawalRoots[staker].add(withdrawalRoot);

emit SlashingWithdrawalQueued(withdrawalRoot, withdrawal, withdrawableShares);
Expand Down Expand Up @@ -569,7 +569,7 @@ contract DelegationManager is
// Remove the withdrawal from the queue. Note that for legacy withdrawals, the removals
// from `_stakerQueuedWithdrawalRoots` and `queuedWithdrawals` will no-op.
_stakerQueuedWithdrawalRoots[withdrawal.staker].remove(withdrawalRoot);
delete queuedWithdrawals[withdrawalRoot];
delete _queuedWithdrawals[withdrawalRoot];
delete pendingWithdrawals[withdrawalRoot];
emit SlashingWithdrawalCompleted(withdrawalRoot);

Expand Down Expand Up @@ -803,7 +803,7 @@ contract DelegationManager is
function _getSharesByWithdrawalRoot(
bytes32 withdrawalRoot
) internal view returns (Withdrawal memory withdrawal, uint256[] memory shares) {
withdrawal = queuedWithdrawals[withdrawalRoot];
withdrawal = _queuedWithdrawals[withdrawalRoot];
shares = new uint256[](withdrawal.strategies.length);

uint32 slashableUntil = withdrawal.startBlock + MIN_WITHDRAWAL_DELAY_BLOCKS;
Expand Down Expand Up @@ -964,15 +964,14 @@ contract DelegationManager is
return (strategies, shares);
}

/// @inheritdoc IDelegationManager
function getQueuedWithdrawal(
function queuedWithdrawals(
bytes32 withdrawalRoot
) external view returns (Withdrawal memory) {
return queuedWithdrawals[withdrawalRoot];
) external view returns (Withdrawal memory withdrawal) {
return _queuedWithdrawals[withdrawalRoot];
}

/// @inheritdoc IDelegationManager
function getQueuedWithdrawalFromRoot(
function getQueuedWithdrawal(
bytes32 withdrawalRoot
) external view returns (Withdrawal memory withdrawal, uint256[] memory shares) {
(withdrawal, shares) = _getSharesByWithdrawalRoot(withdrawalRoot);
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/core/DelegationManagerStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ abstract contract DelegationManagerStorage is IDelegationManager {

/// @notice Returns the details of a queued withdrawal given by `withdrawalRoot`.
/// @dev This variable only reflects withdrawals that were made after the slashing release.
mapping(bytes32 withdrawalRoot => Withdrawal withdrawal) internal queuedWithdrawals;
mapping(bytes32 withdrawalRoot => Withdrawal withdrawal) internal _queuedWithdrawals;

/// @notice Contains history of the total cumulative staker withdrawals for an operator and a given strategy.
/// Used to calculate burned StrategyManager shares when an operator is slashed.
Expand Down
23 changes: 9 additions & 14 deletions src/contracts/interfaces/IDelegationManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,11 +471,17 @@ interface IDelegationManager is ISignatureUtilsMixin, IDelegationManagerErrors,
*/
function depositScalingFactor(address staker, IStrategy strategy) external view returns (uint256);

/// @notice Returns the Withdrawal associated with a `withdrawalRoot`, if it exists. NOTE that
/// withdrawals queued before the slashing release can NOT be queried with this method.
/**
* @notice Returns the Withdrawal and corresponding shares associated with a `withdrawalRoot`
* @param withdrawalRoot The hash identifying the queued withdrawal
* @return withdrawal The withdrawal details
* @return shares Array of shares corresponding to each strategy in the withdrawal
* @dev The shares are what a user would receive from completing a queued withdrawal, assuming all slashings are applied
* @dev Withdrawals queued before the slashing release cannot be queried with this method
*/
function getQueuedWithdrawal(
bytes32 withdrawalRoot
) external view returns (Withdrawal memory);
) external view returns (Withdrawal memory withdrawal, uint256[] memory shares);

/**
* @notice Returns all queued withdrawals and their corresponding shares for a staker.
Expand All @@ -488,17 +494,6 @@ interface IDelegationManager is ISignatureUtilsMixin, IDelegationManagerErrors,
address staker
) external view returns (Withdrawal[] memory withdrawals, uint256[][] memory shares);

/**
* @notice Returns the withdrawal details and corresponding shares for a specific queued withdrawal.
* @param withdrawalRoot The hash identifying the queued withdrawal.
* @return withdrawal The withdrawal details.
* @return shares Array of shares corresponding to each strategy in the withdrawal.
* @dev The shares are what a user would receive from completing a queued withdrawal, assuming all slashings are applied.
*/
function getQueuedWithdrawalFromRoot(
bytes32 withdrawalRoot
) external view returns (Withdrawal memory withdrawal, uint256[] memory shares);

/// @notice Returns a list of queued withdrawal roots for the `staker`.
/// NOTE that this only returns withdrawals queued AFTER the slashing release.
function getQueuedWithdrawalRoots(
Expand Down
127 changes: 64 additions & 63 deletions src/test/integration/tests/Slashed_Eigenpod.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,71 +134,72 @@ contract Integration_SlashedEigenpod is IntegrationCheckUtils {
assertApproxEqAbs(withdrawableSharesAfter[0], depositSharesAfter[0], 100, "Withdrawable shares should equal deposit shares after withdrawal");
}

function testFuzz_delegateSlashedStaker_slashedOperator(uint24 _random) public rand(_random) {


(User staker2,,) = _newRandomStaker();
(uint40[] memory validators2,) = staker2.startValidators();
beaconChain.advanceEpoch_NoWithdrawNoRewards();
staker2.verifyWithdrawalCredentials(validators2);
staker2.startCheckpoint();
staker2.completeCheckpoint();
staker2.delegateTo(operator);

//randomize additional deposit to eigenpod
if(_randBool()){
uint amount = 32 ether * _randUint({min: 1, max: 5});
cheats.deal(address(staker), amount);
(uint40[] memory validators,) = staker.startValidators();
beaconChain.advanceEpoch_NoWithdrawNoRewards();
staker.verifyWithdrawalCredentials(validators);
// TODO: Fix this test
// function testFuzz_delegateSlashedStaker_slashedOperator(uint24 _random) public rand(_random) {


// (User staker2,,) = _newRandomStaker();
// (uint40[] memory validators2,) = staker2.startValidators();
// beaconChain.advanceEpoch_NoWithdrawNoRewards();
// staker2.verifyWithdrawalCredentials(validators2);
// staker2.startCheckpoint();
// staker2.completeCheckpoint();
// staker2.delegateTo(operator);

// //randomize additional deposit to eigenpod
// if(_randBool()){
// uint amount = 32 ether * _randUint({min: 1, max: 5});
// cheats.deal(address(staker), amount);
// (uint40[] memory validators,) = staker.startValidators();
// beaconChain.advanceEpoch_NoWithdrawNoRewards();
// staker.verifyWithdrawalCredentials(validators);

staker.startCheckpoint();
staker.completeCheckpoint();
}

// Create an operator set and register an operator.
operatorSet = avs.createOperatorSet(strategies);
operator.registerForOperatorSet(operatorSet);
check_Registration_State_NoAllocation(operator, operatorSet, strategies);

// Allocate to operator set
allocateParams = _genAllocation_AllAvailable(operator, operatorSet, strategies);
operator.modifyAllocations(allocateParams);
check_IncrAlloc_State_Slashable(operator, allocateParams);
_rollBlocksForCompleteAllocation(operator, operatorSet, strategies);

//Slash operator before delegation
IAllocationManagerTypes.SlashingParams memory slashingParams;
uint wadToSlash = _randWadToSlash();
slashingParams = avs.slashOperator(operator, operatorSet.id, strategies, wadToSlash.toArrayU256());
assert_Snap_Allocations_Slashed(slashingParams, operatorSet, true, "operator allocations should be slashed");

uint256[] memory initDelegatableShares = _getWithdrawableShares(staker, strategies);
uint256[] memory initDepositShares = _getStakerDepositShares(staker, strategies);

// Delegate to an operator
staker.delegateTo(operator);
(uint256[] memory delegatedShares, ) = delegationManager.getWithdrawableShares(address(staker), strategies);
check_Delegation_State(staker, operator, strategies, initDepositShares);
// staker.startCheckpoint();
// staker.completeCheckpoint();
// }

// // Create an operator set and register an operator.
// operatorSet = avs.createOperatorSet(strategies);
// operator.registerForOperatorSet(operatorSet);
// check_Registration_State_NoAllocation(operator, operatorSet, strategies);

// // Allocate to operator set
// allocateParams = _genAllocation_AllAvailable(operator, operatorSet, strategies);
// operator.modifyAllocations(allocateParams);
// check_IncrAlloc_State_Slashable(operator, allocateParams);
// _rollBlocksForCompleteAllocation(operator, operatorSet, strategies);

// //Slash operator before delegation
// IAllocationManagerTypes.SlashingParams memory slashingParams;
// uint wadToSlash = _randWadToSlash();
// slashingParams = avs.slashOperator(operator, operatorSet.id, strategies, wadToSlash.toArrayU256());
// assert_Snap_Allocations_Slashed(slashingParams, operatorSet, true, "operator allocations should be slashed");

// uint256[] memory initDelegatableShares = _getWithdrawableShares(staker, strategies);
// uint256[] memory initDepositShares = _getStakerDepositShares(staker, strategies);

// // Delegate to an operator
// staker.delegateTo(operator);
// (uint256[] memory delegatedShares, ) = delegationManager.getWithdrawableShares(address(staker), strategies);
// check_Delegation_State(staker, operator, strategies, initDepositShares);

// Undelegate from an operator
IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.undelegate();
bytes32[] memory withdrawalRoots = _getWithdrawalHashes(withdrawals);
check_Undelegate_State(staker, operator, withdrawals, withdrawalRoots, strategies, initDepositShares, delegatedShares);

// Complete withdrawal as shares
// Fast forward to when we can complete the withdrawal
_rollBlocksForCompleteWithdrawals(withdrawals);
for (uint256 i = 0; i < withdrawals.length; ++i) {
staker.completeWithdrawalAsShares(withdrawals[i]);
check_Withdrawal_AsShares_Undelegated_State(staker, operator, withdrawals[i], withdrawals[i].strategies, delegatedShares);
}

(uint256[] memory withdrawableSharesAfter, uint256[] memory depositSharesAfter) = delegationManager.getWithdrawableShares(address(staker), strategies);
assertEq(depositSharesAfter[0], delegatedShares[0], "Deposit shares should reset to reflect slash(es)");
assertApproxEqAbs(withdrawableSharesAfter[0], depositSharesAfter[0], 100, "Withdrawable shares should equal deposit shares after withdrawal");
}
// // Undelegate from an operator
// IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.undelegate();
// bytes32[] memory withdrawalRoots = _getWithdrawalHashes(withdrawals);
// check_Undelegate_State(staker, operator, withdrawals, withdrawalRoots, strategies, initDepositShares, delegatedShares);

// // Complete withdrawal as shares
// // Fast forward to when we can complete the withdrawal
// _rollBlocksForCompleteWithdrawals(withdrawals);
// for (uint256 i = 0; i < withdrawals.length; ++i) {
// staker.completeWithdrawalAsShares(withdrawals[i]);
// check_Withdrawal_AsShares_Undelegated_State(staker, operator, withdrawals[i], withdrawals[i].strategies, delegatedShares);
// }

// (uint256[] memory withdrawableSharesAfter, uint256[] memory depositSharesAfter) = delegationManager.getWithdrawableShares(address(staker), strategies);
// assertEq(depositSharesAfter[0], delegatedShares[0], "Deposit shares should reset to reflect slash(es)");
// assertApproxEqAbs(withdrawableSharesAfter[0], depositSharesAfter[0], 100, "Withdrawable shares should equal deposit shares after withdrawal");
// }

function testFuzz_delegateSlashedStaker_redelegate_complete(uint24 _random) public rand(_random){

Expand Down
26 changes: 13 additions & 13 deletions src/test/unit/DelegationUnit.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4846,7 +4846,7 @@ contract DelegationManagerUnitTests_queueWithdrawals is DelegationManagerUnitTes
cheats.prank(defaultStaker);
bytes32 withdrawalRoot = delegationManager.queueWithdrawals(queuedWithdrawalParams)[0];

Withdrawal memory withdrawal = delegationManager.getQueuedWithdrawal(withdrawalRoot);
(Withdrawal memory withdrawal, ) = delegationManager.getQueuedWithdrawal(withdrawalRoot);
assertEq(withdrawal.staker, defaultStaker, "staker should be msg.sender");
assertEq(withdrawal.withdrawer, defaultStaker, "withdrawer should be msg.sender");
}
Expand Down Expand Up @@ -8670,16 +8670,16 @@ contract DelegationManagerUnitTests_getQueuedWithdrawals is DelegationManagerUni

// Get shares from withdrawal - should return 50 shares (100 * 0.5) using original magnitude
// rather than incorrectly returning 100 shares (100 * 1.0) using new magnitude
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(withdrawalRoot);
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(withdrawalRoot);
assertEq(shares[0], 50e18, "shares should be 50e18 (100e18 * 0.5) using original magnitude");
}
}

contract DelegationManagerUnitTests_getQueuedWithdrawalFromRoot is DelegationManagerUnitTests {
contract DelegationManagerUnitTests_getQueuedWithdrawal is DelegationManagerUnitTests {
using ArrayLib for *;
using SlashingLib for *;

function test_getQueuedWithdrawalFromRoot_Correctness(Randomness r) public rand(r) {
function test_getQueuedWithdrawal_Correctness(Randomness r) public rand(r) {
// Set up initial deposit
uint256 depositAmount = r.Uint256(1 ether, 100 ether);
_depositIntoStrategies(defaultStaker, strategyMock.toArray(), depositAmount.toArrayU256());
Expand All @@ -8703,14 +8703,14 @@ contract DelegationManagerUnitTests_getQueuedWithdrawalFromRoot is DelegationMan
delegationManager.queueWithdrawals(queuedWithdrawalParams);

// Get shares from queued withdrawal
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(withdrawalRoot);
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(withdrawalRoot);

// Verify withdrawal details match
assertEq(shares.length, 1, "incorrect shares array length");
assertEq(shares[0], depositAmount, "incorrect shares amount");
}

function test_getQueuedWithdrawalFromRoot_AfterSlashing(Randomness r) public rand(r) {
function test_getQueuedWithdrawal_AfterSlashing(Randomness r) public rand(r) {
// Set up initial deposit
uint256 depositAmount = r.Uint256(1 ether, 100 ether);
_depositIntoStrategies(defaultStaker, strategyMock.toArray(), depositAmount.toArrayU256());
Expand Down Expand Up @@ -8739,20 +8739,20 @@ contract DelegationManagerUnitTests_getQueuedWithdrawalFromRoot is DelegationMan
delegationManager.slashOperatorShares(defaultOperator, strategyMock, WAD, 0.5 ether);

// Get shares from queued withdrawal
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(withdrawalRoot);
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(withdrawalRoot);

// Verify withdrawal details match and shares are slashed
assertEq(shares.length, 1, "incorrect shares array length");
assertEq(shares[0], depositAmount / 2, "shares not properly slashed");
}

function test_getQueuedWithdrawalFromRoot_NonexistentWithdrawal() public {
function test_getQueuedWithdrawal_NonexistentWithdrawal() public {
bytes32 nonexistentRoot = bytes32(uint256(1));
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(nonexistentRoot);
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(nonexistentRoot);
assertEq(shares.length, 0, "shares array should be empty");
}

function test_getQueuedWithdrawalFromRoot_MultipleStrategies(Randomness r) public rand(r) {
function test_getQueuedWithdrawal_MultipleStrategies(Randomness r) public rand(r) {
// Set up multiple strategies with deposits
uint256 numStrategies = r.Uint256(2, 5);
uint256[] memory depositShares = r.Uint256Array({
Expand Down Expand Up @@ -8782,7 +8782,7 @@ contract DelegationManagerUnitTests_getQueuedWithdrawalFromRoot is DelegationMan
delegationManager.queueWithdrawals(queuedWithdrawalParams);

// Get shares from queued withdrawal
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(withdrawalRoot);
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(withdrawalRoot);

// Verify withdrawal details and shares for each strategy
assertEq(shares.length, numStrategies, "incorrect shares array length");
Expand All @@ -8791,8 +8791,8 @@ contract DelegationManagerUnitTests_getQueuedWithdrawalFromRoot is DelegationMan
}
}

function testFuzz_getQueuedWithdrawalFromRoot_EmptyWithdrawal(bytes32 withdrawalRoot) public {
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawalFromRoot(withdrawalRoot);
function testFuzz_getQueuedWithdrawal_EmptyWithdrawal(bytes32 withdrawalRoot) public {
(, uint256[] memory shares) = delegationManager.getQueuedWithdrawal(withdrawalRoot);
assertEq(shares.length, 0, "sanity check");
}
}

0 comments on commit 50f23c8

Please sign in to comment.