From 9d131b193567147952e56a82111558f183668922 Mon Sep 17 00:00:00 2001 From: Yash Patil <40046473+ypatil12@users.noreply.github.com> Date: Tue, 1 Oct 2024 11:22:51 -0700 Subject: [PATCH] Feat: AllocationManager Storage Simplification (#787) * feat: cleanup * feat: add helper func * fix: simplification * chore: clean up magnitude info usage and type conversions * refactor: review changes * fix: order struct params by size * fix: correct and simplify calc for slashed pending magnitude * fix: storage gap --------- Co-authored-by: wadealexc Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com> --- lib/openzeppelin-contracts | 1 + lib/openzeppelin-contracts-upgradeable | 1 + src/contracts/core/AllocationManager.sol | 381 +++++++----------- .../core/AllocationManagerStorage.sol | 15 +- .../interfaces/IAllocationManager.sol | 31 +- src/test/events/IRewardsCoordinatorEvents.sol | 30 +- src/test/mocks/DelegationManagerMock.sol | 4 +- src/test/mocks/EigenPodManagerMock.sol | 4 +- src/test/mocks/StrategyManagerMock.sol | 4 +- src/test/unit/EigenPodManagerUnit.t.sol | 16 +- src/test/unit/StrategyManagerUnit.t.sol | 12 +- 11 files changed, 209 insertions(+), 290 deletions(-) create mode 160000 lib/openzeppelin-contracts create mode 160000 lib/openzeppelin-contracts-upgradeable diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts new file mode 160000 index 000000000..3b8b4ba82 --- /dev/null +++ b/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit 3b8b4ba82c880c31cd3b96dd5e15741d7e26658e diff --git a/lib/openzeppelin-contracts-upgradeable b/lib/openzeppelin-contracts-upgradeable new file mode 160000 index 000000000..6b9807b06 --- /dev/null +++ b/lib/openzeppelin-contracts-upgradeable @@ -0,0 +1 @@ +Subproject commit 6b9807b0639e1dd75e07fa062e9432eb3f35dd8c diff --git a/src/contracts/core/AllocationManager.sol b/src/contracts/core/AllocationManager.sol index d23ff445e..cd86c4c80 100644 --- a/src/contracts/core/AllocationManager.sol +++ b/src/contracts/core/AllocationManager.sol @@ -22,10 +22,6 @@ contract AllocationManager is /// @dev BIPS factor for slashable bips uint256 internal constant BIPS_FACTOR = 10_000; - /// @dev Maximum number of pending updates that can be queued for allocations/deallocations - /// Note this max applies for a single (operator, strategy, operatorSet) tuple. - uint256 public constant MAX_PENDING_UPDATES = 1; - /// @dev Returns the chain ID from the time the contract was deployed. uint256 internal immutable ORIGINAL_CHAIN_ID; @@ -132,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 deallocationCompletableTimestamp = 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({ @@ -152,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, - deallocationCompletableTimestamp: deallocationCompletableTimestamp - }); + 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; + } } } @@ -186,50 +222,20 @@ contract AllocationManager is require(isOperatorSlashable(operator, operatorSet), InvalidOperator()); for (uint256 i = 0; i < strategies.length; ++i) { - // 1. Get pendingAllocations/Deallocations - (uint256 numPendingAllocations, uint256 numPendingDeallocations) = - _getPendingAllocationsAndDeallocations(operator, strategies[i], operatorSetKey); - uint64 slashedMagnitude; - - // 2. If there are pending allocations or deallocations, decrement accordingly - if (numPendingAllocations != 0) { - Snapshots.History storage magnitudeUpdates = _magnitudeUpdate[operator][strategies[i]][operatorSetKey]; - uint64 currentMagnitude = uint64(magnitudeUpdates.upperLookupRecent(uint32(block.timestamp))); - // skip if no actively allocated magnitude to slash - if (currentMagnitude != 0) { - /// TODO: add wrapping library for rounding up for slashing accounting - slashedMagnitude = uint64(uint256(bipsToSlash) * uint256(currentMagnitude) / BIPS_FACTOR); - - // propagate slashing to current and future and allocations for the (operator, strategy, operatorSet) - magnitudeUpdates.decrementAtAndFutureSnapshots({ - key: uint32(block.timestamp), - decrementValue: slashedMagnitude - }); - } - - // There can be at most 1 pending allocation + deallocation at a time, hence the else-if statement - } else if (numPendingDeallocations != 0) { - // Get the number of queued deallocations that can be completed - uint256 queuedDeallocationIndicesLen = - _queuedDeallocationIndices[operator][strategies[i]][operatorSetKey].length; - - // index of pendingFreeMagnitude/deallocation to check for slashing - uint256 index = _queuedDeallocationIndices[operator][strategies[i]][operatorSetKey][queuedDeallocationIndicesLen - 1]; - PendingFreeMagnitude storage pendingFreeMagnitude = - _pendingFreeMagnitude[operator][strategies[i]][index]; - - // The most recent deallocation may not be completable and, therefore, slashable - if (block.timestamp < pendingFreeMagnitude.completableTimestamp) { - // slash the pending deallocation proportionally and store the updated magnitudeDiff - uint64 slashedAmount = - uint64(uint256(bipsToSlash) * uint256(pendingFreeMagnitude.magnitudeDiff) / BIPS_FACTOR); - pendingFreeMagnitude.magnitudeDiff -= slashedAmount; - // keep track of total slashed magnitude - slashedMagnitude += slashedAmount; - } + // 1. Slash from pending deallocations and allocations + MagnitudeInfo memory mInfo = _getCurrentMagnitudeWithPending(operator, strategies[i], operatorSetKey); + + uint64 slashedMagnitude = uint64(uint256(mInfo.currentMagnitude) * bipsToSlash / BIPS_FACTOR); + mInfo.currentMagnitude -= slashedMagnitude; + // if there is a pending deallocation, slash pending deallocation proportionally + if (mInfo.pendingMagnitudeDiff < 0) { + uint128 slashedPending = uint128(uint128(-mInfo.pendingMagnitudeDiff) * bipsToSlash / BIPS_FACTOR); + mInfo.pendingMagnitudeDiff += int128(slashedPending); } + // update operatorMagnitudeInfo + _operatorMagnitudeInfo[operator][strategies[i]][operatorSetKey] = mInfo; - // 3. update totalMagnitude, get total magnitude and subtract slashedMagnitude + // 2. update totalMagnitude, get total magnitude and subtract slashedMagnitude // this will be reflected in the conversion of delegatedShares to shares in the DM Snapshots.History storage totalMagnitudes = _totalMagnitudeUpdate[operator][strategies[i]]; totalMagnitudes.push({key: uint32(block.timestamp), value: totalMagnitudes.latest() - slashedMagnitude}); @@ -278,137 +284,14 @@ contract AllocationManager is * In addition to updating freeMagnitude, updates next starting index to read from for pending free magnitudes after completing */ function _updateFreeMagnitude(address operator, IStrategy strategy, uint16 numToComplete) internal { - OperatorMagnitudeInfo memory info = operatorMagnitudeInfo[operator][strategy]; + FreeMagnitudeInfo memory freeInfo = operatorFreeMagnitudeInfo[operator][strategy]; (uint64 freeMagnitudeToAdd, uint192 completed) = - _getPendingFreeMagnitude(operator, strategy, numToComplete, info.nextPendingFreeMagnitudeIndex); - info.freeMagnitude += freeMagnitudeToAdd; - info.nextPendingFreeMagnitudeIndex += completed; + _getPendingFreeMagnitude(operator, strategy, numToComplete, freeInfo.nextPendingIndex); + freeInfo.freeMagnitude += freeMagnitudeToAdd; + freeInfo.nextPendingIndex += completed; - operatorMagnitudeInfo[operator][strategy] = info; - } - - /** - * @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 deallocationCompletableTimestamp 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 deallocationCompletableTimestamp - ) 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]); - // prep magnitudeUpdates in storage - - // Check that there is at MOST `MAX_PENDING_UPDATES` combined allocations & deallocations for the operator, operatorSet, strategy - (uint256 numPendingAllocations, uint256 numPendingDeallocations) = - _getPendingAllocationsAndDeallocations(operator, allocation.strategy, operatorSetKey); - - require( - numPendingAllocations + numPendingDeallocations < MAX_PENDING_UPDATES, - PendingAllocationOrDeallocation() - ); - - Snapshots.History storage magnitudeUpdates = _magnitudeUpdate[operator][allocation.strategy][operatorSetKey]; - - // Read current magnitude allocation including its respective array index and length. - // We'll use these values later to check the number of pending allocations/deallocations. - (uint224 currentMagnitude,,) = - magnitudeUpdates.upperLookupRecentWithPos(uint32(block.timestamp)); - require(currentMagnitude != allocation.magnitudes[i], SameMagnitude()); - - if (allocation.magnitudes[i] < uint64(currentMagnitude)) { - // Newly configured magnitude is less than current value. - // Therefore we handle this as a deallocation - - // Note: MAX_PENDING_UPDATES == 1, so we do not have to decrement any allocations - - // 1. push PendingFreeMagnitude and respective array index into (op,opSet,Strategy) queued deallocations - uint64 magnitudeToDeallocate = uint64(currentMagnitude) - allocation.magnitudes[i]; - _queuedDeallocationIndices[operator][allocation.strategy][operatorSetKey].push( - _pendingFreeMagnitude[operator][allocation.strategy].length - ); - _pendingFreeMagnitude[operator][allocation.strategy].push( - PendingFreeMagnitude({ - magnitudeDiff: magnitudeToDeallocate, - completableTimestamp: deallocationCompletableTimestamp - }) - ); - - // 2. decrement allocated magnitude - magnitudeUpdates.decrementAtAndFutureSnapshots({ - key: uint32(block.timestamp), - decrementValue: magnitudeToDeallocate - }); - } else if (allocation.magnitudes[i] > uint64(currentMagnitude)) { - // Newly configured magnitude is greater than current value. - // Therefore we handle this as an allocation - - // 1. decrement free magnitude by incremented amount - uint64 magnitudeToAllocate = allocation.magnitudes[i] - uint64(currentMagnitude); - OperatorMagnitudeInfo storage info = operatorMagnitudeInfo[operator][allocation.strategy]; - require(info.freeMagnitude >= magnitudeToAllocate, InsufficientAllocatableMagnitude()); - info.freeMagnitude -= magnitudeToAllocate; - - // 2. allocate magnitude which will take effect in the future 21 days from now - magnitudeUpdates.push({key: allocationEffectTimestamp, value: allocation.magnitudes[i]}); - } - } - } - - /** - * @notice Get the number of queued allocation/deallocations for the given (operator, strategy, operatorSetKey) tuple - * @param operator address to get queued allocation/deallocations for - * @param strategy the strategy to get queued allocation/deallocations for - * @param operatorSetKey the encoded operatorSet to get queued allocation/deallocations for - * @dev Does not make an assumption on the number of pending allocations or deallocations at a time - */ - function _getPendingAllocationsAndDeallocations( - address operator, - IStrategy strategy, - bytes32 operatorSetKey - ) internal view returns (uint256 numPendingAllocations, uint256 numPendingDeallocations) { - // Get pending allocations - Snapshots.History storage magnitudeUpdates = _magnitudeUpdate[operator][strategy][operatorSetKey]; - - // Read current magnitude's respective array index and length. - (,uint256 latestAllocPos, uint256 allocLength) = - magnitudeUpdates.upperLookupRecentWithPos(uint32(block.timestamp)); - - if (latestAllocPos < allocLength - 1) { - numPendingAllocations = allocLength - 1 - latestAllocPos; - } - // Else, latestAllocPos >= allocLength - 1; - // latestAllocPos cannot be greater than length - 1, thus when pos == length - 1 there are no pending allocations - - // Get pending deallocations - uint256 deallocationsLength = _queuedDeallocationIndices[operator][strategy][operatorSetKey].length; - for (uint256 i = deallocationsLength; i > 0; --i) { - // index of pendingFreeMagnitude/deallocation to check for slashing - uint256 index = _queuedDeallocationIndices[operator][strategy][operatorSetKey][i - 1]; - - // If completableTimestamp is greater than completeUntilTimestamp, break - if (block.timestamp < _pendingFreeMagnitude[operator][strategy][index].completableTimestamp) { - ++numPendingDeallocations; - } else { - break; - } - } + operatorFreeMagnitudeInfo[operator][strategy] = freeInfo; } /// @dev gets the latest total magnitude or overwrites it if it is not set @@ -417,7 +300,7 @@ contract AllocationManager is if (!exists) { totalMagnitude = WAD; _totalMagnitudeUpdate[operator][strategy].push({key: uint32(block.timestamp), value: totalMagnitude}); - operatorMagnitudeInfo[operator][strategy].freeMagnitude = WAD; + operatorFreeMagnitudeInfo[operator][strategy].freeMagnitude = WAD; } return uint64(totalMagnitude); @@ -430,25 +313,53 @@ contract AllocationManager is IStrategy strategy, uint16 numToComplete, uint192 nextIndex - ) internal view returns (uint64 freeMagnitudeToAdd, uint192 completed) { - uint256 pendingFreeMagnitudeLength = _pendingFreeMagnitude[operator][strategy].length; + ) internal returns (uint64 freeMagnitudeToAdd, uint192 completed) { + uint256 numDeallocations = _pendingDeallocationOperatorSets[operator][strategy].length; freeMagnitudeToAdd = 0; - while (nextIndex < pendingFreeMagnitudeLength && completed < numToComplete) { - PendingFreeMagnitude memory pendingFreeMagnitude = _pendingFreeMagnitude[operator][strategy][nextIndex]; - // pendingFreeMagnitude is ordered by completableTimestamp. If we reach one that is not completable yet, then break + while (nextIndex < numDeallocations && completed < numToComplete) { + bytes32 opsetKey = _pendingDeallocationOperatorSets[operator][strategy][nextIndex]; + MagnitudeInfo memory opsetMagnitudeInfo = _operatorMagnitudeInfo[operator][strategy][opsetKey]; + // _pendingDeallocationOperatorSets is ordered by `effectTimestamp`. If we reach one that is not completable yet, then break // loop until completableTimestamp is < block.timestamp - if (block.timestamp < pendingFreeMagnitude.completableTimestamp) { + if (block.timestamp < opsetMagnitudeInfo.effectTimestamp) { break; } // pending free magnitude can be added to freeMagnitude - freeMagnitudeToAdd += pendingFreeMagnitude.magnitudeDiff; + freeMagnitudeToAdd += uint64(uint128(-opsetMagnitudeInfo.pendingMagnitudeDiff)); + _operatorMagnitudeInfo[operator][strategy][opsetKey] = MagnitudeInfo({ + pendingMagnitudeDiff: 0, + currentMagnitude: opsetMagnitudeInfo.currentMagnitude - uint64(uint128(-opsetMagnitudeInfo.pendingMagnitudeDiff)), + effectTimestamp: 0 + }); + ++nextIndex; ++completed; } return (freeMagnitudeToAdd, completed); } + /// @dev Fetch the operator's current magnitude, applying a pending diff if the effect timestamp is passed + /// @notice This may return something that is not recorded in state. Remember to store this updated value if needed! + function _getCurrentMagnitudeWithPending(address operator, IStrategy strategy, bytes32 operatorSetKey) internal view returns (MagnitudeInfo memory) { + MagnitudeInfo memory mInfo = _operatorMagnitudeInfo[operator][strategy][operatorSetKey]; + + // If the magnitude change is not yet in effect, return unaltered + if (block.timestamp < mInfo.effectTimestamp) { + return mInfo; + } + + // Otherwise, calculate the new current magnitude and return the modified struct + if (mInfo.pendingMagnitudeDiff >= 0) { + mInfo.currentMagnitude += uint64(uint128(mInfo.pendingMagnitudeDiff)); + } else { + mInfo.currentMagnitude -= uint64(uint128(-mInfo.pendingMagnitudeDiff)); + } + + mInfo.pendingMagnitudeDiff = 0; + return mInfo; + } + /** * @notice Returns the allocation delay of an operator * @param operator The operator to get the allocation delay for @@ -480,11 +391,8 @@ contract AllocationManager is for (uint256 i = 0; i < strategies.length; ++i) { slashableMagnitudes[i] = new uint64[](operatorSets.length); for (uint256 j = 0; j < operatorSets.length; ++j) { - slashableMagnitudes[i][j] = uint64( - _magnitudeUpdate[operator][strategies[i]][_encodeOperatorSet(operatorSets[j])].upperLookupLinear( - uint32(block.timestamp) - ) - ); + MagnitudeInfo memory mInfo = _getCurrentMagnitudeWithPending(operator, strategies[i], _encodeOperatorSet(operatorSets[j])); + slashableMagnitudes[i][j] = mInfo.currentMagnitude; } } return (operatorSets, slashableMagnitudes); @@ -497,13 +405,17 @@ contract AllocationManager is * @param strategy the strategy to get the allocatable magnitude for */ function getAllocatableMagnitude(address operator, IStrategy strategy) external view returns (uint64) { - OperatorMagnitudeInfo storage info = operatorMagnitudeInfo[operator][strategy]; - (uint64 freeMagnitudeToAdd,) = _getPendingFreeMagnitude({ - operator: operator, - strategy: strategy, - numToComplete: type(uint16).max, - nextIndex: info.nextPendingFreeMagnitudeIndex - }); + FreeMagnitudeInfo memory info = operatorFreeMagnitudeInfo[operator][strategy]; + uint256 numDeallocations = _pendingDeallocationOperatorSets[operator][strategy].length; + uint64 freeMagnitudeToAdd = 0; + for (uint192 i = info.nextPendingIndex; i < numDeallocations; ++i) { + bytes32 opsetKey = _pendingDeallocationOperatorSets[operator][strategy][i]; + MagnitudeInfo memory opsetMagnitudeInfo = _operatorMagnitudeInfo[operator][strategy][opsetKey]; + if (block.timestamp < opsetMagnitudeInfo.effectTimestamp) { + break; + } + freeMagnitudeToAdd += uint64(uint128(-opsetMagnitudeInfo.pendingMagnitudeDiff)); + } return info.freeMagnitude + freeMagnitudeToAdd; } @@ -579,7 +491,6 @@ contract AllocationManager is } else { totalMagnitude = uint64(value); } - return totalMagnitude; } @@ -590,31 +501,27 @@ 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 pendingMagnitude = 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) { - // We use latestSnapshot to get the latest pending allocation for an operatorSet. - (bool exists, uint32 key, uint224 value) = - _magnitudeUpdate[operator][strategy][_encodeOperatorSet(operatorSets[i])].latestSnapshot(); - if (exists) { - if (key > block.timestamp) { - pendingMagnitude[i] = uint64(value); - timestamps[i] = key; - } else { - pendingMagnitude[i] = 0; - timestamps[i] = 0; - } + 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; + } else { + pendingMagnitudes[i] = 0; + timestamps[i] = 0; } } - return (pendingMagnitude, timestamps); } /** @@ -624,28 +531,28 @@ contract AllocationManager is * @param operator the operator to get the pending deallocations for * @param strategy the strategy to get the pending deallocations for * @param operatorSets the operatorSets to get the pending deallocations for - * @return pendingMagnitudes the latest pending deallocation + * @return pendingMagnitudeDiffs the pending deallocation diffs for each operatorSet + * @return timestamps the timestamps for each pending dealloction */ function getPendingDeallocations( address operator, IStrategy strategy, OperatorSet[] calldata operatorSets - ) external view returns (PendingFreeMagnitude[] memory) { - PendingFreeMagnitude[] memory pendingMagnitudes = new PendingFreeMagnitude[](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) { - uint256[] storage deallocationIndices = - _queuedDeallocationIndices[operator][strategy][_encodeOperatorSet(operatorSets[i])]; - - if (deallocationIndices.length > 0) { - uint256 deallocationIndex = deallocationIndices[deallocationIndices.length - 1]; - PendingFreeMagnitude storage latestPendingMagnitude = - _pendingFreeMagnitude[operator][strategy][deallocationIndex]; - if (latestPendingMagnitude.completableTimestamp > block.timestamp) { - pendingMagnitudes[i] = latestPendingMagnitude; - } + MagnitudeInfo memory opsetMagnitudeInfo = _operatorMagnitudeInfo[operator][strategy][_encodeOperatorSet(operatorSets[i])]; + + if (opsetMagnitudeInfo.effectTimestamp < block.timestamp && opsetMagnitudeInfo.pendingMagnitudeDiff < 0) { + pendingMagnitudeDiffs[i] = uint64(uint128(-opsetMagnitudeInfo.pendingMagnitudeDiff)); + timestamps[i] = opsetMagnitudeInfo.effectTimestamp; + } else { + pendingMagnitudeDiffs[i] = 0; + timestamps[i] = 0; } } - return pendingMagnitudes; } /// @dev Verify operator's signature and spend salt diff --git a/src/contracts/core/AllocationManagerStorage.sol b/src/contracts/core/AllocationManagerStorage.sol index 8abe6a3a1..879602d22 100644 --- a/src/contracts/core/AllocationManagerStorage.sol +++ b/src/contracts/core/AllocationManagerStorage.sol @@ -45,17 +45,14 @@ abstract contract AllocationManagerStorage is IAllocationManager { /// Note that totalMagnitude is monotonically decreasing and only gets updated upon slashing mapping(address => mapping(IStrategy => Snapshots.History)) internal _totalMagnitudeUpdate; - /// @notice Mapping: operator => strategy => operatorSet (encoded) => snapshotted magnitude - mapping(address => mapping(IStrategy => mapping(bytes32 => Snapshots.History))) internal _magnitudeUpdate; - /// @notice Mapping: operator => strategy => OperatorMagnitudeInfo to keep track of info regarding pending magnitude allocations. - mapping(address => mapping(IStrategy => OperatorMagnitudeInfo)) public operatorMagnitudeInfo; + mapping(address => mapping(IStrategy => FreeMagnitudeInfo)) public operatorFreeMagnitudeInfo; - /// @notice Mapping: operator => strategy => PendingFreeMagnitude[] to keep track of pending free magnitude from deallocations - mapping(address => mapping(IStrategy => PendingFreeMagnitude[])) internal _pendingFreeMagnitude; + /// @notice Mapping: operator => strategy => operatorSet (encoded) => MagnitudeInfo + mapping(address => mapping(IStrategy => mapping(bytes32 => MagnitudeInfo))) internal _operatorMagnitudeInfo; - /// @notice Mapping: operator => strategy => operatorSet (encoded) => list of queuedDeallocation indices - mapping(address => mapping(IStrategy => mapping(bytes32 => uint256[]))) internal _queuedDeallocationIndices; + /// @notice Mapping: operator => strategy => operatorSet[] (encoded) to keep track of pending free magnitude for operatorSet from deallocations + mapping(address => mapping(IStrategy => bytes32[])) internal _pendingDeallocationOperatorSets; /// @notice Mapping: operator => allocation delay (in seconds) for the operator. /// This determines how long it takes for allocations to take effect in the future. @@ -78,5 +75,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[44] private __gap; } diff --git a/src/contracts/interfaces/IAllocationManager.sol b/src/contracts/interfaces/IAllocationManager.sol index 7b967ae20..89fa1a6c1 100644 --- a/src/contracts/interfaces/IAllocationManager.sol +++ b/src/contracts/interfaces/IAllocationManager.sol @@ -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. @@ -53,18 +53,30 @@ interface IAllocationManager is ISignatureUtils { * @param magnitudeDiff the amount of magnitude to deallocate * @param completableTimestamp the timestamp at which the deallocation can be completed, 21 days from when queued */ - struct PendingFreeMagnitude { - uint64 magnitudeDiff; - uint32 completableTimestamp; + // struct PendingFreeMagnitude { + // uint64 magnitudeDiff; + // uint32 completableTimestamp; + // } + + /** + * @notice struct used for operator magnitude updates. Stored in _operatorMagnitudeInfo mapping + * @param currentMagnitude the current magnitude of the operator + * @param pendingMagnitudeIdff the pending magnitude difference of the operator + * @param effectTimestamp the timestamp at which the pending magnitude will take effect + */ + struct MagnitudeInfo { + int128 pendingMagnitudeDiff; + uint64 currentMagnitude; + uint32 effectTimestamp; } /** * @notice Struct containing info regarding free allocatable magnitude. - * @param nextPendingFreeMagnitudeIndex The next available update index. + * @param nextPendingIndex The next available update index. * @param freeMagnitude The total amount of free allocatable magnitude. */ - struct OperatorMagnitudeInfo { - uint192 nextPendingFreeMagnitudeIndex; + struct FreeMagnitudeInfo { + uint192 nextPendingIndex; uint64 freeMagnitude; } @@ -239,13 +251,14 @@ interface IAllocationManager is ISignatureUtils { * @param operator the operator to get the pending deallocations for * @param strategy the strategy to get the pending deallocations for * @param operatorSets the operatorSets to get the pending deallocations for - * @return pendingMagnitudes the latest pending deallocation + * @return pendingMagnitudeDiffs the pending deallocation diffs for each operatorSet + * @return timestamps the timestamps for each pending dealloction */ function getPendingDeallocations( address operator, IStrategy strategy, OperatorSet[] calldata operatorSets - ) external view returns (PendingFreeMagnitude[] memory); + ) external view returns (uint64[] memory, uint32[] memory); /** * @notice operator is slashable by operatorSet if currently registered OR last deregistered within 21 days diff --git a/src/test/events/IRewardsCoordinatorEvents.sol b/src/test/events/IRewardsCoordinatorEvents.sol index 30894a91b..94a24aefc 100644 --- a/src/test/events/IRewardsCoordinatorEvents.sol +++ b/src/test/events/IRewardsCoordinatorEvents.sol @@ -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( @@ -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 + // ); diff --git a/src/test/mocks/DelegationManagerMock.sol b/src/test/mocks/DelegationManagerMock.sol index 4af0c239b..483bf7b5f 100644 --- a/src/test/mocks/DelegationManagerMock.sol +++ b/src/test/mocks/DelegationManagerMock.sol @@ -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( @@ -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); } } diff --git a/src/test/mocks/EigenPodManagerMock.sol b/src/test/mocks/EigenPodManagerMock.sol index 607186959..3641fd24f 100644 --- a/src/test/mocks/EigenPodManagerMock.sol +++ b/src/test/mocks/EigenPodManagerMock.sol @@ -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) {} diff --git a/src/test/mocks/StrategyManagerMock.sol b/src/test/mocks/StrategyManagerMock.sol index 0c34951df..01d1bf734 100644 --- a/src/test/mocks/StrategyManagerMock.sol +++ b/src/test/mocks/StrategyManagerMock.sol @@ -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) {} diff --git a/src/test/unit/EigenPodManagerUnit.t.sol b/src/test/unit/EigenPodManagerUnit.t.sol index 8fe8c14c7..c64bc1583 100644 --- a/src/test/unit/EigenPodManagerUnit.t.sol +++ b/src/test/unit/EigenPodManagerUnit.t.sol @@ -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 { @@ -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"); diff --git a/src/test/unit/StrategyManagerUnit.t.sol b/src/test/unit/StrategyManagerUnit.t.sol index 0ec6b8858..54e62ed76 100644 --- a/src/test/unit/StrategyManagerUnit.t.sol +++ b/src/test/unit/StrategyManagerUnit.t.sol @@ -979,18 +979,18 @@ contract StrategyManagerUnitTests_addShares is StrategyManagerUnitTests { function test_Revert_DelegationManagerModifier() external { DelegationManagerMock invalidDelegationManager = new DelegationManagerMock(); cheats.expectRevert("StrategyManager.onlyDelegationManager: not the DelegationManager"); - invalidDelegationManager.addShares(strategyManager, address(this), dummyToken, dummyStrat, 1); + invalidDelegationManager.addOwnedShares(strategyManager, address(this), dummyToken, dummyStrat, 1); } function testFuzz_Revert_StakerZeroAddress(uint256 amount) external { cheats.expectRevert("StrategyManager._addShares: staker cannot be zero address"); - delegationManagerMock.addShares(strategyManager, address(0), dummyToken, dummyStrat, amount); + delegationManagerMock.addOwnedShares(strategyManager, address(0), dummyToken, dummyStrat, amount); } function testFuzz_Revert_ZeroShares(address staker) external filterFuzzedAddressInputs(staker) { cheats.assume(staker != address(0)); cheats.expectRevert("StrategyManager._addShares: shares should not be zero!"); - delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, 0); + delegationManagerMock.addOwnedShares(strategyManager, staker, dummyToken, dummyStrat, 0); } function testFuzz_AppendsStakerStrategyList( @@ -1003,7 +1003,7 @@ contract StrategyManagerUnitTests_addShares is StrategyManagerUnitTests { assertEq(sharesBefore, 0, "Staker has already deposited into this strategy"); assertFalse(_isDepositedStrategy(staker, dummyStrat), "strategy should not be deposited"); - delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, amount); + delegationManagerMock.addOwnedShares(strategyManager, staker, dummyToken, dummyStrat, amount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq( @@ -1028,7 +1028,7 @@ contract StrategyManagerUnitTests_addShares is StrategyManagerUnitTests { assertEq(sharesBefore, initialAmount, "Staker has not deposited amount into strategy"); assertTrue(_isDepositedStrategy(staker, strategy), "strategy should be deposited"); - delegationManagerMock.addShares(strategyManager, staker, dummyToken, dummyStrat, sharesAmount); + delegationManagerMock.addOwnedShares(strategyManager, staker, dummyToken, dummyStrat, sharesAmount); uint256 stakerStrategyListLengthAfter = strategyManager.stakerStrategyListLength(staker); uint256 sharesAfter = strategyManager.stakerStrategyShares(staker, dummyStrat); assertEq( @@ -1077,7 +1077,7 @@ contract StrategyManagerUnitTests_addShares is StrategyManagerUnitTests { cheats.prank(staker); cheats.expectRevert("StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); - delegationManagerMock.addShares(strategyManager, staker, dummyToken, strategy, amount); + delegationManagerMock.addOwnedShares(strategyManager, staker, dummyToken, strategy, amount); cheats.expectRevert("StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"); strategyManager.depositIntoStrategy(strategy, token, amount);