f - a bunch of updates

Author: matiasedgeandnode

IndexingPaymentsTodo.md

@@ -1,25 +1,33 @@
 # Still pending
 
 * Arbitration Charter: Update to support disputing IndexingFee.
-* Economics
-  * If service wants to collect more than collector allows. Collector limits but doesn't tell the service?
-  * Since an allocation is required for collecting, do we want to expect that the allocation is not stale? Do we want to add code to collect rewards as part of the collection of fees? Make sure allocation is more than one epoch old if we attempt this.
-  * What should happen if the escrow doesn't have enough funds?
-  * Don't pay for entities on initial collection? Where did we land in terms of payment terms?
-  * Should we set a different param for initial collection time max? Some subgraphs take a lot to catch up.
-  * How do we solve for the case where an indexer has reached their max expected payout for the initial sync but haven't reached the current epoch (thus their POI is incorrect)?
-* Double check cancelation policy. Who can cancel when? Right now is either party at any time.
+* Double check cancelation policy. Who can cancel when? Right now is either party at any time. If gateway cancels allow collection till that point.
 * Expose a function that indexers can use to calculate the tokens to be collected and other collection params?
-* 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
-* Switch `duration` for `endsAt`?
+* Switch timestamps to uint64.
 * Check that UUID-v4 fits in `bytes16`
-* Test `upgrade` paths
-* Test lock stake
+* Check code coverage
+* Check contract size
+* Add upgrade path to v2 collector terms
+* It's more like a collect + cancel since the indexer is expected to stop work then and there. When posting a POI that's < N-1 epoch. Answer: Emit signal that the collection is meant to be final.
+* Don't love cancel agreement on stop service / close stale allocation.
+* Place all agreement terms into one struct
 
 # Done
 
+* DONE: ~~If an indexer closes an allocation, what should happen to the accepeted agreement? Answer: Look into canceling agreement as part of stop service.~~
+* DONE: ~~Switch `duration` for `endsAt`? Answer: Do it.~~
+* DONE: ~~Support a way for gateway to shop an agreement around? Deadline + dedup key? So only one agreement with the dedupe key can be accepted? Answer: No. Agreements will be "signaled" as approved or rejected on the API call that sends the agreement. We'll trust (and verify) that that's the case.~~
+* DONE: ~~Test `upgrade` paths~~
+* DONE: ~~Fix upgrade.t.sol, lots of comments~~
+* DONE: ~~How do we solve for the case where an indexer has reached their max expected payout for the initial sync but haven't reached the current epoch (thus their POI is incorrect)? Answer: Signal in the event that the max amount was collected, so that fisherman understand the case.~~
+* DONE: ~~Debate epoch check protocol team. Maybe don't revert but store it in event. Pablo suggest block number instead of epoch.~~
+* DONE: ~~Should we set a different param for initial collection time max? Some subgraphs take a lot to catch up. Answer: Do nothing. Make sure that zero POIs allow to eventually sync~~
+* DONE: ~~Since an allocation is required for collecting, do we want to expect that the allocation is not stale? Do we want to add code to collect rewards as part of the collection of fees? Make sure allocation is more than one epoch old if we attempt this. Answer: Ignore stale allocation~~
+* DONE: ~~If service wants to collect more than collector allows. Collector limits but doesn't tell the service? Currently reverts. Answer: Allow for max allowed~~
+* DONE: ~~What should happen if the escrow doesn't have enough funds? Answer: Reverts~~
+* DONE: ~~Don't pay for entities on initial collection? Where did we land in terms of payment terms? Answer: pay initial~~
+* DONE: ~~Test lock stake~~
 * 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.~~

packages/horizon/contracts/interfaces/IRecurringCollector.sol

@@ -27,8 +27,8 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         bytes16 agreementId;
         // The deadline for accepting the RCA
         uint256 deadline;
-        // The duration of the RCA in seconds
-        uint256 duration;
+        // The timestamp when the agreement ends
+        uint256 endsAt;
         // The address of the payer the RCA was issued by
         address payer;
         // The address of the data service the RCA was issued to
@@ -62,8 +62,8 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         bytes16 agreementId;
         // The deadline for upgrading
         uint256 deadline;
-        // The duration of the agreement in seconds
-        uint256 duration;
+        // The timestamp when the agreement ends
+        uint256 endsAt;
         // The maximum amount of tokens that can be collected in the first collection
         // on top of the amount allowed for subsequent collections
         uint256 maxInitialTokens;
@@ -90,8 +90,8 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         uint256 acceptedAt;
         // The timestamp when the agreement was last collected at
         uint256 lastCollectionAt;
-        // The duration of the agreement in seconds
-        uint256 duration;
+        // The timestamp when the agreement ends
+        uint256 endsAt;
         // The maximum amount of tokens that can be collected in the first collection
         // on top of the amount allowed for subsequent collections
         uint256 maxInitialTokens;
@@ -120,14 +120,21 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      * @param dataService The address of the data service
      * @param payer The address of the payer
      * @param serviceProvider The address of the service provider
+     * @param agreementId The agreement ID
+     * @param acceptedAt The timestamp when the agreement was accepted
+     * @param endsAt The timestamp when the agreement ends
+     * @param maxInitialTokens The maximum amount of tokens that can be collected in the first collection
+     * @param maxOngoingTokensPerSecond The maximum amount of tokens that can be collected per second
+     * @param minSecondsPerCollection The minimum amount of seconds that must pass between collections
+     * @param maxSecondsPerCollection The maximum amount of seconds that can pass between collections
      */
     event AgreementAccepted(
         address indexed dataService,
         address indexed payer,
         address indexed serviceProvider,
         bytes16 agreementId,
         uint256 acceptedAt,
-        uint256 duration,
+        uint256 endsAt,
         uint256 maxInitialTokens,
         uint256 maxOngoingTokensPerSecond,
         uint32 minSecondsPerCollection,
@@ -153,14 +160,21 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      * @param dataService The address of the data service
      * @param payer The address of the payer
      * @param serviceProvider The address of the service provider
+     * @param agreementId The agreement ID
+     * @param upgradedAt The timestamp when the agreement was upgraded
+     * @param endsAt The timestamp when the agreement ends
+     * @param maxInitialTokens The maximum amount of tokens that can be collected in the first collection
+     * @param maxOngoingTokensPerSecond The maximum amount of tokens that can be collected per second
+     * @param minSecondsPerCollection The minimum amount of seconds that must pass between collections
+     * @param maxSecondsPerCollection The maximum amount of seconds that can pass between collections
      */
     event AgreementUpgraded(
         address indexed dataService,
         address indexed payer,
         address indexed serviceProvider,
         bytes16 agreementId,
         uint256 upgradedAt,
-        uint256 duration,
+        uint256 endsAt,
         uint256 maxInitialTokens,
         uint256 maxOngoingTokensPerSecond,
         uint32 minSecondsPerCollection,
@@ -183,6 +197,11 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
         uint256 dataServiceCut
     );
 
+    /**
+     * Thrown when accepting an agreement with a zero ID
+     */
+    error RecurringCollectorAgreementIdZero();
+
     /**
      * Thrown when interacting with an agreement not owned by the message sender
      * @param agreementId The agreement ID
@@ -241,14 +260,14 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
     /**
      * Thrown when accepting or upgrading an agreement with invalid parameters
      */
-    error RecurringCollectorAgreementInvalidParameters();
+    error RecurringCollectorAgreementInvalidParameters(string message);
 
     /**
      * Thrown when calling collect() on an elapsed agreement
      * @param agreementId The agreement ID
-     * @param agreementEnd The agreement end timestamp
+     * @param endsAt The agreement end timestamp
      */
-    error RecurringCollectorAgreementElapsed(bytes16 agreementId, uint256 agreementEnd);
+    error RecurringCollectorAgreementElapsed(bytes16 agreementId, uint256 endsAt);
 
     /**
      * Thrown when calling collect() too soon
@@ -266,14 +285,6 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
      */
     error RecurringCollectorCollectionTooLate(bytes16 agreementId, uint256 secondsSinceLast, uint256 maxSeconds);
 
-    /**
-     * Thrown when calling collect() too late
-     * @param agreementId The agreement ID
-     * @param tokens The amount of tokens to collect
-     * @param maxTokens The maximum amount of tokens allowed to collect
-     */
-    error RecurringCollectorCollectAmountTooHigh(bytes16 agreementId, uint256 tokens, uint256 maxTokens);
-
     /**
      * @dev Accept an indexing agreement.
      * @param signedRCA The signed Recurring Collection Agreement which is to be accepted.