From 3cf210518a82a4d365b6911f4c75831ce2222e2f Mon Sep 17 00:00:00 2001 From: gpsanant Date: Thu, 3 Aug 2023 17:40:46 -0700 Subject: [PATCH 1/6] added signature capability, exisiting tests work --- .../IBLSRegistryCoordinatorWithIndices.sol | 3 +- .../interfaces/IDelegationManager.sol | 13 +--- src/contracts/interfaces/ISignatureUtils.sol | 17 ++++ .../BLSRegistryCoordinatorWithIndices.sol | 77 ++++++++++++++++++- ...LSRegistryCoordinatorWithIndicesUnit.t.sol | 23 ++++-- src/test/unit/DelegationUnit.t.sol | 56 +++++++------- src/test/utils/MockAVSDeployer.sol | 19 +++++ 7 files changed, 158 insertions(+), 50 deletions(-) create mode 100644 src/contracts/interfaces/ISignatureUtils.sol diff --git a/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol b/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol index 7d6e64cbd..a29571bf6 100644 --- a/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity =0.8.12; +import "./ISignatureUtils.sol"; import "./IRegistryCoordinator.sol"; import "./IStakeRegistry.sol"; import "./IBLSPubkeyRegistry.sol"; @@ -10,7 +11,7 @@ import "./IIndexRegistry.sol"; * @title Minimal interface for the `IBLSStakeRegistryCoordinator` contract. * @author Layr Labs, Inc. */ -interface IBLSRegistryCoordinatorWithIndices is IRegistryCoordinator { +interface IBLSRegistryCoordinatorWithIndices is ISignatureUtils, IRegistryCoordinator { // STRUCT /** diff --git a/src/contracts/interfaces/IDelegationManager.sol b/src/contracts/interfaces/IDelegationManager.sol index 6802a9e34..cb60aaac4 100644 --- a/src/contracts/interfaces/IDelegationManager.sol +++ b/src/contracts/interfaces/IDelegationManager.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity >=0.5.0; +import "./ISignatureUtils.sol"; import "./IStrategy.sol"; /** @@ -13,7 +14,7 @@ import "./IStrategy.sol"; * - enabling any staker to delegate its stake to the operator of its choice (a given staker can only delegate to a single operator at a time) * - enabling a staker to undelegate its assets from the operator it is delegated to (performed as part of the withdrawal process, initiated through the StrategyManager) */ -interface IDelegationManager { +interface IDelegationManager is ISignatureUtils { // @notice Struct used for storing information about a single operator who has registered with EigenLayer struct OperatorDetails { // @notice address to receive the rewards that the operator earns via serving applications built on EigenLayer. @@ -66,15 +67,7 @@ interface IDelegationManager { // the expiration timestamp (UTC) of the signature uint256 expiry; } - - // @notice Struct that bundles together a signature and an expiration time for the signature. Used primarily for stack management. - struct SignatureWithExpiry { - // the signature itself, formatted as a single bytes object - bytes signature; - // the expiration timestamp (UTC) of the signature - uint256 expiry; - } - + // @notice Emitted when a new operator registers in EigenLayer and provides their OperatorDetails. event OperatorRegistered(address indexed operator, OperatorDetails operatorDetails); diff --git a/src/contracts/interfaces/ISignatureUtils.sol b/src/contracts/interfaces/ISignatureUtils.sol new file mode 100644 index 000000000..4c69b5f47 --- /dev/null +++ b/src/contracts/interfaces/ISignatureUtils.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity >=0.5.0; + +/** + * @title The interface for common signature utilities. + * @author Layr Labs, Inc. + * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service + */ +interface ISignatureUtils { + // @notice Struct that bundles together a signature and an expiration time for the signature. Used primarily for stack management. + struct SignatureWithExpiry { + // the signature itself, formatted as a single bytes object + bytes signature; + // the expiration timestamp (UTC) of the signature + uint256 expiry; + } +} \ No newline at end of file diff --git a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol index 089a78b42..07e690aee 100644 --- a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity =0.8.12; +import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol"; import "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol"; import "../interfaces/IBLSRegistryCoordinatorWithIndices.sol"; @@ -11,6 +12,7 @@ import "../interfaces/IVoteWeigher.sol"; import "../interfaces/IStakeRegistry.sol"; import "../interfaces/IIndexRegistry.sol"; +import "../libraries/EIP1271SignatureUtils.sol"; import "../libraries/BitmapUtils.sol"; import "../libraries/MiddlewareUtils.sol"; @@ -24,9 +26,13 @@ import "forge-std/Test.sol"; * * @author Layr Labs, Inc. */ -contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordinatorWithIndices, ISocketUpdater { +contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistryCoordinatorWithIndices, ISocketUpdater, Test { using BN254 for BN254.G1Point; + /// @notice The EIP-712 typehash for the `DelegationApproval` struct used by the contract + bytes32 public constant OPERATOR_CHURN_APPROVAL_TYPEHASH = + keccak256("OperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] operatorKickParams)OperatorKickParam(address operator, BN254.G1Point pubkey, bytes32[] operatorIdsToSwap)BN254.G1Point(uint256 x, uint256 y)"); + uint16 internal constant BIPS_DENOMINATOR = 10000; /// @notice the EigenLayer Slasher @@ -39,6 +45,14 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin IStakeRegistry public immutable stakeRegistry; /// @notice the Index Registry contract that will keep track of operators' indexes IIndexRegistry public immutable indexRegistry; + + /** + * @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 the mapping from quorum number to the maximum number of operators that can be registered for that quorum mapping(uint8 => OperatorSetParam) internal _quorumOperatorSetParams; /// @notice the mapping from operator's operatorId to the updates of the bitmap of quorums they are registered for @@ -47,6 +61,10 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin mapping(address => Operator) internal _operators; /// @notice the dynamic-length array of the registries this coordinator is coordinating address[] public registries; + /// @notice the address of the entity allowed to sign off on operators getting kicked out of the AVS during registration + address public churner; + /// @notice the nonce of the churner used in EIP-712 signatures + uint256 public churnerNonce; modifier onlyServiceManagerOwner { require(msg.sender == serviceManager.owner(), "BLSRegistryCoordinatorWithIndices.onlyServiceManagerOwner: caller is not the service manager owner"); @@ -59,7 +77,7 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin IStakeRegistry _stakeRegistry, IBLSPubkeyRegistry _blsPubkeyRegistry, IIndexRegistry _indexRegistry - ) { + ) EIP712("AVSRegistryCoordinator", "v0.0.1") { slasher = _slasher; serviceManager = _serviceManager; stakeRegistry = _stakeRegistry; @@ -67,7 +85,9 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin indexRegistry = _indexRegistry; } - function initialize(OperatorSetParam[] memory _operatorSetParams) external initializer { + function initialize(address _churner, OperatorSetParam[] memory _operatorSetParams) external initializer { + // set the churner + churner = _churner; // the stake registry is this contract itself registries.push(address(stakeRegistry)); registries.push(address(blsPubkeyRegistry)); @@ -79,6 +99,8 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin _setOperatorSetParams(i, _operatorSetParams[i]); } } + + // VIEW FUNCTIONS /// @notice Returns the operator set params for the given `quorumNumber` function getOperatorSetParams(uint8 quorumNumber) external view returns (OperatorSetParam memory) { @@ -158,6 +180,40 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin return registries.length; } + /** + * @notice Public function for the the churner signature hash calculation when operators are being kicked from quorums + * @param registeringOperatorId The is of the registering operator + * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps + * @param expiry The desired expiry time of the churner's signature + */ + function calculateCurrentOperatorChurnApprovalDigestHash( + bytes32 registeringOperatorId, + OperatorKickParam[] memory operatorKickParams, + uint256 expiry + ) public view returns (bytes32) { + // calculate the digest hash + return calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnerNonce, expiry); + } + + /** + * @notice Public function for the the churner signature hash calculation when operators are being kicked from quorums + * @param registeringOperatorId The is of the registering operator + * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps + * @param churnerNonceToUse nonce of the churner. In practice we use the churner's current nonce, stored at `churnerNonce` + * @param expiry The desired expiry time of the churner's signature + */ + function calculateOperatorChurnApprovalDigestHash( + bytes32 registeringOperatorId, + OperatorKickParam[] memory operatorKickParams, + uint256 churnerNonceToUse, + uint256 expiry + ) public view returns (bytes32) { + // calculate the digest hash + return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, churnerNonceToUse, expiry))); + } + + // STATE CHANGING FUNCTIONS + /** * @notice Sets parameters of the operator set for the given `quorumNumber` * @param quorumNumber is the quorum number to set the maximum number of operators for @@ -203,7 +259,8 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin bytes calldata quorumNumbers, BN254.G1Point memory pubkey, string calldata socket, - OperatorKickParam[] calldata operatorKickParams + OperatorKickParam[] calldata operatorKickParams, + SignatureWithExpiry memory signatureAndExpiry ) external { // register the operator uint32[] memory numOperatorsPerQuorum = _registerOperatorWithCoordinator(msg.sender, quorumNumbers, pubkey, socket); @@ -211,6 +268,9 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin // get the registering operator's operatorId bytes32 registeringOperatorId = _operators[msg.sender].operatorId; + // verify the churner's signature + _verifyChurnerSignatureOnOperatorChurnApproval(registeringOperatorId, operatorKickParams, signatureAndExpiry); + // kick the operators for (uint256 i = 0; i < quorumNumbers.length; i++) { // check that the quorum has reached the max operator count @@ -398,4 +458,13 @@ contract BLSRegistryCoordinatorWithIndices is Initializable, IBLSRegistryCoordin _operators[operator].status = OperatorStatus.DEREGISTERED; } } + + /// @notice verifies churner's signature on operator churn approval and increments the churner nonce + function _verifyChurnerSignatureOnOperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, SignatureWithExpiry memory signatureWithExpiry) internal { + uint256 churnerNonceMem = churnerNonce; + require(signatureWithExpiry.expiry >= block.timestamp, "BLSRegistryCoordinatorWithIndices._verifyChurnerSignatureOnOperatorChurnApproval: churner signature expired"); + EIP1271SignatureUtils.checkSignature_EIP1271(churner, calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnerNonceMem, signatureWithExpiry.expiry), signatureWithExpiry.signature); + // increment the churner nonce + churnerNonce = churnerNonceMem + 1; + } } \ No newline at end of file diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 26f4f9376..8a76dffc0 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -49,7 +49,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { // make sure the contract intializers are disabled cheats.expectRevert(bytes("Initializable: contract is already initialized")); - registryCoordinator.initialize(operatorSetParams); + registryCoordinator.initialize(churner, operatorSetParams); } function testRegisterOperatorWithCoordinator_EmptyQuorumNumbers_Reverts() public { @@ -491,7 +491,6 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { uint96 registeringStake = defaultKickBIPsOfOperatorStake * defaultStake; stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); - cheats.prank(operatorToRegister); cheats.roll(registrationBlockNumber); cheats.expectEmit(true, true, true, true, address(blsPubkeyRegistry)); emit OperatorAddedToQuorums(operatorToRegister, quorumNumbers); @@ -508,8 +507,16 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1); { + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + cheats.prank(operatorToRegister); uint256 gasBefore = gasleft(); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams); + registryCoordinator.registerOperatorWithCoordinator( + quorumNumbers, + operatorToRegisterPubKey, + defaultSocket, + operatorKickParams, + signatureWithExpiry + ); uint256 gasAfter = gasleft(); emit log_named_uint("gasUsed", gasBefore - gasAfter); } @@ -587,10 +594,11 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, defaultStake); - cheats.prank(operatorToRegister); cheats.roll(registrationBlockNumber); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + cheats.prank(operatorToRegister); cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: registering operator has less than kickBIPsOfOperatorStake"); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); } function testRegisterOperatorWithCoordinatorWithKicks_LessThanKickBIPsOfTotalStake_Reverts(uint256 pseudoRandomNumber) public { @@ -645,10 +653,11 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { // set the stake of the operator to register to the defaultKickBIPsOfOperatorStake multiple of the operatorToKickStake stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, operatorToKickStake * defaultKickBIPsOfOperatorStake / 10000 + 1); - cheats.prank(operatorToRegister); cheats.roll(registrationBlockNumber); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + cheats.prank(operatorToRegister); cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: operator to kick has more than kickBIPSOfTotalStake"); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); } function testUpdateSocket() public { diff --git a/src/test/unit/DelegationUnit.t.sol b/src/test/unit/DelegationUnit.t.sol index 30a098d4b..d50731feb 100644 --- a/src/test/unit/DelegationUnit.t.sol +++ b/src/test/unit/DelegationUnit.t.sol @@ -201,7 +201,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // delegate from the `staker` to the operator cheats.startPrank(staker); - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; delegationManager.delegateTo(operator, approverSignatureAndExpiry); cheats.expectRevert(bytes("DelegationManager._delegate: staker is already actively delegated")); @@ -311,7 +311,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { * Reverts if the staker is already delegated (to the operator or to anyone else) * Reverts if the ‘operator’ is not actually registered as an operator */ - function testDelegateToOperatorWhoAcceptsAllStakers(address staker, IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry) public + function testDelegateToOperatorWhoAcceptsAllStakers(address staker, ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry) public filterFuzzedAddressInputs(staker) { // register *this contract* as an operator @@ -347,7 +347,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { /** * @notice Delegates from `staker` to an operator, then verifies that the `staker` cannot delegate to another `operator` (at least without first undelegating) */ - function testCannotDelegateWhileDelegated(address staker, address operator, IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry) public + function testCannotDelegateWhileDelegated(address staker, address operator, ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry) public filterFuzzedAddressInputs(staker) filterFuzzedAddressInputs(operator) { @@ -384,7 +384,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // try to delegate and check that the call reverts cheats.startPrank(staker); cheats.expectRevert(bytes("DelegationManager._delegate: operator is not registered in EigenLayer")); - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; delegationManager.delegateTo(operator, approverSignatureAndExpiry); cheats.stopPrank(); } @@ -421,7 +421,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // fetch the delegationApprover's current nonce uint256 currentApproverNonce = delegationManager.delegationApproverNonce(delegationManager.delegationApprover(operator)); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // delegate from the `staker` to the operator cheats.startPrank(staker); @@ -467,7 +467,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { testRegisterAsOperator(operator, operatorDetails, emptyStringForMetadataURI); // calculate the signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; approverSignatureAndExpiry.expiry = expiry; { bytes32 digestHash = delegationManager.calculateCurrentDelegationApprovalDigestHash(staker, operator, expiry); @@ -510,7 +510,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { testRegisterAsOperator(operator, operatorDetails, emptyStringForMetadataURI); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // delegate from the `staker` to the operator cheats.startPrank(staker); @@ -558,7 +558,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // fetch the delegationApprover's current nonce uint256 currentApproverNonce = delegationManager.delegationApproverNonce(delegationManager.delegationApprover(operator)); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // delegate from the `staker` to the operator cheats.startPrank(staker); @@ -607,7 +607,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { testRegisterAsOperator(operator, operatorDetails, emptyStringForMetadataURI); // create the signature struct - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; approverSignatureAndExpiry.expiry = expiry; // try to delegate from the `staker` to the operator, and check reversion @@ -654,14 +654,14 @@ contract DelegationUnitTests is EigenLayerTestHelper { // fetch the staker's current nonce uint256 currentStakerNonce = delegationManager.stakerNonce(staker); // calculate the staker signature - IDelegationManager.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); // delegate from the `staker` to the operator, via having the `caller` call `DelegationManager.delegateToBySignature` cheats.startPrank(caller); cheats.expectEmit(true, true, true, true, address(delegationManager)); emit StakerDelegated(staker, operator); // use an empty approver signature input since none is needed / the input is unchecked - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; delegationManager.delegateToBySignature(staker, operator, stakerSignatureAndExpiry, approverSignatureAndExpiry); cheats.stopPrank(); @@ -712,12 +712,12 @@ contract DelegationUnitTests is EigenLayerTestHelper { // fetch the delegationApprover's current nonce uint256 currentApproverNonce = delegationManager.delegationApproverNonce(delegationManager.delegationApprover(operator)); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // fetch the staker's current nonce uint256 currentStakerNonce = delegationManager.stakerNonce(staker); // calculate the staker signature - IDelegationManager.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); // delegate from the `staker` to the operator, via having the `caller` call `DelegationManager.delegateToBySignature` cheats.startPrank(caller); @@ -785,12 +785,12 @@ contract DelegationUnitTests is EigenLayerTestHelper { // fetch the delegationApprover's current nonce uint256 currentApproverNonce = delegationManager.delegationApproverNonce(delegationManager.delegationApprover(operator)); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // fetch the staker's current nonce uint256 currentStakerNonce = delegationManager.stakerNonce(staker); // calculate the staker signature - IDelegationManager.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, expiry); // delegate from the `staker` to the operator, via having the `caller` call `DelegationManager.delegateToBySignature` cheats.startPrank(caller); @@ -821,7 +821,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { function testDelegateBySignatureRevertsWhenStakerSignatureExpired(address staker, address operator, uint256 expiry, bytes memory signature) public{ cheats.assume(expiry < block.timestamp); cheats.expectRevert(bytes("DelegationManager.delegateToBySignature: staker signature expired")); - IDelegationManager.SignatureWithExpiry memory signatureWithExpiry = IDelegationManager.SignatureWithExpiry({ + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = ISignatureUtils.SignatureWithExpiry({ signature: signature, expiry: expiry }); @@ -855,10 +855,10 @@ contract DelegationUnitTests is EigenLayerTestHelper { testRegisterAsOperator(operator, operatorDetails, emptyStringForMetadataURI); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, delegationApproverExpiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, delegationApproverExpiry); // calculate the staker signature - IDelegationManager.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, stakerExpiry); + ISignatureUtils.SignatureWithExpiry memory stakerSignatureAndExpiry = _getStakerSignature(stakerPrivateKey, operator, stakerExpiry); // try delegate from the `staker` to the operator, via having the `caller` call `DelegationManager.delegateToBySignature`, and check for reversion cheats.startPrank(caller); @@ -893,7 +893,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { testRegisterAsOperator(operator, operatorDetails, emptyStringForMetadataURI); // calculate the delegationSigner's signature - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry = _getApproverSignature(delegationSignerPrivateKey, staker, operator, expiry); // delegate from the `staker` to the operator cheats.startPrank(staker); @@ -912,7 +912,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { */ function testUndelegateFromOperator(address staker) public { // register *this contract* as an operator and delegate from the `staker` to them (already filters out case when staker is the operator since it will revert) - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; testDelegateToOperatorWhoAcceptsAllStakers(staker, approverSignatureAndExpiry); cheats.startPrank(address(strategyManagerMock)); @@ -960,7 +960,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // register *this contract* as an operator address operator = address(this); - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; // filter inputs, since delegating to the operator will fail when the staker is already registered as an operator cheats.assume(staker != operator); @@ -1006,7 +1006,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { cheats.assume(strategies.length <= 64); // register *this contract* as an operator address operator = address(this); - IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry; + ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry; // filter inputs, since delegating to the operator will fail when the staker is already registered as an operator cheats.assume(staker != operator); @@ -1093,7 +1093,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { slasherMock.setOperatorFrozenStatus(operator, true); cheats.expectRevert(bytes("DelegationManager._delegate: cannot delegate to a frozen operator")); cheats.startPrank(staker); - IDelegationManager.SignatureWithExpiry memory signatureWithExpiry; + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; delegationManager.delegateTo(operator, signatureWithExpiry); cheats.stopPrank(); } @@ -1122,7 +1122,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { cheats.stopPrank(); cheats.startPrank(staker); - IDelegationManager.SignatureWithExpiry memory signatureWithExpiry; + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; delegationManager.delegateTo(operator, signatureWithExpiry); cheats.stopPrank(); @@ -1135,7 +1135,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { // @notice Verifies that it is not possible to delegate to an unregistered operator function testCannotDelegateToUnregisteredOperator(address operator) public { cheats.expectRevert(bytes("DelegationManager._delegate: operator is not registered in EigenLayer")); - IDelegationManager.SignatureWithExpiry memory signatureWithExpiry; + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; delegationManager.delegateTo(operator, signatureWithExpiry); } @@ -1147,7 +1147,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { cheats.startPrank(staker); cheats.expectRevert(bytes("Pausable: index is paused")); - IDelegationManager.SignatureWithExpiry memory signatureWithExpiry; + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; delegationManager.delegateTo(operator, signatureWithExpiry); cheats.stopPrank(); } @@ -1251,7 +1251,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { * the `staker` to delegate to `operator`, and expiring at `expiry`. */ function _getApproverSignature(uint256 _delegationSignerPrivateKey, address staker, address operator, uint256 expiry) - internal view returns (IDelegationManager.SignatureWithExpiry memory approverSignatureAndExpiry) + internal view returns (ISignatureUtils.SignatureWithExpiry memory approverSignatureAndExpiry) { approverSignatureAndExpiry.expiry = expiry; { @@ -1267,7 +1267,7 @@ contract DelegationUnitTests is EigenLayerTestHelper { * the `operator`, and expiring at `expiry`. */ function _getStakerSignature(uint256 _stakerPrivateKey, address operator, uint256 expiry) - internal view returns (IDelegationManager.SignatureWithExpiry memory stakerSignatureAndExpiry) + internal view returns (ISignatureUtils.SignatureWithExpiry memory stakerSignatureAndExpiry) { address staker = cheats.addr(stakerPrivateKey); stakerSignatureAndExpiry.expiry = expiry; diff --git a/src/test/utils/MockAVSDeployer.sol b/src/test/utils/MockAVSDeployer.sol index cff580bed..78558f7e8 100644 --- a/src/test/utils/MockAVSDeployer.sol +++ b/src/test/utils/MockAVSDeployer.sol @@ -70,6 +70,9 @@ contract MockAVSDeployer is Test { address public pauser = address(uint160(uint256(keccak256("pauser")))); address public unpauser = address(uint160(uint256(keccak256("unpauser")))); + uint256 churnerPrivateKey = uint256(keccak256("churnerPrivateKey")); + address churner = cheats.addr(churnerPrivateKey); + address defaultOperator = address(uint160(uint256(keccak256("defaultOperator")))); bytes32 defaultOperatorId; BN254.G1Point internal defaultPubKey = BN254.G1Point(18260007818883133054078754218619977578772505796600400998181738095793040006897,3432351341799135763167709827653955074218841517684851694584291831827675065899); @@ -238,6 +241,7 @@ contract MockAVSDeployer is Test { address(registryCoordinatorImplementation), abi.encodeWithSelector( BLSRegistryCoordinatorWithIndices.initialize.selector, + churner, operatorSetParams ) ); @@ -361,4 +365,19 @@ contract MockAVSDeployer is Test { function _incrementBytes32(bytes32 start, uint256 inc) internal pure returns(bytes32) { return bytes32(uint256(start) + inc); } + + function _signOperatorChurnApproval(bytes32 registeringOperatorId, IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams, uint256 expiry) internal returns(ISignatureUtils.SignatureWithExpiry memory) { + bytes32 digestHash = registryCoordinator.calculateCurrentOperatorChurnApprovalDigestHash( + registeringOperatorId, + operatorKickParams, + expiry + ); + emit log_named_address("churner", churner); + emit log_named_bytes32("digestHash", digestHash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(churnerPrivateKey, digestHash); + return ISignatureUtils.SignatureWithExpiry({ + signature: abi.encodePacked(r, s, v), + expiry: expiry + }); + } } \ No newline at end of file From 809d92b9c27533d74ceb43c2d4ad2bd04536a54c Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 7 Aug 2023 12:41:20 -0700 Subject: [PATCH 2/6] added churner and tests --- ...LSRegistryCoordinatorWithIndicesUnit.t.sol | 171 ++++++++++-------- 1 file changed, 94 insertions(+), 77 deletions(-) diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 8a76dffc0..2b4ff5f42 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -546,93 +546,139 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { } function testRegisterOperatorWithCoordinatorWithKicks_LessThanKickBIPsOfOperatorStake_Reverts(uint256 pseudoRandomNumber) public { - uint32 numOperators = defaultMaxOperatorCount; - uint32 kickRegistrationBlockNumber = 100; - uint32 registrationBlockNumber = 200; - bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); + uint96 operatorToKickStake = defaultMaxOperatorCount * defaultStake; + ( + address operatorToRegister, + BN254.G1Point memory operatorToRegisterPubKey, + IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams + ) = _testRegisterOperatorWithKicks_SetUp(pseudoRandomNumber, quorumNumbers, defaultStake); + bytes32 operatorToRegisterId = operatorToRegisterPubKey.hashG1Point(); - cheats.roll(kickRegistrationBlockNumber); + stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, defaultStake); - for (uint i = 0; i < numOperators - 1; i++) { - BN254.G1Point memory pubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, i))); - address operator = _incrementAddress(defaultOperator, i); - - _registerOperatorWithCoordinator(operator, quorumBitmap, pubKey); - } + cheats.roll(registrationBlockNumber); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + cheats.prank(operatorToRegister); + cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: registering operator has less than kickBIPsOfOperatorStake"); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); + } - address operatorToRegister = _incrementAddress(defaultOperator, numOperators); - BN254.G1Point memory operatorToRegisterPubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, numOperators))); + function testRegisterOperatorWithCoordinatorWithKicks_LessThanKickBIPsOfTotalStake_Reverts(uint256 pseudoRandomNumber) public { + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); + + uint96 operatorToKickStake = defaultMaxOperatorCount * defaultStake; + ( + address operatorToRegister, + BN254.G1Point memory operatorToRegisterPubKey, + IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams + ) = _testRegisterOperatorWithKicks_SetUp(pseudoRandomNumber, quorumNumbers, operatorToKickStake); bytes32 operatorToRegisterId = operatorToRegisterPubKey.hashG1Point(); - bytes32 operatorToKickId; - address operatorToKick; - - // register last operator before kick - IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams = new IBLSRegistryCoordinatorWithIndices.OperatorKickParam[](1); - { - BN254.G1Point memory pubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, numOperators - 1))); - operatorToKickId = pubKey.hashG1Point(); - operatorToKick = _incrementAddress(defaultOperator, numOperators - 1); - _registerOperatorWithCoordinator(operatorToKick, quorumBitmap, pubKey); - bytes32[] memory operatorIdsToSwap = new bytes32[](1); - // operatorIdsToSwap[0] = operatorToRegisterId - operatorIdsToSwap[0] = operatorToRegisterId; + // set the stake of the operator to register to the defaultKickBIPsOfOperatorStake multiple of the operatorToKickStake + stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, operatorToKickStake * defaultKickBIPsOfOperatorStake / 10000 + 1); - operatorKickParams[0] = IBLSRegistryCoordinatorWithIndices.OperatorKickParam({ - operator: operatorToKick, - pubkey: pubKey, - operatorIdsToSwap: operatorIdsToSwap - }); - } + cheats.roll(registrationBlockNumber); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + cheats.prank(operatorToRegister); + cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: operator to kick has more than kickBIPSOfTotalStake"); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); + } - pubkeyCompendium.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); + function testRegisterOperatorWithCoordinatorWithKicks_InvalidSignatures_Reverts(uint256 pseudoRandomNumber) public { + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); - stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, defaultStake); + ( + address operatorToRegister, + BN254.G1Point memory operatorToRegisterPubKey, + IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams + ) = _testRegisterOperatorWithKicks_SetUp(pseudoRandomNumber, quorumNumbers, defaultStake); + + uint96 registeringStake = defaultKickBIPsOfOperatorStake * defaultStake; + stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; + signatureWithExpiry.expiry = block.timestamp + 10; + signatureWithExpiry.signature = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001B"; cheats.prank(operatorToRegister); - cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: registering operator has less than kickBIPsOfOperatorStake"); + cheats.expectRevert("ECDSA: invalid signature"); registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); } - function testRegisterOperatorWithCoordinatorWithKicks_LessThanKickBIPsOfTotalStake_Reverts(uint256 pseudoRandomNumber) public { - uint32 numOperators = defaultMaxOperatorCount; - uint32 kickRegistrationBlockNumber = 100; - uint32 registrationBlockNumber = 200; + function testRegisterOperatorWithCoordinatorWithKicks_ExpiredSignatures_Reverts(uint256 pseudoRandomNumber) public { + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); + + ( + address operatorToRegister, + BN254.G1Point memory operatorToRegisterPubKey, + IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams + ) = _testRegisterOperatorWithKicks_SetUp(pseudoRandomNumber, quorumNumbers, defaultStake); + bytes32 operatorToRegisterId = operatorToRegisterPubKey.hashG1Point(); + uint96 registeringStake = defaultKickBIPsOfOperatorStake * defaultStake; + stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); + + cheats.roll(registrationBlockNumber); + ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp - 1); + cheats.prank(operatorToRegister); + cheats.expectRevert("BLSRegistryCoordinatorWithIndices._verifyChurnerSignatureOnOperatorChurnApproval: churner signature expired"); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); + } + + function testUpdateSocket() public { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); + _registerOperatorWithCoordinator(defaultOperator, quorumBitmap, defaultPubKey); + + cheats.prank(defaultOperator); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit OperatorSocketUpdate(defaultOperatorId, "localhost:32004"); + registryCoordinator.updateSocket("localhost:32004"); + + } + + function testUpdateSocket_NotRegistered_Reverts() public { + cheats.prank(defaultOperator); + cheats.expectRevert("BLSRegistryCoordinatorWithIndicies.updateSocket: operator is not registered"); + registryCoordinator.updateSocket("localhost:32004"); + } + + function _testRegisterOperatorWithKicks_SetUp(uint256 pseudoRandomNumber, bytes memory quorumNumbers, uint96 operatorToKickStake) internal returns(address operatorToRegister, BN254.G1Point memory operatorToRegisterPubKey, IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams) { + uint32 kickRegistrationBlockNumber = 100; + uint32 registrationBlockNumber = 200; + uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); cheats.roll(kickRegistrationBlockNumber); - for (uint i = 0; i < numOperators - 1; i++) { + for (uint i = 0; i < defaultMaxOperatorCount - 1; i++) { BN254.G1Point memory pubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, i))); address operator = _incrementAddress(defaultOperator, i); _registerOperatorWithCoordinator(operator, quorumBitmap, pubKey); } - address operatorToRegister = _incrementAddress(defaultOperator, numOperators); - BN254.G1Point memory operatorToRegisterPubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, numOperators))); + operatorToRegister = _incrementAddress(defaultOperator, defaultMaxOperatorCount); + operatorToRegisterPubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, defaultMaxOperatorCount))); bytes32 operatorToRegisterId = operatorToRegisterPubKey.hashG1Point(); bytes32 operatorToKickId; address operatorToKick; - uint96 operatorToKickStake = defaultStake * numOperators; // register last operator before kick - IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams = new IBLSRegistryCoordinatorWithIndices.OperatorKickParam[](1); + operatorKickParams = new IBLSRegistryCoordinatorWithIndices.OperatorKickParam[](1); { - BN254.G1Point memory pubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, numOperators - 1))); + BN254.G1Point memory pubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, defaultMaxOperatorCount - 1))); operatorToKickId = pubKey.hashG1Point(); - operatorToKick = _incrementAddress(defaultOperator, numOperators - 1); + operatorToKick = _incrementAddress(defaultOperator, defaultMaxOperatorCount - 1); // register last operator with much more than the kickBIPsOfTotalStake stake _registerOperatorWithCoordinator(operatorToKick, quorumBitmap, pubKey, operatorToKickStake); @@ -649,34 +695,5 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { } pubkeyCompendium.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); - - // set the stake of the operator to register to the defaultKickBIPsOfOperatorStake multiple of the operatorToKickStake - stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, operatorToKickStake * defaultKickBIPsOfOperatorStake / 10000 + 1); - - cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); - cheats.prank(operatorToRegister); - cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: operator to kick has more than kickBIPSOfTotalStake"); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); - } - - function testUpdateSocket() public { - bytes memory quorumNumbers = new bytes(1); - quorumNumbers[0] = bytes1(defaultQuorumNumber); - - uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); - _registerOperatorWithCoordinator(defaultOperator, quorumBitmap, defaultPubKey); - - cheats.prank(defaultOperator); - cheats.expectEmit(true, true, true, true, address(registryCoordinator)); - emit OperatorSocketUpdate(defaultOperatorId, "localhost:32004"); - registryCoordinator.updateSocket("localhost:32004"); - - } - - function testUpdateSocket_NotRegistered_Reverts() public { - cheats.prank(defaultOperator); - cheats.expectRevert("BLSRegistryCoordinatorWithIndicies.updateSocket: operator is not registered"); - registryCoordinator.updateSocket("localhost:32004"); } } \ No newline at end of file From c423ca33fa2bd89ffc0f61db09c5d8b112796664 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 7 Aug 2023 13:29:13 -0700 Subject: [PATCH 3/6] change to churnApprover and add setChurner --- .../BLSRegistryCoordinatorWithIndices.sol | 63 ++++++++++--------- ...LSRegistryCoordinatorWithIndicesUnit.t.sol | 4 +- src/test/utils/MockAVSDeployer.sol | 10 ++- 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol index 07e690aee..35f539112 100644 --- a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol @@ -46,13 +46,6 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr /// @notice the Index Registry contract that will keep track of operators' indexes IIndexRegistry public immutable indexRegistry; - /** - * @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 the mapping from quorum number to the maximum number of operators that can be registered for that quorum mapping(uint8 => OperatorSetParam) internal _quorumOperatorSetParams; /// @notice the mapping from operator's operatorId to the updates of the bitmap of quorums they are registered for @@ -62,9 +55,9 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr /// @notice the dynamic-length array of the registries this coordinator is coordinating address[] public registries; /// @notice the address of the entity allowed to sign off on operators getting kicked out of the AVS during registration - address public churner; - /// @notice the nonce of the churner used in EIP-712 signatures - uint256 public churnerNonce; + address public churnApprover; + /// @notice the nonce of the churnApprover used in EIP-712 signatures + uint256 public churnApproverNonce; modifier onlyServiceManagerOwner { require(msg.sender == serviceManager.owner(), "BLSRegistryCoordinatorWithIndices.onlyServiceManagerOwner: caller is not the service manager owner"); @@ -85,9 +78,9 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr indexRegistry = _indexRegistry; } - function initialize(address _churner, OperatorSetParam[] memory _operatorSetParams) external initializer { - // set the churner - churner = _churner; + function initialize(address _churnApprover, OperatorSetParam[] memory _operatorSetParams) external initializer { + // set the churnApprover + churnApprover = _churnApprover; // the stake registry is this contract itself registries.push(address(stakeRegistry)); registries.push(address(blsPubkeyRegistry)); @@ -181,10 +174,10 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr } /** - * @notice Public function for the the churner signature hash calculation when operators are being kicked from quorums + * @notice Public function for the the churnApprover signature hash calculation when operators are being kicked from quorums * @param registeringOperatorId The is of the registering operator * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps - * @param expiry The desired expiry time of the churner's signature + * @param expiry The desired expiry time of the churnApprover's signature */ function calculateCurrentOperatorChurnApprovalDigestHash( bytes32 registeringOperatorId, @@ -192,24 +185,24 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr uint256 expiry ) public view returns (bytes32) { // calculate the digest hash - return calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnerNonce, expiry); + return calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnApproverNonce, expiry); } /** - * @notice Public function for the the churner signature hash calculation when operators are being kicked from quorums + * @notice Public function for the the churnApprover signature hash calculation when operators are being kicked from quorums * @param registeringOperatorId The is of the registering operator * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps - * @param churnerNonceToUse nonce of the churner. In practice we use the churner's current nonce, stored at `churnerNonce` - * @param expiry The desired expiry time of the churner's signature + * @param churnApproverNonceToUse nonce of the churnApprover. In practice we use the churnApprover's current nonce, stored at `churnApproverNonce` + * @param expiry The desired expiry time of the churnApprover's signature */ function calculateOperatorChurnApprovalDigestHash( bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, - uint256 churnerNonceToUse, + uint256 churnApproverNonceToUse, uint256 expiry ) public view returns (bytes32) { // calculate the digest hash - return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, churnerNonceToUse, expiry))); + return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, churnApproverNonceToUse, expiry))); } // STATE CHANGING FUNCTIONS @@ -218,11 +211,21 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr * @notice Sets parameters of the operator set for the given `quorumNumber` * @param quorumNumber is the quorum number to set the maximum number of operators for * @param operatorSetParam is the parameters of the operator set for the `quorumNumber` + * @dev only callable by the service manager owner */ function setOperatorSetParams(uint8 quorumNumber, OperatorSetParam memory operatorSetParam) external onlyServiceManagerOwner { _setOperatorSetParams(quorumNumber, operatorSetParam); } + /** + * @notice Sets the churnApprover + * @param _churnApprover is the address of the churnApprover + * @dev only callable by the service manager owner + */ + function setChurnApprover(address _churnApprover) external onlyServiceManagerOwner { + churnApprover = _churnApprover; + } + /** * @notice Registers msg.sender as an operator with the middleware * @param quorumNumbers are the bytes representing the quorum numbers that the operator is registering for @@ -268,8 +271,8 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr // get the registering operator's operatorId bytes32 registeringOperatorId = _operators[msg.sender].operatorId; - // verify the churner's signature - _verifyChurnerSignatureOnOperatorChurnApproval(registeringOperatorId, operatorKickParams, signatureAndExpiry); + // verify the churnApprover's signature + _verifychurnApproverSignatureOnOperatorChurnApproval(registeringOperatorId, operatorKickParams, signatureAndExpiry); // kick the operators for (uint256 i = 0; i < quorumNumbers.length; i++) { @@ -459,12 +462,12 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr } } - /// @notice verifies churner's signature on operator churn approval and increments the churner nonce - function _verifyChurnerSignatureOnOperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, SignatureWithExpiry memory signatureWithExpiry) internal { - uint256 churnerNonceMem = churnerNonce; - require(signatureWithExpiry.expiry >= block.timestamp, "BLSRegistryCoordinatorWithIndices._verifyChurnerSignatureOnOperatorChurnApproval: churner signature expired"); - EIP1271SignatureUtils.checkSignature_EIP1271(churner, calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnerNonceMem, signatureWithExpiry.expiry), signatureWithExpiry.signature); - // increment the churner nonce - churnerNonce = churnerNonceMem + 1; + /// @notice verifies churnApprover's signature on operator churn approval and increments the churnApprover nonce + function _verifychurnApproverSignatureOnOperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, SignatureWithExpiry memory signatureWithExpiry) internal { + uint256 churnApproverNonceMem = churnApproverNonce; + require(signatureWithExpiry.expiry >= block.timestamp, "BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover signature expired"); + EIP1271SignatureUtils.checkSignature_EIP1271(churnApprover, calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnApproverNonceMem, signatureWithExpiry.expiry), signatureWithExpiry.signature); + // increment the churnApprover nonce + churnApproverNonce = churnApproverNonceMem + 1; } } \ No newline at end of file diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 2b4ff5f42..90964165a 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -49,7 +49,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { // make sure the contract intializers are disabled cheats.expectRevert(bytes("Initializable: contract is already initialized")); - registryCoordinator.initialize(churner, operatorSetParams); + registryCoordinator.initialize(churnApprover, operatorSetParams); } function testRegisterOperatorWithCoordinator_EmptyQuorumNumbers_Reverts() public { @@ -628,7 +628,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { cheats.roll(registrationBlockNumber); ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp - 1); cheats.prank(operatorToRegister); - cheats.expectRevert("BLSRegistryCoordinatorWithIndices._verifyChurnerSignatureOnOperatorChurnApproval: churner signature expired"); + cheats.expectRevert("BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover signature expired"); registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); } diff --git a/src/test/utils/MockAVSDeployer.sol b/src/test/utils/MockAVSDeployer.sol index 78558f7e8..fa9d2e833 100644 --- a/src/test/utils/MockAVSDeployer.sol +++ b/src/test/utils/MockAVSDeployer.sol @@ -70,8 +70,8 @@ contract MockAVSDeployer is Test { address public pauser = address(uint160(uint256(keccak256("pauser")))); address public unpauser = address(uint160(uint256(keccak256("unpauser")))); - uint256 churnerPrivateKey = uint256(keccak256("churnerPrivateKey")); - address churner = cheats.addr(churnerPrivateKey); + uint256 churnApproverPrivateKey = uint256(keccak256("churnApproverPrivateKey")); + address churnApprover = cheats.addr(churnApproverPrivateKey); address defaultOperator = address(uint160(uint256(keccak256("defaultOperator")))); bytes32 defaultOperatorId; @@ -241,7 +241,7 @@ contract MockAVSDeployer is Test { address(registryCoordinatorImplementation), abi.encodeWithSelector( BLSRegistryCoordinatorWithIndices.initialize.selector, - churner, + churnApprover, operatorSetParams ) ); @@ -372,9 +372,7 @@ contract MockAVSDeployer is Test { operatorKickParams, expiry ); - emit log_named_address("churner", churner); - emit log_named_bytes32("digestHash", digestHash); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(churnerPrivateKey, digestHash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(churnApproverPrivateKey, digestHash); return ISignatureUtils.SignatureWithExpiry({ signature: abi.encodePacked(r, s, v), expiry: expiry From 294a2bb7fa38fe7bc522b4a89561407b497a2e28 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 7 Aug 2023 15:19:13 -0700 Subject: [PATCH 4/6] update to use salt --- src/contracts/interfaces/ISignatureUtils.sol | 10 +++++ .../BLSRegistryCoordinatorWithIndices.sol | 43 +++++++------------ ...LSRegistryCoordinatorWithIndicesUnit.t.sol | 19 ++++---- src/test/utils/MockAVSDeployer.sol | 11 +++-- 4 files changed, 42 insertions(+), 41 deletions(-) diff --git a/src/contracts/interfaces/ISignatureUtils.sol b/src/contracts/interfaces/ISignatureUtils.sol index 4c69b5f47..e1f488779 100644 --- a/src/contracts/interfaces/ISignatureUtils.sol +++ b/src/contracts/interfaces/ISignatureUtils.sol @@ -14,4 +14,14 @@ interface ISignatureUtils { // the expiration timestamp (UTC) of the signature uint256 expiry; } + + // @notice Struct that bundles together a signature, a salt for uniqueness, and an expiration time for the signature. Used primarily for stack management. + struct SignatureWithSaltAndExpiry { + // the signature itself, formatted as a single bytes object + bytes signature; + // the salt used to generate the signature + bytes32 salt; + // the expiration timestamp (UTC) of the signature + uint256 expiry; + } } \ No newline at end of file diff --git a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol index 35f539112..8380e7417 100644 --- a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol @@ -56,8 +56,8 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr address[] public registries; /// @notice the address of the entity allowed to sign off on operators getting kicked out of the AVS during registration address public churnApprover; - /// @notice the nonce of the churnApprover used in EIP-712 signatures - uint256 public churnApproverNonce; + /// @notice whether the salt has been used for an operator churn approval + mapping(bytes32 => bool) public isChurnApproverSaltUsed; modifier onlyServiceManagerOwner { require(msg.sender == serviceManager.owner(), "BLSRegistryCoordinatorWithIndices.onlyServiceManagerOwner: caller is not the service manager owner"); @@ -177,32 +177,17 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr * @notice Public function for the the churnApprover signature hash calculation when operators are being kicked from quorums * @param registeringOperatorId The is of the registering operator * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps - * @param expiry The desired expiry time of the churnApprover's signature - */ - function calculateCurrentOperatorChurnApprovalDigestHash( - bytes32 registeringOperatorId, - OperatorKickParam[] memory operatorKickParams, - uint256 expiry - ) public view returns (bytes32) { - // calculate the digest hash - return calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnApproverNonce, expiry); - } - - /** - * @notice Public function for the the churnApprover signature hash calculation when operators are being kicked from quorums - * @param registeringOperatorId The is of the registering operator - * @param operatorKickParams The parameters needed to kick the operator from the quorums that have reached their caps - * @param churnApproverNonceToUse nonce of the churnApprover. In practice we use the churnApprover's current nonce, stored at `churnApproverNonce` + * @param salt The salt to use for the churnApprover's signature * @param expiry The desired expiry time of the churnApprover's signature */ function calculateOperatorChurnApprovalDigestHash( bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, - uint256 churnApproverNonceToUse, + bytes32 salt, uint256 expiry ) public view returns (bytes32) { // calculate the digest hash - return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, churnApproverNonceToUse, expiry))); + return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, salt, expiry))); } // STATE CHANGING FUNCTIONS @@ -257,13 +242,14 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr * @param operatorKickParams are the parameters for the deregistration of the operator that is being kicked from each * quorum that will be filled after the operator registers. These parameters should include an operator, their pubkey, * and ids of the operators to swap with the kicked operator. + * @param signatureWithSaltAndExpiry is the signature of the churnApprover on the operator kick params with a salt and expiry */ function registerOperatorWithCoordinator( bytes calldata quorumNumbers, BN254.G1Point memory pubkey, string calldata socket, OperatorKickParam[] calldata operatorKickParams, - SignatureWithExpiry memory signatureAndExpiry + SignatureWithSaltAndExpiry memory signatureWithSaltAndExpiry ) external { // register the operator uint32[] memory numOperatorsPerQuorum = _registerOperatorWithCoordinator(msg.sender, quorumNumbers, pubkey, socket); @@ -272,7 +258,7 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr bytes32 registeringOperatorId = _operators[msg.sender].operatorId; // verify the churnApprover's signature - _verifychurnApproverSignatureOnOperatorChurnApproval(registeringOperatorId, operatorKickParams, signatureAndExpiry); + _verifychurnApproverSignatureOnOperatorChurnApproval(registeringOperatorId, operatorKickParams, signatureWithSaltAndExpiry); // kick the operators for (uint256 i = 0; i < quorumNumbers.length; i++) { @@ -463,11 +449,12 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr } /// @notice verifies churnApprover's signature on operator churn approval and increments the churnApprover nonce - function _verifychurnApproverSignatureOnOperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, SignatureWithExpiry memory signatureWithExpiry) internal { - uint256 churnApproverNonceMem = churnApproverNonce; - require(signatureWithExpiry.expiry >= block.timestamp, "BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover signature expired"); - EIP1271SignatureUtils.checkSignature_EIP1271(churnApprover, calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, churnApproverNonceMem, signatureWithExpiry.expiry), signatureWithExpiry.signature); - // increment the churnApprover nonce - churnApproverNonce = churnApproverNonceMem + 1; + function _verifychurnApproverSignatureOnOperatorChurnApproval(bytes32 registeringOperatorId, OperatorKickParam[] memory operatorKickParams, SignatureWithSaltAndExpiry memory signatureWithSaltAndExpiry) internal { + // make sure the salt hasn't been used already + require(!isChurnApproverSaltUsed[signatureWithSaltAndExpiry.salt], "BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover salt already used"); + require(signatureWithSaltAndExpiry.expiry >= block.timestamp, "BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover signature expired"); + EIP1271SignatureUtils.checkSignature_EIP1271(churnApprover, calculateOperatorChurnApprovalDigestHash(registeringOperatorId, operatorKickParams, signatureWithSaltAndExpiry.salt, signatureWithSaltAndExpiry.expiry), signatureWithSaltAndExpiry.signature); + // set salt used to true + isChurnApproverSaltUsed[signatureWithSaltAndExpiry.salt] = true; } } \ No newline at end of file diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 90964165a..307931012 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -507,7 +507,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1); { - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); uint256 gasBefore = gasleft(); registryCoordinator.registerOperatorWithCoordinator( @@ -560,7 +560,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, defaultStake); cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: registering operator has less than kickBIPsOfOperatorStake"); registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); @@ -583,7 +583,7 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, operatorToKickStake * defaultKickBIPsOfOperatorStake / 10000 + 1); cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp + 10); + ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); cheats.expectRevert("BLSRegistryCoordinatorWithIndices.registerOperatorWithCoordinator: operator to kick has more than kickBIPSOfTotalStake"); registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); @@ -603,12 +603,13 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry; - signatureWithExpiry.expiry = block.timestamp + 10; - signatureWithExpiry.signature = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001B"; + ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithSaltAndExpiry; + signatureWithSaltAndExpiry.expiry = block.timestamp + 10; + signatureWithSaltAndExpiry.signature = hex"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001B"; + signatureWithSaltAndExpiry.salt = defaultSalt; cheats.prank(operatorToRegister); cheats.expectRevert("ECDSA: invalid signature"); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithSaltAndExpiry); } function testRegisterOperatorWithCoordinatorWithKicks_ExpiredSignatures_Reverts(uint256 pseudoRandomNumber) public { @@ -626,10 +627,10 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); cheats.roll(registrationBlockNumber); - ISignatureUtils.SignatureWithExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, block.timestamp - 1); + ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithSaltAndExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp - 1); cheats.prank(operatorToRegister); cheats.expectRevert("BLSRegistryCoordinatorWithIndices._verifyChurnApproverSignatureOnOperatorChurnApproval: churnApprover signature expired"); - registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithExpiry); + registryCoordinator.registerOperatorWithCoordinator(quorumNumbers, operatorToRegisterPubKey, defaultSocket, operatorKickParams, signatureWithSaltAndExpiry); } function testUpdateSocket() public { diff --git a/src/test/utils/MockAVSDeployer.sol b/src/test/utils/MockAVSDeployer.sol index fa9d2e833..5ba37dcd3 100644 --- a/src/test/utils/MockAVSDeployer.sol +++ b/src/test/utils/MockAVSDeployer.sol @@ -72,6 +72,7 @@ contract MockAVSDeployer is Test { uint256 churnApproverPrivateKey = uint256(keccak256("churnApproverPrivateKey")); address churnApprover = cheats.addr(churnApproverPrivateKey); + bytes32 defaultSalt = bytes32(uint256(keccak256("defaultSalt"))); address defaultOperator = address(uint160(uint256(keccak256("defaultOperator")))); bytes32 defaultOperatorId; @@ -366,16 +367,18 @@ contract MockAVSDeployer is Test { return bytes32(uint256(start) + inc); } - function _signOperatorChurnApproval(bytes32 registeringOperatorId, IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams, uint256 expiry) internal returns(ISignatureUtils.SignatureWithExpiry memory) { - bytes32 digestHash = registryCoordinator.calculateCurrentOperatorChurnApprovalDigestHash( + function _signOperatorChurnApproval(bytes32 registeringOperatorId, IBLSRegistryCoordinatorWithIndices.OperatorKickParam[] memory operatorKickParams, bytes32 salt, uint256 expiry) internal returns(ISignatureUtils.SignatureWithSaltAndExpiry memory) { + bytes32 digestHash = registryCoordinator.calculateOperatorChurnApprovalDigestHash( registeringOperatorId, operatorKickParams, + salt, expiry ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(churnApproverPrivateKey, digestHash); - return ISignatureUtils.SignatureWithExpiry({ + return ISignatureUtils.SignatureWithSaltAndExpiry({ signature: abi.encodePacked(r, s, v), - expiry: expiry + expiry: expiry, + salt: salt }); } } \ No newline at end of file From 69c4d99b9a17e2031b36bed1a619b42c0754f910 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 7 Aug 2023 15:38:01 -0700 Subject: [PATCH 5/6] added tests for setters --- .../IBLSRegistryCoordinatorWithIndices.sol | 2 ++ .../BLSRegistryCoordinatorWithIndices.sol | 7 +++++- ...LSRegistryCoordinatorWithIndicesUnit.t.sol | 25 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol b/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol index a29571bf6..025e0041b 100644 --- a/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/interfaces/IBLSRegistryCoordinatorWithIndices.sol @@ -42,6 +42,8 @@ interface IBLSRegistryCoordinatorWithIndices is ISignatureUtils, IRegistryCoordi event OperatorSetParamsUpdated(uint8 indexed quorumNumber, OperatorSetParam operatorSetParams); + event ChurnApproverUpdated(address churnApprover); + /// @notice Returns the operator set params for the given `quorumNumber` function getOperatorSetParams(uint8 quorumNumber) external view returns (OperatorSetParam memory); /// @notice the stake registry for this corrdinator is the contract itself diff --git a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol index 8380e7417..e65cd3685 100644 --- a/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol +++ b/src/contracts/middleware/BLSRegistryCoordinatorWithIndices.sol @@ -208,7 +208,7 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr * @dev only callable by the service manager owner */ function setChurnApprover(address _churnApprover) external onlyServiceManagerOwner { - churnApprover = _churnApprover; + _setChurnApprover(_churnApprover); } /** @@ -341,6 +341,11 @@ contract BLSRegistryCoordinatorWithIndices is EIP712, Initializable, IBLSRegistr _quorumOperatorSetParams[quorumNumber] = operatorSetParam; emit OperatorSetParamsUpdated(quorumNumber, operatorSetParam); } + + function _setChurnApprover(address newChurnApprover) internal { + churnApprover = newChurnApprover; + emit ChurnApproverUpdated(newChurnApprover); + } /// @return numOperatorsPerQuorum is the list of number of operators per quorum in quorumNumberss function _registerOperatorWithCoordinator(address operator, bytes calldata quorumNumbers, BN254.G1Point memory pubkey, string memory socket) internal returns(uint32[] memory) { diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 307931012..9fd84006a 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -30,6 +30,10 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { // emitted when an operator's index in the orderd operator list for the quorum with number `quorumNumber` is updated event QuorumIndexUpdate(bytes32 indexed operatorId, uint8 quorumNumber, uint32 newIndex); + event OperatorSetParamsUpdated(uint8 indexed quorumNumber, IBLSRegistryCoordinatorWithIndices.OperatorSetParam operatorSetParams); + + event ChurnApproverUpdated(address churnApprover); + function setUp() virtual public { _deployMockEigenLayerAndAVS(); } @@ -52,6 +56,27 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { registryCoordinator.initialize(churnApprover, operatorSetParams); } + function testSetOperatorSetParams_NotServiceManagerOwner_Reverts() public { + cheats.expectRevert("BLSRegistryCoordinatorWithIndices.onlyServiceManagerOwner: caller is not the service manager owner"); + cheats.prank(defaultOperator); + registryCoordinator.setOperatorSetParams(0, operatorSetParams[0]); + } + + function testSetOperatorSetParams_Valid() public { + cheats.prank(serviceManagerOwner); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit OperatorSetParamsUpdated(0, operatorSetParams[1]); + registryCoordinator.setOperatorSetParams(0, operatorSetParams[1]); + } + + function testSetChurnApprover_NotServiceManagerOwner_Reverts() public { + address newChurnApprover = address(uint160(uint256(keccak256("newChurnApprover")))); + cheats.prank(serviceManagerOwner); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit ChurnApproverUpdated(newChurnApprover); + registryCoordinator.setChurnApprover(newChurnApprover); + } + function testRegisterOperatorWithCoordinator_EmptyQuorumNumbers_Reverts() public { bytes memory emptyQuorumNumbers = new bytes(0); cheats.expectRevert("BLSRegistryCoordinatorWithIndices._registerOperatorWithCoordinator: quorumBitmap cannot be 0"); From f58e90d6f41a4f9775294a5253af4469d7959b6a Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 7 Aug 2023 15:41:49 -0700 Subject: [PATCH 6/6] added another test for setter --- src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol index 9fd84006a..21a0f5c15 100644 --- a/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol +++ b/src/test/unit/BLSRegistryCoordinatorWithIndicesUnit.t.sol @@ -70,6 +70,13 @@ contract BLSRegistryCoordinatorWithIndicesUnit is MockAVSDeployer { } function testSetChurnApprover_NotServiceManagerOwner_Reverts() public { + address newChurnApprover = address(uint160(uint256(keccak256("newChurnApprover")))); + cheats.expectRevert("BLSRegistryCoordinatorWithIndices.onlyServiceManagerOwner: caller is not the service manager owner"); + cheats.prank(defaultOperator); + registryCoordinator.setChurnApprover(newChurnApprover); + } + + function testSetChurnApprover_Valid() public { address newChurnApprover = address(uint160(uint256(keccak256("newChurnApprover")))); cheats.prank(serviceManagerOwner); cheats.expectEmit(true, true, true, true, address(registryCoordinator));