Skip to content

Commit bf35f19

Browse files
RembrandtKRembrandtK
committed
feat(contracts): add getIssuanceAllocator to IIssuanceTarget interface
Every issuance target should expose its allocator. Add getIssuanceAllocator() returning IIssuanceAllocationDistribution to IIssuanceTarget. Implement in RecurringAgreementManager (reads from storage), DirectAllocation (stores and returns), and RewardsManager (existing impl, moved from IRewardsManager to IIssuanceTarget). Also change IIssuanceTarget.setIssuanceAllocator parameter from address to IIssuanceAllocationDistribution for compile-time type safety.
1 parent 4a554f2 commit bf35f19

21 files changed

Lines changed: 356 additions & 64 deletions

File tree

‎packages/contracts-test/tests/unit/rewards/rewards-interface.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ describe('RewardsManager interfaces', () => {
5454
})
5555

5656
it('IIssuanceTarget should have stable interface ID', () => {
57-
expect(IIssuanceTarget__factory.interfaceId).to.equal('0xaee4dc43')
57+
expect(IIssuanceTarget__factory.interfaceId).to.equal('0x19f6601a')
5858
})
5959

6060
it('IRewardsManager should have stable interface ID', () => {
61-
expect(IRewardsManager__factory.interfaceId).to.equal('0x337b092e')
61+
expect(IRewardsManager__factory.interfaceId).to.equal('0x8469b577')
6262
})
6363
})
6464

‎packages/contracts/contracts/rewards/RewardsManager.sol‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,24 +173,25 @@ contract RewardsManager is
173173
* Note that the IssuanceAllocator can be set to the zero address to disable use of an allocator, and
174174
* use the local `issuancePerBlock` variable instead to control issuance.
175175
*/
176-
function setIssuanceAllocator(address newIssuanceAllocator) external override onlyGovernor {
177-
if (address(issuanceAllocator) != newIssuanceAllocator) {
176+
function setIssuanceAllocator(IIssuanceAllocationDistribution newIssuanceAllocator) external override onlyGovernor {
177+
if (issuanceAllocator != newIssuanceAllocator) {
178178
// Update rewards calculation before changing the issuance allocator
179179
updateAccRewardsPerSignal();
180180

181181
// Check that the contract supports the IIssuanceAllocationDistribution interface
182182
// Allow zero address to disable the allocator
183-
if (newIssuanceAllocator != address(0)) {
183+
if (address(newIssuanceAllocator) != address(0)) {
184184
// solhint-disable-next-line gas-small-strings
185185
require(
186-
IERC165(newIssuanceAllocator).supportsInterface(type(IIssuanceAllocationDistribution).interfaceId),
186+
IERC165(address(newIssuanceAllocator)).supportsInterface(
187+
type(IIssuanceAllocationDistribution).interfaceId
188+
),
187189
"Contract does not support IIssuanceAllocationDistribution interface"
188190
);
189191
}
190192

191-
address oldIssuanceAllocator = address(issuanceAllocator);
192-
issuanceAllocator = IIssuanceAllocationDistribution(newIssuanceAllocator);
193-
emit IssuanceAllocatorSet(oldIssuanceAllocator, newIssuanceAllocator);
193+
emit IssuanceAllocatorSet(issuanceAllocator, newIssuanceAllocator);
194+
issuanceAllocator = newIssuanceAllocator;
194195
}
195196
}
196197

@@ -325,7 +326,7 @@ contract RewardsManager is
325326
}
326327

327328
/**
328-
* @inheritdoc IRewardsManager
329+
* @inheritdoc IIssuanceTarget
329330
*/
330331
function getIssuanceAllocator() external view override returns (IIssuanceAllocationDistribution) {
331332
return issuanceAllocator;

‎packages/deployment/lib/abis.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ function loadAbi(artifactPath: string): Abi {
1717
return artifact.abi as Abi
1818
}
1919

20-
// Interface IDs - these match the generated values from TypeChain factories
21-
// Verified by tests: packages/issuance/testing/tests/allocate/InterfaceIdStability.test.ts
22-
// and packages/contracts-test/tests/unit/rewards/rewards-interface.test.ts
20+
// Interface IDs - these mirror the values the compiler derives from the
21+
// corresponding ABI. Cross-checked by test/interface-id-stability.test.ts;
22+
// update both together whenever an interface changes.
2323
export const IERC165_INTERFACE_ID = '0x01ffc9a7' as const
24-
export const IISSUANCE_TARGET_INTERFACE_ID = '0xaee4dc43' as const
25-
export const IREWARDS_MANAGER_INTERFACE_ID = '0xa0a2f219' as const
24+
export const IISSUANCE_TARGET_INTERFACE_ID = '0x19f6601a' as const
25+
export const IREWARDS_MANAGER_INTERFACE_ID = '0x8469b577' as const
2626

2727
export const REWARDS_MANAGER_ABI = loadAbi(
2828
'@graphprotocol/interfaces/artifacts/contracts/contracts/rewards/IRewardsManager.sol/IRewardsManager.json',

‎packages/deployment/test/interface-id-stability.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { expect } from 'chai'
2+
import type { Abi } from 'viem'
3+
import { toFunctionSelector } from 'viem'
4+
5+
import {
6+
IERC165_ABI,
7+
IERC165_INTERFACE_ID,
8+
IREWARDS_MANAGER_INTERFACE_ID,
9+
IISSUANCE_TARGET_INTERFACE_ID,
10+
ISSUANCE_TARGET_ABI,
11+
REWARDS_MANAGER_ABI,
12+
} from '../lib/abis.js'
13+
14+
function computeInterfaceId(abi: Abi): `0x${string}` {
15+
const xor = abi
16+
.filter((item): item is Extract<(typeof abi)[number], { type: 'function' }> => item.type === 'function')
17+
.map((f) => Number.parseInt(toFunctionSelector(f).slice(2), 16) >>> 0)
18+
.reduce((a, s) => (a ^ s) >>> 0, 0)
19+
return `0x${xor.toString(16).padStart(8, '0')}`
20+
}
21+
22+
describe('Interface ID Stability', function () {
23+
it('IERC165_INTERFACE_ID matches the IERC165 ABI', function () {
24+
expect(IERC165_INTERFACE_ID).to.equal(computeInterfaceId(IERC165_ABI))
25+
})
26+
27+
it('IISSUANCE_TARGET_INTERFACE_ID matches the IIssuanceTarget ABI', function () {
28+
expect(IISSUANCE_TARGET_INTERFACE_ID).to.equal(computeInterfaceId(ISSUANCE_TARGET_ABI))
29+
})
30+
31+
it('IREWARDS_MANAGER_INTERFACE_ID matches the IRewardsManager ABI', function () {
32+
expect(IREWARDS_MANAGER_INTERFACE_ID).to.equal(computeInterfaceId(REWARDS_MANAGER_ABI))
33+
})
34+
})

‎packages/interfaces/contracts/contracts/rewards/IRewardsManager.sol‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
pragma solidity ^0.7.6 || ^0.8.0;
44

5-
import { IIssuanceAllocationDistribution } from "../../issuance/allocate/IIssuanceAllocationDistribution.sol";
65
import { IRewardsIssuer } from "./IRewardsIssuer.sol";
76

87
/**
@@ -179,13 +178,6 @@ interface IRewardsManager {
179178
*/
180179
function subgraphService() external view returns (IRewardsIssuer);
181180

182-
/**
183-
* @notice Get the issuance allocator address
184-
* @dev When set, this allocator controls issuance distribution instead of issuancePerBlock
185-
* @return The issuance allocator contract (zero address if not set)
186-
*/
187-
function getIssuanceAllocator() external view returns (IIssuanceAllocationDistribution);
188-
189181
/**
190182
* @notice Get the reclaim address for a specific reason
191183
* @param reason The reclaim reason identifier

‎packages/interfaces/contracts/issuance/allocate/IIssuanceTarget.sol‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
pragma solidity ^0.7.6 || ^0.8.0;
44

5+
import { IIssuanceAllocationDistribution } from "./IIssuanceAllocationDistribution.sol";
6+
57
/**
68
* @title IIssuanceTarget
79
* @author Edge & Node
@@ -13,7 +15,10 @@ interface IIssuanceTarget {
1315
* @param oldIssuanceAllocator Old issuance allocator address
1416
* @param newIssuanceAllocator New issuance allocator address
1517
*/
16-
event IssuanceAllocatorSet(address indexed oldIssuanceAllocator, address indexed newIssuanceAllocator);
18+
event IssuanceAllocatorSet(
19+
IIssuanceAllocationDistribution indexed oldIssuanceAllocator,
20+
IIssuanceAllocationDistribution indexed newIssuanceAllocator
21+
);
1722

1823
/// @notice Emitted before the issuance allocation changes
1924
event BeforeIssuanceAllocationChange();
@@ -27,11 +32,17 @@ interface IIssuanceTarget {
2732
*/
2833
function beforeIssuanceAllocationChange() external;
2934

35+
/**
36+
* @notice Returns the current issuance allocator
37+
* @return The issuance allocator contract (zero address if not set)
38+
*/
39+
function getIssuanceAllocator() external view returns (IIssuanceAllocationDistribution);
40+
3041
/**
3142
* @notice Sets the issuance allocator for this target
3243
* @dev This function facilitates upgrades by providing a standard way for targets
3344
* to change their allocator. Implementations can define their own access control.
3445
* @param newIssuanceAllocator Address of the issuance allocator
3546
*/
36-
function setIssuanceAllocator(address newIssuanceAllocator) external;
47+
function setIssuanceAllocator(IIssuanceAllocationDistribution newIssuanceAllocator) external;
3748
}

‎packages/issuance/contracts/agreement/RecurringAgreementManager.sol‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,11 @@ contract RecurringAgreementManager is
275275
/// @inheritdoc IIssuanceTarget
276276
function beforeIssuanceAllocationChange() external virtual override {}
277277

278+
/// @inheritdoc IIssuanceTarget
279+
function getIssuanceAllocator() external view virtual override returns (IIssuanceAllocationDistribution) {
280+
return _getStorage().issuanceAllocator;
281+
}
282+
278283
/// @inheritdoc IIssuanceTarget
279284
/// @dev The allocator is expected to call distributeIssuance() (bringing distribution up to
280285
/// the current block) before any configuration change. As a result, the same-block dedup in
@@ -283,21 +288,23 @@ contract RecurringAgreementManager is
283288
/// in a standalone transaction to avoid interleaving with collection in the same block.
284289
/// Even if interleaved, the only effect is a one-block lag before the new allocator's
285290
/// distribution is picked up — corrected automatically on the next block.
286-
function setIssuanceAllocator(address newIssuanceAllocator) external virtual override onlyRole(GOVERNOR_ROLE) {
291+
function setIssuanceAllocator(
292+
IIssuanceAllocationDistribution newIssuanceAllocator
293+
) external virtual override onlyRole(GOVERNOR_ROLE) {
287294
RecurringAgreementManagerStorage storage $ = _getStorage();
288-
if (address($.issuanceAllocator) == newIssuanceAllocator) return;
295+
if (address($.issuanceAllocator) == address(newIssuanceAllocator)) return;
289296

290-
if (newIssuanceAllocator != address(0))
297+
if (address(newIssuanceAllocator) != address(0))
291298
require(
292299
ERC165Checker.supportsInterface(
293-
newIssuanceAllocator,
300+
address(newIssuanceAllocator),
294301
type(IIssuanceAllocationDistribution).interfaceId
295302
),
296-
InvalidIssuanceAllocator(newIssuanceAllocator)
303+
InvalidIssuanceAllocator(address(newIssuanceAllocator))
297304
);
298305

299-
emit IssuanceAllocatorSet(address($.issuanceAllocator), newIssuanceAllocator);
300-
$.issuanceAllocator = IIssuanceAllocationDistribution(newIssuanceAllocator);
306+
emit IssuanceAllocatorSet($.issuanceAllocator, newIssuanceAllocator);
307+
$.issuanceAllocator = newIssuanceAllocator;
301308
}
302309

303310
// -- IAgreementOwner --

‎packages/issuance/contracts/allocate/DirectAllocation.sol‎

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
pragma solidity ^0.8.27;
44

5+
import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
6+
7+
import { IIssuanceAllocationDistribution } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceAllocationDistribution.sol";
58
import { IIssuanceTarget } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceTarget.sol";
69
import { ISendTokens } from "@graphprotocol/interfaces/contracts/issuance/allocate/ISendTokens.sol";
710
import { BaseUpgradeable } from "../common/BaseUpgradeable.sol";
@@ -24,13 +27,47 @@ import { ERC165Upgradeable } from "@openzeppelin/contracts-upgradeable/utils/int
2427
* @custom:security-contact Please email security+contracts@thegraph.com if you find any bugs. We might have an active bug bounty program.
2528
*/
2629
contract DirectAllocation is BaseUpgradeable, IIssuanceTarget, ISendTokens {
30+
// -- Namespaced Storage --
31+
32+
/// @notice ERC-7201 storage location for DirectAllocation
33+
bytes32 private constant DIRECT_ALLOCATION_STORAGE_LOCATION =
34+
// solhint-disable-next-line gas-small-strings
35+
keccak256(abi.encode(uint256(keccak256("graphprotocol.storage.DirectAllocation")) - 1)) &
36+
~bytes32(uint256(0xff));
37+
38+
/// @notice Main storage structure for DirectAllocation using ERC-7201 namespaced storage
39+
/// @param issuanceAllocator The issuance allocator that distributes tokens to this contract
40+
/// @custom:storage-location erc7201:graphprotocol.storage.DirectAllocation
41+
struct DirectAllocationData {
42+
IIssuanceAllocationDistribution issuanceAllocator;
43+
}
44+
45+
/**
46+
* @notice Returns the storage struct for DirectAllocation
47+
* @return $ contract storage
48+
*/
49+
function _getDirectAllocationStorage() private pure returns (DirectAllocationData storage $) {
50+
// solhint-disable-previous-line use-natspec
51+
// Solhint does not support $ return variable in natspec
52+
53+
bytes32 slot = DIRECT_ALLOCATION_STORAGE_LOCATION;
54+
// solhint-disable-next-line no-inline-assembly
55+
assembly {
56+
$.slot := slot
57+
}
58+
}
59+
2760
// -- Custom Errors --
2861

2962
/// @notice Thrown when token transfer fails
3063
/// @param to The address that the transfer was attempted to
3164
/// @param amount The amount of tokens that failed to transfer
3265
error SendTokensFailed(address to, uint256 amount);
3366

67+
/// @notice Thrown when the issuance allocator does not support IIssuanceAllocationDistribution
68+
/// @param allocator The rejected allocator address
69+
error InvalidIssuanceAllocator(address allocator);
70+
3471
// -- Events --
3572

3673
/// @notice Emitted when tokens are sent
@@ -89,9 +126,28 @@ contract DirectAllocation is BaseUpgradeable, IIssuanceTarget, ISendTokens {
89126
*/
90127
function beforeIssuanceAllocationChange() external virtual override {}
91128

92-
/**
93-
* @dev No-op for DirectAllocation; issuanceAllocator is not stored.
94-
* @inheritdoc IIssuanceTarget
95-
*/
96-
function setIssuanceAllocator(address issuanceAllocator) external virtual override onlyRole(GOVERNOR_ROLE) {}
129+
/// @inheritdoc IIssuanceTarget
130+
function getIssuanceAllocator() external view virtual override returns (IIssuanceAllocationDistribution) {
131+
return _getDirectAllocationStorage().issuanceAllocator;
132+
}
133+
134+
/// @inheritdoc IIssuanceTarget
135+
function setIssuanceAllocator(
136+
IIssuanceAllocationDistribution newIssuanceAllocator
137+
) external virtual override onlyRole(GOVERNOR_ROLE) {
138+
DirectAllocationData storage $ = _getDirectAllocationStorage();
139+
if (address(newIssuanceAllocator) == address($.issuanceAllocator)) return;
140+
141+
if (address(newIssuanceAllocator) != address(0))
142+
require(
143+
ERC165Checker.supportsInterface(
144+
address(newIssuanceAllocator),
145+
type(IIssuanceAllocationDistribution).interfaceId
146+
),
147+
InvalidIssuanceAllocator(address(newIssuanceAllocator))
148+
);
149+
150+
emit IssuanceAllocatorSet($.issuanceAllocator, newIssuanceAllocator);
151+
$.issuanceAllocator = newIssuanceAllocator;
152+
}
97153
}

‎packages/issuance/contracts/test/allocate/MockNotificationTracker.sol‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
pragma solidity ^0.8.24;
33

4+
import { IIssuanceAllocationDistribution } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceAllocationDistribution.sol";
45
import { IIssuanceTarget } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceTarget.sol";
56
import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
67

@@ -30,7 +31,12 @@ contract MockNotificationTracker is IIssuanceTarget, ERC165 {
3031
}
3132

3233
/// @inheritdoc IIssuanceTarget
33-
function setIssuanceAllocator(address _issuanceAllocator) external pure override {}
34+
function getIssuanceAllocator() external pure override returns (IIssuanceAllocationDistribution) {
35+
return IIssuanceAllocationDistribution(address(0));
36+
}
37+
38+
/// @inheritdoc IIssuanceTarget
39+
function setIssuanceAllocator(IIssuanceAllocationDistribution _issuanceAllocator) external pure override {}
3440

3541
/// @inheritdoc ERC165
3642
function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {

‎packages/issuance/contracts/test/allocate/MockReentrantTarget.sol‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,13 @@ contract MockReentrantTarget is IIssuanceTarget, ERC165 {
8585
}
8686

8787
/// @inheritdoc IIssuanceTarget
88-
function setIssuanceAllocator(address _issuanceAllocator) external override {
89-
issuanceAllocator = _issuanceAllocator;
88+
function getIssuanceAllocator() external view override returns (IIssuanceAllocationDistribution) {
89+
return IIssuanceAllocationDistribution(issuanceAllocator);
90+
}
91+
92+
/// @inheritdoc IIssuanceTarget
93+
function setIssuanceAllocator(IIssuanceAllocationDistribution _issuanceAllocator) external override {
94+
issuanceAllocator = address(_issuanceAllocator);
9095
}
9196

9297
/// @inheritdoc ERC165

0 commit comments

Comments
 (0)