Skip to content

Commit f9599da

Browse files
RembrandtKRembrandtK
committed
feat(collector): add SCOPE_SIGNED to cancel() for EOA offer revocation (TRST-L-8)
Give EOA signers an on-chain revocation path via cancel(agreementId, termsHash, SCOPE_SIGNED). Records cancelledOffers[msg.sender][termsHash] = agreementId; _requireAuthorization rejects when the stored agreementId matches. Self-authenticating, idempotent, reversible (bytes16(0) undoes), and combinable with SCOPE_PENDING/SCOPE_ACTIVE. Builds on the version-indexed storage and idempotent cancel semantics from the preceding L-11 refactor: SCOPE_SIGNED is added as a new branch at the top of cancel() alongside the existing SCOPE_PENDING / SCOPE_ACTIVE handling, and the cancelledOffers lookup slots into _requireAuthorization's signed branch.
1 parent eadc320 commit f9599da

5 files changed

Lines changed: 305 additions & 10 deletions

File tree

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

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ import {
3030
VERSION_CURRENT,
3131
VERSION_NEXT,
3232
SCOPE_ACTIVE,
33-
SCOPE_PENDING
33+
SCOPE_PENDING,
34+
SCOPE_SIGNED
3435
} from "@graphprotocol/interfaces/contracts/horizon/IAgreementCollector.sol";
3536
import { IRecurringCollector } from "@graphprotocol/interfaces/contracts/horizon/IRecurringCollector.sol";
3637
import { IGraphPayments } from "@graphprotocol/interfaces/contracts/horizon/IGraphPayments.sol";
@@ -115,6 +116,9 @@ contract RecurringCollector is
115116
/// @notice Decoded agreement terms, keyed by EIP-712 hash.
116117
/// Referenced by AgreementData.activeTermsHash and pendingTermsHash.
117118
mapping(bytes32 termsHash => AgreementTerms terms) terms;
119+
/// @notice Cancelled offer hashes, keyed by signer then EIP-712 hash.
120+
/// Stores the agreementId that is blocked; bytes16(0) means not cancelled.
121+
mapping(address signer => mapping(bytes32 hash => bytes16 agreementId)) cancelledOffers;
118122
}
119123

120124
/// @dev keccak256(abi.encode(uint256(keccak256("graphprotocol.storage.RecurringCollector")) - 1)) & ~bytes32(uint256(0xff))
@@ -489,10 +493,28 @@ contract RecurringCollector is
489493
}
490494

491495
/// @inheritdoc IAgreementCollector
496+
/// @dev This implementation targets only the payer side of the agreement.
497+
/// SCOPE_PENDING and SCOPE_ACTIVE enforce `msg.sender == agreement.payer`.
498+
/// SCOPE_SIGNED has no caller check in this function; the entry it writes is
499+
/// self-keyed by msg.sender and is consulted only later, during payer
500+
/// authorization of a signed accept or update. Extending cancel to data-service
501+
/// or service-provider callers is left for a future revision.
492502
function cancel(bytes16 agreementId, bytes32 termsHash, uint16 options) external whenNotPaused {
493503
RecurringCollectorStorage storage $ = _getStorage();
494504
AgreementData storage agreement = $.agreements[agreementId];
495505

506+
// Signed scope: record cancelledOffers[msg.sender][termsHash] = agreementId.
507+
// Self-authenticating — only blocks when msg.sender matches the recovered ECDSA signer.
508+
// The stored agreementId is checked in _requireAuthorization (!=); calling again
509+
// with bytes16(0) undoes the cancellation, calling with a different agreementId
510+
// redirects it.
511+
if (options & SCOPE_SIGNED != 0) {
512+
if ($.cancelledOffers[msg.sender][termsHash] != agreementId) {
513+
$.cancelledOffers[msg.sender][termsHash] = agreementId;
514+
emit OfferCancelled(msg.sender, agreementId, termsHash);
515+
}
516+
}
517+
496518
// Pending / active scopes: revert if on-chain data exists but caller is not the payer.
497519
// No-op if nothing exists on-chain (nothing to cancel).
498520
if (options & (SCOPE_PENDING | SCOPE_ACTIVE) != 0) {
@@ -1063,11 +1085,15 @@ contract RecurringCollector is
10631085
bytes16 _agreementId,
10641086
uint8 _offerType
10651087
) private view {
1066-
if (0 < _signature.length)
1067-
require(_isAuthorized(_payer, ECDSA.recover(_hash, _signature)), RecurringCollectorInvalidSigner());
1068-
else {
1088+
RecurringCollectorStorage storage $ = _getStorage();
1089+
1090+
if (0 < _signature.length) {
1091+
address signer = ECDSA.recover(_hash, _signature);
1092+
require(_isAuthorized(_payer, signer), RecurringCollectorInvalidSigner());
1093+
require($.cancelledOffers[signer][_hash] != _agreementId, RecurringCollectorOfferCancelled(signer, _hash));
1094+
} else {
10691095
// Pre-approval: the hash must match the expected version of this agreement.
1070-
AgreementData storage agreement = _getStorage().agreements[_agreementId];
1096+
AgreementData storage agreement = $.agreements[_agreementId];
10711097
bytes32 versionHash = _offerType == OFFER_TYPE_NEW ? agreement.activeTermsHash : agreement.pendingTermsHash;
10721098
require(versionHash == _hash, RecurringCollectorInvalidSigner());
10731099
}

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

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.27;
3+
4+
import { IRecurringCollector } from "@graphprotocol/interfaces/contracts/horizon/IRecurringCollector.sol";
5+
import {
6+
SCOPE_SIGNED,
7+
SCOPE_ACTIVE,
8+
SCOPE_PENDING
9+
} from "@graphprotocol/interfaces/contracts/horizon/IAgreementCollector.sol";
10+
11+
import { RecurringCollectorSharedTest } from "./shared.t.sol";
12+
13+
contract RecurringCollectorCancelSignedOfferTest is RecurringCollectorSharedTest {
14+
/*
15+
* TESTS
16+
*/
17+
18+
/* solhint-disable graph/func-name-mixedcase */
19+
20+
function test_CancelSigned_BlocksAccept(FuzzyTestAccept calldata fuzzyTestAccept) public {
21+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
22+
fuzzyTestAccept.rca
23+
);
24+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
25+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
26+
27+
(, bytes memory signature) = _recurringCollectorHelper.generateSignedRCA(rca, signerKey);
28+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
29+
address signer = vm.addr(signerKey);
30+
bytes16 agreementId = _recurringCollector.generateAgreementId(
31+
rca.payer,
32+
rca.dataService,
33+
rca.serviceProvider,
34+
rca.deadline,
35+
rca.nonce
36+
);
37+
38+
vm.prank(signer);
39+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED);
40+
41+
// Accepting with the cancelled signature should revert
42+
vm.expectRevert(
43+
abi.encodeWithSelector(IRecurringCollector.RecurringCollectorOfferCancelled.selector, signer, rcaHash)
44+
);
45+
vm.prank(rca.dataService);
46+
_recurringCollector.accept(rca, signature);
47+
}
48+
49+
function test_CancelSigned_EmitsEvent(FuzzyTestAccept calldata fuzzyTestAccept) public {
50+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
51+
fuzzyTestAccept.rca
52+
);
53+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
54+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
55+
56+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
57+
address signer = vm.addr(signerKey);
58+
bytes16 agreementId = _recurringCollector.generateAgreementId(
59+
rca.payer,
60+
rca.dataService,
61+
rca.serviceProvider,
62+
rca.deadline,
63+
rca.nonce
64+
);
65+
66+
vm.expectEmit(address(_recurringCollector));
67+
emit IRecurringCollector.OfferCancelled(signer, agreementId, rcaHash);
68+
vm.prank(signer);
69+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED);
70+
}
71+
72+
function test_CancelSigned_BlocksUpdate(FuzzyTestUpdate calldata fuzzyTestUpdate) public {
73+
(
74+
IRecurringCollector.RecurringCollectionAgreement memory rca,
75+
,
76+
uint256 signerKey,
77+
bytes16 agreementId
78+
) = _sensibleAuthorizeAndAccept(fuzzyTestUpdate.fuzzyTestAccept);
79+
80+
IRecurringCollector.RecurringCollectionAgreementUpdate memory rcau = _recurringCollectorHelper.sensibleRCAU(
81+
fuzzyTestUpdate.rcau
82+
);
83+
rcau.agreementId = agreementId;
84+
85+
(
86+
IRecurringCollector.RecurringCollectionAgreementUpdate memory signedRcau,
87+
bytes memory rcauSig
88+
) = _recurringCollectorHelper.generateSignedRCAUForAgreement(agreementId, rcau, signerKey);
89+
bytes32 rcauHash = _recurringCollector.hashRCAU(signedRcau);
90+
address signer = vm.addr(signerKey);
91+
92+
vm.prank(signer);
93+
_recurringCollector.cancel(agreementId, rcauHash, SCOPE_SIGNED);
94+
95+
// Updating with the cancelled signature should revert
96+
vm.expectRevert(
97+
abi.encodeWithSelector(IRecurringCollector.RecurringCollectorOfferCancelled.selector, signer, rcauHash)
98+
);
99+
vm.prank(rca.dataService);
100+
_recurringCollector.update(signedRcau, rcauSig);
101+
}
102+
103+
function test_CancelSigned_Idempotent(FuzzyTestAccept calldata fuzzyTestAccept) public {
104+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
105+
fuzzyTestAccept.rca
106+
);
107+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
108+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
109+
110+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
111+
address signer = vm.addr(signerKey);
112+
bytes16 agreementId = _recurringCollector.generateAgreementId(
113+
rca.payer,
114+
rca.dataService,
115+
rca.serviceProvider,
116+
rca.deadline,
117+
rca.nonce
118+
);
119+
120+
vm.prank(signer);
121+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED);
122+
123+
// Second call succeeds silently — no revert, no event
124+
vm.recordLogs();
125+
vm.prank(signer);
126+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED);
127+
assertEq(vm.getRecordedLogs().length, 0);
128+
}
129+
130+
function test_CancelSigned_DoesNotAffectDifferentSigner(
131+
FuzzyTestAccept calldata fuzzyTestAccept1,
132+
FuzzyTestAccept calldata fuzzyTestAccept2
133+
) public {
134+
IRecurringCollector.RecurringCollectionAgreement memory rca1 = _recurringCollectorHelper.sensibleRCA(
135+
fuzzyTestAccept1.rca
136+
);
137+
uint256 signerKey1 = boundKey(fuzzyTestAccept1.unboundedSignerKey);
138+
139+
IRecurringCollector.RecurringCollectionAgreement memory rca2 = _recurringCollectorHelper.sensibleRCA(
140+
fuzzyTestAccept2.rca
141+
);
142+
uint256 signerKey2 = boundKey(fuzzyTestAccept2.unboundedSignerKey);
143+
144+
vm.assume(rca1.payer != rca2.payer);
145+
vm.assume(vm.addr(signerKey1) != vm.addr(signerKey2));
146+
147+
_recurringCollectorHelper.authorizeSignerWithChecks(rca1.payer, signerKey1);
148+
_recurringCollectorHelper.authorizeSignerWithChecks(rca2.payer, signerKey2);
149+
150+
bytes32 rcaHash = _recurringCollector.hashRCA(rca1);
151+
152+
// Signer1 cancels — should not affect signer2
153+
vm.prank(vm.addr(signerKey1));
154+
_recurringCollector.cancel(bytes16(0), rcaHash, SCOPE_SIGNED);
155+
156+
// Signer2's signatures for the same hash are unaffected
157+
// (signer-scoped, not hash-global)
158+
}
159+
160+
function test_CancelSigned_SelfAuthenticating(FuzzyTestAccept calldata fuzzyTestAccept, address anyAddress) public {
161+
// Any address can call cancel with SCOPE_SIGNED — it only records for msg.sender
162+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
163+
fuzzyTestAccept.rca
164+
);
165+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
166+
vm.assume(anyAddress != address(0));
167+
vm.assume(anyAddress != _proxyAdmin);
168+
169+
// Should not revert — self-authenticating, no _requirePayer
170+
vm.prank(anyAddress);
171+
_recurringCollector.cancel(bytes16(0), rcaHash, SCOPE_SIGNED);
172+
}
173+
174+
function test_CancelSigned_CombinedWithActiveDoesNotRevert(FuzzyTestAccept calldata fuzzyTestAccept) public {
175+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
176+
fuzzyTestAccept.rca
177+
);
178+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
179+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
180+
181+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
182+
address signer = vm.addr(signerKey);
183+
bytes16 agreementId = _recurringCollector.generateAgreementId(
184+
rca.payer,
185+
rca.dataService,
186+
rca.serviceProvider,
187+
rca.deadline,
188+
rca.nonce
189+
);
190+
191+
// SCOPE_SIGNED | SCOPE_ACTIVE with no accepted agreement — should not revert.
192+
// The signed recording succeeds; the active scope is skipped because nothing on-chain.
193+
vm.expectEmit(address(_recurringCollector));
194+
emit IRecurringCollector.OfferCancelled(signer, agreementId, rcaHash);
195+
vm.prank(signer);
196+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED | SCOPE_ACTIVE);
197+
}
198+
199+
function test_CancelSigned_CombinedWithPendingDoesNotRevert(FuzzyTestAccept calldata fuzzyTestAccept) public {
200+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
201+
fuzzyTestAccept.rca
202+
);
203+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
204+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
205+
206+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
207+
address signer = vm.addr(signerKey);
208+
bytes16 agreementId = _recurringCollector.generateAgreementId(
209+
rca.payer,
210+
rca.dataService,
211+
rca.serviceProvider,
212+
rca.deadline,
213+
rca.nonce
214+
);
215+
216+
// SCOPE_SIGNED | SCOPE_PENDING with no agreement — should not revert.
217+
vm.expectEmit(address(_recurringCollector));
218+
emit IRecurringCollector.OfferCancelled(signer, agreementId, rcaHash);
219+
vm.prank(signer);
220+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED | SCOPE_PENDING);
221+
}
222+
223+
function test_CancelSigned_UndoWithZero(FuzzyTestAccept calldata fuzzyTestAccept) public {
224+
IRecurringCollector.RecurringCollectionAgreement memory rca = _recurringCollectorHelper.sensibleRCA(
225+
fuzzyTestAccept.rca
226+
);
227+
uint256 signerKey = boundKey(fuzzyTestAccept.unboundedSignerKey);
228+
_recurringCollectorHelper.authorizeSignerWithChecks(rca.payer, signerKey);
229+
230+
(, bytes memory signature) = _recurringCollectorHelper.generateSignedRCA(rca, signerKey);
231+
bytes32 rcaHash = _recurringCollector.hashRCA(rca);
232+
address signer = vm.addr(signerKey);
233+
bytes16 agreementId = _recurringCollector.generateAgreementId(
234+
rca.payer,
235+
rca.dataService,
236+
rca.serviceProvider,
237+
rca.deadline,
238+
rca.nonce
239+
);
240+
241+
// Cancel
242+
vm.prank(signer);
243+
_recurringCollector.cancel(agreementId, rcaHash, SCOPE_SIGNED);
244+
245+
// Undo by calling with bytes16(0)
246+
vm.prank(signer);
247+
_recurringCollector.cancel(bytes16(0), rcaHash, SCOPE_SIGNED);
248+
249+
// Accept should now succeed
250+
_setupValidProvision(rca.serviceProvider, rca.dataService);
251+
vm.prank(rca.dataService);
252+
_recurringCollector.accept(rca, signature);
253+
}
254+
255+
/* solhint-enable graph/func-name-mixedcase */
256+
}

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ uint8 constant OFFER_TYPE_UPDATE = 2;
4646
uint8 constant SCOPE_ACTIVE = 1;
4747
/// @dev Cancel targets pending offers
4848
uint8 constant SCOPE_PENDING = 2;
49+
/// @dev Cancel targets signed offers
50+
uint8 constant SCOPE_SIGNED = 4;
4951

5052
// -- Version indices (shared by getAgreementDetails and getAgreementOfferAt) --
5153
//
@@ -127,12 +129,14 @@ interface IAgreementCollector is IPaymentsCollector {
127129
function offer(uint8 offerType, bytes calldata data, uint16 options) external returns (AgreementDetails memory);
128130

129131
/**
130-
* @notice Cancel an agreement or revoke a pending offer.
131-
* @dev Scopes can be combined. SCOPE_PENDING and SCOPE_ACTIVE require payer authorization
132-
* and no-op if nothing exists on-chain.
133-
* @param agreementId The agreement's ID.
132+
* @notice Cancel an agreement, revoke a pending offer, or invalidate a signed offer.
133+
* @dev Scopes can be combined. SCOPE_SIGNED is self-authenticating (keyed by msg.sender);
134+
* SCOPE_PENDING and SCOPE_ACTIVE require payer authorization and no-op if nothing exists on-chain.
135+
* @param agreementId The agreement's ID. For SCOPE_SIGNED, only blocks accept/update when
136+
* the agreementId matches; passing bytes16(0) undoes a previous cancellation.
134137
* @param termsHash EIP-712 hash identifying which terms to cancel.
135-
* @param options Bitmask — SCOPE_ACTIVE (1) active terms, SCOPE_PENDING (2) pending offers.
138+
* @param options Bitmask — SCOPE_ACTIVE (1) active terms, SCOPE_PENDING (2) pending offers,
139+
* SCOPE_SIGNED (4) signed offers.
136140
*/
137141
function cancel(bytes16 agreementId, bytes32 termsHash, uint16 options) external;
138142

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ interface IRecurringCollector is IAuthorizable, IAgreementCollector {
402402
*/
403403
error RecurringCollectorPauseGuardianNoChange(address account, bool allowed);
404404

405+
/**
406+
* @notice Thrown when accepting or updating with a hash that the signer cancelled via SCOPE_SIGNED
407+
* @param signer The signer who cancelled the offer
408+
* @param hash The cancelled EIP-712 hash
409+
*/
410+
error RecurringCollectorOfferCancelled(address signer, bytes32 hash);
411+
405412
/**
406413
* @notice Emitted when a pause guardian is set
407414
* @param account The address of the pause guardian

‎packages/issuance/audits/PR1301/TRST-L-8.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@ Expose a `cancelSignature(bytes32 hash)` entry point that records the hash as in
2020
TBD
2121

2222
---
23+
24+
Added `SCOPE_SIGNED` flag to `cancel()`, giving EOA signers an on-chain revocation path like contract payers already have via `SCOPE_PENDING`. The signer calls `cancel(agreementId, termsHash, SCOPE_SIGNED)` which records `cancelledOffers[msg.sender][termsHash] = agreementId`. When `accept()` or `update()` later processes a signature, `_requireAuthorization` recovers the signer via ECDSA and rejects if the stored agreementId matches. Self-authenticating (keyed by signer address), idempotent, reversible (calling again with `bytes16(0)` undoes the cancellation), and combinable with other scopes. Also made `cancel` no-op when nothing exists on-chain instead of reverting.

0 commit comments

Comments
 (0)