Test: StakeRegistry unit tests (#118)

* test: refactor and add tests to bitmap unit * test: added tests and using asserts * chore: remove single quote * feat: addNumberToBitmap function * test: tree file and Alexs bitmap fix * test: add back updated stakeRegistry tests * test: config tests setMinimumStake tests addStrategies tests initializeQuorum tests * refactor: using MockAVSDeployer for test file * test: more config tests * refactor: stake registry harness weighting * fix: stake weighting refactor removed overriding weightOfOperator functions in StakeRegistry and fixed broken tests using the helper _setOperatorWeight() * test: refactor and add tests to bitmap unit * test: added tests and using asserts * chore: remove single quote * feat: addNumberToBitmap function * test: tree file and Alexs bitmap fix * test: add back updated stakeRegistry tests * test: config tests setMinimumStake tests addStrategies tests initializeQuorum tests * refactor: using MockAVSDeployer for test file * test: more config tests * refactor: stake registry harness weighting * fix: stake weighting refactor removed overriding weightOfOperator functions in StakeRegistry and fixed broken tests using the helper _setOperatorWeight() * fix: undo change * chore: rebase fixes * test: voteweighing tests * chore: updated eigenlayer-contract ref to m2-mainnet head (#119) * updated eigenlayer-contract ref to m2-mainnet head * fixed test - interface mismatch from previous update * docs: update reg coord to include pubkey registration and service man… (#116) * docs: update reg coord to include pubkey registration and service manager usage * docs: add service manager to tech docs intro * fix: update table of contents * docs: add docs for BLSSignatureChecker and OperatorStateRetriever * docs: address feedback * docs: add documentation for each registry (#125) * docs: standardize capitalization of Operator since thats what we do everywhere else * docs: add BLSApkRegistry docs * chore: remove old files - i've incorporated all the info from these files into the current docs, so i'm removing them * docs: add wip for IndexRegistry and StakeRegistry, and fix spacing in StakeRegistry * docs: add IndexRegistry docs * docs: Add StakeRegistry * docs: clarify wording * test: fix unit tests * feat: registry coordinator unit test improvements (#121) * feat: add tree diagram for RegistryManager * chore: reorder and rename tests * feat: add a couple simple tests note that the `test_createQuorum` test is currently failing because initialization already sets up the max number of quorums something will need to be adjusted here * docs: update reg coord to include pubkey registration and service man… (#116) * docs: update reg coord to include pubkey registration and service manager usage * docs: add service manager to tech docs intro * fix: update table of contents * docs: add docs for BLSSignatureChecker and OperatorStateRetriever * docs: address feedback * docs: add documentation for each registry (#125) * docs: standardize capitalization of Operator since thats what we do everywhere else * docs: add BLSApkRegistry docs * chore: remove old files - i've incorporated all the info from these files into the current docs, so i'm removing them * docs: add wip for IndexRegistry and StakeRegistry, and fix spacing in StakeRegistry * docs: add IndexRegistry docs * docs: Add StakeRegistry * docs: clarify wording * chore: fix tree file name * chore: add a couple post-checks on state also fix a couple test names * chore: fix breaking test set up one less than the max number of quorums in a single test's setup, rather than the full max number, so that the test will properly allow the creation of a new quorum also fix a typo in a test name * feat: add tree diagram for RegistryManager * chore: reorder and rename tests * feat: add a couple simple tests note that the `test_createQuorum` test is currently failing because initialization already sets up the max number of quorums something will need to be adjusted here * chore: fix tree file name * chore: add a couple post-checks on state also fix a couple test names * chore: fix breaking test set up one less than the max number of quorums in a single test's setup, rather than the full max number, so that the test will properly allow the creation of a new quorum also fix a typo in a test name * feat: add a test for partial deregistration also improve some formatting + fix some typos, and add a touch more documentation * feat: expose more internal functions in harnessed contract * feat: add some simple coverage for the internal `_registerOperator` fnc * feat: add test coverage for the internal `deregisterOperator` fnc also clarify somewhat ambiguous wording in the tree file * feat: add simple test coverage for the internal `_updateOperatorBitmap` fnc * chore: remove commitlint job that reviews all commits in PR from CI This was causing a *lot* of CI failures, including for merge commits With this commit, CI will still check the _latest_ commit for meeting conventions, it just won't run over all commits in a PR This may lead to a few more "unconventional" commits making it through, but the CI should still flag when someone is just not using conventional commits at all, which I think was the original goal. * feat: add some coverage for `updateOperators(ForQuorums)` fncs * feat: add testing for `updateOperatorsForQuorum` function * feat: add some coverage for complex view functions note: this commit also adds a TODO around a currently-failing test. I plan to discuss the correct path forwards here and then push another commit. * chore: clarify NatSpec comments * fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator was registered the logic is now more in-line with the logic in the StakeRegistry -- for reference, see: https://github.com/layr-labs/eigenlayer-middleware/blob/ 98f8844/src/StakeRegistry.sol#L297-L299 * feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber` also improve wording in the 'tree' file * chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section with other internal fncs * chore: remove unnecessary require statement + improve code clarity * fix: correct a compiler error for implicit type conversion * feat: address TODOs in tests * feat: add tree diagram for RegistryManager * chore: fix tree file name * feat: add a test for partial deregistration also improve some formatting + fix some typos, and add a touch more documentation * feat: expose more internal functions in harnessed contract * feat: add some simple coverage for the internal `_registerOperator` fnc * feat: add test coverage for the internal `deregisterOperator` fnc also clarify somewhat ambiguous wording in the tree file * feat: add simple test coverage for the internal `_updateOperatorBitmap` fnc * chore: remove commitlint job that reviews all commits in PR from CI This was causing a *lot* of CI failures, including for merge commits With this commit, CI will still check the _latest_ commit for meeting conventions, it just won't run over all commits in a PR This may lead to a few more "unconventional" commits making it through, but the CI should still flag when someone is just not using conventional commits at all, which I think was the original goal. * feat: add some coverage for `updateOperators(ForQuorums)` fncs * feat: add testing for `updateOperatorsForQuorum` function * feat: add some coverage for complex view functions note: this commit also adds a TODO around a currently-failing test. I plan to discuss the correct path forwards here and then push another commit. * chore: clarify NatSpec comments * fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator was registered the logic is now more in-line with the logic in the StakeRegistry -- for reference, see: https://github.com/layr-labs/eigenlayer-middleware/blob/ 98f8844/src/StakeRegistry.sol#L297-L299 * feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber` also improve wording in the 'tree' file * chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section with other internal fncs * chore: remove unnecessary require statement + improve code clarity * fix: correct a compiler error for implicit type conversion * feat: address TODOs in tests --------- Co-authored-by: Alex <[email protected]> * Test: bitmap utils unit tests (#101) * test: refactor and add tests to bitmap unit * test: added tests and using asserts * chore: remove single quote * feat: addNumberToBitmap function * chore: remove unused bitmap functions * feat: addNumberToBitmap function * test: tree file and Alexs bitmap fix * test: add back updated stakeRegistry tests * test: config tests setMinimumStake tests addStrategies tests initializeQuorum tests * refactor: using MockAVSDeployer for test file * test: more config tests * refactor: stake registry harness weighting * fix: stake weighting refactor removed overriding weightOfOperator functions in StakeRegistry and fixed broken tests using the helper _setOperatorWeight() * feat: addNumberToBitmap function * test: add back updated stakeRegistry tests * test: config tests setMinimumStake tests addStrategies tests initializeQuorum tests * refactor: using MockAVSDeployer for test file * test: more config tests * refactor: stake registry harness weighting * fix: stake weighting refactor removed overriding weightOfOperator functions in StakeRegistry and fixed broken tests using the helper _setOperatorWeight() * chore: rebase fixes * fix: undo change * test: voteweighing tests * test: fix unit tests * fix: rebase errors * fix: broken tests from rebase --------- Co-authored-by: Samuel Laferriere <[email protected]> Co-authored-by: Alex <[email protected]> Co-authored-by: ChaoticWalrus <[email protected]>
Layr-Labs · Jan 10, 2024 · 3a2b3b6 · 3a2b3b6
1 parent c53bb81
commit 3a2b3b6
Show file tree

Hide file tree

Showing 8 changed files with 1,867 additions and 505 deletions.
diff --git a/test/events/IStakeRegistryEvents.sol b/test/events/IStakeRegistryEvents.sol
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity =0.8.12;
+
+import {IStakeRegistry, IStrategy} from "src/interfaces/IStakeRegistry.sol";
+
+interface IStakeRegistryEvents {
+    /// @notice emitted whenever the stake of `operator` is updated
+    event OperatorStakeUpdate(
+        bytes32 indexed operatorId,
+        uint8 quorumNumber,
+        uint96 stake
+    );
+    /// @notice emitted when the minimum stake for a quorum is updated
+    event MinimumStakeForQuorumUpdated(uint8 indexed quorumNumber, uint96 minimumStake);
+    /// @notice emitted when a new quorum is created
+    event QuorumCreated(uint8 indexed quorumNumber);
+    /// @notice emitted when `strategy` has been added to the array at `strategyParams[quorumNumber]`
+    event StrategyAddedToQuorum(uint8 indexed quorumNumber, IStrategy strategy);
+    /// @notice emitted when `strategy` has removed from the array at `strategyParams[quorumNumber]`
+    event StrategyRemovedFromQuorum(uint8 indexed quorumNumber, IStrategy strategy);
+    /// @notice emitted when `strategy` has its `multiplier` updated in the array at `strategyParams[quorumNumber]`
+    event StrategyMultiplierUpdated(uint8 indexed quorumNumber, IStrategy strategy, uint256 multiplier);
+}
diff --git a/test/harnesses/StakeRegistryHarness.sol b/test/harnesses/StakeRegistryHarness.sol
@@ -5,8 +5,6 @@ import "../../src/StakeRegistry.sol";
 
 // wrapper around the StakeRegistry contract that exposes the internal functions for unit testing.
 contract StakeRegistryHarness is StakeRegistry {
-    mapping(uint8 => mapping(address => uint96)) private __weightOfOperatorForQuorum;
-
     constructor(
         IRegistryCoordinator _registryCoordinator,
         IDelegationManager _delegationManager
@@ -21,54 +19,11 @@ contract StakeRegistryHarness is StakeRegistry {
         _recordTotalStakeUpdate(quorumNumber, stakeDelta);
     }
 
-    // mocked function so we can set this arbitrarily without having to mock other elements
-    function weightOfOperatorForQuorum(uint8 quorumNumber, address operator) public override view returns(uint96) {
-        return __weightOfOperatorForQuorum[quorumNumber][operator];
-    }
-
-    function _weightOfOperatorForQuorum(uint8 quorumNumber, address operator) internal override view returns(uint96, bool) {
-        uint96 weight = __weightOfOperatorForQuorum[quorumNumber][operator];
-        return (
-            weight, 
-            weight >= minimumStakeForQuorum[quorumNumber]
-        );
-    }
-
-    // mocked function so we can set this arbitrarily without having to mock other elements
-    function setOperatorWeight(uint8 quorumNumber, address operator, uint96 weight) external {
-        __weightOfOperatorForQuorum[quorumNumber][operator] = weight;
+    function calculateDelta(uint96 prev, uint96 cur) external pure returns (int256) {
+        return _calculateDelta(prev, cur);
     }
 
-    // mocked function to register an operator without having to mock other elements
-    // This is just a copy/paste from `registerOperator`, since that no longer uses an internal method
-    function registerOperatorNonCoordinator(address operator, bytes32 operatorId, bytes calldata quorumNumbers) external returns (uint96[] memory, uint96[] memory) {
-        uint96[] memory currentStakes = new uint96[](quorumNumbers.length);
-        uint96[] memory totalStakes = new uint96[](quorumNumbers.length);
-        for (uint256 i = 0; i < quorumNumbers.length; i++) {            
-
-            uint8 quorumNumber = uint8(quorumNumbers[i]);
-            require(_quorumExists(quorumNumber), "StakeRegistry.registerOperator: quorum does not exist");
-
-            // Retrieve the operator's current weighted stake for the quorum, reverting if they have not met
-            // the minimum.
-            (uint96 currentStake, bool hasMinimumStake) = _weightOfOperatorForQuorum(quorumNumber, operator);
-            require(
-                hasMinimumStake,
-                "StakeRegistry.registerOperator: Operator does not meet minimum stake requirement for quorum"
-            );
-
-            // Update the operator's stake
-            int256 stakeDelta = _recordOperatorStakeUpdate({
-                operatorId: operatorId, 
-                quorumNumber: quorumNumber,
-                newStake: currentStake
-            });
-
-            // Update this quorum's total stake by applying the operator's delta
-            currentStakes[i] = currentStake;
-            totalStakes[i] = _recordTotalStakeUpdate(quorumNumber, stakeDelta);
-        }
-
-        return (currentStakes, totalStakes);
+    function applyDelta(uint96 value, int256 delta) external pure returns (uint96) {
+        return _applyDelta(value, delta);
     }
 }
diff --git a/test/integration/CoreRegistration.t.sol b/test/integration/CoreRegistration.t.sol
@@ -83,7 +83,7 @@ contract Test_CoreRegistration is MockAVSDeployer {
         // Set operator weight in single quorum
         bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(MAX_QUORUM_BITMAP);
         for (uint i = 0; i < quorumNumbers.length; i++) {
-            stakeRegistry.setOperatorWeight(uint8(quorumNumbers[i]), operator, defaultStake);
+            _setOperatorWeight(operator, uint8(quorumNumbers[i]), defaultStake);
         }    
     }
 

diff --git a/test/tree/StakeRegistryUnit.tree b/test/tree/StakeRegistryUnit.tree
@@ -0,0 +1,114 @@
+.
+├── StakeRegistry tree (*** denotes that integration tests are needed to validate path)
+├── when any function is called (invariants)
+│   └── when parameters contain uninitialized quorumNumbers
+│       └── it should revert
+├── when registerOperator is called
+│   ├── given caller is not the registry coordinator
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   ├── given the operator does not meet the minimum stake for a quorum
+│   │   └── it should revert
+│   └── given that the above conditions are satisfied
+│       ├── given the operator's stake is unchanged ***
+│       │   └── it should not update the operator's stake
+│       ├── given the operator does not have a stake update from the current block
+│       │   └── it should push a new stake update for the operator
+│       ├── given the operator does have a stake update for the current block ***
+│       │   └── it should update the operator's last stake update with the new stake
+│       ├── given the total stake history was not updated in the current block
+│       │   └── it should push a new stake update for the total stake
+│       └── given the total stake history was updated in the current block
+│           └── it should update the last total stake update with the new stake
+├── when deregisterOperator is called
+│   ├── given caller is not the registry coordinator
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   └── given that the above conditions are satisfied
+│       ├── given the operator's stake history is empty (although shouldn't be possible to register with empty stake history)
+│       │   └── it should push a single entry with 0 stake
+│       ├── given the operator's current stake is 0 and their history is nonempty
+│       │   └── it should not update the operator's stake history or total history
+│       ├── given the operator's current stake is nonzero and does not have a stake update for current block
+│       │   └── it should push a new stake update for the operator with 0 stake
+│       ├── given the operator's current stake is nonzero and was last updated in current block
+│       │   └── it should update the operator's last stake update with 0 stake
+│       ├── given the total stake history was not updated in the current block
+│       │   └── it should push a new stake update for the total stake
+│       └── given the total stake history was updated in the current block
+│           └── it should update the last total stake update with the new stake
+├── when updateOperatorStake is called
+│   ├── given caller is not the registry coordinator
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   ├── given the operator currently meets the minimum stake for the quorum
+│   │   ├── given they still do after updating
+│   │   │   └── it should update their history and apply the change to the total history
+│   │   └── given they no longer meet the minimum
+│   │       └── it should set their stake to zero, remove it from the total stake, and add the quorum to the return bitmap
+│   └── given the operator does not currently meet the minimum stake for the quorum
+│       ├── given their updated stake does not meet the minimum
+│       │   └── it should not perform any updates, and it should add the quorum to the return bitmap
+│       └── given they meet the minimum after the update
+│           └── it should update the operator and total history with the stake
+├── when initializeQuorum is called
+│   ├── given quorum already exists
+│   │   └── it should revert
+│   ├── given strategyParams is empty
+│   │   └── it should revert
+│   ├── given strategyParams length is > MAX_WEIGHING_FUNCTION_LENGTH
+│   │   └── it should revert
+│   └── given quorum doesn't already exist
+│       └── it should call _addStrategyParams and _setMinimumStakeForQuorum (internal functions below)
+│           └── given internal functions succeed
+│               └── it should set the first _totalStakeHistory entry for the initialized quorumNumber
+├── when setMinimumStakeForQuorum is called
+│   ├── given caller is not the registry coordinator owner
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   └── it should set the minimum stake and emit MinimumStakeForQuorumUpdated
+├── when addStrategies is called
+│   ├── given caller is not the registry coordinator owner
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   └── it should call _addStrategyParams (internal function)
+├── when _addStrategyParams is called
+│   ├── given strategyParams is empty
+│   │   └── it should revert
+│   ├── given strategyParams length is > MAX_WEIGHING_FUNCTION_LENGTH
+│   │   └── it should revert
+│   ├── given a strategy being added already exists in quorum
+│   │   └── it should revert
+│   ├── given a multiplier being added is 0
+│   │   └── it should revert
+│   └── given unique strategies and non-zero multipliers
+│       └── it should add the strategies and multipliers to the quorum
+├── when removeStrategies is called
+│   ├── given caller is not the registry coordinator owner
+│   │   └── it should revert
+│   ├── given quorum does not exist
+│   │   └── it should revert
+│   ├── given indicesToRemove length is 0
+│   │   └── it should revert
+│   ├── given an index in indicesToRemove is >= length of strategies in quorum
+│   │   └── it should revert
+│   ├── given valid indicesToRemove for an existing quorum but not in decreasing order
+│   │   └── it should revert
+│   └── given valid indicesToRemove for an existing quorum and in decreasing order
+│       └── it should remove the desired strategies from the quorum
+└── when modifyStrategyParams is called
+    ├── given caller is not the registry coordinator owner
+    │   └── it should revert
+    ├── given quorum does not exist
+    │   └── it should revert
+    ├── given strategyIndices length is 0 or has mismatch length with newMultipliers
+    │   └── it should revert
+    ├── given a index in strategyIndices is >= length of strategies in quorum
+    │   └── it should revert
+    └── given matching lengths and valid indices for an existing quorum
+        └── it should modify the weights of existing strategies in the quorum
diff --git a/test/unit/OperatorStateRetrieverUnit.t.sol b/test/unit/OperatorStateRetrieverUnit.t.sol
@@ -178,7 +178,12 @@ contract OperatorStateRetrieverUnitTests is MockAVSDeployer {
             for (uint k = 0; k < operators[j].length; k++) {
                 uint8 quorumNumber = uint8(quorumNumbers[j]);
                 assertEq(operators[j][k].operatorId, operatorMetadatas[expectedOperatorOverallIndices[quorumNumber][k]].operatorId);
-                assertEq(operators[j][k].stake, operatorMetadatas[expectedOperatorOverallIndices[quorumNumber][k]].stakes[quorumNumber]);
+                // using assertApprox to account for rounding errors
+                assertApproxEqAbs(
+                    operators[j][k].stake,
+                    operatorMetadatas[expectedOperatorOverallIndices[quorumNumber][k]].stakes[quorumNumber],
+                    1
+                );
             }
         }
     }