Skip to content

fix: prevent legacy slash from breaking provision accounting (OZ H-01) #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: horizon-oz2/cr04-missing-event-params
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ interface IHorizonStakingBase {
/**
* @notice Gets the amount of thawed tokens that can be releasedfor a given provision.
* @dev Note that the value returned by this function does not return the total amount of thawed tokens
* but only those that can be released. If thaw requests are created with different thawing periods it's
* possible that an unexpired thaw request temporarily blocks the release of other ones that have already
* but only those that can be released. If thaw requests are created with different thawing periods it's
* possible that an unexpired thaw request temporarily blocks the release of other ones that have already
* expired. This function will stop counting when it encounters the first thaw request that is not yet expired.
* @param thawRequestType The type of thaw request.
* @param serviceProvider The address of the service provider.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ interface IHorizonStakingExtension is IRewardsIssuer {

/**
* @notice Slash the indexer stake. Delegated tokens are not subject to slashing.
* Note that depending on the state of the indexer's stake, the slashed amount might be smaller than the
* requested slash amount. This can happen if the indexer has moved a significant part of their stake to
* a provision. Any outstanding slashing amount should be settled using Horizon's slash function
* {IHorizonStaking.slash}.
* @dev Can only be called by the slasher role.
* @param indexer Address of indexer to slash
* @param tokens Amount of tokens to slash from the indexer stake
Expand Down
12 changes: 12 additions & 0 deletions packages/horizon/contracts/staking/HorizonStakingExtension.sol
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
}
}

// Slashing tokens that are already provisioned would break provision accounting, we need to limit
// the slash amount. This can be compensated for, by slashing with the main slash function if needed.
uint256 slashableStake = indexerStake.tokensStaked - indexerStake.tokensProvisioned;
if (slashableStake == 0) {
emit StakeSlashed(indexer, 0, 0, beneficiary);
return;
}
if (tokens > slashableStake) {
reward = (reward * slashableStake) / tokens;
tokens = slashableStake;
}

// Remove tokens to slash from the stake
indexerStake.tokensStaked = indexerStake.tokensStaked - tokens;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1602,8 +1602,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
);
if (isDelegationSlashingEnabled) {
uint256 poolThawingTokens = (before.pool.tokensThawing *
(before.pool.tokens - calcValues.delegationTokensSlashed)) /
before.pool.tokens;
(before.pool.tokens - calcValues.delegationTokensSlashed)) / before.pool.tokens;
assertEq(afterPool.tokens + calcValues.delegationTokensSlashed, before.pool.tokens);
assertEq(afterPool.shares, before.pool.shares);
assertEq(afterPool.tokensThawing, poolThawingTokens);
Expand Down
35 changes: 27 additions & 8 deletions packages/horizon/test/staking/slash/legacySlash.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,34 +45,53 @@ contract HorizonStakingLegacySlashTest is HorizonStakingTest {
// before
uint256 beforeStakingBalance = token.balanceOf(address(staking));
uint256 beforeRewardsDestinationBalance = token.balanceOf(_beneficiary);
ServiceProviderInternal memory beforeIndexer = _getStorage_ServiceProviderInternal(_indexer);

// calculate slashable stake
uint256 slashableStake = beforeIndexer.tokensStaked - beforeIndexer.tokensProvisioned;
uint256 actualTokens = _tokens;
uint256 actualRewards = _rewards;
if (slashableStake == 0) {
actualTokens = 0;
actualRewards = 0;
} else if (_tokens > slashableStake) {
actualRewards = (_rewards * slashableStake) / _tokens;
actualTokens = slashableStake;
}

// slash
vm.expectEmit(address(staking));
emit IHorizonStakingExtension.StakeSlashed(_indexer, _tokens, _rewards, _beneficiary);
emit IHorizonStakingExtension.StakeSlashed(_indexer, actualTokens, actualRewards, _beneficiary);
staking.slash(_indexer, _tokens, _rewards, _beneficiary);

// after
uint256 afterStakingBalance = token.balanceOf(address(staking));
uint256 afterRewardsDestinationBalance = token.balanceOf(_beneficiary);
ServiceProviderInternal memory afterIndexer = _getStorage_ServiceProviderInternal(_indexer);

assertEq(beforeStakingBalance - _tokens, afterStakingBalance);
assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - _rewards);
assertEq(beforeStakingBalance - actualTokens, afterStakingBalance);
assertEq(beforeRewardsDestinationBalance, afterRewardsDestinationBalance - actualRewards);
assertEq(afterIndexer.tokensStaked, beforeIndexer.tokensStaked - actualTokens);
}

/*
* TESTS
*/

function testSlash_Legacy(
uint256 tokens,
uint256 tokensStaked,
uint256 tokensProvisioned,
uint256 slashTokens,
uint256 reward
) public useIndexer useLegacySlasher(users.legacySlasher) {
vm.assume(tokens > 1);
slashTokens = bound(slashTokens, 1, tokens);
vm.assume(tokensStaked > 0);
vm.assume(tokensStaked <= MAX_STAKING_TOKENS);
vm.assume(tokensProvisioned > 0);
vm.assume(tokensProvisioned <= tokensStaked);
slashTokens = bound(slashTokens, 1, tokensStaked);
reward = bound(reward, 0, slashTokens);

_createProvision(users.indexer, subgraphDataServiceLegacyAddress, tokens, 0, 0);
_stake(tokensStaked);
_provision(users.indexer, subgraphDataServiceLegacyAddress, tokensProvisioned, 0, 0);

resetPrank(users.legacySlasher);
_legacySlash(users.indexer, slashTokens, reward, makeAddr("fisherman"));
Expand Down
4 changes: 1 addition & 3 deletions packages/horizon/test/staking/slash/slash.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {

function testSlash_RevertWhen_NoProvision(uint256 tokens, uint256 slashTokens) public useIndexer useStake(tokens) {
vm.assume(slashTokens > 0);
bytes memory expectedError = abi.encodeWithSelector(
IHorizonStakingMain.HorizonStakingNoTokensToSlash.selector
);
bytes memory expectedError = abi.encodeWithSelector(IHorizonStakingMain.HorizonStakingNoTokensToSlash.selector);
vm.expectRevert(expectedError);
vm.startPrank(subgraphDataServiceAddress);
staking.slash(users.indexer, slashTokens, 0, subgraphDataServiceAddress);
Expand Down
39 changes: 39 additions & 0 deletions packages/subgraph-service/contracts/DisputeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,45 @@ contract DisputeManager is
return (dId1, dId2);
}

/// @inheritdoc IDisputeManager
function createLegacyDispute(
address allocationId,
address fisherman,
uint256 tokensSlash,
uint256 tokensRewards
) external override onlyArbitrator returns (bytes32) {
// Create a disputeId
bytes32 disputeId = keccak256(abi.encodePacked(allocationId, "legacy"));

// Get the indexer for the legacy allocation
address indexer = _graphStaking().getAllocation(allocationId).indexer;
require(indexer != address(0), DisputeManagerIndexerNotFound(allocationId));

// Store dispute
disputes[disputeId] = Dispute(
indexer,
fisherman,
0,
0,
DisputeType.LegacyDispute,
IDisputeManager.DisputeStatus.Accepted,
block.timestamp,
0
);

// Slash the indexer
ISubgraphService subgraphService_ = _getSubgraphService();
subgraphService_.slash(indexer, abi.encode(tokensSlash, tokensRewards));

// Reward the fisherman
_graphToken().pushTokens(fisherman, tokensRewards);

emit LegacyDisputeCreated(disputeId, indexer, fisherman, allocationId, tokensSlash, tokensRewards);
emit DisputeAccepted(disputeId, indexer, fisherman, tokensRewards);

return disputeId;
}

/// @inheritdoc IDisputeManager
function acceptDispute(
bytes32 disputeId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ interface IDisputeManager {
enum DisputeType {
Null,
IndexingDispute,
QueryDispute
QueryDispute,
LegacyDispute
}

/// @notice Status of a dispute
Expand Down Expand Up @@ -130,6 +131,25 @@ interface IDisputeManager {
uint256 stakeSnapshot
);

/**
* @dev Emitted when a legacy dispute is created for `allocationId` and `fisherman`.
* The event emits the amount of `tokensSlash` to slash and `tokensRewards` to reward the fisherman.
* @param disputeId The dispute id
* @param indexer The indexer address
* @param fisherman The fisherman address to be credited with the rewards
* @param allocationId The allocation id
* @param tokensSlash The amount of tokens to slash
* @param tokensRewards The amount of tokens to reward the fisherman
*/
event LegacyDisputeCreated(
bytes32 indexed disputeId,
address indexed indexer,
address indexed fisherman,
address allocationId,
uint256 tokensSlash,
uint256 tokensRewards
);

/**
* @dev Emitted when arbitrator accepts a `disputeId` to `indexer` created by `fisherman`.
* The event emits the amount `tokens` transferred to the fisherman, the deposit plus reward.
Expand Down Expand Up @@ -436,6 +456,39 @@ interface IDisputeManager {
*/
function createIndexingDispute(address allocationId, bytes32 poi) external returns (bytes32);

/**
* @notice Create a legacy dispute.
* This disputes can be created to settle outstanding slashing amounts with an indexer that has been
* "legacy slashed" during or shortly after the transition period. See {HorizonStakingExtension.legacySlash}
* for more details.
*
* Note that this type of dispute:
* - can only be created by the arbitrator
* - does not require a bond
* - is automatically accepted when created
*
* Additionally, note that this type of disputes allow the arbitrator to directly set the slash and rewards
* amounts, bypassing the usual mechanisms that impose restrictions on those. This is done to give arbitrators
* maximum flexibility to ensure outstanding slashing amounts are settled fairly. This function needs to be removed
* after the transition period.
*
* Requirements:
* - Indexer must have been legacy slashed during or shortly after the transition period
* - Indexer must have provisioned funds to the Subgraph Service
*
* @param allocationId The allocation to dispute
* @param fisherman The fisherman address to be credited with the rewards
* @param tokensSlash The amount of tokens to slash
* @param tokensRewards The amount of tokens to reward the fisherman
* @return The dispute id
*/
function createLegacyDispute(
address allocationId,
address fisherman,
uint256 tokensSlash,
uint256 tokensRewards
) external returns (bytes32);

// -- Arbitrator --

/**
Expand Down
75 changes: 75 additions & 0 deletions packages/subgraph-service/test/disputeManager/DisputeManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,81 @@ contract DisputeManagerTest is SubgraphServiceSharedTest {
return _disputeID;
}

struct Balances {
uint256 indexer;
uint256 fisherman;
uint256 arbitrator;
uint256 disputeManager;
uint256 staking;
}

function _createLegacyDispute(
address _allocationId,
address _fisherman,
uint256 _tokensSlash,
uint256 _tokensRewards
) internal returns (bytes32) {
(, address arbitrator, ) = vm.readCallers();
address indexer = staking.getAllocation(_allocationId).indexer;

Balances memory beforeBalances = Balances({
indexer: token.balanceOf(indexer),
fisherman: token.balanceOf(_fisherman),
arbitrator: token.balanceOf(arbitrator),
disputeManager: token.balanceOf(address(disputeManager)),
staking: token.balanceOf(address(staking))
});

vm.expectEmit(address(disputeManager));
emit IDisputeManager.LegacyDisputeCreated(
keccak256(abi.encodePacked(_allocationId, "legacy")),
indexer,
_fisherman,
_allocationId,
_tokensSlash,
_tokensRewards
);
vm.expectEmit(address(disputeManager));
emit IDisputeManager.DisputeAccepted(
keccak256(abi.encodePacked(_allocationId, "legacy")),
indexer,
_fisherman,
_tokensRewards
);
bytes32 _disputeId = disputeManager.createLegacyDispute(
_allocationId,
_fisherman,
_tokensSlash,
_tokensRewards
);

Balances memory afterBalances = Balances({
indexer: token.balanceOf(indexer),
fisherman: token.balanceOf(_fisherman),
arbitrator: token.balanceOf(arbitrator),
disputeManager: token.balanceOf(address(disputeManager)),
staking: token.balanceOf(address(staking))
});

assertEq(afterBalances.indexer, beforeBalances.indexer);
assertEq(afterBalances.fisherman, beforeBalances.fisherman + _tokensRewards);
assertEq(afterBalances.arbitrator, beforeBalances.arbitrator);
assertEq(afterBalances.disputeManager, beforeBalances.disputeManager);
assertEq(afterBalances.staking, beforeBalances.staking - _tokensSlash);

IDisputeManager.Dispute memory dispute = _getDispute(_disputeId);
assertEq(dispute.indexer, indexer);
assertEq(dispute.fisherman, _fisherman);
assertEq(dispute.deposit, 0);
assertEq(dispute.relatedDisputeId, bytes32(0));
assertEq(uint8(dispute.disputeType), uint8(IDisputeManager.DisputeType.LegacyDispute));
assertEq(uint8(dispute.status), uint8(IDisputeManager.DisputeStatus.Accepted));
assertEq(dispute.createdAt, block.timestamp);
assertEq(dispute.stakeSnapshot, 0);

return _disputeId;
}

struct BeforeValues_CreateQueryDisputeConflict {
Attestation.State attestation1;
Attestation.State attestation2;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.27;

import "forge-std/Test.sol";

import { Attestation } from "../../../contracts/libraries/Attestation.sol";
import { PPMMath } from "@graphprotocol/horizon/contracts/libraries/PPMMath.sol";
import { IDisputeManager } from "../../../contracts/interfaces/IDisputeManager.sol";
import { DisputeManagerTest } from "../DisputeManager.t.sol";

contract DisputeManagerLegacyDisputeTest is DisputeManagerTest {
using PPMMath for uint256;

bytes32 private requestCID = keccak256(abi.encodePacked("Request CID"));
bytes32 private responseCID = keccak256(abi.encodePacked("Response CID"));
bytes32 private subgraphDeploymentId = keccak256(abi.encodePacked("Subgraph Deployment ID"));

/*
* TESTS
*/

function test_LegacyDispute(
uint256 tokensStaked,
uint256 tokensProvisioned,
uint256 tokensSlash,
uint256 tokensRewards
) public {
vm.assume(tokensStaked <= MAX_TOKENS);
vm.assume(tokensStaked >= minimumProvisionTokens);
tokensProvisioned = bound(tokensProvisioned, minimumProvisionTokens, tokensStaked);
tokensSlash = bound(tokensSlash, 2, tokensProvisioned);
tokensRewards = bound(tokensRewards, 1, tokensSlash.mulPPM(maxSlashingPercentage));

// setup indexer state
resetPrank(users.indexer);
_stake(tokensStaked);
_setStorage_allocation_hardcoded(users.indexer, allocationID, tokensStaked - tokensProvisioned);
_provision(users.indexer, tokensProvisioned, maxSlashingPercentage, disputePeriod);

resetPrank(users.arbitrator);
_createLegacyDispute(allocationID, users.fisherman, tokensSlash, tokensRewards);
}

function test_LegacyDispute_RevertIf_NotArbitrator() public useIndexer {
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerNotArbitrator.selector));
disputeManager.createLegacyDispute(allocationID, users.fisherman, 0, 0);
}

function test_LegacyDispute_RevertIf_AllocationNotFound() public useIndexer {
resetPrank(users.arbitrator);
vm.expectRevert(abi.encodeWithSelector(IDisputeManager.DisputeManagerIndexerNotFound.selector, address(0)));
disputeManager.createLegacyDispute(address(0), users.fisherman, 0, 0);
}
}
Loading