fix: address inconsistent parameter and mapping order (OZ N-05)

packages/horizon/contracts/data-service/utilities/ProvisionManager.sol

@@ -76,10 +76,10 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa
 
     /**
      * @notice Thrown when the caller is not authorized to manage the provision of a service provider.
+     * @param serviceProvider The address of the serviceProvider.
      * @param caller The address of the caller.
-     * @param serviceProvider The address of the service provider.
      */
-    error ProvisionManagerNotAuthorized(address caller, address serviceProvider);
+    error ProvisionManagerNotAuthorized(address serviceProvider, address caller);
 
     /**
      * @notice Thrown when a provision is not found.
@@ -92,8 +92,8 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa
      */
     modifier onlyAuthorizedForProvision(address serviceProvider) {
         require(
-            _graphStaking().isAuthorized(msg.sender, serviceProvider, address(this)),
-            ProvisionManagerNotAuthorized(msg.sender, serviceProvider)
+            _graphStaking().isAuthorized(serviceProvider, address(this), msg.sender),
+            ProvisionManagerNotAuthorized(serviceProvider, msg.sender)
         );
         _;
     }

packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol

@@ -108,14 +108,14 @@ interface IHorizonStakingMain {
     /**
      * @dev Emitted when an operator is allowed or denied by a service provider for a particular verifier
      * @param serviceProvider The address of the service provider
-     * @param operator The address of the operator
      * @param verifier The address of the verifier
+     * @param operator The address of the operator
      * @param allowed Whether the operator is allowed or denied
      */
     event OperatorSet(
         address indexed serviceProvider,
-        address indexed operator,
         address indexed verifier,
+        address indexed operator,
         bool allowed
     );
 
@@ -336,7 +336,7 @@ interface IHorizonStakingMain {
      * @param serviceProvider The service provider address
      * @param verifier The verifier address
      */
-    error HorizonStakingNotAuthorized(address caller, address serviceProvider, address verifier);
+    error HorizonStakingNotAuthorized(address serviceProvider, address verifier, address caller);
 
     /**
      * @notice Thrown when attempting to create a provision with an invalid maximum verifier cut.
@@ -865,11 +865,11 @@ interface IHorizonStakingMain {
      * Additional requirements:
      * - The `verifier` must be allowed to be used for locked provisions.
      *
-     * @param operator Address to authorize or unauthorize
      * @param verifier The verifier / data service on which they'll be allowed to operate
+     * @param operator Address to authorize or unauthorize
      * @param allowed Whether the operator is authorized or not
      */
-    function setOperatorLocked(address operator, address verifier, bool allowed) external;
+    function setOperatorLocked(address verifier, address operator, bool allowed) external;
 
     /**
      * @notice Sets a verifier as a globally allowed verifier for locked provisions.
@@ -904,18 +904,18 @@ interface IHorizonStakingMain {
     /**
      * @notice Authorize or unauthorize an address to be an operator for the caller on a data service.
      * @dev Emits a {OperatorSet} event.
-     * @param operator Address to authorize or unauthorize
      * @param verifier The verifier / data service on which they'll be allowed to operate
+     * @param operator Address to authorize or unauthorize
      * @param allowed Whether the operator is authorized or not
      */
-    function setOperator(address operator, address verifier, bool allowed) external;
+    function setOperator(address verifier, address operator, bool allowed) external;
 
     /**
      * @notice Check if an operator is authorized for the caller on a specific verifier / data service.
-     * @param operator The address to check for auth
      * @param serviceProvider The service provider on behalf of whom they're claiming to act
      * @param verifier The verifier / data service on which they're claiming to act
+     * @param operator The address to check for auth
      * @return Whether the operator is authorized or not
      */
-    function isAuthorized(address operator, address serviceProvider, address verifier) external view returns (bool);
+    function isAuthorized(address serviceProvider, address verifier, address operator) external view returns (bool);
 }

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -47,8 +47,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
      */
     modifier onlyAuthorized(address serviceProvider, address verifier) {
         require(
-            _isAuthorized(msg.sender, serviceProvider, verifier),
-            HorizonStakingNotAuthorized(msg.sender, serviceProvider, verifier)
+            _isAuthorized(serviceProvider, verifier, msg.sender),
+            HorizonStakingNotAuthorized(serviceProvider, verifier, msg.sender)
         );
         _;
     }
@@ -466,9 +466,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
     /**
      * @notice See {IHorizonStakingMain-setOperatorLocked}.
      */
-    function setOperatorLocked(address operator, address verifier, bool allowed) external override notPaused {
+    function setOperatorLocked(address verifier, address operator, bool allowed) external override notPaused {
         require(_allowedLockedVerifiers[verifier], HorizonStakingVerifierNotAllowed(verifier));
-        _setOperator(operator, verifier, allowed);
+        _setOperator(verifier, operator, allowed);
     }
 
     /*
@@ -514,19 +514,19 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
     /**
      * @notice See {IHorizonStakingMain-setOperator}.
      */
-    function setOperator(address operator, address verifier, bool allowed) external override notPaused {
-        _setOperator(operator, verifier, allowed);
+    function setOperator(address verifier, address operator, bool allowed) external override notPaused {
+        _setOperator(verifier, operator, allowed);
     }
 
     /**
      * @notice See {IHorizonStakingMain-isAuthorized}.
      */
     function isAuthorized(
-        address operator,
         address serviceProvider,
-        address verifier
+        address verifier,
+        address operator
     ) external view override returns (bool) {
-        return _isAuthorized(operator, serviceProvider, verifier);
+        return _isAuthorized(serviceProvider, verifier, operator);
     }
 
     /*
@@ -960,22 +960,22 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
      * @dev Note that this function handles the special case where the verifier is the subgraph data service,
      * where the operator settings are stored in the legacy mapping.
      */
-    function _setOperator(address _operator, address _verifier, bool _allowed) private {
+    function _setOperator(address _verifier, address _operator, bool _allowed) private {
         require(_operator != msg.sender, HorizonStakingCallerIsServiceProvider());
         if (_verifier == SUBGRAPH_DATA_SERVICE_ADDRESS) {
             _legacyOperatorAuth[msg.sender][_operator] = _allowed;
         } else {
             _operatorAuth[msg.sender][_verifier][_operator] = _allowed;
         }
-        emit OperatorSet(msg.sender, _operator, _verifier, _allowed);
+        emit OperatorSet(msg.sender, _verifier, _operator, _allowed);
     }
 
     /**
      * @notice See {IHorizonStakingMain-isAuthorized}.
      * @dev Note that this function handles the special case where the verifier is the subgraph data service,
      * where the operator settings are stored in the legacy mapping.
      */
-    function _isAuthorized(address _operator, address _serviceProvider, address _verifier) private view returns (bool) {
+    function _isAuthorized(address _serviceProvider, address _verifier, address _operator) private view returns (bool) {
         if (_operator == _serviceProvider) {
             return true;
         }

packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol

@@ -41,7 +41,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
 
     modifier useOperator() {
         vm.startPrank(users.indexer);
-        _setOperator(users.operator, subgraphDataServiceAddress, true);
+        _setOperator(subgraphDataServiceAddress, users.operator, true);
         vm.startPrank(users.operator);
         _;
         vm.stopPrank();
@@ -736,39 +736,39 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
         assertEq(afterProvision.createdAt, beforeProvision.createdAt);
     }
 
-    function _setOperator(address operator, address verifier, bool allow) internal {
-        __setOperator(operator, verifier, allow, false);
+    function _setOperator(address verifier, address operator, bool allow) internal {
+        __setOperator(verifier, operator, allow, false);
     }
 
-    function _setOperatorLocked(address operator, address verifier, bool allow) internal {
-        __setOperator(operator, verifier, allow, true);
+    function _setOperatorLocked(address verifier, address operator, bool allow) internal {
+        __setOperator(verifier, operator, allow, true);
     }
 
-    function __setOperator(address operator, address verifier, bool allow, bool locked) private {
+    function __setOperator(address verifier, address operator, bool allow, bool locked) private {
         (, address msgSender, ) = vm.readCallers();
 
         // staking contract knows the address of the legacy subgraph service
         // but we cannot read it as it's an immutable, we have to use the global var :/
         bool legacy = verifier == subgraphDataServiceLegacyAddress;
 
         // before
-        bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy);
-        bool beforeOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier);
+        bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy);
+        bool beforeOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator);
         assertEq(beforeOperatorAllowed, beforeOperatorAllowedGetter);
 
         // setOperator
         vm.expectEmit(address(staking));
-        emit IHorizonStakingMain.OperatorSet(msgSender, operator, verifier, allow);
+        emit IHorizonStakingMain.OperatorSet(msgSender, verifier, operator, allow);
         if (locked) {
-            staking.setOperatorLocked(operator, verifier, allow);
+            staking.setOperatorLocked(verifier, operator, allow);
         } else {
-            staking.setOperator(operator, verifier, allow);
+            staking.setOperator(verifier, operator, allow);
         }
 
         // after
-        bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy);
-        bool afterOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier);
-        assertEq(afterOperatorAllowed, afterOperatorAllowedGetter);
+        bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy);
+        bool afterOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator);
+        assertEq(afterOperatorAllowed, afterOperatorAllowedGetter, "afterOperatorAllowedGetter FAIL");
 
         // assert
         assertEq(afterOperatorAllowed, allow);
@@ -1390,9 +1390,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
         );
 
         bool isAuth = staking.isAuthorized(
-            msgSender,
             beforeValues.allocation.indexer,
-            subgraphDataServiceLegacyAddress
+            subgraphDataServiceLegacyAddress,
+            msgSender
         );
         address rewardsDestination = _getStorage_RewardsDestination(beforeValues.allocation.indexer);
 
@@ -1686,8 +1686,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
 
     function _getStorage_OperatorAuth(
         address serviceProvider,
-        address operator,
         address verifier,
+        address operator,
         bool legacy
     ) internal view returns (bool) {
         uint256 slotNumber = legacy ? 21 : 31;

packages/horizon/test/staking/operator/locked.t.sol

@@ -12,22 +12,22 @@ contract HorizonStakingOperatorLockedTest is HorizonStakingTest {
      */
 
     function testOperatorLocked_Set() public useIndexer useLockedVerifier(subgraphDataServiceAddress) {
-        _setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
+        _setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
     }
 
     function testOperatorLocked_RevertWhen_VerifierNotAllowed() public useIndexer {
         bytes memory expectedError = abi.encodeWithSignature("HorizonStakingVerifierNotAllowed(address)", subgraphDataServiceAddress);
         vm.expectRevert(expectedError);
-        staking.setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
+        staking.setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
     }
 
     function testOperatorLocked_RevertWhen_CallerIsServiceProvider() public useIndexer useLockedVerifier(subgraphDataServiceAddress) {
         bytes memory expectedError = abi.encodeWithSignature("HorizonStakingCallerIsServiceProvider()");
         vm.expectRevert(expectedError);
-        staking.setOperatorLocked(users.indexer, subgraphDataServiceAddress, true);
+        staking.setOperatorLocked(subgraphDataServiceAddress, users.indexer, true);
     }
 
     function testOperatorLocked_SetLegacySubgraphService() public useIndexer useLockedVerifier(subgraphDataServiceLegacyAddress) {
-        _setOperatorLocked(users.operator, subgraphDataServiceLegacyAddress, true);
+        _setOperatorLocked(subgraphDataServiceLegacyAddress, users.operator, true);
     }
 }

packages/horizon/test/staking/operator/operator.t.sol

@@ -12,17 +12,17 @@ contract HorizonStakingOperatorTest is HorizonStakingTest {
      */
 
     function testOperator_SetOperator() public useIndexer {
-        _setOperator(users.operator, subgraphDataServiceAddress, true);
+        _setOperator(subgraphDataServiceAddress, users.operator, true);
     }
 
     function testOperator_RevertWhen_CallerIsServiceProvider() public useIndexer {
         bytes memory expectedError = abi.encodeWithSignature("HorizonStakingCallerIsServiceProvider()");
         vm.expectRevert(expectedError);
-        staking.setOperator(users.indexer, subgraphDataServiceAddress, true);
+        staking.setOperator(subgraphDataServiceAddress, users.indexer, true);
     }
 
     function testOperator_RemoveOperator() public useIndexer {
-        _setOperator(users.operator, subgraphDataServiceAddress, true);
-        _setOperator(users.operator, subgraphDataServiceAddress, false);
+        _setOperator(subgraphDataServiceAddress, users.operator, true);
+        _setOperator(subgraphDataServiceAddress, users.operator, false);
     }
 }

packages/horizon/test/staking/provision/deprovision.t.sol

@@ -75,9 +75,9 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest {
         vm.startPrank(users.operator);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingNotAuthorized(address,address,address)",
-            users.operator,
             users.indexer,
-            subgraphDataServiceAddress
+            subgraphDataServiceAddress,
+            users.operator
         );
         vm.expectRevert(expectedError);
         staking.deprovision(users.indexer, subgraphDataServiceAddress, 0);

packages/horizon/test/staking/provision/locked.t.sol

@@ -17,7 +17,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
         uint256 provisionTokens = staking.getProviderTokensAvailable(users.indexer, subgraphDataServiceAddress);
         assertEq(provisionTokens, 0);
 
-        _setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
+        _setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
 
         vm.startPrank(users.operator);
         _provisionLocked(
@@ -39,7 +39,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
         assertEq(provisionTokens, 0);
 
         // Set operator
-        _setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
+        _setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
 
         // Disable locked verifier
         vm.startPrank(users.governor);
@@ -66,9 +66,9 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
         vm.startPrank(users.operator);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingNotAuthorized(address,address,address)",
-            users.operator,
             users.indexer,
-            subgraphDataServiceAddress
+            subgraphDataServiceAddress,
+            users.operator
         );
         vm.expectRevert(expectedError);
         staking.provisionLocked(

packages/horizon/test/staking/provision/parameters.t.sol

@@ -53,9 +53,9 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest {
         vm.expectRevert(
             abi.encodeWithSignature(
                 "HorizonStakingNotAuthorized(address,address,address)",
-                msg.sender,
                 users.indexer,
-                subgraphDataServiceAddress
+                subgraphDataServiceAddress,
+                msg.sender
             )
         );
         staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod);

packages/horizon/test/staking/provision/provision.t.sol

@@ -101,9 +101,9 @@ contract HorizonStakingProvisionTest is HorizonStakingTest {
         vm.startPrank(users.operator);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingNotAuthorized(address,address,address)",
-            users.operator,
             users.indexer,
-            subgraphDataServiceAddress
+            subgraphDataServiceAddress,
+            users.operator
         );
         vm.expectRevert(expectedError);
         staking.provision(users.indexer, subgraphDataServiceAddress, amount, maxVerifierCut, thawingPeriod);

packages/horizon/test/staking/provision/reprovision.t.sol

@@ -37,7 +37,7 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
 
         // Switch to indexer to set operator for new data service
         vm.startPrank(users.indexer);
-        _setOperator(users.operator, newDataService, true);
+        _setOperator(newDataService, users.operator, true);
 
         // Switch back to operator
         vm.startPrank(users.operator);
@@ -58,9 +58,9 @@ contract HorizonStakingReprovisionTest is HorizonStakingTest {
         vm.startPrank(users.operator);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingNotAuthorized(address,address,address)",
-            users.operator,
             users.indexer,
-            newDataService
+            newDataService,
+            users.operator
         );
         vm.expectRevert(expectedError);
         staking.reprovision(users.indexer, subgraphDataServiceAddress, newDataService, 0);

packages/horizon/test/staking/provision/thaw.t.sol

@@ -49,9 +49,9 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         vm.startPrank(users.operator);
         bytes memory expectedError = abi.encodeWithSignature(
             "HorizonStakingNotAuthorized(address,address,address)",
-            users.operator,
             users.indexer,
-            subgraphDataServiceAddress
+            subgraphDataServiceAddress,
+            users.operator
         );
         vm.expectRevert(expectedError);
         staking.thaw(users.indexer, subgraphDataServiceAddress, amount);