f: remove linter warnings

Author: matiasedgeandnode

IndexingPaymentsTodo.md

@@ -1,6 +1,5 @@
 # Still pending
 
-* Remove linter warnings
 * Remove extension if I can fit everything in one service?
 * `require(provision.tokens != 0, DisputeManagerZeroTokens());` - Document or fix?
 * Check code coverage

packages/horizon/contracts/data-service/libraries/DataServiceFeesLib.sol

@@ -10,20 +10,12 @@ library DataServiceFeesLib {
     using ProvisionTracker for mapping(address => uint256);
     using LinkedList for LinkedList.List;
 
-    /**
-     * @notice Builds a stake claim ID
-     * @param serviceProvider The address of the service provider
-     * @param nonce A nonce of the stake claim
-     * @return The stake claim ID
-     */
-    function _buildStakeClaimId(address serviceProvider, uint256 nonce) internal view returns (bytes32) {
-        return keccak256(abi.encodePacked(address(this), serviceProvider, nonce));
-    }
-
+    // @notice Storage structure for the data service fees library
     struct Storage {
         ProvisionManagerStorage provisionManagerStorage;
     }
 
+    // @notice Storage structure for the provision manager
     struct ProvisionManagerStorage {
         uint256 _minimumProvisionTokens;
         uint256 _maximumProvisionTokens;
@@ -75,6 +67,12 @@ library DataServiceFeesLib {
         emit IDataServiceFees.StakeClaimLocked(_serviceProvider, claimId, _tokens, _unlockTimestamp);
     }
 
+    /**
+     * @notice Processes a stake claim, releasing the tokens if the claim has expired.
+     * @dev This function is used as a callback in the stake claims linked list traversal.
+     * @return Whether the stake claim is still locked, indicating that the traversal should continue or stop.
+     * @return The updated accumulator data
+     */
     function processStakeClaim(
         mapping(address serviceProvider => uint256 tokens) storage feesProvisionTracker,
         mapping(bytes32 claimId => IDataServiceFees.StakeClaim claim) storage claims,
@@ -100,4 +98,14 @@ library DataServiceFeesLib {
         _acc = abi.encode(tokensClaimed + claim.tokens, serviceProvider);
         return (false, _acc);
     }
+
+    /**
+     * @notice Builds a stake claim ID
+     * @param serviceProvider The address of the service provider
+     * @param nonce A nonce of the stake claim
+     * @return The stake claim ID
+     */
+    function _buildStakeClaimId(address serviceProvider, uint256 nonce) internal view returns (bytes32) {
+        return keccak256(abi.encodePacked(address(this), serviceProvider, nonce));
+    }
 }

packages/horizon/contracts/data-service/libraries/ProvisionTracker.sol

@@ -21,6 +21,24 @@ library ProvisionTracker {
      */
     error ProvisionTrackerInsufficientTokens(uint256 tokensAvailable, uint256 tokensRequired);
 
+    /**
+     * @notice Checks if a service provider has enough tokens available to lock
+     * @param self The provision tracker mapping
+     * @param graphStaking The HorizonStaking contract
+     * @param serviceProvider The service provider address
+     * @param delegationRatio A delegation ratio to limit the amount of delegation that's usable
+     * @return true if the service provider has enough tokens available to lock, false otherwise
+     */
+    function check(
+        mapping(address => uint256) storage self,
+        IHorizonStaking graphStaking,
+        address serviceProvider,
+        uint32 delegationRatio
+    ) external view returns (bool) {
+        uint256 tokensAvailable = graphStaking.getTokensAvailable(serviceProvider, address(this), delegationRatio);
+        return self[serviceProvider] <= tokensAvailable;
+    }
+
     /**
      * @notice Locks tokens for a service provider
      * @dev Requirements:
@@ -59,22 +77,4 @@ library ProvisionTracker {
         require(self[serviceProvider] >= tokens, ProvisionTrackerInsufficientTokens(self[serviceProvider], tokens));
         self[serviceProvider] -= tokens;
     }
-
-    /**
-     * @notice Checks if a service provider has enough tokens available to lock
-     * @param self The provision tracker mapping
-     * @param graphStaking The HorizonStaking contract
-     * @param serviceProvider The service provider address
-     * @param delegationRatio A delegation ratio to limit the amount of delegation that's usable
-     * @return true if the service provider has enough tokens available to lock, false otherwise
-     */
-    function check(
-        mapping(address => uint256) storage self,
-        IHorizonStaking graphStaking,
-        address serviceProvider,
-        uint32 delegationRatio
-    ) external view returns (bool) {
-        uint256 tokensAvailable = graphStaking.getTokensAvailable(serviceProvider, address(this), delegationRatio);
-        return self[serviceProvider] <= tokensAvailable;
-    }
 }

packages/subgraph-service/contracts/SubgraphService.sol

@@ -60,10 +60,6 @@ contract SubgraphService is
         _;
     }
 
-    function requireRegisteredIndexer(address indexer) external view {
-        require(indexers[indexer].registeredAt != 0, SubgraphServiceIndexerNotRegistered(indexer));
-    }
-
     /**
      * @notice Constructor for the SubgraphService contract
      * @dev DataService and Directory constructors set a bunch of immutable variables
@@ -104,6 +100,35 @@ contract SubgraphService is
         _setStakeToFeesRatio(stakeToFeesRatio_);
     }
 
+    /**
+     * @notice Delegates the call to the SubgraphServiceExtension implementation.
+     * @dev This function does not return to its internal call site, it will return directly to the
+     * external caller.
+     */
+    // solhint-disable-next-line payable-fallback, no-complex-fallback
+    fallback() external {
+        address extImpl = _subgraphServiceExtensionImpl();
+        require(extImpl != address(0), "only through proxy");
+
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            // copy function selector and any arguments
+            calldatacopy(0, 0, calldatasize())
+            // execute function call using the extension implementation
+            let result := delegatecall(gas(), extImpl, 0, calldatasize(), 0, 0)
+            // get any return value
+            returndatacopy(0, 0, returndatasize())
+            // return any return value or error back to the caller
+            switch result
+            case 0 {
+                revert(0, returndatasize())
+            }
+            default {
+                return(0, returndatasize())
+            }
+        }
+    }
+
     /**
      * @notice
      * @dev Implements {IDataService.register}
@@ -393,6 +418,44 @@ contract SubgraphService is
         emit CurationCutSet(curationCut);
     }
 
+    /**
+     * @notice Accept an indexing agreement.
+     * See {ISubgraphService.acceptIndexingAgreement}.
+     *
+     * Requirements:
+     * - The agreement's indexer must be registered
+     * - The caller must be authorized by the agreement's indexer
+     * - The provision must be valid according to the subgraph service rules
+     * - Allocation must belong to the indexer and be open
+     * - Agreement must be for this data service
+     * - Agreement's subgraph deployment must match the allocation's subgraph deployment
+     * - Agreement must not have been accepted before
+     * - Allocation must not have an agreement already
+     *
+     * @dev signedRCA.rca.metadata is an encoding of {IndexingAgreement.AcceptIndexingAgreementMetadata}
+     *
+     * Emits {IndexingAgreementAccepted} event
+     *
+     * @param allocationId The id of the allocation
+     * @param signedRCA The signed Recurring Collection Agreement
+     */
+    function acceptIndexingAgreement(
+        address allocationId,
+        IRecurringCollector.SignedRCA calldata signedRCA
+    )
+        external
+        whenNotPaused
+        onlyAuthorizedForProvision(signedRCA.rca.serviceProvider)
+        onlyValidProvision(signedRCA.rca.serviceProvider)
+        onlyRegisteredIndexer(signedRCA.rca.serviceProvider)
+    {
+        IndexingAgreement._getManager().accept(_allocations, allocationId, signedRCA);
+    }
+
+    function requireRegisteredIndexer(address indexer) external view {
+        require(indexers[indexer].registeredAt != 0, SubgraphServiceIndexerNotRegistered(indexer));
+    }
+
     /// @inheritdoc ISubgraphService
     function getAllocation(address allocationId) external view override returns (Allocation.State memory) {
         return _allocations[allocationId];
@@ -448,6 +511,14 @@ contract SubgraphService is
         return _isOverAllocated(indexer, _delegationRatio);
     }
 
+    function getGraphStaking() external view returns (address) {
+        return address(_graphStaking());
+    }
+
+    function _cancelAllocationIndexingAgreement(address _allocationId) internal {
+        IndexingAgreement._getManager().cancelForAllocation(_allocationId);
+    }
+
     /**
      * @notice Sets the payments destination for an indexer to receive payments
      * @dev Emits a {PaymentsDestinationSet} event
@@ -601,68 +672,6 @@ contract SubgraphService is
         return _presentPOI(allocationId, poi_, poiMetadata_, _delegationRatio, paymentsDestination[_indexer]);
     }
 
-    /**
-     * @notice Set the stake to fees ratio.
-     * @param _stakeToFeesRatio The stake to fees ratio
-     */
-    function _setStakeToFeesRatio(uint256 _stakeToFeesRatio) private {
-        require(_stakeToFeesRatio != 0, SubgraphServiceInvalidZeroStakeToFeesRatio());
-        stakeToFeesRatio = _stakeToFeesRatio;
-        emit StakeToFeesRatioSet(_stakeToFeesRatio);
-    }
-
-    /**
-     * @notice Encodes the data for the GraphTallyCollector
-     * @dev The purpose of this function is just to avoid stack too deep errors
-     * @param _signedRav The signed RAV
-     * @param _curationCut The curation cut
-     * @return The encoded data
-     */
-    function _encodeGraphTallyData(
-        IGraphTallyCollector.SignedRAV memory _signedRav,
-        uint256 _curationCut
-    ) private view returns (bytes memory) {
-        return abi.encode(_signedRav, _curationCut, paymentsDestination[_signedRav.rav.serviceProvider]);
-    }
-
-    function _cancelAllocationIndexingAgreement(address _allocationId) internal {
-        IndexingAgreement._getManager().cancelForAllocation(_allocationId);
-    }
-
-    /**
-     * @notice Accept an indexing agreement.
-     * See {ISubgraphService.acceptIndexingAgreement}.
-     *
-     * Requirements:
-     * - The agreement's indexer must be registered
-     * - The caller must be authorized by the agreement's indexer
-     * - The provision must be valid according to the subgraph service rules
-     * - Allocation must belong to the indexer and be open
-     * - Agreement must be for this data service
-     * - Agreement's subgraph deployment must match the allocation's subgraph deployment
-     * - Agreement must not have been accepted before
-     * - Allocation must not have an agreement already
-     *
-     * @dev signedRCA.rca.metadata is an encoding of {IndexingAgreement.AcceptIndexingAgreementMetadata}
-     *
-     * Emits {IndexingAgreementAccepted} event
-     *
-     * @param allocationId The id of the allocation
-     * @param signedRCA The signed Recurring Collection Agreement
-     */
-    function acceptIndexingAgreement(
-        address allocationId,
-        IRecurringCollector.SignedRCA calldata signedRCA
-    )
-        external
-        whenNotPaused
-        onlyAuthorizedForProvision(signedRCA.rca.serviceProvider)
-        onlyValidProvision(signedRCA.rca.serviceProvider)
-        onlyRegisteredIndexer(signedRCA.rca.serviceProvider)
-    {
-        IndexingAgreement._getManager().accept(_allocations, allocationId, signedRCA);
-    }
-
     /**
      * @notice Collect Indexing fees
      * Stake equal to the amount being collected times the `stakeToFeesRatio` is locked into a stake claim.
@@ -702,6 +711,16 @@ contract SubgraphService is
         return tokensCollected;
     }
 
+    /**
+     * @notice Set the stake to fees ratio.
+     * @param _stakeToFeesRatio The stake to fees ratio
+     */
+    function _setStakeToFeesRatio(uint256 _stakeToFeesRatio) private {
+        require(_stakeToFeesRatio != 0, SubgraphServiceInvalidZeroStakeToFeesRatio());
+        stakeToFeesRatio = _stakeToFeesRatio;
+        emit StakeToFeesRatioSet(_stakeToFeesRatio);
+    }
+
     function _releaseAndLockStake(address _indexer, uint256 _tokensCollected) private {
         _releaseStake(_indexer, 0);
         if (_tokensCollected > 0) {
@@ -714,36 +733,17 @@ contract SubgraphService is
         }
     }
 
-    function getGraphStaking() external view returns (address) {
-        return address(_graphStaking());
-    }
-
     /**
-     * @notice Delegates the call to the SubgraphServiceExtension implementation.
-     * @dev This function does not return to its internal call site, it will return directly to the
-     * external caller.
+     * @notice Encodes the data for the GraphTallyCollector
+     * @dev The purpose of this function is just to avoid stack too deep errors
+     * @param _signedRav The signed RAV
+     * @param _curationCut The curation cut
+     * @return The encoded data
      */
-    // solhint-disable-next-line payable-fallback, no-complex-fallback
-    fallback() external {
-        address extImpl = _subgraphServiceExtensionImpl();
-        require(extImpl != address(0), "only through proxy");
-
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            // copy function selector and any arguments
-            calldatacopy(0, 0, calldatasize())
-            // execute function call using the extension implementation
-            let result := delegatecall(gas(), extImpl, 0, calldatasize(), 0, 0)
-            // get any return value
-            returndatacopy(0, 0, returndatasize())
-            // return any return value or error back to the caller
-            switch result
-            case 0 {
-                revert(0, returndatasize())
-            }
-            default {
-                return(0, returndatasize())
-            }
-        }
+    function _encodeGraphTallyData(
+        IGraphTallyCollector.SignedRAV memory _signedRav,
+        uint256 _curationCut
+    ) private view returns (bytes memory) {
+        return abi.encode(_signedRav, _curationCut, paymentsDestination[_signedRav.rav.serviceProvider]);
     }
 }

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

@@ -259,6 +259,14 @@ interface ISubgraphService is IDataServiceFees {
      */
     function setPaymentsDestination(address paymentsDestination) external;
 
+    /**
+     * @notice Accept an indexing agreement.
+     * @dev Emits a {IndexingAgreement.IndexingAgreementAccepted} event
+     * @param allocationId The id of the allocation
+     * @param signedRCA The signed recurring collector agreement (RCA) that the indexer accepts
+     */
+    function acceptIndexingAgreement(address allocationId, IRecurringCollector.SignedRCA calldata signedRCA) external;
+
     /**
      * @notice Gets the details of an allocation
      * For legacy allocations use {getLegacyAllocation}
@@ -307,9 +315,4 @@ interface ISubgraphService is IDataServiceFees {
      * @return The address of the curation contract
      */
     function getCuration() external view returns (address);
-
-    /**
-     * @notice Accept an indexing agreement.
-     */
-    function acceptIndexingAgreement(address allocationId, IRecurringCollector.SignedRCA calldata signedRCA) external;
 }