Skip to content

Commit 3d7d423

Browse files
RembrandtKRembrandtK
committed
feat(collector): hash-keyed decoded terms with version-indexed views (TRST-L-11)
Restructure agreement storage around decoded terms keyed by EIP-712 hash. All internal reads access terms[hash] uniformly — no abi.decode, no offerType discrimination in read paths. Storage changes: - rcaOffers/rcauOffers → terms[hash] (single mapping, decoded fields) - AgreementData slimmed to identity + lifecycle + hash pointers (5 slots, was 7). Terms fields removed; getAgreement() returns the slim struct. - AgreementData.pendingTermsHash added for queued RCAU offers. Architecture: - _storeTerms: validates + stores + links (active or pending). Single write gate — every stored term is validated (invariant). - _termsFromRCA/_termsFromRCAU: type → AgreementTerms extraction. - _registerNew: composite for NEW path (register identity + store + link). Shared by offer(NEW) and accept(). Idempotent. - _activeClaimWindow + _maxClaimForTerms: replace 3-way state dispatch in max-claim calculation. Interface additions: - OfferCancelled event for SCOPE_PENDING cancellations. - AgreementData.pendingTermsHash.
1 parent e85c045 commit 3d7d423

15 files changed

Lines changed: 1039 additions & 375 deletions

File tree

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

Lines changed: 285 additions & 293 deletions
Large diffs are not rendered by default.

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

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
pragma solidity ^0.8.27;
33

44
import { IRecurringCollector } from "@graphprotocol/interfaces/contracts/horizon/IRecurringCollector.sol";
5+
import { OFFER_TYPE_NEW } from "@graphprotocol/interfaces/contracts/horizon/IAgreementCollector.sol";
56

67
import { RecurringCollectorSharedTest } from "./shared.t.sol";
78

@@ -65,5 +66,171 @@ contract RecurringCollectorAcceptTest is RecurringCollectorSharedTest {
6566
assertEq(vm.getRecordedLogs().length, 0, "no event emitted on idempotent re-accept");
6667
}
6768

69+
/// @notice Re-accepting an already-accepted RCA at the same hash must still succeed after
70+
/// the RCA's acceptance deadline has elapsed. The idempotent short-circuit runs before the
71+
/// deadline check so signature lifetime is not consumed — this is the path the SubgraphService
72+
/// relies on to rebind an agreement to a new allocation after the original acceptance window
73+
/// has closed.
74+
function test_Accept_Idempotent_AfterDeadline_SameHash(FuzzyTestAccept calldata fuzzyTestAccept) public {
75+
(
76+
IRecurringCollector.RecurringCollectionAgreement memory acceptedRca,
77+
bytes memory signature,
78+
,
79+
bytes16 agreementId
80+
) = _sensibleAuthorizeAndAccept(fuzzyTestAccept);
81+
82+
// Warp past the RCA's deadline — a fresh accept would now revert with
83+
// RecurringCollectorAgreementDeadlineElapsed.
84+
vm.warp(uint256(acceptedRca.deadline) + 1);
85+
86+
vm.recordLogs();
87+
vm.prank(acceptedRca.dataService);
88+
bytes16 returnedId = _recurringCollector.accept(acceptedRca, signature);
89+
assertEq(returnedId, agreementId, "returns the same agreementId");
90+
assertEq(vm.getRecordedLogs().length, 0, "no event emitted on idempotent re-accept after deadline");
91+
92+
// Sanity: the collector-side agreement is still in Accepted state, unchanged by the no-op.
93+
IRecurringCollector.AgreementData memory agreement = _recurringCollector.getAgreement(agreementId);
94+
assertEq(uint8(agreement.state), uint8(IRecurringCollector.AgreementState.Accepted));
95+
}
96+
97+
/// @notice A fresh accept (no prior offer()) stores terms via _validateAndStoreTerms, which must
98+
/// emit OfferStored. AgreementAccepted follows. Both events observable in order.
99+
function test_Accept_EmitsOfferStored_WhenFreshTerms(FuzzyTestAccept calldata fuzzyTestAccept) public {
100+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
101+
fuzzyTestAccept.rca
102+
);
103+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
104+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
105+
(, bytes memory signature) = _recurringCollectorHelper.generateSignedRCA(rca, signerKey);
106+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
107+
bytes16 agreementId = _recurringCollector.generateAgreementId(
108+
rca.payer,
109+
rca.dataService,
110+
rca.serviceProvider,
111+
rca.deadline,
112+
rca.nonce
113+
);
114+
_setupValidProvision(rca.serviceProvider, rca.dataService);
115+
116+
// OfferStored fires from _validateAndStoreTerms before _storeAgreement; AgreementAccepted
117+
// follows the state transition at the end of accept().
118+
vm.expectEmit(address(_recurringCollector));
119+
emit IRecurringCollector.OfferStored(agreementId, rca.payer, OFFER_TYPE_NEW, rcaHash);
120+
vm.expectEmit(address(_recurringCollector));
121+
emit IRecurringCollector.AgreementAccepted(
122+
rca.dataService,
123+
rca.payer,
124+
rca.serviceProvider,
125+
agreementId,
126+
rca.endsAt,
127+
rca.maxInitialTokens,
128+
rca.maxOngoingTokensPerSecond,
129+
rca.minSecondsPerCollection,
130+
rca.maxSecondsPerCollection
131+
);
132+
vm.prank(rca.dataService);
133+
_recurringCollector.accept(rca, signature);
134+
}
135+
136+
/// @notice A second RCA sharing the same agreementId seed (payer, dataService, serviceProvider,
137+
/// deadline, nonce) but with different other fields — so different rcaHash — must not be
138+
/// accepted against an already-Accepted agreement. The idempotent short-circuit only fires on
139+
/// exact hash match; everything else must fall through to the state guard and revert. Proves
140+
/// the short-circuit can't be abused as an overwrite path even in an imagined 128-bit
141+
/// agreementId collision.
142+
function test_Accept_Revert_WhenDifferentHashSameAgreementId(FuzzyTestAccept calldata fuzzyTestAccept) public {
143+
(
144+
IRecurringCollector.RecurringCollectionAgreement memory acceptedRca,
145+
,
146+
uint256 signerKey,
147+
bytes16 agreementId
148+
) = _sensibleAuthorizeAndAccept(fuzzyTestAccept);
149+
150+
// Snapshot the original hash before constructing the variant. `variant = acceptedRca` in
151+
// Solidity memory is a reference copy, so rebuild explicitly to vary one pricing field
152+
// while keeping the 5 agreementId-seed fields (payer, dataService, serviceProvider,
153+
// deadline, nonce) verbatim.
154+
bytes32 originalHash = _recurringCollector.hashRCA(acceptedRca);
155+
IRecurringCollector.RecurringCollectionAgreement memory variant = IRecurringCollector
156+
.RecurringCollectionAgreement({
157+
deadline: acceptedRca.deadline,
158+
endsAt: acceptedRca.endsAt,
159+
payer: acceptedRca.payer,
160+
dataService: acceptedRca.dataService,
161+
serviceProvider: acceptedRca.serviceProvider,
162+
maxInitialTokens: acceptedRca.maxInitialTokens + 1, // <-- vary
163+
maxOngoingTokensPerSecond: acceptedRca.maxOngoingTokensPerSecond,
164+
minSecondsPerCollection: acceptedRca.minSecondsPerCollection,
165+
maxSecondsPerCollection: acceptedRca.maxSecondsPerCollection,
166+
conditions: acceptedRca.conditions,
167+
nonce: acceptedRca.nonce,
168+
metadata: acceptedRca.metadata
169+
});
170+
171+
bytes32 variantHash = _recurringCollector.hashRCA(variant);
172+
assertTrue(originalHash != variantHash, "hashes must differ when any field differs");
173+
assertEq(
174+
_recurringCollector.generateAgreementId(
175+
variant.payer,
176+
variant.dataService,
177+
variant.serviceProvider,
178+
variant.deadline,
179+
variant.nonce
180+
),
181+
agreementId,
182+
"same agreementId seed yields same id"
183+
);
184+
185+
(, bytes memory variantSig) = _recurringCollectorHelper.generateSignedRCA(variant, signerKey);
186+
187+
// Short-circuit doesn't fire (hash differs); falls through to _storeAgreement's state guard.
188+
vm.expectRevert(
189+
abi.encodeWithSelector(
190+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
191+
agreementId,
192+
IRecurringCollector.AgreementState.Accepted
193+
)
194+
);
195+
vm.prank(acceptedRca.dataService);
196+
_recurringCollector.accept(variant, variantSig);
197+
198+
// Post-revert sanity: storage reflects the original, not the variant.
199+
IRecurringCollector.AgreementData memory agreement = _recurringCollector.getAgreement(agreementId);
200+
assertEq(agreement.activeTermsHash, originalHash, "activeTermsHash unchanged");
201+
}
202+
203+
/// @notice After a cancellation, re-accepting the same RCA at the same hash must revert — the
204+
/// short-circuit only fires when state == Accepted, so a cancelled agreement falls through to
205+
/// the NotAccepted state guard. Proves cancelled is terminal and the short-circuit cannot
206+
/// resurrect it.
207+
function test_Accept_Revert_AfterCancellation_SameHash(FuzzyTestAccept calldata fuzzyTestAccept) public {
208+
(
209+
IRecurringCollector.RecurringCollectionAgreement memory acceptedRca,
210+
bytes memory signature,
211+
,
212+
bytes16 agreementId
213+
) = _sensibleAuthorizeAndAccept(fuzzyTestAccept);
214+
215+
vm.prank(acceptedRca.dataService);
216+
_recurringCollector.cancel(agreementId, IRecurringCollector.CancelAgreementBy.ServiceProvider);
217+
218+
assertEq(
219+
uint8(_recurringCollector.getAgreement(agreementId).state),
220+
uint8(IRecurringCollector.AgreementState.CanceledByServiceProvider),
221+
"precondition: cancelled"
222+
);
223+
224+
vm.expectRevert(
225+
abi.encodeWithSelector(
226+
IRecurringCollector.RecurringCollectorAgreementIncorrectState.selector,
227+
agreementId,
228+
IRecurringCollector.AgreementState.CanceledByServiceProvider
229+
)
230+
);
231+
vm.prank(acceptedRca.dataService);
232+
_recurringCollector.accept(acceptedRca, signature);
233+
}
234+
68235
/* solhint-enable graph/func-name-mixedcase */
69236
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,79 @@ contract RecurringCollectorCoverageGapsTest is RecurringCollectorSharedTest {
849849
assertEq(dataAfter.length, 0, "Offer data should be empty after cancel");
850850
}
851851

852+
function test_Cancel_PendingRcaAndRcau_IndependentOrder() public {
853+
MockAgreementOwner approver = new MockAgreementOwner();
854+
855+
IRecurringCollector.RecurringCollectionAgreement memory rca = IRecurringCollector.RecurringCollectionAgreement({
856+
deadline: uint64(block.timestamp + 1 hours),
857+
endsAt: uint64(block.timestamp + 365 days),
858+
payer: address(approver),
859+
dataService: makeAddr("ds"),
860+
serviceProvider: makeAddr("sp"),
861+
maxInitialTokens: 100 ether,
862+
maxOngoingTokensPerSecond: 1 ether,
863+
minSecondsPerCollection: 600,
864+
maxSecondsPerCollection: 3600,
865+
conditions: 0,
866+
nonce: 1,
867+
metadata: ""
868+
});
869+
870+
// Offer RCA (not yet accepted)
871+
vm.prank(address(approver));
872+
IAgreementCollector.AgreementDetails memory details = _recurringCollector.offer(
873+
OFFER_TYPE_NEW,
874+
abi.encode(rca),
875+
0
876+
);
877+
bytes16 agreementId = details.agreementId;
878+
bytes32 rcaHash = details.versionHash;
879+
880+
// Offer RCAU on top of the pending RCA
881+
IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = IRecurringCollector
882+
.RecurringCollectionAgreementUpdate({
883+
agreementId: agreementId,
884+
deadline: uint64(block.timestamp + 1 hours),
885+
endsAt: uint64(block.timestamp + 365 days),
886+
maxInitialTokens: 200 ether,
887+
maxOngoingTokensPerSecond: 2 ether,
888+
minSecondsPerCollection: 600,
889+
maxSecondsPerCollection: 3600,
890+
conditions: 0,
891+
nonce: 1,
892+
metadata: ""
893+
});
894+
vm.prank(address(approver));
895+
IAgreementCollector.AgreementDetails memory updateDetails = _recurringCollector.offer(
896+
OFFER_TYPE_UPDATE,
897+
abi.encode(rcau),
898+
0
899+
);
900+
bytes32 rcauHash = updateDetails.versionHash;
901+
902+
// Cancel the RCA offer first — pending RCAU survives independently
903+
vm.expectEmit(true, true, false, true);
904+
emit IRecurringCollector.OfferCancelled(address(approver), agreementId, rcaHash);
905+
vm.prank(address(approver));
906+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_PENDING);
907+
908+
IRecurringCollector.AgreementData memory after1 = _recurringCollector.getAgreement(agreementId);
909+
assertEq(after1.activeTermsHash, bytes32(0), "active should be cleared");
910+
assertEq(after1.pendingTermsHash, rcauHash, "pending RCAU should survive RCA cancel");
911+
assertEq(after1.payer, address(approver), "agreement.payer persists for subsequent auth");
912+
913+
// Now cancel the pending RCAU — payer auth still works via persistent agreement.payer
914+
vm.expectEmit(true, true, false, true);
915+
emit IRecurringCollector.OfferCancelled(address(approver), agreementId, rcauHash);
916+
vm.prank(address(approver));
917+
_recurringCollector.cancel(agreementId, rcauHash, SCOPE_PENDING);
918+
919+
(uint8 activeType, ) = _recurringCollector.getAgreementOfferAt(agreementId, 0);
920+
assertEq(activeType, 0, "Active offer should be gone");
921+
(uint8 pendingType, ) = _recurringCollector.getAgreementOfferAt(agreementId, 1);
922+
assertEq(pendingType, 0, "Pending offer should be gone");
923+
}
924+
852925
// ══════════════════════════════════════════════════════════════════════
853926
// Gap 16 — cancel: silent no-op when agreement not found
854927
// ══════════════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)