From 1d535e1da3afa859796eef639dc4f78f46acd17e Mon Sep 17 00:00:00 2001 From: Kevin Halliday Date: Tue, 21 May 2024 14:24:21 -0400 Subject: [PATCH] chore(contracts): revert avs key separation (#1075) Reverts omni-network/omni#1054 We do not need those changes. Key separation will be handled separately, by moving registration to OmniStake task: none --- contracts/.gas-snapshot | 107 ++++----- contracts/src/interfaces/IOmniAVS.sol | 38 +-- contracts/src/protocol/OmniAVS.sol | 108 ++------- contracts/src/protocol/OmniAVSStorage.sol | 10 +- contracts/test/avs/OmniAVS_admin.t.sol | 2 +- contracts/test/avs/OmniAVS_allowlist.t.sol | 6 +- contracts/test/avs/OmniAVS_pause.t.sol | 2 +- contracts/test/avs/OmniAVS_registration.t.sol | 221 +----------------- .../test/avs/OmniAVS_serviceManager.t.sol | 2 +- contracts/test/avs/OmniAVS_syncWithOmni.t.sol | 8 +- contracts/test/avs/common/Utils.sol | 120 ++-------- 11 files changed, 128 insertions(+), 496 deletions(-) diff --git a/contracts/.gas-snapshot b/contracts/.gas-snapshot index f6ea96d7f..c832b45c3 100644 --- a/contracts/.gas-snapshot +++ b/contracts/.gas-snapshot @@ -6,70 +6,65 @@ FeeOracleV1_Test:test_setManager() (gas: 51736) FeeOracleV1_Test:test_setProtocolFee() (gas: 33039) FeeOracleV1_Test:test_setToNativeRate() (gas: 43576) OmniAVS_admin_Test:test_avsDirectory_succeeds() (gas: 15019) -OmniAVS_admin_Test:test_ejectOwner_byOwner_succeeds() (gas: 575409) -OmniAVS_admin_Test:test_ejectOwner_notOwner_reverts() (gas: 197834) -OmniAVS_admin_Test:test_getOperatorRestakedStrategies_noStrategies_succeeds() (gas: 683530) -OmniAVS_admin_Test:test_getOperatorRestakedStrategies_notOperator_succeeds() (gas: 198165) -OmniAVS_admin_Test:test_getOperatorRestakedStrategies_succeeds() (gas: 747431) -OmniAVS_admin_Test:test_getRestakeableStrategies_noStrategies_succeeds() (gas: 27488) -OmniAVS_admin_Test:test_getRestakeableStrategies_succeeds() (gas: 89269) -OmniAVS_admin_Test:test_pause_byOwner_succeeds() (gas: 44841) -OmniAVS_admin_Test:test_registerOperator_whenPaused_reverts() (gas: 228252) -OmniAVS_admin_Test:test_setEthStakeInbox_notOwner_reverts() (gas: 17977) -OmniAVS_admin_Test:test_setEthStakeInbox_succeeds() (gas: 27983) -OmniAVS_admin_Test:test_setEthStakeInbox_zeroAddress_reverts() (gas: 20524) -OmniAVS_admin_Test:test_setMaxOperatorCount_notOwner_reverts() (gas: 17924) -OmniAVS_admin_Test:test_setMaxOperatorCount_succeeds() (gas: 29324) +OmniAVS_admin_Test:test_ejectOwner_byOwner_succeeds() (gas: 430904) +OmniAVS_admin_Test:test_ejectOwner_notOwner_reverts() (gas: 107965) +OmniAVS_admin_Test:test_getOPeratorRestakedStrategies_notOperator_succeeds() (gas: 108351) +OmniAVS_admin_Test:test_getOperatorRestakedStrategies_noStrategies_succeeds() (gas: 513975) +OmniAVS_admin_Test:test_getOperatorRestakedStrategies_succeeds() (gas: 577871) +OmniAVS_admin_Test:test_getRestakeableStrategies_noStrategies_succeeds() (gas: 27453) +OmniAVS_admin_Test:test_getRestakeableStrategies_succeeds() (gas: 89247) +OmniAVS_admin_Test:test_pause_byOwner_succeeds() (gas: 44863) +OmniAVS_admin_Test:test_registerOperator_whenPaused_reverts() (gas: 138161) +OmniAVS_admin_Test:test_setEthStakeInbox_notOwner_reverts() (gas: 18021) +OmniAVS_admin_Test:test_setEthStakeInbox_succeeds() (gas: 28049) +OmniAVS_admin_Test:test_setEthStakeInbox_zeroAddress_reverts() (gas: 20568) +OmniAVS_admin_Test:test_setMaxOperatorCount_notOwner_reverts() (gas: 17968) +OmniAVS_admin_Test:test_setMaxOperatorCount_succeeds() (gas: 29456) OmniAVS_admin_Test:test_setMetadataURI_notOwner_reverts() (gas: 18700) OmniAVS_admin_Test:test_setMetadataURI_succeeds() (gas: 38538) -OmniAVS_admin_Test:test_setMinOperatorStake_notOwner_reverts() (gas: 18006) +OmniAVS_admin_Test:test_setMinOperatorStake_notOwner_reverts() (gas: 18072) OmniAVS_admin_Test:test_setMinOperatorStake_succeeds() (gas: 30026) OmniAVS_admin_Test:test_setOmniChainId_notOwner_reverts() (gas: 17973) OmniAVS_admin_Test:test_setOmniChainId_succeeds() (gas: 27834) OmniAVS_admin_Test:test_setPortal_notOwner_reverts() (gas: 17912) OmniAVS_admin_Test:test_setPortal_succeeds() (gas: 27920) OmniAVS_admin_Test:test_setPortal_zeroAddress_reverts() (gas: 20448) -OmniAVS_admin_Test:test_setStrategyParams_duplicateStrategy_reverts() (gas: 38127) -OmniAVS_admin_Test:test_setStrategyParams_notOwner_reverts() (gas: 18351) -OmniAVS_admin_Test:test_setStrategyParams_succeds() (gas: 74851) -OmniAVS_admin_Test:test_setStrategyParams_twice_succeeds() (gas: 56003) -OmniAVS_admin_Test:test_setStrategyParams_zeroAddress_reverts() (gas: 36848) -OmniAVS_admin_Test:test_setStrategyParams_zeroMul_reverts() (gas: 37116) -OmniAVS_admin_Test:test_setXCallGasLimits_notOwner_reverts() (gas: 18157) -OmniAVS_admin_Test:test_setXCallGasLimits_succeeds() (gas: 33504) -OmniAVS_admin_Test:test_syncWithOmni_whenPaused_reverts() (gas: 45242) -OmniAVS_admin_Test:test_unpause_byOwner_succeeds() (gas: 35649) -OmniAVS_allowlist_Test:test_addToAllowlist_notOwner_reverts() (gas: 197789) -OmniAVS_allowlist_Test:test_addToAllowlist_succeeds() (gas: 225166) -OmniAVS_allowlist_Test:test_disableAllowlist_alreadyDisabled_reverts() (gas: 31644) -OmniAVS_allowlist_Test:test_disableAllowlist_notOwner_reverts() (gas: 17801) -OmniAVS_allowlist_Test:test_disableAllowlist_succeeds() (gas: 28912) -OmniAVS_allowlist_Test:test_enableAllowlist_alreadyEnabled_reverts() (gas: 25699) -OmniAVS_allowlist_Test:test_enableAllowlist_notOwner_reverts() (gas: 17845) -OmniAVS_allowlist_Test:test_enableAllowlist_succeeds() (gas: 29411) -OmniAVS_allowlist_Test:test_registerOperator_allowlistDisabled_succeeds() (gas: 686392) -OmniAVS_allowlist_Test:test_registerOperator_nowAllowed_reverts() (gas: 206766) -OmniAVS_allowlist_Test:test_registerOperator_succeeds() (gas: 704810) -OmniAVS_allowlist_Test:test_removeFromAllowlist_notOwner_reverts() (gas: 197759) -OmniAVS_allowlist_Test:test_removeFromAllowlist_succeeds() (gas: 420243) -OmniAVS_canRegister_Test:test_canRegister_allowed() (gas: 417890) -OmniAVS_canRegister_Test:test_canRegister_allowlistDisabled() (gas: 397819) -OmniAVS_canRegister_Test:test_canRegister_alreadyRegistered() (gas: 689393) -OmniAVS_canRegister_Test:test_canRegister_maxOperatorsReached() (gas: 351776) -OmniAVS_canRegister_Test:test_canRegister_minStakeNotMet() (gas: 418372) -OmniAVS_canRegister_Test:test_canRegister_notAllowed() (gas: 312646) -OmniAVS_canRegister_Test:test_canRegister_notOperator() (gas: 209798) -OmniAVS_initialize_Test:test_initialize_defaultParams_succeeds() (gas: 4078613) -OmniAVS_registration_Test:test_registerOperator_activeValKey_reverts() (gas: 1043345) -OmniAVS_registration_Test:test_registerOperator_expiredSig_reverts() (gas: 441910) -OmniAVS_registration_Test:test_registerOperator_invalidDigestHash_reverts() (gas: 448834) -OmniAVS_registration_Test:test_registerOperator_invalidPubkey_reverts() (gas: 447106) -OmniAVS_registration_Test:test_registerOperator_invalidSignature_reverts() (gas: 631600) -OmniAVS_registration_Test:test_registerOperator_registerTwice_succeeds() (gas: 898975) -OmniAVS_registration_Test:test_registerOperator_spentSalt_reverts() (gas: 566947) -OmniAVS_registration_Test:test_registerOperator_succeeds() (gas: 723501) -OmniAVS_syncWithOmni_Test:test_depositBeaconEth_succeeds() (gas: 819127) -OmniAVS_syncWithOmni_Test:test_unsupportedStrategyDeposit_succeeds() (gas: 1631277) +OmniAVS_admin_Test:test_setStrategyParams_duplicateStrategy_reverts() (gas: 38149) +OmniAVS_admin_Test:test_setStrategyParams_notOwner_reverts() (gas: 18373) +OmniAVS_admin_Test:test_setStrategyParams_succeds() (gas: 74917) +OmniAVS_admin_Test:test_setStrategyParams_twice_succeeds() (gas: 56091) +OmniAVS_admin_Test:test_setStrategyParams_zeroAddress_reverts() (gas: 36870) +OmniAVS_admin_Test:test_setStrategyParams_zeroMul_reverts() (gas: 37138) +OmniAVS_admin_Test:test_setXCallGasLimits_notOwner_reverts() (gas: 18090) +OmniAVS_admin_Test:test_setXCallGasLimits_succeeds() (gas: 33393) +OmniAVS_admin_Test:test_syncWithOmni_whenPaused_reverts() (gas: 45264) +OmniAVS_admin_Test:test_unpause_byOwner_succeeds() (gas: 35667) +OmniAVS_allowlist_Test:test_addToAllowlist_notOwner_reverts() (gas: 108019) +OmniAVS_allowlist_Test:test_addToAllowlist_succeeds() (gas: 135395) +OmniAVS_allowlist_Test:test_disableAllowlist_alreadyDisabled_reverts() (gas: 31776) +OmniAVS_allowlist_Test:test_disableAllowlist_notOwner_reverts() (gas: 17845) +OmniAVS_allowlist_Test:test_disableAllowlist_succeeds() (gas: 29000) +OmniAVS_allowlist_Test:test_enableAllowlist_alreadyEnabled_reverts() (gas: 25676) +OmniAVS_allowlist_Test:test_enableAllowlist_notOwner_reverts() (gas: 17778) +OmniAVS_allowlist_Test:test_enableAllowlist_succeeds() (gas: 29432) +OmniAVS_allowlist_Test:test_registerOperator_allowlistDisabled_succeeds() (gas: 516871) +OmniAVS_allowlist_Test:test_registerOperator_invalidPubkey_reverts() (gas: 312409) +OmniAVS_allowlist_Test:test_registerOperator_nowAllowed_reverts() (gas: 117050) +OmniAVS_allowlist_Test:test_registerOperator_succeeds() (gas: 535223) +OmniAVS_allowlist_Test:test_registerOperator_succeeds() (gas: 554055) +OmniAVS_allowlist_Test:test_registerOperator_wrongPubkey_reverts() (gas: 402056) +OmniAVS_allowlist_Test:test_removeFromAllowlist_notOwner_reverts() (gas: 107989) +OmniAVS_allowlist_Test:test_removeFromAllowlist_succeeds() (gas: 240700) +OmniAVS_canRegister_Test:test_canRegister_allowed() (gas: 328138) +OmniAVS_canRegister_Test:test_canRegister_allowlistDisabled() (gas: 308111) +OmniAVS_canRegister_Test:test_canRegister_alreadyRegistered() (gas: 519760) +OmniAVS_canRegister_Test:test_canRegister_maxOperatorsReached() (gas: 262015) +OmniAVS_canRegister_Test:test_canRegister_minStakeNotMet() (gas: 328620) +OmniAVS_canRegister_Test:test_canRegister_notAllowed() (gas: 222863) +OmniAVS_canRegister_Test:test_canRegister_notOperator() (gas: 119994) +OmniAVS_initialize_Test:test_initialize_defaultParams_succeeds() (gas: 3588147) +OmniAVS_syncWithOmni_Test:test_depositBeaconEth_succeeds() (gas: 630918) +OmniAVS_syncWithOmni_Test:test_unsupportedStrategyDeposit_succeeds() (gas: 1532940) OmniPortal_admin_Test:test_pause() (gas: 64853901) OmniPortal_admin_Test:test_setFeeOracle() (gas: 34843) OmniPortal_adversarial:test_xcallToPortal_adminFunc_fails() (gas: 82603) diff --git a/contracts/src/interfaces/IOmniAVS.sol b/contracts/src/interfaces/IOmniAVS.sol index 98f50f51e..00eea5f5a 100644 --- a/contracts/src/interfaces/IOmniAVS.sol +++ b/contracts/src/interfaces/IOmniAVS.sol @@ -13,9 +13,8 @@ interface IOmniAVS { /** * @notice Emitted when an operator is added to the OmniAVS. * @param operator The address of the operator - * @param valPubKey The operator's 64 byte uncompressed secp256k1 validator public key */ - event OperatorAdded(address indexed operator, bytes valPubKey); + event OperatorAdded(address indexed operator); /** * @notice Emitted when an operator is removed from the OmniAVS. @@ -25,14 +24,14 @@ interface IOmniAVS { /** * @notice Struct representing an OmniAVS operator - * @custom:field operator The operator's ethereum address - * @custom:field validatorPubKey The operator's 64 byte uncompressed secp256k1 validator public key - * @custom:field delegated The total amount delegated, not including operator stake - * @custom:field staked The total amount staked by the operator, not including delegations + * @custom:field addr The operator's ethereum address + * @custom:field pubkey The operator's 64 byte uncompressed secp256k1 public key + * @custom:field delegated The total amount delegated, not including operator stake + * @custom:field staked The total amount staked by the operator, not including delegations */ struct Operator { - address operator; - bytes validatorPubKey; + address addr; + bytes pubkey; uint96 delegated; uint96 staked; } @@ -69,30 +68,15 @@ interface IOmniAVS { /** * @notice Register an operator with the AVS. Forwards call to EigenLayer' AVSDirectory. - * @param validatorPubKey The operator's 64 byte uncompressed secp256k1 validator public key. - * @param validatorSignature The operator's validator pubkey registration signature, with salt and expiry. - * Signature must match `validatorPubKey` - * @param operatorSignature The operator's AVS registration signature, with salt and expiry. - * Signed must match `msg.sender` + * @param pubkey 64 byte uncompressed secp256k1 public key (no 0x04 prefix) + * Pubkey must match operator's address (msg.sender) + * @param operatorSignature The signature, salt, and expiry of the operator's signature. */ function registerOperator( - bytes calldata validatorPubKey, - ISignatureUtils.SignatureWithSaltAndExpiry calldata validatorSignature, + bytes calldata pubkey, ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature ) external; - /** - * @notice Returns the digest hash to be signed by an opeator's validator key on registration. - * @param operator The operator's ethereum address - * @param valPubKey The operator's 64 byte uncompressed secp256k1 validator public key - * @param salt A salt unique to this registration - * @param expiry The timestamp at which this registration expires - */ - function validatorRegistrationDigestHash(address operator, bytes calldata valPubKey, bytes32 salt, uint256 expiry) - external - view - returns (bytes32); - /** * @notice Check if an operator can register to the AVS. * Returns true, with no reason, if the operator can register to the AVS. diff --git a/contracts/src/protocol/OmniAVS.sol b/contracts/src/protocol/OmniAVS.sol index 2c8e0f38f..975ddede4 100644 --- a/contracts/src/protocol/OmniAVS.sol +++ b/contracts/src/protocol/OmniAVS.sol @@ -3,7 +3,6 @@ pragma solidity =0.8.12; import { OwnableUpgradeable } from "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol"; import { PausableUpgradeable } from "@openzeppelin-upgrades/contracts/security/PausableUpgradeable.sol"; -import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import { IAVSDirectory } from "eigenlayer-contracts/src/contracts/interfaces/IAVSDirectory.sol"; import { IStrategy } from "eigenlayer-contracts/src/contracts/interfaces/IStrategy.sol"; @@ -24,31 +23,13 @@ import { OmniAVSStorage } from "./OmniAVSStorage.sol"; * EigenLayer opators, and for syncing operator delegations with the Omni chain. */ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgradeable, OmniAVSStorage { - /** - * @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 validator registration struct - */ - bytes32 public constant VALIDATOR_REGISTRATION_TYPEHASH = - keccak256("ValidatorRegistration(address operator,bytes validatorPubKey,bytes32 salt,uint256 expiry)"); - - /** - * @notice Constant used as a divisor in calculating weights - */ + /// @notice Constant used as a divisor in calculating weights uint256 internal constant STRATEGY_WEIGHTING_DIVISOR = 1e18; - /** - * @notice EigenLayer core DelegationManager - */ + /// @notice EigenLayer core DelegationManager IDelegationManager internal immutable _delegationManager; - /** - * @notice EigenLayer core AVSDirectory - */ + /// @notice EigenLayer core AVSDirectory IAVSDirectory internal immutable _avsDirectory; constructor(IDelegationManager delegationManager_, IAVSDirectory avsDirectory_) { @@ -107,43 +88,26 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade /** * @notice Register an operator with the AVS. Forwards call to EigenLayer' AVSDirectory. - * @param valPubKey The operator's 64 byte uncompressed secp256k1 validator public key. - * @param valSig The operator's validator pubkey registration signature, with salt and expiry. - * Signature must match `validatorPubKey`. This signature is verified by then OmniAVS. - * @param operatorSig The operator's AVS registration signature, with salt and expiry. - * Signed must match `msg.sender`. This signature is verified by the AVSDirectory. + * @param pubkey 64 byte uncompressed secp256k1 public key (no 0x04 prefix) + * Pubkey must match operator's address (msg.sender) + * @param operatorSignature The signature, salt, and expiry of the operator's signature. */ function registerOperator( - bytes calldata valPubKey, - ISignatureUtils.SignatureWithSaltAndExpiry calldata valSig, - ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSig + bytes calldata pubkey, + ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature ) external whenNotPaused { address operator = msg.sender; - // verify the operator can register + require(operator == Secp256k1.pubkeyToAddress(pubkey), "OmniAVS: pubkey != sender"); require(!allowlistEnabled || _allowlist[operator], "OmniAVS: not allowed"); require(!_isOperator(operator), "OmniAVS: already an operator"); require(_operators.length < maxOperatorCount, "OmniAVS: max operators reached"); require(_getTotalDelegations(operator) >= minOperatorStake, "OmniAVS: min stake not met"); - // verify the the validator signature is not expired or spent - require(valSig.expiry >= block.timestamp, "OmniAVS: signature expired"); - require(!_isValActive[valPubKey], "OmniAVS: pubkey already active"); - require(!_isValSaltSpent[valPubKey][valSig.salt], "OmniAVS: spent salt"); - - // verify the the validator signature is a valid digest hash signature for the provided pubkey - require( - ECDSA.recover( - validatorRegistrationDigestHash(operator, valPubKey, valSig.salt, valSig.expiry), valSig.signature - ) == Secp256k1.pubkeyToAddress(valPubKey), - "OmniAVS: invalid val signature" - ); - - _isValSaltSpent[valPubKey][valSig.salt] = true; - _addOperator(operator, valPubKey); - _avsDirectory.registerOperatorToAVS(operator, operatorSig); + _addOperator(operator, pubkey); + _avsDirectory.registerOperatorToAVS(operator, operatorSignature); - emit OperatorAdded(operator, valPubKey); + emit OperatorAdded(operator); } /** @@ -163,29 +127,6 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade return address(_avsDirectory); } - /** - * @notice Returns the digest hash to be signed by an opeator's validator key on registration. - * @param operator The operator's ethereum address - * @param valPubKey The operator's 64 byte uncompressed secp256k1 validator public key - * @param salt A salt unique to this registration - * @param expiry The timestamp at which this registration expires - */ - function validatorRegistrationDigestHash(address operator, bytes calldata valPubKey, bytes32 salt, uint256 expiry) - public - view - returns (bytes32) - { - bytes32 structHash = keccak256(abi.encode(VALIDATOR_REGISTRATION_TYPEHASH, operator, valPubKey, salt, expiry)); - return keccak256(abi.encodePacked("\x19\x01", domainSeparator(), structHash)); - } - - /** - * @notice Domain separator for EIP-712 signatures. - */ - function domainSeparator() public view returns (bytes32) { - return keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes("OmniAVS")), block.chainid, address(this))); - } - ////////////////////////////////////////////////////////////////////////////// // Omni Sync // ////////////////////////////////////////////////////////////////////////////// @@ -219,10 +160,10 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade /** * @notice Returns the currrent list of operator registered as OmniAVS. - * Operator.operator = The operator's ethereum address - * Operator.validatorPubKey = The operator's 64 byte uncompressed validator secp256k1 public key - * Operator.staked = The total amount staked by the operator, not including delegations - * Operator.delegated = The total amount delegated, not including operator stake + * Operator.addr = The operator's ethereum address + * Operator.pubkey = The operator's 64 byte uncompressed secp256k1 public key + * Operator.staked = The total amount staked by the operator, not including delegations + * Operator.delegated = The total amount delegated, not including operator stake */ function operators() external view returns (Operator[] memory) { return _getOperators(); @@ -423,13 +364,12 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade } /** - * @notice Add an operator to internal AVS state + * @notice Add an operator to internal AVS state (_operators, _operatorPubkeys) * @dev Does not check if operator already exists */ - function _addOperator(address operator, bytes calldata valPubKey) private { + function _addOperator(address operator, bytes calldata pubkey) private { _operators.push(operator); - validatorPubKey[operator] = valPubKey; - _isValActive[valPubKey] = true; + _operatorPubkeys[operator] = pubkey; } /** @@ -447,9 +387,7 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade i++; } } - - _isValActive[validatorPubKey[operator]] = false; - delete validatorPubKey[operator]; + delete _operatorPubkeys[operator]; } /** @@ -571,7 +509,7 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade * @notice Returns true if the operator is in the list of operators */ function _isOperator(address operator) private view returns (bool) { - return validatorPubKey[operator].length > 0; + return _operatorPubkeys[operator].length > 0; } /** @@ -588,9 +526,9 @@ contract OmniAVS is IOmniAVS, IOmniAVSAdmin, OwnableUpgradeable, PausableUpgrade // this should never happen, but just in case uint96 delegated = total > staked ? total - staked : 0; - bytes memory valPubKey = validatorPubKey[operator]; + bytes memory pubkey = _operatorPubkeys[operator]; - ops[i] = Operator(operator, valPubKey, delegated, staked); + ops[i] = Operator(operator, pubkey, delegated, staked); unchecked { i++; } diff --git a/contracts/src/protocol/OmniAVSStorage.sol b/contracts/src/protocol/OmniAVSStorage.sol index 9e0f2508a..6822856c3 100644 --- a/contracts/src/protocol/OmniAVSStorage.sol +++ b/contracts/src/protocol/OmniAVSStorage.sol @@ -11,8 +11,8 @@ abstract contract OmniAVSStorage { /// @notice Ethereum addresses of currently register operators address[] internal _operators; - /// @notice Map operator address to their validator's secp256k1 public key - mapping(address => bytes) public validatorPubKey; + /// @notice Map operator address to secp256k1 public key + mapping(address => bytes) internal _operatorPubkeys; /// @notice Set of operators that are allowed to register mapping(address => bool) internal _allowlist; @@ -40,10 +40,4 @@ abstract contract OmniAVSStorage { /// @notice Omni portal contract, used to make xcalls to Omni IOmniPortal public omni; - - /// @notice Tracks salts spent per validator pub key - mapping(bytes => mapping(bytes32 => bool)) internal _isValSaltSpent; - - // @notice Maps validator pubkey to true, if the key is in use by some operator - mapping(bytes => bool) internal _isValActive; } diff --git a/contracts/test/avs/OmniAVS_admin.t.sol b/contracts/test/avs/OmniAVS_admin.t.sol index 96a6fc081..e0f223600 100644 --- a/contracts/test/avs/OmniAVS_admin.t.sol +++ b/contracts/test/avs/OmniAVS_admin.t.sol @@ -26,7 +26,7 @@ contract OmniAVS_admin_Test is Base { // assert operator is registered IOmniAVS.Operator[] memory operators = omniAVS.operators(); assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); + assertEq(operators[0].addr, operator); // deregister operator vm.prank(omniAVSOwner); diff --git a/contracts/test/avs/OmniAVS_allowlist.t.sol b/contracts/test/avs/OmniAVS_allowlist.t.sol index 4fedc6615..aeddbaad7 100644 --- a/contracts/test/avs/OmniAVS_allowlist.t.sol +++ b/contracts/test/avs/OmniAVS_allowlist.t.sol @@ -60,7 +60,7 @@ contract OmniAVS_allowlist_Test is Base { IOmniAVS.Operator[] memory operators = omniAVS.operators(); assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); + assertEq(operators[0].addr, operator); } /// @dev Test that an operator can't register if not in allowlist @@ -72,7 +72,7 @@ contract OmniAVS_allowlist_Test is Base { vm.expectRevert("OmniAVS: not allowed"); vm.prank(operator); - omniAVS.registerOperator(_valPubKey(operator), emptySig, emptySig); + omniAVS.registerOperator(_pubkey(operator), emptySig); } /// @dev Test that the owner can disable the allowlist @@ -138,6 +138,6 @@ contract OmniAVS_allowlist_Test is Base { IOmniAVS.Operator[] memory operators = omniAVS.operators(); assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); + assertEq(operators[0].addr, operator); } } diff --git a/contracts/test/avs/OmniAVS_pause.t.sol b/contracts/test/avs/OmniAVS_pause.t.sol index 9d01ca715..ce808faa2 100644 --- a/contracts/test/avs/OmniAVS_pause.t.sol +++ b/contracts/test/avs/OmniAVS_pause.t.sol @@ -37,7 +37,7 @@ contract OmniAVS_admin_Test is Base { vm.expectRevert("Pausable: paused"); vm.prank(operator); - omniAVS.registerOperator(_valPubKey(operator), emptySig, emptySig); + omniAVS.registerOperator(_pubkey(operator), emptySig); } /// @dev Test that when paused, you cannot syncWithOmni diff --git a/contracts/test/avs/OmniAVS_registration.t.sol b/contracts/test/avs/OmniAVS_registration.t.sol index bb9462452..6104cc986 100644 --- a/contracts/test/avs/OmniAVS_registration.t.sol +++ b/contracts/test/avs/OmniAVS_registration.t.sol @@ -9,7 +9,7 @@ import { Base } from "./common/Base.sol"; * @title OmniAVS_registration_Test * @dev Test suite for the AVS registration functionality */ -contract OmniAVS_registration_Test is Base { +contract OmniAVS_allowlist_Test is Base { /// @dev Test that an operator can register function test_registerOperator_succeeds() public { address operator = _operator(0); @@ -22,178 +22,12 @@ contract OmniAVS_registration_Test is Base { IOmniAVS.Operator[] memory operators = omniAVS.operators(); assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); - assertEq(operators[0].validatorPubKey, _valPubKey(operator)); + assertEq(operators[0].addr, operator); + assertEq(operators[0].pubkey, _pubkey(operator)); } - /// @dev Test that an operator can register, be ejected, than register again - function test_registerOperator_registerTwice_succeeds() public { - // register once - address operator = _operator(0); - _addToAllowlist(operator); - _registerAsOperator(operator); - _depositIntoSupportedStrategy(operator, minOperatorStake); - - bytes memory valPubKey = _valPubKey(operator); - bytes32 salt1 = keccak256("test"); - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = _validatorSig(operator, salt1); - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator, salt1); - - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - - // assert operator is registerd - IOmniAVS.Operator[] memory operators = omniAVS.operators(); - assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); - assertEq(operators[0].validatorPubKey, _valPubKey(operator)); - assertEq(omniAVS.validatorPubKey(operator), valPubKey); - - // eject operator - vm.prank(omniAVSOwner); - omniAVS.ejectOperator(operator); - - // try registering again with same salt - vm.expectRevert("OmniAVS: spent salt"); - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - - // register again, requires new salt - bytes32 salt2 = keccak256("test2"); - valsig = _validatorSig(operator, salt2); - opsig = _operatorSig(operator, salt2); - - // re-register - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - - // assert operator is registerd - operators = omniAVS.operators(); - assertEq(operators.length, 1); - assertEq(operators[0].operator, operator); - assertEq(operators[0].validatorPubKey, _valPubKey(operator)); - assertEq(omniAVS.validatorPubKey(operator), valPubKey); - } - - // @dev Test that another operator cannot register an active validator key - function test_registerOperator_activeValKey_reverts() public { - // register operator - address operator0 = _operator(0); - _addToAllowlist(operator0); - _registerAsOperator(operator0); - _depositIntoSupportedStrategy(operator0, minOperatorStake); - _registerOperatorWithAVS(operator0); - - // attempt to register a new operator with same pubkey - address operator1 = _operator(1); - _addToAllowlist(operator1); - _registerAsOperator(operator1); - _depositIntoSupportedStrategy(operator1, minOperatorStake); - - // uses operator0 validator key - bytes memory valPubKey = _valPubKey(operator0); - - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = ISignatureUtils.SignatureWithSaltAndExpiry({ - signature: new bytes(0), - salt: keccak256("test"), - expiry: block.timestamp + 1 days - }); - - bytes32 validatorRegistrationDigestHash = omniAVS.validatorRegistrationDigestHash({ - operator: operator1, // but use operator1 in registration digest - valPubKey: valPubKey, - salt: valsig.salt, - expiry: valsig.expiry - }); - - valsig.signature = _sign(_valPrivKey(operator0), validatorRegistrationDigestHash); - - // use operator1 for AVSDirectory operator signature - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator1); - - // try registering again with same salt - vm.expectRevert("OmniAVS: pubkey already active"); - vm.prank(operator1); - omniAVS.registerOperator(valPubKey, valsig, opsig); - } - - /// @dev Test that a validator signature salt cannot be double spent - function test_registerOperator_spentSalt_reverts() public { - // register opeator - address operator = _operator(0); - _addToAllowlist(operator); - _registerAsOperator(operator); - _depositIntoSupportedStrategy(operator, minOperatorStake); - - bytes memory valPubKey = _valPubKey(operator); - bytes32 salt = keccak256("test"); - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = _validatorSig(operator, salt); - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator); - - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - assertEq(omniAVS.validatorPubKey(operator), valPubKey); - - // deregister operator - vm.prank(omniAVSOwner); - omniAVS.ejectOperator(operator); - - // try registering again with same salt - vm.expectRevert("OmniAVS: spent salt"); - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - } - - /// @dev Test that using a validator signature after it's expiry reverts - function test_registerOperator_expiredSig_reverts() public { - address operator = _operator(0); - _addToAllowlist(operator); - _registerAsOperator(operator); - _depositIntoSupportedStrategy(operator, minOperatorStake); - - bytes memory valPubKey = _valPubKey(operator); - bytes32 salt = keccak256("test"); - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = _validatorSig(operator, salt); - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator); - - vm.warp(valsig.expiry + 1 days); - vm.expectRevert("OmniAVS: signature expired"); - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - } - - function test_registerOperator_invalidDigestHash_reverts() public { - address operator = _operator(0); - _addToAllowlist(operator); - _registerAsOperator(operator); - _depositIntoSupportedStrategy(operator, minOperatorStake); - - bytes memory valPubKey = _valPubKey(operator); - - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = ISignatureUtils.SignatureWithSaltAndExpiry({ - signature: new bytes(0), - salt: keccak256("test"), - expiry: block.timestamp + 1 days - }); - - bytes32 validatorRegistrationDigestHash = valRegDigestHash_wrongDomainSeperator({ - operator: operator, - valPubKey: valPubKey, - salt: valsig.salt, - expiry: valsig.expiry - }); - - valsig.signature = _sign(_valPrivKey(operator), validatorRegistrationDigestHash); - - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator); - - vm.expectRevert("OmniAVS: invalid val signature"); - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - } - - /// @dev That that an operator cannot register with a pubkey that did not sign the registration digest - function test_registerOperator_invalidSignature_reverts() public { + /// @dev That that an operator cannot register with the wrong pubkey + function test_registerOperator_wrongPubkey_reverts() public { address operator1 = _operator(0); address operator2 = _operator(1); @@ -201,14 +35,11 @@ contract OmniAVS_registration_Test is Base { _registerAsOperator(operator1); _depositIntoSupportedStrategy(operator1, minOperatorStake); - ISignatureUtils.SignatureWithSaltAndExpiry memory val1Sig = _validatorSig(operator1); - ISignatureUtils.SignatureWithSaltAndExpiry memory op1Sig = _operatorSig(operator1); + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; - vm.expectRevert("OmniAVS: invalid val signature"); + vm.expectRevert("OmniAVS: pubkey != sender"); vm.prank(operator1); - - // use operator2 pubkey - omniAVS.registerOperator(_valPubKey(operator2), val1Sig, op1Sig); + omniAVS.registerOperator(_pubkey(operator2), emptySig); } /// @dev Test that an operator cannot register with an invalid pubkey @@ -219,41 +50,11 @@ contract OmniAVS_registration_Test is Base { _registerAsOperator(operator); _depositIntoSupportedStrategy(operator, minOperatorStake); - bytes memory valPubKey = bytes.concat(hex"04", _valPubKey(operator)); - - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = _validatorSig(operator); - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator); + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + bytes memory pubkey = bytes.concat(hex"04", _pubkey(operator)); vm.expectRevert("Secp256k1: invalid pubkey length"); vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - } - - /** - * Utils & constants copied from OmniAVS. - */ - - /// @dev OmniAVS.DOMAIN_TYPEHASH - bytes32 public constant DOMAIN_TYPEHASH = - keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); - - /// @dev OmniAVS.VALIDATOR_REGISTRATION_TYPEHASH - bytes32 public constant VALIDATOR_REGISTRATION_TYPEHASH = - keccak256("ValidatorRegistration(address operator,bytes validatorPubKey,bytes32 salt,uint256 expiry)"); - - /// @dev Same as OmniAVS.validatorRegistrationDigestHash(...), but uses a different domainSeparator() - function valRegDigestHash_wrongDomainSeperator( - address operator, - bytes memory valPubKey, - bytes32 salt, - uint256 expiry - ) public view returns (bytes32) { - bytes32 structHash = keccak256(abi.encode(VALIDATOR_REGISTRATION_TYPEHASH, operator, valPubKey, salt, expiry)); - return keccak256(abi.encodePacked("\x19\x01", domainSeparator(), structHash)); - } - - // @dev This domain separator will be different than OmniAVS.domainSeparator(), because it includes address - function domainSeparator() public view returns (bytes32) { - return keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes("OmniAVS")), block.chainid, address(this))); + omniAVS.registerOperator(pubkey, emptySig); } } diff --git a/contracts/test/avs/OmniAVS_serviceManager.t.sol b/contracts/test/avs/OmniAVS_serviceManager.t.sol index a151681a1..aa13ab789 100644 --- a/contracts/test/avs/OmniAVS_serviceManager.t.sol +++ b/contracts/test/avs/OmniAVS_serviceManager.t.sol @@ -95,7 +95,7 @@ contract OmniAVS_admin_Test is Base { } /// @dev Test that getOperatorRestakedStrategies() returns an empty list when the operator is not registered - function test_getOperatorRestakedStrategies_notOperator_succeeds() public { + function test_getOPeratorRestakedStrategies_notOperator_succeeds() public { address operator = _operator(0); address[] memory operatorRestakedStrategies = omniAVS.getOperatorRestakedStrategies(operator); diff --git a/contracts/test/avs/OmniAVS_syncWithOmni.t.sol b/contracts/test/avs/OmniAVS_syncWithOmni.t.sol index f4afce294..f6e7e1cbe 100644 --- a/contracts/test/avs/OmniAVS_syncWithOmni.t.sol +++ b/contracts/test/avs/OmniAVS_syncWithOmni.t.sol @@ -314,7 +314,7 @@ contract OmniAVS_syncWithOmni_Test is Base { // assert that none of the operators left is the operator that was just deregistered for (uint32 j = 0; j < numOperatorsLeft; j++) { - assertNotEq(ops[j].operator, operator, "_testEjectOperators: operator should not be in validators list"); + assertNotEq(ops[j].addr, operator, "_testEjectOperators: operator should not be in validators list"); } } } @@ -356,8 +356,7 @@ contract OmniAVS_syncWithOmni_Test is Base { IOmniAVS.Operator[] memory ops = omniAVS.operators(); assertEq(ops.length, 1); - assertEq(ops[0].operator, operator); - assertEq(ops[0].validatorPubKey, _valPubKey(operator)); + assertEq(ops[0].addr, operator); assertEq(ops[0].staked, amount); assertEq(ops[0].delegated, 0); @@ -390,8 +389,7 @@ contract OmniAVS_syncWithOmni_Test is Base { // assert unsupported strategy deposit does not affect operator stake assertEq(ops.length, 1); - assertEq(ops[0].operator, operator); - assertEq(ops[0].validatorPubKey, _valPubKey(operator)); + assertEq(ops[0].addr, operator); assertEq(ops[0].staked, stakeAmt); assertEq(ops[0].delegated, delegateAmt); } diff --git a/contracts/test/avs/common/Utils.sol b/contracts/test/avs/common/Utils.sol index c81eecdcc..05b3b9d67 100644 --- a/contracts/test/avs/common/Utils.sol +++ b/contracts/test/avs/common/Utils.sol @@ -18,11 +18,8 @@ import { Vm } from "forge-std/Vm.sol"; * @dev Common utilities for AVS tests */ contract Utils is Fixtures { - // map operator addr to operator key wallet - mapping(address => Vm.Wallet) _operatorWallets; - - // map operator addr to validator key wallet - mapping(address => Vm.Wallet) _validatorWallets; + // map addr to private key + mapping(address => Vm.Wallet) _wallets; /// @dev register an operator with eigenlayer core function _registerAsOperator(address operator) internal { @@ -37,70 +34,10 @@ contract Utils is Fixtures { /// @dev register an operator with OmniAVS function _registerOperatorWithAVS(address operator) internal { - bytes memory valPubKey = _valPubKey(operator); - - ISignatureUtils.SignatureWithSaltAndExpiry memory valsig = _validatorSig(operator); - ISignatureUtils.SignatureWithSaltAndExpiry memory opsig = _operatorSig(operator); - - vm.prank(operator); - omniAVS.registerOperator(valPubKey, valsig, opsig); - - assertEq(omniAVS.validatorPubKey(operator), valPubKey, "_registerOperatorWithAVS: validatorPubKey not set"); - } - - /// @dev return's the operator's validator registration signature, required by OmniAVS - /// uses static salt - function _validatorSig(address operator) - internal - view - returns (ISignatureUtils.SignatureWithSaltAndExpiry memory) - { - bytes32 salt = keccak256(abi.encodePacked(operator)); - return _validatorSig(operator, salt); - } - - /// @dev return's the operator's validator registration signature, required by OmniAVS - /// uses given salt - function _validatorSig(address operator, bytes32 salt) - internal - view - returns (ISignatureUtils.SignatureWithSaltAndExpiry memory) - { - ISignatureUtils.SignatureWithSaltAndExpiry memory sig = ISignatureUtils.SignatureWithSaltAndExpiry({ - signature: new bytes(0), - salt: salt, - expiry: block.timestamp + 1 days - }); - - bytes32 validatorRegistrationDigestHash = omniAVS.validatorRegistrationDigestHash({ - operator: operator, - valPubKey: _valPubKey(operator), - salt: sig.salt, - expiry: sig.expiry - }); - - sig.signature = _sign(_valPrivKey(operator), validatorRegistrationDigestHash); - - return sig; - } - - /// @dev return's the operators registration signature, required by AVSDirectory - /// uses static salt. - function _operatorSig(address operator) internal view returns (ISignatureUtils.SignatureWithSaltAndExpiry memory) { - bytes32 salt = keccak256(abi.encodePacked(operator)); - return _operatorSig(operator, salt); - } - - /// @dev return's the operators registration signature, required by AVSDirectory - /// uses given salt - function _operatorSig(address operator, bytes32 salt) - internal - view - returns (ISignatureUtils.SignatureWithSaltAndExpiry memory) - { + // don't matter ISignatureUtils.SignatureWithSaltAndExpiry memory sig = ISignatureUtils.SignatureWithSaltAndExpiry({ signature: new bytes(0), - salt: salt, + salt: keccak256(abi.encodePacked(operator)), expiry: block.timestamp + 1 days }); @@ -111,9 +48,10 @@ contract Utils is Fixtures { expiry: sig.expiry }); - sig.signature = _sign(_operatorPrivKey(operator), operatorRegistrationDigestHash); + sig.signature = _sign(operator, operatorRegistrationDigestHash); - return sig; + vm.prank(operator); + omniAVS.registerOperator(_pubkey(operator), sig); } /// @dev add an operator to the allowlist @@ -148,46 +86,30 @@ contract Utils is Fixtures { /// @dev create an operator address function _operator(uint256 index) internal returns (address) { - uint256 pk = uint256(keccak256(abi.encode("operator", index))); - address addr = vm.addr(pk); - - // create operator wallet if it doesn't exist - if (_operatorWallets[addr].privateKey == 0) { - _operatorWallets[addr] = vm.createWallet(pk); - } - - // create a validator walelt if one does not exist (using separate private key) - if (_validatorWallets[addr].privateKey == 0) { - _validatorWallets[addr] = vm.createWallet(uint256(keccak256(abi.encode("val", addr)))); - } - - return addr; + return _addr("operator", index); } /// @dev create a delegator address - function _delegator(uint256 index) internal pure returns (address) { - return address(uint160(uint256(keccak256(abi.encode("delegator", index))))); + function _delegator(uint256 index) internal returns (address) { + return _addr("delegator", index); } - /// @dev return the operator's validator public key, bytes encoded - function _valPubKey(address operator) internal view returns (bytes memory) { - Vm.Wallet storage w = _validatorWallets[operator]; - return abi.encodePacked(w.publicKeyX, w.publicKeyY); + /// @dev create a namespaced address, save the addresses wallet + function _addr(string memory namespace, uint256 index) internal returns (address) { + Vm.Wallet memory w = vm.createWallet(uint256(keccak256(abi.encode(namespace, index)))); + _wallets[w.addr] = w; + return w.addr; } - /// @dev return the operator's validator private key - function _valPrivKey(address operator) internal view returns (uint256) { - return _validatorWallets[operator].privateKey; - } - - /// @dev return the operator's private key - function _operatorPrivKey(address operator) internal view returns (uint256) { - return _operatorWallets[operator].privateKey; + /// @dev return the public key of an operator + function _pubkey(address operator) internal view returns (bytes memory) { + Vm.Wallet memory w = _wallets[operator]; + return abi.encodePacked(w.publicKeyX, w.publicKeyY); } /// @dev sign a digest - function _sign(uint256 privateKey, bytes32 digest) internal pure returns (bytes memory) { - (uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, digest); + function _sign(address signer, bytes32 digest) internal view returns (bytes memory) { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(_wallets[signer].privateKey, digest); return abi.encodePacked(r, s, v); }