From 5159d3b694def86113e42fb6dae07c5e449d09a8 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Thu, 7 Dec 2023 14:05:36 -0800 Subject: [PATCH 01/25] chore: copy BLSPublicKeyCompendium storage and functions into BLSApkRegistry --- src/BLSApkRegistry.sol | 82 ++++++++++++++++++++++++++++++ src/BLSApkRegistryStorage.sol | 14 ++++- src/interfaces/IBLSApkRegistry.sol | 40 ++++++++++++++- 3 files changed, 133 insertions(+), 3 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 14093fa0..b5e9ca23 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -95,6 +95,60 @@ contract BLSApkRegistry is BLSApkRegistryStorage { })); } + /** + * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. + * @param signedMessageHash is the registration message hash signed by the private key of the operator + * @param pubkeyG1 is the corresponding G1 public key of the operator + * @param pubkeyG2 is the corresponding G2 public key of the operator + */ + function registerBLSPublicKey( + BN254.G1Point memory signedMessageHash, + BN254.G1Point memory pubkeyG1, + BN254.G2Point memory pubkeyG2 + ) external { + bytes32 pubkeyHash = BN254.hashG1Point(pubkeyG1); + require( + pubkeyHash != ZERO_PK_HASH, "BLSPublicKeyCompendium.registerBLSPublicKey: cannot register zero pubkey" + ); + require( + operatorToPubkeyHash[msg.sender] == bytes32(0), + "BLSPublicKeyCompendium.registerBLSPublicKey: operator already registered pubkey" + ); + require( + pubkeyHashToOperator[pubkeyHash] == address(0), + "BLSPublicKeyCompendium.registerBLSPublicKey: public key already registered" + ); + + // H(m) + BN254.G1Point memory messageHash = getMessageHash(msg.sender); + + // gamma = h(sigma, P, P', H(m)) + uint256 gamma = uint256(keccak256(abi.encodePacked( + signedMessageHash.X, + signedMessageHash.Y, + pubkeyG1.X, + pubkeyG1.Y, + pubkeyG2.X, + pubkeyG2.Y, + messageHash.X, + messageHash.Y + ))) % BN254.FR_MODULUS; + + // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P') + require(BN254.pairing( + signedMessageHash.plus(pubkeyG1.scalar_mul(gamma)), + BN254.negGeneratorG2(), + messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), + pubkeyG2 + ), "BLSPublicKeyCompendium.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); + + operatorToPubkey[msg.sender] = pubkeyG1; + operatorToPubkeyHash[msg.sender] = pubkeyHash; + pubkeyHashToOperator[pubkeyHash] = msg.sender; + + emit NewPubkeyRegistration(msg.sender, pubkeyG1, pubkeyG2); + } + /******************************************************************************* INTERNAL FUNCTIONS *******************************************************************************/ @@ -148,6 +202,34 @@ contract BLSApkRegistry is BLSApkRegistryStorage { /******************************************************************************* VIEW FUNCTIONS *******************************************************************************/ + /** + * @notice Returns the pubkey and pubkey hash of an operator + * @dev Reverts if the operator has not registered a valid pubkey + */ + function getRegisteredPubkey(address operator) public view returns (BN254.G1Point memory, bytes32) { + BN254.G1Point memory pubkey = operatorToPubkey[operator]; + bytes32 pubkeyHash = operatorToPubkeyHash[operator]; + + require( + pubkeyHash != bytes32(0), + "BLSApkRegistry.getRegisteredPubkey: operator is not registered" + ); + + return (pubkey, pubkeyHash); + } + + /** + * @notice Returns the message hash that an operator must sign to register their BLS public key. + * @param operator is the address of the operator registering their BLS public key + */ + function getMessageHash(address operator) public view returns (BN254.G1Point memory) { + return BN254.hashToG1(keccak256(abi.encodePacked( + operator, + address(this), + block.chainid, + "EigenLayer_BN254_Pubkey_Registration" + ))); + } /** * @notice Returns the indices of the quorumApks index at `blockNumber` for the provided `quorumNumbers` diff --git a/src/BLSApkRegistryStorage.sol b/src/BLSApkRegistryStorage.sol index 1629d708..3f56cd42 100644 --- a/src/BLSApkRegistryStorage.sol +++ b/src/BLSApkRegistryStorage.sol @@ -10,11 +10,23 @@ import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initia import {BN254} from "src/libraries/BN254.sol"; abstract contract BLSApkRegistryStorage is Initializable, IBLSApkRegistry { + /// @notice the hash of the zero pubkey aka BN254.G1Point(0,0) + bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; + /// @notice the registry coordinator contract IRegistryCoordinator public immutable registryCoordinator; /// @notice the BLSPublicKeyCompendium contract against which pubkey ownership is checked IBLSPublicKeyCompendium public immutable pubkeyCompendium; + // storage for individual pubkeys + /// @notice maps operator address to pubkey hash + mapping(address => bytes32) public operatorToPubkeyHash; + /// @notice maps pubkey hash to operator address + mapping(bytes32 => address) public pubkeyHashToOperator; + /// @notice maps operator address to pubkeyG1 + mapping(address => BN254.G1Point) public operatorToPubkey; + + // storage for aggregate pubkeys (APKs) /// @notice maps quorumNumber => historical aggregate pubkey updates mapping(uint8 => ApkUpdate[]) public apkHistory; /// @notice maps quorumNumber => current aggregate pubkey of quorum @@ -28,5 +40,5 @@ abstract contract BLSApkRegistryStorage is Initializable, IBLSApkRegistry { } // storage gap for upgradeability - uint256[48] private __GAP; + uint256[45] private __GAP; } diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 58cdc10b..0569bc9d 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -11,13 +11,16 @@ import {BN254} from "src/libraries/BN254.sol"; */ interface IBLSApkRegistry is IRegistry { // EVENTS - // Emitted when a new operator pubkey is registered for a set of quorums + /// @notice Emitted when `operator` registers with the public keys `pubkeyG1` and `pubkeyG2`. + event NewPubkeyRegistration(address indexed operator, BN254.G1Point pubkeyG1, BN254.G2Point pubkeyG2); + + // @notice Emitted when a new operator pubkey is registered for a set of quorums event OperatorAddedToQuorums( address operator, bytes quorumNumbers ); - // Emitted when an operator pubkey is removed from a set of quorums + // @notice Emitted when an operator pubkey is removed from a set of quorums event OperatorRemovedFromQuorums( address operator, bytes quorumNumbers @@ -66,6 +69,39 @@ interface IBLSApkRegistry is IRegistry { */ function initializeQuorum(uint8 quorumNumber) external; + /** + * @notice mapping from operator address to pubkey hash. + * Returns *zero* if the `operator` has never registered, and otherwise returns the hash of the public key of the operator. + */ + function operatorToPubkeyHash(address operator) external view returns (bytes32); + + /** + * @notice mapping from pubkey hash to operator address. + * Returns *zero* if no operator has ever registered the public key corresponding to `pubkeyHash`, + * and otherwise returns the (unique) registered operator who owns the BLS public key that is the preimage of `pubkeyHash`. + */ + function pubkeyHashToOperator(bytes32 pubkeyHash) external view returns (address); + + /** + * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. + * @param signedMessageHash is the registration message hash signed by the private key of the operator + * @param pubkeyG1 is the corresponding G1 public key of the operator + * @param pubkeyG2 is the corresponding G2 public key of the operator + */ + function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external; + + /** + * @notice Returns the pubkey and pubkey hash of an operator + * @dev Reverts if the operator has not registered a valid pubkey + */ + function getRegisteredPubkey(address operator) external view returns (BN254.G1Point memory, bytes32); + + /** + * @notice Returns the message hash that an operator must sign to register their BLS public key. + * @param operator is the address of the operator registering their BLS public key + */ + function getMessageHash(address operator) external view returns (BN254.G1Point memory); + /// @notice Returns the current APK for the provided `quorumNumber ` function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory); From 68f381747bd519d6713ffd6ad60107e5ebc795a8 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Thu, 7 Dec 2023 15:34:49 -0800 Subject: [PATCH 02/25] chore: delete BLSPublicKeyCompendium and associated interface requires changing a bunch of files to make things work. I had to comment out a few calls in tests (now marked with a `TODO`) to get this past the compiler. Will have to do additional cleanup to get this ready for review. --- script/AVSContractsDeploy.s.sol | 2 - script/DeploySharedContracts.s.sol | 15 +-- src/BLSApkRegistry.sol | 29 +++-- src/BLSApkRegistryStorage.sol | 6 +- src/BLSPublicKeyCompendium.sol | 115 ------------------- src/interfaces/IBLSApkRegistry.sol | 2 +- src/interfaces/IBLSPublicKeyCompendium.sol | 49 -------- test/ffi/BLSPubKeyCompendiumFFI.t.sol | 17 +-- test/mocks/BLSApkRegistryMock.sol | 127 +++++++++++++++++++++ test/mocks/BLSPublicKeyCompendiumMock.sol | 63 ---------- test/unit/BLSApkRegistryUnit.t.sol | 89 +++++++++++++-- test/unit/BLSPublicKeyCompendiumUnit.t.sol | 80 ------------- test/unit/RegistryCoordinatorUnit.t.sol | 6 +- test/utils/MockAVSDeployer.sol | 22 ++-- 14 files changed, 246 insertions(+), 376 deletions(-) delete mode 100644 src/BLSPublicKeyCompendium.sol delete mode 100644 src/interfaces/IBLSPublicKeyCompendium.sol create mode 100644 test/mocks/BLSApkRegistryMock.sol delete mode 100644 test/mocks/BLSPublicKeyCompendiumMock.sol delete mode 100644 test/unit/BLSPublicKeyCompendiumUnit.t.sol diff --git a/script/AVSContractsDeploy.s.sol b/script/AVSContractsDeploy.s.sol index d3386dd3..8ec6c200 100644 --- a/script/AVSContractsDeploy.s.sol +++ b/script/AVSContractsDeploy.s.sol @@ -19,10 +19,8 @@ import "eigenlayer-contracts/src/contracts/pods/EigenPodManager.sol"; import "eigenlayer-contracts/src/contracts/pods/DelayedWithdrawalRouter.sol"; import "eigenlayer-contracts/src/contracts/permissions/PauserRegistry.sol"; -import "src/BLSPublicKeyCompendium.sol"; import "eigenlayer-contracts/src/test/mocks/EmptyContract.sol"; -import "eigenlayer-contracts/src/test/mocks/ETHDepositMock.sol"; import "eigenlayer-contracts/src/test/mocks/ERC20Mock.sol"; import "forge-std/Script.sol"; diff --git a/script/DeploySharedContracts.s.sol b/script/DeploySharedContracts.s.sol index d139f8fb..56535aec 100644 --- a/script/DeploySharedContracts.s.sol +++ b/script/DeploySharedContracts.s.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity =0.8.12; -import "../src/BLSPublicKeyCompendium.sol"; import "../src/OperatorStateRetriever.sol"; import "forge-std/Script.sol"; @@ -15,26 +14,24 @@ import "forge-std/Test.sol"; contract DeploySharedContracts is Script, Test { Vm cheats = Vm(HEVM_ADDRESS); - BLSPublicKeyCompendium public blsPublicKeyCompendium; OperatorStateRetriever public blsOperatorStateRetriever; function run() external { vm.startBroadcast(); - blsPublicKeyCompendium = new BLSPublicKeyCompendium(); blsOperatorStateRetriever = new OperatorStateRetriever(); vm.stopBroadcast(); string memory deployed_addresses = "addresses"; - vm.serializeAddress( + // vm.serializeAddress( + // deployed_addresses, + // "blsOperatorStateRetriever", + // address(blsOperatorStateRetriever) + // ); + string memory finalJson = vm.serializeAddress( deployed_addresses, "blsOperatorStateRetriever", address(blsOperatorStateRetriever) ); - string memory finalJson = vm.serializeAddress( - deployed_addresses, - "blsPublicKeyCompendium", - address(blsPublicKeyCompendium) - ); vm.writeJson(finalJson, outputFileName()); } diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index b5e9ca23..99f48288 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -3,7 +3,6 @@ pragma solidity =0.8.12; import {BLSApkRegistryStorage} from "src/BLSApkRegistryStorage.sol"; -import {IBLSPublicKeyCompendium} from "src/interfaces/IBLSPublicKeyCompendium.sol"; import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; import {BN254} from "src/libraries/BN254.sol"; @@ -20,11 +19,10 @@ contract BLSApkRegistry is BLSApkRegistryStorage { _; } - /// @notice Sets the (immutable) `registryCoordinator` and `pubkeyCompendium` addresses + /// @notice Sets the (immutable) `registryCoordinator` address constructor( - IRegistryCoordinator _registryCoordinator, - IBLSPublicKeyCompendium _pubkeyCompendium - ) BLSApkRegistryStorage(_registryCoordinator, _pubkeyCompendium) {} + IRegistryCoordinator _registryCoordinator + ) BLSApkRegistryStorage(_registryCoordinator) {} /******************************************************************************* EXTERNAL FUNCTIONS - REGISTRY COORDINATOR @@ -46,8 +44,8 @@ contract BLSApkRegistry is BLSApkRegistryStorage { address operator, bytes memory quorumNumbers ) public virtual onlyRegistryCoordinator returns (bytes32) { - // Get the operator's pubkey from the compendium. Reverts if they have not registered a key - (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = pubkeyCompendium.getRegisteredPubkey(operator); + // Get the operator's pubkey. Reverts if they have not registered a key + (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = getRegisteredPubkey(operator); // Update each quorum's aggregate pubkey _processQuorumApkUpdate(quorumNumbers, pubkey); @@ -73,8 +71,8 @@ contract BLSApkRegistry is BLSApkRegistryStorage { address operator, bytes memory quorumNumbers ) public virtual onlyRegistryCoordinator { - // Get the operator's pubkey from the compendium. Reverts if they have not registered a key - (BN254.G1Point memory pubkey, ) = pubkeyCompendium.getRegisteredPubkey(operator); + // Get the operator's pubkey. Reverts if they have not registered a key + (BN254.G1Point memory pubkey, ) = getRegisteredPubkey(operator); // Update each quorum's aggregate pubkey _processQuorumApkUpdate(quorumNumbers, pubkey.negate()); @@ -108,15 +106,15 @@ contract BLSApkRegistry is BLSApkRegistryStorage { ) external { bytes32 pubkeyHash = BN254.hashG1Point(pubkeyG1); require( - pubkeyHash != ZERO_PK_HASH, "BLSPublicKeyCompendium.registerBLSPublicKey: cannot register zero pubkey" + pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey" ); require( operatorToPubkeyHash[msg.sender] == bytes32(0), - "BLSPublicKeyCompendium.registerBLSPublicKey: operator already registered pubkey" + "BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey" ); require( pubkeyHashToOperator[pubkeyHash] == address(0), - "BLSPublicKeyCompendium.registerBLSPublicKey: public key already registered" + "BLSApkRegistry.registerBLSPublicKey: public key already registered" ); // H(m) @@ -140,7 +138,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { BN254.negGeneratorG2(), messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), pubkeyG2 - ), "BLSPublicKeyCompendium.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); + ), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); operatorToPubkey[msg.sender] = pubkeyG1; operatorToPubkeyHash[msg.sender] = pubkeyHash; @@ -294,11 +292,10 @@ contract BLSApkRegistry is BLSApkRegistryStorage { /// @notice Returns the operator address for the given `pubkeyHash` function getOperatorFromPubkeyHash(bytes32 pubkeyHash) public view returns (address) { - return pubkeyCompendium.pubkeyHashToOperator(pubkeyHash); + return pubkeyHashToOperator[pubkeyHash]; } function getOperatorId(address operator) public view returns (bytes32) { - (, bytes32 pubkeyHash) = pubkeyCompendium.getRegisteredPubkey(operator); - return pubkeyHash; + return operatorToPubkeyHash[operator]; } } diff --git a/src/BLSApkRegistryStorage.sol b/src/BLSApkRegistryStorage.sol index 3f56cd42..4676a6f5 100644 --- a/src/BLSApkRegistryStorage.sol +++ b/src/BLSApkRegistryStorage.sol @@ -3,7 +3,6 @@ pragma solidity =0.8.12; import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol"; import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; -import {IBLSPublicKeyCompendium} from "src/interfaces/IBLSPublicKeyCompendium.sol"; import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol"; @@ -15,8 +14,6 @@ abstract contract BLSApkRegistryStorage is Initializable, IBLSApkRegistry { /// @notice the registry coordinator contract IRegistryCoordinator public immutable registryCoordinator; - /// @notice the BLSPublicKeyCompendium contract against which pubkey ownership is checked - IBLSPublicKeyCompendium public immutable pubkeyCompendium; // storage for individual pubkeys /// @notice maps operator address to pubkey hash @@ -32,9 +29,8 @@ abstract contract BLSApkRegistryStorage is Initializable, IBLSApkRegistry { /// @notice maps quorumNumber => current aggregate pubkey of quorum mapping(uint8 => BN254.G1Point) public currentApk; - constructor(IRegistryCoordinator _registryCoordinator, IBLSPublicKeyCompendium _pubkeyCompendium) { + constructor(IRegistryCoordinator _registryCoordinator) { registryCoordinator = _registryCoordinator; - pubkeyCompendium = _pubkeyCompendium; // disable initializers so that the implementation contract cannot be initialized _disableInitializers(); } diff --git a/src/BLSPublicKeyCompendium.sol b/src/BLSPublicKeyCompendium.sol deleted file mode 100644 index f9aad7f1..00000000 --- a/src/BLSPublicKeyCompendium.sol +++ /dev/null @@ -1,115 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity =0.8.12; - -import {IBLSPublicKeyCompendium} from "src/interfaces/IBLSPublicKeyCompendium.sol"; -import {BN254} from "src/libraries/BN254.sol"; - -/** - * @title A shared contract for EigenLayer operators to register their BLS public keys. - * @author Layr Labs, Inc. - * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service - */ -contract BLSPublicKeyCompendium is IBLSPublicKeyCompendium { - using BN254 for BN254.G1Point; - - /// @notice the hash of the zero pubkey aka BN254.G1Point(0,0) - bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; - - /// @notice maps operator address to pubkey hash - mapping(address => bytes32) public operatorToPubkeyHash; - /// @notice maps pubkey hash to operator address - mapping(bytes32 => address) public pubkeyHashToOperator; - /// @notice maps operator address to pubkeyG1 - mapping(address => BN254.G1Point) public operatorToPubkey; - - /******************************************************************************* - EXTERNAL FUNCTIONS - *******************************************************************************/ - - /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator - */ - function registerBLSPublicKey( - BN254.G1Point memory signedMessageHash, - BN254.G1Point memory pubkeyG1, - BN254.G2Point memory pubkeyG2 - ) external { - bytes32 pubkeyHash = BN254.hashG1Point(pubkeyG1); - require( - pubkeyHash != ZERO_PK_HASH, "BLSPublicKeyCompendium.registerBLSPublicKey: cannot register zero pubkey" - ); - require( - operatorToPubkeyHash[msg.sender] == bytes32(0), - "BLSPublicKeyCompendium.registerBLSPublicKey: operator already registered pubkey" - ); - require( - pubkeyHashToOperator[pubkeyHash] == address(0), - "BLSPublicKeyCompendium.registerBLSPublicKey: public key already registered" - ); - - // H(m) - BN254.G1Point memory messageHash = getMessageHash(msg.sender); - - // gamma = h(sigma, P, P', H(m)) - uint256 gamma = uint256(keccak256(abi.encodePacked( - signedMessageHash.X, - signedMessageHash.Y, - pubkeyG1.X, - pubkeyG1.Y, - pubkeyG2.X, - pubkeyG2.Y, - messageHash.X, - messageHash.Y - ))) % BN254.FR_MODULUS; - - // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P') - require(BN254.pairing( - signedMessageHash.plus(pubkeyG1.scalar_mul(gamma)), - BN254.negGeneratorG2(), - messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), - pubkeyG2 - ), "BLSPublicKeyCompendium.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); - - operatorToPubkey[msg.sender] = pubkeyG1; - operatorToPubkeyHash[msg.sender] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = msg.sender; - - emit NewPubkeyRegistration(msg.sender, pubkeyG1, pubkeyG2); - } - - /******************************************************************************* - VIEW FUNCTIONS - *******************************************************************************/ - - /** - * @notice Returns the pubkey and pubkey hash of an operator - * @dev Reverts if the operator has not registered a valid pubkey - */ - function getRegisteredPubkey(address operator) public view returns (BN254.G1Point memory, bytes32) { - BN254.G1Point memory pubkey = operatorToPubkey[operator]; - bytes32 pubkeyHash = operatorToPubkeyHash[operator]; - - require( - pubkeyHash != bytes32(0), - "BLSPublicKeyCompendium.getRegisteredPubkey: operator is not registered" - ); - - return (pubkey, pubkeyHash); - } - - /** - * @notice Returns the message hash that an operator must sign to register their BLS public key. - * @param operator is the address of the operator registering their BLS public key - */ - function getMessageHash(address operator) public view returns (BN254.G1Point memory) { - return BN254.hashToG1(keccak256(abi.encodePacked( - operator, - address(this), - block.chainid, - "EigenLayer_BN254_Pubkey_Registration" - ))); - } -} diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 0569bc9d..89550777 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -35,7 +35,7 @@ interface IBLSApkRegistry is IRegistry { // block number at which the next update occurred uint32 nextUpdateBlockNumber; } - + /** * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. * @param operator The address of the operator to register. diff --git a/src/interfaces/IBLSPublicKeyCompendium.sol b/src/interfaces/IBLSPublicKeyCompendium.sol deleted file mode 100644 index b7ab1a8e..00000000 --- a/src/interfaces/IBLSPublicKeyCompendium.sol +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity >=0.5.0; - -import {BN254} from "src/libraries/BN254.sol"; - -/** - * @title Minimal interface for the `BLSPublicKeyCompendium` contract. - * @author Layr Labs, Inc. - * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service - */ -interface IBLSPublicKeyCompendium { - - // EVENTS - /// @notice Emitted when `operator` registers with the public keys `pubkeyG1` and `pubkeyG2`. - event NewPubkeyRegistration(address indexed operator, BN254.G1Point pubkeyG1, BN254.G2Point pubkeyG2); - - /** - * @notice mapping from operator address to pubkey hash. - * Returns *zero* if the `operator` has never registered, and otherwise returns the hash of the public key of the operator. - */ - function operatorToPubkeyHash(address operator) external view returns (bytes32); - - /** - * @notice mapping from pubkey hash to operator address. - * Returns *zero* if no operator has ever registered the public key corresponding to `pubkeyHash`, - * and otherwise returns the (unique) registered operator who owns the BLS public key that is the preimage of `pubkeyHash`. - */ - function pubkeyHashToOperator(bytes32 pubkeyHash) external view returns (address); - - /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator - */ - function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external; - - /** - * @notice Returns the pubkey and pubkey hash of an operator - * @dev Reverts if the operator has not registered a valid pubkey - */ - function getRegisteredPubkey(address operator) external view returns (BN254.G1Point memory, bytes32); - - /** - * @notice Returns the message hash that an operator must sign to register their BLS public key. - * @param operator is the address of the operator registering their BLS public key - */ - function getMessageHash(address operator) external view returns (BN254.G1Point memory); -} diff --git a/test/ffi/BLSPubKeyCompendiumFFI.t.sol b/test/ffi/BLSPubKeyCompendiumFFI.t.sol index c3a5216d..8a1a5a99 100644 --- a/test/ffi/BLSPubKeyCompendiumFFI.t.sol +++ b/test/ffi/BLSPubKeyCompendiumFFI.t.sol @@ -1,16 +1,16 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity =0.8.12; -import "src/BLSPublicKeyCompendium.sol"; +import "src/BLSApkRegistry.sol"; import "test/ffi/util/G2Operations.sol"; -contract BLSPublicKeyCompendiumFFITests is G2Operations { +contract BLSApkRegistryFFITests is G2Operations { using BN254 for BN254.G1Point; using Strings for uint256; Vm cheats = Vm(HEVM_ADDRESS); - BLSPublicKeyCompendium compendium; + BLSApkRegistry blsApkRegistry; uint256 privKey; BN254.G1Point pubKeyG1; @@ -20,7 +20,8 @@ contract BLSPublicKeyCompendiumFFITests is G2Operations { address alice = address(0x69); function setUp() public { - compendium = new BLSPublicKeyCompendium(); + IRegistryCoordinator registryCoordinator; + blsApkRegistry = new BLSApkRegistry(registryCoordinator); } function testRegisterBLSPublicKey(uint256 _privKey) public { @@ -30,10 +31,10 @@ contract BLSPublicKeyCompendiumFFITests is G2Operations { signedMessageHash = _signMessage(alice); vm.prank(alice); - compendium.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); - assertEq(compendium.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); - assertEq(compendium.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); + assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); + assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); } function _setKeys(uint256 _privKey) internal { @@ -43,7 +44,7 @@ contract BLSPublicKeyCompendiumFFITests is G2Operations { } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { - BN254.G1Point memory messageHash = compendium.getMessageHash(signer); + BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer); return BN254.scalar_mul(messageHash, privKey); } } diff --git a/test/mocks/BLSApkRegistryMock.sol b/test/mocks/BLSApkRegistryMock.sol new file mode 100644 index 00000000..61cb3d95 --- /dev/null +++ b/test/mocks/BLSApkRegistryMock.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity =0.8.12; + +import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; +import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol"; +import {BN254} from "src/libraries/BN254.sol"; + +/** + * @title A shared contract for EigenLayer operators to register their BLS public keys. + * @author Layr Labs, Inc. + * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service + */ +contract BLSApkRegistryMock is IBLSApkRegistry { + + /// @notice the hash of the zero pubkey aka BN254.G1Point(0,0) + bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; + + /// @notice maps operator address to pubkey hash + mapping(address => bytes32) public operatorToPubkeyHash; + /// @notice maps pubkey hash to operator address + mapping(bytes32 => address) public pubkeyHashToOperator; + /// @notice maps operator address to pubkeyG1 + mapping(address => BN254.G1Point) public operatorToPubkey; + + function registryCoordinator() external view returns (IRegistryCoordinator) {} + + /** + * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. + * @param operator The address of the operator to register. + * @param quorumNumbers The quorum numbers the operator is registering for, where each byte is an 8 bit integer quorumNumber. + * @dev access restricted to the RegistryCoordinator + * @dev Preconditions (these are assumed, not validated in this contract): + * 1) `quorumNumbers` has no duplicates + * 2) `quorumNumbers.length` != 0 + * 3) `quorumNumbers` is ordered in ascending order + * 4) the operator is not already registered + */ + function registerOperator(address operator, bytes calldata quorumNumbers) external returns(bytes32) {} + + /** + * @notice Deregisters the `operator`'s pubkey for the specified `quorumNumbers`. + * @param operator The address of the operator to deregister. + * @param quorumNumbers The quorum numbers the operator is deregistering from, where each byte is an 8 bit integer quorumNumber. + * @dev access restricted to the RegistryCoordinator + * @dev Preconditions (these are assumed, not validated in this contract): + * 1) `quorumNumbers` has no duplicates + * 2) `quorumNumbers.length` != 0 + * 3) `quorumNumbers` is ordered in ascending order + * 4) the operator is not already deregistered + * 5) `quorumNumbers` is a subset of the quorumNumbers that the operator is registered for + */ + function deregisterOperator(address operator, bytes calldata quorumNumbers) external {} + + /** + * @notice Initializes a new quorum by pushing its first apk update + * @param quorumNumber The number of the new quorum + */ + function initializeQuorum(uint8 quorumNumber) external {} + + /** + * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. + * @param signedMessageHash is the registration message hash signed by the private key of the operator + * @param pubkeyG1 is the corresponding G1 public key of the operator + * @param pubkeyG2 is the corresponding G2 public key of the operator + */ + function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external {} + + /** + * @notice Returns the message hash that an operator must sign to register their BLS public key. + * @param operator is the address of the operator registering their BLS public key + */ + function getMessageHash(address operator) external view returns (BN254.G1Point memory) {} + + /// @notice Returns the current APK for the provided `quorumNumber ` + function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory) {} + + /// @notice Returns the index of the quorumApk index at `blockNumber` for the provided `quorumNumber` + function getApkIndicesAtBlockNumber(bytes calldata quorumNumbers, uint256 blockNumber) external view returns(uint32[] memory) {} + + /// @notice Returns the `ApkUpdate` struct at `index` in the list of APK updates for the `quorumNumber` + function getApkUpdateAtIndex(uint8 quorumNumber, uint256 index) external view returns (ApkUpdate memory) {} + + /// @notice Returns the operator address for the given `pubkeyHash` + function getOperatorFromPubkeyHash(bytes32 pubkeyHash) external view returns (address) {} + + /** + * @notice get 24 byte hash of the apk of `quorumNumber` at `blockNumber` using the provided `index`; + * called by checkSignatures in BLSSignatureChecker.sol. + * @param quorumNumber is the quorum whose ApkHash is being retrieved + * @param blockNumber is the number of the block for which the latest ApkHash will be retrieved + * @param index is the index of the apkUpdate being retrieved from the list of quorum apkUpdates in storage + */ + function getApkHashAtBlockNumberAndIndex(uint8 quorumNumber, uint32 blockNumber, uint256 index) external view returns (bytes24) {} + + function getOperatorId(address operator) external view returns (bytes32) {} + + function registerPublicKey(BN254.G1Point memory pk) external { + + bytes32 pubkeyHash = BN254.hashG1Point(pk); + // store updates + operatorToPubkeyHash[msg.sender] = pubkeyHash; + pubkeyHashToOperator[pubkeyHash] = msg.sender; + operatorToPubkey[msg.sender] = pk; + } + + function setBLSPublicKey(address account, BN254.G1Point memory pk) external { + + bytes32 pubkeyHash = BN254.hashG1Point(pk); + // store updates + operatorToPubkeyHash[account] = pubkeyHash; + pubkeyHashToOperator[pubkeyHash] = account; + operatorToPubkey[account] = pk; + } + + function getRegisteredPubkey(address operator) public view returns (BN254.G1Point memory, bytes32) { + BN254.G1Point memory pubkey = operatorToPubkey[operator]; + bytes32 pubkeyHash = operatorToPubkeyHash[operator]; + + require( + pubkeyHash != bytes32(0), + "BLSPublicKeyCompendium.getRegisteredPubkey: operator is not registered" + ); + + return (pubkey, pubkeyHash); + } + +} diff --git a/test/mocks/BLSPublicKeyCompendiumMock.sol b/test/mocks/BLSPublicKeyCompendiumMock.sol deleted file mode 100644 index fa5c7867..00000000 --- a/test/mocks/BLSPublicKeyCompendiumMock.sol +++ /dev/null @@ -1,63 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity =0.8.12; - -import "src/interfaces/IBLSPublicKeyCompendium.sol"; -import "src/libraries/BN254.sol"; -/** - * @title A shared contract for EigenLayer operators to register their BLS public keys. - * @author Layr Labs, Inc. - * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service - */ -contract BLSPublicKeyCompendiumMock is IBLSPublicKeyCompendium{ - - /// @notice the hash of the zero pubkey aka BN254.G1Point(0,0) - bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; - - /// @notice maps operator address to pubkey hash - mapping(address => bytes32) public operatorToPubkeyHash; - /// @notice maps pubkey hash to operator address - mapping(bytes32 => address) public pubkeyHashToOperator; - /// @notice maps operator address to pubkeyG1 - mapping(address => BN254.G1Point) public operatorToPubkey; - - /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator - */ - function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external { - } - - function registerPublicKey(BN254.G1Point memory pk) external { - - bytes32 pubkeyHash = BN254.hashG1Point(pk); - // store updates - operatorToPubkeyHash[msg.sender] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = msg.sender; - operatorToPubkey[msg.sender] = pk; - } - - function setBLSPublicKey(address account, BN254.G1Point memory pk) external { - - bytes32 pubkeyHash = BN254.hashG1Point(pk); - // store updates - operatorToPubkeyHash[account] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = account; - operatorToPubkey[account] = pk; - } - - function getRegisteredPubkey(address operator) public view returns (BN254.G1Point memory, bytes32) { - BN254.G1Point memory pubkey = operatorToPubkey[operator]; - bytes32 pubkeyHash = operatorToPubkeyHash[operator]; - - require( - pubkeyHash != bytes32(0), - "BLSPublicKeyCompendium.getRegisteredPubkey: operator is not registered" - ); - - return (pubkey, pubkeyHash); - } - - function getMessageHash(address operator) external view returns (BN254.G1Point memory) {} -} diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index fe82c373..2e26086e 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -4,8 +4,6 @@ pragma solidity =0.8.12; import "forge-std/Test.sol"; import "src/BLSApkRegistry.sol"; -import "src/interfaces/IRegistryCoordinator.sol"; -import "test/mocks/BLSPublicKeyCompendiumMock.sol"; import "test/mocks/RegistryCoordinatorMock.sol"; @@ -19,11 +17,19 @@ contract BLSApkRegistryUnitTests is Test { bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; BLSApkRegistry public blsApkRegistry; - BLSPublicKeyCompendiumMock public pkCompendium; RegistryCoordinatorMock public registryCoordinator; BN254.G1Point internal defaultPubKey = BN254.G1Point(18260007818883133054078754218619977578772505796600400998181738095793040006897,3432351341799135763167709827653955074218841517684851694584291831827675065899); + BN254.G1Point pubKeyG1; + BN254.G2Point pubKeyG2; + BN254.G1Point signedMessageHash; + + address alice = address(1); + address bob = address(2); + + uint256 privKey = 9001; + uint8 internal defaultQuorumNumber = 0; // Track initialized quorums so we can filter these out when fuzzing @@ -31,8 +37,16 @@ contract BLSApkRegistryUnitTests is Test { function setUp() external { registryCoordinator = new RegistryCoordinatorMock(); - pkCompendium = new BLSPublicKeyCompendiumMock(); - blsApkRegistry = new BLSApkRegistry(registryCoordinator, pkCompendium); + blsApkRegistry = new BLSApkRegistry(registryCoordinator); + + pubKeyG1 = BN254.generatorG1().scalar_mul(privKey); + + //privKey*G2 + pubKeyG2.X[1] = 19101821850089705274637533855249918363070101489527618151493230256975900223847; + pubKeyG2.X[0] = 5334410886741819556325359147377682006012228123419628681352847439302316235957; + pubKeyG2.Y[1] = 354176189041917478648604979334478067325821134838555150300539079146482658331; + pubKeyG2.Y[0] = 4185483097059047421902184823581361466320657066600218863748375739772335928910; + // Initialize a quorum _initializeQuorum(defaultQuorumNumber); @@ -40,7 +54,6 @@ contract BLSApkRegistryUnitTests is Test { function testConstructorArgs() public view { require(blsApkRegistry.registryCoordinator() == registryCoordinator, "registryCoordinator not set correctly"); - require(blsApkRegistry.pubkeyCompendium() == pkCompendium, "pubkeyCompendium not set correctly"); } function testCallRegisterOperatorFromNonCoordinatorAddress(address nonCoordinatorAddress) public { @@ -63,7 +76,7 @@ contract BLSApkRegistryUnitTests is Test { function testOperatorDoesNotOwnPubKeyRegister() public { cheats.startPrank(address(registryCoordinator)); - cheats.expectRevert(bytes("BLSPublicKeyCompendium.getRegisteredPubkey: operator is not registered")); + cheats.expectRevert(bytes("BLSApkRegistry.getRegisteredPubkey: operator is not registered")); blsApkRegistry.registerOperator(defaultOperator, new bytes(1)); cheats.stopPrank(); } @@ -74,7 +87,8 @@ contract BLSApkRegistryUnitTests is Test { bytes32 pkHash = BN254.hashG1Point(pubkey); cheats.startPrank(operator); - pkCompendium.registerPublicKey(pubkey); + // TODO: fix this. it was using a mock contract but is now internal to this contract + // blsApkRegistry.registerBLSPublicKey(pubkey); cheats.stopPrank(); //register for one quorum @@ -111,7 +125,8 @@ contract BLSApkRegistryUnitTests is Test { } cheats.prank(defaultOperator); - pkCompendium.registerPublicKey(defaultPubKey); + // TODO: fix this. it was using a mock contract but is now internal to this contract + // blsApkRegistry.registerBLSPublicKey(defaultPubKey); cheats.prank(address(registryCoordinator)); blsApkRegistry.registerOperator(defaultOperator, quorumNumbers); @@ -136,7 +151,8 @@ contract BLSApkRegistryUnitTests is Test { quorumNumbers[0] = bytes1(defaultQuorumNumber); cheats.startPrank(operator); - pkCompendium.registerPublicKey(negatedQuorumApk); + // TODO: fix this. it was using a mock contract but is now internal to this contract + // blsApkRegistry.registerBLSPublicKey(negatedQuorumApk); cheats.stopPrank(); cheats.startPrank(address(registryCoordinator)); @@ -183,7 +199,8 @@ contract BLSApkRegistryUnitTests is Test { quorumNumbers[0] = bytes1(defaultQuorumNumber); cheats.startPrank(defaultOperator); - pkCompendium.registerPublicKey(quorumApksBefore); + // TODO: fix this. it was using a mock contract but is now internal to this contract + // blsApkRegistry.registerBLSPublicKey(quorumApksBefore); cheats.stopPrank(); cheats.prank(address(registryCoordinator)); @@ -277,4 +294,54 @@ contract BLSApkRegistryUnitTests is Test { cheats.stopPrank(); } + + // TODO: better organize / integrate tests migrated from `BLSPublicKeyCompendium` unit tests + function testRegisterBLSPublicKey() public { + signedMessageHash = _signMessage(alice); + vm.prank(alice); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + + assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); + assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); + } + + function testRegisterBLSPublicKey_NoMatch_Reverts() public { + signedMessageHash = _signMessage(alice); + BN254.G1Point memory badPubKeyG1 = BN254.generatorG1().scalar_mul(420); // mismatch public keys + + vm.prank(alice); + vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, badPubKeyG1, pubKeyG2); + } + + function testRegisterBLSPublicKey_BadSig_Reverts() public { + signedMessageHash = _signMessage(bob); // sign with wrong private key + + vm.prank(alice); + vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + } + + function testRegisterBLSPublicKey_OpRegistered_Reverts() public { + testRegisterBLSPublicKey(); // register alice + + vm.prank(alice); + vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey")); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + } + + function testRegisterBLSPublicKey_PkRegistered_Reverts() public { + testRegisterBLSPublicKey(); + signedMessageHash = _signMessage(bob); // same private key different operator + + vm.prank(bob); + vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: public key already registered")); + blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + } + + function _signMessage(address signer) internal view returns(BN254.G1Point memory) { + BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer); + return BN254.scalar_mul(messageHash, privKey); + } + } diff --git a/test/unit/BLSPublicKeyCompendiumUnit.t.sol b/test/unit/BLSPublicKeyCompendiumUnit.t.sol deleted file mode 100644 index 31ba0a95..00000000 --- a/test/unit/BLSPublicKeyCompendiumUnit.t.sol +++ /dev/null @@ -1,80 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity =0.8.12; - -import "forge-std/Test.sol"; -import "src/BLSPublicKeyCompendium.sol"; - -contract BLSPublicKeyCompendiumUnitTests is Test { - using BN254 for BN254.G1Point; - - BLSPublicKeyCompendium compendium; - uint256 privKey = 69; - - BN254.G1Point pubKeyG1; - BN254.G2Point pubKeyG2; - BN254.G1Point signedMessageHash; - - address alice = address(1); - address bob = address(2); - - function setUp() public { - compendium = new BLSPublicKeyCompendium(); - - pubKeyG1 = BN254.generatorG1().scalar_mul(privKey); - - //privKey*G2 - pubKeyG2.X[1] = 19101821850089705274637533855249918363070101489527618151493230256975900223847; - pubKeyG2.X[0] = 5334410886741819556325359147377682006012228123419628681352847439302316235957; - pubKeyG2.Y[1] = 354176189041917478648604979334478067325821134838555150300539079146482658331; - pubKeyG2.Y[0] = 4185483097059047421902184823581361466320657066600218863748375739772335928910; - } - - function testRegisterBLSPublicKey() public { - signedMessageHash = _signMessage(alice); - vm.prank(alice); - compendium.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); - - assertEq(compendium.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); - assertEq(compendium.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); - } - - function testRegisterBLSPublicKey_NoMatch_Reverts() public { - signedMessageHash = _signMessage(alice); - BN254.G1Point memory badPubKeyG1 = BN254.generatorG1().scalar_mul(420); // mismatch public keys - - vm.prank(alice); - vm.expectRevert(bytes("BLSPublicKeyCompendium.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - compendium.registerBLSPublicKey(signedMessageHash, badPubKeyG1, pubKeyG2); - } - - function testRegisterBLSPublicKey_BadSig_Reverts() public { - signedMessageHash = _signMessage(bob); // sign with wrong private key - - vm.prank(alice); - vm.expectRevert(bytes("BLSPublicKeyCompendium.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - compendium.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); - } - - function testRegisterBLSPublicKey_OpRegistered_Reverts() public { - testRegisterBLSPublicKey(); // register alice - - vm.prank(alice); - vm.expectRevert(bytes("BLSPublicKeyCompendium.registerBLSPublicKey: operator already registered pubkey")); - compendium.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); - } - - function testRegisterBLSPublicKey_PkRegistered_Reverts() public { - testRegisterBLSPublicKey(); - signedMessageHash = _signMessage(bob); // same private key different operator - - vm.prank(bob); - vm.expectRevert(bytes("BLSPublicKeyCompendium.registerBLSPublicKey: public key already registered")); - compendium.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); - } - - function _signMessage(address signer) internal view returns(BN254.G1Point memory) { - BN254.G1Point memory messageHash = compendium.getMessageHash(signer); - return BN254.scalar_mul(messageHash, privKey); - } - -} diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 1674d429..ac321395 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -319,7 +319,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { address operatorToRegister = _incrementAddress(defaultOperator, numOperators); BN254.G1Point memory operatorToRegisterPubKey = BN254.hashToG1(keccak256(abi.encodePacked(pseudoRandomNumber, numOperators))); - pubkeyCompendium.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); + blsApkRegistry.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, defaultStake); @@ -654,7 +654,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { }); } - pubkeyCompendium.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); + blsApkRegistry.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); uint96 registeringStake = defaultKickBIPsOfOperatorStake * defaultStake; stakeRegistry.setOperatorWeight(defaultQuorumNumber, operatorToRegister, registeringStake); @@ -946,6 +946,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { }); } - pubkeyCompendium.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); + blsApkRegistry.setBLSPublicKey(operatorToRegister, operatorToRegisterPubKey); } } diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index d8396966..1d975727 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -7,13 +7,11 @@ import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.so import {Slasher} from "eigenlayer-contracts/src/contracts/core/Slasher.sol"; import {ISlasher} from "eigenlayer-contracts/src/contracts/interfaces/ISlasher.sol"; import {PauserRegistry} from "eigenlayer-contracts/src/contracts/permissions/PauserRegistry.sol"; -import {IStrategyManager} from "eigenlayer-contracts/src/contracts/interfaces/IStrategyManager.sol"; import {IStrategy} from "eigenlayer-contracts/src/contracts/interfaces/IStrategy.sol"; import {ISignatureUtils} from "eigenlayer-contracts/src/contracts/interfaces/ISignatureUtils.sol"; import {BitmapUtils} from "src/libraries/BitmapUtils.sol"; import {BN254} from "src/libraries/BN254.sol"; -import {BLSPublicKeyCompendium} from "src/BLSPublicKeyCompendium.sol"; import {OperatorStateRetriever} from "src/OperatorStateRetriever.sol"; import {RegistryCoordinator} from "src/RegistryCoordinator.sol"; import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.sol"; @@ -30,10 +28,8 @@ import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; import {StrategyManagerMock} from "eigenlayer-contracts/src/test/mocks/StrategyManagerMock.sol"; import {EigenPodManagerMock} from "eigenlayer-contracts/src/test/mocks/EigenPodManagerMock.sol"; import {ServiceManagerMock} from "test/mocks/ServiceManagerMock.sol"; -import {OwnableMock} from "eigenlayer-contracts/src/test/mocks/OwnableMock.sol"; import {DelegationManagerMock} from "eigenlayer-contracts/src/test/mocks/DelegationManagerMock.sol"; -import {SlasherMock} from "eigenlayer-contracts/src/test/mocks/SlasherMock.sol"; -import {BLSPublicKeyCompendiumMock} from "test/mocks/BLSPublicKeyCompendiumMock.sol"; +import {BLSApkRegistryMock} from "test/mocks/BLSApkRegistryMock.sol"; import {EmptyContract} from "eigenlayer-contracts/src/test/mocks/EmptyContract.sol"; import {StakeRegistryHarness} from "test/harnesses/StakeRegistryHarness.sol"; @@ -52,7 +48,6 @@ contract MockAVSDeployer is Test { Slasher public slasherImplementation; EmptyContract public emptyContract; - BLSPublicKeyCompendiumMock public pubkeyCompendium; RegistryCoordinatorHarness public registryCoordinatorImplementation; StakeRegistryHarness public stakeRegistryImplementation; @@ -62,7 +57,7 @@ contract MockAVSDeployer is Test { OperatorStateRetriever public operatorStateRetriever; RegistryCoordinatorHarness public registryCoordinator; StakeRegistryHarness public stakeRegistry; - IBLSApkRegistry public blsApkRegistry; + BLSApkRegistryMock public blsApkRegistry; IIndexRegistry public indexRegistry; ServiceManagerMock public serviceManagerMock; @@ -146,8 +141,8 @@ contract MockAVSDeployer is Test { slasher ); - pubkeyCompendium = new BLSPublicKeyCompendiumMock(); - pubkeyCompendium.setBLSPublicKey(defaultOperator, defaultPubKey); + blsApkRegistry = new BLSApkRegistryMock(); + blsApkRegistry.setBLSPublicKey(defaultOperator, defaultPubKey); cheats.stopPrank(); @@ -182,7 +177,7 @@ contract MockAVSDeployer is Test { ) ); - blsApkRegistry = BLSApkRegistry( + blsApkRegistry = BLSApkRegistryMock( address( new TransparentUpgradeableProxy( address(emptyContract), @@ -207,8 +202,7 @@ contract MockAVSDeployer is Test { ); blsApkRegistryImplementation = new BLSApkRegistry( - registryCoordinator, - BLSPublicKeyCompendium(address(pubkeyCompendium)) + registryCoordinator ); proxyAdmin.upgrade( @@ -295,7 +289,7 @@ contract MockAVSDeployer is Test { // quorumBitmap can only have 192 least significant bits quorumBitmap &= MAX_QUORUM_BITMAP; - pubkeyCompendium.setBLSPublicKey(operator, pubKey); + blsApkRegistry.setBLSPublicKey(operator, pubKey); bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { @@ -313,7 +307,7 @@ contract MockAVSDeployer is Test { // quorumBitmap can only have 192 least significant bits quorumBitmap &= MAX_QUORUM_BITMAP; - pubkeyCompendium.setBLSPublicKey(operator, pubKey); + blsApkRegistry.setBLSPublicKey(operator, pubKey); bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { From 1aa538d7a20cb43749984bdca8a34f2286e1221f Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 13:12:44 -0800 Subject: [PATCH 03/25] feat: create harness for BLSApkRegistry and use it in tests These changes appear to fix all existing tests. I've done what I can to at least comment specifically where harnessed functions are used, as this pattern could give some false assurance and I'd like to avoid that. --- test/harnesses/BLSApkRegistryHarness.sol | 21 ++++++++++++++++++ test/mocks/BLSApkRegistryMock.sol | 11 +++++++++- test/unit/BLSApkRegistryUnit.t.sol | 28 ++++++++++-------------- test/utils/MockAVSDeployer.sol | 16 +++++++------- 4 files changed, 51 insertions(+), 25 deletions(-) create mode 100644 test/harnesses/BLSApkRegistryHarness.sol diff --git a/test/harnesses/BLSApkRegistryHarness.sol b/test/harnesses/BLSApkRegistryHarness.sol new file mode 100644 index 00000000..a888986e --- /dev/null +++ b/test/harnesses/BLSApkRegistryHarness.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity =0.8.12; + +import "src/BLSApkRegistry.sol"; + +// wrapper around the BLSApkRegistry contract that exposes internal functionality, for unit testing _other functionality_. +contract BLSApkRegistryHarness is BLSApkRegistry { + + constructor( + IRegistryCoordinator _registryCoordinator + ) BLSApkRegistry(_registryCoordinator) {} + + function setBLSPublicKey(address account, BN254.G1Point memory pk) external { + + bytes32 pubkeyHash = BN254.hashG1Point(pk); + // store updates + operatorToPubkeyHash[account] = pubkeyHash; + pubkeyHashToOperator[pubkeyHash] = account; + operatorToPubkey[account] = pk; + } +} diff --git a/test/mocks/BLSApkRegistryMock.sol b/test/mocks/BLSApkRegistryMock.sol index 61cb3d95..1f050f4c 100644 --- a/test/mocks/BLSApkRegistryMock.sol +++ b/test/mocks/BLSApkRegistryMock.sol @@ -90,7 +90,16 @@ contract BLSApkRegistryMock is IBLSApkRegistry { * @param blockNumber is the number of the block for which the latest ApkHash will be retrieved * @param index is the index of the apkUpdate being retrieved from the list of quorum apkUpdates in storage */ - function getApkHashAtBlockNumberAndIndex(uint8 quorumNumber, uint32 blockNumber, uint256 index) external view returns (bytes24) {} + function getApkHashAtBlockNumberAndIndex( + uint8 quorumNumber, + uint32 blockNumber, + uint256 index + ) external view returns (bytes24) { + // ApkUpdate memory quorumApkUpdate = apkHistory[quorumNumber][index]; + // _validateApkHashAtBlockNumber(quorumApkUpdate, blockNumber); + ApkUpdate memory quorumApkUpdate; + return quorumApkUpdate.apkHash; + } function getOperatorId(address operator) external view returns (bytes32) {} diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index 2e26086e..c30e1bdd 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -3,7 +3,7 @@ pragma solidity =0.8.12; import "forge-std/Test.sol"; -import "src/BLSApkRegistry.sol"; +import "test/harnesses/BLSApkRegistryHarness.sol"; import "test/mocks/RegistryCoordinatorMock.sol"; @@ -16,7 +16,7 @@ contract BLSApkRegistryUnitTests is Test { bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; - BLSApkRegistry public blsApkRegistry; + BLSApkRegistryHarness public blsApkRegistry; RegistryCoordinatorMock public registryCoordinator; BN254.G1Point internal defaultPubKey = BN254.G1Point(18260007818883133054078754218619977578772505796600400998181738095793040006897,3432351341799135763167709827653955074218841517684851694584291831827675065899); @@ -28,7 +28,7 @@ contract BLSApkRegistryUnitTests is Test { address alice = address(1); address bob = address(2); - uint256 privKey = 9001; + uint256 privKey = 69; uint8 internal defaultQuorumNumber = 0; @@ -37,7 +37,7 @@ contract BLSApkRegistryUnitTests is Test { function setUp() external { registryCoordinator = new RegistryCoordinatorMock(); - blsApkRegistry = new BLSApkRegistry(registryCoordinator); + blsApkRegistry = new BLSApkRegistryHarness(registryCoordinator); pubKeyG1 = BN254.generatorG1().scalar_mul(privKey); @@ -86,9 +86,8 @@ contract BLSApkRegistryUnitTests is Test { BN254.G1Point memory pubkey = BN254.hashToG1(x); bytes32 pkHash = BN254.hashG1Point(pubkey); - cheats.startPrank(operator); - // TODO: fix this. it was using a mock contract but is now internal to this contract - // blsApkRegistry.registerBLSPublicKey(pubkey); + // use harnessed function to directly set the pubkey, bypassing the ordinary checks + blsApkRegistry.setBLSPublicKey(operator, pubkey); cheats.stopPrank(); //register for one quorum @@ -124,9 +123,8 @@ contract BLSApkRegistryUnitTests is Test { quorumApksBefore[i] = blsApkRegistry.getApk(uint8(quorumNumbers[i])); } - cheats.prank(defaultOperator); - // TODO: fix this. it was using a mock contract but is now internal to this contract - // blsApkRegistry.registerBLSPublicKey(defaultPubKey); + // use harnessed function to directly set the pubkey, bypassing the ordinary checks + blsApkRegistry.setBLSPublicKey(defaultOperator, defaultPubKey); cheats.prank(address(registryCoordinator)); blsApkRegistry.registerOperator(defaultOperator, quorumNumbers); @@ -150,9 +148,8 @@ contract BLSApkRegistryUnitTests is Test { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - cheats.startPrank(operator); - // TODO: fix this. it was using a mock contract but is now internal to this contract - // blsApkRegistry.registerBLSPublicKey(negatedQuorumApk); + // use harnessed function to directly set the pubkey, bypassing the ordinary checks + blsApkRegistry.setBLSPublicKey(operator, negatedQuorumApk); cheats.stopPrank(); cheats.startPrank(address(registryCoordinator)); @@ -198,9 +195,8 @@ contract BLSApkRegistryUnitTests is Test { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - cheats.startPrank(defaultOperator); - // TODO: fix this. it was using a mock contract but is now internal to this contract - // blsApkRegistry.registerBLSPublicKey(quorumApksBefore); + // use harnessed function to directly set the pubkey, bypassing the ordinary checks + blsApkRegistry.setBLSPublicKey(defaultOperator, quorumApksBefore); cheats.stopPrank(); cheats.prank(address(registryCoordinator)); diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index 1d975727..c40de9a9 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -29,7 +29,7 @@ import {StrategyManagerMock} from "eigenlayer-contracts/src/test/mocks/StrategyM import {EigenPodManagerMock} from "eigenlayer-contracts/src/test/mocks/EigenPodManagerMock.sol"; import {ServiceManagerMock} from "test/mocks/ServiceManagerMock.sol"; import {DelegationManagerMock} from "eigenlayer-contracts/src/test/mocks/DelegationManagerMock.sol"; -import {BLSApkRegistryMock} from "test/mocks/BLSApkRegistryMock.sol"; +import {BLSApkRegistryHarness} from "test/harnesses/BLSApkRegistryHarness.sol"; import {EmptyContract} from "eigenlayer-contracts/src/test/mocks/EmptyContract.sol"; import {StakeRegistryHarness} from "test/harnesses/StakeRegistryHarness.sol"; @@ -57,7 +57,7 @@ contract MockAVSDeployer is Test { OperatorStateRetriever public operatorStateRetriever; RegistryCoordinatorHarness public registryCoordinator; StakeRegistryHarness public stakeRegistry; - BLSApkRegistryMock public blsApkRegistry; + BLSApkRegistryHarness public blsApkRegistry; IIndexRegistry public indexRegistry; ServiceManagerMock public serviceManagerMock; @@ -140,10 +140,6 @@ contract MockAVSDeployer is Test { eigenPodManagerMock, slasher ); - - blsApkRegistry = new BLSApkRegistryMock(); - blsApkRegistry.setBLSPublicKey(defaultOperator, defaultPubKey); - cheats.stopPrank(); cheats.startPrank(serviceManagerOwner); @@ -177,7 +173,7 @@ contract MockAVSDeployer is Test { ) ); - blsApkRegistry = BLSApkRegistryMock( + blsApkRegistry = BLSApkRegistryHarness( address( new TransparentUpgradeableProxy( address(emptyContract), @@ -188,6 +184,7 @@ contract MockAVSDeployer is Test { ); cheats.stopPrank(); + cheats.startPrank(proxyAdminOwner); stakeRegistryImplementation = new StakeRegistryHarness( @@ -201,7 +198,7 @@ contract MockAVSDeployer is Test { address(stakeRegistryImplementation) ); - blsApkRegistryImplementation = new BLSApkRegistry( + blsApkRegistryImplementation = new BLSApkRegistryHarness( registryCoordinator ); @@ -219,6 +216,9 @@ contract MockAVSDeployer is Test { address(indexRegistryImplementation) ); + // set the public key for an operator, using harnessed function to bypass checks + blsApkRegistry.setBLSPublicKey(defaultOperator, defaultPubKey); + // setup the dummy minimum stake for quorum uint96[] memory minimumStakeForQuorum = new uint96[](numQuorumsToAdd); for (uint256 i = 0; i < minimumStakeForQuorum.length; i++) { From f42ff73761dfe18b7b26c67fdf0c9d5957aafe07 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 13:14:28 -0800 Subject: [PATCH 04/25] chore: delete unused mock file I created this mock while trying to sort out compilation issues and test failures This mock is no longer being used anywhere, so I am deleting it for cleanup --- test/mocks/BLSApkRegistryMock.sol | 136 ------------------------------ 1 file changed, 136 deletions(-) delete mode 100644 test/mocks/BLSApkRegistryMock.sol diff --git a/test/mocks/BLSApkRegistryMock.sol b/test/mocks/BLSApkRegistryMock.sol deleted file mode 100644 index 1f050f4c..00000000 --- a/test/mocks/BLSApkRegistryMock.sol +++ /dev/null @@ -1,136 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity =0.8.12; - -import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; -import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol"; -import {BN254} from "src/libraries/BN254.sol"; - -/** - * @title A shared contract for EigenLayer operators to register their BLS public keys. - * @author Layr Labs, Inc. - * @notice Terms of Service: https://docs.eigenlayer.xyz/overview/terms-of-service - */ -contract BLSApkRegistryMock is IBLSApkRegistry { - - /// @notice the hash of the zero pubkey aka BN254.G1Point(0,0) - bytes32 internal constant ZERO_PK_HASH = hex"ad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5"; - - /// @notice maps operator address to pubkey hash - mapping(address => bytes32) public operatorToPubkeyHash; - /// @notice maps pubkey hash to operator address - mapping(bytes32 => address) public pubkeyHashToOperator; - /// @notice maps operator address to pubkeyG1 - mapping(address => BN254.G1Point) public operatorToPubkey; - - function registryCoordinator() external view returns (IRegistryCoordinator) {} - - /** - * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. - * @param operator The address of the operator to register. - * @param quorumNumbers The quorum numbers the operator is registering for, where each byte is an 8 bit integer quorumNumber. - * @dev access restricted to the RegistryCoordinator - * @dev Preconditions (these are assumed, not validated in this contract): - * 1) `quorumNumbers` has no duplicates - * 2) `quorumNumbers.length` != 0 - * 3) `quorumNumbers` is ordered in ascending order - * 4) the operator is not already registered - */ - function registerOperator(address operator, bytes calldata quorumNumbers) external returns(bytes32) {} - - /** - * @notice Deregisters the `operator`'s pubkey for the specified `quorumNumbers`. - * @param operator The address of the operator to deregister. - * @param quorumNumbers The quorum numbers the operator is deregistering from, where each byte is an 8 bit integer quorumNumber. - * @dev access restricted to the RegistryCoordinator - * @dev Preconditions (these are assumed, not validated in this contract): - * 1) `quorumNumbers` has no duplicates - * 2) `quorumNumbers.length` != 0 - * 3) `quorumNumbers` is ordered in ascending order - * 4) the operator is not already deregistered - * 5) `quorumNumbers` is a subset of the quorumNumbers that the operator is registered for - */ - function deregisterOperator(address operator, bytes calldata quorumNumbers) external {} - - /** - * @notice Initializes a new quorum by pushing its first apk update - * @param quorumNumber The number of the new quorum - */ - function initializeQuorum(uint8 quorumNumber) external {} - - /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator - */ - function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external {} - - /** - * @notice Returns the message hash that an operator must sign to register their BLS public key. - * @param operator is the address of the operator registering their BLS public key - */ - function getMessageHash(address operator) external view returns (BN254.G1Point memory) {} - - /// @notice Returns the current APK for the provided `quorumNumber ` - function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory) {} - - /// @notice Returns the index of the quorumApk index at `blockNumber` for the provided `quorumNumber` - function getApkIndicesAtBlockNumber(bytes calldata quorumNumbers, uint256 blockNumber) external view returns(uint32[] memory) {} - - /// @notice Returns the `ApkUpdate` struct at `index` in the list of APK updates for the `quorumNumber` - function getApkUpdateAtIndex(uint8 quorumNumber, uint256 index) external view returns (ApkUpdate memory) {} - - /// @notice Returns the operator address for the given `pubkeyHash` - function getOperatorFromPubkeyHash(bytes32 pubkeyHash) external view returns (address) {} - - /** - * @notice get 24 byte hash of the apk of `quorumNumber` at `blockNumber` using the provided `index`; - * called by checkSignatures in BLSSignatureChecker.sol. - * @param quorumNumber is the quorum whose ApkHash is being retrieved - * @param blockNumber is the number of the block for which the latest ApkHash will be retrieved - * @param index is the index of the apkUpdate being retrieved from the list of quorum apkUpdates in storage - */ - function getApkHashAtBlockNumberAndIndex( - uint8 quorumNumber, - uint32 blockNumber, - uint256 index - ) external view returns (bytes24) { - // ApkUpdate memory quorumApkUpdate = apkHistory[quorumNumber][index]; - // _validateApkHashAtBlockNumber(quorumApkUpdate, blockNumber); - ApkUpdate memory quorumApkUpdate; - return quorumApkUpdate.apkHash; - } - - function getOperatorId(address operator) external view returns (bytes32) {} - - function registerPublicKey(BN254.G1Point memory pk) external { - - bytes32 pubkeyHash = BN254.hashG1Point(pk); - // store updates - operatorToPubkeyHash[msg.sender] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = msg.sender; - operatorToPubkey[msg.sender] = pk; - } - - function setBLSPublicKey(address account, BN254.G1Point memory pk) external { - - bytes32 pubkeyHash = BN254.hashG1Point(pk); - // store updates - operatorToPubkeyHash[account] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = account; - operatorToPubkey[account] = pk; - } - - function getRegisteredPubkey(address operator) public view returns (BN254.G1Point memory, bytes32) { - BN254.G1Point memory pubkey = operatorToPubkey[operator]; - bytes32 pubkeyHash = operatorToPubkeyHash[operator]; - - require( - pubkeyHash != bytes32(0), - "BLSPublicKeyCompendium.getRegisteredPubkey: operator is not registered" - ); - - return (pubkey, pubkeyHash); - } - -} From 89d815a0da618cdb583896520b1c1f241c7586db Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 13:23:51 -0800 Subject: [PATCH 05/25] chore: cleanup unused file and reduce mutability of test utils functions this commit fixes a ton of compiler warnings on build, in particular. --- test/mocks/MiddlewareRegistryMock.sol | 39 --------------------------- test/mocks/StakeRegistryMock.sol | 2 +- test/utils/Operators.sol | 14 +++++----- test/utils/Owners.sol | 4 +-- test/utils/ProofParsing.sol | 26 +++++++++--------- 5 files changed, 23 insertions(+), 62 deletions(-) delete mode 100644 test/mocks/MiddlewareRegistryMock.sol diff --git a/test/mocks/MiddlewareRegistryMock.sol b/test/mocks/MiddlewareRegistryMock.sol deleted file mode 100644 index be13d43a..00000000 --- a/test/mocks/MiddlewareRegistryMock.sol +++ /dev/null @@ -1,39 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity =0.8.12; - -import "src/interfaces/IServiceManager.sol"; -import "eigenlayer-contracts/src/contracts/interfaces/IStrategyManager.sol"; -import "eigenlayer-contracts/src/contracts/interfaces/ISlasher.sol"; - -contract MiddlewareRegistryMock{ - IServiceManager public serviceManager; - IStrategyManager public strategyManager; - ISlasher public slasher; - - - constructor( - IServiceManager _serviceManager, - IStrategyManager _strategyManager - ) { - serviceManager = _serviceManager; - strategyManager = _strategyManager; - slasher = _strategyManager.slasher(); - } - - function registerOperator(address operator, uint32 serveUntil) public { - require(slasher.canSlash(operator, address(serviceManager)), "Not opted into slashing"); - - } - - function deregisterOperator(address operator) public { - } - - function isActiveOperator(address operator) external pure returns (bool) { - if (operator != address(0)) { - return true; - } else { - return false; - } - } - -} diff --git a/test/mocks/StakeRegistryMock.sol b/test/mocks/StakeRegistryMock.sol index b3c3e679..51934308 100644 --- a/test/mocks/StakeRegistryMock.sol +++ b/test/mocks/StakeRegistryMock.sol @@ -201,7 +201,7 @@ contract StakeRegistryMock is IStakeRegistry { bytes calldata quorumNumbers ) external returns (uint192) {} - function getMockOperatorId(address operator) external returns(bytes32) { + function getMockOperatorId(address operator) external pure returns(bytes32) { return bytes32(uint256(keccak256(abi.encodePacked(operator, "operatorId")))); } } diff --git a/test/utils/Operators.sol b/test/utils/Operators.sol index 910165e7..5bc607f7 100644 --- a/test/utils/Operators.sol +++ b/test/utils/Operators.sol @@ -16,15 +16,15 @@ contract Operators is Test { return string.concat(".operators[", string.concat(vm.toString(index), "].")); } - function getNumOperators() public returns(uint256) { + function getNumOperators() public view returns(uint256) { return stdJson.readUint(operatorConfigJson, ".numOperators"); } - function getOperatorAddress(uint256 index) public returns(address) { + function getOperatorAddress(uint256 index) public view returns(address) { return stdJson.readAddress(operatorConfigJson, string.concat(operatorPrefix(index), "Address")); } - function getOperatorSchnorrSignature(uint256 index) public returns(uint256, BN254.G1Point memory) { + function getOperatorSchnorrSignature(uint256 index) public view returns(uint256, BN254.G1Point memory) { uint256 s = readUint(operatorConfigJson, index, "SField"); BN254.G1Point memory pubkey = BN254.G1Point({ X: readUint(operatorConfigJson, index, "RPoint.X"), @@ -33,11 +33,11 @@ contract Operators is Test { return (s, pubkey); } - function getOperatorSecretKey(uint256 index) public returns(uint256) { + function getOperatorSecretKey(uint256 index) public view returns(uint256) { return readUint(operatorConfigJson, index, "SecretKey"); } - function getOperatorPubkeyG1(uint256 index) public returns(BN254.G1Point memory) { + function getOperatorPubkeyG1(uint256 index) public view returns(BN254.G1Point memory) { BN254.G1Point memory pubkey = BN254.G1Point({ X: readUint(operatorConfigJson, index, "PubkeyG1.X"), Y: readUint(operatorConfigJson, index, "PubkeyG1.Y") @@ -45,7 +45,7 @@ contract Operators is Test { return pubkey; } - function getOperatorPubkeyG2(uint256 index) public returns(BN254.G2Point memory) { + function getOperatorPubkeyG2(uint256 index) public view returns(BN254.G2Point memory) { BN254.G2Point memory pubkey = BN254.G2Point({ X: [ readUint(operatorConfigJson, index, "PubkeyG2.X.A1"), @@ -59,7 +59,7 @@ contract Operators is Test { return pubkey; } - function readUint(string memory json, uint256 index, string memory key) public returns (uint256) { + function readUint(string memory json, uint256 index, string memory key) public pure returns (uint256) { return stringToUint(stdJson.readString(json, string.concat(operatorPrefix(index), key))); } diff --git a/test/utils/Owners.sol b/test/utils/Owners.sol index 5f93d50b..0a0643e7 100644 --- a/test/utils/Owners.sol +++ b/test/utils/Owners.sol @@ -17,11 +17,11 @@ contract Owners is Test { return string.concat(".owners[", string.concat(vm.toString(index), "].")); } - function getNumOperators() public returns(uint256) { + function getNumOperators() public view returns(uint256) { return stdJson.readUint(ownersConfigJson, ".numOwners"); } - function getOwnerAddress(uint256 index) public returns(address) { + function getOwnerAddress(uint256 index) public view returns(address) { return stdJson.readAddress(ownersConfigJson, string.concat(ownerPrefix(index), "Address")); } diff --git a/test/utils/ProofParsing.sol b/test/utils/ProofParsing.sol index f73f74b8..ddc22259 100644 --- a/test/utils/ProofParsing.sol +++ b/test/utils/ProofParsing.sol @@ -29,55 +29,55 @@ contract ProofParsing is Test{ proofConfigJson = vm.readFile(path); } - function getSlot() public returns(uint256) { + function getSlot() public view returns(uint256) { return stdJson.readUint(proofConfigJson, ".slot"); } - function getValidatorIndex() public returns(uint256) { + function getValidatorIndex() public view returns(uint256) { return stdJson.readUint(proofConfigJson, ".validatorIndex"); } - function getValidatorPubkeyHash() public returns(bytes32) { + function getValidatorPubkeyHash() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".ValidatorFields[0]"); } - function getWithdrawalIndex() public returns(uint256) { + function getWithdrawalIndex() public view returns(uint256) { return stdJson.readUint(proofConfigJson, ".withdrawalIndex"); } - function getBlockRootIndex() public returns(uint256) { + function getBlockRootIndex() public view returns(uint256) { return stdJson.readUint(proofConfigJson, ".blockHeaderRootIndex"); } - function getHistoricalSummaryIndex() public returns(uint256) { + function getHistoricalSummaryIndex() public view returns(uint256) { return stdJson.readUint(proofConfigJson, ".historicalSummaryIndex"); } - function getBeaconStateRoot() public returns(bytes32) { + function getBeaconStateRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".beaconStateRoot"); } - function getBlockRoot() public returns(bytes32) { + function getBlockRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".blockHeaderRoot"); } - function getSlotRoot() public returns(bytes32) { + function getSlotRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".slotRoot"); } - function getBalanceRoot() public returns(bytes32) { + function getBalanceRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".balanceRoot"); } - function getTimestampRoot() public returns(bytes32) { + function getTimestampRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".timestampRoot"); } - function getExecutionPayloadRoot() public returns(bytes32) { + function getExecutionPayloadRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".executionPayloadRoot"); } - function getLatestBlockRoot() public returns(bytes32) { + function getLatestBlockRoot() public view returns(bytes32) { return stdJson.readBytes32(proofConfigJson, ".latestBlockHeaderRoot"); } function getExecutionPayloadProof () public returns(bytes32[7] memory) { From ec18a4ca05b5bf78d3ed29912061592e895f61f0 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 15:27:38 -0800 Subject: [PATCH 06/25] feat: make BLSApkRegistry. registerBLSPublicKey only callable by RegistryCoordinator also make a version of `RegistryCoordinator. registerOperator` that accepts these inputs new function passes these inputs on appropriately TODO: remove old `registerOperator` function and fix associated tests --- src/BLSApkRegistry.sol | 18 +++++----- src/RegistryCoordinator.sol | 47 +++++++++++++++++++++++++++ src/interfaces/IBLSApkRegistry.sol | 10 ++++-- test/ffi/BLSPubKeyCompendiumFFI.t.sol | 6 ++-- test/unit/BLSApkRegistryUnit.t.sol | 20 ++++++------ 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 99f48288..880ef9c2 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -94,22 +94,24 @@ contract BLSApkRegistry is BLSApkRegistryStorage { } /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. + * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. + * @param operator is the operator for whom the key is being registered * @param signedMessageHash is the registration message hash signed by the private key of the operator * @param pubkeyG1 is the corresponding G1 public key of the operator * @param pubkeyG2 is the corresponding G2 public key of the operator */ function registerBLSPublicKey( + address operator, BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2 - ) external { + ) external onlyRegistryCoordinator { bytes32 pubkeyHash = BN254.hashG1Point(pubkeyG1); require( pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey" ); require( - operatorToPubkeyHash[msg.sender] == bytes32(0), + operatorToPubkeyHash[operator] == bytes32(0), "BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey" ); require( @@ -118,7 +120,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { ); // H(m) - BN254.G1Point memory messageHash = getMessageHash(msg.sender); + BN254.G1Point memory messageHash = getMessageHash(operator); // gamma = h(sigma, P, P', H(m)) uint256 gamma = uint256(keccak256(abi.encodePacked( @@ -140,11 +142,11 @@ contract BLSApkRegistry is BLSApkRegistryStorage { pubkeyG2 ), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); - operatorToPubkey[msg.sender] = pubkeyG1; - operatorToPubkeyHash[msg.sender] = pubkeyHash; - pubkeyHashToOperator[pubkeyHash] = msg.sender; + operatorToPubkey[operator] = pubkeyG1; + operatorToPubkeyHash[operator] = pubkeyHash; + pubkeyHashToOperator[pubkeyHash] = operator; - emit NewPubkeyRegistration(msg.sender, pubkeyG1, pubkeyG2); + emit NewPubkeyRegistration(operator, pubkeyG1, pubkeyG2); } /******************************************************************************* diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 615879f3..9a2b034f 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -18,6 +18,7 @@ import {IStakeRegistry} from "src/interfaces/IStakeRegistry.sol"; import {IIndexRegistry} from "src/interfaces/IIndexRegistry.sol"; import {BitmapUtils} from "src/libraries/BitmapUtils.sol"; +import {BN254} from "src/libraries/BN254.sol"; /** * @title A `RegistryCoordinator` that has three registries: @@ -29,6 +30,7 @@ import {BitmapUtils} from "src/libraries/BitmapUtils.sol"; */ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISocketUpdater, ISignatureUtils, Pausable { using BitmapUtils for *; + 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 = @@ -141,6 +143,51 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo EXTERNAL FUNCTIONS *******************************************************************************/ + /** + * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum + * operator capacity, this method will fail. + * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for + */ + function registerOperator( + bytes calldata quorumNumbers, + string calldata socket, + BN254.G1Point memory pubkeyRegistrationSignature, + BN254.G1Point memory pubkeyG1, + BN254.G2Point memory pubkeyG2 + ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { + /** + * IF the operator has never registered a pubkey before, THEN register their pubkey + * OTHERWISE, simply ignore the `pubkeyRegistrationSignature`, `pubkeyG1`, and `pubkeyG2` inputs + */ + if (blsApkRegistry.operatorToPubkeyHash(msg.sender) == 0) { + blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + } + bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); + + // Register the operator in each of the registry contracts + RegisterResults memory results = _registerOperator({ + operator: msg.sender, + operatorId: operatorId, + quorumNumbers: quorumNumbers, + socket: socket + }); + + for (uint256 i = 0; i < quorumNumbers.length; i++) { + uint8 quorumNumber = uint8(quorumNumbers[i]); + + OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; + + /** + * The new operator count for each quorum may not exceed the configured maximum + * If it does, use `registerOperatorWithChurn` instead. + */ + require( + results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, + "RegistryCoordinator.registerOperator: operator count exceeds maximum" + ); + } + } + /** * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum * operator capacity, this method will fail. diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 89550777..af241658 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -83,12 +83,18 @@ interface IBLSApkRegistry is IRegistry { function pubkeyHashToOperator(bytes32 pubkeyHash) external view returns (address); /** - * @notice Called by an operator to register themselves as the owner of a BLS public key and reveal their G1 and G2 public key. + * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. + * @param operator is the operator for whom the key is being registered * @param signedMessageHash is the registration message hash signed by the private key of the operator * @param pubkeyG1 is the corresponding G1 public key of the operator * @param pubkeyG2 is the corresponding G2 public key of the operator */ - function registerBLSPublicKey(BN254.G1Point memory signedMessageHash, BN254.G1Point memory pubkeyG1, BN254.G2Point memory pubkeyG2) external; + function registerBLSPublicKey( + address operator, + BN254.G1Point memory signedMessageHash, + BN254.G1Point memory pubkeyG1, + BN254.G2Point memory pubkeyG2 + ) external; /** * @notice Returns the pubkey and pubkey hash of an operator diff --git a/test/ffi/BLSPubKeyCompendiumFFI.t.sol b/test/ffi/BLSPubKeyCompendiumFFI.t.sol index 8a1a5a99..39121a96 100644 --- a/test/ffi/BLSPubKeyCompendiumFFI.t.sol +++ b/test/ffi/BLSPubKeyCompendiumFFI.t.sol @@ -11,6 +11,7 @@ contract BLSApkRegistryFFITests is G2Operations { Vm cheats = Vm(HEVM_ADDRESS); BLSApkRegistry blsApkRegistry; + IRegistryCoordinator registryCoordinator; uint256 privKey; BN254.G1Point pubKeyG1; @@ -20,7 +21,6 @@ contract BLSApkRegistryFFITests is G2Operations { address alice = address(0x69); function setUp() public { - IRegistryCoordinator registryCoordinator; blsApkRegistry = new BLSApkRegistry(registryCoordinator); } @@ -30,8 +30,8 @@ contract BLSApkRegistryFFITests is G2Operations { signedMessageHash = _signMessage(alice); - vm.prank(alice); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + vm.prank(address(registryCoordinator)); + blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index c30e1bdd..493a44d4 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -294,8 +294,8 @@ contract BLSApkRegistryUnitTests is Test { // TODO: better organize / integrate tests migrated from `BLSPublicKeyCompendium` unit tests function testRegisterBLSPublicKey() public { signedMessageHash = _signMessage(alice); - vm.prank(alice); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + vm.prank(address(registryCoordinator)); + blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); @@ -305,34 +305,34 @@ contract BLSApkRegistryUnitTests is Test { signedMessageHash = _signMessage(alice); BN254.G1Point memory badPubKeyG1 = BN254.generatorG1().scalar_mul(420); // mismatch public keys - vm.prank(alice); + vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, badPubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, badPubKeyG1, pubKeyG2); } function testRegisterBLSPublicKey_BadSig_Reverts() public { signedMessageHash = _signMessage(bob); // sign with wrong private key - vm.prank(alice); + vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); } function testRegisterBLSPublicKey_OpRegistered_Reverts() public { testRegisterBLSPublicKey(); // register alice - vm.prank(alice); + vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey")); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); } function testRegisterBLSPublicKey_PkRegistered_Reverts() public { testRegisterBLSPublicKey(); signedMessageHash = _signMessage(bob); // same private key different operator - vm.prank(bob); + vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: public key already registered")); - blsApkRegistry.registerBLSPublicKey(signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(bob, signedMessageHash, pubKeyG1, pubKeyG2); } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { From 5a022ae081c1b0ac3ee2ac8ca86631294efe49ea Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 16:16:19 -0800 Subject: [PATCH 07/25] chore: remove `RegistryCoordinator.registerOperator` method without pubkey registration inputs also fix tests to use the new method --- src/RegistryCoordinator.sol | 35 ----------- test/unit/RegistryCoordinatorUnit.t.sol | 83 ++++++++++++++++++++----- test/utils/MockAVSDeployer.sol | 12 +++- 3 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 9a2b034f..31398cae 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -188,41 +188,6 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo } } - /** - * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum - * operator capacity, this method will fail. - * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for - */ - function registerOperator( - bytes calldata quorumNumbers, - string calldata socket - ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { - bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); - - // Register the operator in each of the registry contracts - RegisterResults memory results = _registerOperator({ - operator: msg.sender, - operatorId: operatorId, - quorumNumbers: quorumNumbers, - socket: socket - }); - - for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - - OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; - - /** - * The new operator count for each quorum may not exceed the configured maximum - * If it does, use `registerOperatorWithChurn` instead. - */ - require( - results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, - "RegistryCoordinator.registerOperator: operator count exceeds maximum" - ); - } - } - /** * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum operator * capacity, `operatorKickParams` is used to replace an old operator with the new one. diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index ac321395..a577df7a 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -115,42 +115,61 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testRegisterOperatorWithCoordinator_WhenPaused_Reverts() public { bytes memory emptyQuorumNumbers = new bytes(0); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + // pause registerOperator cheats.prank(pauser); registryCoordinator.pause(2 ** PAUSED_REGISTER_OPERATOR); cheats.startPrank(defaultOperator); cheats.expectRevert(bytes("Pausable: index is paused")); - registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket); + registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testRegisterOperatorWithCoordinator_EmptyQuorumNumbers_Reverts() public { bytes memory emptyQuorumNumbers = new bytes(0); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + cheats.expectRevert("RegistryCoordinator._registerOperator: bitmap cannot be 0"); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket); + registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testRegisterOperatorWithCoordinator_QuorumNumbersTooLarge_Reverts() public { bytes memory quorumNumbersTooLarge = new bytes(1); quorumNumbersTooLarge[0] = 0xC0; + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + cheats.expectRevert("BitmapUtils.orderedBytesArrayToBitmap: bitmap exceeds max value"); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbersTooLarge, defaultSocket); + registryCoordinator.registerOperator(quorumNumbersTooLarge, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testRegisterOperatorWithCoordinator_QuorumNotCreated_Reverts() public { _deployMockEigenLayerAndAVS(10); bytes memory quorumNumbersNotCreated = new bytes(1); quorumNumbersNotCreated[0] = 0x0B; + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + cheats.prank(defaultOperator); cheats.expectRevert("BitmapUtils.orderedBytesArrayToBitmap: bitmap exceeds max value"); - registryCoordinator.registerOperator(quorumNumbersNotCreated, defaultSocket); + registryCoordinator.registerOperator(quorumNumbersNotCreated, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testRegisterOperatorWithCoordinatorForSingleQuorum_Valid() public { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); @@ -165,7 +184,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { uint256 gasBefore = gasleft(); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); uint256 gasAfter = gasleft(); emit log_named_uint("gasUsed", gasBefore - gasAfter); @@ -194,6 +213,9 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { quorumBitmap = quorumBitmap & MAX_QUORUM_BITMAP; cheats.assume(quorumBitmap != 0); bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), defaultOperator, defaultStake); @@ -217,7 +239,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { uint256 gasBefore = gasleft(); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); uint256 gasAfter = gasleft(); emit log_named_uint("gasUsed", gasBefore - gasAfter); emit log_named_uint("numQuorums", quorumNumbers.length); @@ -247,11 +269,14 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); bytes memory newQuorumNumbers = new bytes(1); newQuorumNumbers[0] = bytes1(defaultQuorumNumber+1); @@ -267,7 +292,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { emit QuorumIndexUpdate(defaultOperatorId, uint8(newQuorumNumbers[0]), 0); cheats.roll(nextRegistrationBlockNumber); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(newQuorumNumbers, defaultSocket); + registryCoordinator.registerOperator(newQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers) | BitmapUtils.orderedBytesArrayToBitmap(newQuorumNumbers); @@ -304,6 +329,9 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -325,7 +353,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.prank(operatorToRegister); cheats.expectRevert("RegistryCoordinator.registerOperator: operator count exceeds maximum"); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testRegisterOperatorWithCoordinator_RegisteredOperatorForSameQuorums_Reverts() public { @@ -334,16 +362,19 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); cheats.prank(defaultOperator); cheats.roll(nextRegistrationBlockNumber); cheats.expectRevert("RegistryCoordinator._registerOperator: operator already registered for some quorums being registered for"); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function testDeregisterOperatorWithCoordinator_WhenPaused_Reverts() public { @@ -393,6 +424,9 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); @@ -400,7 +434,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -437,6 +471,9 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testDeregisterOperatorWithCoordinatorForFuzzedQuorumAndSingleOperator_Valid(uint256 quorumBitmap) public { uint32 registrationBlockNumber = 100; uint32 deregistrationBlockNumber = 200; + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; quorumBitmap = quorumBitmap & MAX_QUORUM_BITMAP; cheats.assume(quorumBitmap != 0); @@ -450,7 +487,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorRemovedFromQuorums(defaultOperator, quorumNumbers); @@ -567,6 +604,9 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; cheats.startPrank(defaultOperator); @@ -577,7 +617,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { registryCoordinator.getQuorumBitmapUpdateByIndex(defaultOperatorId, 0); // re-register the operator - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); // check success of registration uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -803,11 +843,14 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { // register operator with default stake with default quorum number bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorRemovedFromQuorums(defaultOperator, quorumNumbers); @@ -836,13 +879,16 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(2); quorumNumbers[0] = bytes1(defaultQuorumNumber); quorumNumbers[1] = bytes1(defaultQuorumNumber + 1); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), defaultOperator, defaultStake); } cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); // eject from only first quorum bytes memory quorumNumbersToEject = new bytes(1); @@ -875,11 +921,14 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testEjectOperatorFromCoordinator_NotEjector_Reverts() public { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); cheats.expectRevert("RegistryCoordinator.onlyEjector: caller is not the ejector"); cheats.prank(defaultOperator); diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index c40de9a9..5112746d 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -291,13 +291,17 @@ contract MockAVSDeployer is Test { blsApkRegistry.setBLSPublicKey(operator, pubKey); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), operator, stake); } cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } /** @@ -309,13 +313,17 @@ contract MockAVSDeployer is Test { blsApkRegistry.setBLSPublicKey(operator, pubKey); + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; + bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), operator, stakes[uint8(quorumNumbers[i])]); } cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); } function _registerRandomOperators(uint256 pseudoRandomNumber) internal returns(OperatorMetadata[] memory, uint256[][] memory) { From 11c51e1007e9e5c4159fa9064e47ef9d0df5fab1 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 17:04:31 -0800 Subject: [PATCH 08/25] feat: create struct for pubkey registration params note: code is currently not compiling due to a stack-too-deep error error here: src/RegistryCoordinator.sol:254:69: | 254 | _deregisterOperator(operatorKickParams[i].operator, quorumNumbers[i:i+1]); --- src/BLSApkRegistry.sol | 30 +++--- src/RegistryCoordinator.sol | 20 +++- src/interfaces/IBLSApkRegistry.sol | 41 +++++---- test/ffi/BLSPubKeyCompendiumFFI.t.sol | 19 ++-- test/unit/BLSApkRegistryUnit.t.sol | 42 +++++---- test/unit/RegistryCoordinatorUnit.t.sol | 117 ++++++++++-------------- test/utils/MockAVSDeployer.sol | 14 +-- 7 files changed, 139 insertions(+), 144 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 880ef9c2..03abe2b5 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -96,17 +96,13 @@ contract BLSApkRegistry is BLSApkRegistryStorage { /** * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator + * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership */ function registerBLSPublicKey( address operator, - BN254.G1Point memory signedMessageHash, - BN254.G1Point memory pubkeyG1, - BN254.G2Point memory pubkeyG2 + PubkeyRegistrationParams calldata pubkeyRegistrationParams ) external onlyRegistryCoordinator { - bytes32 pubkeyHash = BN254.hashG1Point(pubkeyG1); + bytes32 pubkeyHash = BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1); require( pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey" ); @@ -124,29 +120,29 @@ contract BLSApkRegistry is BLSApkRegistryStorage { // gamma = h(sigma, P, P', H(m)) uint256 gamma = uint256(keccak256(abi.encodePacked( - signedMessageHash.X, - signedMessageHash.Y, - pubkeyG1.X, - pubkeyG1.Y, - pubkeyG2.X, - pubkeyG2.Y, + pubkeyRegistrationParams.pubkeyRegistrationSignature.X, + pubkeyRegistrationParams.pubkeyRegistrationSignature.Y, + pubkeyRegistrationParams.pubkeyG1.X, + pubkeyRegistrationParams.pubkeyG1.Y, + pubkeyRegistrationParams.pubkeyG2.X, + pubkeyRegistrationParams.pubkeyG2.Y, messageHash.X, messageHash.Y ))) % BN254.FR_MODULUS; // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P') require(BN254.pairing( - signedMessageHash.plus(pubkeyG1.scalar_mul(gamma)), + pubkeyRegistrationParams.pubkeyRegistrationSignature.plus(pubkeyRegistrationParams.pubkeyG1.scalar_mul(gamma)), BN254.negGeneratorG2(), messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), - pubkeyG2 + pubkeyRegistrationParams.pubkeyG2 ), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); - operatorToPubkey[operator] = pubkeyG1; + operatorToPubkey[operator] = pubkeyRegistrationParams.pubkeyG1; operatorToPubkeyHash[operator] = pubkeyHash; pubkeyHashToOperator[pubkeyHash] = operator; - emit NewPubkeyRegistration(operator, pubkeyG1, pubkeyG2); + emit NewPubkeyRegistration(operator, pubkeyRegistrationParams.pubkeyG1, pubkeyRegistrationParams.pubkeyG2); } /******************************************************************************* diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 31398cae..68b5d4bd 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -147,20 +147,20 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum * operator capacity, this method will fail. * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for + * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @dev the `pubkeyRegistrationParams` input param is ignored if the caller has previously registered a public key */ function registerOperator( bytes calldata quorumNumbers, string calldata socket, - BN254.G1Point memory pubkeyRegistrationSignature, - BN254.G1Point memory pubkeyG1, - BN254.G2Point memory pubkeyG2 + IBLSApkRegistry.PubkeyRegistrationParams calldata pubkeyRegistrationParams ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { /** * IF the operator has never registered a pubkey before, THEN register their pubkey - * OTHERWISE, simply ignore the `pubkeyRegistrationSignature`, `pubkeyG1`, and `pubkeyG2` inputs + * OTHERWISE, simply ignore the provided `pubkeyRegistrationParams` */ if (blsApkRegistry.operatorToPubkeyHash(msg.sender) == 0) { - blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); } bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); @@ -192,18 +192,28 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum operator * capacity, `operatorKickParams` is used to replace an old operator with the new one. * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for + * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership * @param operatorKickParams are used to determine which operator is removed to maintain quorum capacity as the * operator registers for quorums. * @param churnApproverSignature is the signature of the churnApprover on the operator kick params + * @dev the `pubkeyRegistrationParams` input param is ignored if the caller has previously registered a public key */ function registerOperatorWithChurn( bytes calldata quorumNumbers, string calldata socket, + IBLSApkRegistry.PubkeyRegistrationParams calldata pubkeyRegistrationParams, OperatorKickParam[] calldata operatorKickParams, SignatureWithSaltAndExpiry memory churnApproverSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(operatorKickParams.length == quorumNumbers.length, "RegistryCoordinator.registerOperatorWithChurn: input length mismatch"); + /** + * IF the operator has never registered a pubkey before, THEN register their pubkey + * OTHERWISE, simply ignore the `pubkeyRegistrationSignature`, `pubkeyG1`, and `pubkeyG2` inputs + */ + if (blsApkRegistry.operatorToPubkeyHash(msg.sender) == 0) { + blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); + } bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); // Verify the churn approver's signature for the registering operator and kick params diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index af241658..3c5a0c27 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -10,6 +10,29 @@ import {BN254} from "src/libraries/BN254.sol"; * @author Layr Labs, Inc. */ interface IBLSApkRegistry is IRegistry { + // STRUCTS + /// @notice Data structure used to track the history of the Aggregate Public Key of all operators + struct ApkUpdate { + // first 24 bytes of keccak256(apk_x0, apk_x1, apk_y0, apk_y1) + bytes24 apkHash; + // block number at which the update occurred + uint32 updateBlockNumber; + // block number at which the next update occurred + uint32 nextUpdateBlockNumber; + } + + /** + * @notice Struct used when registering a new public key + * @param signedMessageHash is the registration message hash signed by the private key of the operator + * @param pubkeyG1 is the corresponding G1 public key of the operator + * @param pubkeyG2 is the corresponding G2 public key of the operator + */ + struct PubkeyRegistrationParams { + BN254.G1Point pubkeyRegistrationSignature; + BN254.G1Point pubkeyG1; + BN254.G2Point pubkeyG2; + } + // EVENTS /// @notice Emitted when `operator` registers with the public keys `pubkeyG1` and `pubkeyG2`. event NewPubkeyRegistration(address indexed operator, BN254.G1Point pubkeyG1, BN254.G2Point pubkeyG2); @@ -26,16 +49,6 @@ interface IBLSApkRegistry is IRegistry { bytes quorumNumbers ); - /// @notice Data structure used to track the history of the Aggregate Public Key of all operators - struct ApkUpdate { - // first 24 bytes of keccak256(apk_x0, apk_x1, apk_y0, apk_y1) - bytes24 apkHash; - // block number at which the update occurred - uint32 updateBlockNumber; - // block number at which the next update occurred - uint32 nextUpdateBlockNumber; - } - /** * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. * @param operator The address of the operator to register. @@ -85,15 +98,11 @@ interface IBLSApkRegistry is IRegistry { /** * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered - * @param signedMessageHash is the registration message hash signed by the private key of the operator - * @param pubkeyG1 is the corresponding G1 public key of the operator - * @param pubkeyG2 is the corresponding G2 public key of the operator + * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership */ function registerBLSPublicKey( address operator, - BN254.G1Point memory signedMessageHash, - BN254.G1Point memory pubkeyG1, - BN254.G2Point memory pubkeyG2 + PubkeyRegistrationParams calldata pubkeyRegistrationParams ) external; /** diff --git a/test/ffi/BLSPubKeyCompendiumFFI.t.sol b/test/ffi/BLSPubKeyCompendiumFFI.t.sol index 39121a96..0910ab5d 100644 --- a/test/ffi/BLSPubKeyCompendiumFFI.t.sol +++ b/test/ffi/BLSPubKeyCompendiumFFI.t.sol @@ -3,6 +3,7 @@ pragma solidity =0.8.12; import "src/BLSApkRegistry.sol"; import "test/ffi/util/G2Operations.sol"; +import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol"; contract BLSApkRegistryFFITests is G2Operations { using BN254 for BN254.G1Point; @@ -14,9 +15,7 @@ contract BLSApkRegistryFFITests is G2Operations { IRegistryCoordinator registryCoordinator; uint256 privKey; - BN254.G1Point pubKeyG1; - BN254.G2Point pubKeyG2; - BN254.G1Point signedMessageHash; + IBLSApkRegistry.PubkeyRegistrationParams pubkeyRegistrationParams; address alice = address(0x69); @@ -28,19 +27,21 @@ contract BLSApkRegistryFFITests is G2Operations { cheats.assume(_privKey != 0); _setKeys(_privKey); - signedMessageHash = _signMessage(alice); + pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice); vm.prank(address(registryCoordinator)); - blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); - assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); - assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); + assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1), + "pubkey hash not stored correctly"); + assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1)), alice, + "operator address not stored correctly"); } function _setKeys(uint256 _privKey) internal { privKey = _privKey; - pubKeyG1 = BN254.generatorG1().scalar_mul(_privKey); - pubKeyG2 = G2Operations.mul(_privKey); + pubkeyRegistrationParams.pubkeyG1 = BN254.generatorG1().scalar_mul(_privKey); + pubkeyRegistrationParams.pubkeyG2 = G2Operations.mul(_privKey); } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index 493a44d4..683d59ff 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -21,9 +21,7 @@ contract BLSApkRegistryUnitTests is Test { BN254.G1Point internal defaultPubKey = BN254.G1Point(18260007818883133054078754218619977578772505796600400998181738095793040006897,3432351341799135763167709827653955074218841517684851694584291831827675065899); - BN254.G1Point pubKeyG1; - BN254.G2Point pubKeyG2; - BN254.G1Point signedMessageHash; + IBLSApkRegistry.PubkeyRegistrationParams pubkeyRegistrationParams; address alice = address(1); address bob = address(2); @@ -39,13 +37,13 @@ contract BLSApkRegistryUnitTests is Test { registryCoordinator = new RegistryCoordinatorMock(); blsApkRegistry = new BLSApkRegistryHarness(registryCoordinator); - pubKeyG1 = BN254.generatorG1().scalar_mul(privKey); + pubkeyRegistrationParams.pubkeyG1 = BN254.generatorG1().scalar_mul(privKey); //privKey*G2 - pubKeyG2.X[1] = 19101821850089705274637533855249918363070101489527618151493230256975900223847; - pubKeyG2.X[0] = 5334410886741819556325359147377682006012228123419628681352847439302316235957; - pubKeyG2.Y[1] = 354176189041917478648604979334478067325821134838555150300539079146482658331; - pubKeyG2.Y[0] = 4185483097059047421902184823581361466320657066600218863748375739772335928910; + pubkeyRegistrationParams.pubkeyG2.X[1] = 19101821850089705274637533855249918363070101489527618151493230256975900223847; + pubkeyRegistrationParams.pubkeyG2.X[0] = 5334410886741819556325359147377682006012228123419628681352847439302316235957; + pubkeyRegistrationParams.pubkeyG2.Y[1] = 354176189041917478648604979334478067325821134838555150300539079146482658331; + pubkeyRegistrationParams.pubkeyG2.Y[0] = 4185483097059047421902184823581361466320657066600218863748375739772335928910; // Initialize a quorum @@ -293,29 +291,33 @@ contract BLSApkRegistryUnitTests is Test { // TODO: better organize / integrate tests migrated from `BLSPublicKeyCompendium` unit tests function testRegisterBLSPublicKey() public { - signedMessageHash = _signMessage(alice); + pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice); vm.prank(address(registryCoordinator)); - blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); - assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubKeyG1), "pubkey hash not stored correctly"); - assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubKeyG1)), alice, "operator address not stored correctly"); + assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1), + "pubkey hash not stored correctly"); + assertEq(blsApkRegistry.pubkeyHashToOperator(BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1)), alice, + "operator address not stored correctly"); } function testRegisterBLSPublicKey_NoMatch_Reverts() public { - signedMessageHash = _signMessage(alice); - BN254.G1Point memory badPubKeyG1 = BN254.generatorG1().scalar_mul(420); // mismatch public keys + pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice); + BN254.G1Point memory badPubkeyG1 = BN254.generatorG1().scalar_mul(420); // mismatch public keys + + pubkeyRegistrationParams.pubkeyG1 = badPubkeyG1; vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, badPubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); } function testRegisterBLSPublicKey_BadSig_Reverts() public { - signedMessageHash = _signMessage(bob); // sign with wrong private key + pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // sign with wrong private key vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); } function testRegisterBLSPublicKey_OpRegistered_Reverts() public { @@ -323,16 +325,16 @@ contract BLSApkRegistryUnitTests is Test { vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey")); - blsApkRegistry.registerBLSPublicKey(alice, signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); } function testRegisterBLSPublicKey_PkRegistered_Reverts() public { testRegisterBLSPublicKey(); - signedMessageHash = _signMessage(bob); // same private key different operator + pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // same private key different operator vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: public key already registered")); - blsApkRegistry.registerBLSPublicKey(bob, signedMessageHash, pubKeyG1, pubKeyG2); + blsApkRegistry.registerBLSPublicKey(bob, pubkeyRegistrationParams); } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index a577df7a..2e88a222 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -115,9 +115,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testRegisterOperatorWithCoordinator_WhenPaused_Reverts() public { bytes memory emptyQuorumNumbers = new bytes(0); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; // pause registerOperator cheats.prank(pauser); @@ -125,51 +122,39 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.startPrank(defaultOperator); cheats.expectRevert(bytes("Pausable: index is paused")); - registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationParams); } function testRegisterOperatorWithCoordinator_EmptyQuorumNumbers_Reverts() public { bytes memory emptyQuorumNumbers = new bytes(0); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; cheats.expectRevert("RegistryCoordinator._registerOperator: bitmap cannot be 0"); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(emptyQuorumNumbers, defaultSocket, pubkeyRegistrationParams); } function testRegisterOperatorWithCoordinator_QuorumNumbersTooLarge_Reverts() public { bytes memory quorumNumbersTooLarge = new bytes(1); quorumNumbersTooLarge[0] = 0xC0; - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; cheats.expectRevert("BitmapUtils.orderedBytesArrayToBitmap: bitmap exceeds max value"); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbersTooLarge, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbersTooLarge, defaultSocket, pubkeyRegistrationParams); } function testRegisterOperatorWithCoordinator_QuorumNotCreated_Reverts() public { _deployMockEigenLayerAndAVS(10); bytes memory quorumNumbersNotCreated = new bytes(1); quorumNumbersNotCreated[0] = 0x0B; - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; cheats.prank(defaultOperator); cheats.expectRevert("BitmapUtils.orderedBytesArrayToBitmap: bitmap exceeds max value"); - registryCoordinator.registerOperator(quorumNumbersNotCreated, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbersNotCreated, defaultSocket, pubkeyRegistrationParams); } function testRegisterOperatorWithCoordinatorForSingleQuorum_Valid() public { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); @@ -184,7 +169,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { uint256 gasBefore = gasleft(); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); uint256 gasAfter = gasleft(); emit log_named_uint("gasUsed", gasBefore - gasAfter); @@ -213,9 +198,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { quorumBitmap = quorumBitmap & MAX_QUORUM_BITMAP; cheats.assume(quorumBitmap != 0); bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), defaultOperator, defaultStake); @@ -239,7 +221,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { uint256 gasBefore = gasleft(); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); uint256 gasAfter = gasleft(); emit log_named_uint("gasUsed", gasBefore - gasAfter); emit log_named_uint("numQuorums", quorumNumbers.length); @@ -269,14 +251,11 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); bytes memory newQuorumNumbers = new bytes(1); newQuorumNumbers[0] = bytes1(defaultQuorumNumber+1); @@ -292,7 +271,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { emit QuorumIndexUpdate(defaultOperatorId, uint8(newQuorumNumbers[0]), 0); cheats.roll(nextRegistrationBlockNumber); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(newQuorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(newQuorumNumbers, defaultSocket, pubkeyRegistrationParams); uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers) | BitmapUtils.orderedBytesArrayToBitmap(newQuorumNumbers); @@ -329,9 +308,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -353,7 +329,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.prank(operatorToRegister); cheats.expectRevert("RegistryCoordinator.registerOperator: operator count exceeds maximum"); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); } function testRegisterOperatorWithCoordinator_RegisteredOperatorForSameQuorums_Reverts() public { @@ -362,19 +338,16 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); cheats.prank(defaultOperator); cheats.roll(nextRegistrationBlockNumber); cheats.expectRevert("RegistryCoordinator._registerOperator: operator already registered for some quorums being registered for"); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); } function testDeregisterOperatorWithCoordinator_WhenPaused_Reverts() public { @@ -424,9 +397,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); @@ -434,7 +404,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -471,9 +441,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testDeregisterOperatorWithCoordinatorForFuzzedQuorumAndSingleOperator_Valid(uint256 quorumBitmap) public { uint32 registrationBlockNumber = 100; uint32 deregistrationBlockNumber = 200; - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; quorumBitmap = quorumBitmap & MAX_QUORUM_BITMAP; cheats.assume(quorumBitmap != 0); @@ -487,7 +454,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorRemovedFromQuorums(defaultOperator, quorumNumbers); @@ -604,9 +571,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; cheats.startPrank(defaultOperator); @@ -617,7 +581,7 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { registryCoordinator.getQuorumBitmapUpdateByIndex(defaultOperatorId, 0); // re-register the operator - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); // check success of registration uint256 quorumBitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers); @@ -715,12 +679,16 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1); { + BN254.G1Point memory pubkeyRegistrationSignature; + BN254.G1Point memory pubkeyG1; + BN254.G2Point memory pubkeyG2; ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); uint256 gasBefore = gasleft(); registryCoordinator.registerOperatorWithChurn( quorumNumbers, - defaultSocket, + defaultSocket, + pubkeyRegistrationParams, operatorKickParams, signatureWithExpiry ); @@ -769,7 +737,13 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); cheats.expectRevert("RegistryCoordinator._validateChurn: incoming operator has insufficient stake for churn"); - registryCoordinator.registerOperatorWithChurn(quorumNumbers, defaultSocket, operatorKickParams, signatureWithExpiry); + registryCoordinator.registerOperatorWithChurn( + quorumNumbers, + defaultSocket, + pubkeyRegistrationParams, + operatorKickParams, + signatureWithExpiry + ); } function testRegisterOperatorWithCoordinatorWithKicks_LessThanKickBIPsOfTotalStake_Reverts(uint256 pseudoRandomNumber) public { @@ -792,7 +766,13 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); cheats.expectRevert("RegistryCoordinator._validateChurn: cannot kick operator with more than kickBIPsOfTotalStake"); - registryCoordinator.registerOperatorWithChurn(quorumNumbers, defaultSocket, operatorKickParams, signatureWithExpiry); + registryCoordinator.registerOperatorWithChurn( + quorumNumbers, + defaultSocket, + pubkeyRegistrationParams, + operatorKickParams, + signatureWithExpiry + ); } function testRegisterOperatorWithCoordinatorWithKicks_InvalidSignatures_Reverts(uint256 pseudoRandomNumber) public { @@ -815,7 +795,13 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { signatureWithSaltAndExpiry.salt = defaultSalt; cheats.prank(operatorToRegister); cheats.expectRevert("ECDSA: invalid signature"); - registryCoordinator.registerOperatorWithChurn(quorumNumbers, defaultSocket, operatorKickParams, signatureWithSaltAndExpiry); + registryCoordinator.registerOperatorWithChurn( + quorumNumbers, + defaultSocket, + pubkeyRegistrationParams, + operatorKickParams, + signatureWithSaltAndExpiry + ); } function testRegisterOperatorWithCoordinatorWithKicks_ExpiredSignatures_Reverts(uint256 pseudoRandomNumber) public { @@ -836,21 +822,24 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithSaltAndExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp - 1); cheats.prank(operatorToRegister); cheats.expectRevert("RegistryCoordinator._verifyChurnApproverSignature: churnApprover signature expired"); - registryCoordinator.registerOperatorWithChurn(quorumNumbers, defaultSocket, operatorKickParams, signatureWithSaltAndExpiry); + registryCoordinator.registerOperatorWithChurn( + quorumNumbers, + defaultSocket, + pubkeyRegistrationParams, + operatorKickParams, + signatureWithSaltAndExpiry + ); } function testEjectOperatorFromCoordinator_AllQuorums_Valid() public { // register operator with default stake with default quorum number bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorRemovedFromQuorums(defaultOperator, quorumNumbers); @@ -879,16 +868,13 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { bytes memory quorumNumbers = new bytes(2); quorumNumbers[0] = bytes1(defaultQuorumNumber); quorumNumbers[1] = bytes1(defaultQuorumNumber + 1); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), defaultOperator, defaultStake); } cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); // eject from only first quorum bytes memory quorumNumbersToEject = new bytes(1); @@ -921,14 +907,11 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { function testEjectOperatorFromCoordinator_NotEjector_Reverts() public { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; stakeRegistry.setOperatorWeight(uint8(quorumNumbers[0]), defaultOperator, defaultStake); cheats.prank(defaultOperator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); cheats.expectRevert("RegistryCoordinator.onlyEjector: caller is not the ejector"); cheats.prank(defaultOperator); diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index 5112746d..89fac517 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -95,6 +95,8 @@ contract MockAVSDeployer is Test { uint32 registrationBlockNumber = 100; uint32 blocksBetweenRegistrations = 10; + IBLSApkRegistry.PubkeyRegistrationParams pubkeyRegistrationParams; + struct OperatorMetadata { uint256 quorumBitmap; address operator; @@ -291,17 +293,13 @@ contract MockAVSDeployer is Test { blsApkRegistry.setBLSPublicKey(operator, pubKey); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; - bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), operator, stake); } cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); } /** @@ -313,17 +311,13 @@ contract MockAVSDeployer is Test { blsApkRegistry.setBLSPublicKey(operator, pubKey); - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; - bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap); for (uint i = 0; i < quorumNumbers.length; i++) { stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), operator, stakes[uint8(quorumNumbers[i])]); } cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationSignature, pubkeyG1, pubkeyG2); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams); } function _registerRandomOperators(uint256 pseudoRandomNumber) internal returns(OperatorMetadata[] memory, uint256[][] memory) { From da6f3a5e2650749dada6d66204e79db8e9c1833b Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Mon, 11 Dec 2023 17:27:39 -0800 Subject: [PATCH 09/25] fix: address stack-too-deep issue in RegistryCoordinator --- src/RegistryCoordinator.sol | 7 +++---- test/unit/RegistryCoordinatorUnit.t.sol | 3 --- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 68b5d4bd..5fc49713 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -233,9 +233,8 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo uint256 kickIndex = 0; for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - - OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; + // reference: uint8 quorumNumber = uint8(quorumNumbers[i]); + OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])]; /** * If the new operator count for any quorum exceeds the maximum, validate @@ -243,7 +242,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo */ if (results.numOperatorsPerQuorum[i] > operatorSetParams.maxOperatorCount) { _validateChurn({ - quorumNumber: quorumNumber, + quorumNumber: uint8(quorumNumbers[i]), totalQuorumStake: results.totalStakes[i], newOperator: msg.sender, newOperatorStake: results.operatorStakes[i], diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 2e88a222..90179d74 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -679,9 +679,6 @@ contract RegistryCoordinatorUnit is MockAVSDeployer { emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1); { - BN254.G1Point memory pubkeyRegistrationSignature; - BN254.G1Point memory pubkeyG1; - BN254.G2Point memory pubkeyG2; ISignatureUtils.SignatureWithSaltAndExpiry memory signatureWithExpiry = _signOperatorChurnApproval(operatorToRegisterId, operatorKickParams, defaultSalt, block.timestamp + 10); cheats.prank(operatorToRegister); uint256 gasBefore = gasleft(); From 9948e01a7701e1f9f4ea37e83ba3e9d4343bd852 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:51:17 -0800 Subject: [PATCH 10/25] chore: delete unused index see reasoning here https://github.com/Layr-Labs/eigenlayer-middleware/pull/100#discussion_r1424577318 this allows undoing a previous fix for a stack-too-deep error, which... helps make the logic a little clearer / easier to parse here. --- src/RegistryCoordinator.sol | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 5fc49713..9941f269 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -231,10 +231,9 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo socket: socket }); - uint256 kickIndex = 0; for (uint256 i = 0; i < quorumNumbers.length; i++) { - // reference: uint8 quorumNumber = uint8(quorumNumbers[i]); - OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])]; + uint8 quorumNumber = uint8(quorumNumbers[i]); + OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; /** * If the new operator count for any quorum exceeds the maximum, validate @@ -242,7 +241,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo */ if (results.numOperatorsPerQuorum[i] > operatorSetParams.maxOperatorCount) { _validateChurn({ - quorumNumber: uint8(quorumNumbers[i]), + quorumNumber: quorumNumber, totalQuorumStake: results.totalStakes[i], newOperator: msg.sender, newOperatorStake: results.operatorStakes[i], @@ -251,7 +250,6 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo }); _deregisterOperator(operatorKickParams[i].operator, quorumNumbers[i:i+1]); - kickIndex++; } } } From db000925f55ba3bca2db005008fb55feab68e60b Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:56:39 -0800 Subject: [PATCH 11/25] feat: optimization for fetching operatorId NOTE: this changes the IBLSApkRegistry interface to make `registerBLSPublicKey` return a bytes32 object --- src/BLSApkRegistry.sol | 3 ++- src/RegistryCoordinator.sol | 12 ++++++------ src/interfaces/IBLSApkRegistry.sol | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 03abe2b5..1dd52d2b 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -101,7 +101,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { function registerBLSPublicKey( address operator, PubkeyRegistrationParams calldata pubkeyRegistrationParams - ) external onlyRegistryCoordinator { + ) external onlyRegistryCoordinator returns (bytes32 operatorId) { bytes32 pubkeyHash = BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1); require( pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey" @@ -143,6 +143,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { pubkeyHashToOperator[pubkeyHash] = operator; emit NewPubkeyRegistration(operator, pubkeyRegistrationParams.pubkeyG1, pubkeyRegistrationParams.pubkeyG2); + return pubkeyHash; } /******************************************************************************* diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 9941f269..f4c4184e 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -159,10 +159,10 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * IF the operator has never registered a pubkey before, THEN register their pubkey * OTHERWISE, simply ignore the provided `pubkeyRegistrationParams` */ - if (blsApkRegistry.operatorToPubkeyHash(msg.sender) == 0) { - blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); - } bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); + if (operatorId == 0) { + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); + } // Register the operator in each of the registry contracts RegisterResults memory results = _registerOperator({ @@ -211,10 +211,10 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * IF the operator has never registered a pubkey before, THEN register their pubkey * OTHERWISE, simply ignore the `pubkeyRegistrationSignature`, `pubkeyG1`, and `pubkeyG2` inputs */ - if (blsApkRegistry.operatorToPubkeyHash(msg.sender) == 0) { - blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); - } bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); + if (operatorId == 0) { + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); + } // Verify the churn approver's signature for the registering operator and kick params _verifyChurnApproverSignature({ diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 3c5a0c27..47a9dd47 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -103,7 +103,7 @@ interface IBLSApkRegistry is IRegistry { function registerBLSPublicKey( address operator, PubkeyRegistrationParams calldata pubkeyRegistrationParams - ) external; + ) external returns (bytes32 operatorId); /** * @notice Returns the pubkey and pubkey hash of an operator From da59eee8a66a1d83895dfc5994203ecb903823f9 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:00:46 -0800 Subject: [PATCH 12/25] chore: shorten variable name that is quite clear from context `pubkeyRegistrationParams` => `params` see discussion here: https://github.com/Layr-Labs/eigenlayer-middleware/pull/100#discussion_r1424469756 --- src/BLSApkRegistry.sol | 26 +++++++++++++------------- src/RegistryCoordinator.sol | 18 +++++++++--------- src/interfaces/IBLSApkRegistry.sol | 4 ++-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 1dd52d2b..060de2d0 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -96,13 +96,13 @@ contract BLSApkRegistry is BLSApkRegistryStorage { /** * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered - * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership */ function registerBLSPublicKey( address operator, - PubkeyRegistrationParams calldata pubkeyRegistrationParams + PubkeyRegistrationParams calldata params ) external onlyRegistryCoordinator returns (bytes32 operatorId) { - bytes32 pubkeyHash = BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1); + bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1); require( pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey" ); @@ -120,29 +120,29 @@ contract BLSApkRegistry is BLSApkRegistryStorage { // gamma = h(sigma, P, P', H(m)) uint256 gamma = uint256(keccak256(abi.encodePacked( - pubkeyRegistrationParams.pubkeyRegistrationSignature.X, - pubkeyRegistrationParams.pubkeyRegistrationSignature.Y, - pubkeyRegistrationParams.pubkeyG1.X, - pubkeyRegistrationParams.pubkeyG1.Y, - pubkeyRegistrationParams.pubkeyG2.X, - pubkeyRegistrationParams.pubkeyG2.Y, + params.pubkeyRegistrationSignature.X, + params.pubkeyRegistrationSignature.Y, + params.pubkeyG1.X, + params.pubkeyG1.Y, + params.pubkeyG2.X, + params.pubkeyG2.Y, messageHash.X, messageHash.Y ))) % BN254.FR_MODULUS; // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P') require(BN254.pairing( - pubkeyRegistrationParams.pubkeyRegistrationSignature.plus(pubkeyRegistrationParams.pubkeyG1.scalar_mul(gamma)), + params.pubkeyRegistrationSignature.plus(params.pubkeyG1.scalar_mul(gamma)), BN254.negGeneratorG2(), messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), - pubkeyRegistrationParams.pubkeyG2 + params.pubkeyG2 ), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); - operatorToPubkey[operator] = pubkeyRegistrationParams.pubkeyG1; + operatorToPubkey[operator] = params.pubkeyG1; operatorToPubkeyHash[operator] = pubkeyHash; pubkeyHashToOperator[pubkeyHash] = operator; - emit NewPubkeyRegistration(operator, pubkeyRegistrationParams.pubkeyG1, pubkeyRegistrationParams.pubkeyG2); + emit NewPubkeyRegistration(operator, params.pubkeyG1, params.pubkeyG2); return pubkeyHash; } diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index f4c4184e..0eef3632 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -147,21 +147,21 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum * operator capacity, this method will fail. * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for - * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership - * @dev the `pubkeyRegistrationParams` input param is ignored if the caller has previously registered a public key + * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @dev the `params` input param is ignored if the caller has previously registered a public key */ function registerOperator( bytes calldata quorumNumbers, string calldata socket, - IBLSApkRegistry.PubkeyRegistrationParams calldata pubkeyRegistrationParams + IBLSApkRegistry.PubkeyRegistrationParams calldata params ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { /** * IF the operator has never registered a pubkey before, THEN register their pubkey - * OTHERWISE, simply ignore the provided `pubkeyRegistrationParams` + * OTHERWISE, simply ignore the provided `params` */ bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params); } // Register the operator in each of the registry contracts @@ -192,16 +192,16 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo * @notice Registers msg.sender as an operator for one or more quorums. If any quorum reaches its maximum operator * capacity, `operatorKickParams` is used to replace an old operator with the new one. * @param quorumNumbers is an ordered byte array containing the quorum numbers being registered for - * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership * @param operatorKickParams are used to determine which operator is removed to maintain quorum capacity as the * operator registers for quorums. * @param churnApproverSignature is the signature of the churnApprover on the operator kick params - * @dev the `pubkeyRegistrationParams` input param is ignored if the caller has previously registered a public key + * @dev the `params` input param is ignored if the caller has previously registered a public key */ function registerOperatorWithChurn( bytes calldata quorumNumbers, string calldata socket, - IBLSApkRegistry.PubkeyRegistrationParams calldata pubkeyRegistrationParams, + IBLSApkRegistry.PubkeyRegistrationParams calldata params, OperatorKickParam[] calldata operatorKickParams, SignatureWithSaltAndExpiry memory churnApproverSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { @@ -213,7 +213,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo */ bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, pubkeyRegistrationParams); + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params); } // Verify the churn approver's signature for the registering operator and kick params diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 47a9dd47..b35b45fe 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -98,11 +98,11 @@ interface IBLSApkRegistry is IRegistry { /** * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered - * @param pubkeyRegistrationParams contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership */ function registerBLSPublicKey( address operator, - PubkeyRegistrationParams calldata pubkeyRegistrationParams + PubkeyRegistrationParams calldata params ) external returns (bytes32 operatorId); /** From 630b27d394191b89d6c88e7a1b8dda07750a31db Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 12 Dec 2023 15:36:48 -0800 Subject: [PATCH 13/25] feat: use EIP712 for the pubkeyRegistrationMessageHash this commit also moves the definition to the RegistryCoordinator contract and updates the function naming to be clearer --- src/BLSApkRegistry.sol | 26 ++++++------------------- src/RegistryCoordinator.sol | 18 +++++++++++++++-- src/interfaces/IBLSApkRegistry.sol | 10 +++------- src/interfaces/IRegistryCoordinator.sol | 7 +++++++ test/ffi/BLSPubKeyCompendiumFFI.t.sol | 4 ++-- test/mocks/RegistryCoordinatorMock.sol | 6 ++++++ test/unit/BLSApkRegistryUnit.t.sol | 17 ++++++++++------ 7 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 060de2d0..5bb14fba 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -97,10 +97,12 @@ contract BLSApkRegistry is BLSApkRegistryStorage { * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @param pubkeyRegistrationMessageHash is a hash that the operator must sign to prove key ownership */ function registerBLSPublicKey( address operator, - PubkeyRegistrationParams calldata params + PubkeyRegistrationParams calldata params, + BN254.G1Point calldata pubkeyRegistrationMessageHash ) external onlyRegistryCoordinator returns (bytes32 operatorId) { bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1); require( @@ -115,9 +117,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { "BLSApkRegistry.registerBLSPublicKey: public key already registered" ); - // H(m) - BN254.G1Point memory messageHash = getMessageHash(operator); - // gamma = h(sigma, P, P', H(m)) uint256 gamma = uint256(keccak256(abi.encodePacked( params.pubkeyRegistrationSignature.X, @@ -126,15 +125,15 @@ contract BLSApkRegistry is BLSApkRegistryStorage { params.pubkeyG1.Y, params.pubkeyG2.X, params.pubkeyG2.Y, - messageHash.X, - messageHash.Y + pubkeyRegistrationMessageHash.X, + pubkeyRegistrationMessageHash.Y ))) % BN254.FR_MODULUS; // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P') require(BN254.pairing( params.pubkeyRegistrationSignature.plus(params.pubkeyG1.scalar_mul(gamma)), BN254.negGeneratorG2(), - messageHash.plus(BN254.generatorG1().scalar_mul(gamma)), + pubkeyRegistrationMessageHash.plus(BN254.generatorG1().scalar_mul(gamma)), params.pubkeyG2 ), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"); @@ -215,19 +214,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { return (pubkey, pubkeyHash); } - /** - * @notice Returns the message hash that an operator must sign to register their BLS public key. - * @param operator is the address of the operator registering their BLS public key - */ - function getMessageHash(address operator) public view returns (BN254.G1Point memory) { - return BN254.hashToG1(keccak256(abi.encodePacked( - operator, - address(this), - block.chainid, - "EigenLayer_BN254_Pubkey_Registration" - ))); - } - /** * @notice Returns the indices of the quorumApks index at `blockNumber` for the provided `quorumNumbers` * @dev Returns the current indices if `blockNumber >= block.number` diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 0eef3632..c96e969b 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -35,6 +35,8 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo /// @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,bytes32[] operatorIdsToSwap)"); + /// @notice The EIP-712 typehash used for registering BLS public keys + bytes32 public constant PUBKEY_REGISTRATION_TYPEHASH = keccak256("BN254PubkeyRegistration(address operator)"); /// @notice The maximum value of a quorum bitmap uint256 internal constant MAX_QUORUM_BITMAP = type(uint192).max; /// @notice The basis point denominator @@ -161,7 +163,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo */ bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params); + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); } // Register the operator in each of the registry contracts @@ -213,7 +215,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo */ bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params); + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); } // Verify the churn approver's signature for the registering operator and kick params @@ -760,4 +762,16 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo // calculate the digest hash return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, salt, expiry))); } + + /** + * @notice Returns the message hash that an operator must sign to register their BLS public key. + * @param operator is the address of the operator registering their BLS public key + */ + function pubkeyRegistrationMessageHash(address operator) public view returns (BN254.G1Point memory) { + return BN254.hashToG1( + _hashTypedDataV4( + keccak256(abi.encode(PUBKEY_REGISTRATION_TYPEHASH, operator)) + ) + ); + } } diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index b35b45fe..48fc680f 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -99,10 +99,12 @@ interface IBLSApkRegistry is IRegistry { * @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key. * @param operator is the operator for whom the key is being registered * @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership + * @param pubkeyRegistrationMessageHash is a hash that the operator must sign to prove key ownership */ function registerBLSPublicKey( address operator, - PubkeyRegistrationParams calldata params + PubkeyRegistrationParams calldata params, + BN254.G1Point calldata pubkeyRegistrationMessageHash ) external returns (bytes32 operatorId); /** @@ -111,12 +113,6 @@ interface IBLSApkRegistry is IRegistry { */ function getRegisteredPubkey(address operator) external view returns (BN254.G1Point memory, bytes32); - /** - * @notice Returns the message hash that an operator must sign to register their BLS public key. - * @param operator is the address of the operator registering their BLS public key - */ - function getMessageHash(address operator) external view returns (BN254.G1Point memory); - /// @notice Returns the current APK for the provided `quorumNumber ` function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory); diff --git a/src/interfaces/IRegistryCoordinator.sol b/src/interfaces/IRegistryCoordinator.sol index ca6f8629..6d2c153d 100644 --- a/src/interfaces/IRegistryCoordinator.sol +++ b/src/interfaces/IRegistryCoordinator.sol @@ -4,6 +4,7 @@ pragma solidity =0.8.12; import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol"; import {IStakeRegistry} from "src/interfaces/IStakeRegistry.sol"; import {IIndexRegistry} from "src/interfaces/IIndexRegistry.sol"; +import {BN254} from "src/libraries/BN254.sol"; /** * @title Interface for a contract that coordinates between various registries for an AVS. @@ -134,4 +135,10 @@ interface IRegistryCoordinator { /// @notice Returns the number of registries function numRegistries() external view returns (uint256); + + /** + * @notice Returns the message hash that an operator must sign to register their BLS public key. + * @param operator is the address of the operator registering their BLS public key + */ + function pubkeyRegistrationMessageHash(address operator) external view returns (BN254.G1Point memory); } \ No newline at end of file diff --git a/test/ffi/BLSPubKeyCompendiumFFI.t.sol b/test/ffi/BLSPubKeyCompendiumFFI.t.sol index 0910ab5d..3bb75118 100644 --- a/test/ffi/BLSPubKeyCompendiumFFI.t.sol +++ b/test/ffi/BLSPubKeyCompendiumFFI.t.sol @@ -30,7 +30,7 @@ contract BLSApkRegistryFFITests is G2Operations { pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice); vm.prank(address(registryCoordinator)); - blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, registryCoordinator.pubkeyRegistrationMessageHash(alice)); assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1), "pubkey hash not stored correctly"); @@ -45,7 +45,7 @@ contract BLSApkRegistryFFITests is G2Operations { } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { - BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer); + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(signer); return BN254.scalar_mul(messageHash, privKey); } } diff --git a/test/mocks/RegistryCoordinatorMock.sol b/test/mocks/RegistryCoordinatorMock.sol index 87cbb436..fccf4d92 100644 --- a/test/mocks/RegistryCoordinatorMock.sol +++ b/test/mocks/RegistryCoordinatorMock.sol @@ -58,4 +58,10 @@ contract RegistryCoordinatorMock is IRegistryCoordinator { function registerOperator(bytes memory quorumNumbers, bytes calldata) external {} function deregisterOperator(bytes calldata quorumNumbers, bytes calldata) external {} + + function pubkeyRegistrationMessageHash(address operator) public view returns (BN254.G1Point memory) { + return BN254.hashToG1( + keccak256(abi.encode(operator)) + ); + } } diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index 683d59ff..0d8ed07a 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -292,8 +292,9 @@ contract BLSApkRegistryUnitTests is Test { // TODO: better organize / integrate tests migrated from `BLSPublicKeyCompendium` unit tests function testRegisterBLSPublicKey() public { pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice); + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice); vm.prank(address(registryCoordinator)); - blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash); assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1), "pubkey hash not stored correctly"); @@ -307,38 +308,42 @@ contract BLSApkRegistryUnitTests is Test { pubkeyRegistrationParams.pubkeyG1 = badPubkeyG1; + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice); vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash); } function testRegisterBLSPublicKey_BadSig_Reverts() public { pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // sign with wrong private key + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice); vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match")); - blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash); } function testRegisterBLSPublicKey_OpRegistered_Reverts() public { testRegisterBLSPublicKey(); // register alice + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice); vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey")); - blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash); } function testRegisterBLSPublicKey_PkRegistered_Reverts() public { testRegisterBLSPublicKey(); pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // same private key different operator + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(bob); vm.prank(address(registryCoordinator)); vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: public key already registered")); - blsApkRegistry.registerBLSPublicKey(bob, pubkeyRegistrationParams); + blsApkRegistry.registerBLSPublicKey(bob, pubkeyRegistrationParams, messageHash); } function _signMessage(address signer) internal view returns(BN254.G1Point memory) { - BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer); + BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(signer); return BN254.scalar_mul(messageHash, privKey); } From ef7147f9984649b7635464c14dbee21a1d6acf72 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 12 Dec 2023 15:43:21 -0800 Subject: [PATCH 14/25] chore: add NatSpec to getter function --- src/BLSApkRegistry.sol | 2 ++ src/interfaces/IBLSApkRegistry.sol | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 5bb14fba..71e3a77b 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -280,6 +280,8 @@ contract BLSApkRegistry is BLSApkRegistryStorage { return pubkeyHashToOperator[pubkeyHash]; } + /// @notice returns the ID used to identify the `operator` within this AVS + /// @dev Returns zero in the event that the `operator` has never registered for the AVS function getOperatorId(address operator) public view returns (bytes32) { return operatorToPubkeyHash[operator]; } diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 48fc680f..916320ec 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -134,5 +134,7 @@ interface IBLSApkRegistry is IRegistry { */ function getApkHashAtBlockNumberAndIndex(uint8 quorumNumber, uint32 blockNumber, uint256 index) external view returns (bytes24); + /// @notice returns the ID used to identify the `operator` within this AVS. + /// @dev Returns zero in the event that the `operator` has never registered for the AVS function getOperatorId(address operator) external view returns (bytes32); } From a47ee2b7bdf25886241659abe6333ea420a87c4b Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:12:01 -0800 Subject: [PATCH 15/25] chore: fix merge artifacts / broken calls these weren't caught in merging but were causing compiler errors --- test/integration/CoreRegistration.t.sol | 6 +++--- test/utils/MockAVSDeployer.sol | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/test/integration/CoreRegistration.t.sol b/test/integration/CoreRegistration.t.sol index 1df04dfc..fa16bbb3 100644 --- a/test/integration/CoreRegistration.t.sol +++ b/test/integration/CoreRegistration.t.sol @@ -57,7 +57,7 @@ contract Test_CoreRegistration is MockAVSDeployer { // Set operator address operator = cheats.addr(operatorPrivateKey); - pubkeyCompendium.setBLSPublicKey(operator, defaultPubKey); + blsApkRegistry.setBLSPublicKey(operator, defaultPubKey); // Register operator to EigenLayer cheats.prank(operator); @@ -91,7 +91,7 @@ contract Test_CoreRegistration is MockAVSDeployer { // Register operator cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, operatorSignature); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, operatorSignature); // Check operator is registered IDelegationManager.OperatorAVSRegistrationStatus operatorStatus = delegationManager.avsOperatorStatus(address(registryCoordinator), operator); @@ -152,7 +152,7 @@ contract Test_CoreRegistration is MockAVSDeployer { // Register operator cheats.prank(operator); - registryCoordinator.registerOperator(quorumNumbers, defaultSocket, operatorSignature); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, operatorSignature); } function _getOperatorSignature( diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index facdbb00..c7f5a757 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -26,8 +26,6 @@ import {IRegistryCoordinator} from "src/interfaces/IRegistryCoordinator.sol"; import {StrategyManagerMock} from "eigenlayer-contracts/src/test/mocks/StrategyManagerMock.sol"; import {EigenPodManagerMock} from "eigenlayer-contracts/src/test/mocks/EigenPodManagerMock.sol"; -import {ServiceManagerMock} from "test/mocks/ServiceManagerMock.sol"; -import {OwnableMock} from "eigenlayer-contracts/src/test/mocks/OwnableMock.sol"; import {DelegationManagerMock} from "eigenlayer-contracts/src/test/mocks/DelegationManagerMock.sol"; import {BLSApkRegistryHarness} from "test/harnesses/BLSApkRegistryHarness.sol"; import {EmptyContract} from "eigenlayer-contracts/src/test/mocks/EmptyContract.sol"; From 0db5eb5dd4f497437ca9e90d97247c0854876459 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:13:56 -0800 Subject: [PATCH 16/25] chore: re-implement stack-too-deep fix I previously implemented this fix, then reverted it as not needed. Now upon merging with m2-mainnet, `registerOperatorWithChurn` has an additional input. This is reintroducing the same stack-too-deep error so I am reintroducing the same fix (one less local var) --- src/RegistryCoordinator.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 07f851d3..3bef4fea 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -252,8 +252,8 @@ contract RegistryCoordinator is }); for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; + // reference: uint8 quorumNumber = uint8(quorumNumbers[i]); + OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])]; /** * If the new operator count for any quorum exceeds the maximum, validate @@ -261,7 +261,7 @@ contract RegistryCoordinator is */ if (results.numOperatorsPerQuorum[i] > operatorSetParams.maxOperatorCount) { _validateChurn({ - quorumNumber: quorumNumber, + quorumNumber: uint8(quorumNumbers[i]), totalQuorumStake: results.totalStakes[i], newOperator: msg.sender, newOperatorStake: results.operatorStakes[i], From 891a219239a5b7503350e3189018765f6595f869 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:37:25 -0800 Subject: [PATCH 17/25] chore: de-duplicate code into an internal function see suggestion here: https://github.com/Layr-Labs/eigenlayer-middleware/pull/102#discussion_r1425509482 saves ~0.25kb in code size --- src/RegistryCoordinator.sol | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 3bef4fea..3a84a0f9 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -173,12 +173,9 @@ contract RegistryCoordinator is ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { /** * IF the operator has never registered a pubkey before, THEN register their pubkey - * OTHERWISE, simply ignore the provided `params` + * OTHERWISE, simply ignore the provided `params` input */ - bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); - if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); - } + bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); // Register the operator in each of the registry contracts RegisterResults memory results = _registerOperator({ @@ -228,12 +225,9 @@ contract RegistryCoordinator is /** * IF the operator has never registered a pubkey before, THEN register their pubkey - * OTHERWISE, simply ignore the `pubkeyRegistrationSignature`, `pubkeyG1`, and `pubkeyG2` inputs + * OTHERWISE, simply ignore the provided `params` input */ - bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender); - if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); - } + bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); // Verify the churn approver's signature for the registering operator and kick params _verifyChurnApproverSignature({ @@ -519,6 +513,17 @@ contract RegistryCoordinator is }); } + function _getOrCreateOperatorId( + address operator, + IBLSApkRegistry.PubkeyRegistrationParams calldata params + ) internal returns (bytes32 operatorId) { + operatorId = blsApkRegistry.getOperatorId(msg.sender); + if (operatorId == 0) { + operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); + } + return operatorId; + } + function _validateChurn( uint8 quorumNumber, uint96 totalQuorumStake, From 1e27ee5ed84a6662ba672d83652c808d8f38b560 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:53:40 -0800 Subject: [PATCH 18/25] fix: actually use the `operator` input to `_getOrCreateOperatorId` also reduce memory copying operations (I think, at least) in the `registerOperator` function specifically, only copy the `numOperatorsPerQuorum` returned by the internal `_registerOperator` function --- src/RegistryCoordinator.sol | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 3a84a0f9..d4e2f930 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -178,25 +178,23 @@ contract RegistryCoordinator is bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); // Register the operator in each of the registry contracts - RegisterResults memory results = _registerOperator({ + uint32[] memory numOperatorsPerQuorum = _registerOperator({ operator: msg.sender, operatorId: operatorId, quorumNumbers: quorumNumbers, socket: socket, operatorSignature: operatorSignature - }); + }).numOperatorsPerQuorum; for (uint256 i = 0; i < quorumNumbers.length; i++) { uint8 quorumNumber = uint8(quorumNumbers[i]); - - OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; - + /** * The new operator count for each quorum may not exceed the configured maximum * If it does, use `registerOperatorWithChurn` instead. */ require( - results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, + numOperatorsPerQuorum[i] <= _quorumParams[quorumNumber].maxOperatorCount, "RegistryCoordinator.registerOperator: operator count exceeds maximum" ); } @@ -517,9 +515,9 @@ contract RegistryCoordinator is address operator, IBLSApkRegistry.PubkeyRegistrationParams calldata params ) internal returns (bytes32 operatorId) { - operatorId = blsApkRegistry.getOperatorId(msg.sender); + operatorId = blsApkRegistry.getOperatorId(operator); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); + operatorId = blsApkRegistry.registerBLSPublicKey(operator, params, pubkeyRegistrationMessageHash(operator)); } return operatorId; } From e729464988f84793cddb32cbf3f88e18dfe6f9d5 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:11:25 -0800 Subject: [PATCH 19/25] feat: add optimizer runs count to foundry config --- foundry.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/foundry.toml b/foundry.toml index 6a48adda..5c1f5469 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,4 +6,7 @@ fs_permissions = [{ access = "read-write", path = "./" }] ffi = true +# The number of optimizer runs +optimizer_runs = 200 + # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options From 43ed47465190675425dc3268ad98532aba308d52 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:18:12 -0800 Subject: [PATCH 20/25] chore: remove redundant check and return data The `operatorId` in this check is already fetched from the `blsApkRegistry` With the other changes, this return data is no longer necessary at all --- src/BLSApkRegistry.sol | 4 +--- src/RegistryCoordinator.sol | 3 +-- src/interfaces/IBLSApkRegistry.sol | 2 +- test/unit/BLSApkRegistryUnit.t.sol | 8 +++----- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 71e3a77b..fe16c7fa 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -32,7 +32,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. * @param operator The address of the operator to register. * @param quorumNumbers The quorum numbers the operator is registering for, where each byte is an 8 bit integer quorumNumber. - * @return pubkeyHash of the operator's pubkey * @dev access restricted to the RegistryCoordinator * @dev Preconditions (these are assumed, not validated in this contract): * 1) `quorumNumbers` has no duplicates @@ -43,7 +42,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { function registerOperator( address operator, bytes memory quorumNumbers - ) public virtual onlyRegistryCoordinator returns (bytes32) { + ) public virtual onlyRegistryCoordinator { // Get the operator's pubkey. Reverts if they have not registered a key (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = getRegisteredPubkey(operator); @@ -52,7 +51,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { // Return pubkeyHash, which will become the operator's unique id emit OperatorAddedToQuorums(operator, quorumNumbers); - return pubkeyHash; } /** diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index d4e2f930..810ef6cb 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -498,8 +498,7 @@ contract RegistryCoordinator is /** * Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry */ - bytes32 registeredId = blsApkRegistry.registerOperator(operator, quorumNumbers); - require(registeredId == operatorId, "RegistryCoordinator._registerOperator: operatorId mismatch"); + blsApkRegistry.registerOperator(operator, quorumNumbers); (uint96[] memory operatorStakes, uint96[] memory totalStakes) = stakeRegistry.registerOperator(operator, operatorId, quorumNumbers); uint32[] memory numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 916320ec..62a3e6b1 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -60,7 +60,7 @@ interface IBLSApkRegistry is IRegistry { * 3) `quorumNumbers` is ordered in ascending order * 4) the operator is not already registered */ - function registerOperator(address operator, bytes calldata quorumNumbers) external returns(bytes32); + function registerOperator(address operator, bytes calldata quorumNumbers) external; /** * @notice Deregisters the `operator`'s pubkey for the specified `quorumNumbers`. diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index 0d8ed07a..81c02654 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -92,13 +92,11 @@ contract BLSApkRegistryUnitTests is Test { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - cheats.startPrank(address(registryCoordinator)); - bytes32 registeredpkHash = blsApkRegistry.registerOperator(operator, quorumNumbers); - cheats.stopPrank(); - + cheats.prank(address(registryCoordinator)); + blsApkRegistry.registerOperator(operator, quorumNumbers); + (, bytes32 registeredpkHash) = blsApkRegistry.getRegisteredPubkey(operator); require(registeredpkHash == pkHash, "registeredpkHash not set correctly"); - emit log("ehey"); return pkHash; } From b205bded62c27e6b3b6f6f0cafbfa8a182f40ee6 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:20:35 -0800 Subject: [PATCH 21/25] chore: fewer memory operations(?) I believe this change cuts down on the memory copying being done here --- src/RegistryCoordinator.sol | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 810ef6cb..041e7586 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -458,7 +458,7 @@ contract RegistryCoordinator is bytes calldata quorumNumbers, string memory socket, SignatureWithSaltAndExpiry memory operatorSignature - ) internal virtual returns (RegisterResults memory) { + ) internal virtual returns (RegisterResults memory results) { /** * Get bitmap of quorums to register for and operator's current bitmap. Validate that: * - we're trying to register for at least 1 quorum @@ -499,15 +499,11 @@ contract RegistryCoordinator is * Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry */ blsApkRegistry.registerOperator(operator, quorumNumbers); - (uint96[] memory operatorStakes, uint96[] memory totalStakes) = + (results.operatorStakes, results.totalStakes) = stakeRegistry.registerOperator(operator, operatorId, quorumNumbers); - uint32[] memory numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); + results.numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); - return RegisterResults({ - numOperatorsPerQuorum: numOperatorsPerQuorum, - operatorStakes: operatorStakes, - totalStakes: totalStakes - }); + return results; } function _getOrCreateOperatorId( From c2f5719ea565409d273194a339efde4ad55b9641 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:25:26 -0800 Subject: [PATCH 22/25] fix: reduce optimizer runs to meet contract code size limits Likely not the ideal fix, but it gets the job done for the moment, at least. --- foundry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/foundry.toml b/foundry.toml index 5c1f5469..f8f557c1 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,6 +7,6 @@ fs_permissions = [{ access = "read-write", path = "./" }] ffi = true # The number of optimizer runs -optimizer_runs = 200 +optimizer_runs = 100 # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options From caf9960d9808484afb8ffa7682d996f5e89f1dd1 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:39:13 -0800 Subject: [PATCH 23/25] chore: rename file to reflect it only being used in tests `RegistryCoordinatorHarness.sol` -> `RegistryCoordinatorHarness.t.sol` --- src/RegistryCoordinator.sol | 2 +- ...yCoordinatorHarness.sol => RegistryCoordinatorHarness.t.sol} | 0 test/unit/StakeRegistryUnit.t.sol | 2 +- test/utils/MockAVSDeployer.sol | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename test/harnesses/{RegistryCoordinatorHarness.sol => RegistryCoordinatorHarness.t.sol} (100%) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 041e7586..b86f9dc6 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -432,7 +432,7 @@ contract RegistryCoordinator is /** * @notice Sets the metadata URI for the AVS * @param _metadataURI is the metadata URI for the AVS - * @dev only callable by the service manager owner + * @dev only callable by the owner */ function setMetadataURI(string memory _metadataURI) external onlyOwner { delegationManager.updateAVSMetadataURI(_metadataURI); diff --git a/test/harnesses/RegistryCoordinatorHarness.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol similarity index 100% rename from test/harnesses/RegistryCoordinatorHarness.sol rename to test/harnesses/RegistryCoordinatorHarness.t.sol diff --git a/test/unit/StakeRegistryUnit.t.sol b/test/unit/StakeRegistryUnit.t.sol index c0ae719b..766e5fe3 100644 --- a/test/unit/StakeRegistryUnit.t.sol +++ b/test/unit/StakeRegistryUnit.t.sol @@ -25,7 +25,7 @@ import {SlasherMock} from "eigenlayer-contracts/src/test/mocks/SlasherMock.sol"; import {StakeRegistryHarness} from "test/harnesses/StakeRegistryHarness.sol"; import {StakeRegistry} from "src/StakeRegistry.sol"; -import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.sol"; +import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.t.sol"; import "forge-std/Test.sol"; diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index c7f5a757..118cbd03 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -14,7 +14,7 @@ import {BN254} from "src/libraries/BN254.sol"; import {OperatorStateRetriever} from "src/OperatorStateRetriever.sol"; import {RegistryCoordinator} from "src/RegistryCoordinator.sol"; -import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.sol"; +import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.t.sol"; import {BLSApkRegistry} from "src/BLSApkRegistry.sol"; import {StakeRegistry} from "src/StakeRegistry.sol"; import {IndexRegistry} from "src/IndexRegistry.sol"; From b659b2f74d661bbffd94bab8f7558e69f0e68b6c Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:40:22 -0800 Subject: [PATCH 24/25] chore: delete unused (memory) variable --- src/BLSApkRegistry.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index fe16c7fa..a732a44a 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -44,7 +44,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { bytes memory quorumNumbers ) public virtual onlyRegistryCoordinator { // Get the operator's pubkey. Reverts if they have not registered a key - (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = getRegisteredPubkey(operator); + (BN254.G1Point memory pubkey, ) = getRegisteredPubkey(operator); // Update each quorum's aggregate pubkey _processQuorumApkUpdate(quorumNumbers, pubkey); From d03039fbaf6d41330b29af0b0c26ddf9ba035a6e Mon Sep 17 00:00:00 2001 From: wadealexc Date: Wed, 13 Dec 2023 21:55:09 +0000 Subject: [PATCH 25/25] fix: have the harness import Test so it gets ignored in build sizes --- test/harnesses/RegistryCoordinatorHarness.t.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 246ea705..f3ce41a7 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -3,8 +3,10 @@ pragma solidity =0.8.12; import "src/RegistryCoordinator.sol"; +import "forge-std/Test.sol"; + // wrapper around the RegistryCoordinator contract that exposes the internal functions for unit testing. -contract RegistryCoordinatorHarness is RegistryCoordinator { +contract RegistryCoordinatorHarness is RegistryCoordinator, Test { constructor( IDelegationManager _delegationManager, ISlasher _slasher,