Skip to content

Commit 8e15c54

Browse files
matiasedgeandnodematiasedgeandnode
committed
f - allow collection on gateway cancelation
Plus major refactor of testing harness
1 parent a38e617 commit 8e15c54

File tree

18 files changed

+832
-643
lines changed

18 files changed

+832
-643
lines changed

‎IndexingPaymentsTodo.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# Still pending
22

33
* Arbitration Charter: Update to support disputing IndexingFee.
4-
* Double check cancelation policy. Who can cancel when? Right now is either party at any time. If gateway cancels allow collection till that point.
54
* Expose a function that indexers can use to calculate the tokens to be collected and other collection params?
65
* test_SubgraphService_CollectIndexingFee_Integration fails with PaymentsEscrowInconsistentCollection
76
* Switch timestamps to uint64.
@@ -15,6 +14,7 @@
1514

1615
# Done
1716

17+
* DONE: ~~Double check cancelation policy. Who can cancel when? Right now is either party at any time. Answer: If gateway cancels allow collection till that point.~~
1818
* DONE: ~~If an indexer closes an allocation, what should happen to the accepeted agreement? Answer: Look into canceling agreement as part of stop service.~~
1919
* DONE: ~~Switch `duration` for `endsAt`? Answer: Do it.~~
2020
* 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.~~

‎packages/horizon/contracts/interfaces/IRecurringCollector.sol

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ import { IAuthorizable } from "./IAuthorizable.sol";
1313
* recurrent payments.
1414
*/
1515
interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
16+
enum AgreementState {
17+
NotAccepted,
18+
Accepted,
19+
CanceledByServiceProvider,
20+
CanceledByPayer
21+
}
22+
23+
enum CancelAgreementBy {
24+
ServiceProvider,
25+
Payer
26+
}
27+
1628
/// @notice A representation of a signed Recurring Collection Agreement (RCA)
1729
struct SignedRCA {
1830
// The RCA
@@ -102,6 +114,10 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
102114
uint32 minSecondsPerCollection;
103115
// The maximum amount of seconds that can pass between collections
104116
uint32 maxSecondsPerCollection;
117+
// The timestamp when the agreement was canceled
118+
uint256 canceledAt;
119+
// The state of the agreement
120+
AgreementState state;
105121
}
106122

107123
/// @notice The params for collecting an agreement
@@ -152,7 +168,8 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
152168
address indexed payer,
153169
address indexed serviceProvider,
154170
bytes16 agreementId,
155-
uint256 canceledAt
171+
uint256 canceledAt,
172+
address canceledBy
156173
);
157174

158175
/**
@@ -240,22 +257,11 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
240257
error RecurringCollectorInvalidCollectData(bytes invalidData);
241258

242259
/**
243-
* Thrown when calling accept() for an already accepted agreement
244-
* @param agreementId The agreement ID
245-
*/
246-
error RecurringCollectorAgreementAlreadyAccepted(bytes16 agreementId);
247-
248-
/**
249-
* Thrown when interacting with an agreement that was never accepted
250-
* @param agreementId The agreement ID
251-
*/
252-
error RecurringCollectorAgreementNeverAccepted(bytes16 agreementId);
253-
254-
/**
255-
* Thrown when interacting with an agreement that was canceled
260+
* Thrown when interacting with an agreement that has an incorrect state
256261
* @param agreementId The agreement ID
262+
* @param incorrectState The incorrect state
257263
*/
258-
error RecurringCollectorAgreementCanceled(bytes16 agreementId);
264+
error RecurringCollectorAgreementIncorrectState(bytes16 agreementId, AgreementState incorrectState);
259265

260266
/**
261267
* Thrown when accepting or upgrading an agreement with invalid parameters
@@ -294,8 +300,9 @@ interface IRecurringCollector is IAuthorizable, IPaymentsCollector {
294300
/**
295301
* @dev Cancel an indexing agreement.
296302
* @param agreementId The agreement's ID.
303+
* @param by The party that is canceling the agreement.
297304
*/
298-
function cancel(bytes16 agreementId) external;
305+
function cancel(bytes16 agreementId, CancelAgreementBy by) external;
299306

300307
/**
301308
* @dev Upgrade an indexing agreement.

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

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
3333
"RecurringCollectionAgreementUpgrade(bytes16 agreementId,uint256 deadline,uint256 endsAt,uint256 maxInitialTokens,uint256 maxOngoingTokensPerSecond,uint32 minSecondsPerCollection,uint32 maxSecondsPerCollection,bytes metadata)"
3434
);
3535

36-
/// @notice Sentinel value to indicate an agreement has been canceled
37-
uint256 public constant CANCELED = type(uint256).max;
38-
3936
/// @notice Tracks agreements
4037
mapping(bytes16 agreementId => AgreementData data) public agreements;
4138

@@ -91,10 +88,14 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
9188

9289
AgreementData storage agreement = _getForUpdateAgreement(signedRCA.rca.agreementId);
9390
// check that the agreement is not already accepted
94-
require(agreement.acceptedAt == 0, RecurringCollectorAgreementAlreadyAccepted(signedRCA.rca.agreementId));
91+
require(
92+
agreement.state == AgreementState.NotAccepted,
93+
RecurringCollectorAgreementIncorrectState(signedRCA.rca.agreementId, agreement.state)
94+
);
9595

9696
// accept the agreement
9797
agreement.acceptedAt = block.timestamp;
98+
agreement.state = AgreementState.Accepted;
9899
agreement.dataService = signedRCA.rca.dataService;
99100
agreement.payer = signedRCA.rca.payer;
100101
agreement.serviceProvider = signedRCA.rca.serviceProvider;
@@ -124,21 +125,35 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
124125
* See {IRecurringCollector.cancel}.
125126
* @dev Caller must be the data service for the agreement.
126127
*/
127-
function cancel(bytes16 agreementId) external {
128+
function cancel(bytes16 agreementId, CancelAgreementBy by) external {
128129
AgreementData storage agreement = _getForUpdateAgreement(agreementId);
129-
require(agreement.acceptedAt > 0, RecurringCollectorAgreementNeverAccepted(agreementId));
130+
require(
131+
agreement.state == AgreementState.Accepted,
132+
RecurringCollectorAgreementIncorrectState(agreementId, agreement.state)
133+
);
130134
require(
131135
agreement.dataService == msg.sender,
132136
RecurringCollectorDataServiceNotAuthorized(agreementId, msg.sender)
133137
);
134-
agreement.acceptedAt = CANCELED;
138+
agreement.canceledAt = block.timestamp;
139+
address canceledBy;
140+
if (by == CancelAgreementBy.Payer) {
141+
agreement.state = AgreementState.CanceledByPayer;
142+
canceledBy = agreement.payer;
143+
} else if (by == CancelAgreementBy.ServiceProvider) {
144+
agreement.state = AgreementState.CanceledByServiceProvider;
145+
canceledBy = agreement.serviceProvider;
146+
} else {
147+
revert("invalid CancelAgreementBy");
148+
}
135149

136150
emit AgreementCanceled(
137151
agreement.dataService,
138152
agreement.payer,
139153
agreement.serviceProvider,
140154
agreementId,
141-
block.timestamp
155+
block.timestamp,
156+
canceledBy
142157
);
143158
}
144159

@@ -154,7 +169,10 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
154169
);
155170

156171
AgreementData storage agreement = _getForUpdateAgreement(signedRCAU.rcau.agreementId);
157-
require(agreement.acceptedAt > 0, RecurringCollectorAgreementNeverAccepted(signedRCAU.rcau.agreementId));
172+
require(
173+
agreement.state == AgreementState.Accepted,
174+
RecurringCollectorAgreementIncorrectState(signedRCAU.rcau.agreementId, agreement.state)
175+
);
158176
require(
159177
agreement.dataService == msg.sender,
160178
RecurringCollectorDataServiceNotAuthorized(signedRCAU.rcau.agreementId, msg.sender)
@@ -238,13 +256,20 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
238256
*/
239257
function _collect(CollectParams memory _params) private returns (uint256) {
240258
AgreementData storage agreement = _getForUpdateAgreement(_params.agreementId);
241-
require(agreement.acceptedAt > 0, RecurringCollectorAgreementNeverAccepted(_params.agreementId));
259+
require(
260+
agreement.state == AgreementState.Accepted || agreement.state == AgreementState.CanceledByPayer,
261+
RecurringCollectorAgreementIncorrectState(_params.agreementId, agreement.state)
262+
);
263+
242264
require(
243265
msg.sender == agreement.dataService,
244266
RecurringCollectorDataServiceNotAuthorized(_params.agreementId, msg.sender)
245267
);
246268

247-
_requireCollectableAgreement(agreement, _params.agreementId);
269+
require(
270+
agreement.endsAt >= block.timestamp,
271+
RecurringCollectorAgreementElapsed(_params.agreementId, agreement.endsAt)
272+
);
248273

249274
uint256 tokensToCollect = 0;
250275
if (_params.tokens != 0) {
@@ -311,15 +336,6 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
311336
);
312337
}
313338

314-
function _requireCollectableAgreement(AgreementData memory _agreement, bytes16 _agreementId) private view {
315-
require(_agreement.acceptedAt != CANCELED, RecurringCollectorAgreementCanceled(_agreementId));
316-
317-
require(
318-
_agreement.endsAt >= block.timestamp,
319-
RecurringCollectorAgreementElapsed(_agreementId, _agreement.endsAt)
320-
);
321-
}
322-
323339
/**
324340
* @notice Requires that the collection params are valid.
325341
*/
@@ -328,7 +344,9 @@ contract RecurringCollector is EIP712, GraphDirectory, Authorizable, IRecurringC
328344
bytes16 _agreementId,
329345
uint256 _tokens
330346
) private view returns (uint256) {
331-
uint256 collectionSeconds = block.timestamp;
347+
uint256 collectionSeconds = _agreement.state == AgreementState.CanceledByPayer
348+
? _agreement.canceledAt
349+
: block.timestamp;
332350
collectionSeconds -= _agreementCollectionStartAt(_agreement);
333351
require(
334352
collectionSeconds >= _agreement.minSecondsPerCollection,

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,79 @@ contract RecurringCollectorHelper is AuthorizableHelper, Bounder {
5959
rca.deadline = boundTimestampMin(rca.deadline, block.timestamp);
6060
return rca;
6161
}
62+
63+
function sensibleRCA(
64+
IRecurringCollector.RecurringCollectionAgreement memory rca
65+
) public view returns (IRecurringCollector.RecurringCollectionAgreement memory) {
66+
vm.assume(rca.agreementId != bytes16(0));
67+
vm.assume(rca.dataService != address(0));
68+
vm.assume(rca.payer != address(0));
69+
vm.assume(rca.serviceProvider != address(0));
70+
71+
rca.minSecondsPerCollection = _sensibleMinSecondsPerCollection(rca.minSecondsPerCollection);
72+
rca.maxSecondsPerCollection = _sensibleMaxSecondsPerCollection(
73+
rca.maxSecondsPerCollection,
74+
rca.minSecondsPerCollection
75+
);
76+
77+
rca.deadline = _sensibleDeadline(rca.deadline);
78+
rca.endsAt = _sensibleEndsAt(rca.endsAt, rca.maxSecondsPerCollection);
79+
80+
rca.maxInitialTokens = _sensibleMaxInitialTokens(rca.maxInitialTokens);
81+
rca.maxOngoingTokensPerSecond = _sensibleMaxOngoingTokensPerSecond(rca.maxOngoingTokensPerSecond);
82+
83+
return rca;
84+
}
85+
86+
function sensibleRCAU(
87+
IRecurringCollector.RecurringCollectionAgreementUpgrade memory rcau
88+
) public view returns (IRecurringCollector.RecurringCollectionAgreementUpgrade memory) {
89+
rcau.minSecondsPerCollection = _sensibleMinSecondsPerCollection(rcau.minSecondsPerCollection);
90+
rcau.maxSecondsPerCollection = _sensibleMaxSecondsPerCollection(
91+
rcau.maxSecondsPerCollection,
92+
rcau.minSecondsPerCollection
93+
);
94+
95+
rcau.deadline = _sensibleDeadline(rcau.deadline);
96+
rcau.endsAt = _sensibleEndsAt(rcau.endsAt, rcau.maxSecondsPerCollection);
97+
rcau.maxInitialTokens = _sensibleMaxInitialTokens(rcau.maxInitialTokens);
98+
rcau.maxOngoingTokensPerSecond = _sensibleMaxOngoingTokensPerSecond(rcau.maxOngoingTokensPerSecond);
99+
100+
return rcau;
101+
}
102+
103+
function _sensibleDeadline(uint256 _seed) internal view returns (uint256) {
104+
return bound(_seed, block.timestamp + 1, block.timestamp + 7200); // between now and 2h
105+
}
106+
107+
function _sensibleEndsAt(uint256 _seed, uint32 _maxSecondsPerCollection) internal view returns (uint256) {
108+
return
109+
bound(
110+
_seed,
111+
block.timestamp + (10 * uint256(_maxSecondsPerCollection)),
112+
block.timestamp + (1_000_000 * uint256(_maxSecondsPerCollection))
113+
); // between 10 and 1M max collections
114+
}
115+
116+
function _sensibleMaxInitialTokens(uint256 _seed) internal pure returns (uint256) {
117+
return bound(_seed, 0, 1e18 * 100_000_000); // between 0 and 100M tokens
118+
}
119+
120+
function _sensibleMaxOngoingTokensPerSecond(uint256 _seed) internal pure returns (uint256) {
121+
return bound(_seed, 1, 1e18); // between 1 and 1e18 tokens per second
122+
}
123+
124+
function _sensibleMinSecondsPerCollection(uint32 _seed) internal pure returns (uint32) {
125+
return uint32(bound(_seed, 10 * 60, 24 * 60 * 60)); // between 10 min and 24h
126+
}
127+
128+
function _sensibleMaxSecondsPerCollection(
129+
uint32 _seed,
130+
uint32 _minSecondsPerCollection
131+
) internal pure returns (uint32) {
132+
return
133+
uint32(
134+
bound(_seed, _minSecondsPerCollection + 7200, 60 * 60 * 24 * 30) // between minSecondsPerCollection + 2h and 30 days
135+
);
136+
}
62137
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ contract RecurringCollectorAcceptTest is RecurringCollectorSharedTest {
3737
(IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzyTestAccept);
3838

3939
bytes memory expectedErr = abi.encodeWithSelector(
40-
IRecurringCollector.RecurringCollectorAgreementAlreadyAccepted.selector,
41-
accepted.rca.agreementId
40+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
41+
accepted.rca.agreementId,
42+
IRecurringCollector.AgreementState.Accepted
4243
);
4344
vm.expectRevert(expectedErr);
4445
vm.prank(accepted.rca.dataService);

‎packages/horizon/test/payments/recurring-collector/cancel.t.sol

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,28 @@ contract RecurringCollectorCancelTest is RecurringCollectorSharedTest {
1212

1313
/* solhint-disable graph/func-name-mixedcase */
1414

15-
function test_Cancel(FuzzyTestAccept calldata fuzzyTestAccept) public {
15+
function test_Cancel(FuzzyTestAccept calldata fuzzyTestAccept, uint8 unboundedCanceler) public {
1616
_sensibleAuthorizeAndAccept(fuzzyTestAccept);
17-
_cancel(fuzzyTestAccept.rca);
17+
_cancel(fuzzyTestAccept.rca, _fuzzyCancelAgreementBy(unboundedCanceler));
1818
}
1919

2020
function test_Cancel_Revert_WhenNotAccepted(
21-
IRecurringCollector.RecurringCollectionAgreement memory fuzzyRCA
21+
IRecurringCollector.RecurringCollectionAgreement memory fuzzyRCA,
22+
uint8 unboundedCanceler
2223
) public {
2324
bytes memory expectedErr = abi.encodeWithSelector(
24-
IRecurringCollector.RecurringCollectorAgreementNeverAccepted.selector,
25-
fuzzyRCA.agreementId
25+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
26+
fuzzyRCA.agreementId,
27+
IRecurringCollector.AgreementState.NotAccepted
2628
);
2729
vm.expectRevert(expectedErr);
2830
vm.prank(fuzzyRCA.dataService);
29-
_recurringCollector.cancel(fuzzyRCA.agreementId);
31+
_recurringCollector.cancel(fuzzyRCA.agreementId, _fuzzyCancelAgreementBy(unboundedCanceler));
3032
}
3133

3234
function test_Cancel_Revert_WhenNotDataService(
3335
FuzzyTestAccept calldata fuzzyTestAccept,
36+
uint8 unboundedCanceler,
3437
address notDataService
3538
) public {
3639
vm.assume(fuzzyTestAccept.rca.dataService != notDataService);
@@ -44,7 +47,7 @@ contract RecurringCollectorCancelTest is RecurringCollectorSharedTest {
4447
);
4548
vm.expectRevert(expectedErr);
4649
vm.prank(notDataService);
47-
_recurringCollector.cancel(fuzzyTestAccept.rca.agreementId);
50+
_recurringCollector.cancel(fuzzyTestAccept.rca.agreementId, _fuzzyCancelAgreementBy(unboundedCanceler));
4851
}
4952
/* solhint-enable graph/func-name-mixedcase */
5053
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,18 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest {
7272
bytes memory data = _generateCollectData(fuzzy.collectParams);
7373

7474
bytes memory expectedErr = abi.encodeWithSelector(
75-
IRecurringCollector.RecurringCollectorAgreementNeverAccepted.selector,
76-
fuzzy.collectParams.agreementId
75+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
76+
fuzzy.collectParams.agreementId,
77+
IRecurringCollector.AgreementState.NotAccepted
7778
);
7879
vm.expectRevert(expectedErr);
7980
vm.prank(dataService);
8081
_recurringCollector.collect(IGraphPayments.PaymentTypes.IndexingFee, data);
8182
}
8283

83-
function test_Collect_Revert_WhenCanceledAgreement(FuzzyTestCollect calldata fuzzy) public {
84+
function test_Collect_Revert_WhenCanceledAgreementByServiceProvider(FuzzyTestCollect calldata fuzzy) public {
8485
(IRecurringCollector.SignedRCA memory accepted, ) = _sensibleAuthorizeAndAccept(fuzzy.fuzzyTestAccept);
85-
_cancel(accepted.rca);
86+
_cancel(accepted.rca, IRecurringCollector.CancelAgreementBy.ServiceProvider);
8687
IRecurringCollector.CollectParams memory collectData = fuzzy.collectParams;
8788
collectData.tokens = bound(collectData.tokens, 1, type(uint256).max);
8889
IRecurringCollector.CollectParams memory collectParams = _generateCollectParams(
@@ -94,8 +95,9 @@ contract RecurringCollectorCollectTest is RecurringCollectorSharedTest {
9495
bytes memory data = _generateCollectData(collectParams);
9596

9697
bytes memory expectedErr = abi.encodeWithSelector(
97-
IRecurringCollector.RecurringCollectorAgreementCanceled.selector,
98-
collectParams.agreementId
98+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
99+
collectParams.agreementId,
100+
IRecurringCollector.AgreementState.CanceledByServiceProvider
99101
);
100102
vm.expectRevert(expectedErr);
101103
vm.prank(accepted.rca.dataService);

0 commit comments

Comments
 (0)