fix: missing validations (OZ L-07)

packages/horizon/contracts/interfaces/IGraphPayments.sol

@@ -39,12 +39,18 @@ interface IGraphPayments {
     );
 
     /**
-     * @notice Emitted when there are insufficient tokens to pay the required amount
+     * @notice Thrown when there are insufficient tokens to pay the required amount
      * @param tokens The amount of tokens available
      * @param minTokens The amount of tokens being collected
      */
     error GraphPaymentsInsufficientTokens(uint256 tokens, uint256 minTokens);
 
+    /**
+     * @notice Thrown when the protocol payment cut is invalid
+     * @param protocolPaymentCut The protocol payment cut
+     */
+    error GraphPaymentsInvalidProtocolPaymentCut(uint256 protocolPaymentCut);
+
     /**
      * @notice Collects funds from a payer.
      * It will pay cuts to all relevant parties and forward the rest to the receiver.

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

@@ -427,6 +427,12 @@ interface IHorizonStakingMain {
      */
     error HorizonStakingCallerIsServiceProvider();
 
+    /**
+     * @notice Thrown when trying to set a delegation fee cut that is not valid.
+     * @param feeCut The fee cut
+     */
+    error HorizonStakingInvalidDelegationFeeCut(uint256 feeCut);
+
     // -- Functions --
 
     /**
@@ -839,11 +845,10 @@ interface IHorizonStakingMain {
     function setAllowedLockedVerifier(address verifier, bool allowed) external;
 
     /**
-     * @notice Set the global delegation slashing flag.
+     * @notice Set the global delegation slashing flag to true.
      * @dev This function can only be called by the contract governor.
-     * @param enabled Whether delegation slashing is enabled or disabled.
      */
-    function setDelegationSlashingEnabled(bool enabled) external;
+    function setDelegationSlashingEnabled() external;
 
     /**
      * @notice Clear the legacy global thawing period.

packages/horizon/contracts/libraries/LinkedList.sol

@@ -53,6 +53,11 @@ library LinkedList {
      */
     error LinkedListInvalidIterations();
 
+    /**
+     * @notice Thrown when trying to add an item with id equal to bytes32(0)
+     */
+    error LinkedListInvalidZeroId();
+
     /**
      * @notice Adds an item to the list.
      * The item is added to the end of the list.
@@ -65,6 +70,7 @@ library LinkedList {
      */
     function addTail(List storage self, bytes32 id) internal {
         require(self.count < MAX_ITEMS, LinkedListMaxElementsExceeded());
+        require(id != bytes32(0), LinkedListInvalidZeroId());
         self.tail = id;
         self.nonce += 1;
         if (self.count == 0) self.head = id;

packages/horizon/contracts/payments/GraphPayments.sol

@@ -30,6 +30,7 @@ contract GraphPayments is Initializable, MulticallUpgradeable, GraphDirectory, I
      * @param protocolPaymentCut The protocol tax in PPM
      */
     constructor(address controller, uint256 protocolPaymentCut) GraphDirectory(controller) {
+        require(PPMMath.isValidPPM(protocolPaymentCut), GraphPaymentsInvalidProtocolPaymentCut(protocolPaymentCut));
         PROTOCOL_PAYMENT_CUT = protocolPaymentCut;
         _disableInitializers();
     }

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -318,6 +318,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         IGraphPayments.PaymentTypes paymentType,
         uint256 feeCut
     ) external override notPaused onlyAuthorized(serviceProvider, verifier) {
+        require(PPMMath.isValidPPM(feeCut), HorizonStakingInvalidDelegationFeeCut(feeCut));
         _delegationFeeCut[serviceProvider][verifier][paymentType] = feeCut;
         emit DelegationFeeCutSet(serviceProvider, verifier, paymentType, feeCut);
     }
@@ -468,9 +469,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
     /**
      * @notice See {IHorizonStakingMain-setDelegationSlashingEnabled}.
      */
-    function setDelegationSlashingEnabled(bool enabled) external override onlyGovernor {
-        _delegationSlashingEnabled = enabled;
-        emit DelegationSlashingEnabled(enabled);
+    function setDelegationSlashingEnabled() external override onlyGovernor {
+        _delegationSlashingEnabled = true;
+        emit DelegationSlashingEnabled(_delegationSlashingEnabled);
     }
 
     /**

packages/horizon/contracts/staking/HorizonStakingExtension.sol

@@ -346,6 +346,7 @@ contract HorizonStakingExtension is HorizonStakingBase, IL2StakingBase, IHorizon
         uint256 _tokens,
         IL2StakingTypes.ReceiveDelegationData memory _delegationData
     ) internal {
+        require(_provisions[_delegationData.indexer][SUBGRAPH_DATA_SERVICE_ADDRESS].createdAt != 0, "!provision");
         // Get the delegation pool of the indexer
         DelegationPoolInternal storage pool = _legacyDelegationPools[_delegationData.indexer];
         IHorizonStakingTypes.DelegationInternal storage delegation = pool.delegators[_delegationData.delegator];

packages/horizon/test/libraries/LinkedList.t.sol

@@ -14,6 +14,11 @@ contract LinkedListTest is Test, ListImplementation {
         list = LinkedList.List({head: bytes32(0), tail: bytes32(0), nonce: 0, count: 0});
     }
 
+    function test_Add_RevertGiven_TheItemIdIsZero() external {
+        vm.expectRevert(LinkedList.LinkedListInvalidZeroId.selector);
+        list.addTail(bytes32(0));
+    }
+
     function test_Add_GivenTheListIsEmpty() external {
         _assert_addItem(_buildItemId(list.nonce), 0);
     }

packages/horizon/test/payments/GraphPayments.t.sol

@@ -4,6 +4,7 @@ pragma solidity 0.8.26;
 import "forge-std/Test.sol";
 
 import { IGraphPayments } from "../../contracts/interfaces/IGraphPayments.sol";
+import { GraphPayments } from "../../contracts/payments/GraphPayments.sol";
 
 import { HorizonStakingSharedTest } from "../shared/horizon-staking/HorizonStakingShared.t.sol";
 
@@ -13,6 +14,18 @@ contract GraphPaymentsTest is HorizonStakingSharedTest {
      * TESTS
      */
 
+    function testConstructor_RevertIf_InvalidProtocolPaymentCut(uint256 protocolPaymentCut) public {
+        protocolPaymentCut = bound(protocolPaymentCut, MAX_PPM + 1, MAX_PPM + 100);
+
+        resetPrank(users.deployer);
+        bytes memory expectedError = abi.encodeWithSelector(
+            IGraphPayments.GraphPaymentsInvalidProtocolPaymentCut.selector,
+            protocolPaymentCut
+        );
+        vm.expectRevert(expectedError);
+        new GraphPayments(address(controller), protocolPaymentCut);
+    }
+
     function testCollect(
         uint256 amount,
         uint256 tokensDataService

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

@@ -77,5 +77,7 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
 
     function _setDelegationFeeCut(IGraphPayments.PaymentTypes paymentType, uint256 cut) internal {
         staking.setDelegationFeeCut(users.indexer, subgraphDataServiceAddress, paymentType, cut);
+        uint256 delegationFeeCut = staking.getDelegationFeeCut(users.indexer, subgraphDataServiceAddress, paymentType);
+        assertEq(delegationFeeCut, cut);
     }
 }

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

@@ -80,11 +80,11 @@ contract HorizonStakingTest is HorizonStakingSharedTest, IHorizonStakingTypes {
         _;
     }
 
-    modifier useDelegationSlashing(bool enabled) {
+    modifier useDelegationSlashing() {
         address msgSender;
         (, msgSender, ) = vm.readCallers();
         resetPrank(users.governor);
-        staking.setDelegationSlashingEnabled(enabled);
+        staking.setDelegationSlashingEnabled();
         resetPrank(msgSender);
         _;
     }

packages/horizon/test/staking/delegation/delegate.t.sol

@@ -87,7 +87,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
     function testDelegate_RevertWhen_InvalidPool(
         uint256 tokens,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing(true) {
+    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         resetPrank(users.delegator);
         _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);
@@ -108,7 +108,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
     function testDelegate_RevertWhen_ThawingShares_InvalidPool(
         uint256 tokens,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing(true) {
+    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         resetPrank(users.delegator);
         _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

packages/horizon/test/staking/delegation/undelegate.t.sol

@@ -116,7 +116,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
     function testUndelegate_RevertWhen_InvalidPool(
         uint256 tokens,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing(true) {
+    ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         resetPrank(users.delegator);
         _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

packages/horizon/test/staking/delegation/withdraw.t.sol

@@ -180,7 +180,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
     function testWithdrawDelegation_RevertWhen_InvalidPool(
         uint256 tokens,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing(true) {
+    ) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing() {
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         resetPrank(users.delegator);
         _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

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

@@ -31,15 +31,15 @@ contract HorizonStakingGovernanceTest is HorizonStakingTest {
         staking.setAllowedLockedVerifier(subgraphDataServiceAddress, true);
     }
 
-    function testGovernance_SetDelgationSlashing(bool enabled) public useGovernor {
-        staking.setDelegationSlashingEnabled(enabled);
-        assertEq(staking.isDelegationSlashingEnabled(), enabled);
+    function testGovernance_SetDelgationSlashingEnabled() public useGovernor {
+        staking.setDelegationSlashingEnabled();
+        assertTrue(staking.isDelegationSlashingEnabled());
     }
 
-    function testGovernance_SetDelgationSlashing_NotGovernor(bool enabled) public useIndexer {
+    function testGovernance_SetDelgationSlashing_NotGovernor() public useIndexer {
         bytes memory expectedError = abi.encodeWithSignature("ManagedOnlyGovernor()");
         vm.expectRevert(expectedError);
-        staking.setDelegationSlashingEnabled(enabled);
+        staking.setDelegationSlashingEnabled();
     }
 
     function testGovernance_ClearThawingPeriod(uint32 thawingPeriod) public useGovernor {

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

@@ -3,9 +3,11 @@ pragma solidity 0.8.26;
 
 import "forge-std/Test.sol";
 
-import { HorizonStakingTest } from "../HorizonStaking.t.sol";
+import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol";
 import { IGraphPayments } from "../../../contracts/interfaces/IGraphPayments.sol";
 
+import { HorizonStakingTest } from "../HorizonStaking.t.sol";
+
 contract HorizonStakingServiceProviderTest is HorizonStakingTest {
 
     /*
@@ -31,21 +33,14 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest {
         assertEq(sp.tokensProvisioned, amount);
     }
 
-    function testServiceProvider_GetDelegationFeeCut(
-        uint256 queryCut,
-        uint256 indexingCut,
-        uint256 rewardsCut
+    function testServiceProvider_SetDelegationFeeCut(
+        uint256 feeCut,
+        uint8 paymentTypeInput
     ) public useIndexer {
-        _setDelegationFeeCut(IGraphPayments.PaymentTypes.QueryFee, queryCut);
-        _setDelegationFeeCut(IGraphPayments.PaymentTypes.IndexingFee, indexingCut);
-        _setDelegationFeeCut(IGraphPayments.PaymentTypes.IndexingRewards, rewardsCut);
-
-        uint256 queryFeeCut = staking.getDelegationFeeCut(users.indexer, subgraphDataServiceAddress, IGraphPayments.PaymentTypes.QueryFee);
-        uint256 indexingFeeCut = staking.getDelegationFeeCut(users.indexer, subgraphDataServiceAddress, IGraphPayments.PaymentTypes.IndexingFee);
-        uint256 indexingRewardsCut = staking.getDelegationFeeCut(users.indexer, subgraphDataServiceAddress, IGraphPayments.PaymentTypes.IndexingRewards);
-        assertEq(queryFeeCut, queryCut);
-        assertEq(indexingFeeCut, indexingCut);
-        assertEq(indexingRewardsCut, rewardsCut);
+        vm.assume(paymentTypeInput < 3);
+        IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes(paymentTypeInput);
+        feeCut = bound(feeCut, 0, MAX_PPM);
+        _setDelegationFeeCut(paymentType, feeCut);
     }
 
     function testServiceProvider_GetProvision(
@@ -136,4 +131,18 @@ contract HorizonStakingServiceProviderTest is HorizonStakingTest {
         staking.unstake(amount);
         assertEq(staking.getIndexerStakedTokens(users.indexer), 0);
     }
+
+    function testServiceProvider_RevertIf_InvalidDelegationFeeCut(
+        uint256 cut,
+        uint8 paymentTypeInput
+    ) public useIndexer {
+        vm.assume(paymentTypeInput < 3);
+        IGraphPayments.PaymentTypes paymentType = IGraphPayments.PaymentTypes(paymentTypeInput);
+        cut = bound(cut, MAX_PPM + 1, MAX_PPM + 100);
+        vm.expectRevert(abi.encodeWithSelector(
+            IHorizonStakingMain.HorizonStakingInvalidDelegationFeeCut.selector,
+            cut
+        ));
+        staking.setDelegationFeeCut(users.indexer, subgraphDataServiceAddress, paymentType, cut);
+    }
 }

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

@@ -61,7 +61,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         uint256 slashTokens,
         uint256 verifierCutAmount,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) useDelegationSlashing(false) {
+    ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) {
         vm.assume(slashTokens > tokens);
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);
@@ -80,7 +80,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
         uint256 slashTokens,
         uint256 verifierCutAmount,
         uint256 delegationTokens
-    ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) useDelegationSlashing(true) {
+    ) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) useDelegationSlashing() {
         delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
         slashTokens = bound(slashTokens, tokens + 1, tokens + delegationTokens);
         verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);

packages/horizon/test/staking/transfer-tools/ttools.t.sol

@@ -44,6 +44,18 @@ contract HorizonStakingTransferToolsTest is HorizonStakingTest {
         staking.onTokenTransfer(counterpartStaking, 0, data);
     }
 
+    function testOnTransfer_RevertWhen_ProvisionNotFound(uint256 amount) public {
+        amount = bound(amount, 1 ether, MAX_STAKING_TOKENS);
+
+        resetPrank(graphTokenGatewayAddress);
+        bytes memory data = abi.encode(
+            uint8(IL2StakingTypes.L1MessageCodes.RECEIVE_DELEGATION_CODE),
+            abi.encode(users.indexer, users.delegator)
+        );
+        vm.expectRevert(bytes("!provision"));
+        staking.onTokenTransfer(counterpartStaking, amount, data);
+    }
+
     function testOnTransfer_ReceiveDelegation_RevertWhen_InvalidData() public {
         resetPrank(graphTokenGatewayAddress);
 
@@ -101,7 +113,7 @@ contract HorizonStakingTransferToolsTest is HorizonStakingTest {
         _onTokenTransfer_ReceiveDelegation(counterpartStaking, amount, data);
     }
 
-    function testOnTransfer_ReceiveDelegation_WhenInvalidPool(uint256 amount) public useDelegationSlashing(true) {
+    function testOnTransfer_ReceiveDelegation_WhenInvalidPool(uint256 amount) public useDelegationSlashing() {
         amount = bound(amount, 1 ether, MAX_STAKING_TOKENS);
         uint256 originalDelegationAmount = 10 ether;
         uint256 provisionSize = 100 ether;
@@ -196,7 +208,7 @@ contract HorizonStakingTransferToolsTest is HorizonStakingTest {
             vm.expectEmit();
             emit IHorizonStakingExtension.StakeDelegated(serviceProvider, delegator, tokens, calcShares);
         }
-        staking.onTokenTransfer(counterpartStaking, tokens, data);
+        staking.onTokenTransfer(from, tokens, data);
 
         // after
         DelegationPool memory afterPool = staking.getDelegationPool(serviceProvider, subgraphDataServiceLegacyAddress);

packages/subgraph-service/contracts/SubgraphService.sol

@@ -126,6 +126,7 @@ contract SubgraphService is
         );
 
         require(bytes(url).length > 0, SubgraphServiceEmptyUrl());
+        require(bytes(geohash).length > 0, SubgraphServiceEmptyGeohash());
         require(indexers[indexer].registeredAt == 0, SubgraphServiceIndexerAlreadyRegistered());
 
         // Register the indexer

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

@@ -57,6 +57,11 @@ interface ISubgraphService is IDataServiceFees {
      */
     error SubgraphServiceEmptyUrl();
 
+    /**
+     * @notice Thrown when an indexer tries to register with an empty geohash
+     */
+    error SubgraphServiceEmptyGeohash();
+
     /**
      * @notice Thrown when an indexer tries to register but they are already registered
      */

packages/subgraph-service/test/subgraphService/provider/register.t.sol

@@ -72,4 +72,20 @@ contract SubgraphServiceRegisterTest is SubgraphServiceTest {
         ));
         _registerIndexer(address(0));
     }
+
+    function testRegister_RevertIf_EmptyUrl(uint256 tokens) public useIndexer {
+        tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
+        _createProvision(tokens);
+        bytes memory data = abi.encode("", "geoHash", users.rewardsDestination);
+        vm.expectRevert(abi.encodeWithSelector(ISubgraphService.SubgraphServiceEmptyUrl.selector));
+        subgraphService.register(users.indexer, data);
+    }
+
+    function testRegister_RevertIf_EmptyGeohash(uint256 tokens) public useIndexer {
+        tokens = bound(tokens, minimumProvisionTokens, MAX_TOKENS);
+        _createProvision(tokens);
+        bytes memory data = abi.encode("url", "", users.rewardsDestination);
+        vm.expectRevert(abi.encodeWithSelector(ISubgraphService.SubgraphServiceEmptyGeohash.selector));
+        subgraphService.register(users.indexer, data);
+    }
 }