Skip to content
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

refactor: decouple AVS<>Operator mapping from DelegationManager #403

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
174 changes: 174 additions & 0 deletions src/contracts/core/AVSDirectory.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity =0.8.12;

import "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
import "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol";
import "@openzeppelin-upgrades/contracts/security/ReentrancyGuardUpgradeable.sol";
import "../permissions/Pausable.sol";
import "../libraries/EIP1271SignatureUtils.sol";
import "./AVSDirectoryStorage.sol";

contract AVSDirectory is
Initializable,
OwnableUpgradeable,
Pausable,
AVSDirectoryStorage,
ReentrancyGuardUpgradeable
{
// @dev Index for flag that pauses operator register/deregister to avs when set.
uint8 internal constant PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS = 0;

// @dev Chain ID at the time of contract deployment
uint256 internal immutable ORIGINAL_CHAIN_ID;

/*******************************************************************************
INITIALIZING FUNCTIONS
*******************************************************************************/

/**
* @dev Initializes the immutable addresses of the strategy mananger, delegationManager, slasher,
* and eigenpodManager contracts
*/
constructor(IDelegationManager _delegation) AVSDirectoryStorage(_delegation) {
_disableInitializers();
ORIGINAL_CHAIN_ID = block.chainid;
}

/**
* @dev Initializes the addresses of the initial owner, pauser registry, and paused status.
* minWithdrawalDelayBlocks is set only once here
*/
function initialize(
address initialOwner,
IPauserRegistry _pauserRegistry,
uint256 initialPausedStatus
) external initializer {
_initializePauser(_pauserRegistry, initialPausedStatus);
_DOMAIN_SEPARATOR = _calculateDomainSeparator();
_transferOwnership(initialOwner);
}

/*******************************************************************************
EXTERNAL FUNCTIONS
*******************************************************************************/


/**
* @notice Called by the AVS's service manager contract to register an operator with the avs.
* @param operator The address of the operator to register.
* @param operatorSignature The signature, salt, and expiry of the operator's signature.
*/
function registerOperatorToAVS(
address operator,
ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature
) external onlyWhenNotPaused(PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS) {

require(
operatorSignature.expiry >= block.timestamp,
"AVSDirectory.registerOperatorToAVS: operator signature expired"
);
require(
avsOperatorStatus[msg.sender][operator] != OperatorAVSRegistrationStatus.REGISTERED,
"AVSDirectory.registerOperatorToAVS: operator already registered"
);
require(
!operatorSaltIsSpent[operator][operatorSignature.salt],
"AVSDirectory.registerOperatorToAVS: salt already spent"
);
require(
delegation.isOperator(operator),
"AVSDirectory.registerOperatorToAVS: operator not registered to EigenLayer yet");

// Calculate the digest hash
bytes32 operatorRegistrationDigestHash = calculateOperatorAVSRegistrationDigestHash({
operator: operator,
avs: msg.sender,
salt: operatorSignature.salt,
expiry: operatorSignature.expiry
});

// Check that the signature is valid
EIP1271SignatureUtils.checkSignature_EIP1271(
operator,
operatorRegistrationDigestHash,
operatorSignature.signature
);

// Set the operator as registered
avsOperatorStatus[msg.sender][operator] = OperatorAVSRegistrationStatus.REGISTERED;

// Mark the salt as spent
operatorSaltIsSpent[operator][operatorSignature.salt] = true;

emit OperatorAVSRegistrationStatusUpdated(operator, msg.sender, OperatorAVSRegistrationStatus.REGISTERED);
}

/**
* @notice Called by an avs to deregister an operator with the avs.
* @param operator The address of the operator to deregister.
*/
function deregisterOperatorFromAVS(address operator) external onlyWhenNotPaused(PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS) {
require(
avsOperatorStatus[msg.sender][operator] == OperatorAVSRegistrationStatus.REGISTERED,
"AVSDirectory.deregisterOperatorFromAVS: operator not registered"
);

// Set the operator as deregistered
avsOperatorStatus[msg.sender][operator] = OperatorAVSRegistrationStatus.UNREGISTERED;

emit OperatorAVSRegistrationStatusUpdated(operator, msg.sender, OperatorAVSRegistrationStatus.UNREGISTERED);
}

/**
* @notice Called by an avs to emit an `AVSMetadataURIUpdated` event indicating the information has updated.
* @param metadataURI The URI for metadata associated with an avs
*/
function updateAVSMetadataURI(string calldata metadataURI) external {
emit AVSMetadataURIUpdated(msg.sender, metadataURI);
}

/*******************************************************************************
VIEW FUNCTIONS
*******************************************************************************/

/**
* @notice Calculates the digest hash to be signed by an operator to register with an AVS
* @param operator The account registering as an operator
* @param avs The address of the service manager contract for the AVS that the operator is registering to
* @param salt A unique and single use value associated with the approver signature.
* @param expiry Time after which the approver's signature becomes invalid
*/
function calculateOperatorAVSRegistrationDigestHash(
address operator,
address avs,
bytes32 salt,
uint256 expiry
) public view returns (bytes32) {
// calculate the struct hash
bytes32 structHash = keccak256(
abi.encode(OPERATOR_AVS_REGISTRATION_TYPEHASH, operator, avs, salt, expiry)
);
// calculate the digest hash
bytes32 digestHash = keccak256(
abi.encodePacked("\x19\x01", domainSeparator(), structHash)
);
return digestHash;
}

/**
* @notice Getter function for the current EIP-712 domain separator for this contract.
* @dev The domain separator will change in the event of a fork that changes the ChainID.
*/
function domainSeparator() public view returns (bytes32) {
if (block.chainid == ORIGINAL_CHAIN_ID) {
return _DOMAIN_SEPARATOR;
} else {
return _calculateDomainSeparator();
}
}

// @notice Internal function for calculating the current domain separator of this contract
function _calculateDomainSeparator() internal view returns (bytes32) {
return keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes("EigenLayer")), block.chainid, address(this)));
}
}
46 changes: 46 additions & 0 deletions src/contracts/core/AVSDirectoryStorage.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity =0.8.12;

import "../interfaces/IAVSDirectory.sol";
import "../interfaces/IStrategyManager.sol";
import "../interfaces/IDelegationManager.sol";
import "../interfaces/ISlasher.sol";
import "../interfaces/IEigenPodManager.sol";

abstract contract AVSDirectoryStorage is IAVSDirectory {
/// @notice The EIP-712 typehash for the contract's domain
bytes32 public constant DOMAIN_TYPEHASH =
keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");

/// @notice The EIP-712 typehash for the `Registration` struct used by the contract
bytes32 public constant OPERATOR_AVS_REGISTRATION_TYPEHASH =
keccak256("OperatorAVSRegistration(address operator,address avs,bytes32 salt,uint256 expiry)");

/// @notice The DelegationManager contract for EigenLayer
IDelegationManager public immutable delegation;
8sunyuan marked this conversation as resolved.
Show resolved Hide resolved

/**
* @notice Original EIP-712 Domain separator for this contract.
* @dev The domain separator may change in the event of a fork that modifies the ChainID.
* Use the getter function `domainSeparator` to get the current domain separator for this contract.
*/
bytes32 internal _DOMAIN_SEPARATOR;

/// @notice Mapping: AVS => operator => enum of operator status to the AVS
mapping(address => mapping(address => OperatorAVSRegistrationStatus)) public avsOperatorStatus;

/// @notice Mapping: operator => 32-byte salt => whether or not the salt has already been used by the operator.
/// @dev Salt is used in the `registerOperatorToAVS` function.
mapping(address => mapping(bytes32 => bool)) public operatorSaltIsSpent;

constructor(IDelegationManager _delegation) {
delegation = _delegation;
}

/**
* @dev This empty reserved space is put in place to allow future versions to add new
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[47] private __gap;
}
104 changes: 1 addition & 103 deletions src/contracts/core/DelegationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ pragma solidity =0.8.12;
import "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
import "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol";
import "@openzeppelin-upgrades/contracts/security/ReentrancyGuardUpgradeable.sol";
import "../interfaces/ISlasher.sol";
import "./DelegationManagerStorage.sol";
import "../permissions/Pausable.sol";
import "../libraries/EIP1271SignatureUtils.sol";
import "./DelegationManagerStorage.sol";

/**
* @title DelegationManager
Expand All @@ -29,9 +28,6 @@ contract DelegationManager is Initializable, OwnableUpgradeable, Pausable, Deleg
// @dev Index for flag that pauses completing existing withdrawals when set.
uint8 internal constant PAUSED_EXIT_WITHDRAWAL_QUEUE = 2;

// @dev Index for flag that pauses operator register/deregister to avs when set.
uint8 internal constant PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS = 3;

// @dev Chain ID at the time of contract deployment
uint256 internal immutable ORIGINAL_CHAIN_ID;

Expand Down Expand Up @@ -136,14 +132,6 @@ contract DelegationManager is Initializable, OwnableUpgradeable, Pausable, Deleg
emit OperatorMetadataURIUpdated(msg.sender, metadataURI);
}

/**
* @notice Called by an avs to emit an `AVSMetadataURIUpdated` event indicating the information has updated.
* @param metadataURI The URI for metadata associated with an avs
*/
function updateAVSMetadataURI(string calldata metadataURI) external {
emit AVSMetadataURIUpdated(msg.sender, metadataURI);
}

/**
* @notice Caller delegates their stake to an operator.
* @param operator The account (`msg.sender`) is delegating its assets to for use in serving applications built on EigenLayer.
Expand Down Expand Up @@ -435,72 +423,6 @@ contract DelegationManager is Initializable, OwnableUpgradeable, Pausable, Deleg
}
}

/**
* @notice Called by the AVS's service manager contract to register an operator with the avs.
* @param operator The address of the operator to register.
* @param operatorSignature The signature, salt, and expiry of the operator's signature.
*/
function registerOperatorToAVS(
address operator,
ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature
) external onlyWhenNotPaused(PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS) {

require(
operatorSignature.expiry >= block.timestamp,
"DelegationManager.registerOperatorToAVS: operator signature expired"
);
require(
avsOperatorStatus[msg.sender][operator] != OperatorAVSRegistrationStatus.REGISTERED,
"DelegationManager.registerOperatorToAVS: operator already registered"
);
require(
!operatorSaltIsSpent[operator][operatorSignature.salt],
"DelegationManager.registerOperatorToAVS: salt already spent"
);
require(
isOperator(operator),
"DelegationManager.registerOperatorToAVS: operator not registered to EigenLayer yet");

// Calculate the digest hash
bytes32 operatorRegistrationDigestHash = calculateOperatorAVSRegistrationDigestHash({
operator: operator,
avs: msg.sender,
salt: operatorSignature.salt,
expiry: operatorSignature.expiry
});

// Check that the signature is valid
EIP1271SignatureUtils.checkSignature_EIP1271(
operator,
operatorRegistrationDigestHash,
operatorSignature.signature
);

// Set the operator as registered
avsOperatorStatus[msg.sender][operator] = OperatorAVSRegistrationStatus.REGISTERED;

// Mark the salt as spent
operatorSaltIsSpent[operator][operatorSignature.salt] = true;

emit OperatorAVSRegistrationStatusUpdated(operator, msg.sender, OperatorAVSRegistrationStatus.REGISTERED);
}

/**
* @notice Called by an avs to deregister an operator with the avs.
* @param operator The address of the operator to deregister.
*/
function deregisterOperatorFromAVS(address operator) external onlyWhenNotPaused(PAUSED_OPERATOR_REGISTER_DEREGISTER_TO_AVS) {
require(
avsOperatorStatus[msg.sender][operator] == OperatorAVSRegistrationStatus.REGISTERED,
"DelegationManager.deregisterOperatorFromAVS: operator not registered"
);

// Set the operator as deregistered
avsOperatorStatus[msg.sender][operator] = OperatorAVSRegistrationStatus.UNREGISTERED;

emit OperatorAVSRegistrationStatusUpdated(operator, msg.sender, OperatorAVSRegistrationStatus.UNREGISTERED);
}

/**
* @notice Called by owner to set the minimum withdrawal delay blocks for each passed in strategy
* Note that the min number of blocks to complete a withdrawal of a strategy is
Expand Down Expand Up @@ -1061,30 +983,6 @@ contract DelegationManager is Initializable, OwnableUpgradeable, Pausable, Deleg
return approverDigestHash;
}

/**
* @notice Calculates the digest hash to be signed by an operator to register with an AVS
* @param operator The account registering as an operator
* @param avs The address of the service manager contract for the AVS that the operator is registering to
* @param salt A unique and single use value associated with the approver signature.
* @param expiry Time after which the approver's signature becomes invalid
*/
function calculateOperatorAVSRegistrationDigestHash(
address operator,
address avs,
bytes32 salt,
uint256 expiry
) public view returns (bytes32) {
// calculate the struct hash
bytes32 structHash = keccak256(
abi.encode(OPERATOR_AVS_REGISTRATION_TYPEHASH, operator, avs, salt, expiry)
);
// calculate the digest hash
bytes32 digestHash = keccak256(
abi.encodePacked("\x19\x01", domainSeparator(), structHash)
);
return digestHash;
}

/**
* @dev Recalculates the domain separator when the chainid changes due to a fork.
*/
Expand Down
13 changes: 1 addition & 12 deletions src/contracts/core/DelegationManagerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ abstract contract DelegationManagerStorage is IDelegationManager {
bytes32 public constant DELEGATION_APPROVAL_TYPEHASH =
keccak256("DelegationApproval(address staker,address operator,bytes32 salt,uint256 expiry)");

/// @notice The EIP-712 typehash for the `Registration` struct used by the contract
bytes32 public constant OPERATOR_AVS_REGISTRATION_TYPEHASH =
keccak256("OperatorAVSRegistration(address operator,address avs,bytes32 salt,uint256 expiry)");

/**
* @notice Original EIP-712 Domain separator for this contract.
* @dev The domain separator may change in the event of a fork that modifies the ChainID.
Expand Down Expand Up @@ -99,13 +95,6 @@ abstract contract DelegationManagerStorage is IDelegationManager {
/// See conversation here: https://github.com/Layr-Labs/eigenlayer-contracts/pull/365/files#r1417525270
address private __deprecated_stakeRegistry;

/// @notice Mapping: AVS => operator => enum of operator status to the AVS
mapping(address => mapping(address => OperatorAVSRegistrationStatus)) public avsOperatorStatus;

/// @notice Mapping: operator => 32-byte salt => whether or not the salt has already been used by the operator.
/// @dev Salt is used in the `registerOperatorToAVS` function.
mapping(address => mapping(bytes32 => bool)) public operatorSaltIsSpent;
Comment on lines -102 to -107
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to deprecate these variables instead? if they're internal and unused, i don't think it would contribute to bytecode size, though maybe i should double check that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm wondering what happens if we upgrade the current m2 deployment to something that doesn't have these variables when they've already been used -- if we have an m3 down the line that reintroduces variables at these slots, we might find storage collisions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH maybe it doesnt matter since we'll deprecate Goerli well before m3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These storage variables have not been set in the DM yet on mainnet so should be safe to remove. For second goerli upgrade however I need to doublecheck...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These variables weren't in the initial Goerli deployment either, at least if I recall correctly.
so that would mean super safe to delete.


/**
* @notice Minimum delay enforced by this contract per Strategy for completing queued withdrawals. Measured in blocks, and adjustable by this contract's owner,
* up to a maximum of `MAX_WITHDRAWAL_DELAY_BLOCKS`. Minimum value is 0 (i.e. no delay enforced).
Expand All @@ -123,5 +112,5 @@ abstract contract DelegationManagerStorage is IDelegationManager {
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[37] private __gap;
uint256[39] private __gap;
}
Loading
Loading