feat: avs registrar #484
feat: avs registrar #484
Conversation
ypatil12
force-pushed
the
yash/avs-registrar
branch
from
ee780a5 to
5bb905e
Compare
There was a problem hiding this comment.
Have we considered at all about upgrading OZ?
There was a problem hiding this comment.
Not yet, are you suggesting for unstructured storage?
| * | ||
| */ | ||
|
|
||
| /// @notice The AVS that this registar is for |
| import {Allowlist} from "../modules/Allowlist.sol"; | ||
| import {OperatorSet} from "eigenlayer-contracts/src/contracts/libraries/OperatorSetLib.sol"; | ||
|
|
||
| contract AVSRegistrarWithAllowlist is AVSRegistrar, Allowlist { |
There was a problem hiding this comment.
contract name doesn't match file name
Stake evaluation done via |
| import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol"; | ||
|
|
||
| /// @notice A minimal AVSRegistrar contract that is used to register/deregister operators for an AVS | ||
| contract AVSRegistrar is Initializable, AVSRegistrarStorage { |
There was a problem hiding this comment.
If I needed to make this upgradeable, I'm assuming I would need to inherit and make the child contract upgradeable?
There was a problem hiding this comment.
Also should this be abstract or is it intended to be deployed as such if you want the bare functionality?
There was a problem hiding this comment.
Yup, see what we do with the presets. Intended to be deployed as such if you want the bare functionality
| _beforeDeregisterOperator(operator, operatorSetIds); | ||
|
|
||
| _afterDeregisterOperator(operator, operatorSetIds); |
There was a problem hiding this comment.
Don't we just need 1 hook here?
There was a problem hiding this comment.
Leaving it maximally flexible for now, can change later on. One idea rn is that you may want to preempt some gas-heavy optimizations in the before hook
|
|
||
| /// @inheritdoc ISocketRegistry | ||
| function updateSocket(address operator, string memory socket) external { | ||
| require(msg.sender == operator, CallerNotOperator()); |
There was a problem hiding this comment.
Should there be a check to see if the operator is registerd to atleast one operatorSet of the AVS?
There was a problem hiding this comment.
Doesn't matter since the value will be useless until operator is registered
| * @param socket The socket (any arbitrary string as deemed useful by an AVS) to set. | ||
| * @dev This function sets a single socket per operator, regardless of operatorSet. | ||
| */ | ||
| function _setOperatorSocket(address operator, string memory socket) internal { |
There was a problem hiding this comment.
Would this also mean that an operator has the same socket for all operator sets of an AVS?
There was a problem hiding this comment.
Haven't seen a compelling reason for it to be different given HG and EigenDA
There was a problem hiding this comment.
But we set the keys per operator set in Layr-Labs/eigenlayer-contracts#1421 right? Shouldn't we be uniform in implementation?
**Motivation:** We want to update the AVS registrar to be minimal & modular. **Modifications:** Add a new `AVSRegistrar` with first class support with the `KeyRegistrar`. We also have the following modules: 1. `SocketRegistry` 2. `AllowList` The above modules are _abstract_ and are inherited by `AVSRegistrars` in the `presets` folder: `AVSRegistrarWithAllowlist` and `AVSRegistrarWithSocket`. **Result:** Simple AVSRegistrar. Note that we are still pending on the `KeyRegistrar` interface update: Layr-Labs/eigenlayer-contracts#1421
Motivation:
We want to update the AVS registrar to be minimal & modular.
Modifications:
Add a new
AVSRegistrarwith first class support with theKeyRegistrar. We also have the following modules:SocketRegistryAllowListThe above modules are abstract and are inherited by
AVSRegistrarsin thepresetsfolder:AVSRegistrarWithAllowlistandAVSRegistrarWithSocket.Result:
Simple AVSRegistrar. Note that we are still pending on the
KeyRegistrarinterface update: Layr-Labs/eigenlayer-contracts#1421