From 8b3a86b7d8a277b9761a94f43e3ae84e5b3e9568 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Thu, 3 Oct 2024 13:29:29 +0100 Subject: [PATCH 1/9] fix: apply consistent mapping order HorizonStakingNotAuthorized (OZ_N-05) --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 2 +- packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- packages/horizon/test/staking/provision/deprovision.t.sol | 4 ++-- packages/horizon/test/staking/provision/locked.t.sol | 4 ++-- packages/horizon/test/staking/provision/parameters.t.sol | 4 ++-- packages/horizon/test/staking/provision/provision.t.sol | 4 ++-- packages/horizon/test/staking/provision/reprovision.t.sol | 4 ++-- packages/horizon/test/staking/provision/thaw.t.sol | 4 ++-- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 210146bf4..002f9e9b3 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -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. diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index a00cda8af..b6009043a 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -48,7 +48,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { modifier onlyAuthorized(address serviceProvider, address verifier) { require( _isAuthorized(msg.sender, serviceProvider, verifier), - HorizonStakingNotAuthorized(msg.sender, serviceProvider, verifier) + HorizonStakingNotAuthorized(serviceProvider, verifier, msg.sender) ); _; } diff --git a/packages/horizon/test/staking/provision/deprovision.t.sol b/packages/horizon/test/staking/provision/deprovision.t.sol index 6d03c3f6e..376b05e14 100644 --- a/packages/horizon/test/staking/provision/deprovision.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/provision/locked.t.sol b/packages/horizon/test/staking/provision/locked.t.sol index a4d8eda92..66745839b 100644 --- a/packages/horizon/test/staking/provision/locked.t.sol +++ b/packages/horizon/test/staking/provision/locked.t.sol @@ -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( diff --git a/packages/horizon/test/staking/provision/parameters.t.sol b/packages/horizon/test/staking/provision/parameters.t.sol index 6eccca06e..377684630 100644 --- a/packages/horizon/test/staking/provision/parameters.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/provision/provision.t.sol b/packages/horizon/test/staking/provision/provision.t.sol index cabe2f25e..81ed628fd 100644 --- a/packages/horizon/test/staking/provision/provision.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/provision/reprovision.t.sol b/packages/horizon/test/staking/provision/reprovision.t.sol index 722ecfe59..cdcdb6237 100644 --- a/packages/horizon/test/staking/provision/reprovision.t.sol +++ b/packages/horizon/test/staking/provision/reprovision.t.sol @@ -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); diff --git a/packages/horizon/test/staking/provision/thaw.t.sol b/packages/horizon/test/staking/provision/thaw.t.sol index bb0b3409d..3672683ba 100644 --- a/packages/horizon/test/staking/provision/thaw.t.sol +++ b/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); From fa9da60ff285080cd22c45e2b94ac83f47904994 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Fri, 4 Oct 2024 11:56:13 +0100 Subject: [PATCH 2/9] fix: apply consistent mapping order ProvisionManagerNotAuthorized (OZ_N-05) --- .../contracts/data-service/utilities/ProvisionManager.sol | 4 ++-- .../test/subgraphService/allocation/start.t.sol | 4 ++-- .../test/subgraphService/allocation/stop.t.sol | 4 ++-- .../test/subgraphService/collect/query/query.t.sol | 4 ++-- .../test/subgraphService/provider/register.t.sol | 4 ++-- .../test/subgraphService/provision/accept.t.sol | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index 845c1bdf6..02a51e4b6 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -79,7 +79,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa * @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. @@ -93,7 +93,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa modifier onlyAuthorizedForProvision(address serviceProvider) { require( _graphStaking().isAuthorized(msg.sender, serviceProvider, address(this)), - ProvisionManagerNotAuthorized(msg.sender, serviceProvider) + ProvisionManagerNotAuthorized(serviceProvider, msg.sender) ); _; } diff --git a/packages/subgraph-service/test/subgraphService/allocation/start.t.sol b/packages/subgraph-service/test/subgraphService/allocation/start.t.sol index 9157444fb..f5d76a350 100644 --- a/packages/subgraph-service/test/subgraphService/allocation/start.t.sol +++ b/packages/subgraph-service/test/subgraphService/allocation/start.t.sol @@ -60,8 +60,8 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest { bytes memory data = _generateData(tokens); vm.expectRevert(abi.encodeWithSelector( ProvisionManager.ProvisionManagerNotAuthorized.selector, - users.operator, - users.indexer + users.indexer, + users.operator )); subgraphService.startService(users.indexer, data); } diff --git a/packages/subgraph-service/test/subgraphService/allocation/stop.t.sol b/packages/subgraph-service/test/subgraphService/allocation/stop.t.sol index ee5958115..8a2b2a284 100644 --- a/packages/subgraph-service/test/subgraphService/allocation/stop.t.sol +++ b/packages/subgraph-service/test/subgraphService/allocation/stop.t.sol @@ -49,8 +49,8 @@ contract SubgraphServiceAllocationStopTest is SubgraphServiceTest { vm.expectRevert( abi.encodeWithSelector( ProvisionManager.ProvisionManagerNotAuthorized.selector, - users.operator, - users.indexer + users.indexer, + users.operator ) ); subgraphService.stopService(users.indexer, data); diff --git a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol index 6ad7b5347..7f57f06f8 100644 --- a/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol +++ b/packages/subgraph-service/test/subgraphService/collect/query/query.t.sol @@ -112,8 +112,8 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest { vm.expectRevert( abi.encodeWithSelector( ProvisionManager.ProvisionManagerNotAuthorized.selector, - users.operator, - users.indexer + users.indexer, + users.operator ) ); subgraphService.collect(users.indexer, paymentType, data); diff --git a/packages/subgraph-service/test/subgraphService/provider/register.t.sol b/packages/subgraph-service/test/subgraphService/provider/register.t.sol index 1df1f5571..6a4a36737 100644 --- a/packages/subgraph-service/test/subgraphService/provider/register.t.sol +++ b/packages/subgraph-service/test/subgraphService/provider/register.t.sol @@ -41,8 +41,8 @@ contract SubgraphServiceProviderRegisterTest is SubgraphServiceTest { resetPrank(users.operator); vm.expectRevert(abi.encodeWithSelector( ProvisionManager.ProvisionManagerNotAuthorized.selector, - users.operator, - users.indexer + users.indexer, + users.operator )); bytes memory data = abi.encode("url", "geoHash", users.rewardsDestination); subgraphService.register(users.indexer, data); diff --git a/packages/subgraph-service/test/subgraphService/provision/accept.t.sol b/packages/subgraph-service/test/subgraphService/provision/accept.t.sol index 812a27639..f89cb307e 100644 --- a/packages/subgraph-service/test/subgraphService/provision/accept.t.sol +++ b/packages/subgraph-service/test/subgraphService/provision/accept.t.sol @@ -47,8 +47,8 @@ contract SubgraphServiceProvisionAcceptTest is SubgraphServiceTest { resetPrank(users.operator); vm.expectRevert(abi.encodeWithSelector( ProvisionManager.ProvisionManagerNotAuthorized.selector, - users.operator, - users.indexer + users.indexer, + users.operator )); subgraphService.acceptProvisionPendingParameters(users.indexer, ""); } From b75154c6971d037ac8a828e40b5d572afb7d5c2a Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Fri, 4 Oct 2024 11:58:06 +0100 Subject: [PATCH 3/9] fix: apply consistent mapping order isAuthorized (OZ_N-05) --- .../contracts/interfaces/internal/IHorizonStakingMain.sol | 4 ++-- packages/horizon/contracts/staking/HorizonStaking.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index 002f9e9b3..f4fbe733b 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol @@ -912,10 +912,10 @@ interface IHorizonStakingMain { /** * @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); } diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index b6009043a..e838f3cf3 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -522,9 +522,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * @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); } From cc14383bd5420bfd8ba7d6adc689f7d91375084a Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Fri, 4 Oct 2024 11:59:25 +0100 Subject: [PATCH 4/9] fix: apply consistent mapping order _isAuthorized (OZ_N-05) --- packages/horizon/contracts/staking/HorizonStaking.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index e838f3cf3..3f9653f95 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -47,7 +47,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { */ modifier onlyAuthorized(address serviceProvider, address verifier) { require( - _isAuthorized(msg.sender, serviceProvider, verifier), + _isAuthorized(serviceProvider, verifier, msg.sender), HorizonStakingNotAuthorized(serviceProvider, verifier, msg.sender) ); _; @@ -526,7 +526,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { address verifier, address operator ) external view override returns (bool) { - return _isAuthorized(operator, serviceProvider, verifier); + return _isAuthorized(serviceProvider, verifier, operator); } /* @@ -975,7 +975,7 @@ 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 _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; } From 6af9e66f55d1bd680c61e3056c50decfa19671ee Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Fri, 4 Oct 2024 15:17:03 +0100 Subject: [PATCH 5/9] fix: apply consistent mapping order isAuthorized (OZ_N-05) --- .../contracts/data-service/utilities/ProvisionManager.sol | 2 +- .../shared/horizon-staking/HorizonStakingShared.t.sol | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index 02a51e4b6..3b3ebd356 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -92,7 +92,7 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa */ modifier onlyAuthorizedForProvision(address serviceProvider) { require( - _graphStaking().isAuthorized(msg.sender, serviceProvider, address(this)), + _graphStaking().isAuthorized(serviceProvider, address(this), msg.sender), ProvisionManagerNotAuthorized(serviceProvider, msg.sender) ); _; diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index f9796cf94..d0c46a8dc 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -753,7 +753,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // before bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy); - bool beforeOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier); + bool beforeOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator); assertEq(beforeOperatorAllowed, beforeOperatorAllowedGetter); // setOperator @@ -767,7 +767,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // after bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy); - bool afterOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier); + bool afterOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator); assertEq(afterOperatorAllowed, afterOperatorAllowedGetter); // assert @@ -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); From ac965cda0c212f936ce380937341eb9c56ec2634 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Thu, 3 Oct 2024 13:24:00 +0100 Subject: [PATCH 6/9] fix: apply consistent mapping order _operatorAuth (OZ_N-05) --- packages/horizon/contracts/staking/HorizonStaking.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 3f9653f95..157a65a26 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -967,7 +967,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { } else { _operatorAuth[msg.sender][_verifier][_operator] = _allowed; } - emit OperatorSet(msg.sender, _operator, _verifier, _allowed); + emit OperatorSet(msg.sender, _verifier, _operator, _allowed); } /** From 68157c0e1a21a63ab5ea978ae5c79ded52ccebbb Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Fri, 4 Oct 2024 16:30:51 +0100 Subject: [PATCH 7/9] fix: swap peram order _getStorage_OperatorAuth --- .../horizon-staking/HorizonStakingShared.t.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index d0c46a8dc..4adf6fcd4 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol @@ -752,7 +752,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { bool legacy = verifier == subgraphDataServiceLegacyAddress; // before - bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy); + bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy); bool beforeOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator); assertEq(beforeOperatorAllowed, beforeOperatorAllowedGetter); @@ -766,9 +766,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { } // after - bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy); + bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy); bool afterOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator); - assertEq(afterOperatorAllowed, afterOperatorAllowedGetter); + assertEq(afterOperatorAllowed, afterOperatorAllowedGetter, "afterOperatorAllowedGetter FAIL"); // assert assertEq(afterOperatorAllowed, allow); @@ -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; @@ -1699,8 +1699,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { slot = uint256( keccak256( abi.encode( - operator, - keccak256(abi.encode(verifier, keccak256(abi.encode(serviceProvider, slotNumber)))) + verifier, + keccak256(abi.encode(operator, keccak256(abi.encode(serviceProvider, slotNumber)))) ) ) ); From 1693d7650192cf5490a5168be4dcdf9956e51ad8 Mon Sep 17 00:00:00 2001 From: MoonBoi9001 Date: Mon, 7 Oct 2024 13:19:57 +0100 Subject: [PATCH 8/9] fix: set new peram order various functions (OZ_N-05) --- .../internal/IHorizonStakingMain.sol | 12 +++++----- .../contracts/staking/HorizonStaking.sol | 10 ++++----- .../HorizonStakingShared.t.sol | 22 +++++++++---------- .../test/staking/operator/locked.t.sol | 8 +++---- .../test/staking/operator/operator.t.sol | 8 +++---- .../test/staking/provision/locked.t.sol | 4 ++-- .../test/staking/provision/reprovision.t.sol | 2 +- .../serviceProvider/serviceProvider.t.sol | 2 +- .../subgraphService/SubgraphService.t.sol | 2 +- 9 files changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol b/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol index f4fbe733b..b6a787138 100644 --- a/packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol +++ b/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 ); @@ -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,11 +904,11 @@ 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. diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 157a65a26..8f3fb524a 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -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,8 +514,8 @@ 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); } /** @@ -960,7 +960,7 @@ 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; diff --git a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol index 4adf6fcd4..b13128c72 100644 --- a/packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/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,15 +736,15 @@ 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 @@ -758,11 +758,11 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { // 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 @@ -1699,8 +1699,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { slot = uint256( keccak256( abi.encode( - verifier, - keccak256(abi.encode(operator, keccak256(abi.encode(serviceProvider, slotNumber)))) + operator, + keccak256(abi.encode(verifier, keccak256(abi.encode(serviceProvider, slotNumber)))) ) ) ); diff --git a/packages/horizon/test/staking/operator/locked.t.sol b/packages/horizon/test/staking/operator/locked.t.sol index 706b47a07..3d9b5f1e6 100644 --- a/packages/horizon/test/staking/operator/locked.t.sol +++ b/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); } } \ No newline at end of file diff --git a/packages/horizon/test/staking/operator/operator.t.sol b/packages/horizon/test/staking/operator/operator.t.sol index 5d509ba68..a821e561d 100644 --- a/packages/horizon/test/staking/operator/operator.t.sol +++ b/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); } } \ No newline at end of file diff --git a/packages/horizon/test/staking/provision/locked.t.sol b/packages/horizon/test/staking/provision/locked.t.sol index 66745839b..2dcae964a 100644 --- a/packages/horizon/test/staking/provision/locked.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/provision/reprovision.t.sol b/packages/horizon/test/staking/provision/reprovision.t.sol index cdcdb6237..be650019f 100644 --- a/packages/horizon/test/staking/provision/reprovision.t.sol +++ b/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); diff --git a/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol b/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol index 7db480869..9f4893942 100644 --- a/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol +++ b/packages/horizon/test/staking/serviceProvider/serviceProvider.t.sol @@ -25,7 +25,7 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest { assertEq(sp.tokensStaked, amount); assertEq(sp.tokensProvisioned, amount); - _setOperator(users.operator, subgraphDataServiceAddress, true); + _setOperator(subgraphDataServiceAddress, users.operator, true); resetPrank(users.operator); _stakeTo(users.indexer, operatorAmount); sp = staking.getServiceProvider(users.indexer); diff --git a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol index ce9810d81..013f869a8 100644 --- a/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol +++ b/packages/subgraph-service/test/subgraphService/SubgraphService.t.sol @@ -39,7 +39,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest { modifier useOperator() { resetPrank(users.indexer); - staking.setOperator(users.operator, address(subgraphService), true); + staking.setOperator(address(subgraphService), users.operator, true); resetPrank(users.operator); _; vm.stopPrank(); From 0f1522d58427e292aa8d596b8657e2c303a63f7d Mon Sep 17 00:00:00 2001 From: MoonBoi9001 <67825802+MoonBoi9001@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:34:20 +0100 Subject: [PATCH 9/9] Update packages/horizon/contracts/data-service/utilities/ProvisionManager.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomás Migone --- .../contracts/data-service/utilities/ProvisionManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol index 3b3ebd356..a3f96794c 100644 --- a/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol +++ b/packages/horizon/contracts/data-service/utilities/ProvisionManager.sol @@ -76,8 +76,8 @@ 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 serviceProvider, address caller);