Skip to content

Commit e85c045

Browse files
committed
fix(collector): validate offer terms against deadline, not block.timestamp
Collection-window and duration checks now use the offer's acceptance deadline as the reference point instead of `block.timestamp`, making validation time-independent: if terms pass here they remain valid for any acceptance on or before `deadline`. Callers still enforce `block.timestamp <= deadline` at the acceptance entry point. - `_requireValidCollectionWindowParams` takes a `_deadline` parameter and becomes `pure`. `_endsAt > block.timestamp` becomes `_deadline < _endsAt`; `_endsAt - block.timestamp >= min + WINDOW` becomes `min + WINDOW <= _endsAt - _deadline`. - `_requireValidTerms` propagates `_deadline` to the window check. - Accept/update call sites pass the RCA/RCAU deadline. - Interface: replace `RecurringCollectorAgreementElapsedEndsAt` with `RecurringCollectorAgreementEndsBeforeDeadline(deadline, endsAt)`. Prerequisite for hash-keyed terms storage, where a single stored hash must remain validatable without re-checking against wall clock on every read.
1 parent 1ce36ab commit e85c045

3 files changed

Lines changed: 30 additions & 22 deletions

File tree

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ contract RecurringCollector is
274274
);
275275

276276
_requireValidTerms(
277-
_rca.endsAt, _rca.minSecondsPerCollection, _rca.maxSecondsPerCollection,
277+
_rca.deadline, _rca.endsAt, _rca.minSecondsPerCollection, _rca.maxSecondsPerCollection,
278278
_rca.payer, _rca.conditions, _rca.maxOngoingTokensPerSecond
279279
);
280280

@@ -809,18 +809,23 @@ contract RecurringCollector is
809809

810810
/**
811811
* @notice Requires that the collection window parameters are valid.
812-
*
812+
* @dev Validated against `_deadline` (the offer's acceptance deadline) rather than
813+
* `block.timestamp`, making this check time-independent: if terms pass here they remain
814+
* valid for any acceptance that happens on or before `_deadline`. Callers must enforce
815+
* `block.timestamp <= _deadline` at the acceptance entry point.
816+
* @param _deadline The offer's acceptance deadline
813817
* @param _endsAt The end time of the agreement
814818
* @param _minSecondsPerCollection The minimum seconds per collection
815819
* @param _maxSecondsPerCollection The maximum seconds per collection
816820
*/
817821
function _requireValidCollectionWindowParams(
822+
uint64 _deadline,
818823
uint64 _endsAt,
819824
uint32 _minSecondsPerCollection,
820825
uint32 _maxSecondsPerCollection
821-
) private view {
822-
// Agreement needs to end in the future
823-
require(_endsAt > block.timestamp, RecurringCollectorAgreementElapsedEndsAt(block.timestamp, _endsAt));
826+
) private pure {
827+
// Agreement must end after the deadline
828+
require(_deadline < _endsAt, RecurringCollectorAgreementEndsBeforeDeadline(_deadline, _endsAt));
824829

825830
// Collection window needs to be at least MIN_SECONDS_COLLECTION_WINDOW
826831
require(
@@ -833,19 +838,21 @@ contract RecurringCollector is
833838
)
834839
);
835840

836-
// Agreement needs to last at least one min collection window
841+
// Even if accepted at the deadline at least one min collection window must remain
837842
require(
838-
_endsAt - block.timestamp >= _minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
843+
_minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW <= _endsAt - _deadline,
839844
RecurringCollectorAgreementInvalidDuration(
840845
_minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW,
841-
_endsAt - block.timestamp
846+
_endsAt - _deadline
842847
)
843848
);
844849
}
845850

846851
/**
847852
* @notice Validates offer terms: collection window, eligibility support, and overflow.
848-
* @dev Called by _validateAndStoreAgreement and _validateAndStoreUpdate.
853+
* @dev Called by _validateAndStoreAgreement and _validateAndStoreUpdate. Time-independent —
854+
* validates against the offer's deadline so the check is stable across the offer's lifetime.
855+
* @param _deadline The offer's acceptance deadline
849856
* @param _endsAt The end time of the agreement
850857
* @param _minSecondsPerCollection The minimum seconds per collection
851858
* @param _maxSecondsPerCollection The maximum seconds per collection
@@ -854,14 +861,15 @@ contract RecurringCollector is
854861
* @param _maxOngoingTokensPerSecond The maximum ongoing tokens per second
855862
*/
856863
function _requireValidTerms(
864+
uint64 _deadline,
857865
uint64 _endsAt,
858866
uint32 _minSecondsPerCollection,
859867
uint32 _maxSecondsPerCollection,
860868
address _payer,
861869
uint16 _conditions,
862870
uint256 _maxOngoingTokensPerSecond
863871
) private view {
864-
_requireValidCollectionWindowParams(_endsAt, _minSecondsPerCollection, _maxSecondsPerCollection);
872+
_requireValidCollectionWindowParams(_deadline, _endsAt, _minSecondsPerCollection, _maxSecondsPerCollection);
865873
_requirePayerToSupportEligibilityCheck(_payer, _conditions);
866874
// Reverts on overflow — rejecting excessive terms that could prevent collection
867875
_maxOngoingTokensPerSecond * _maxSecondsPerCollection * 1024;
@@ -1058,7 +1066,7 @@ contract RecurringCollector is
10581066
RecurringCollectorStorage storage $ = _getStorage();
10591067

10601068
_requireValidTerms(
1061-
_rcau.endsAt, _rcau.minSecondsPerCollection, _rcau.maxSecondsPerCollection,
1069+
_rcau.deadline, _rcau.endsAt, _rcau.minSecondsPerCollection, _rcau.maxSecondsPerCollection,
10621070
_agreement.payer, _rcau.conditions, _rcau.maxOngoingTokensPerSecond
10631071
);
10641072

packages/horizon/test/unit/payments/recurring-collector/acceptValidation.t.sol

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,20 +69,20 @@ contract RecurringCollectorAcceptValidationTest is RecurringCollectorSharedTest
6969
_recurringCollector.accept(rca, signature);
7070
}
7171

72-
// ==================== endsAt validation (L545) ====================
72+
// ==================== endsAt validation ====================
7373

74-
function test_Accept_Revert_WhenEndsAtInPast() public {
74+
function test_Accept_Revert_WhenEndsAtNotAfterDeadline() public {
7575
IRecurringCollector.RecurringCollectionAgreement memory rca = _makeValidRCA();
76-
rca.endsAt = uint64(block.timestamp); // endsAt == now, fails "endsAt > block.timestamp"
76+
rca.endsAt = rca.deadline; // endsAt == deadline, fails "endsAt > deadline"
7777

7878
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, SIGNER_KEY);
7979
(, bytes memory signature) = _recurringCollectorHelper.generateSignedRCA(rca, SIGNER_KEY);
8080
_setupValidProvision(rca.serviceProvider, rca.dataService);
8181

8282
vm.expectRevert(
8383
abi.encodeWithSelector(
84-
IRecurringCollector.RecurringCollectorAgreementElapsedEndsAt.selector,
85-
block.timestamp,
84+
IRecurringCollector.RecurringCollectorAgreementEndsBeforeDeadline.selector,
85+
rca.deadline,
8686
rca.endsAt
8787
)
8888
);
@@ -142,12 +142,12 @@ contract RecurringCollectorAcceptValidationTest is RecurringCollectorSharedTest
142142

143143
function test_Accept_Revert_WhenDurationTooShort() public {
144144
IRecurringCollector.RecurringCollectionAgreement memory rca = _makeValidRCA();
145-
// Need: endsAt - now >= minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW
145+
// Need: endsAt - deadline >= minSecondsPerCollection + MIN_SECONDS_COLLECTION_WINDOW
146146
// Set duration just under the minimum
147147
uint32 minWindow = _recurringCollector.MIN_SECONDS_COLLECTION_WINDOW();
148148
rca.minSecondsPerCollection = 600;
149149
rca.maxSecondsPerCollection = 600 + minWindow; // valid window
150-
rca.endsAt = uint64(block.timestamp + rca.minSecondsPerCollection + minWindow - 1); // 1 second too short
150+
rca.endsAt = rca.deadline + rca.minSecondsPerCollection + minWindow - 1; // 1 second too short
151151

152152
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, SIGNER_KEY);
153153
(, bytes memory signature) = _recurringCollectorHelper.generateSignedRCA(rca, SIGNER_KEY);
@@ -157,7 +157,7 @@ contract RecurringCollectorAcceptValidationTest is RecurringCollectorSharedTest
157157
abi.encodeWithSelector(
158158
IRecurringCollector.RecurringCollectorAgreementInvalidDuration.selector,
159159
rca.minSecondsPerCollection + minWindow,
160-
rca.endsAt - block.timestamp
160+
uint256(rca.endsAt - rca.deadline)
161161
)
162162
);
163163
vm.prank(rca.dataService);

packages/interfaces/contracts/horizon/IRecurringCollector.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,11 +316,11 @@ interface IRecurringCollector is IAuthorizable, IAgreementCollector {
316316
error RecurringCollectorAgreementAddressNotSet();
317317

318318
/**
319-
* @notice Thrown when accepting or upgrading an agreement with an elapsed endsAt
320-
* @param currentTimestamp The current timestamp
319+
* @notice Thrown when an agreement's endsAt is not strictly after its acceptance deadline.
320+
* @param deadline The offer acceptance deadline
321321
* @param endsAt The agreement end timestamp
322322
*/
323-
error RecurringCollectorAgreementElapsedEndsAt(uint256 currentTimestamp, uint64 endsAt);
323+
error RecurringCollectorAgreementEndsBeforeDeadline(uint64 deadline, uint64 endsAt);
324324

325325
/**
326326
* @notice Thrown when accepting or upgrading an agreement with an elapsed endsAt

0 commit comments

Comments
 (0)