fix: resolve solhint warnings in RecurringCollector and fix subgraph-service coverage

Maikol · Maikol · commit 639df67402f0 · 2026-02-10T15:32:55.000-03:00
- Revert strict inequality conversions (> x-1, < x+1) back to (>=, <=) and suppress gas-strict-inequalities with solhint comments instead - Suppress gas-small-strings, function-max-lines solhint warnings - Add --ir-minimum to subgraph-service forge coverage to fix stack too deep - Change getCollectionInfo param from memory to calldata - Add @author natspec tag
diff --git a/packages/horizon/contracts/payments/collectors/RecurringCollector.sol b/packages/horizon/contracts/payments/collectors/RecurringCollector.sol
@@ -15,6 +15,7 @@ import { PPMMath } from "../../libraries/PPMMath.sol";
 
 /**
  * @title RecurringCollector contract
+ * @author Edge & Node
  * @dev Implements the {IRecurringCollector} interface.
  * @notice A payments collector contract that can be used to collect payments using a RCA (Recurring Collection Agreement).
  * @custom:security-contact Please email security+contracts@thegraph.com if you find any
@@ -26,6 +27,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
     /// @notice The minimum number of seconds that must be between two collections
     uint32 public constant MIN_SECONDS_COLLECTION_WINDOW = 600;
 
+    /* solhint-disable gas-small-strings */
     /// @notice The EIP712 typehash for the RecurringCollectionAgreement struct
     bytes32 public constant EIP712_RCA_TYPEHASH =
         keccak256(
@@ -37,6 +39,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
         keccak256(
             "RecurringCollectionAgreementUpdate(bytes16 agreementId,uint64 deadline,uint64 endsAt,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,uint32 nonce,bytes metadata)"
         );
+    /* solhint-enable gas-small-strings */
 
     /// @notice Tracks agreements
     mapping(bytes16 agreementId => AgreementData data) public agreements;
@@ -75,7 +78,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
      * See {IRecurringCollector.accept}.
      * @dev Caller must be the data service the RCA was issued to.
      */
-    function accept(SignedRCA calldata signedRCA) external returns (bytes16) {
+    function accept(SignedRCA calldata signedRCA) external returns (bytes16) { // solhint-disable-line function-max-lines
         bytes16 agreementId = _generateAgreementId(
             signedRCA.rca.payer,
             signedRCA.rca.dataService,
@@ -89,10 +92,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
             msg.sender == signedRCA.rca.dataService,
             RecurringCollectorUnauthorizedCaller(msg.sender, signedRCA.rca.dataService)
         );
+        /* solhint-disable gas-strict-inequalities */
         require(
             signedRCA.rca.deadline >= block.timestamp,
             RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCA.rca.deadline)
         );
+        /* solhint-enable gas-strict-inequalities */
 
         // check that the voucher is signed by the payer (or proxy)
         _requireAuthorizedRCASigner(signedRCA);
@@ -187,11 +192,13 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
      * @dev Note: Updated pricing terms apply immediately and will affect the next collection
      * for the entire period since lastCollectionAt.
      */
-    function update(SignedRCAU calldata signedRCAU) external {
+    function update(SignedRCAU calldata signedRCAU) external { // solhint-disable-line function-max-lines
+        /* solhint-disable gas-strict-inequalities */
         require(
             signedRCAU.rcau.deadline >= block.timestamp,
             RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCAU.rcau.deadline)
         );
+        /* solhint-enable gas-strict-inequalities */
 
         AgreementData storage agreement = _getAgreementStorage(signedRCAU.rcau.agreementId);
         require(
@@ -268,7 +275,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
 
     /// @inheritdoc IRecurringCollector
     function getCollectionInfo(
-        AgreementData memory agreement
+        AgreementData calldata agreement
     ) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason) {
         return _getCollectionInfo(agreement);
     }
@@ -303,7 +310,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
      * @param _params The decoded parameters for the collection
      * @return The amount of tokens collected
      */
-    function _collect(
+    function _collect( // solhint-disable-line function-max-lines
         IGraphPayments.PaymentTypes _paymentType,
         CollectParams memory _params
     ) private returns (uint256) {
@@ -336,10 +343,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
             tokensToCollect = _requireValidCollect(agreement, _params.agreementId, _params.tokens, collectionSeconds);
 
             uint256 slippage = _params.tokens - tokensToCollect;
+            /* solhint-disable gas-strict-inequalities */
             require(
                 slippage <= _params.maxSlippage,
                 RecurringCollectorExcessiveSlippage(_params.tokens, tokensToCollect, _params.maxSlippage)
             );
+            /* solhint-enable gas-strict-inequalities */
         }
         agreement.lastCollectionAt = uint64(block.timestamp);
 
@@ -395,6 +404,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
         // Collection window needs to be at least MIN_SECONDS_COLLECTION_WINDOW
         require(
             _maxSecondsPerCollection > _minSecondsPerCollection &&
+                // solhint-disable-next-line gas-strict-inequalities
                 (_maxSecondsPerCollection - _minSecondsPerCollection >= MIN_SECONDS_COLLECTION_WINDOW),
             RecurringCollectorAgreementInvalidCollectionWindow(
                 MIN_SECONDS_COLLECTION_WINDOW,
@@ -405,6 +415,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
 
         // Agreement needs to last at least one min collection window
         require(
+            // solhint-disable-next-line gas-strict-inequalities
             _endsAt - block.timestamp >= _minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
             RecurringCollectorAgreementInvalidDuration(
                 _minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
@@ -431,6 +442,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
             block.timestamp > _agreement.endsAt;
         if (!canceledOrElapsed) {
             require(
+                // solhint-disable-next-line gas-strict-inequalities
                 _collectionSeconds >= _agreement.minSecondsPerCollection,
                 RecurringCollectorCollectionTooSoon(
                     _agreementId,
@@ -440,6 +452,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
             );
         }
         require(
+            // solhint-disable-next-line gas-strict-inequalities
             _collectionSeconds <= _agreement.maxSecondsPerCollection,
             RecurringCollectorCollectionTooLate(
                 _agreementId,
diff --git a/packages/interfaces/contracts/horizon/IRecurringCollector.sol b/packages/interfaces/contracts/horizon/IRecurringCollector.sol
@@ -7,6 +7,7 @@ import { IAuthorizable } from "./IAuthorizable.sol";
 
 /**
  * @title Interface for the {RecurringCollector} contract
+ * @author Edge & Node
  * @dev Implements the {IPaymentCollector} interface as defined by the Graph
  * Horizon payments protocol.
  * @notice Implements a payments collector contract that can be used to collect
@@ -63,6 +64,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      * @param metadata Arbitrary metadata to extend functionality if a data service requires it
      *
      */
+    // solhint-disable-next-line gas-struct-packing
     struct RecurringCollectionAgreement {
         uint64 deadline;
         uint64 endsAt;
@@ -101,6 +103,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      * @param nonce The nonce for preventing replay attacks (must be current nonce + 1)
      * @param metadata Arbitrary metadata to extend functionality if a data service requires it
      */
+    // solhint-disable-next-line gas-struct-packing
     struct RecurringCollectionAgreementUpdate {
         bytes16 agreementId;
         uint64 deadline;
@@ -395,48 +398,48 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
     error RecurringCollectorExcessiveSlippage(uint256 requested, uint256 actual, uint256 maxSlippage);
 
     /**
-     * @dev Accept an indexing agreement.
+     * @notice Accept an indexing agreement.
      * @param signedRCA The signed Recurring Collection Agreement which is to be accepted.
      * @return agreementId The deterministically generated agreement ID
      */
     function accept(SignedRCA calldata signedRCA) external returns (bytes16 agreementId);
 
     /**
-     * @dev Cancel an indexing agreement.
+     * @notice Cancel an indexing agreement.
      * @param agreementId The agreement's ID.
      * @param by The party that is canceling the agreement.
      */
     function cancel(bytes16 agreementId, CancelAgreementBy by) external;
 
     /**
-     * @dev Update an indexing agreement.
+     * @notice Update an indexing agreement.
      * @param signedRCAU The signed Recurring Collection Agreement Update which is to be applied.
      */
     function update(SignedRCAU calldata signedRCAU) external;
 
     /**
-     * @dev Computes the hash of a RecurringCollectionAgreement (RCA).
+     * @notice Computes the hash of a RecurringCollectionAgreement (RCA).
      * @param rca The RCA for which to compute the hash.
      * @return The hash of the RCA.
      */
     function hashRCA(RecurringCollectionAgreement calldata rca) external view returns (bytes32);
 
     /**
-     * @dev Computes the hash of a RecurringCollectionAgreementUpdate (RCAU).
+     * @notice Computes the hash of a RecurringCollectionAgreementUpdate (RCAU).
      * @param rcau The RCAU for which to compute the hash.
      * @return The hash of the RCAU.
      */
     function hashRCAU(RecurringCollectionAgreementUpdate calldata rcau) external view returns (bytes32);
 
     /**
-     * @dev Recovers the signer address of a signed RecurringCollectionAgreement (RCA).
+     * @notice Recovers the signer address of a signed RecurringCollectionAgreement (RCA).
      * @param signedRCA The SignedRCA containing the RCA and its signature.
      * @return The address of the signer.
      */
     function recoverRCASigner(SignedRCA calldata signedRCA) external view returns (address);
 
     /**
-     * @dev Recovers the signer address of a signed RecurringCollectionAgreementUpdate (RCAU).
+     * @notice Recovers the signer address of a signed RecurringCollectionAgreementUpdate (RCAU).
      * @param signedRCAU The SignedRCAU containing the RCAU and its signature.
      * @return The address of the signer.
      */
@@ -458,7 +461,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      * @return reason The reason why the agreement is not collectable (None if collectable)
      */
     function getCollectionInfo(
-        AgreementData memory agreement
+        AgreementData calldata agreement
     ) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason);
 
     /**
diff --git a/packages/interfaces/contracts/subgraph-service/IDisputeManager.sol b/packages/interfaces/contracts/subgraph-service/IDisputeManager.sol
@@ -122,9 +122,9 @@ interface IDisputeManager {
     );
 
     /**
-     * @dev Emitted when an indexing fee dispute is created for `agreementId` and `indexer`
+     * @notice Emitted when an indexing fee dispute is created for `agreementId` and `indexer`
      * by `fisherman`.
-     * The event emits the amount of `tokens` deposited by the fisherman.
+     * @dev The event emits the amount of `tokens` deposited by the fisherman.
      * @param disputeId The dispute id
      * @param indexer The indexer address
      * @param fisherman The fisherman address
diff --git a/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol b/packages/interfaces/contracts/subgraph-service/ISubgraphService.sol
@@ -75,6 +75,7 @@ interface ISubgraphService is IDataServiceFees {
      * @param indexingFeesCut The indexing fees cut
      */
     event IndexingFeesCutSet(uint256 indexingFeesCut);
+    // solhint-disable-previous-line gas-indexed-events
 
     /**
      * @notice Thrown when trying to set a curation cut that is not a valid PPM value
diff --git a/packages/subgraph-service/package.json b/packages/subgraph-service/package.json
@@ -33,7 +33,7 @@
     "test:deployment": "SECURE_ACCOUNTS_DISABLE_PROVIDER=true hardhat test test/deployment/*.ts",
     "test:integration": "./scripts/integration",
     "test:coverage": "pnpm build && pnpm test:coverage:self",
-    "test:coverage:self": "forge coverage",
+    "test:coverage:self": "forge coverage --ir-minimum",
     "prepublishOnly": "pnpm run build"
   },
   "devDependencies": {
