refactor: custom errors in middleware (#355)

* refactor: custom errors * fix: unit tests
Layr-Labs · Jan 10, 2025 · 8778517 · 8778517
1 parent 45eb220
commit 8778517
Show file tree

Hide file tree

Showing 29 changed files with 300 additions and 271 deletions.
diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol
@@ -79,7 +79,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
      * @param quorumNumber The number of the new quorum
      */
     function initializeQuorum(uint8 quorumNumber) public virtual onlyRegistryCoordinator {
-        require(apkHistory[quorumNumber].length == 0, "BLSApkRegistry.initializeQuorum: quorum already exists");
+        require(apkHistory[quorumNumber].length == 0, QuorumAlreadyExists());
 
         apkHistory[quorumNumber].push(ApkUpdate({
             apkHash: bytes24(0),
@@ -100,17 +100,9 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
         BN254.G1Point calldata pubkeyRegistrationMessageHash
     ) external onlyRegistryCoordinator returns (bytes32 operatorId) {
         bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1);
-        require(
-            pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey"
-        );
-        require(
-            operatorToPubkeyHash[operator] == bytes32(0),
-            "BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey"
-        );
-        require(
-            pubkeyHashToOperator[pubkeyHash] == address(0),
-            "BLSApkRegistry.registerBLSPublicKey: public key already registered"
-        );
+        require(pubkeyHash != ZERO_PK_HASH, ZeroPubKey());
+        require(operatorToPubkeyHash[operator] == bytes32(0), OperatorAlreadyRegistered());
+        require(pubkeyHashToOperator[pubkeyHash] == address(0), BLSPubkeyAlreadyRegistered());
 
         // gamma = h(sigma, P, P', H(m))
         uint256 gamma = uint256(keccak256(abi.encodePacked(
@@ -130,7 +122,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
             BN254.negGeneratorG2(),
             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");
+        ), InvalidBLSSignatureOrPrivateKey());
 
         operatorToPubkey[operator] = params.pubkeyG1;
         operatorToPubkeyHash[operator] = pubkeyHash;
@@ -151,7 +143,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
             // Validate quorum exists and get history length
             uint8 quorumNumber = uint8(quorumNumbers[i]);
             uint256 historyLength = apkHistory[quorumNumber].length;
-            require(historyLength != 0, "BLSApkRegistry._processQuorumApkUpdate: quorum does not exist");
+            require(historyLength != 0, QuorumDoesNotExist());
 
             // Update aggregate public key for this quorum
             newApk = currentApk[quorumNumber].plus(point);
@@ -185,10 +177,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
         BN254.G1Point memory pubkey = operatorToPubkey[operator];
         bytes32 pubkeyHash = operatorToPubkeyHash[operator];
 
-        require(
-            pubkeyHash != bytes32(0),
-            "BLSApkRegistry.getRegisteredPubkey: operator is not registered"
-        );
+        require(pubkeyHash != bytes32(0), OperatorNotRegistered());
 
         return (pubkey, pubkeyHash);
     }
@@ -253,11 +242,11 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
          */
         require(
             blockNumber >= quorumApkUpdate.updateBlockNumber,
-            "BLSApkRegistry.getApkHashAtBlockNumberAndIndex: index too recent"
+            BlockNumberTooRecent()
         );
         require(
             quorumApkUpdate.nextUpdateBlockNumber == 0 || blockNumber < quorumApkUpdate.nextUpdateBlockNumber,
-            "BLSApkRegistry.getApkHashAtBlockNumberAndIndex: not latest apk update"
+            BlockNumberNotLatest()
         );
 
         return quorumApkUpdate.apkHash;
@@ -280,9 +269,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
     }
 
     function _checkRegistryCoordinator() internal view {
-        require(
-            msg.sender == address(registryCoordinator),
-            "BLSApkRegistry._checkRegistryCoordinator: caller is not the registry coordinator"
-        );
+        require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinatorOwner());
     }
 }
diff --git a/src/BLSSignatureChecker.sol b/src/BLSSignatureChecker.sol
@@ -31,10 +31,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
     bool public staleStakesForbidden;
 
     modifier onlyCoordinatorOwner() {
-        require(
-            msg.sender == registryCoordinator.owner(),
-            "BLSSignatureChecker.onlyCoordinatorOwner: caller is not the owner of the registryCoordinator"
-        );
+        require(msg.sender == registryCoordinator.owner(), OnlyRegistryCoordinatorOwner());
         _;
     }
 
@@ -91,29 +88,23 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
         uint32 referenceBlockNumber,
         NonSignerStakesAndSignature memory params
     ) public view returns (QuorumStakeTotals memory, bytes32) {
-        require(
-            quorumNumbers.length != 0,
-            "BLSSignatureChecker.checkSignatures: empty quorum input"
-        );
+        require(quorumNumbers.length != 0, InputEmptyQuorumNumbers());
 
         require(
             (quorumNumbers.length == params.quorumApks.length) &&
                 (quorumNumbers.length == params.quorumApkIndices.length) &&
                 (quorumNumbers.length == params.totalStakeIndices.length) &&
                 (quorumNumbers.length == params.nonSignerStakeIndices.length),
-            "BLSSignatureChecker.checkSignatures: input quorum length mismatch"
+            InputArrayLengthMismatch()
         );
 
         require(
             params.nonSignerPubkeys.length ==
                 params.nonSignerQuorumBitmapIndices.length,
-            "BLSSignatureChecker.checkSignatures: input nonsigner length mismatch"
+            InputNonSignerLengthMismatch()
         );
 
-        require(
-            referenceBlockNumber < uint32(block.number),
-            "BLSSignatureChecker.checkSignatures: invalid reference block"
-        );
+        require(referenceBlockNumber < uint32(block.number), InvalidReferenceBlocknumber());
 
         // This method needs to calculate the aggregate pubkey for all signing operators across
         // all signing quorums. To do that, we can query the aggregate pubkey for each quorum
@@ -155,7 +146,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
                     require(
                         uint256(nonSigners.pubkeyHashes[j]) >
                             uint256(nonSigners.pubkeyHashes[j - 1]),
-                        "BLSSignatureChecker.checkSignatures: nonSignerPubkeys not sorted"
+                        NonSignerPubkeysNotSorted()
                     );
                 }
 
@@ -207,7 +198,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
                         ) +
                             withdrawalDelayBlocks >
                             referenceBlockNumber,
-                        "BLSSignatureChecker.checkSignatures: StakeRegistry updates must be within withdrawalDelayBlocks window"
+                        StaleStakesForbidden()
                     );
                 }
 
@@ -220,7 +211,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
                             blockNumber: referenceBlockNumber,
                             index: params.quorumApkIndices[i]
                         }),
-                    "BLSSignatureChecker.checkSignatures: quorumApk hash in storage does not match provided quorum apk"
+                    InvalidQuorumApkHash()
                 );
                 apk = apk.plus(params.quorumApks[i]);
 
@@ -274,14 +265,8 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
                     params.apkG2,
                     params.sigma
                 );
-            require(
-                pairingSuccessful,
-                "BLSSignatureChecker.checkSignatures: pairing precompile call failed"
-            );
-            require(
-                signatureIsValid,
-                "BLSSignatureChecker.checkSignatures: signature is invalid"
-            );
+            require(pairingSuccessful, InvalidBLSPairingKey());
+            require(signatureIsValid, InvalidBLSSignature());
         }
         // set signatoryRecordHash variable used for fraudproofs
         bytes32 signatoryRecordHash = keccak256(

diff --git a/src/EjectionManager.sol b/src/EjectionManager.sol
@@ -67,7 +67,7 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable {
      * @dev The owner can eject operators without recording of stake ejection
      */
     function ejectOperators(bytes32[][] memory _operatorIds) external {
-        require(isEjector[msg.sender] || msg.sender == owner(), "EjectionManager.ejectOperators: Only owner or ejector can eject");
+        require(isEjector[msg.sender] || msg.sender == owner(), OnlyOwnerOrEjector());
 
         for(uint i = 0; i < _operatorIds.length; ++i) {
             uint8 quorumNumber = uint8(i);
@@ -137,7 +137,7 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable {
 
     ///@dev internal function to set the quorum ejection params
     function _setQuorumEjectionParams(uint8 _quorumNumber, QuorumEjectionParams memory _quorumEjectionParams) internal {
-        require(_quorumNumber < MAX_QUORUM_COUNT, "EjectionManager._setQuorumEjectionParams: Quorum number exceeds MAX_QUORUM_COUNT");
+        require(_quorumNumber < MAX_QUORUM_COUNT, MaxQuorumCount());
         quorumEjectionParams[_quorumNumber] = _quorumEjectionParams;
         emit QuorumEjectionParamsSet(_quorumNumber, _quorumEjectionParams.rateLimitWindow, _quorumEjectionParams.ejectableStakePercent);
     }

diff --git a/src/IndexRegistry.sol b/src/IndexRegistry.sol
@@ -47,7 +47,7 @@ contract IndexRegistry is IndexRegistryStorage {
             // Validate quorum exists and get current operator count
             uint8 quorumNumber = uint8(quorumNumbers[i]);
             uint256 historyLength = _operatorCountHistory[quorumNumber].length;
-            require(historyLength != 0, "IndexRegistry.registerOperator: quorum does not exist");
+            require(historyLength != 0, QuorumDoesNotExist());
 
             /**
              * Increase the number of operators currently active for this quorum,
@@ -87,7 +87,7 @@ contract IndexRegistry is IndexRegistryStorage {
             // Validate quorum exists and get the operatorIndex of the operator being deregistered
             uint8 quorumNumber = uint8(quorumNumbers[i]);
             uint256 historyLength = _operatorCountHistory[quorumNumber].length;
-            require(historyLength != 0, "IndexRegistry.registerOperator: quorum does not exist");
+            require(historyLength != 0, QuorumDoesNotExist());
             uint32 operatorIndexToRemove = currentOperatorIndex[quorumNumber][operatorId];
 
             /**
@@ -113,7 +113,7 @@ contract IndexRegistry is IndexRegistryStorage {
      * @param quorumNumber The number of the new quorum
      */
     function initializeQuorum(uint8 quorumNumber) public virtual onlyRegistryCoordinator {
-        require(_operatorCountHistory[quorumNumber].length == 0, "IndexRegistry.createQuorum: quorum already exists");
+        require(_operatorCountHistory[quorumNumber].length == 0, QuorumDoesNotExist());
 
         _operatorCountHistory[quorumNumber].push(QuorumUpdate({
             numOperators: 0,
@@ -329,7 +329,7 @@ contract IndexRegistry is IndexRegistryStorage {
             operatorList[i] = _operatorIdForIndexAtBlockNumber(quorumNumber, uint32(i), blockNumber);
             require(
                 operatorList[i] != OPERATOR_DOES_NOT_EXIST_ID,
-                "IndexRegistry.getOperatorListAtBlockNumber: operator does not exist at the given block number"
+                OperatorIdDoesNotExist()
             );
         }
         return operatorList;
@@ -342,6 +342,6 @@ contract IndexRegistry is IndexRegistryStorage {
     }
 
     function _checkRegistryCoordinator() internal view {
-        require(msg.sender == address(registryCoordinator), "IndexRegistry._checkRegistryCoordinator: caller is not the registry coordinator");
+        require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinator());
     }
 }
diff --git a/src/OperatorStateRetriever.sol b/src/OperatorStateRetriever.sol
@@ -26,6 +26,8 @@ contract OperatorStateRetriever {
         uint32[][] nonSignerStakeIndices; // nonSignerStakeIndices[quorumNumberIndex][nonSignerIndex]
     }
 
+    error OperatorNotRegistered();
+
     /**
      * @notice This function is intended to to be called by AVS operators every time a new task is created (i.e.)
      * the AVS coordinator makes a request to AVS operators. Since all of the crucial information is kept onchain,
@@ -131,7 +133,7 @@ contract OperatorStateRetriever {
                         checkSignaturesIndices.nonSignerQuorumBitmapIndices[i]
                     );
 
-                require(nonSignerQuorumBitmap != 0, "OperatorStateRetriever.getCheckSignaturesIndices: operator must be registered at blocknumber");
+                require(nonSignerQuorumBitmap != 0, OperatorNotRegistered());
 
                 // 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) {

diff --git a/src/ServiceManagerBase.sol b/src/ServiceManagerBase.sol
@@ -30,10 +30,7 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {
 
     /// @notice when applied to a function, only allows the RegistryCoordinator to call it
     modifier onlyRegistryCoordinator() {
-        require(
-            msg.sender == address(_registryCoordinator),
-            "ServiceManagerBase.onlyRegistryCoordinator: caller is not the registry coordinator"
-        );
+        require(msg.sender == address(_registryCoordinator), OnlyRegistryCoordinator());
         _;
     }
 
@@ -209,7 +206,7 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {
     function acceptProposedSlasher() external onlyOwner {
         require(
             block.timestamp >= slasherProposalTimestamp + SLASHER_PROPOSAL_DELAY,
-            "ServiceManager: Slasher proposal delay not met"
+            DelayPeriodNotPassed()
         );
         _setSlasher(proposedSlasher);
         delete proposedSlasher;
@@ -314,24 +311,14 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {
     }
 
     function _checkRewardsInitiator() internal view {
-        require(
-            msg.sender == rewardsInitiator,
-            "ServiceManagerBase.onlyRewardsInitiator: caller is not the rewards initiator"
-        );
+        require(msg.sender == rewardsInitiator, OnlyRewardsInitiator());
     }
 
     function _checkStakeRegistry() internal view {
-        require(
-            msg.sender == address(_stakeRegistry),
-            "ServiceManagerBase.onlyStakeRegistry: caller is not the stake registry"
-        );
+        require(msg.sender == address(_stakeRegistry), OnlyStakeRegistry());
     }
 
-
     function _checkSlasher() internal view {
-        require(
-            msg.sender == slasher,
-            "ServiceManagerBase.onlySlasher: caller is not the slasher"
-        );
+        require(msg.sender == slasher, OnlySlasher());
     }
 }