refactor: quorum creation is the divine right of the registry coordinator

src/BLSOperatorStateRetriever.sol

@@ -42,9 +42,9 @@ contract BLSOperatorStateRetriever {
     ) external view returns (uint256, Operator[][] memory) {
         bytes32[] memory operatorIds = new bytes32[](1);
         operatorIds[0] = operatorId;
-        uint256 index = registryCoordinator.getQuorumBitmapIndicesByOperatorIdsAtBlockNumber(blockNumber, operatorIds)[0];
+        uint256 index = registryCoordinator.getQuorumBitmapIndicesAtBlockNumber(blockNumber, operatorIds)[0];
 
-        uint256 quorumBitmap = registryCoordinator.getQuorumBitmapByOperatorIdAtBlockNumberByIndex(operatorId, blockNumber, index);
+        uint256 quorumBitmap = registryCoordinator.getQuorumBitmapAtBlockNumberByIndex(operatorId, blockNumber, index);
 
         bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(quorumBitmap);
 
@@ -70,13 +70,13 @@ contract BLSOperatorStateRetriever {
         Operator[][] memory operators = new Operator[][](quorumNumbers.length);
         for (uint256 i = 0; i < quorumNumbers.length; i++) {
             uint8 quorumNumber = uint8(quorumNumbers[i]);
-            bytes32[] memory operatorIds = indexRegistry.getOperatorListForQuorumAtBlockNumber(quorumNumber, blockNumber);
+            bytes32[] memory operatorIds = indexRegistry.getOperatorListAtBlockNumber(quorumNumber, blockNumber);
             operators[i] = new Operator[](operatorIds.length);
             for (uint256 j = 0; j < operatorIds.length; j++) {
                 bytes32 operatorId = bytes32(operatorIds[j]);
                 operators[i][j] = Operator({
                     operatorId: operatorId,
-                    stake: stakeRegistry.getStakeForOperatorIdForQuorumAtBlockNumber(operatorId, quorumNumber, blockNumber)
+                    stake: stakeRegistry.getOperatorStakeAtBlockNumber(operatorId, quorumNumber, blockNumber)
                 });
             }
         }
@@ -108,9 +108,9 @@ contract BLSOperatorStateRetriever {
         CheckSignaturesIndices memory checkSignaturesIndices;
 
         // get the indices of the quorumBitmap updates for each of the operators in the nonSignerOperatorIds array
-        checkSignaturesIndices.nonSignerQuorumBitmapIndices = registryCoordinator.getQuorumBitmapIndicesByOperatorIdsAtBlockNumber(referenceBlockNumber, nonSignerOperatorIds);
+        checkSignaturesIndices.nonSignerQuorumBitmapIndices = registryCoordinator.getQuorumBitmapIndicesAtBlockNumber(referenceBlockNumber, nonSignerOperatorIds);
         // get the indices of the totalStake updates for each of the quorums in the quorumNumbers array
-        checkSignaturesIndices.totalStakeIndices = stakeRegistry.getTotalStakeIndicesByQuorumNumbersAtBlockNumber(referenceBlockNumber, quorumNumbers);
+        checkSignaturesIndices.totalStakeIndices = stakeRegistry.getTotalStakeIndicesAtBlockNumber(referenceBlockNumber, quorumNumbers);
 
         checkSignaturesIndices.nonSignerStakeIndices = new uint32[][](quorumNumbers.length);
         for (uint8 quorumNumberIndex = 0; quorumNumberIndex < quorumNumbers.length; quorumNumberIndex++) {
@@ -121,7 +121,7 @@ contract BLSOperatorStateRetriever {
             for (uint i = 0; i < nonSignerOperatorIds.length; i++) {
                 // get the quorumBitmap for the operator at the given blocknumber and index
                 uint192 nonSignerQuorumBitmap = 
-                    registryCoordinator.getQuorumBitmapByOperatorIdAtBlockNumberByIndex(
+                    registryCoordinator.getQuorumBitmapAtBlockNumberByIndex(
                         nonSignerOperatorIds[i], 
                         referenceBlockNumber, 
                         checkSignaturesIndices.nonSignerQuorumBitmapIndices[i]
@@ -130,7 +130,7 @@ contract BLSOperatorStateRetriever {
                 // if the operator was a part of the quorum and the quorum is a part of the provided quorumNumbers
                 if ((nonSignerQuorumBitmap >> uint8(quorumNumbers[quorumNumberIndex])) & 1 == 1) {
                     // get the index of the stake update for the operator at the given blocknumber and quorum number
-                    checkSignaturesIndices.nonSignerStakeIndices[quorumNumberIndex][numNonSignersForQuorum] = stakeRegistry.getStakeUpdateIndexForOperatorIdForQuorumAtBlockNumber(
+                    checkSignaturesIndices.nonSignerStakeIndices[quorumNumberIndex][numNonSignersForQuorum] = stakeRegistry.getStakeUpdateIndexForOperatorAtBlockNumber(
                         nonSignerOperatorIds[i],
                         uint8(quorumNumbers[quorumNumberIndex]),
                         referenceBlockNumber
@@ -149,7 +149,7 @@ contract BLSOperatorStateRetriever {
 
         IBLSPubkeyRegistry blsPubkeyRegistry = registryCoordinator.blsPubkeyRegistry();
         // get the indices of the quorum apks for each of the provided quorums at the given blocknumber
-        checkSignaturesIndices.quorumApkIndices = blsPubkeyRegistry.getApkIndicesForQuorumsAtBlockNumber(quorumNumbers, referenceBlockNumber);
+        checkSignaturesIndices.quorumApkIndices = blsPubkeyRegistry.getApkIndicesAtBlockNumber(quorumNumbers, referenceBlockNumber);
 
         return checkSignaturesIndices;
     }

src/BLSPubkeyRegistry.sol

@@ -30,7 +30,6 @@ contract BLSPubkeyRegistry is BLSPubkeyRegistryStorage {
      * @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.
-     * @param pubkey The operator's BLS public key.
      * @return pubkeyHash of the operator's pubkey
      * @dev access restricted to the RegistryCoordinator
      * @dev Preconditions (these are assumed, not validated in this contract):
@@ -41,102 +40,104 @@ contract BLSPubkeyRegistry is BLSPubkeyRegistryStorage {
      */
     function registerOperator(
         address operator,
-        bytes memory quorumNumbers,
-        BN254.G1Point memory pubkey
+        bytes memory quorumNumbers
     ) public virtual onlyRegistryCoordinator returns (bytes32) {
-        //calculate hash of the operator's pubkey
-        bytes32 pubkeyHash = BN254.hashG1Point(pubkey);
+        // Get the operator's pubkey from the compendium. Reverts if they have not registered a key
+        BN254.G1Point memory pubkey = pubkeyCompendium.getRegisteredPubkey(operator);
 
-        require(pubkeyHash != ZERO_PK_HASH, "BLSPubkeyRegistry.registerOperator: cannot register zero pubkey");
-        //ensure that the operator owns their public key by referencing the BLSPubkeyCompendium
-        require(
-            getOperatorFromPubkeyHash(pubkeyHash) == operator,
-            "BLSPubkeyRegistry.registerOperator: operator does not own pubkey"
-        );
-        // update each quorum's aggregate pubkey
+        // Update each quorum's aggregate pubkey
         _processQuorumApkUpdate(quorumNumbers, pubkey);
 
-        // emit event so offchain actors can update their state
+        // Return pubkeyHash, which will become the operator's unique id
         emit OperatorAddedToQuorums(operator, quorumNumbers);
-        return pubkeyHash;
+        return BN254.hashG1Point(pubkey);
     }
 
     /**
      * @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.
-     * @param pubkey The public key of the operator.
      * @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
-     *         6) `pubkey` is the same as the parameter used when registering
      */
     function deregisterOperator(
         address operator,
-        bytes memory quorumNumbers,
-        BN254.G1Point memory pubkey
+        bytes memory quorumNumbers
     ) public virtual onlyRegistryCoordinator {
-        bytes32 pubkeyHash = BN254.hashG1Point(pubkey);
-
-        require(
-            getOperatorFromPubkeyHash(pubkeyHash) == operator,
-            "BLSPubkeyRegistry.registerOperator: operator does not own pubkey"
-        );
+        // Get the operator's pubkey from the compendium. Reverts if they have not registered a key
+        BN254.G1Point memory pubkey = pubkeyCompendium.getRegisteredPubkey(operator);
 
-        // update each quorum's aggregate pubkey
+        // Update each quorum's aggregate pubkey
         _processQuorumApkUpdate(quorumNumbers, pubkey.negate());
-
         emit OperatorRemovedFromQuorums(operator, quorumNumbers);
     }
 
+    /**
+     * @notice Initializes a new quorum by pushing its first apk update
+     * @param quorumNumber The number of the new quorum
+     */
+    function initializeQuorum(uint8 quorumNumber) public virtual onlyRegistryCoordinator {
+        require(apkHistory[quorumNumber].length == 0, "BLSPubkeyRegistry.initializeQuorum: quorum already exists");
+
+        apkHistory[quorumNumber].push(ApkUpdate({
+            apkHash: bytes24(0),
+            updateBlockNumber: uint32(block.number),
+            nextUpdateBlockNumber: 0
+        }));
+    }
+
     /*******************************************************************************
                             INTERNAL FUNCTIONS
     *******************************************************************************/
 
     function _processQuorumApkUpdate(bytes memory quorumNumbers, BN254.G1Point memory point) internal {
-        BN254.G1Point memory apkAfterUpdate;
+        BN254.G1Point memory newApk;
 
-        for (uint i = 0; i < quorumNumbers.length; ) {
+        for (uint256 i = 0; i < quorumNumbers.length; i++) {
+            // Validate quorum exists and get history length
             uint8 quorumNumber = uint8(quorumNumbers[i]);
-
-            uint256 quorumApkUpdatesLength = quorumApkUpdates[quorumNumber].length;
-            if (quorumApkUpdatesLength > 0) {
-                // update nextUpdateBlockNumber of the current latest ApkUpdate
-                quorumApkUpdates[quorumNumber][quorumApkUpdatesLength - 1].nextUpdateBlockNumber = uint32(block.number);
-            }
-
-            apkAfterUpdate = quorumApk[quorumNumber].plus(point);
-
-            //update aggregate public key for this quorum
-            quorumApk[quorumNumber] = apkAfterUpdate;
-            //create new ApkUpdate to add to the mapping
-            ApkUpdate memory latestApkUpdate;
-            latestApkUpdate.apkHash = bytes24(BN254.hashG1Point(apkAfterUpdate));
-            latestApkUpdate.updateBlockNumber = uint32(block.number);
-            quorumApkUpdates[quorumNumber].push(latestApkUpdate);
-
-            unchecked {
-                ++i;
+            uint256 historyLength = apkHistory[quorumNumber].length;
+            require(historyLength != 0, "BLSPubkeyRegistry._processQuorumApkUpdate: quorum does not exist");
+
+            // Update aggregate public key for this quorum
+            newApk = currentApk[quorumNumber].plus(point);
+            currentApk[quorumNumber] = newApk;
+            bytes24 newApkHash = bytes24(BN254.hashG1Point(newApk));
+
+            // Update apk history. If the last update was made in this block, update the entry
+            // Otherwise, push a new historical entry and update the prev->next pointer
+            ApkUpdate storage lastUpdate = apkHistory[quorumNumber][historyLength - 1];
+            if (lastUpdate.updateBlockNumber == uint32(block.number)) {
+                lastUpdate.apkHash = newApkHash;
+            } else {
+                lastUpdate.nextUpdateBlockNumber = uint32(block.number);
+                apkHistory[quorumNumber].push(ApkUpdate({
+                    apkHash: newApkHash,
+                    updateBlockNumber: uint32(block.number),
+                    nextUpdateBlockNumber: 0
+                }));
             }
         }
     }
 
-    function _validateApkHashForQuorumAtBlockNumber(ApkUpdate memory apkUpdate, uint32 blockNumber) internal pure {
+    // TODO - should this fail if apkUpdate.apkHash == 0? This will be the case for the first entry in each quorum
+    function _validateApkHashAtBlockNumber(ApkUpdate memory apkUpdate, uint32 blockNumber) internal pure {
         require(
             blockNumber >= apkUpdate.updateBlockNumber,
-            "BLSPubkeyRegistry._validateApkHashForQuorumAtBlockNumber: index too recent"
+            "BLSPubkeyRegistry._validateApkHashAtBlockNumber: index too recent"
         );
         /**
          * if there is a next update, check that the blockNumber is before the next update or if
          * there is no next update, then apkUpdate.nextUpdateBlockNumber is 0.
          */
         require(
             apkUpdate.nextUpdateBlockNumber == 0 || blockNumber < apkUpdate.nextUpdateBlockNumber,
-            "BLSPubkeyRegistry._validateApkHashForQuorumAtBlockNumber: not latest apk update"
+            "BLSPubkeyRegistry._validateApkHashAtBlockNumber: not latest apk update"
         );
     }
 
@@ -148,23 +149,23 @@ contract BLSPubkeyRegistry is BLSPubkeyRegistryStorage {
      * @notice Returns the indices of the quorumApks index at `blockNumber` for the provided `quorumNumbers`
      * @dev Returns the current indices if `blockNumber >= block.number`
      */
-     function getApkIndicesForQuorumsAtBlockNumber(
+     function getApkIndicesAtBlockNumber(
         bytes calldata quorumNumbers,
         uint256 blockNumber
     ) external view returns (uint32[] memory) {
         uint32[] memory indices = new uint32[](quorumNumbers.length);
         for (uint i = 0; i < quorumNumbers.length; i++) {
             uint8 quorumNumber = uint8(quorumNumbers[i]);
-            uint32 quorumApkUpdatesLength = uint32(quorumApkUpdates[quorumNumber].length);
+            uint32 quorumApkUpdatesLength = uint32(apkHistory[quorumNumber].length);
 
-            if (quorumApkUpdatesLength == 0 || blockNumber < quorumApkUpdates[quorumNumber][0].updateBlockNumber) {
+            if (quorumApkUpdatesLength == 0 || blockNumber < apkHistory[quorumNumber][0].updateBlockNumber) {
                 revert(
-                    "BLSPubkeyRegistry.getApkIndicesForQuorumsAtBlockNumber: blockNumber is before the first update"
+                    "BLSPubkeyRegistry.getApkIndicesAtBlockNumber: blockNumber is before the first update"
                 );
             }
 
             for (uint32 j = 0; j < quorumApkUpdatesLength; j++) {
-                if (quorumApkUpdates[quorumNumber][quorumApkUpdatesLength - j - 1].updateBlockNumber <= blockNumber) {
+                if (apkHistory[quorumNumber][quorumApkUpdatesLength - j - 1].updateBlockNumber <= blockNumber) {
                     indices[i] = quorumApkUpdatesLength - j - 1;
                     break;
                 }
@@ -174,13 +175,13 @@ contract BLSPubkeyRegistry is BLSPubkeyRegistryStorage {
     }
 
     /// @notice Returns the current APK for the provided `quorumNumber `
-    function getApkForQuorum(uint8 quorumNumber) external view returns (BN254.G1Point memory) {
-        return quorumApk[quorumNumber];
+    function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory) {
+        return currentApk[quorumNumber];
     }
 
     /// @notice Returns the `ApkUpdate` struct at `index` in the list of APK updates for the `quorumNumber`
-    function getApkUpdateForQuorumByIndex(uint8 quorumNumber, uint256 index) external view returns (ApkUpdate memory) {
-        return quorumApkUpdates[quorumNumber][index];
+    function getApkUpdateAtIndex(uint8 quorumNumber, uint256 index) external view returns (ApkUpdate memory) {
+        return apkHistory[quorumNumber][index];
     }
 
     /**
@@ -190,19 +191,19 @@ contract BLSPubkeyRegistry is BLSPubkeyRegistryStorage {
      * @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 getApkHashForQuorumAtBlockNumberFromIndex(
+    function getApkHashAtBlockNumberAndIndex(
         uint8 quorumNumber,
         uint32 blockNumber,
         uint256 index
     ) external view returns (bytes24) {
-        ApkUpdate memory quorumApkUpdate = quorumApkUpdates[quorumNumber][index];
-        _validateApkHashForQuorumAtBlockNumber(quorumApkUpdate, blockNumber);
+        ApkUpdate memory quorumApkUpdate = apkHistory[quorumNumber][index];
+        _validateApkHashAtBlockNumber(quorumApkUpdate, blockNumber);
         return quorumApkUpdate.apkHash;
     }
 
     /// @notice Returns the length of ApkUpdates for the provided `quorumNumber`
-    function getQuorumApkHistoryLength(uint8 quorumNumber) external view returns (uint32) {
-        return uint32(quorumApkUpdates[quorumNumber].length);
+    function getApkHistoryLength(uint8 quorumNumber) external view returns (uint32) {
+        return uint32(apkHistory[quorumNumber].length);
     }
 
     /// @notice Returns the operator address for the given `pubkeyHash`

src/BLSPubkeyRegistryStorage.sol

@@ -9,17 +9,15 @@ import "eigenlayer-contracts/src/contracts/libraries/BN254.sol";
 import "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
 
 abstract contract BLSPubkeyRegistryStorage is Initializable, IBLSPubkeyRegistry {
-    /// @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;
 
-    /// @notice mapping of quorumNumber => ApkUpdate[], tracking the aggregate pubkey updates of every quorum
-    mapping(uint8 => ApkUpdate[]) public quorumApkUpdates;
-    /// @notice mapping of quorumNumber => current aggregate pubkey of quorum
-    mapping(uint8 => BN254.G1Point) public quorumApk;
+    /// @notice maps quorumNumber => historical aggregate pubkey updates
+    mapping(uint8 => ApkUpdate[]) public apkHistory;
+    /// @notice maps quorumNumber => current aggregate pubkey of quorum
+    mapping(uint8 => BN254.G1Point) public currentApk;
 
     constructor(IRegistryCoordinator _registryCoordinator, IBLSPublicKeyCompendium _pubkeyCompendium) {
         registryCoordinator = _registryCoordinator;