Skip to content

Commit

Permalink
refactor: review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xClandestine
0xClandestine committed Oct 1, 2024
1 parent 4edc1dc commit e7da526
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 123 deletions.
146 changes: 60 additions & 86 deletions src/contracts/core/AllocationManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,9 @@ contract AllocationManager is
}
require(delegation.isOperator(operator), OperatorNotRegistered());

(bool isSet, uint32 delay) = allocationDelay(operator);
(bool isSet, uint32 operatorAllocationDelay) = allocationDelay(operator);
require(isSet, UninitializedAllocationDelay());

// effect timestamp for allocations to take effect. This is configurable by operators
uint32 allocationEffectTimestamp = uint32(block.timestamp) + delay;
// completable timestamp for deallocations
uint32 deallocationEffectTimestamp = uint32(block.timestamp) + DEALLOCATION_DELAY;

for (uint256 i = 0; i < allocations.length; ++i) {
// 1. For the given (operator,strategy) clear all pending free magnitude for the strategy and update freeMagnitude
_updateFreeMagnitude({
Expand All @@ -148,15 +143,60 @@ contract AllocationManager is
// where an operator gets slashed from an operatorSet and as a result all the configured allocations have larger
// proprtional magnitudes relative to each other.
uint64 currentTotalMagnitude = _getLatestTotalMagnitude(operator, allocations[i].strategy);

require(currentTotalMagnitude == allocations[i].expectedTotalMagnitude, InvalidExpectedTotalMagnitude());

// 3. set allocations for the strategy after updating freeMagnitude
_modifyAllocations({
operator: operator,
allocation: allocations[i],
allocationEffectTimestamp: allocationEffectTimestamp,
deallocationEffectTimestamp: deallocationEffectTimestamp
});
MagnitudeAllocation calldata allocation = allocations[i];

require(allocation.operatorSets.length == allocation.magnitudes.length, InputArrayLengthMismatch());

for (uint256 j = 0; j < allocation.operatorSets.length; ++j) {
require(
avsDirectory.isOperatorSet(allocation.operatorSets[j].avs, allocation.operatorSets[j].operatorSetId),
InvalidOperatorSet()
);

bytes32 operatorSetKey = _encodeOperatorSet(allocation.operatorSets[j]);

// Check that there are no pending allocations & deallocations for the operator, operatorSet, strategy
MagnitudeInfo memory mInfo = _getCurrentMagnitudeWithPending(operator, allocation.strategy, operatorSetKey);
require(
block.timestamp >= mInfo.effectTimestamp,
ModificationAlreadyPending()
);

// Calculate the new pending diff with this modification
mInfo.pendingMagnitudeDiff = int128(uint128(allocation.magnitudes[j])) - int128(uint128(mInfo.currentMagnitude));
require(mInfo.pendingMagnitudeDiff != 0, SameMagnitude());

// Handle deallocation/allocation and modification effect timestamp
if (mInfo.pendingMagnitudeDiff < 0) {
// This is a deallocation

// 1. push PendingFreeMagnitude and respective array index into (op,opSet,Strategy) queued deallocations
_pendingDeallocationOperatorSets[operator][allocation.strategy].push(operatorSetKey);

// 2. Update the effect timestamp for the deallocation
mInfo.effectTimestamp = uint32(block.timestamp) + DEALLOCATION_DELAY;
} else if (mInfo.pendingMagnitudeDiff > 0) {
// This is an allocation

// 1. decrement free magnitude by incremented amount
uint64 magnitudeToAllocate = uint64(uint128(mInfo.pendingMagnitudeDiff));
FreeMagnitudeInfo memory freeInfo = operatorFreeMagnitudeInfo[operator][allocation.strategy];
require(freeInfo.freeMagnitude >= magnitudeToAllocate, InsufficientAllocatableMagnitude());
freeInfo.freeMagnitude -= magnitudeToAllocate;

// 2. Update the effectTimestamp for the allocation
mInfo.effectTimestamp = uint32(block.timestamp) + operatorAllocationDelay;

operatorFreeMagnitudeInfo[operator][allocation.strategy] = freeInfo;
}

// Allocate magnitude which will take effect at the `effectTimestamp`
_operatorMagnitudeInfo[operator][allocation.strategy][operatorSetKey] = mInfo;
}
}
}

Expand Down Expand Up @@ -254,70 +294,6 @@ contract AllocationManager is
operatorFreeMagnitudeInfo[operator][strategy] = freeInfo;
}

/**
* @notice For a single strategy, modify magnitude allocations for each of the specified operatorSets
* @param operator address to modify allocations for
* @param allocation the magnitude allocations to modify for a single strategy
* @param allocationEffectTimestamp the timestamp when the allocations will take effect
* @param deallocationEffectTimestamp the timestamp when the deallocations will be completable
* @dev For each allocation, allocation.operatorSets MUST be ordered in ascending order according to the
* encoding of the operatorSet. This is to prevent duplicate operatorSets being passed in. The easiest way to ensure
* ordering is to sort allocated operatorSets by address first, and then sort for each avs by ascending operatorSetIds.
*/
function _modifyAllocations(
address operator,
MagnitudeAllocation calldata allocation,
uint32 allocationEffectTimestamp,
uint32 deallocationEffectTimestamp
) internal {
require(allocation.operatorSets.length == allocation.magnitudes.length, InputArrayLengthMismatch());

for (uint256 i = 0; i < allocation.operatorSets.length; ++i) {
require(
avsDirectory.isOperatorSet(allocation.operatorSets[i].avs, allocation.operatorSets[i].operatorSetId),
InvalidOperatorSet()
);

bytes32 operatorSetKey = _encodeOperatorSet(allocation.operatorSets[i]);

// Check that there are no pending allocations & deallocations for the operator, operatorSet, strategy
MagnitudeInfo memory mInfo = _getCurrentMagnitudeWithPending(operator, allocation.strategy, operatorSetKey);
require(
block.timestamp >= mInfo.effectTimestamp,
PendingAllocationOrDeallocation()
);

// Calculate the new pending diff with this modification
mInfo.pendingMagnitudeDiff = int128(uint128(allocation.magnitudes[i])) - int128(uint128(mInfo.currentMagnitude));
require(mInfo.pendingMagnitudeDiff != 0, SameMagnitude());

// Handle deallocation/allocation and modification effect timestamp
if (mInfo.pendingMagnitudeDiff < 0) {
// This is a deallocation

// 1. push PendingFreeMagnitude and respective array index into (op,opSet,Strategy) queued deallocations
_pendingDeallocationOperatorSets[operator][allocation.strategy].push(operatorSetKey);

// 2. Update the effect timestamp for the deallocation
mInfo.effectTimestamp = deallocationEffectTimestamp;
} else if (mInfo.pendingMagnitudeDiff > 0) {
// This is an allocation

// 1. decrement free magnitude by incremented amount
uint64 magnitudeToAllocate = uint64(uint128(mInfo.pendingMagnitudeDiff));
FreeMagnitudeInfo storage freeInfo = operatorFreeMagnitudeInfo[operator][allocation.strategy];
require(freeInfo.freeMagnitude >= magnitudeToAllocate, InsufficientAllocatableMagnitude());
freeInfo.freeMagnitude -= magnitudeToAllocate;

// 2. Update the effectTimestamp for the allocation
mInfo.effectTimestamp = allocationEffectTimestamp;
}

// Allocate magnitude which will take effect at the `effectTimestamp`
_operatorMagnitudeInfo[operator][allocation.strategy][operatorSetKey] = mInfo;
}
}

/// @dev gets the latest total magnitude or overwrites it if it is not set
function _getLatestTotalMagnitude(address operator, IStrategy strategy) internal returns (uint64) {
(bool exists,, uint224 totalMagnitude) = _totalMagnitudeUpdate[operator][strategy].latestSnapshot();
Expand Down Expand Up @@ -525,19 +501,19 @@ contract AllocationManager is
* @param operator the operator to get the pending allocations for
* @param strategy the strategy to get the pending allocations for
* @param operatorSets the operatorSets to get the pending allocations for
* @return pendingMagnitude the pending allocations for each operatorSet
* @return pendingMagnitudes the pending allocations for each operatorSet
* @return timestamps the timestamps for each pending allocation
*/
function getPendingAllocations(
address operator,
IStrategy strategy,
OperatorSet[] calldata operatorSets
) external view returns (uint64[] memory, uint32[] memory) {
uint64[] memory pendingMagnitudes = new uint64[](operatorSets.length);
uint32[] memory timestamps = new uint32[](operatorSets.length);
) external view returns (uint64[] memory pendingMagnitudes, uint32[] memory timestamps) {
pendingMagnitudes = new uint64[](operatorSets.length);
timestamps = new uint32[](operatorSets.length);
for (uint256 i = 0; i < operatorSets.length; ++i) {
MagnitudeInfo memory opsetMagnitudeInfo = _operatorMagnitudeInfo[operator][strategy][_encodeOperatorSet(operatorSets[i])];

if (opsetMagnitudeInfo.effectTimestamp < block.timestamp && opsetMagnitudeInfo.pendingMagnitudeDiff > 0) {
pendingMagnitudes[i] = opsetMagnitudeInfo.currentMagnitude + uint64(uint128(opsetMagnitudeInfo.pendingMagnitudeDiff));
timestamps[i] = opsetMagnitudeInfo.effectTimestamp;
Expand All @@ -546,7 +522,6 @@ contract AllocationManager is
timestamps[i] = 0;
}
}
return (pendingMagnitudes, timestamps);
}

/**
Expand All @@ -563,9 +538,9 @@ contract AllocationManager is
address operator,
IStrategy strategy,
OperatorSet[] calldata operatorSets
) external view returns (uint64[] memory, uint32[] memory) {
uint64[] memory pendingMagnitudeDiffs = new uint64[](operatorSets.length);
uint32[] memory timestamps = new uint32[](operatorSets.length);
) external view returns (uint64[] memory pendingMagnitudeDiffs, uint32[] memory timestamps) {
pendingMagnitudeDiffs = new uint64[](operatorSets.length);
timestamps = new uint32[](operatorSets.length);

for (uint256 i = 0; i < operatorSets.length; ++i) {
MagnitudeInfo memory opsetMagnitudeInfo = _operatorMagnitudeInfo[operator][strategy][_encodeOperatorSet(operatorSets[i])];
Expand All @@ -578,7 +553,6 @@ contract AllocationManager is
timestamps[i] = 0;
}
}
return (pendingMagnitudeDiffs, timestamps);
}

/// @dev Verify operator's signature and spend salt
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/core/AllocationManagerStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ abstract contract AllocationManagerStorage is IAllocationManager {
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[42] private __gap;
uint256[43] private __gap;
}
2 changes: 1 addition & 1 deletion src/contracts/interfaces/IAllocationManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface IAllocationManager is ISignatureUtils {
/// @dev Thrown when an operator attempts to set their allocation for an operatorSet to the same value
error SameMagnitude();
/// @dev Thrown when an allocation is attempted for a given operator when they have pending allocations or deallocations.
error PendingAllocationOrDeallocation();
error ModificationAlreadyPending();
/// @dev Thrown when an allocation is attempted that exceeds a given operators total allocatable magnitude.
error InsufficientAllocatableMagnitude();
/// @dev Thrown when attempting to use an expired eip-712 signature.
Expand Down
30 changes: 15 additions & 15 deletions src/test/events/IRewardsCoordinatorEvents.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ interface IRewardsCoordinatorEvents {
);
event ActivationDelaySet(uint32 oldActivationDelay, uint32 newActivationDelay);
event GlobalCommissionBipsSet(uint16 oldGlobalCommissionBips, uint16 newGlobalCommissionBips);
/// @notice emitted when an operator commission is set for a specific OperatorSet
event OperatorCommissionUpdated(
address indexed operator,
OperatorSet indexed operatorSet,
IRewardsCoordinator.RewardType rewardType,
uint16 newCommissionBips,
uint32 effectTimestamp
);
// /// @notice emitted when an operator commission is set for a specific OperatorSet
// event OperatorCommissionUpdated(
// address indexed operator,
// OperatorSet indexed operatorSet,
// IRewardsCoordinator.RewardType rewardType,
// uint16 newCommissionBips,
// uint32 effectTimestamp
// );
event ClaimerForSet(address indexed earner, address indexed oldClaimer, address indexed claimer);
/// @notice rootIndex is the specific array index of the newly created root in the storage array
event DistributionRootSubmitted(
Expand All @@ -62,13 +62,13 @@ interface IRewardsCoordinatorEvents {
IERC20 token,
uint256 claimedAmount
);
/// @notice emitted when an AVS creates a valid OperatorSetRewardsSubmission
event OperatorSetRewardCreated(
address indexed avs,
uint256 indexed submissionNonce,
bytes32 indexed rewardsSubmissionHash,
IRewardsCoordinator.OperatorSetRewardsSubmission rewardsSubmission
);
// /// @notice emitted when an AVS creates a valid OperatorSetRewardsSubmission
// event OperatorSetRewardCreated(
// address indexed avs,
// uint256 indexed submissionNonce,
// bytes32 indexed rewardsSubmissionHash,
// IRewardsCoordinator.OperatorSetRewardsSubmission rewardsSubmission
// );



Expand Down
4 changes: 2 additions & 2 deletions src/test/mocks/DelegationManagerMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ contract DelegationManagerMock is IDelegationManager, Test {
IStrategy strategy,
OwnedShares shares
) external {
strategyManager.addShares(staker, token, strategy, shares);
strategyManager.addOwnedShares(staker, strategy, token, shares);
}

function removeShares(
Expand All @@ -198,6 +198,6 @@ contract DelegationManagerMock is IDelegationManager, Test {
OwnedShares shares,
IERC20 token
) external {
strategyManager.withdrawSharesAsTokens(recipient, strategy, shares, token);
strategyManager.withdrawSharesAsTokens(recipient, strategy, token, shares);
}
}
4 changes: 2 additions & 2 deletions src/test/mocks/EigenPodManagerMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ contract EigenPodManagerMock is IEigenPodManager, Test, Pausable {

function removeShares(address staker, IStrategy strategy, Shares shares) external {}

function addShares(address staker, IERC20 token, IStrategy strategy, OwnedShares shares) external {}
function addOwnedShares(address staker, IStrategy strategy, IERC20 token, OwnedShares shares) external {}

function withdrawSharesAsTokens(address recipient, IStrategy strategy, OwnedShares shares, IERC20 token) external {}
function withdrawSharesAsTokens(address recipient, IStrategy strategy, IERC20 token, OwnedShares shares) external {}

function stakerStrategyShares(address user, IStrategy strategy) external view returns (Shares shares) {}

Expand Down
4 changes: 2 additions & 2 deletions src/test/mocks/StrategyManagerMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ contract StrategyManagerMock is

function removeShares(address staker, IStrategy strategy, Shares shares) external {}

function addShares(address staker, IERC20 token, IStrategy strategy, OwnedShares shares) external {}
function addOwnedShares(address staker, IStrategy strategy, IERC20 token, OwnedShares shares) external {}

function withdrawSharesAsTokens(address recipient, IStrategy strategy, OwnedShares shares, IERC20 token) external {}
function withdrawSharesAsTokens(address recipient, IStrategy strategy, IERC20 token, OwnedShares shares) external {}

/// @notice returns the enshrined beaconChainETH Strategy
function beaconChainETHStrategy() external view returns (IStrategy) {}
Expand Down
16 changes: 8 additions & 8 deletions src/test/unit/EigenPodManagerUnit.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,28 +194,28 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {
cheats.assume(notDelegationManager != address(delegationManagerMock));
cheats.prank(notDelegationManager);
cheats.expectRevert("EigenPodManager.onlyDelegationManager: not the DelegationManager");
eigenPodManager.addShares(defaultStaker, 0);
eigenPodManager.addOwnedShares(defaultStaker, 0);
}

function test_addShares_revert_podOwnerInputAddressZero() public {
cheats.prank(address(delegationManagerMock));
cheats.expectRevert("EigenPodManager.addShares: podOwner cannot be zero address");
eigenPodManager.addShares(address(0), 0);
cheats.expectRevert("EigenPodManager.addOwnedShares: podOwner cannot be zero address");
eigenPodManager.addOwnedShares(address(0), 0);
}

function testFuzz_addShares_revert_sharesNegative(int256 shares) public {
cheats.assume(shares < 0);
cheats.prank(address(delegationManagerMock));
cheats.expectRevert("EigenPodManager.addShares: shares cannot be negative");
eigenPodManager.addShares(defaultStaker, uint256(shares));
cheats.expectRevert("EigenPodManager.addOwnedShares: shares cannot be negative");
eigenPodManager.addOwnedShares(defaultStaker, uint256(shares));
}

function testFuzz_addShares_revert_sharesNotWholeGwei(uint256 shares) public {
cheats.assume(int256(shares) >= 0);
cheats.assume(shares % GWEI_TO_WEI != 0);
cheats.prank(address(delegationManagerMock));
cheats.expectRevert("EigenPodManager.addShares: shares must be a whole Gwei amount");
eigenPodManager.addShares(defaultStaker, shares);
cheats.expectRevert("EigenPodManager.addOwnedShares: shares must be a whole Gwei amount");
eigenPodManager.addOwnedShares(defaultStaker, shares);
}

function testFuzz_addShares(uint256 shares) public {
Expand All @@ -226,7 +226,7 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {

// Add shares
cheats.prank(address(delegationManagerMock));
eigenPodManager.addShares(defaultStaker, shares);
eigenPodManager.addOwnedShares(defaultStaker, shares);

// Check storage update
assertEq(eigenPodManager.podOwnerShares(defaultStaker), int256(shares), "Incorrect number of shares added");
Expand Down
Loading

0 comments on commit e7da526

Please sign in to comment.