Skip to content

Commit 46010c8

Browse files
tmigonetmigone
committed
fix: prevent slashing DoS (OZ M-02)
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
1 parent 6fb20d7 commit 46010c8

File tree

17 files changed

+160
-35
lines changed

17 files changed

+160
-35
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -579,10 +579,15 @@ contract DisputeManager is
579579
DisputeManagerInvalidTokensSlash(_tokensSlash, maxTokensSlash)
580580
);
581581

582-
// Rewards amount can only be extracted from service provider tokens so
583-
// we grab the minimum between the slash amount and indexer's tokens
584-
uint256 maxRewardableTokens = Math.min(_tokensSlash, provision.tokens);
585-
uint256 tokensRewards = uint256(fishermanRewardCut).mulPPM(maxRewardableTokens);
582+
// Rewards calculation:
583+
// - Rewards can only be extracted from service provider tokens so we grab the minimum between the slash
584+
// amount and indexer's tokens
585+
// - The applied cut is the minimum between the provision's maxVerifierCut and the current fishermanRewardCut. This
586+
// protects the indexer from sudden changes to the fishermanRewardCut while ensuring the slashing does not revert due
587+
// to excessive rewards being requested.
588+
uint256 maxRewardableTokens = MathUtils.min(_tokensSlash, provision.tokens);
589+
uint256 effectiveCut = MathUtils.min(provision.maxVerifierCut, fishermanRewardCut);
590+
uint256 tokensRewards = effectiveCut.mulPPM(maxRewardableTokens);
586591

587592
subgraphService_.slash(_indexer, abi.encode(_tokensSlash, tokensRewards));
588593
return tokensRewards;

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

Lines changed: 12 additions & 2 deletions
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 fishermanPreviousBalance = token.balanceOf(fisherman);
413414
uint256 indexerTokensAvailable = staking.getProviderTokensAvailable(dispute.indexer, address(subgraphService));
414415
uint256 disputeDeposit = dispute.deposit;
415-
uint256 fishermanRewardPercentage = disputeManager.fishermanRewardCut();
416-
uint256 fishermanReward = _tokensSlash.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 = _tokensSlash.mulPPM(fishermanRewardPercentage);
426+
}
417427

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

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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,109 @@ contract DisputeManagerQueryAcceptDisputeTest is DisputeManagerTest {
118118
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerDisputeNotInConflict.selector, disputeID));
119119
disputeManager.acceptDisputeConflict(disputeID, tokensSlash, true, 0);
120120
}
121+
122+
function test_Query_Accept_RevertWhen_SlashingOverMaxSlashPercentage_WithDelegation(
123+
uint256 tokens,
124+
uint256 tokensDelegated,
125+
uint256 tokensSlash
126+
) public useIndexer useAllocation(tokens) useDelegation(tokensDelegated) {
127+
uint256 maxTokensToSlash = uint256(maxSlashingPercentage).mulPPM(
128+
_calculateStakeSnapshot(tokens, tokensDelegated)
129+
);
130+
tokensSlash = bound(tokensSlash, maxTokensToSlash + 1, type(uint256).max);
131+
132+
resetPrank(users.fisherman);
133+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
134+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
135+
bytes32 disputeID = _createQueryDispute(attestationData);
136+
137+
// max slashing percentage is 50%
138+
resetPrank(users.arbitrator);
139+
bytes memory expectedError = abi.encodeWithSelector(
140+
IDisputeManager.DisputeManagerInvalidTokensSlash.selector,
141+
tokensSlash,
142+
maxTokensToSlash
143+
);
144+
vm.expectRevert(expectedError);
145+
disputeManager.acceptDispute(disputeID, tokensSlash);
146+
}
147+
148+
149+
function test_Query_Accept_RevertWhen_SlashingOverMaxSlashPercentage_WithDelegation_DelegationSlashing(
150+
uint256 tokens,
151+
uint256 tokensDelegated,
152+
uint256 tokensSlash
153+
) public useIndexer useAllocation(tokens) useDelegation(tokensDelegated) {
154+
// enable delegation slashing
155+
resetPrank(users.governor);
156+
staking.setDelegationSlashingEnabled();
157+
158+
resetPrank(users.fisherman);
159+
uint256 maxTokensToSlash = uint256(maxSlashingPercentage).mulPPM(
160+
_calculateStakeSnapshot(tokens, tokensDelegated)
161+
);
162+
tokensSlash = bound(tokensSlash, maxTokensToSlash + 1, type(uint256).max);
163+
164+
// Create a new dispute with delegation slashing enabled
165+
resetPrank(users.fisherman);
166+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
167+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
168+
bytes32 disputeID = _createQueryDispute(attestationData);
169+
170+
// max slashing percentage is 50%
171+
resetPrank(users.arbitrator);
172+
bytes memory expectedError = abi.encodeWithSelector(
173+
IDisputeManager.DisputeManagerInvalidTokensSlash.selector,
174+
tokensSlash,
175+
maxTokensToSlash
176+
);
177+
vm.expectRevert(expectedError);
178+
disputeManager.acceptDispute(disputeID, tokensSlash);
179+
}
180+
181+
function test_Query_Accept_Dispute_AfterFishermanRewardCutIncreased(
182+
uint256 tokens,
183+
uint256 tokensSlash
184+
) public useIndexer {
185+
vm.assume(tokens >= minimumProvisionTokens);
186+
vm.assume(tokens < 10_000_000_000 ether);
187+
tokensSlash = bound(tokensSlash, 1, uint256(maxSlashingPercentage).mulPPM(tokens));
188+
189+
// Set fishermanRewardCut to 25%
190+
resetPrank(users.governor);
191+
uint32 oldFishermanRewardCut = 250_000;
192+
disputeManager.setFishermanRewardCut(oldFishermanRewardCut);
193+
194+
// Create provision with maxVerifierCut == fishermanRewardCut and allocate
195+
resetPrank(users.indexer);
196+
_createProvision(users.indexer, tokens, oldFishermanRewardCut, disputePeriod);
197+
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
198+
bytes memory data = _createSubgraphAllocationData(
199+
users.indexer,
200+
subgraphDeployment,
201+
allocationIDPrivateKey,
202+
tokens
203+
);
204+
_startService(users.indexer, data);
205+
206+
// Create a dispute with prov.maxVerifierCut == fishermanRewardCut
207+
uint256 beforeFishermanBalance = token.balanceOf(users.fisherman);
208+
resetPrank(users.fisherman);
209+
Attestation.Receipt memory receipt = _createAttestationReceipt(requestCID, responseCID, subgraphDeploymentId);
210+
bytes memory attestationData = _createAtestationData(receipt, allocationIDPrivateKey);
211+
bytes32 disputeID = _createQueryDispute(attestationData);
212+
213+
// Now bump the fishermanRewardCut to 50%
214+
resetPrank(users.governor);
215+
disputeManager.setFishermanRewardCut(500_000);
216+
217+
// Accept the dispute
218+
resetPrank(users.arbitrator);
219+
_acceptDispute(disputeID, tokensSlash);
220+
221+
// Check that the fisherman received the correct amount of tokens
222+
// which should use the old fishermanRewardCut
223+
uint256 afterFishermanBalance = token.balanceOf(users.fisherman);
224+
assertEq(afterFishermanBalance, beforeFishermanBalance + tokensSlash.mulPPM(oldFishermanRewardCut));
225+
}
121226
}

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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 6 additions & 1 deletion
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 { Allocation } from "../../contracts/libraries/Allocation.sol";
78
import { AllocationManager } from "../../contracts/utilities/AllocationManager.sol";
89
import { IDataService } from "@graphprotocol/horizon/contracts/data-service/interfaces/IDataService.sol";
@@ -34,7 +35,7 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
3435
modifier useAllocation(uint256 tokens) {
3536
vm.assume(tokens >= minimumProvisionTokens);
3637
vm.assume(tokens < 10_000_000_000 ether);
37-
_createProvision(users.indexer, tokens, maxSlashingPercentage, disputePeriod);
38+
_createProvision(users.indexer, tokens, fishermanRewardPercentage, disputePeriod);
3839
_register(users.indexer, abi.encode("url", "geoHash", address(0)));
3940
bytes memory data = _createSubgraphAllocationData(
4041
users.indexer,
@@ -194,4 +195,8 @@ abstract contract SubgraphServiceSharedTest is HorizonStakingSharedTest {
194195
(uint256 registeredAt, string memory url, string memory geoHash) = subgraphService.indexers(_indexer);
195196
return ISubgraphService.Indexer({ registeredAt: registeredAt, url: url, geoHash: geoHash });
196197
}
198+
199+
function _calculateStakeSnapshot(uint256 _tokens, uint256 _tokensDelegated) internal view returns (uint256) {
200+
return _tokens + MathUtils.min(_tokensDelegated, _tokens * subgraphService.getDelegationRatio());
201+
}
197202
}

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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

0 commit comments

Comments
 (0)