feat: slashing audit fixes

hashmap0x · hashmap0x · commit d06ea7d7990c · 2025-05-07T19:10:33.000-04:00
**Motivation:** Middleware slashing audit fixes from Hexens and Dedaub. **Modifications:** **Medium Sev** - Layr-Labs/eigenlayer-middleware#467 **Low Sev** - Layr-Labs/eigenlayer-middleware#462 - Layr-Labs/eigenlayer-middleware#463 - Layr-Labs/eigenlayer-middleware#468 - Layr-Labs/eigenlayer-middleware#474 - note: this removes stale stakes checks from the `BLSSignatureChecker` **Informational/Docs** - Layr-Labs/eigenlayer-middleware#465 - Layr-Labs/eigenlayer-middleware#466 - Layr-Labs/eigenlayer-middleware#472 **Result:** Audit fixes applied to slashing.
diff --git a/audits/Slashing - Dedaub - April 2025.pdf b/audits/Slashing - Dedaub - April 2025.pdf
diff --git a/audits/Slashing - Hexens - April 2025.pdf b/audits/Slashing - Hexens - April 2025.pdf
diff --git a/docs/BLSSignatureChecker.md b/docs/BLSSignatureChecker.md
@@ -90,7 +90,6 @@ This method performs the following steps. Note that each step involves lookups o
     * `quorumNumbers` MUST be an ordered list of valid, initialized quorums
     * `params.nonSignerPubkeys` MUST ONLY contain unique pubkeys, in ascending order of their pubkey hash
 * For each quorum:
-    * If stale stakes are forbidden (see [`BLSSignatureChecker.setStaleStakesForbidden`](#blssignaturecheckersetstalestakesforbidden)), check the last `quorumUpdateBlockNumber` is within `DelegationManager.minWithdrawalDelayBlocks` of `referenceBlockNumber`. This references a value in the EigenLayer core contracts - see [EigenLayer core docs][core-docs-dev] for more info.
     * Validate that each `params.quorumApks` corresponds to the quorum's apk at the `referenceBlockNumber`
 * For each historical state lookup, the `referenceBlockNumber` and provided index MUST point to a valid historical entry: 
     * `referenceBlockNumber` MUST come after the entry's `updateBlockNumber`
@@ -172,28 +171,4 @@ For each quorum, this returns:
 * `uint32[] nonSignerQuorumBitmapIndices`: The indices in `RegistryCoordinator._operatorBitmapHistory` where each nonsigner's registered quorum bitmap can be found at `referenceBlockNumber`. Length is equal to the number of nonsigners included in `nonSignerOperatorIds`
 * `uint32[] quorumApkIndices`: The indices in `BLSApkRegistry.apkHistory` where the quorum's apk can be found at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`.
 * `uint32[] totalStakeIndices`: The indices in `StakeRegistry._totalStakeHistory` where each quorum's total stake can be found at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`.
-* `uint32[][] nonSignerStakeIndices`: For each quorum, a list of the indices of each nonsigner's `StakeRegistry.operatorStakeHistory` entry at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`, and each sub-list is equal in length to the number of nonsigners in `nonSignerOperatorIds` registered for that quorum at `referenceBlockNumber`
-
----
-
-### System Configuration
-
-#### `BLSSignatureChecker.setStaleStakesForbidden`
-
-```solidity
-function setStaleStakesForbidden(
-    bool value
-) 
-    external 
-    onlyCoordinatorOwner
-```
-
-This method allows the `RegistryCoordinator` Owner to update `staleStakesForbidden` in the `BLSSignatureChecker`. If stale stakes are forbidden, `BLSSignatureChecker.checkSignatures` will perform an additional check when querying each quorum's apk, Operator stakes, and total stakes.
-
-This additional check requires that each quorum was updated within a certain block window of the `referenceBlockNumber` passed into `BLSSignatureChecker.checkSignatures`.
-
-*Effects*:
-* Sets `staleStakesForbidden` to `value`
-
-*Requirements*:
-* Caller MUST be the `RegistryCoordinator` Owner
+* `uint32[][] nonSignerStakeIndices`: For each quorum, a list of the indices of each nonsigner's `StakeRegistry.operatorStakeHistory` entry at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`, and each sub-list is equal in length to the number of nonsigners in `nonSignerOperatorIds` registered for that quorum at `referenceBlockNumber`
diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol
@@ -104,7 +104,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
             )
         ) % BN254.FR_MODULUS;
 
-        // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P')
+        // e(sigma + P * gamma, [1]_2) = e(H(m) + [1]_1 * gamma, P')
         require(
             BN254.pairing(
                 params.pubkeyRegistrationSignature.plus(params.pubkeyG1.scalar_mul(gamma)),
@@ -314,7 +314,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
     }
 
     function _checkRegistryCoordinator() internal view {
-        require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinatorOwner());
+        require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinator());
     }
 
     function _checkRegistryCoordinatorOwner() internal view {
diff --git a/src/BLSSignatureChecker.sol b/src/BLSSignatureChecker.sol
@@ -33,15 +33,6 @@ contract BLSSignatureChecker is BLSSignatureCheckerStorage {
         ISlashingRegistryCoordinator _registryCoordinator
     ) BLSSignatureCheckerStorage(_registryCoordinator) {}
 
-    /// ACTIONS
-
-    /// @inheritdoc IBLSSignatureChecker
-    function setStaleStakesForbidden(
-        bool value
-    ) external onlyCoordinatorOwner {
-        _setStaleStakesForbidden(value);
-    }
-
     /// VIEW
 
     /// @inheritdoc IBLSSignatureChecker
@@ -138,21 +129,7 @@ contract BLSSignatureChecker is BLSSignatureCheckerStorage {
          * - subtract the stake for each nonsigner to calculate the stake belonging to signers
          */
         {
-            bool _staleStakesForbidden = staleStakesForbidden;
-            uint256 withdrawalDelayBlocks =
-                _staleStakesForbidden ? delegation.minWithdrawalDelayBlocks() : 0;
-
             for (uint256 i = 0; i < quorumNumbers.length; i++) {
-                // If we're disallowing stale stake updates, check that each quorum's last update block
-                // is within withdrawalDelayBlocks
-                if (_staleStakesForbidden) {
-                    require(
-                        registryCoordinator.quorumUpdateBlockNumber(uint8(quorumNumbers[i]))
-                            + withdrawalDelayBlocks > referenceBlockNumber,
-                        StaleStakesForbidden()
-                    );
-                }
-
                 // Validate params.quorumApks is correct for this quorum at the referenceBlockNumber,
                 // then add it to the total apk
                 require(
@@ -244,11 +221,4 @@ contract BLSSignatureChecker is BLSSignatureCheckerStorage {
             PAIRING_EQUALITY_CHECK_GAS
         );
     }
-
-    function _setStaleStakesForbidden(
-        bool value
-    ) internal {
-        staleStakesForbidden = value;
-        emit StaleStakesForbiddenUpdate(value);
-    }
 }
diff --git a/src/BLSSignatureCheckerStorage.sol b/src/BLSSignatureCheckerStorage.sol
@@ -21,8 +21,8 @@ abstract contract BLSSignatureCheckerStorage is IBLSSignatureChecker {
 
     /// STATE
 
-    /// @inheritdoc IBLSSignatureChecker
-    bool public staleStakesForbidden;
+    /// @dev Deprecated storage for the staleStakesForbidden flag.
+    bool internal __deprecated_staleStakesForbidden;
 
     constructor(
         ISlashingRegistryCoordinator _registryCoordinator
diff --git a/src/EjectionManager.sol b/src/EjectionManager.sol
@@ -62,16 +62,6 @@ contract EjectionManager is OwnableUpgradeable, EjectionManagerStorage {
                     ) {
                         ratelimitHit = true;
 
-                        stakeForEjection += operatorStake;
-                        ++ejectedOperators;
-
-                        slashingRegistryCoordinator.ejectOperator(
-                            slashingRegistryCoordinator.getOperatorFromId(operatorIds[i][j]),
-                            abi.encodePacked(quorumNumber)
-                        );
-
-                        emit OperatorEjected(operatorIds[i][j], quorumNumber);
-
                         break;
                     }
 
diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol
@@ -187,10 +187,9 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, RegistryCoordinator
         // filter out M2 quorums from the quorum numbers
         uint256 operatorSetBitmap =
             quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap());
-        if (!operatorSetBitmap.isEmpty()) {
-            // call the parent _forceDeregisterOperator function for operator sets quorums
-            super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray());
-        }
+
+        // call the parent _forceDeregisterOperator function for operator sets quorums
+        super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray());
     }
 
     /// @dev Hook to prevent any new quorums from being created if operator sets are not enabled
@@ -235,10 +234,7 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, RegistryCoordinator
 
     /**
      * @dev Helper function to update operator stakes and deregister operators with insufficient stake
-     * This function handles two cases:
-     * 1. Operators who no longer meet the minimum stake requirement for a quorum
-     * 2. Operators who have been force-deregistered from the AllocationManager but not from this contract
-     * (e.g. due to out of gas errors in the deregistration callback)
+     * This function handles Operators who no longer meet the minimum stake requirement for a quorum
      * @param operators The list of operators to check and update
      * @param operatorIds The corresponding operator IDs
      * @param quorumNumber The quorum number to check stakes for
diff --git a/src/ServiceManagerBase.sol b/src/ServiceManagerBase.sol
@@ -28,7 +28,6 @@ import {ISlashingRegistryCoordinator} from "./interfaces/ISlashingRegistryCoordi
 import {IStakeRegistry} from "./interfaces/IStakeRegistry.sol";
 
 import {BitmapUtils} from "./libraries/BitmapUtils.sol";
-import {LibMergeSort} from "./libraries/LibMergeSort.sol";
 
 /**
  * @title Minimal implementation of a ServiceManager-type contract.
diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol
@@ -175,24 +175,13 @@ contract SlashingRegistryCoordinator is
         bytes32 operatorId = _getOrCreateOperatorId(operator, params);
 
         if (registrationType == RegistrationType.NORMAL) {
-            uint32[] memory numOperatorsPerQuorum = _registerOperator({
+            _registerOperator({
                 operator: operator,
                 operatorId: operatorId,
                 quorumNumbers: quorumNumbers,
                 socket: socket,
                 checkMaxOperatorCount: true
             }).numOperatorsPerQuorum;
-
-            // For each quorum, validate that the new operator count does not exceed the maximum
-            // (If it does, an operator needs to be replaced -- see `registerOperatorWithChurn`)
-            for (uint256 i = 0; i < quorumNumbers.length; i++) {
-                uint8 quorumNumber = uint8(quorumNumbers[i]);
-
-                require(
-                    numOperatorsPerQuorum[i] <= _quorumParams[quorumNumber].maxOperatorCount,
-                    MaxOperatorCountReached()
-                );
-            }
         } else if (registrationType == RegistrationType.CHURN) {
             // Decode registration data from bytes
             (
@@ -677,6 +666,7 @@ contract SlashingRegistryCoordinator is
     /**
      * @notice Validates that an incoming operator is eligible to replace an existing
      * operator based on the stake of both
+     * @dev In order to be churned out, the existing operator must be registered for the quorum
      * @dev In order to churn, the incoming operator needs to have more stake than the
      * existing operator by a proportion given by `kickBIPsOfOperatorStake`
      * @dev In order to be churned out, the existing operator needs to have a proportion
@@ -705,6 +695,13 @@ contract SlashingRegistryCoordinator is
         require(newOperator != operatorToKick, CannotChurnSelf());
         require(kickParams.quorumNumber == quorumNumber, QuorumOperatorCountMismatch());
 
+        uint192 quorumBitmap;
+        quorumBitmap = uint192(BitmapUtils.setBit(quorumBitmap, quorumNumber));
+        require(
+            quorumBitmap.isSubsetOf(_currentOperatorBitmap(idToKick)),
+            OperatorNotRegisteredForQuorum()
+        );
+
         // Get the target operator's stake and check that it is below the kick thresholds
         uint96 operatorToKickStake = stakeRegistry.getCurrentStake(idToKick, quorumNumber);
         require(
@@ -830,7 +827,8 @@ contract SlashingRegistryCoordinator is
         } else if (stakeType == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE) {
             // For slashable stake quorums, ensure lookAheadPeriod is less than DEALLOCATION_DELAY
             require(
-                AllocationManager(address(allocationManager)).DEALLOCATION_DELAY() > lookAheadPeriod,
+                lookAheadPeriod
+                    <= AllocationManager(address(allocationManager)).DEALLOCATION_DELAY(),
                 LookAheadPeriodTooLong()
             );
             stakeRegistry.initializeSlashableStakeQuorum(
diff --git a/src/StakeRegistry.sol b/src/StakeRegistry.sol
@@ -9,6 +9,7 @@ import {IAVSDirectory} from "eigenlayer-contracts/src/contracts/interfaces/IAVSD
 import {OperatorSet} from "eigenlayer-contracts/src/contracts/interfaces/IAllocationManager.sol";
 import {IAllocationManager} from
     "eigenlayer-contracts/src/contracts/interfaces/IAllocationManager.sol";
+import {AllocationManager} from "eigenlayer-contracts/src/contracts/core/AllocationManager.sol";
 
 import {StakeRegistryStorage, IStrategy} from "./StakeRegistryStorage.sol";
 
@@ -137,8 +138,7 @@ contract StakeRegistry is StakeRegistryStorage {
          * in the quorum's total stake.
          *
          * If the operator no longer has the minimum stake required to be registered
-         * in the quorum, the quorum number is added to `quorumsToRemove`, which
-         * is returned to the registry coordinator.
+         * in the quorum, the operator is marked for removal.
          */
         _checkQuorumExists(quorumNumber);
 
@@ -796,6 +796,10 @@ contract StakeRegistry is StakeRegistryStorage {
             stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE,
             QuorumNotSlashable()
         );
+        require(
+            _lookAheadBlocks <= AllocationManager(address(allocationManager)).DEALLOCATION_DELAY(),
+            LookAheadPeriodTooLong()
+        );
         uint32 oldLookAheadDays = slashableStakeLookAheadPerQuorum[quorumNumber];
         slashableStakeLookAheadPerQuorum[quorumNumber] = _lookAheadBlocks;
         emit LookAheadPeriodChanged(oldLookAheadDays, _lookAheadBlocks);
diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol
@@ -4,8 +4,10 @@ pragma solidity ^0.8.27;
 import {BN254} from "../libraries/BN254.sol";
 
 interface IBLSApkRegistryErrors {
-    /// @notice Thrown when a non-RegistryCoordinator address calls a restricted function.
+    /// @notice Thrown when a non-RegistryCoordinator owner address calls a restricted function.
     error OnlyRegistryCoordinatorOwner();
+    /// @notice Thrown when a non-RegistryCoordinator address calls a restricted function.
+    error OnlyRegistryCoordinator();
     /// @notice Thrown when attempting to initialize a quorum that already exists.
     error QuorumAlreadyExists();
     /// @notice Thrown when a quorum does not exist.
diff --git a/src/interfaces/IBLSSignatureChecker.sol b/src/interfaces/IBLSSignatureChecker.sol
@@ -20,8 +20,6 @@ interface IBLSSignatureCheckerErrors {
     error InvalidReferenceBlocknumber();
     /// @notice Thrown when the non signer pubkeys are not sorted.
     error NonSignerPubkeysNotSorted();
-    /// @notice Thrown when StakeRegistry updates have not been updated within withdrawalDelayBlocks window
-    error StaleStakesForbidden();
     /// @notice Thrown when the quorum apk hash in storage does not match provided quorum apk.
     error InvalidQuorumApkHash();
     /// @notice Thrown when BLS pairing precompile call fails.
@@ -70,12 +68,7 @@ interface IBLSSignatureCheckerTypes {
     }
 }
 
-interface IBLSSignatureCheckerEvents is IBLSSignatureCheckerTypes {
-    /// @notice Emitted when `staleStakesForbiddenUpdate` is set.
-    event StaleStakesForbiddenUpdate(bool value);
-}
-
-interface IBLSSignatureChecker is IBLSSignatureCheckerErrors, IBLSSignatureCheckerEvents {
+interface IBLSSignatureChecker is IBLSSignatureCheckerErrors, IBLSSignatureCheckerTypes {
     /* STATE */
 
     /*
@@ -106,23 +99,6 @@ interface IBLSSignatureChecker is IBLSSignatureCheckerErrors, IBLSSignatureCheck
      */
     function delegation() external view returns (IDelegationManager);
 
-    /*
-     * @notice Returns whether stale stakes are forbidden in signature verification.
-     * @return True if stale stakes are forbidden, false otherwise.
-     */
-    function staleStakesForbidden() external view returns (bool);
-
-    /* ACTIONS */
-
-    /*
-     * @notice Sets `value` as the new staleStakesForbidden flag.
-     * @param value True to forbid stale stakes, false to allow them.
-     * @dev Access restricted to the registry coordinator owner.
-     */
-    function setStaleStakesForbidden(
-        bool value
-    ) external;
-
     /* VIEW */
 
     /*
diff --git a/src/interfaces/ISlashingRegistryCoordinator.sol b/src/interfaces/ISlashingRegistryCoordinator.sol
@@ -59,6 +59,8 @@ interface ISlashingRegistryCoordinatorErrors {
     error LookAheadPeriodTooLong();
     /// @notice Thrown when the number of operators in a quorum would exceed the maximum allowed.
     error MaxOperatorCountReached();
+    /// @notice Thrown when the operator is not registered for the quorum.
+    error OperatorNotRegisteredForQuorum();
 }
 
 interface ISlashingRegistryCoordinatorTypes {
diff --git a/src/interfaces/IStakeRegistry.sol b/src/interfaces/IStakeRegistry.sol
@@ -31,6 +31,8 @@ interface IStakeRegistryErrors {
     error EmptyStakeHistory();
     /// @notice Thrown when the quorum is not slashable and the caller attempts to set the look ahead period.
     error QuorumNotSlashable();
+    /// @notice Thrown when the look ahead period is too long.
+    error LookAheadPeriodTooLong();
 }
 
 interface IStakeRegistryTypes {
diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol
@@ -287,7 +287,7 @@ contract BLSApkRegistryUnitTests_configAndGetters is BLSApkRegistryUnitTests {
         cheats.assume(nonCoordinatorAddress != address(registryCoordinator));
 
         cheats.prank(address(nonCoordinatorAddress));
-        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinatorOwner.selector);
+        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinator.selector);
         blsApkRegistry.initializeQuorum(defaultQuorumNumber);
     }
 }
@@ -306,7 +306,7 @@ contract BLSApkRegistryUnitTests_registerBLSPublicKey is BLSApkRegistryUnitTests
             registryCoordinator.pubkeyRegistrationMessageHash(defaultOperator);
 
         cheats.prank(address(nonCoordinatorAddress));
-        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinatorOwner.selector);
+        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinator.selector);
         blsApkRegistry.registerBLSPublicKey(defaultOperator, pubkeyRegistrationParams, messageHash);
     }
 
@@ -434,7 +434,7 @@ contract BLSApkRegistryUnitTests_registerOperator is BLSApkRegistryUnitTests {
         cheats.assume(nonCoordinatorAddress != address(registryCoordinator));
 
         cheats.prank(nonCoordinatorAddress);
-        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinatorOwner.selector);
+        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinator.selector);
         blsApkRegistry.registerOperator(nonCoordinatorAddress, new bytes(0));
     }
 
@@ -537,7 +537,7 @@ contract BLSApkRegistryUnitTests_deregisterOperator is BLSApkRegistryUnitTests {
         cheats.assume(nonCoordinatorAddress != address(registryCoordinator));
 
         cheats.prank(nonCoordinatorAddress);
-        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinatorOwner.selector);
+        cheats.expectRevert(IBLSApkRegistryErrors.OnlyRegistryCoordinator.selector);
         blsApkRegistry.deregisterOperator(nonCoordinatorAddress, new bytes(0));
     }
 
diff --git a/test/unit/BLSSignatureCheckerUnit.t.sol b/test/unit/BLSSignatureCheckerUnit.t.sol
diff --git a/test/unit/EjectionManagerUnit.t.sol b/test/unit/EjectionManagerUnit.t.sol
diff --git a/test/unit/SlashingRegistryCoordinatorUnit.t.sol b/test/unit/SlashingRegistryCoordinatorUnit.t.sol
diff --git a/test/unit/StakeRegistryUnit.t.sol b/test/unit/StakeRegistryUnit.t.sol

Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {`
`104`	`104`	`)`
`105`	`105`	`) % BN254.FR_MODULUS;`
`106`	`106`
`107`		`- // e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P')`
	`107`	`+ // e(sigma + P * gamma, [1]_2) = e(H(m) + [1]_1 * gamma, P')`
`108`	`108`	`require(`
`109`	`109`	`BN254.pairing(`
`110`	`110`	`params.pubkeyRegistrationSignature.plus(params.pubkeyG1.scalar_mul(gamma)),`
`@@ -314,7 +314,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {`
`314`	`314`	`}`
`315`	`315`
`316`	`316`	`function _checkRegistryCoordinator() internal view {`
`317`		`- require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinatorOwner());`
	`317`	`+ require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinator());`
`318`	`318`	`}`
`319`	`319`
`320`	`320`	`function _checkRegistryCoordinatorOwner() internal view {`