Skip to content

refactor: rename inconsistent getQueuedWithdrawal alias #1133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 19, 2025
Merged

refactor: rename inconsistent getQueuedWithdrawal alias #1133

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
377b9b8
refactor: remove inconsistent alias
0xClandestine Feb 19, 2025
f1ed2d5
chore: forge fmt
0xClandestine Feb 19, 2025
ef603a7
refactor: review changes
0xClandestine Feb 19, 2025
15d55bc
chore: comment out failing test
ypatil12 Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions src/contracts/core/DelegationManager.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
26 changes: 13 additions & 13 deletions src/test/unit/DelegationUnit.t.sol
View file
Open in desktop
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");
}
}
Loading