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

Signed-off-by: Tomás Migone <[email protected]>

Author: tmigone

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

@@ -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.

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

@@ -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

packages/horizon/contracts/staking/HorizonStakingExtension.sol

@@ -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;
 

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

@@ -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);

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

@@ -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,
+    function testSlash_LegacyOnly(
+        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"));

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

@@ -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);

packages/subgraph-service/contracts/DisputeManager.sol

@@ -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,

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

@@ -15,7 +15,8 @@ interface IDisputeManager {
     enum DisputeType {
         Null,
         IndexingDispute,
-        QueryDispute
+        QueryDispute,
+        LegacyDispute
     }
 
     /// @notice Status of a dispute
@@ -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.
@@ -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 --
 
     /**

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

@@ -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;

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

@@ -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);
+    }
+}