test: fixes and refactor of weightOfOperators

8sunyuan · 8sunyuan · commit bde687d56de9 · 2025-02-05T16:13:32.000-05:00
diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol
@@ -731,17 +731,17 @@ contract SlashingRegistryCoordinator is
         IStakeRegistryTypes.StakeType stakeType,
         uint32 lookAheadPeriod
     ) internal {
-        // Increment the total quorum count. Fails if we're already at the max
-        uint8 prevQuorumCount = quorumCount;
-        require(prevQuorumCount < MAX_QUORUM_COUNT, MaxQuorumsReached());
-        quorumCount = prevQuorumCount + 1;
-
-        // The previous count is the new quorum's number
-        uint8 quorumNumber = prevQuorumCount;
+        // The previous quorum count is the new quorum's number,
+        // this is because quorum numbers begin from index 0.
+        uint8 quorumNumber = quorumCount;
 
         // Hook to allow for any pre-create quorum logic
         _beforeCreateQuorum(quorumNumber);
 
+        // Increment the total quorum count. Fails if we're already at the max
+        require(quorumNumber < MAX_QUORUM_COUNT, MaxQuorumsReached());
+        quorumCount += 1;
+
         // Initialize the quorum here and in each registry
         _setOperatorSetParams(quorumNumber, operatorSetParams);
 
diff --git a/src/StakeRegistry.sol b/src/StakeRegistry.sol
@@ -534,11 +534,11 @@ contract StakeRegistry is StakeRegistryStorage {
         uint8 quorumNumber,
         address[] memory operators
     ) internal view virtual returns (uint96[] memory, bool[] memory) {
-        uint96[] memory weights;
-        bool[] memory hasMinimumStakes;
+        uint96[] memory weights = new uint96[](operators.length);
+        bool[] memory hasMinimumStakes = new bool[](operators.length);
 
         uint256 stratsLength = strategyParamsLength(quorumNumber);
-        StrategyParams memory strategyAndMultiplier;
+        StrategyParams[] memory stratsAndMultipliers = strategyParams[quorumNumber];
         uint256[][] memory strategyShares;
 
         if (stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE) {
@@ -550,12 +550,15 @@ contract StakeRegistry is StakeRegistryStorage {
                 delegation.getOperatorsShares(operators, strategiesPerQuorum[quorumNumber]);
         }
 
-        for (uint256 stratIndex = 0; stratIndex < stratsLength; stratIndex++) {
-            // accessing i'th StrategyParams struct for the quorumNumber
-            strategyAndMultiplier = strategyParams[quorumNumber][stratIndex];
+        // Calculate weight of each operator and whether they contain minimum stake for the quorum
+        for (uint256 opIndex = 0; opIndex < operators.length; opIndex++) {
+            // 1. For the given operator, loop through the strategies and calculate the operator's
+            // weight for the quorum
+            for (uint256 stratIndex = 0; stratIndex < stratsLength; stratIndex++) {
+                // get multiplier for strategy
+                StrategyParams memory strategyAndMultiplier = stratsAndMultipliers[stratIndex];
 
-            for (uint256 opIndex = 0; opIndex < operators.length; opIndex++) {
-                // add the weight from the shares for this strategy to the total weight
+                // calculate added weight for strategy and multiplier
                 if (strategyShares[opIndex][stratIndex] > 0) {
                     weights[opIndex] += uint96(
                         strategyShares[opIndex][stratIndex] * strategyAndMultiplier.multiplier
@@ -564,9 +567,8 @@ contract StakeRegistry is StakeRegistryStorage {
                 }
             }
 
-            // check if the operator meets the quorum's minimum stake
-            hasMinimumStakes[stratIndex] =
-                weights[stratIndex] >= minimumStakeForQuorum[quorumNumber];
+            // 2. Check whether operator is above minimum stake threshold
+            hasMinimumStakes[opIndex] = weights[opIndex] >= minimumStakeForQuorum[quorumNumber];
         }
 
         return (weights, hasMinimumStakes);
diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol
@@ -79,4 +79,8 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test {
     ) external {
         isM2QuorumRegistrationDisabled = disabled;
     }
+
+    function setM2QuorumBitmap(uint256 bitmap) external {
+        _m2QuorumBitmap = bitmap;
+    }
 }
diff --git a/test/mocks/DelegationMock.sol b/test/mocks/DelegationMock.sol
@@ -282,6 +282,20 @@ contract DelegationMock is DelegationIntermediate {
         return shares;
     }
 
+    function getOperatorsShares(
+        address[] memory operators,
+        IStrategy[] memory strategies
+    ) external view override returns (uint256[][] memory) {
+        uint256[][] memory operatorSharesArray = new uint256[][](operators.length);
+        for (uint256 i = 0; i < operators.length; i++) {
+            operatorSharesArray[i] = new uint256[](strategies.length);
+            for (uint256 j = 0; j < strategies.length; j++) {
+                operatorSharesArray[i][j] = _weightOf[operators[i]][strategies[j]];
+            }
+        }
+        return operatorSharesArray;
+    }
+
     function minWithdrawalDelayBlocks() external view override returns (uint32) {
         return 10000;
     }
diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol
@@ -8,6 +8,12 @@ import {
     ISlashingRegistryCoordinatorErrors
 } from "../../src/interfaces/ISlashingRegistryCoordinator.sol";
 
+import {
+    IRegistryCoordinator,
+    IRegistryCoordinatorTypes,
+    IRegistryCoordinatorErrors
+} from "../../src/interfaces/IRegistryCoordinator.sol";
+
 import {IBLSApkRegistryTypes} from "../../src/interfaces/IBLSApkRegistry.sol";
 import {QuorumBitmapHistoryLib} from "../../src/libraries/QuorumBitmapHistoryLib.sol";
 import {BitmapUtils} from "../../src/libraries/BitmapUtils.sol";
@@ -302,7 +308,7 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni
 
         quorumNumbersTooLarge[0] = 0xC0;
 
-        cheats.expectRevert(BitmapUtils.BitmapValueTooLarge.selector);
+        cheats.expectRevert(IRegistryCoordinatorErrors.OnlyM2QuorumsAllowed.selector);
         cheats.prank(defaultOperator);
         registryCoordinator.registerOperator(
             quorumNumbersTooLarge, defaultSocket, pubkeyRegistrationParams, emptySig
@@ -317,7 +323,7 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni
         quorumNumbersNotCreated[0] = 0x0B;
 
         cheats.prank(defaultOperator);
-        cheats.expectRevert(BitmapUtils.BitmapValueTooLarge.selector);
+        cheats.expectRevert(IRegistryCoordinatorErrors.OnlyM2QuorumsAllowed.selector);
         registryCoordinator.registerOperator(
             quorumNumbersNotCreated, defaultSocket, pubkeyRegistrationParams, emptySig
         );
@@ -395,6 +401,8 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni
 
         cheats.expectEmit(true, true, true, true, address(registryCoordinator));
         emit OperatorSocketUpdate(defaultOperatorId, defaultSocket);
+        cheats.expectEmit(true, true, true, true, address(registryCoordinator));
+        emit OperatorRegistered(defaultOperator, defaultOperatorId);
         cheats.expectEmit(true, true, true, true, address(blsApkRegistry));
         emit OperatorAddedToQuorums(defaultOperator, defaultOperatorId, quorumNumbers);
         for (uint256 i = 0; i < quorumNumbers.length; i++) {
@@ -405,8 +413,6 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni
             cheats.expectEmit(true, true, true, true, address(indexRegistry));
             emit QuorumIndexUpdate(defaultOperatorId, uint8(quorumNumbers[i]), 0);
         }
-        cheats.expectEmit(true, true, true, true, address(registryCoordinator));
-        emit OperatorRegistered(defaultOperator, defaultOperatorId);
 
         uint256 gasBefore = gasleft();
         cheats.prank(defaultOperator);
@@ -636,6 +642,8 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni
 
         cheats.expectEmit(true, true, true, true, address(registryCoordinator));
         emit OperatorSocketUpdate(defaultOperatorId, defaultSocket);
+        cheats.expectEmit(true, true, true, true, address(registryCoordinator));
+        emit OperatorRegistered(defaultOperator, defaultOperatorId);
         cheats.expectEmit(true, true, true, true, address(blsApkRegistry));
         emit OperatorAddedToQuorums(defaultOperator, defaultOperatorId, quorumNumbers);
         cheats.expectEmit(true, true, true, true, address(stakeRegistry));
@@ -1707,6 +1715,9 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord
         cheats.expectEmit(true, true, true, true, address(registryCoordinator));
         emit OperatorSocketUpdate(operatorToRegisterId, defaultSocket);
 
+        cheats.expectEmit(true, true, true, true, address(registryCoordinator));
+        emit OperatorRegistered(operatorToRegister, operatorToRegisterId);
+
         cheats.expectEmit(true, true, true, true, address(blsApkRegistry));
         emit OperatorAddedToQuorums(operatorToRegister, operatorToRegisterId, quorumNumbers);
         cheats.expectEmit(true, true, true, false, address(stakeRegistry));
@@ -1725,8 +1736,6 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord
         emit OperatorStakeUpdate(operatorToKickId, defaultQuorumNumber, 0);
         cheats.expectEmit(true, true, true, true, address(indexRegistry));
         emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1);
-        cheats.expectEmit(true, true, true, true, address(registryCoordinator));
-        emit OperatorRegistered(operatorToRegister, operatorToRegisterId);
 
         {
             ISignatureUtils.SignatureWithSaltAndExpiry memory emptyAVSRegSig;
@@ -2394,12 +2403,26 @@ contract RegistryCoordinatorUnitTests_BeforeMigration is RegistryCoordinatorUnit
         });
         uint32 lookAheadPeriod = 100;
 
+        assertEq(
+            registryCoordinator.quorumCount(),
+            0,
+            "No quorums should exist before"
+        );
+
         // Attempt to create quorum with slashable stake type before enabling operator sets
         cheats.prank(registryCoordinatorOwner);
-        cheats.expectRevert();
         registryCoordinator.createSlashableStakeQuorum(
             operatorSetParams, minimumStake, strategyParams, lookAheadPeriod
         );
+        assertEq(
+            registryCoordinator.quorumCount(),
+            1,
+            "New quorum 0 should be created"
+        );
+        assertFalse(
+            registryCoordinator.isM2Quorum(0),
+            "Quorum created should not be an M2 quorum"
+        );
     }
 }
 
@@ -2744,8 +2767,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT
     function test_deregisterHook_Reverts_WhenNotALM() public {
         _deployMockEigenLayerAndAVS(0);
 
-        assertTrue(registryCoordinator.operatorSetsEnabled(), "operatorSetsEnabled should be true");
-
         // Create quorum params
         ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams =
         ISlashingRegistryCoordinatorTypes.OperatorSetParam({
@@ -2764,6 +2785,9 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT
         cheats.prank(registryCoordinatorOwner);
         registryCoordinator.createTotalDelegatedStakeQuorum(operatorSetParams, 0, strategyParams);
 
+        // operator sets should be enabled after creating a new quorum
+        assertTrue(registryCoordinator.operatorSetsEnabled(), "operatorSetsEnabled should be true");
+
         // Prank as allocation manager and call register hook
         uint32[] memory operatorSetIds = new uint32[](1);
         operatorSetIds[0] = 0;
diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol
@@ -357,6 +357,8 @@ contract MockAVSDeployer is Test {
 
         operatorStateRetriever = new OperatorStateRetriever();
 
+        // Set RegistryCoordinator as M2 state with existing quorums
+        registryCoordinator.setM2QuorumBitmap(0);
         registryCoordinator.setOperatorSetsEnabled(false);
         registryCoordinator.setM2QuorumRegistrationDisabled(false);
     }

Original file line number	Diff line number	Diff line change
`@@ -79,4 +79,8 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test {`
`79`	`79`	`) external {`
`80`	`80`	`isM2QuorumRegistrationDisabled = disabled;`
`81`	`81`	`}`
	`82`	`+`
	`83`	`+ function setM2QuorumBitmap(uint256 bitmap) external {`
	`84`	`+ _m2QuorumBitmap = bitmap;`
	`85`	`+ }`
`82`	`86`	`}`
Original file line number	Diff line number	Diff line change
`@@ -357,6 +357,8 @@ contract MockAVSDeployer is Test {`
`357`	`357`
`358`	`358`	`operatorStateRetriever = new OperatorStateRetriever();`
`359`	`359`
	`360`	`+ // Set RegistryCoordinator as M2 state with existing quorums`
	`361`	`+ registryCoordinator.setM2QuorumBitmap(0);`
`360`	`362`	`registryCoordinator.setOperatorSetsEnabled(false);`
`361`	`363`	`registryCoordinator.setM2QuorumRegistrationDisabled(false);`
`362`	`364`	`}`