Skip to content

Commit 0a31b58

Browse files
tmigonetmigone
committed
fix: ensure subgraph service is set before using it (TRST-R10)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 4d75947 commit 0a31b58

File tree

8 files changed

+140
-21
lines changed

8 files changed

+140
-21
lines changed

‎packages/subgraph-service/contracts/DisputeManager.sol

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,10 @@ contract DisputeManager is
402402
* @param indexer The indexer address
403403
*/
404404
function getStakeSnapshot(address indexer) external view override returns (uint256) {
405-
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService));
405+
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(
406+
indexer,
407+
address(_getSubgraphService())
408+
);
406409
return _getStakeSnapshot(indexer, provision.tokens);
407410
}
408411

@@ -427,7 +430,7 @@ contract DisputeManager is
427430
// Get attestation signer. Indexers signs with the allocationId
428431
address allocationId = _recoverSigner(attestation);
429432

430-
Allocation.State memory alloc = subgraphService.getAllocation(allocationId);
433+
Allocation.State memory alloc = _getSubgraphService().getAllocation(allocationId);
431434
require(alloc.indexer != address(0), DisputeManagerIndexerNotFound(allocationId));
432435
require(
433436
alloc.subgraphDeploymentId == attestation.subgraphDeploymentId,
@@ -466,7 +469,10 @@ contract DisputeManager is
466469
address indexer = getAttestationIndexer(_attestation);
467470

468471
// The indexer is disputable
469-
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService));
472+
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(
473+
indexer,
474+
address(_getSubgraphService())
475+
);
470476
require(provision.tokens != 0, DisputeManagerZeroTokens());
471477

472478
// Create a disputeId
@@ -529,12 +535,13 @@ contract DisputeManager is
529535
require(!isDisputeCreated(disputeId), DisputeManagerDisputeAlreadyCreated(disputeId));
530536

531537
// Allocation must exist
532-
Allocation.State memory alloc = subgraphService.getAllocation(_allocationId);
538+
ISubgraphService subgraphService_ = _getSubgraphService();
539+
Allocation.State memory alloc = subgraphService_.getAllocation(_allocationId);
533540
address indexer = alloc.indexer;
534541
require(indexer != address(0), DisputeManagerIndexerNotFound(_allocationId));
535542

536543
// The indexer must be disputable
537-
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService));
544+
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(indexer, address(subgraphService_));
538545
require(provision.tokens != 0, DisputeManagerZeroTokens());
539546

540547
// Store dispute
@@ -596,8 +603,10 @@ contract DisputeManager is
596603
uint256 _tokensSlash,
597604
uint256 _tokensStakeSnapshot
598605
) private returns (uint256) {
606+
ISubgraphService subgraphService_ = _getSubgraphService();
607+
599608
// Get slashable amount for indexer
600-
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(_indexer, address(subgraphService));
609+
IHorizonStaking.Provision memory provision = _graphStaking().getProvision(_indexer, address(subgraphService_));
601610

602611
// Ensure slash amount is within the cap
603612
uint256 maxTokensSlash = _tokensStakeSnapshot.mulPPM(maxSlashingCut);
@@ -611,7 +620,7 @@ contract DisputeManager is
611620
uint256 maxRewardableTokens = Math.min(_tokensSlash, provision.tokens);
612621
uint256 tokensRewards = uint256(fishermanRewardCut).mulPPM(maxRewardableTokens);
613622

614-
subgraphService.slash(_indexer, abi.encode(_tokensSlash, tokensRewards));
623+
subgraphService_.slash(_indexer, abi.encode(_tokensSlash, tokensRewards));
615624
return tokensRewards;
616625
}
617626

@@ -684,6 +693,16 @@ contract DisputeManager is
684693
emit SubgraphServiceSet(_subgraphService);
685694
}
686695

696+
/**
697+
* @notice Get the address of the subgraph service
698+
* @dev Will revert if the subgraph service is not set
699+
* @return The subgraph service address
700+
*/
701+
function _getSubgraphService() private view returns (ISubgraphService) {
702+
require(address(subgraphService) != address(0), DisputeManagerSubgraphServiceNotSet());
703+
return subgraphService;
704+
}
705+
687706
/**
688707
* @notice Returns whether the dispute is for a conflicting attestation or not.
689708
* @param _dispute Dispute
@@ -705,8 +724,9 @@ contract DisputeManager is
705724
* @return Total stake snapshot
706725
*/
707726
function _getStakeSnapshot(address _indexer, uint256 _indexerStake) private view returns (uint256) {
708-
uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, address(subgraphService)).tokens;
709-
uint256 delegatorsStakeMax = _indexerStake * uint256(subgraphService.getDelegationRatio());
727+
ISubgraphService subgraphService_ = _getSubgraphService();
728+
uint256 delegatorsStake = _graphStaking().getDelegationPool(_indexer, address(subgraphService_)).tokens;
729+
uint256 delegatorsStakeMax = _indexerStake * uint256(subgraphService_.getDelegationRatio());
710730
uint256 stakeSnapshot = _indexerStake + MathUtils.min(delegatorsStake, delegatorsStakeMax);
711731
return stakeSnapshot;
712732
}

‎packages/subgraph-service/contracts/interfaces/IDisputeManager.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ interface IDisputeManager {
181181
bytes32 responseCID2,
182182
bytes32 subgraphDeploymentId2
183183
);
184+
error DisputeManagerSubgraphServiceNotSet();
184185

185186
function setDisputePeriod(uint64 disputePeriod) external;
186187

‎packages/subgraph-service/test/disputeManager/DisputeManager.t.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
6262
assertEq(disputeManager.disputeDeposit(), _disputeDeposit, "Dispute deposit should be set.");
6363
}
6464

65+
function _setSubgraphService(address _subgraphService) internal {
66+
vm.expectEmit(address(disputeManager));
67+
emit IDisputeManager.SubgraphServiceSet(_subgraphService);
68+
disputeManager.setSubgraphService(_subgraphService);
69+
assertEq(address(disputeManager.subgraphService()), _subgraphService, "Subgraph service should be set.");
70+
}
71+
6572
function _createIndexingDispute(address _allocationId, bytes32 _poi) internal returns (bytes32) {
6673
(, address fisherman, ) = vm.readCallers();
6774
bytes32 expectedDisputeId = keccak256(abi.encodePacked(_allocationId, _poi));
@@ -621,4 +628,8 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
621628
stakeSnapshot: stakeSnapshot
622629
});
623630
}
631+
632+
function _setStorage_SubgraphService(address _subgraphService) internal {
633+
vm.store(address(disputeManager), bytes32(uint256(51)), bytes32(uint256(uint160(_subgraphService))));
634+
}
624635
}

‎packages/subgraph-service/test/disputeManager/disputes/indexing/accept.t.sol

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,23 @@ contract DisputeManagerIndexingAcceptDisputeTest is DisputeManagerTest {
2727
_acceptDispute(disputeID, tokensSlash, false);
2828
}
2929

30+
function test_Indexing_Accept_Dispute_RevertWhen_SubgraphServiceNotSet(
31+
uint256 tokens,
32+
uint256 tokensSlash
33+
) public useIndexer useAllocation(tokens) {
34+
tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
35+
36+
resetPrank(users.fisherman);
37+
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
38+
39+
resetPrank(users.arbitrator);
40+
// clear subgraph service address from storage
41+
_setStorage_SubgraphService(address(0));
42+
43+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
44+
disputeManager.acceptDispute(disputeID, tokensSlash, false);
45+
}
46+
3047
function test_Indexing_Accept_Dispute_OptParam(
3148
uint256 tokens,
3249
uint256 tokensSlash

‎packages/subgraph-service/test/disputeManager/disputes/indexing/create.t.sol

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,28 @@ import { IDisputeManager } from "../../../../contracts/interfaces/IDisputeManage
77
import { DisputeManagerTest } from "../../DisputeManager.t.sol";
88

99
contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
10-
1110
/*
1211
* TESTS
1312
*/
1413

15-
function test_Indexing_Create_Dispute(
14+
function test_Indexing_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) {
15+
resetPrank(users.fisherman);
16+
_createIndexingDispute(allocationID, bytes32("POI1"));
17+
}
18+
19+
function test_Indexing_Create_Dispute_RevertWhen_SubgraphServiceNotSet(
1620
uint256 tokens
1721
) public useIndexer useAllocation(tokens) {
1822
resetPrank(users.fisherman);
19-
_createIndexingDispute(allocationID, bytes32("POI1"));
23+
24+
// clear subgraph service address from storage
25+
_setStorage_SubgraphService(address(0));
26+
27+
// // Approve the dispute deposit
28+
token.approve(address(disputeManager), disputeDeposit);
29+
30+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
31+
disputeManager.createIndexingDispute(allocationID, bytes32("POI2"));
2032
}
2133

2234
function test_Indexing_Create_MultipleDisputes() public {
@@ -33,7 +45,12 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
3345
_createProvision(indexer, tokens, maxSlashingPercentage, disputePeriod);
3446
_register(indexer, abi.encode("url", "geoHash", address(0)));
3547
uint256 allocationIDPrivateKey = uint256(keccak256(abi.encodePacked(i)));
36-
bytes memory data = _createSubgraphAllocationData(indexer, subgraphDeployment, allocationIDPrivateKey, tokens);
48+
bytes memory data = _createSubgraphAllocationData(
49+
indexer,
50+
subgraphDeployment,
51+
allocationIDPrivateKey,
52+
tokens
53+
);
3754
_startService(indexer, data);
3855
allocationIDPrivateKeys[i] = allocationIDPrivateKey;
3956
}
@@ -48,7 +65,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
4865
uint256 tokens
4966
) public useIndexer useAllocation(tokens) {
5067
resetPrank(users.fisherman);
51-
bytes32 disputeID =_createIndexingDispute(allocationID, bytes32("POI1"));
68+
bytes32 disputeID = _createIndexingDispute(allocationID, bytes32("POI1"));
5269

5370
// Create another dispute with different fisherman
5471
address otherFisherman = makeAddr("otherFisherman");
@@ -78,9 +95,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
7895
vm.stopPrank();
7996
}
8097

81-
function test_Indexing_Create_RevertIf_AllocationDoesNotExist(
82-
uint256 tokens
83-
) public useFisherman {
98+
function test_Indexing_Create_RevertIf_AllocationDoesNotExist(uint256 tokens) public useFisherman {
8499
tokens = bound(tokens, disputeDeposit, 10_000_000_000 ether);
85100
token.approve(address(disputeManager), tokens);
86101
bytes memory expectedError = abi.encodeWithSelector(
@@ -92,9 +107,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
92107
vm.stopPrank();
93108
}
94109

95-
function test_Indexing_Create_RevertIf_IndexerIsBelowStake(
96-
uint256 tokens
97-
) public useIndexer useAllocation(tokens) {
110+
function test_Indexing_Create_RevertIf_IndexerIsBelowStake(uint256 tokens) public useIndexer useAllocation(tokens) {
98111
// Close allocation
99112
bytes memory data = abi.encode(allocationID);
100113
_stopService(users.indexer, data);

‎packages/subgraph-service/test/disputeManager/disputes/query/accept.t.sol

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,24 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest {
3434
_acceptDispute(disputeID, tokensSlash, false);
3535
}
3636

37+
function test_Query_Accept_Dispute_RevertWhen_SubgraphServiceNotSet(
38+
uint256 tokens,
39+
uint256 tokensSlash
40+
) public useIndexer useAllocation(tokens) {
41+
tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
42+
43+
resetPrank(users.fisherman);
44+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
45+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
46+
bytes32 disputeID = _createQueryDispute(attestationData);
47+
48+
resetPrank(users.arbitrator);
49+
// clear subgraph service address from storage
50+
_setStorage_SubgraphService(address(0));
51+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
52+
disputeManager.acceptDispute(disputeID, tokensSlash, false);
53+
}
54+
3755
function test_Query_Accept_Dispute_OptParam(
3856
uint256 tokens,
3957
uint256 tokensSlash

‎packages/subgraph-service/test/disputeManager/disputes/query/create.t.sol

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,28 @@ contract DisputeManagerQueryCreateDisputeTest is DisputeManagerTest {
1717
* TESTS
1818
*/
1919

20-
function test_Query_Create_Dispute(uint256 tokens) public useIndexer useAllocation(tokens) {
20+
function test_Query_Create_Dispute_Only(uint256 tokens) public useIndexer useAllocation(tokens) {
2121
resetPrank(users.fisherman);
2222
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
2323
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
2424
_createQueryDispute(attestationData);
2525
}
2626

27+
function test_Query_Create_Dispute_RevertWhen_SubgraphServiceNotSet(uint256 tokens) public useIndexer useAllocation(tokens) {
28+
resetPrank(users.fisherman);
29+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
30+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
31+
32+
// clear subgraph service address from storage
33+
_setStorage_SubgraphService(address(0));
34+
35+
// // Approve the dispute deposit
36+
token.approve(address(disputeManager), disputeDeposit);
37+
38+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerSubgraphServiceNotSet.selector));
39+
disputeManager.createQueryDispute(attestationData);
40+
}
41+
2742
function test_Query_Create_MultipleDisputes_DifferentFisherman(
2843
uint256 tokens
2944
) public useIndexer useAllocation(tokens) {

‎packages/subgraph-service/test/disputeManager/governance/subgraphService.t.sol

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity 0.8.27;
3+
4+
import "forge-std/Test.sol";
5+
6+
import { IDisputeManager } from "../../../contracts/interfaces/IDisputeManager.sol";
7+
import { DisputeManagerTest } from "../DisputeManager.t.sol";
8+
9+
contract DisputeManagerGovernanceSubgraphService is DisputeManagerTest {
10+
11+
/*
12+
* TESTS
13+
*/
14+
15+
function test_Governance_SetSubgraphService(address subgraphService) public useGovernor {
16+
vm.assume(subgraphService != address(0));
17+
_setSubgraphService(subgraphService);
18+
}
19+
20+
function test_Governance_SetSubgraphService_RevertWhenZero() public useGovernor {
21+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerInvalidZeroAddress.selector));
22+
disputeManager.setSubgraphService(address(0));
23+
}
24+
}

0 commit comments

Comments
 (0)