Skip to content

Commit 02cac66

Browse files
tmigonetmigone
committed
fix: prevent legacy slash from breaking provision accounting (OZ H-01)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 120ec50 commit 02cac66

File tree

11 files changed

+284
-21
lines changed

11 files changed

+284
-21
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ interface IHorizonStakingBase {
171171
/**
172172
* @notice Gets the amount of thawed tokens that can be releasedfor a given provision.
173173
* @dev Note that the value returned by this function does not return the total amount of thawed tokens
174-
* but only those that can be released. If thaw requests are created with different thawing periods it's
175-
* possible that an unexpired thaw request temporarily blocks the release of other ones that have already
174+
* but only those that can be released. If thaw requests are created with different thawing periods it's
175+
* possible that an unexpired thaw request temporarily blocks the release of other ones that have already
176176
* expired. This function will stop counting when it encounters the first thaw request that is not yet expired.
177177
* @param thawRequestType The type of thaw request.
178178
* @param serviceProvider The address of the service provider.

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

+4
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ interface IHorizonStakingExtension is IRewardsIssuer {
140140

141141
/**
142142
* @notice Slash the indexer stake. Delegated tokens are not subject to slashing.
143+
* Note that depending on the state of the indexer's stake, the slashed amount might be smaller than the
144+
* requested slash amount. This can happen if the indexer has moved a significant part of their stake to
145+
* a provision. Any outstanding slashing amount should be settled using Horizon's slash function
146+
* {IHorizonStaking.slash}.
143147
* @dev Can only be called by the slasher role.
144148
* @param indexer Address of indexer to slash
145149
* @param tokens Amount of tokens to slash from the indexer stake

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

+12
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,18 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
184184
}
185185
}
186186

187+
// Slashing tokens that are already provisioned would break provision accounting, we need to limit
188+
// the slash amount. This can be compensated for, by slashing with the main slash function if needed.
189+
uint256 slashableStake = indexerStake.tokensStaked - indexerStake.tokensProvisioned;
190+
if (slashableStake == 0) {
191+
emit StakeSlashed(indexer, 0, 0, beneficiary);
192+
return;
193+
}
194+
if (tokens > slashableStake) {
195+
reward = (reward * slashableStake) / tokens;
196+
tokens = slashableStake;
197+
}
198+
187199
// Remove tokens to slash from the stake
188200
indexerStake.tokensStaked = indexerStake.tokensStaked - tokens;
189201

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -1602,8 +1602,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
16021602
);
16031603
if (isDelegationSlashingEnabled) {
16041604
uint256 poolThawingTokens = (before.pool.tokensThawing *
1605-
(before.pool.tokens - calcValues.delegationTokensSlashed)) /
1606-
before.pool.tokens;
1605+
(before.pool.tokens - calcValues.delegationTokensSlashed)) / before.pool.tokens;
16071606
assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens);
16081607
assertEq(afterPool.shares, before.pool.shares);
16091608
assertEq(afterPool.tokensThawing, poolThawingTokens);

‎packages/horizon/test/staking/slash/legacySlash.t.sol

+27-8
Original file line numberDiff line numberDiff line change
@@ -45,34 +45,53 @@ contract HorizonStakingLegacySlashTest is HorizonStakingTest {
4545
// before
4646
uint256 beforeStakingBalance = token.balanceOf(address(staking));
4747
uint256 beforeRewardsDestinationBalance = token.balanceOf(_beneficiary);
48+
ServiceProviderInternal memory beforeIndexer = _getStorage_ServiceProviderInternal(_indexer);
49+
50+
// calculate slashable stake
51+
uint256 slashableStake = beforeIndexer.tokensStaked - beforeIndexer.tokensProvisioned;
52+
uint256 actualTokens = _tokens;
53+
uint256 actualRewards = _rewards;
54+
if (slashableStake == 0) {
55+
actualTokens = 0;
56+
actualRewards = 0;
57+
} else if (_tokens > slashableStake) {
58+
actualRewards = (_rewards * slashableStake) / _tokens;
59+
actualTokens = slashableStake;
60+
}
4861

4962
// slash
5063
vm.expectEmit(address(staking));
51-
emit IHorizonStakingExtension.StakeSlashed(_indexer, _tokens, _rewards, _beneficiary);
64+
emit IHorizonStakingExtension.StakeSlashed(_indexer, actualTokens, actualRewards, _beneficiary);
5265
staking.slash(_indexer, _tokens, _rewards, _beneficiary);
5366

5467
// after
5568
uint256 afterStakingBalance = token.balanceOf(address(staking));
5669
uint256 afterRewardsDestinationBalance = token.balanceOf(_beneficiary);
70+
ServiceProviderInternal memory afterIndexer = _getStorage_ServiceProviderInternal(_indexer);
5771

58-
assertEq(beforeStakingBalance - _tokens, afterStakingBalance);
59-
assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - _rewards);
72+
assertEq(beforeStakingBalance - actualTokens, afterStakingBalance);
73+
assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - actualRewards);
74+
assertEq(afterIndexer.tokensStaked, beforeIndexer.tokensStaked - actualTokens);
6075
}
6176

6277
/*
6378
* TESTS
6479
*/
65-
6680
function testSlash_Legacy(
67-
uint256 tokens,
81+
uint256 tokensStaked,
82+
uint256 tokensProvisioned,
6883
uint256 slashTokens,
6984
uint256 reward
7085
) public useIndexer useLegacySlasher(users.legacySlasher) {
71-
vm.assume(tokens > 1);
72-
slashTokens = bound(slashTokens, 1, tokens);
86+
vm.assume(tokensStaked > 0);
87+
vm.assume(tokensStaked <= MAX_STAKING_TOKENS);
88+
vm.assume(tokensProvisioned > 0);
89+
vm.assume(tokensProvisioned <= tokensStaked);
90+
slashTokens = bound(slashTokens, 1, tokensStaked);
7391
reward = bound(reward, 0, slashTokens);
7492

75-
_createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0);
93+
_stake(tokensStaked);
94+
_provision(users.indexer, subgraphDataServiceLegacyAddress, tokensProvisioned, 0, 0);
7695

7796
resetPrank(users.legacySlasher);
7897
_legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman"));

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
9696

9797
function testSlash_RevertWhen_NoProvision(uint256 tokens, uint256 slashTokens) public useIndexer useStake(tokens) {
9898
vm.assume(slashTokens > 0);
99-
bytes memory expectedError = abi.encodeWithSelector(
100-
IHorizonStakingMain.HorizonStakingNoTokensToSlash.selector
101-
);
99+
bytes memory expectedError = abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingNoTokensToSlash.selector);
102100
vm.expectRevert(expectedError);
103101
vm.startPrank(subgraphDataServiceAddress);
104102
staking.slash(users.indexer, slashTokens, 0, subgraphDataServiceAddress);

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

+39
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,45 @@ contract DisputeManager is
196196
return (dId1, dId2);
197197
}
198198

199+
/// @inheritdoc IDisputeManager
200+
function createLegacyDispute(
201+
address allocationId,
202+
address fisherman,
203+
uint256 tokensSlash,
204+
uint256 tokensRewards
205+
) external override onlyArbitrator returns (bytes32) {
206+
// Create a disputeId
207+
bytes32 disputeId = keccak256(abi.encodePacked(allocationId, "legacy"));
208+
209+
// Get the indexer for the legacy allocation
210+
address indexer = _graphStaking().getAllocation(allocationId).indexer;
211+
require(indexer != address(0), DisputeManagerIndexerNotFound(allocationId));
212+
213+
// Store dispute
214+
disputes[disputeId] = Dispute(
215+
indexer,
216+
fisherman,
217+
0,
218+
0,
219+
DisputeType.LegacyDispute,
220+
IDisputeManager.DisputeStatus.Accepted,
221+
block.timestamp,
222+
0
223+
);
224+
225+
// Slash the indexer
226+
ISubgraphService subgraphService_ = _getSubgraphService();
227+
subgraphService_.slash(indexer, abi.encode(tokensSlash, tokensRewards));
228+
229+
// Reward the fisherman
230+
_graphToken().pushTokens(fisherman, tokensRewards);
231+
232+
emit LegacyDisputeCreated(disputeId, indexer, fisherman, allocationId, tokensSlash, tokensRewards);
233+
emit DisputeAccepted(disputeId, indexer, fisherman, tokensRewards);
234+
235+
return disputeId;
236+
}
237+
199238
/// @inheritdoc IDisputeManager
200239
function acceptDispute(
201240
bytes32 disputeId,

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

+54-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ interface IDisputeManager {
1515
enum DisputeType {
1616
Null,
1717
IndexingDispute,
18-
QueryDispute
18+
QueryDispute,
19+
LegacyDispute
1920
}
2021

2122
/// @notice Status of a dispute
@@ -130,6 +131,25 @@ interface IDisputeManager {
130131
uint256 stakeSnapshot
131132
);
132133

134+
/**
135+
* @dev Emitted when a legacy dispute is created for `allocationId` and `fisherman`.
136+
* The event emits the amount of `tokensSlash` to slash and `tokensRewards` to reward the fisherman.
137+
* @param disputeId The dispute id
138+
* @param indexer The indexer address
139+
* @param fisherman The fisherman address to be credited with the rewards
140+
* @param allocationId The allocation id
141+
* @param tokensSlash The amount of tokens to slash
142+
* @param tokensRewards The amount of tokens to reward the fisherman
143+
*/
144+
event LegacyDisputeCreated(
145+
bytes32 indexed disputeId,
146+
address indexed indexer,
147+
address indexed fisherman,
148+
address allocationId,
149+
uint256 tokensSlash,
150+
uint256 tokensRewards
151+
);
152+
133153
/**
134154
* @dev Emitted when arbitrator accepts a `disputeId` to `indexer` created by `fisherman`.
135155
* The event emits the amount `tokens` transferred to the fisherman, the deposit plus reward.
@@ -436,6 +456,39 @@ interface IDisputeManager {
436456
*/
437457
function createIndexingDispute(address allocationId, bytes32 poi) external returns (bytes32);
438458

459+
/**
460+
* @notice Create a legacy dispute.
461+
* This disputes can be created to settle outstanding slashing amounts with an indexer that has been
462+
* "legacy slashed" during or shortly after the transition period. See {HorizonStakingExtension.legacySlash}
463+
* for more details.
464+
*
465+
* Note that this type of dispute:
466+
* - can only be created by the arbitrator
467+
* - does not require a bond
468+
* - is automatically accepted when created
469+
*
470+
* Additionally, note that this type of disputes allow the arbitrator to directly set the slash and rewards
471+
* amounts, bypassing the usual mechanisms that impose restrictions on those. This is done to give arbitrators
472+
* maximum flexibility to ensure outstanding slashing amounts are settled fairly. This function needs to be removed
473+
* after the transition period.
474+
*
475+
* Requirements:
476+
* - Indexer must have been legacy slashed during or shortly after the transition period
477+
* - Indexer must have provisioned funds to the Subgraph Service
478+
*
479+
* @param allocationId The allocation to dispute
480+
* @param fisherman The fisherman address to be credited with the rewards
481+
* @param tokensSlash The amount of tokens to slash
482+
* @param tokensRewards The amount of tokens to reward the fisherman
483+
* @return The dispute id
484+
*/
485+
function createLegacyDispute(
486+
address allocationId,
487+
address fisherman,
488+
uint256 tokensSlash,
489+
uint256 tokensRewards
490+
) external returns (bytes32);
491+
439492
// -- Arbitrator --
440493

441494
/**

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

+75
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,81 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
195195
return _disputeID;
196196
}
197197

198+
struct Balances {
199+
uint256 indexer;
200+
uint256 fisherman;
201+
uint256 arbitrator;
202+
uint256 disputeManager;
203+
uint256 staking;
204+
}
205+
206+
function _createLegacyDispute(
207+
address _allocationId,
208+
address _fisherman,
209+
uint256 _tokensSlash,
210+
uint256 _tokensRewards
211+
) internal returns (bytes32) {
212+
(, address arbitrator, ) = vm.readCallers();
213+
address indexer = staking.getAllocation(_allocationId).indexer;
214+
215+
Balances memory beforeBalances = Balances({
216+
indexer: token.balanceOf(indexer),
217+
fisherman: token.balanceOf(_fisherman),
218+
arbitrator: token.balanceOf(arbitrator),
219+
disputeManager: token.balanceOf(address(disputeManager)),
220+
staking: token.balanceOf(address(staking))
221+
});
222+
223+
vm.expectEmit(address(disputeManager));
224+
emit IDisputeManager.LegacyDisputeCreated(
225+
keccak256(abi.encodePacked(_allocationId, "legacy")),
226+
indexer,
227+
_fisherman,
228+
_allocationId,
229+
_tokensSlash,
230+
_tokensRewards
231+
);
232+
vm.expectEmit(address(disputeManager));
233+
emit IDisputeManager.DisputeAccepted(
234+
keccak256(abi.encodePacked(_allocationId, "legacy")),
235+
indexer,
236+
_fisherman,
237+
_tokensRewards
238+
);
239+
bytes32 _disputeId = disputeManager.createLegacyDispute(
240+
_allocationId,
241+
_fisherman,
242+
_tokensSlash,
243+
_tokensRewards
244+
);
245+
246+
Balances memory afterBalances = Balances({
247+
indexer: token.balanceOf(indexer),
248+
fisherman: token.balanceOf(_fisherman),
249+
arbitrator: token.balanceOf(arbitrator),
250+
disputeManager: token.balanceOf(address(disputeManager)),
251+
staking: token.balanceOf(address(staking))
252+
});
253+
254+
assertEq(afterBalances.indexer, beforeBalances.indexer);
255+
assertEq(afterBalances.fisherman, beforeBalances.fisherman + _tokensRewards);
256+
assertEq(afterBalances.arbitrator, beforeBalances.arbitrator);
257+
assertEq(afterBalances.disputeManager, beforeBalances.disputeManager);
258+
assertEq(afterBalances.staking, beforeBalances.staking - _tokensSlash);
259+
260+
IDisputeManager.Dispute memory dispute = _getDispute(_disputeId);
261+
assertEq(dispute.indexer, indexer);
262+
assertEq(dispute.fisherman, _fisherman);
263+
assertEq(dispute.deposit, 0);
264+
assertEq(dispute.relatedDisputeId, bytes32(0));
265+
assertEq(uint8(dispute.disputeType), uint8(IDisputeManager.DisputeType.LegacyDispute));
266+
assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Accepted));
267+
assertEq(dispute.createdAt, block.timestamp);
268+
assertEq(dispute.stakeSnapshot, 0);
269+
270+
return _disputeId;
271+
}
272+
198273
struct BeforeValues_CreateQueryDisputeConflict {
199274
Attestation.State attestation1;
200275
Attestation.State attestation2;

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

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity 0.8.27;
3+
4+
import "forge-std/Test.sol";
5+
6+
import { Attestation } from "../../../contracts/libraries/Attestation.sol";
7+
import { PPMMath } from "@graphprotocol/horizon/contracts/libraries/PPMMath.sol";
8+
import { IDisputeManager } from "../../../contracts/interfaces/IDisputeManager.sol";
9+
import { DisputeManagerTest } from "../DisputeManager.t.sol";
10+
11+
contract DisputeManagerLegacyDisputeTest is DisputeManagerTest {
12+
using PPMMath for uint256;
13+
14+
bytes32 private requestCID = keccak256(abi.encodePacked("Request CID"));
15+
bytes32 private responseCID = keccak256(abi.encodePacked("Response CID"));
16+
bytes32 private subgraphDeploymentId = keccak256(abi.encodePacked("Subgraph Deployment ID"));
17+
18+
/*
19+
* TESTS
20+
*/
21+
22+
function test_LegacyDispute(
23+
uint256 tokensStaked,
24+
uint256 tokensProvisioned,
25+
uint256 tokensSlash,
26+
uint256 tokensRewards
27+
) public {
28+
vm.assume(tokensStaked <= MAX_TOKENS);
29+
vm.assume(tokensStaked >= minimumProvisionTokens);
30+
tokensProvisioned = bound(tokensProvisioned, minimumProvisionTokens, tokensStaked);
31+
tokensSlash = bound(tokensSlash, 2, tokensProvisioned);
32+
tokensRewards = bound(tokensRewards, 1, tokensSlash.mulPPM(maxSlashingPercentage));
33+
34+
// setup indexer state
35+
resetPrank(users.indexer);
36+
_stake(tokensStaked);
37+
_setStorage_allocation_hardcoded(users.indexer, allocationID, tokensStaked - tokensProvisioned);
38+
_provision(users.indexer, tokensProvisioned, maxSlashingPercentage, disputePeriod);
39+
40+
resetPrank(users.arbitrator);
41+
_createLegacyDispute(allocationID, users.fisherman, tokensSlash, tokensRewards);
42+
}
43+
44+
function test_LegacyDispute_RevertIf_NotArbitrator() public useIndexer {
45+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerNotArbitrator.selector));
46+
disputeManager.createLegacyDispute(allocationID, users.fisherman, 0, 0);
47+
}
48+
49+
function test_LegacyDispute_RevertIf_AllocationNotFound() public useIndexer {
50+
resetPrank(users.arbitrator);
51+
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerIndexerNotFound.selector, address(0)));
52+
disputeManager.createLegacyDispute(address(0), users.fisherman, 0, 0);
53+
}
54+
}

0 commit comments

Comments
 (0)