Skip to content

Commit 3d371e1

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 3d371e1

File tree

3 files changed

+19
-6
lines changed

3 files changed

+19
-6
lines changed

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

Lines changed: 17 additions & 4 deletions
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;
@@ -75,7 +78,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
7578
* See {IRecurringCollector.accept}.
7679
* @dev Caller must be the data service the RCA was issued to.
7780
*/
78-
function accept(SignedRCA calldata signedRCA) external returns (bytes16) {
81+
function accept(SignedRCA calldata signedRCA) external returns (bytes16) { // solhint-disable-line function-max-lines
7982
bytes16 agreementId = _generateAgreementId(
8083
signedRCA.rca.payer,
8184
signedRCA.rca.dataService,
@@ -89,10 +92,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
8992
msg.sender == signedRCA.rca.dataService,
9093
RecurringCollectorUnauthorizedCaller(msg.sender, signedRCA.rca.dataService)
9194
);
95+
/* solhint-disable gas-strict-inequalities */
9296
require(
9397
signedRCA.rca.deadline >= block.timestamp,
9498
RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCA.rca.deadline)
9599
);
100+
/* solhint-enable gas-strict-inequalities */
96101

97102
// check that the voucher is signed by the payer (or proxy)
98103
_requireAuthorizedRCASigner(signedRCA);
@@ -187,11 +192,13 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
187192
* @dev Note: Updated pricing terms apply immediately and will affect the next collection
188193
* for the entire period since lastCollectionAt.
189194
*/
190-
function update(SignedRCAU calldata signedRCAU) external {
195+
function update(SignedRCAU calldata signedRCAU) external { // solhint-disable-line function-max-lines
196+
/* solhint-disable gas-strict-inequalities */
191197
require(
192198
signedRCAU.rcau.deadline >= block.timestamp,
193199
RecurringCollectorAgreementDeadlineElapsed(block.timestamp, signedRCAU.rcau.deadline)
194200
);
201+
/* solhint-enable gas-strict-inequalities */
195202

196203
AgreementData storage agreement = _getAgreementStorage(signedRCAU.rcau.agreementId);
197204
require(
@@ -268,7 +275,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
268275

269276
/// @inheritdoc IRecurringCollector
270277
function getCollectionInfo(
271-
AgreementData memory agreement
278+
AgreementData calldata agreement
272279
) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason) {
273280
return _getCollectionInfo(agreement);
274281
}
@@ -303,7 +310,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
303310
* @param _params The decoded parameters for the collection
304311
* @return The amount of tokens collected
305312
*/
306-
function _collect(
313+
function _collect( // solhint-disable-line function-max-lines
307314
IGraphPayments.PaymentTypes _paymentType,
308315
CollectParams memory _params
309316
) private returns (uint256) {
@@ -336,10 +343,12 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
336343
tokensToCollect = _requireValidCollect(agreement, _params.agreementId, _params.tokens, collectionSeconds);
337344

338345
uint256 slippage = _params.tokens - tokensToCollect;
346+
/* solhint-disable gas-strict-inequalities */
339347
require(
340348
slippage <= _params.maxSlippage,
341349
RecurringCollectorExcessiveSlippage(_params.tokens, tokensToCollect, _params.maxSlippage)
342350
);
351+
/* solhint-enable gas-strict-inequalities */
343352
}
344353
agreement.lastCollectionAt = uint64(block.timestamp);
345354

@@ -395,6 +404,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
395404
// Collection window needs to be at least MIN_SECONDS_COLLECTION_WINDOW
396405
require(
397406
_maxSecondsPerCollection > _minSecondsPerCollection &&
407+
// solhint-disable-next-line gas-strict-inequalities
398408
(_maxSecondsPerCollection - _minSecondsPerCollection >= MIN_SECONDS_COLLECTION_WINDOW),
399409
RecurringCollectorAgreementInvalidCollectionWindow(
400410
MIN_SECONDS_COLLECTION_WINDOW,
@@ -405,6 +415,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
405415

406416
// Agreement needs to last at least one min collection window
407417
require(
418+
// solhint-disable-next-line gas-strict-inequalities
408419
_endsAt - block.timestamp >= _minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
409420
RecurringCollectorAgreementInvalidDuration(
410421
_minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
@@ -431,6 +442,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
431442
block.timestamp > _agreement.endsAt;
432443
if (!canceledOrElapsed) {
433444
require(
445+
// solhint-disable-next-line gas-strict-inequalities
434446
_collectionSeconds >= _agreement.minSecondsPerCollection,
435447
RecurringCollectorCollectionTooSoon(
436448
_agreementId,
@@ -440,6 +452,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
440452
);
441453
}
442454
require(
455+
// solhint-disable-next-line gas-strict-inequalities
443456
_collectionSeconds <= _agreement.maxSecondsPerCollection,
444457
RecurringCollectorCollectionTooLate(
445458
_agreementId,

packages/interfaces/contracts/horizon/IRecurringCollector.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
458458
* @return reason The reason why the agreement is not collectable (None if collectable)
459459
*/
460460
function getCollectionInfo(
461-
AgreementData memory agreement
461+
AgreementData calldata agreement
462462
) external view returns (bool isCollectable, uint256 collectionSeconds, AgreementNotCollectableReason reason);
463463

464464
/**

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)