Skip to content

Commit c3f6f07

Browse files
committed
fix: resolve solhint warnings in RecurringCollector and fix subgraph-service coverage
- 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
1 parent 4450e9c commit c3f6f07

File tree

5 files changed

+35
-12
lines changed

5 files changed

+35
-12
lines changed

packages/horizon/contracts/payments/collectors/RecurringCollector.sol

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { PPMMath } from "../../libraries/PPMMath.sol";
1515

1616
/**
1717
* @title RecurringCollector contract
18+
* @author Edge & Node
1819
* @dev Implements the {IRecurringCollector} interface.
1920
* @notice A payments collector contract that can be used to collect payments using a RCA (Recurring Collection Agreement).
2021
* @custom:security-contact Please email security+contracts@thegraph.com if you find any
@@ -26,6 +27,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
2627
/// @notice The minimum number of seconds that must be between two collections
2728
uint32 public constant MIN_SECONDS_COLLECTION_WINDOW = 600;
2829

30+
/* solhint-disable gas-small-strings */
2931
/// @notice The EIP712 typehash for the RecurringCollectionAgreement struct
3032
bytes32 public constant EIP712_RCA_TYPEHASH =
3133
keccak256(
@@ -37,6 +39,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
3739
keccak256(
3840
"RecurringCollectionAgreementUpdate(bytes16 agreementId,uint64 deadline,uint64 endsAt,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,uint32 nonce,bytes metadata)"
3941
);
42+
/* solhint-enable gas-small-strings */
4043

4144
/// @notice Tracks agreements
4245
mapping(bytes16 agreementId => AgreementData data) public agreements;
@@ -69,6 +72,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
6972
}
7073
}
7174

75+
/* solhint-disable function-max-lines */
7276
/**
7377
* @inheritdoc IRecurringCollector
7478
* @notice Accept a Recurring Collection Agreement.
@@ -89,10 +93,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
8993
msg.sender == signedRCA.rca.dataService,
9094
RecurringCollectorUnauthorizedCaller(msg.sender, signedRCA.rca.dataService)
9195
);
96+
/* solhint-disable gas-strict-inequalities */
9297
require(
9398
signedRCA.rca.deadline >= block.timestamp,
9499
RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCA.rca.deadline)
95100
);
101+
/* solhint-enable gas-strict-inequalities */
96102

97103
// check that the voucher is signed by the payer (or proxy)
98104
_requireAuthorizedRCASigner(signedRCA);
@@ -145,6 +151,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
145151

146152
return agreementId;
147153
}
154+
/* solhint-enable function-max-lines */
148155

149156
/**
150157
* @inheritdoc IRecurringCollector
@@ -179,6 +186,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
179186
);
180187
}
181188

189+
/* solhint-disable function-max-lines */
182190
/**
183191
* @inheritdoc IRecurringCollector
184192
* @notice Update a Recurring Collection Agreement.
@@ -188,10 +196,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
188196
* for the entire period since lastCollectionAt.
189197
*/
190198
function update(SignedRCAU calldata signedRCAU) external {
199+
/* solhint-disable gas-strict-inequalities */
191200
require(
192201
signedRCAU.rcau.deadline >= block.timestamp,
193202
RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCAU.rcau.deadline)
194203
);
204+
/* solhint-enable gas-strict-inequalities */
195205

196206
AgreementData storage agreement = _getAgreementStorage(signedRCAU.rcau.agreementId);
197207
require(
@@ -240,6 +250,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
240250
agreement.maxSecondsPerCollection
241251
);
242252
}
253+
/* solhint-enable function-max-lines */
243254

244255
/// @inheritdoc IRecurringCollector
245256
function recoverRCASigner(SignedRCA calldata signedRCA) external view returns (address) {
@@ -268,7 +279,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
268279

269280
/// @inheritdoc IRecurringCollector
270281
function getCollectionInfo(
271-
AgreementData memory agreement
282+
AgreementData calldata agreement
272283
) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason) {
273284
return _getCollectionInfo(agreement);
274285
}
@@ -293,6 +304,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
293304
return abi.decode(data, (CollectParams));
294305
}
295306

307+
/* solhint-disable function-max-lines */
296308
/**
297309
* @notice Collect payment through the payments protocol.
298310
* @dev Caller must be the data service the RCA was issued to.
@@ -336,10 +348,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
336348
tokensToCollect = _requireValidCollect(agreement, _params.agreementId, _params.tokens, collectionSeconds);
337349

338350
uint256 slippage = _params.tokens - tokensToCollect;
351+
/* solhint-disable gas-strict-inequalities */
339352
require(
340353
slippage <= _params.maxSlippage,
341354
RecurringCollectorExcessiveSlippage(_params.tokens, tokensToCollect, _params.maxSlippage)
342355
);
356+
/* solhint-enable gas-strict-inequalities */
343357
}
344358
agreement.lastCollectionAt = uint64(block.timestamp);
345359

@@ -376,6 +390,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
376390

377391
return tokensToCollect;
378392
}
393+
/* solhint-enable function-max-lines */
379394

380395
/**
381396
* @notice Requires that the collection window parameters are valid.
@@ -395,6 +410,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
395410
// Collection window needs to be at least MIN_SECONDS_COLLECTION_WINDOW
396411
require(
397412
_maxSecondsPerCollection > _minSecondsPerCollection &&
413+
// solhint-disable-next-line gas-strict-inequalities
398414
(_maxSecondsPerCollection - _minSecondsPerCollection >= MIN_SECONDS_COLLECTION_WINDOW),
399415
RecurringCollectorAgreementInvalidCollectionWindow(
400416
MIN_SECONDS_COLLECTION_WINDOW,
@@ -405,6 +421,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
405421

406422
// Agreement needs to last at least one min collection window
407423
require(
424+
// solhint-disable-next-line gas-strict-inequalities
408425
_endsAt - block.timestamp >= _minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
409426
RecurringCollectorAgreementInvalidDuration(
410427
_minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
@@ -431,6 +448,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
431448
block.timestamp > _agreement.endsAt;
432449
if (!canceledOrElapsed) {
433450
require(
451+
// solhint-disable-next-line gas-strict-inequalities
434452
_collectionSeconds >= _agreement.minSecondsPerCollection,
435453
RecurringCollectorCollectionTooSoon(
436454
_agreementId,
@@ -440,6 +458,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
440458
);
441459
}
442460
require(
461+
// solhint-disable-next-line gas-strict-inequalities
443462
_collectionSeconds <= _agreement.maxSecondsPerCollection,
444463
RecurringCollectorCollectionTooLate(
445464
_agreementId,

packages/interfaces/contracts/horizon/IRecurringCollector.sol

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { IAuthorizable } from "./IAuthorizable.sol";
77

88
/**
99
* @title Interface for the {RecurringCollector} contract
10+
* @author Edge & Node
1011
* @dev Implements the {IPaymentCollector} interface as defined by the Graph
1112
* Horizon payments protocol.
1213
* @notice Implements a payments collector contract that can be used to collect
@@ -63,6 +64,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
6364
* @param metadata Arbitrary metadata to extend functionality if a data service requires it
6465
*
6566
*/
67+
// solhint-disable-next-line gas-struct-packing
6668
struct RecurringCollectionAgreement {
6769
uint64 deadline;
6870
uint64 endsAt;
@@ -101,6 +103,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
101103
* @param nonce The nonce for preventing replay attacks (must be current nonce + 1)
102104
* @param metadata Arbitrary metadata to extend functionality if a data service requires it
103105
*/
106+
// solhint-disable-next-line gas-struct-packing
104107
struct RecurringCollectionAgreementUpdate {
105108
bytes16 agreementId;
106109
uint64 deadline;
@@ -395,48 +398,48 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
395398
error RecurringCollectorExcessiveSlippage(uint256 requested, uint256 actual, uint256 maxSlippage);
396399

397400
/**
398-
* @dev Accept an indexing agreement.
401+
* @notice Accept an indexing agreement.
399402
* @param signedRCA The signed Recurring Collection Agreement which is to be accepted.
400403
* @return agreementId The deterministically generated agreement ID
401404
*/
402405
function accept(SignedRCA calldata signedRCA) external returns (bytes16 agreementId);
403406

404407
/**
405-
* @dev Cancel an indexing agreement.
408+
* @notice Cancel an indexing agreement.
406409
* @param agreementId The agreement's ID.
407410
* @param by The party that is canceling the agreement.
408411
*/
409412
function cancel(bytes16 agreementId, CancelAgreementBy by) external;
410413

411414
/**
412-
* @dev Update an indexing agreement.
415+
* @notice Update an indexing agreement.
413416
* @param signedRCAU The signed Recurring Collection Agreement Update which is to be applied.
414417
*/
415418
function update(SignedRCAU calldata signedRCAU) external;
416419

417420
/**
418-
* @dev Computes the hash of a RecurringCollectionAgreement (RCA).
421+
* @notice Computes the hash of a RecurringCollectionAgreement (RCA).
419422
* @param rca The RCA for which to compute the hash.
420423
* @return The hash of the RCA.
421424
*/
422425
function hashRCA(RecurringCollectionAgreement calldata rca) external view returns (bytes32);
423426

424427
/**
425-
* @dev Computes the hash of a RecurringCollectionAgreementUpdate (RCAU).
428+
* @notice Computes the hash of a RecurringCollectionAgreementUpdate (RCAU).
426429
* @param rcau The RCAU for which to compute the hash.
427430
* @return The hash of the RCAU.
428431
*/
429432
function hashRCAU(RecurringCollectionAgreementUpdate calldata rcau) external view returns (bytes32);
430433

431434
/**
432-
* @dev Recovers the signer address of a signed RecurringCollectionAgreement (RCA).
435+
* @notice Recovers the signer address of a signed RecurringCollectionAgreement (RCA).
433436
* @param signedRCA The SignedRCA containing the RCA and its signature.
434437
* @return The address of the signer.
435438
*/
436439
function recoverRCASigner(SignedRCA calldata signedRCA) external view returns (address);
437440

438441
/**
439-
* @dev Recovers the signer address of a signed RecurringCollectionAgreementUpdate (RCAU).
442+
* @notice Recovers the signer address of a signed RecurringCollectionAgreementUpdate (RCAU).
440443
* @param signedRCAU The SignedRCAU containing the RCAU and its signature.
441444
* @return The address of the signer.
442445
*/
@@ -458,7 +461,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
458461
* @return reason The reason why the agreement is not collectable (None if collectable)
459462
*/
460463
function getCollectionInfo(
461-
AgreementData memory agreement
464+
AgreementData calldata agreement
462465
) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason);
463466

464467
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ interface IDisputeManager {
122122
);
123123

124124
/**
125-
* @dev Emitted when an indexing fee dispute is created for `agreementId` and `indexer`
125+
* @notice Emitted when an indexing fee dispute is created for `agreementId` and `indexer`
126126
* by `fisherman`.
127-
* The event emits the amount of `tokens` deposited by the fisherman.
127+
* @dev The event emits the amount of `tokens` deposited by the fisherman.
128128
* @param disputeId The dispute id
129129
* @param indexer The indexer address
130130
* @param fisherman The fisherman address

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ interface ISubgraphService is IDataServiceFees {
7575
* @param indexingFeesCut The indexing fees cut
7676
*/
7777
event IndexingFeesCutSet(uint256 indexingFeesCut);
78+
// solhint-disable-previous-line gas-indexed-events
7879

7980
/**
8081
* @notice Thrown when trying to set a curation cut that is not a valid PPM value

packages/subgraph-service/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"test:deployment": "SECURE_ACCOUNTS_DISABLE_PROVIDER=true hardhat test test/deployment/*.ts",
3434
"test:integration": "./scripts/integration",
3535
"test:coverage": "pnpm build && pnpm test:coverage:self",
36-
"test:coverage:self": "forge coverage",
36+
"test:coverage:self": "forge coverage --ir-minimum",
3737
"prepublishOnly": "pnpm run build"
3838
},
3939
"devDependencies": {

0 commit comments

Comments
 (0)