-
Notifications
You must be signed in to change notification settings - Fork 417
feat(draft): AllocationManager
redistribution support
#1346
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.
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
feat(draft): AllocationManager
redistribution support
#1346
Conversation
AllocationManager
redistribution changesAllocationManager
redistribution support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass
/// @notice Returns the address where slashed funds will be sent for a given operator set. | ||
/// @dev For redistributing Operator Sets, returns the configured redistribution address set during Operator Set creation. | ||
/// For non-redistributing operator sets, returns the `DEFAULT_BURN_ADDRESS`. | ||
mapping(bytes32 operatorSetKey => address redistributionAddr) internal _redistributionRecipients; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could combine the above two mappings into an OperatorSetInfo
struct? Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scott had an issue with storage packing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's with regards to using uint96
instead of uint256
. Using a struct should be fine IMO, see:
eigenlayer-contracts/src/contracts/core/DelegationManagerStorage.sol
Lines 71 to 74 in 734f736
/// @notice Returns the operator details for a given `operator`. | |
/// Note: two of the `OperatorDetails` fields are deprecated. The only relevant field | |
/// is `OperatorDetails.delegationApprover`. | |
mapping(address operator => OperatorDetails) internal _operatorDetails; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why combine them if not packing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Less total storage slots used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we added slashers, way easier to just have operatorSetDetails
. Not blocking review rn but we should consider
@@ -13,6 +13,8 @@ contract AllocationManagerUnitTests is EigenLayerUnitTestSetup, IAllocationManag | |||
/// Constants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should assert the slashCount
in tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And add a TODO to assert shares once we add to DM
@@ -4135,3 +4239,19 @@ contract AllocationManagerUnitTests_getAllocatedStake is AllocationManagerUnitTe | |||
assertEq(allocatedStake[0][0], DEFAULT_OPERATOR_SHARES.mulWad(5e17), "allocated stake should remain same after deregistration"); | |||
} | |||
} | |||
|
|||
contract AllocationManagerUnitTests_isRedistributingOperatorSet is AllocationManagerUnitTests { | |||
function testFuzz_isRedistributingOperatorSet(Randomness r) public rand(r) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to fuzz here?
|
||
OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); | ||
address redistributionRecipient = r.Address(); | ||
_createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test does the same as the previous AllocationManagerUnitTests_createRedistributingOperatorSets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
draft LGTM
701c569
into
Layr-Labs:release-dev/redistribution
Motivation:
Add support for redistributing slashed funds to specified recipients instead of burning them, while optimizing code size and improving test coverage.
Modifications:
Added a new redistribution system that allows AVSs to specify recipients for slashed funds. Code optimizations were made to reduce codesize, including consolidating repeated logic and a small
PermissionControllerMixin
refactor.Result:
Slashed funds can now be redirected to specified recipients instead of being burned, with improved code efficiency and test coverage.