f - simplify deadline, reduce errors

Author: matiasedgeandnode

IndexingPaymentsTodo.md

@@ -13,11 +13,14 @@
 * Support a way for gateway to shop an agreement around? Deadline + dedup key? So only one agreement with the dedupe key can be accepted?
 * If an indexer closes an allocation, what should happen to the accepeted agreement?
 * test_SubgraphService_CollectIndexingFee_Integration fails with PaymentsEscrowInconsistentCollection
-* Reduce the number of errors declared and returned
 * Switch `duration` for `endsAt`?
+* Check that UUID-v4 fits in `bytes16`
+* Test `upgrade` paths
+* Test lock stake
 
 # Done
 
+* DONE: ~~Reduce the number of errors declared and returned~~
 * DONE: ~~Support `DisputeManager`~~
 * DONE: ~~Check upgrade conditions. Support indexing agreement upgradability, so that there is a mechanism to adjust the rates without having to cancel and start over.~~
 * DONE: ~~Maybe check that the epoch the indexer is sending is the one the transaction will be run in?~~

packages/horizon/contracts/interfaces/IRecurringCollector.sol

@@ -26,7 +26,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         // The agreement ID of the RCA
         bytes16 agreementId;
         // The deadline for accepting the RCA
-        uint256 acceptDeadline;
+        uint256 deadline;
         // The duration of the RCA in seconds
         uint256 duration;
         // The address of the payer the RCA was issued by
@@ -61,7 +61,7 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         // The agreement ID
         bytes16 agreementId;
         // The deadline for upgrading
-        uint256 upgradeDeadline;
+        uint256 deadline;
         // The duration of the agreement in seconds
         uint256 duration;
         // The maximum amount of tokens that can be collected in the first collection
@@ -184,23 +184,17 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
     );
 
     /**
-     * Thrown when calling cancel() for an agreement not owned by the calling data service
+     * Thrown when interacting with an agreement not owned by the message sender
      * @param agreementId The agreement ID
-     * @param dataService The address of the data service
+     * @param unauthorizedDataService The address of the unauthorized data service
      */
-    error RecurringCollectorDataServiceNotAuthorized(bytes16 agreementId, address dataService);
+    error RecurringCollectorDataServiceNotAuthorized(bytes16 agreementId, address unauthorizedDataService);
 
     /**
-     * Thrown when calling accept() for an agreement with an elapsed acceptance deadline
-     * @param elapsedAt The timestamp when the acceptance deadline elapsed
+     * Thrown when interacting with an agreement with an elapsed deadline
+     * @param elapsedAt The timestamp when the deadline elapsed
      */
-    error RecurringCollectorAgreementAcceptanceElapsed(uint256 elapsedAt);
-
-    /**
-     * Thrown when calling upgrade() for an agreement with an elapsed upgrade deadline
-     * @param elapsedAt The timestamp when the upgrade deadline elapsed
-     */
-    error RecurringCollectorAgreementUpgradeElapsed(uint256 elapsedAt);
+    error RecurringCollectorAgreementDeadlineElapsed(uint256 elapsedAt);
 
     /**
      * Thrown when the signer is invalid
@@ -209,22 +203,22 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
 
     /**
      * Thrown when the payment type is not IndexingFee
-     * @param paymentType The provided payment type
+     * @param invalidPaymentType The invalid payment type
      */
-    error RecurringCollectorInvalidPaymentType(IGraphPayments.PaymentTypes paymentType);
+    error RecurringCollectorInvalidPaymentType(IGraphPayments.PaymentTypes invalidPaymentType);
 
     /**
      * Thrown when the caller is not the data service the RCA was issued to
-     * @param caller The address of the caller
+     * @param unauthorizedCaller The address of the caller
      * @param dataService The address of the data service
      */
-    error RecurringCollectorCallerNotDataService(address caller, address dataService);
+    error RecurringCollectorUnauthorizedCaller(address unauthorizedCaller, address dataService);
 
     /**
      * Thrown when calling collect() with invalid data
-     * @param data The invalid data
+     * @param invalidData The invalid data
      */
-    error RecurringCollectorInvalidCollectData(bytes data);
+    error RecurringCollectorInvalidCollectData(bytes invalidData);
 
     /**
      * Thrown when calling accept() for an already accepted agreement
@@ -233,23 +227,21 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
     error RecurringCollectorAgreementAlreadyAccepted(bytes16 agreementId);
 
     /**
-     * Thrown when calling cancel() for a never accepted agreement
+     * Thrown when interacting with an agreement that was never accepted
      * @param agreementId The agreement ID
      */
     error RecurringCollectorAgreementNeverAccepted(bytes16 agreementId);
 
     /**
-     * Thrown when calling collect() on an invalid agreement
+     * Thrown when interacting with an agreement that was canceled
      * @param agreementId The agreement ID
-     * @param acceptedAt The agreement accepted timestamp
      */
-    error RecurringCollectorAgreementInvalid(bytes16 agreementId, uint256 acceptedAt);
+    error RecurringCollectorAgreementCanceled(bytes16 agreementId);
 
     /**
-     * Thrown when accepting or upgrading an agreement with invalid params
-     * @param agreementId The agreement ID
+     * Thrown when accepting or upgrading an agreement with invalid parameters
      */
-    error RecurringCollectorAgreementInvalidParams(bytes16 agreementId);
+    error RecurringCollectorAgreementInvalidParameters();
 
     /**
      * Thrown when calling collect() on an elapsed agreement

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

@@ -23,13 +23,13 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
     /// @notice The EIP712 typehash for the RecurringCollectionAgreement struct
     bytes32 public constant EIP712_RCA_TYPEHASH =
         keccak256(
-            "RecurringCollectionAgreement(bytes16 agreementId,uint256 acceptDeadline,uint256 duration,address payer,address dataService,address serviceProvider,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)"
+            "RecurringCollectionAgreement(bytes16 agreementId,uint256 deadline,uint256 duration,address payer,address dataService,address serviceProvider,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)"
         );
 
     /// @notice The EIP712 typehash for the RecurringCollectionAgreementUpgrade struct
     bytes32 public constant EIP712_RCAU_TYPEHASH =
         keccak256(
-            "RecurringCollectionAgreementUpgrade(bytes16 agreementId,uint256 upgradeDeadline,uint256 duration,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)"
+            "RecurringCollectionAgreementUpgrade(bytes16 agreementId,uint256 deadline,uint256 duration,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)"
         );
 
     /// @notice Sentinel value to indicate an agreement has been canceled
@@ -77,11 +77,11 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
     function accept(SignedRCA calldata signedRCA) external {
         require(
             msg.sender == signedRCA.rca.dataService,
-            RecurringCollectorCallerNotDataService(msg.sender, signedRCA.rca.dataService)
+            RecurringCollectorUnauthorizedCaller(msg.sender, signedRCA.rca.dataService)
         );
         require(
-            signedRCA.rca.acceptDeadline >= block.timestamp,
-            RecurringCollectorAgreementAcceptanceElapsed(signedRCA.rca.acceptDeadline)
+            signedRCA.rca.deadline >= block.timestamp,
+            RecurringCollectorAgreementDeadlineElapsed(signedRCA.rca.deadline)
         );
 
         // check that the voucher is signed by the payer (or proxy)
@@ -101,7 +101,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
         agreement.maxOngoingTokensPerSecond = signedRCA.rca.maxOngoingTokensPerSecond;
         agreement.minSecondsPerCollection = signedRCA.rca.minSecondsPerCollection;
         agreement.maxSecondsPerCollection = signedRCA.rca.maxSecondsPerCollection;
-        _requireValidAgreement(agreement, signedRCA.rca.agreementId);
+        _requireValidAgreement(agreement);
 
         emit AgreementAccepted(
             agreement.dataService,
@@ -147,8 +147,8 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
      */
     function upgrade(SignedRCAU calldata signedRCAU) external {
         require(
-            signedRCAU.rcau.upgradeDeadline >= block.timestamp,
-            RecurringCollectorAgreementUpgradeElapsed(signedRCAU.rcau.upgradeDeadline)
+            signedRCAU.rcau.deadline >= block.timestamp,
+            RecurringCollectorAgreementDeadlineElapsed(signedRCAU.rcau.deadline)
         );
 
         AgreementData storage agreement = _getForUpdateAgreement(signedRCAU.rcau.agreementId);
@@ -167,7 +167,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
         agreement.maxOngoingTokensPerSecond = signedRCAU.rcau.maxOngoingTokensPerSecond;
         agreement.minSecondsPerCollection = signedRCAU.rcau.minSecondsPerCollection;
         agreement.maxSecondsPerCollection = signedRCAU.rcau.maxSecondsPerCollection;
-        _requireValidAgreement(agreement, signedRCAU.rcau.agreementId);
+        _requireValidAgreement(agreement);
 
         emit AgreementUpgraded(
             agreement.dataService,
@@ -236,10 +236,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
      */
     function _collect(CollectParams memory _params) private returns (uint256) {
         AgreementData storage agreement = _getForUpdateAgreement(_params.agreementId);
-        require(
-            agreement.acceptedAt > 0,
-            RecurringCollectorAgreementInvalid(_params.agreementId, agreement.acceptedAt)
-        );
+        require(agreement.acceptedAt > 0, RecurringCollectorAgreementNeverAccepted(_params.agreementId));
         require(
             msg.sender == agreement.dataService,
             RecurringCollectorDataServiceNotAuthorized(_params.agreementId, msg.sender)
@@ -283,36 +280,33 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
         return _params.tokens;
     }
 
-    function _requireValidAgreement(AgreementData memory _agreement, bytes16 _agreementId) private view {
+    function _requireValidAgreement(AgreementData memory _agreement) private view {
         require(
             _agreement.dataService != address(0) &&
                 _agreement.payer != address(0) &&
                 _agreement.serviceProvider != address(0),
-            RecurringCollectorAgreementInvalidParams(_agreementId)
+            RecurringCollectorAgreementInvalidParameters()
         );
 
         // Agreement needs to end in the future
-        require(_agreementEndsAt(_agreement) > block.timestamp, RecurringCollectorAgreementInvalidParams(_agreementId));
+        require(_agreementEndsAt(_agreement) > block.timestamp, RecurringCollectorAgreementInvalidParameters());
 
         // Collection window needs to be at least 2 hours
         require(
             _agreement.maxSecondsPerCollection > _agreement.minSecondsPerCollection &&
                 (_agreement.maxSecondsPerCollection - _agreement.minSecondsPerCollection >= 7200),
-            RecurringCollectorAgreementInvalidParams(_agreementId)
+            RecurringCollectorAgreementInvalidParameters()
         );
 
         // Agreement needs to last at least one min collection window
         require(
             _agreement.duration >= _agreement.minSecondsPerCollection + 7200,
-            RecurringCollectorAgreementInvalidParams(_agreementId)
+            RecurringCollectorAgreementInvalidParameters()
         );
     }
 
     function _requireCollectableAgreement(AgreementData memory _agreement, bytes16 _agreementId) private view {
-        require(
-            _agreement.acceptedAt > 0 && _agreement.acceptedAt != CANCELED,
-            RecurringCollectorAgreementInvalid(_agreementId, _agreement.acceptedAt)
-        );
+        require(_agreement.acceptedAt != CANCELED, RecurringCollectorAgreementCanceled(_agreementId));
 
         uint256 agreementEnd = _agreement.duration < type(uint256).max - _agreement.acceptedAt
             ? _agreement.acceptedAt + _agreement.duration
@@ -367,7 +361,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
                     abi.encode(
                         EIP712_RCA_TYPEHASH,
                         _rca.agreementId,
-                        _rca.acceptDeadline,
+                        _rca.deadline,
                         _rca.duration,
                         _rca.payer,
                         _rca.dataService,
@@ -392,7 +386,7 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
                     abi.encode(
                         EIP712_RCAU_TYPEHASH,
                         _rcau.agreementId,
-                        _rcau.upgradeDeadline,
+                        _rcau.deadline,
                         _rcau.duration,
                         _rcau.maxInitialTokens,
                         _rcau.maxOngoingTokensPerSecond,

packages/horizon/test/payments/recurring-collector/RecurringCollectorHelper.t.sol

@@ -49,14 +49,14 @@ contract RecurringCollectorHelper is AuthorizableHelper, Bounder {
         IRecurringCollector.RecurringCollectionAgreement memory rca
     ) public view returns (IRecurringCollector.RecurringCollectionAgreement memory) {
         require(block.timestamp > 0, "block.timestamp can't be zero");
-        rca.acceptDeadline = bound(rca.acceptDeadline, 0, block.timestamp - 1);
+        rca.deadline = bound(rca.deadline, 0, block.timestamp - 1);
         return rca;
     }
 
     function withOKAcceptDeadline(
         IRecurringCollector.RecurringCollectionAgreement memory rca
     ) public view returns (IRecurringCollector.RecurringCollectionAgreement memory) {
-        rca.acceptDeadline = boundTimestampMin(rca.acceptDeadline, block.timestamp);
+        rca.deadline = boundTimestampMin(rca.deadline, block.timestamp);
         return rca;
     }
 }

packages/horizon/test/payments/recurring-collector/accept.t.sol

@@ -24,8 +24,8 @@ contract RecurringCollectorAcceptTest is RecurringCollectorSharedTest {
         fuzzySignedRCA.rca = _recurringCollectorHelper.withElapsedAcceptDeadline(fuzzySignedRCA.rca);
 
         bytes memory expectedErr = abi.encodeWithSelector(
-            IRecurringCollector.RecurringCollectorAgreementAcceptanceElapsed.selector,
-            fuzzySignedRCA.rca.acceptDeadline
+            IRecurringCollector.RecurringCollectorAgreementDeadlineElapsed.selector,
+            fuzzySignedRCA.rca.deadline
         );
         vm.expectRevert(expectedErr);
         vm.prank(fuzzySignedRCA.rca.dataService);

packages/horizon/test/payments/recurring-collector/collect.t.sol

@@ -72,9 +72,8 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest {
         bytes memory data = _generateCollectData(params.collectData);
 
         bytes memory expectedErr = abi.encodeWithSelector(
-            IRecurringCollector.RecurringCollectorAgreementInvalid.selector,
-            params.collectData.agreementId,
-            0
+            IRecurringCollector.RecurringCollectorAgreementNeverAccepted.selector,
+            params.collectData.agreementId
         );
         vm.expectRevert(expectedErr);
         vm.prank(params.dataService);
@@ -100,9 +99,8 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest {
         bytes memory data = _generateCollectData(collectParams);
 
         bytes memory expectedErr = abi.encodeWithSelector(
-            IRecurringCollector.RecurringCollectorAgreementInvalid.selector,
-            collectParams.agreementId,
-            type(uint256).max
+            IRecurringCollector.RecurringCollectorAgreementCanceled.selector,
+            collectParams.agreementId
         );
         vm.expectRevert(expectedErr);
         vm.prank(rca.dataService);

packages/horizon/test/payments/recurring-collector/shared.t.sol

@@ -52,7 +52,7 @@ contract RecurringCollectorSharedTest is Test, Bounder {
         uint256 _signerKey
     ) internal returns (IRecurringCollector.SignedRCA memory) {
         vm.assume(_rca.payer != address(0));
-        _rca.acceptDeadline = boundTimestampMin(_rca.acceptDeadline, block.timestamp + 1);
+        _rca.deadline = boundTimestampMin(_rca.deadline, block.timestamp + 1);
         return _authorizeAndAcceptV2(_rca, _signerKey);
     }
 

packages/horizon/test/payments/recurring-collector/upgrade.t.sol

@@ -20,15 +20,15 @@ contract RecurringCollectorUpgradeTest is RecurringCollectorSharedTest {
         IRecurringCollector.RecurringCollectionAgreementUpgrade memory rcau = _sensibleRCAU(rca);
 
         boundSkipCeil(unboundedUpgradeSkip, type(uint256).max);
-        rcau.upgradeDeadline = bound(rcau.upgradeDeadline, 0, block.timestamp - 1);
+        rcau.deadline = bound(rcau.deadline, 0, block.timestamp - 1);
         IRecurringCollector.SignedRCAU memory signedRCAU = IRecurringCollector.SignedRCAU({
             rcau: rcau,
             signature: ""
         });
 
         bytes memory expectedErr = abi.encodeWithSelector(
-            IRecurringCollector.RecurringCollectorAgreementUpgradeElapsed.selector,
-            rcau.upgradeDeadline
+            IRecurringCollector.RecurringCollectorAgreementDeadlineElapsed.selector,
+            rcau.deadline
         );
         vm.expectRevert(expectedErr);
         vm.prank(rca.dataService);
@@ -39,7 +39,7 @@ contract RecurringCollectorUpgradeTest is RecurringCollectorSharedTest {
         rca = _sensibleRCA(rca);
         IRecurringCollector.RecurringCollectionAgreementUpgrade memory rcau = _sensibleRCAU(rca);
 
-        rcau.upgradeDeadline = block.timestamp;
+        rcau.deadline = block.timestamp;
         IRecurringCollector.SignedRCAU memory signedRCAU = IRecurringCollector.SignedRCAU({
             rcau: rcau,
             signature: ""
@@ -67,7 +67,7 @@ contract RecurringCollectorUpgradeTest is RecurringCollectorSharedTest {
         _authorizeAndAccept(rca, signerKey);
 
         boundSkipCeil(unboundedUpgradeSkip, type(uint256).max);
-        rcau.upgradeDeadline = boundTimestampMin(rcau.upgradeDeadline, block.timestamp + 1);
+        rcau.deadline = boundTimestampMin(rcau.deadline, block.timestamp + 1);
         IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU(
             rcau,
             signerKey
@@ -98,7 +98,7 @@ contract RecurringCollectorUpgradeTest is RecurringCollectorSharedTest {
         _authorizeAndAccept(rca, signerKey);
 
         boundSkipCeil(unboundedUpgradeSkip, type(uint256).max);
-        rcau.upgradeDeadline = boundTimestampMin(rcau.upgradeDeadline, block.timestamp + 1);
+        rcau.deadline = boundTimestampMin(rcau.deadline, block.timestamp + 1);
 
         IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU(
             rcau,
@@ -121,7 +121,7 @@ contract RecurringCollectorUpgradeTest is RecurringCollectorSharedTest {
         _authorizeAndAccept(rca, signerKey);
 
         boundSkipCeil(unboundedUpgradeSkip, type(uint256).max);
-        rcau.upgradeDeadline = boundTimestampMin(rcau.upgradeDeadline, block.timestamp + 1);
+        rcau.deadline = boundTimestampMin(rcau.deadline, block.timestamp + 1);
         IRecurringCollector.SignedRCAU memory signedRCAU = _recurringCollectorHelper.generateSignedRCAU(
             rcau,
             signerKey

packages/subgraph-service/contracts/SubgraphService.sol

@@ -593,7 +593,7 @@ contract SubgraphService is
     {
         require(
             signedRCA.rca.dataService == address(this),
-            SubgraphServiceIndexingAgreementDataServiceMismatch(signedRCA.rca.dataService)
+            SubgraphServiceIndexingAgreementWrongDataService(signedRCA.rca.dataService)
         );
 
         AcceptIndexingAgreementMetadata memory metadata = _decodeRCAMetadata(signedRCA.rca.metadata);