Fixes for BLSSigCheckOperatorStateRetriever

src/unaudited/BLSSigCheckOperatorStateRetriever.sol

@@ -10,13 +10,18 @@ import {BitmapUtils} from "../libraries/BitmapUtils.sol";
 import {BN254} from "../libraries/BN254.sol";
 import {BN256G2} from "./BN256G2.sol";
 import {OperatorStateRetriever} from "../OperatorStateRetriever.sol";
+import {ECUtils} from "./ECUtils.sol";
 
 /**
  * @title BLSSigCheckOperatorStateRetriever with view functions that allow to retrieve the state of an AVSs registry system.
  * @dev This contract inherits from OperatorStateRetriever and adds the getNonSignerStakesAndSignature function.
  * @author Bread coop
  */
 contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
+    using ECUtils for BN254.G1Point;
+    using BN254 for BN254.G1Point;
+    using BitmapUtils for uint256;
+
     /// @dev Thrown when the signature is not on the curve.
     error InvalidSigma();
     // avoid stack too deep
@@ -61,7 +66,7 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
         m.blsApkRegistry = registryCoordinator.blsApkRegistry();
 
         // Safe guard AVSs from generating NonSignerStakesAndSignature with invalid sigma
-        require(_isOnCurve(sigma), InvalidSigma());
+        require(sigma.isOnCurve(), InvalidSigma());
 
         // Compute the g2 APK of the signing operator set
         m.signingOperatorIds = new bytes32[](operators.length);
@@ -84,13 +89,17 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
         {
             uint32[] memory signingOperatorQuorumBitmapIndices = registryCoordinator
                 .getQuorumBitmapIndicesAtBlockNumber(blockNumber, m.signingOperatorIds);
+            uint256 bitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers);
             // Check that all operators are registered (this is like the check in getCheckSignaturesIndices, but we check against _signing_ operators)
             for (uint256 i = 0; i < operators.length; i++) {
                 uint192 signingOperatorQuorumBitmap = registryCoordinator
                     .getQuorumBitmapAtBlockNumberByIndex(
                     m.signingOperatorIds[i], blockNumber, signingOperatorQuorumBitmapIndices[i]
                 );
-                require(signingOperatorQuorumBitmap != 0, OperatorNotRegistered());
+                require(
+                    !uint256(signingOperatorQuorumBitmap).noBitsInCommon(bitmap),
+                    OperatorNotRegistered()
+                );
             }
         }
 
@@ -141,12 +150,9 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
 
         // Trim the nonSignerOperatorIds array to the actual count
         bytes32[] memory trimmedNonSignerOperatorIds = new bytes32[](nonSignerOperatorsCount);
-        for (uint256 i = 0; i < nonSignerOperatorsCount; i++) {
-            trimmedNonSignerOperatorIds[i] = nonSignerOperatorIds[i];
-        }
-
         BN254.G1Point[] memory nonSignerPubkeys = new BN254.G1Point[](nonSignerOperatorsCount);
         for (uint256 i = 0; i < nonSignerOperatorsCount; i++) {
+            trimmedNonSignerOperatorIds[i] = nonSignerOperatorIds[i];
             address nonSignerOperator =
                 registryCoordinator.getOperatorFromId(trimmedNonSignerOperatorIds[i]);
             (nonSignerPubkeys[i],) = m.blsApkRegistry.getRegisteredPubkey(nonSignerOperator);
@@ -184,24 +190,8 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
             address operator = registryCoordinator.getOperatorFromId(operatorIds[i]);
             BN254.G1Point memory operatorPk;
             (operatorPk.X, operatorPk.Y) = blsApkRegistry.operatorToPubkey(operator);
-            apk = BN254.plus(apk, operatorPk);
+            apk = apk.plus(operatorPk);
         }
         return apk;
     }
-
-    /**
-     * @notice Checks if a point lies on the BN254 elliptic curve
-     * @dev The curve equation is y^2 = x^3 + 3 (mod p)
-     * @param p The point to check, in G1
-     * @return true if the point lies on the curve, false otherwise
-     */
-    function _isOnCurve(
-        BN254.G1Point memory p
-    ) internal pure returns (bool) {
-        uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS);
-        uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS);
-        uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS);
-        uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS);
-        return y2 == rhs;
-    }
 }

src/unaudited/ECUtils.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.12;
+
+import {BN254} from "../libraries/BN254.sol";
+
+/**
+ * @title ECUtils
+ * @notice Library containing utility functions for elliptic curve operations
+ */
+library ECUtils {
+    /**
+     * @notice Checks if a point lies on the BN254 elliptic curve
+     * @dev The curve equation is y^2 = x^3 + 3 (mod p)
+     * @param p The point to check, in G1
+     * @return true if the point lies on the curve, false otherwise
+     */
+    function isOnCurve(
+        BN254.G1Point memory p
+    ) internal pure returns (bool) {
+        uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS);
+        uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS);
+        uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS);
+        uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS);
+        return y2 == rhs;
+    }
+}

test/unit/BLSSigCheckOperatorStateRetriever.t.sol

@@ -526,8 +526,9 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
         signingOperators[0] = defaultOperator;
 
         // Try to query for a non-existent quorum (quorum 9)
-        bytes memory invalidQuorumNumbers = new bytes(1);
-        invalidQuorumNumbers[0] = bytes1(uint8(9)); // Invalid quorum number
+        bytes memory invalidQuorumNumbers = new bytes(2);
+        invalidQuorumNumbers[0] = bytes1(uint8(0));
+        invalidQuorumNumbers[1] = bytes1(uint8(9)); // Invalid quorum number
 
         // Should revert because quorum 9 doesn't exist, but with a different error message
         cheats.expectRevert(
@@ -593,8 +594,9 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
         signingOperators[0] = defaultOperator;
 
         // Try to query for the newly created quorum but at a historical block
-        bytes memory newQuorumNumbers = new bytes(1);
-        newQuorumNumbers[0] = bytes1(uint8(numQuorums));
+        bytes memory newQuorumNumbers = new bytes(2);
+        newQuorumNumbers[0] = bytes1(uint8(0));
+        newQuorumNumbers[1] = bytes1(uint8(8));
 
         // Should revert when querying for the newly created quorum at a block before it was created
         cheats.expectRevert(
@@ -607,6 +609,54 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
         );
     }
 
+    function test_getNonSignerStakesAndSignature_revert_operatorRegisteredToIrrelevantQuorum()
+        public
+    {
+        // setup
+        uint256 quorumBitmapOne = 1;
+        cheats.roll(registrationBlockNumber);
+
+        _registerOperatorWithCoordinator(defaultOperator, quorumBitmapOne, defaultPubKey);
+
+        address otherOperator = _incrementAddress(defaultOperator, 1);
+        BN254.G1Point memory otherPubKey = BN254.G1Point(1, 2);
+        _registerOperatorWithCoordinator(
+            otherOperator, quorumBitmapOne, otherPubKey, defaultStake - 1
+        );
+
+        // Generate actual G2 pubkeys
+        BN254.G2Point memory op1G2 = _makeG2Point(2);
+        BN254.G2Point memory op2G2 = _makeG2Point(3);
+
+        // Mock the registry calls so the contract sees those G2 points
+        vm.mockCall(
+            address(blsApkRegistry),
+            abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, defaultOperator),
+            abi.encode(op1G2)
+        );
+        vm.mockCall(
+            address(blsApkRegistry),
+            abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, otherOperator),
+            abi.encode(op2G2)
+        );
+
+        // Prepare inputs
+        BN254.G1Point memory dummySigma = BN254.scalar_mul_tiny(BN254.generatorG1(), 123);
+        address[] memory signingOperators = new address[](2);
+        signingOperators[0] = defaultOperator;
+        signingOperators[1] = otherOperator;
+
+        bytes memory quorumNumbers = new bytes(1);
+        quorumNumbers[0] = bytes1(uint8(2));
+
+        // Call the function under test
+        vm.expectRevert(OperatorStateRetriever.OperatorNotRegistered.selector);
+        IBLSSignatureCheckerTypes.NonSignerStakesAndSignature memory result =
+        sigCheckOperatorStateRetriever.getNonSignerStakesAndSignature(
+            registryCoordinator, quorumNumbers, dummySigma, signingOperators, uint32(block.number)
+        );
+    }
+
     function _getApkAtBlocknumber(
         ISlashingRegistryCoordinator registryCoordinator,
         uint8 quorumNumber,