Skip to content

Commit 54a4ecc

Browse files
committed
refactor: move agreement lifecycle and state to RecurringCollector
Move agreement lifecycle management into RecurringCollector: - State model: replace AgreementState enum with uint16 bitmask flags (REGISTERED, ACCEPTED, NOTICE_GIVEN, SETTLED, BY_PAYER, etc.) - API: two-phase offer/accept flow replaces single-call accept(rca, sig) - Drop ECDSA signing and Authorizable from RC - Callback model: _shouldCallback + _notifyStateChange with gas caps (MAX_CALLBACK_GAS) and PayerCallbackFailed events baked in - Extract RecurringCollectorHashing library - RAM: nested storage per collector, pair-keyed enumeration - SS: callback integration (acceptAgreement, afterAgreementStateChange) replacing direct accept/update/cancel methods - Error simplification: drop RecurringCollector prefix from all errors Includes callback hardening (TRST-H-1, L-1, H-2, H-4, SR-4) as part of the new callback design rather than as a separate bolt-on. feat: make RecurringCollector upgradeable Convert RC to upgradeable proxy pattern: - Add Initializable - Move agreements mapping into ERC-7201 namespaced storage - Split constructor into constructor + initialize() - Add Ignition deployment modules for RC proxy - Update test setups to deploy behind TransparentUpgradeableProxy feat: pause RecurringCollector and RecurringAgreementManager (TRST-L-3) Add PausableUpgradeable to RC with pause guardian management: - pause/unpause/setPauseGuardian functions - whenNotPaused on collect, offer, accept, cancel - Pause guardian storage in ERC-7201 namespace Add IEmergencyRoleControl + emergencyRevokeRole to RecurringAgreementManager for role revocation as alternative to pause.
1 parent 60468b0 commit 54a4ecc

114 files changed

Lines changed: 11233 additions & 5475 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/PaymentsTrustModel.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ RecurringCollector adds payer callbacks when the payer is a contract:
9999
Caveats on effective escrow (contract payers introduce additional trust requirements — see caveat 3):
100100

101101
1. **Thawing reduces effective balance** — a payer can initiate a thaw; once the thaw period completes, those tokens are withdrawable. The receiver should account for the thawing period and any in-progress thaws when assessing available escrow.
102-
2. **Cancellation freezes the collection window** at `canceledAt` — the receiver can still collect for the period up to cancellation (with `minSecondsPerCollection` bypassed), but no further.
102+
2. **Cancellation freezes the collection window** at `collectableUntil` — the receiver can still collect for the period up to cancellation; `minSecondsPerCollection` is enforced during the notice period and bypassed only after `collectableUntil` is reached.
103103
3. **Contract payers can block** — if the payer is a contract that implements `IProviderEligibility`, it can deny collection via `isEligible` (see [RecurringCollector Extensions](#recurringcollector-extensions)).
104104

105105
**Mitigation**: The thawing period provides a window for the receiver to collect before funds are withdrawn. The escrow balance and thaw state are publicly visible on-chain.

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

Lines changed: 968 additions & 511 deletions
Large diffs are not rendered by default.

packages/horizon/ignition/modules/core/HorizonProxies.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { ethers } from 'ethers'
33

44
import GraphPaymentsArtifact from '../../../build/contracts/contracts/payments/GraphPayments.sol/GraphPayments.json'
55
import PaymentsEscrowArtifact from '../../../build/contracts/contracts/payments/PaymentsEscrow.sol/PaymentsEscrow.json'
6+
import RecurringCollectorArtifact from '../../../build/contracts/contracts/payments/collectors/RecurringCollector.sol/RecurringCollector.json'
67
import { MigrateControllerGovernorModule } from '../periphery/Controller'
78
import GraphPeripheryModule from '../periphery/periphery'
89
import { deployGraphProxy } from '../proxy/GraphProxy'
@@ -40,12 +41,21 @@ export default buildModule('HorizonProxies', (m) => {
4041
{ id: 'setContractProxy_PaymentsEscrow' },
4142
)
4243

44+
// Deploy RecurringCollector proxy (not registered in Controller — RC reads from Controller but is not looked up by others)
45+
const { Proxy: RecurringCollectorProxy, ProxyAdmin: RecurringCollectorProxyAdmin } =
46+
deployTransparentUpgradeableProxy(m, {
47+
name: 'RecurringCollector',
48+
artifact: RecurringCollectorArtifact,
49+
})
50+
4351
return {
4452
HorizonStakingProxy,
4553
GraphPaymentsProxy,
4654
PaymentsEscrowProxy,
55+
RecurringCollectorProxy,
4756
GraphPaymentsProxyAdmin,
4857
PaymentsEscrowProxyAdmin,
58+
RecurringCollectorProxyAdmin,
4959
}
5060
})
5161

@@ -62,7 +72,21 @@ export const MigrateHorizonProxiesDeployerModule = buildModule('HorizonProxiesDe
6272
artifact: PaymentsEscrowArtifact,
6373
})
6474

65-
return { GraphPaymentsProxy, PaymentsEscrowProxy, GraphPaymentsProxyAdmin, PaymentsEscrowProxyAdmin }
75+
// Deploy RecurringCollector proxy
76+
const { Proxy: RecurringCollectorProxy, ProxyAdmin: RecurringCollectorProxyAdmin } =
77+
deployTransparentUpgradeableProxy(m, {
78+
name: 'RecurringCollector',
79+
artifact: RecurringCollectorArtifact,
80+
})
81+
82+
return {
83+
GraphPaymentsProxy,
84+
PaymentsEscrowProxy,
85+
RecurringCollectorProxy,
86+
GraphPaymentsProxyAdmin,
87+
PaymentsEscrowProxyAdmin,
88+
RecurringCollectorProxyAdmin,
89+
}
6690
})
6791

6892
export const MigrateHorizonProxiesGovernorModule = buildModule('HorizonProxiesGovernor', (m) => {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { buildModule } from '@nomicfoundation/hardhat-ignition/modules'
2+
3+
import RecurringCollectorArtifact from '../../../build/contracts/contracts/payments/collectors/RecurringCollector.sol/RecurringCollector.json'
4+
import GraphPeripheryModule from '../periphery/periphery'
5+
import { deployImplementation } from '../proxy/implementation'
6+
import { upgradeTransparentUpgradeableProxy } from '../proxy/TransparentUpgradeableProxy'
7+
import HorizonProxiesModule from './HorizonProxies'
8+
9+
export default buildModule('RecurringCollector', (m) => {
10+
const { Controller } = m.useModule(GraphPeripheryModule)
11+
const { RecurringCollectorProxyAdmin, RecurringCollectorProxy } = m.useModule(HorizonProxiesModule)
12+
13+
const governor = m.getAccount(1)
14+
15+
// Deploy RecurringCollector implementation - requires periphery and proxies to be registered in the controller
16+
const RecurringCollectorImplementation = deployImplementation(
17+
m,
18+
{
19+
name: 'RecurringCollector',
20+
artifact: RecurringCollectorArtifact,
21+
constructorArgs: [Controller],
22+
},
23+
{ after: [GraphPeripheryModule, HorizonProxiesModule] },
24+
)
25+
26+
// Upgrade proxy to implementation contract
27+
const RecurringCollector = upgradeTransparentUpgradeableProxy(
28+
m,
29+
RecurringCollectorProxyAdmin,
30+
RecurringCollectorProxy,
31+
RecurringCollectorImplementation,
32+
{
33+
name: 'RecurringCollector',
34+
artifact: RecurringCollectorArtifact,
35+
initArgs: [],
36+
},
37+
)
38+
39+
m.call(RecurringCollectorProxyAdmin, 'transferOwnership', [governor], { after: [RecurringCollector] })
40+
41+
return { RecurringCollector, RecurringCollectorProxyAdmin, RecurringCollectorImplementation }
42+
})

packages/horizon/ignition/modules/core/core.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@ import GraphPaymentsModule, { MigrateGraphPaymentsModule } from './GraphPayments
44
import GraphTallyCollectorModule, { MigrateGraphTallyCollectorModule } from './GraphTallyCollector'
55
import HorizonStakingModule, { MigrateHorizonStakingDeployerModule } from './HorizonStaking'
66
import PaymentsEscrowModule, { MigratePaymentsEscrowModule } from './PaymentsEscrow'
7+
import RecurringCollectorModule from './RecurringCollector'
78

89
export default buildModule('GraphHorizon_Core', (m) => {
910
const { HorizonStaking, HorizonStakingImplementation } = m.useModule(HorizonStakingModule)
1011
const { GraphPaymentsProxyAdmin, GraphPayments, GraphPaymentsImplementation } = m.useModule(GraphPaymentsModule)
1112
const { PaymentsEscrowProxyAdmin, PaymentsEscrow, PaymentsEscrowImplementation } = m.useModule(PaymentsEscrowModule)
1213
const { GraphTallyCollector } = m.useModule(GraphTallyCollectorModule)
14+
const { RecurringCollectorProxyAdmin, RecurringCollector, RecurringCollectorImplementation } =
15+
m.useModule(RecurringCollectorModule)
1316

1417
return {
1518
HorizonStaking,
@@ -21,10 +24,13 @@ export default buildModule('GraphHorizon_Core', (m) => {
2124
PaymentsEscrow,
2225
PaymentsEscrowImplementation,
2326
GraphTallyCollector,
27+
RecurringCollectorProxyAdmin,
28+
RecurringCollector,
29+
RecurringCollectorImplementation,
2430
}
2531
})
2632

27-
export const MigrateHorizonCoreModule = buildModule('GraphHorizon_Core', (m) => {
33+
export const MigrateHorizonCoreModule = buildModule('MigrateGraphHorizon_Core', (m) => {
2834
const { HorizonStakingProxy: HorizonStaking, HorizonStakingImplementation } = m.useModule(
2935
MigrateHorizonStakingDeployerModule,
3036
)

packages/horizon/ignition/modules/deploy.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ export default buildModule('GraphHorizon_Deploy', (m) => {
3131
PaymentsEscrow,
3232
PaymentsEscrowImplementation,
3333
GraphTallyCollector,
34+
RecurringCollectorProxyAdmin,
35+
RecurringCollector,
36+
RecurringCollectorImplementation,
3437
} = m.useModule(GraphHorizonCoreModule)
3538

3639
const governor = m.getAccount(1)
@@ -74,5 +77,8 @@ export default buildModule('GraphHorizon_Deploy', (m) => {
7477
Transparent_Proxy_PaymentsEscrow: PaymentsEscrow,
7578
Implementation_PaymentsEscrow: PaymentsEscrowImplementation,
7679
GraphTallyCollector,
80+
Transparent_ProxyAdmin_RecurringCollector: RecurringCollectorProxyAdmin,
81+
Transparent_Proxy_RecurringCollector: RecurringCollector,
82+
Implementation_RecurringCollector: RecurringCollectorImplementation,
7783
}
7884
})

packages/horizon/ignition/modules/proxy/TransparentUpgradeableProxy.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,9 @@ export function upgradeTransparentUpgradeableProxy(
6565
[proxy, implementation, m.encodeFunctionCall(implementation, 'initialize', metadata.initArgs)],
6666
options,
6767
)
68-
return loadProxyWithABI(m, proxy, metadata, { ...options, after: [upgradeCall] })
68+
return loadProxyWithABI(m, proxy, metadata, {
69+
...options,
70+
id: `${metadata.name}_UpgradedProxyWithABI`,
71+
after: [upgradeCall],
72+
})
6973
}

packages/horizon/ignition/modules/proxy/utils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ export function loadProxyWithABI(
1313
contract: ImplementationMetadata,
1414
options?: ContractOptions,
1515
) {
16+
const { id: customId, ...rest } = options ?? {}
1617
let proxyWithABI
1718
if (contract.artifact === undefined) {
18-
proxyWithABI = m.contractAt(contract.name, proxy, options)
19+
proxyWithABI = m.contractAt(customId ?? contract.name, proxy, rest)
1920
} else {
20-
proxyWithABI = m.contractAt(`${contract.name}_ProxyWithABI`, contract.artifact, proxy, options)
21+
proxyWithABI = m.contractAt(customId ?? `${contract.name}_ProxyWithABI`, contract.artifact, proxy, rest)
2122
}
2223
return proxyWithABI
2324
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,9 @@ contract BareAgreementOwner is IAgreementOwner {
1313
authorizedHashes[agreementHash] = true;
1414
}
1515

16-
function approveAgreement(bytes32 agreementHash) external view override returns (bytes4) {
17-
if (!authorizedHashes[agreementHash]) return bytes4(0);
18-
return IAgreementOwner.approveAgreement.selector;
19-
}
20-
2116
function beforeCollection(bytes16, uint256) external override {}
2217

2318
function afterCollection(bytes16, uint256) external override {}
19+
20+
function afterAgreementStateChange(bytes16, bytes32, uint16) external override {}
2421
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.27;
3+
4+
import { IAgreementOwner } from "@graphprotocol/interfaces/contracts/horizon/IAgreementOwner.sol";
5+
6+
/// @notice Malicious payer that returns empty data from supportsInterface(),
7+
/// causing an ABI decoding revert on the caller side that escapes try/catch.
8+
contract MalformedERC165Payer is IAgreementOwner {
9+
mapping(bytes32 => bool) public authorizedHashes;
10+
11+
function authorize(bytes32 agreementHash) external {
12+
authorizedHashes[agreementHash] = true;
13+
}
14+
15+
function beforeCollection(bytes16, uint256) external override {}
16+
17+
function afterCollection(bytes16, uint256) external override {}
18+
19+
function afterAgreementStateChange(bytes16, bytes32, uint16) external override {}
20+
21+
/// @notice Responds to supportsInterface with empty returndata.
22+
/// The call succeeds at the EVM level but the caller cannot ABI-decode the result.
23+
fallback() external {
24+
// solhint-disable-next-line no-inline-assembly
25+
assembly {
26+
return(0, 0)
27+
}
28+
}
29+
}

0 commit comments

Comments
 (0)