Skip to content

Commit b7991a5

Browse files
tmigonetmigone
committed
fix: prevent slashing DoS (OZ M-02)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 972c4e1 commit b7991a5

File tree

17 files changed

+96
-35
lines changed

17 files changed

+96
-35
lines changed

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

+9-4
Original file line numberDiff line numberDiff line change
@@ -569,10 +569,15 @@ contract DisputeManager is
569569
DisputeManagerInvalidTokensSlash(_tokensSlash, maxTokensSlash)
570570
);
571571

572-
// Rewards amount can only be extracted from service provider tokens so
573-
// we grab the minimum between the slash amount and indexer's tokens
574-
uint256 maxRewardableTokens = Math.min(_tokensSlash, provision.tokens);
575-
uint256 tokensRewards = uint256(fishermanRewardCut).mulPPM(maxRewardableTokens);
572+
// Rewards calculation:
573+
// - Rewards can only be extracted from service provider tokens so we grab the minimum between the slash
574+
// amount and indexer's tokens
575+
// - The applied cut is the minimum between the provision's maxVerifierCut and the current fishermanRewardCut. This
576+
// protects the indexer from sudden changes to the fishermanRewardCut while ensuring the slashing does not revert due
577+
// to excessive rewards being requested.
578+
uint256 maxRewardableTokens = MathUtils.min(_tokensSlash, provision.tokens);
579+
uint256 effectiveCut = MathUtils.min(provision.maxVerifierCut, fishermanRewardCut);
580+
uint256 tokensRewards = effectiveCut.mulPPM(maxRewardableTokens);
576581

577582
subgraphService_.slash(_indexer, abi.encode(_tokensSlash, tokensRewards));
578583
return tokensRewards;

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

+12-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pragma solidity 0.8.27;
33

44
import "forge-std/Test.sol";
55

6+
import { MathUtils } from "@graphprotocol/horizon/contracts/libraries/MathUtils.sol";
67
import { PPMMath } from "@graphprotocol/horizon/contracts/libraries/PPMMath.sol";
78
import { IDisputeManager } from "../../contracts/interfaces/IDisputeManager.sol";
89
import { Attestation } from "../../contracts/libraries/Attestation.sol";
@@ -412,8 +413,17 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
412413
uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService));
413414
uint256 provisionTokens = staking.getProvision(dispute.indexer, address(subgraphService)).tokens;
414415
uint256 disputeDeposit = dispute.deposit;
415-
uint256 fishermanRewardPercentage = disputeManager.fishermanRewardCut();
416-
uint256 fishermanReward = Math.min(_tokensSlash, provisionTokens).mulPPM(fishermanRewardPercentage);
416+
uint256 fishermanReward;
417+
{
418+
uint32 provisionMaxVerifierCut = staking
419+
.getProvision(dispute.indexer, address(subgraphService))
420+
.maxVerifierCut;
421+
uint256 fishermanRewardPercentage = MathUtils.min(
422+
disputeManager.fishermanRewardCut(),
423+
provisionMaxVerifierCut
424+
);
425+
fishermanReward = Math.min(_tokensSlash, provisionTokens).mulPPM(fishermanRewardPercentage);
426+
}
417427

418428
vm.expectEmit(address(disputeManager));
419429
emit IDisputeManager.DisputeAccepted(

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ contract DisputeManagerIndexingCreateDisputeTest is DisputeManagerTest {
4242

4343
resetPrank(indexer);
4444
mint(indexer, tokens);
45-
_createProvision(indexer, tokens, maxSlashingPercentage, disputePeriod);
45+
_createProvision(indexer, tokens, fishermanRewardPercentage, disputePeriod);
4646
_register(indexer, abi.encode("url", "geoHash", address(0)));
4747
uint256 allocationIDPrivateKey = uint256(keccak256(abi.encodePacked(i)));
4848
bytes memory data = _createSubgraphAllocationData(

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ contract DisputeManagerLegacyDisputeTest is DisputeManagerTest {
2929
vm.assume(tokensStaked >= minimumProvisionTokens);
3030
tokensProvisioned = bound(tokensProvisioned, minimumProvisionTokens, tokensStaked);
3131
tokensSlash = bound(tokensSlash, 2, tokensProvisioned);
32-
tokensRewards = bound(tokensRewards, 1, tokensSlash.mulPPM(maxSlashingPercentage));
32+
tokensRewards = bound(tokensRewards, 1, tokensSlash.mulPPM(fishermanRewardPercentage));
3333

3434
// setup indexer state
3535
resetPrank(users.indexer);
3636
_stake(tokensStaked);
3737
_setStorage_allocation_hardcoded(users.indexer, allocationID, tokensStaked - tokensProvisioned);
38-
_provision(users.indexer, tokensProvisioned, maxSlashingPercentage, disputePeriod);
38+
_provision(users.indexer, tokensProvisioned, fishermanRewardPercentage, disputePeriod);
3939

4040
resetPrank(users.arbitrator);
4141
_createAndAcceptLegacyDispute(allocationID, users.fisherman, tokensSlash, tokensRewards);

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

+46
Original file line numberDiff line numberDiff line change
@@ -222,4 +222,50 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest {
222222
vm.expectRevert(expectedError);
223223
disputeManager.acceptDispute(disputeID, tokensSlash);
224224
}
225+
226+
function test_Query_Accept_Dispute_AfterFishermanRewardCutIncreased(
227+
uint256 tokens,
228+
uint256 tokensSlash
229+
) public useIndexer {
230+
vm.assume(tokens >= minimumProvisionTokens);
231+
vm.assume(tokens < 10_000_000_000 ether);
232+
tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
233+
234+
// Set fishermanRewardCut to 25%
235+
resetPrank(users.governor);
236+
uint32 oldFishermanRewardCut = 250_000;
237+
disputeManager.setFishermanRewardCut(oldFishermanRewardCut);
238+
239+
// Create provision with maxVerifierCut == fishermanRewardCut and allocate
240+
resetPrank(users.indexer);
241+
_createProvision(users.indexer, tokens, oldFishermanRewardCut, disputePeriod);
242+
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
243+
bytes memory data = _createSubgraphAllocationData(
244+
users.indexer,
245+
subgraphDeployment,
246+
allocationIDPrivateKey,
247+
tokens
248+
);
249+
_startService(users.indexer, data);
250+
251+
// Create a dispute with prov.maxVerifierCut == fishermanRewardCut
252+
uint256 beforeFishermanBalance = token.balanceOf(users.fisherman);
253+
resetPrank(users.fisherman);
254+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
255+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
256+
bytes32 disputeID = _createQueryDispute(attestationData);
257+
258+
// Now bump the fishermanRewardCut to 50%
259+
resetPrank(users.governor);
260+
disputeManager.setFishermanRewardCut(500_000);
261+
262+
// Accept the dispute
263+
resetPrank(users.arbitrator);
264+
_acceptDispute(disputeID, tokensSlash);
265+
266+
// Check that the fisherman received the correct amount of tokens
267+
// which should use the old fishermanRewardCut
268+
uint256 afterFishermanBalance = token.balanceOf(users.fisherman);
269+
assertEq(afterFishermanBalance, beforeFishermanBalance + tokensSlash.mulPPM(oldFishermanRewardCut));
270+
}
225271
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ contract DisputeManagerQueryCreateDisputeTest is DisputeManagerTest {
7878
uint256 newAllocationIDKey = uint256(keccak256(abi.encodePacked("newAllocationID")));
7979
mint(newIndexer, tokens);
8080
resetPrank(newIndexer);
81-
_createProvision(newIndexer, tokens, maxSlashingPercentage, disputePeriod);
81+
_createProvision(newIndexer, tokens, fishermanRewardPercentage, disputePeriod);
8282
_register(newIndexer, abi.encode("url", "geoHash", 0x0));
8383
bytes memory data = _createSubgraphAllocationData(newIndexer, subgraphDeployment, newAllocationIDKey, tokens);
8484
_startService(newIndexer, data);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ contract DisputeManagerQueryConflictAcceptDisputeTest is DisputeManagerTest {
150150
mint(differentIndexer, tokensSecondIndexer);
151151
uint256 differentIndexerAllocationIDPrivateKey = uint256(keccak256(abi.encodePacked(differentIndexer)));
152152
resetPrank(differentIndexer);
153-
_createProvision(differentIndexer, tokensSecondIndexer, maxSlashingPercentage, disputePeriod);
153+
_createProvision(differentIndexer, tokensSecondIndexer, fishermanRewardPercentage, disputePeriod);
154154
_register(differentIndexer, abi.encode("url", "geoHash", address(0)));
155155
bytes memory data = _createSubgraphAllocationData(
156156
differentIndexer,

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ contract DisputeManagerQueryConflictCreateDisputeTest is DisputeManagerTest {
3838
uint256 newAllocationIDKey = uint256(keccak256(abi.encodePacked("newAllocationID")));
3939
mint(newIndexer, tokens);
4040
resetPrank(newIndexer);
41-
_createProvision(newIndexer, tokens, maxSlashingPercentage, disputePeriod);
41+
_createProvision(newIndexer, tokens, fishermanRewardPercentage, disputePeriod);
4242
_register(newIndexer, abi.encode("url", "geoHash", 0x0));
4343
bytes memory data = _createSubgraphAllocationData(newIndexer, subgraphDeployment, newAllocationIDKey, tokens);
4444
_startService(newIndexer, data);

‎packages/subgraph-service/test/shared/SubgraphServiceShared.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
3535
modifier useAllocation(uint256 tokens) {
3636
vm.assume(tokens >= minimumProvisionTokens);
3737
vm.assume(tokens < 10_000_000_000 ether);
38-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
38+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
3939
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
4040
bytes memory data = _createSubgraphAllocationData(
4141
users.indexer,

‎packages/subgraph-service/test/subgraphService/SubgraphService.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ contract SubgraphServiceTest is SubgraphServiceSharedTest {
481481
resetPrank(_indexer);
482482
token.approve(address(staking), _tokens);
483483
staking.stakeTo(_indexer, _tokens);
484-
staking.provision(_indexer, address(subgraphService), _tokens, maxSlashingPercentage, disputePeriod);
484+
staking.provision(_indexer, address(subgraphService), _tokens, fishermanRewardPercentage, disputePeriod);
485485
_register(_indexer, abi.encode("url", "geoHash", address(0)));
486486

487487
(address newIndexerAllocationId, uint256 newIndexerAllocationKey) = makeAddrAndKey("newIndexerAllocationId");

‎packages/subgraph-service/test/subgraphService/allocation/forceClose.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ contract SubgraphServiceAllocationForceCloseTest is SubgraphServiceTest {
7878
function test_SubgraphService_Allocation_ForceClose_RevertIf_Altruistic(uint256 tokens) public useIndexer {
7979
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
8080

81-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
81+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
8282
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
8383

8484
bytes memory data = _createSubgraphAllocationData(users.indexer, subgraphDeployment, allocationIDPrivateKey, 0);

‎packages/subgraph-service/test/subgraphService/allocation/overDelegated.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ contract SubgraphServiceAllocationOverDelegatedTest is SubgraphServiceTest {
3030

3131
// Create provision
3232
token.approve(address(staking), indexerTokens);
33-
_createProvision(users.indexer, indexerTokens, maxSlashingPercentage, disputePeriod);
33+
_createProvision(users.indexer, indexerTokens, fishermanRewardPercentage, disputePeriod);
3434
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
3535

3636
// Delegate so that indexer is over allocated

‎packages/subgraph-service/test/subgraphService/allocation/start.t.sol

+12-12
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
2121
function test_SubgraphService_Allocation_Start(uint256 tokens) public useIndexer {
2222
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
2323

24-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
24+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
2525
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
2626

2727
bytes memory data = _generateData(tokens);
@@ -31,7 +31,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
3131
function test_SubgraphService_Allocation_Start_AllowsZeroTokens(uint256 tokens) public useIndexer {
3232
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
3333

34-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
34+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
3535
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
3636

3737
bytes memory data = _generateData(0);
@@ -41,7 +41,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
4141
function test_SubgraphService_Allocation_Start_ByOperator(uint256 tokens) public useOperator {
4242
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
4343

44-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
44+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
4545
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
4646

4747
bytes memory data = _generateData(tokens);
@@ -51,7 +51,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
5151
function test_SubgraphService_Allocation_Start_RevertWhen_NotAuthorized(uint256 tokens) public useIndexer {
5252
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
5353

54-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
54+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
5555
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
5656

5757
resetPrank(users.operator);
@@ -79,7 +79,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
7979
function test_SubgraphService_Allocation_Start_RevertWhen_NotRegistered(uint256 tokens) public useIndexer {
8080
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
8181

82-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
82+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
8383

8484
bytes memory data = _generateData(tokens);
8585
vm.expectRevert(
@@ -91,7 +91,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
9191
function test_SubgraphService_Allocation_Start_RevertWhen_ZeroAllocationId(uint256 tokens) public useIndexer {
9292
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
9393

94-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
94+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
9595
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
9696

9797
bytes32 digest = subgraphService.encodeAllocationProof(users.indexer, address(0));
@@ -104,7 +104,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
104104
function test_SubgraphService_Allocation_Start_RevertWhen_InvalidSignature(uint256 tokens) public useIndexer {
105105
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
106106

107-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
107+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
108108
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
109109

110110
(address signer, uint256 signerPrivateKey) = makeAddrAndKey("invalidSigner");
@@ -124,7 +124,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
124124
function test_SubgraphService_Allocation_Start_RevertWhen_InvalidData(uint256 tokens) public useIndexer {
125125
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
126126

127-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
127+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
128128
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
129129

130130
bytes memory data = abi.encode(subgraphDeployment, tokens, allocationID, _generateRandomHexBytes(32));
@@ -137,7 +137,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
137137
) public useIndexer {
138138
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
139139

140-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
140+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
141141
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
142142

143143
bytes memory data = _generateData(tokens);
@@ -150,7 +150,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
150150
function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Migrated(uint256 tokens) public useIndexer {
151151
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
152152

153-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
153+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
154154
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
155155

156156
resetPrank(users.governor);
@@ -165,7 +165,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
165165
function test_SubgraphService_Allocation_Start_RevertWhen_AlreadyExists_Staking(uint256 tokens) public useIndexer {
166166
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
167167

168-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
168+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
169169
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
170170

171171
// create dummy allo in staking contract
@@ -183,7 +183,7 @@ contract SubgraphServiceAllocationStartTest is SubgraphServiceTest {
183183
tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS - 1);
184184
lockTokens = bound(lockTokens, tokens + 1, MAX_TOKENS);
185185

186-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
186+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
187187
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
188188

189189
bytes memory data = _generateData(lockTokens);

‎packages/subgraph-service/test/subgraphService/collect/indexing/indexing.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ contract SubgraphServiceCollectIndexingTest is SubgraphServiceTest {
127127
tokens = bound(tokens, minimumProvisionTokens * 2, 10_000_000_000 ether);
128128

129129
// setup allocation
130-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
130+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
131131
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
132132
bytes memory data = _createSubgraphAllocationData(
133133
users.indexer,

0 commit comments

Comments
 (0)