Skip to content

Commit f0294b0

Browse files
MoonBoi9001MoonBoi9001
authored
Merge pull request #1034 from graphprotocol/fix_oz_n-05
fix: address inconsistent parameter and mapping order (OZ N-05)
2 parents a76a0d6 + 0f1522d commit f0294b0

File tree

19 files changed

+77
-77
lines changed

19 files changed

+77
-77
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa
7676

7777
/**
7878
* @notice Thrown when the caller is not authorized to manage the provision of a service provider.
79+
* @param serviceProvider The address of the serviceProvider.
7980
* @param caller The address of the caller.
80-
* @param serviceProvider The address of the service provider.
8181
*/
82-
error ProvisionManagerNotAuthorized(address caller, address serviceProvider);
82+
error ProvisionManagerNotAuthorized(address serviceProvider, address caller);
8383

8484
/**
8585
* @notice Thrown when a provision is not found.
@@ -92,8 +92,8 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa
9292
*/
9393
modifier onlyAuthorizedForProvision(address serviceProvider) {
9494
require(
95-
_graphStaking().isAuthorized(msg.sender, serviceProvider, address(this)),
96-
ProvisionManagerNotAuthorized(msg.sender, serviceProvider)
95+
_graphStaking().isAuthorized(serviceProvider, address(this), msg.sender),
96+
ProvisionManagerNotAuthorized(serviceProvider, msg.sender)
9797
);
9898
_;
9999
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,14 @@ interface IHorizonStakingMain {
108108
/**
109109
* @dev Emitted when an operator is allowed or denied by a service provider for a particular verifier
110110
* @param serviceProvider The address of the service provider
111-
* @param operator The address of the operator
112111
* @param verifier The address of the verifier
112+
* @param operator The address of the operator
113113
* @param allowed Whether the operator is allowed or denied
114114
*/
115115
event OperatorSet(
116116
address indexed serviceProvider,
117-
address indexed operator,
118117
address indexed verifier,
118+
address indexed operator,
119119
bool allowed
120120
);
121121

@@ -336,7 +336,7 @@ interface IHorizonStakingMain {
336336
* @param serviceProvider The service provider address
337337
* @param verifier The verifier address
338338
*/
339-
error HorizonStakingNotAuthorized(address caller, address serviceProvider, address verifier);
339+
error HorizonStakingNotAuthorized(address serviceProvider, address verifier, address caller);
340340

341341
/**
342342
* @notice Thrown when attempting to create a provision with an invalid maximum verifier cut.
@@ -865,11 +865,11 @@ interface IHorizonStakingMain {
865865
* Additional requirements:
866866
* - The `verifier` must be allowed to be used for locked provisions.
867867
*
868-
* @param operator Address to authorize or unauthorize
869868
* @param verifier The verifier / data service on which they'll be allowed to operate
869+
* @param operator Address to authorize or unauthorize
870870
* @param allowed Whether the operator is authorized or not
871871
*/
872-
function setOperatorLocked(address operator, address verifier, bool allowed) external;
872+
function setOperatorLocked(address verifier, address operator, bool allowed) external;
873873

874874
/**
875875
* @notice Sets a verifier as a globally allowed verifier for locked provisions.
@@ -904,18 +904,18 @@ interface IHorizonStakingMain {
904904
/**
905905
* @notice Authorize or unauthorize an address to be an operator for the caller on a data service.
906906
* @dev Emits a {OperatorSet} event.
907-
* @param operator Address to authorize or unauthorize
908907
* @param verifier The verifier / data service on which they'll be allowed to operate
908+
* @param operator Address to authorize or unauthorize
909909
* @param allowed Whether the operator is authorized or not
910910
*/
911-
function setOperator(address operator, address verifier, bool allowed) external;
911+
function setOperator(address verifier, address operator, bool allowed) external;
912912

913913
/**
914914
* @notice Check if an operator is authorized for the caller on a specific verifier / data service.
915-
* @param operator The address to check for auth
916915
* @param serviceProvider The service provider on behalf of whom they're claiming to act
917916
* @param verifier The verifier / data service on which they're claiming to act
917+
* @param operator The address to check for auth
918918
* @return Whether the operator is authorized or not
919919
*/
920-
function isAuthorized(address operator, address serviceProvider, address verifier) external view returns (bool);
920+
function isAuthorized(address serviceProvider, address verifier, address operator) external view returns (bool);
921921
}

‎packages/horizon/contracts/staking/HorizonStaking.sol

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
4747
*/
4848
modifier onlyAuthorized(address serviceProvider, address verifier) {
4949
require(
50-
_isAuthorized(msg.sender, serviceProvider, verifier),
51-
HorizonStakingNotAuthorized(msg.sender, serviceProvider, verifier)
50+
_isAuthorized(serviceProvider, verifier, msg.sender),
51+
HorizonStakingNotAuthorized(serviceProvider, verifier, msg.sender)
5252
);
5353
_;
5454
}
@@ -466,9 +466,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
466466
/**
467467
* @notice See {IHorizonStakingMain-setOperatorLocked}.
468468
*/
469-
function setOperatorLocked(address operator, address verifier, bool allowed) external override notPaused {
469+
function setOperatorLocked(address verifier, address operator, bool allowed) external override notPaused {
470470
require(_allowedLockedVerifiers[verifier], HorizonStakingVerifierNotAllowed(verifier));
471-
_setOperator(operator, verifier, allowed);
471+
_setOperator(verifier, operator, allowed);
472472
}
473473

474474
/*
@@ -514,19 +514,19 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
514514
/**
515515
* @notice See {IHorizonStakingMain-setOperator}.
516516
*/
517-
function setOperator(address operator, address verifier, bool allowed) external override notPaused {
518-
_setOperator(operator, verifier, allowed);
517+
function setOperator(address verifier, address operator, bool allowed) external override notPaused {
518+
_setOperator(verifier, operator, allowed);
519519
}
520520

521521
/**
522522
* @notice See {IHorizonStakingMain-isAuthorized}.
523523
*/
524524
function isAuthorized(
525-
address operator,
526525
address serviceProvider,
527-
address verifier
526+
address verifier,
527+
address operator
528528
) external view override returns (bool) {
529-
return _isAuthorized(operator, serviceProvider, verifier);
529+
return _isAuthorized(serviceProvider, verifier, operator);
530530
}
531531

532532
/*
@@ -960,22 +960,22 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
960960
* @dev Note that this function handles the special case where the verifier is the subgraph data service,
961961
* where the operator settings are stored in the legacy mapping.
962962
*/
963-
function _setOperator(address _operator, address _verifier, bool _allowed) private {
963+
function _setOperator(address _verifier, address _operator, bool _allowed) private {
964964
require(_operator != msg.sender, HorizonStakingCallerIsServiceProvider());
965965
if (_verifier == SUBGRAPH_DATA_SERVICE_ADDRESS) {
966966
_legacyOperatorAuth[msg.sender][_operator] = _allowed;
967967
} else {
968968
_operatorAuth[msg.sender][_verifier][_operator] = _allowed;
969969
}
970-
emit OperatorSet(msg.sender, _operator, _verifier, _allowed);
970+
emit OperatorSet(msg.sender, _verifier, _operator, _allowed);
971971
}
972972

973973
/**
974974
* @notice See {IHorizonStakingMain-isAuthorized}.
975975
* @dev Note that this function handles the special case where the verifier is the subgraph data service,
976976
* where the operator settings are stored in the legacy mapping.
977977
*/
978-
function _isAuthorized(address _operator, address _serviceProvider, address _verifier) private view returns (bool) {
978+
function _isAuthorized(address _serviceProvider, address _verifier, address _operator) private view returns (bool) {
979979
if (_operator == _serviceProvider) {
980980
return true;
981981
}

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
4141

4242
modifier useOperator() {
4343
vm.startPrank(users.indexer);
44-
_setOperator(users.operator, subgraphDataServiceAddress, true);
44+
_setOperator(subgraphDataServiceAddress, users.operator, true);
4545
vm.startPrank(users.operator);
4646
_;
4747
vm.stopPrank();
@@ -736,39 +736,39 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
736736
assertEq(afterProvision.createdAt, beforeProvision.createdAt);
737737
}
738738

739-
function _setOperator(address operator, address verifier, bool allow) internal {
740-
__setOperator(operator, verifier, allow, false);
739+
function _setOperator(address verifier, address operator, bool allow) internal {
740+
__setOperator(verifier, operator, allow, false);
741741
}
742742

743-
function _setOperatorLocked(address operator, address verifier, bool allow) internal {
744-
__setOperator(operator, verifier, allow, true);
743+
function _setOperatorLocked(address verifier, address operator, bool allow) internal {
744+
__setOperator(verifier, operator, allow, true);
745745
}
746746

747-
function __setOperator(address operator, address verifier, bool allow, bool locked) private {
747+
function __setOperator(address verifier, address operator, bool allow, bool locked) private {
748748
(, address msgSender, ) = vm.readCallers();
749749

750750
// staking contract knows the address of the legacy subgraph service
751751
// but we cannot read it as it's an immutable, we have to use the global var :/
752752
bool legacy = verifier == subgraphDataServiceLegacyAddress;
753753

754754
// before
755-
bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy);
756-
bool beforeOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier);
755+
bool beforeOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy);
756+
bool beforeOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator);
757757
assertEq(beforeOperatorAllowed, beforeOperatorAllowedGetter);
758758

759759
// setOperator
760760
vm.expectEmit(address(staking));
761-
emit IHorizonStakingMain.OperatorSet(msgSender, operator, verifier, allow);
761+
emit IHorizonStakingMain.OperatorSet(msgSender, verifier, operator, allow);
762762
if (locked) {
763-
staking.setOperatorLocked(operator, verifier, allow);
763+
staking.setOperatorLocked(verifier, operator, allow);
764764
} else {
765-
staking.setOperator(operator, verifier, allow);
765+
staking.setOperator(verifier, operator, allow);
766766
}
767767

768768
// after
769-
bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, operator, verifier, legacy);
770-
bool afterOperatorAllowedGetter = staking.isAuthorized(operator, msgSender, verifier);
771-
assertEq(afterOperatorAllowed, afterOperatorAllowedGetter);
769+
bool afterOperatorAllowed = _getStorage_OperatorAuth(msgSender, verifier, operator, legacy);
770+
bool afterOperatorAllowedGetter = staking.isAuthorized(msgSender, verifier, operator);
771+
assertEq(afterOperatorAllowed, afterOperatorAllowedGetter, "afterOperatorAllowedGetter FAIL");
772772

773773
// assert
774774
assertEq(afterOperatorAllowed, allow);
@@ -1390,9 +1390,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
13901390
);
13911391

13921392
bool isAuth = staking.isAuthorized(
1393-
msgSender,
13941393
beforeValues.allocation.indexer,
1395-
subgraphDataServiceLegacyAddress
1394+
subgraphDataServiceLegacyAddress,
1395+
msgSender
13961396
);
13971397
address rewardsDestination = _getStorage_RewardsDestination(beforeValues.allocation.indexer);
13981398

@@ -1686,8 +1686,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
16861686

16871687
function _getStorage_OperatorAuth(
16881688
address serviceProvider,
1689-
address operator,
16901689
address verifier,
1690+
address operator,
16911691
bool legacy
16921692
) internal view returns (bool) {
16931693
uint256 slotNumber = legacy ? 21 : 31;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,22 @@ contract HorizonStakingOperatorLockedTest is HorizonStakingTest {
1212
*/
1313

1414
function testOperatorLocked_Set() public useIndexer useLockedVerifier(subgraphDataServiceAddress) {
15-
_setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
15+
_setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
1616
}
1717

1818
function testOperatorLocked_RevertWhen_VerifierNotAllowed() public useIndexer {
1919
bytes memory expectedError = abi.encodeWithSignature("HorizonStakingVerifierNotAllowed(address)", subgraphDataServiceAddress);
2020
vm.expectRevert(expectedError);
21-
staking.setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
21+
staking.setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
2222
}
2323

2424
function testOperatorLocked_RevertWhen_CallerIsServiceProvider() public useIndexer useLockedVerifier(subgraphDataServiceAddress) {
2525
bytes memory expectedError = abi.encodeWithSignature("HorizonStakingCallerIsServiceProvider()");
2626
vm.expectRevert(expectedError);
27-
staking.setOperatorLocked(users.indexer, subgraphDataServiceAddress, true);
27+
staking.setOperatorLocked(subgraphDataServiceAddress, users.indexer, true);
2828
}
2929

3030
function testOperatorLocked_SetLegacySubgraphService() public useIndexer useLockedVerifier(subgraphDataServiceLegacyAddress) {
31-
_setOperatorLocked(users.operator, subgraphDataServiceLegacyAddress, true);
31+
_setOperatorLocked(subgraphDataServiceLegacyAddress, users.operator, true);
3232
}
3333
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ contract HorizonStakingOperatorTest is HorizonStakingTest {
1212
*/
1313

1414
function testOperator_SetOperator() public useIndexer {
15-
_setOperator(users.operator, subgraphDataServiceAddress, true);
15+
_setOperator(subgraphDataServiceAddress, users.operator, true);
1616
}
1717

1818
function testOperator_RevertWhen_CallerIsServiceProvider() public useIndexer {
1919
bytes memory expectedError = abi.encodeWithSignature("HorizonStakingCallerIsServiceProvider()");
2020
vm.expectRevert(expectedError);
21-
staking.setOperator(users.indexer, subgraphDataServiceAddress, true);
21+
staking.setOperator(subgraphDataServiceAddress, users.indexer, true);
2222
}
2323

2424
function testOperator_RemoveOperator() public useIndexer {
25-
_setOperator(users.operator, subgraphDataServiceAddress, true);
26-
_setOperator(users.operator, subgraphDataServiceAddress, false);
25+
_setOperator(subgraphDataServiceAddress, users.operator, true);
26+
_setOperator(subgraphDataServiceAddress, users.operator, false);
2727
}
2828
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest {
7575
vm.startPrank(users.operator);
7676
bytes memory expectedError = abi.encodeWithSignature(
7777
"HorizonStakingNotAuthorized(address,address,address)",
78-
users.operator,
7978
users.indexer,
80-
subgraphDataServiceAddress
79+
subgraphDataServiceAddress,
80+
users.operator
8181
);
8282
vm.expectRevert(expectedError);
8383
staking.deprovision(users.indexer, subgraphDataServiceAddress, 0);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
1717
uint256 provisionTokens = staking.getProviderTokensAvailable(users.indexer, subgraphDataServiceAddress);
1818
assertEq(provisionTokens, 0);
1919

20-
_setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
20+
_setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
2121

2222
vm.startPrank(users.operator);
2323
_provisionLocked(
@@ -39,7 +39,7 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
3939
assertEq(provisionTokens, 0);
4040

4141
// Set operator
42-
_setOperatorLocked(users.operator, subgraphDataServiceAddress, true);
42+
_setOperatorLocked(subgraphDataServiceAddress, users.operator, true);
4343

4444
// Disable locked verifier
4545
vm.startPrank(users.governor);
@@ -66,9 +66,9 @@ contract HorizonStakingProvisionLockedTest is HorizonStakingTest {
6666
vm.startPrank(users.operator);
6767
bytes memory expectedError = abi.encodeWithSignature(
6868
"HorizonStakingNotAuthorized(address,address,address)",
69-
users.operator,
7069
users.indexer,
71-
subgraphDataServiceAddress
70+
subgraphDataServiceAddress,
71+
users.operator
7272
);
7373
vm.expectRevert(expectedError);
7474
staking.provisionLocked(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ contract HorizonStakingProvisionParametersTest is HorizonStakingTest {
5353
vm.expectRevert(
5454
abi.encodeWithSignature(
5555
"HorizonStakingNotAuthorized(address,address,address)",
56-
msg.sender,
5756
users.indexer,
58-
subgraphDataServiceAddress
57+
subgraphDataServiceAddress,
58+
msg.sender
5959
)
6060
);
6161
staking.setProvisionParameters(users.indexer, subgraphDataServiceAddress, maxVerifierCut, thawingPeriod);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ contract HorizonStakingProvisionTest is HorizonStakingTest {
101101
vm.startPrank(users.operator);
102102
bytes memory expectedError = abi.encodeWithSignature(
103103
"HorizonStakingNotAuthorized(address,address,address)",
104-
users.operator,
105104
users.indexer,
106-
subgraphDataServiceAddress
105+
subgraphDataServiceAddress,
106+
users.operator
107107
);
108108
vm.expectRevert(expectedError);
109109
staking.provision(users.indexer, subgraphDataServiceAddress, amount, maxVerifierCut, thawingPeriod);

0 commit comments

Comments
 (0)