Skip to content

Commit b8e8159

Browse files
committed
fix: enable forced reward reclaiming for TRST-L-1 and clarify precedence for TRST-R-5
Addresses TRST-L-1: "Reward reclaiming efficiency is significantly limited" Addresses TRST-R-5: "Clarify reclaim precedence" The TRST-L-1 audit finding identified that reward reclaiming through takeRewards() is flawed because a rational indexer who would not receive rewards would not trigger the rewarding logic in the first place. The recommended mitigation is to add calls from SubgraphService so that any entity can force a reclaim of denied rewards. (Not adding to StakingExtension as it is deprecated and will soon not be used.) The TRST-R-5 finding noted that the precedence when both denial reasons apply should be documented. Added explicit documentation that subgraph denylist reclaim is prioritized over indexer eligibility reclaim. Key changes: 1. Created RewardsReclaim library with canonical reclaim reason constants using bytes32 identifiers (like OpenZeppelin roles) for extensibility 2. Migrated RewardsManagerStorage from separate reclaim address storage to extensible mapping: - Old: indexerEligibilityReclaimAddress, subgraphDeniedReclaimAddress - New: mapping(bytes32 => address) reclaimAddresses 3. Replaced type-specific setters with generic setReclaimAddress(bytes32, address) 4. Refactored RewardsManager.takeRewards() with early return for zero rewards and extracted helper functions: - _calcAllocationRewards(): extract allocation data calculation - _reclaimRewards(): common reclaim logic with named return value - _deniedRewards(): implements short-circuit pattern with precedence (SUBGRAPH_DENIED checked before INDEXER_INELIGIBLE) 5. Added public reclaimRewards() for external contracts to reclaim rewards with custom reasons and data 6. Updated AllocationManager._presentPOI() to explicitly call reclaimRewards() with specific reasons (STALE_POI, ALTRUISTIC_ALLOCATION, ZERO_POI, ALLOCATION_TOO_YOUNG) instead of relying on takeRewards(). This enables forced reclaiming of denied rewards, addressing the core TRST-L-1 issue. 7. Updated tests and mocks to use new bytes32-based API with RewardsReclaim constants This allows any entity to force reclaim of denied rewards through SubgraphService, and enables future extension with new reclaim reasons without contract changes.
1 parent 8edca9a commit b8e8159

15 files changed

Lines changed: 890 additions & 214 deletions

File tree

packages/contracts/contracts/rewards/RewardsManager.sol

Lines changed: 149 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { IRewardsManager } from "@graphprotocol/interfaces/contracts/contracts/r
2020
import { IIssuanceAllocationDistribution } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceAllocationDistribution.sol";
2121
import { IIssuanceTarget } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceTarget.sol";
2222
import { IRewardsEligibility } from "@graphprotocol/interfaces/contracts/issuance/eligibility/IRewardsEligibility.sol";
23+
import { RewardsReclaim } from "@graphprotocol/interfaces/contracts/contracts/rewards/RewardsReclaim.sol";
2324

2425
/**
2526
* @title Rewards Manager Contract
@@ -109,43 +110,29 @@ contract RewardsManager is RewardsManagerV6Storage, GraphUpgradeable, IERC165, I
109110
);
110111

111112
/**
112-
* @notice Emitted when the eligibility reclaim address is set
113-
* @param oldReclaimAddress Previous eligibility reclaim address
114-
* @param newReclaimAddress New eligibility reclaim address
113+
* @notice Emitted when a reclaim address is set
114+
* @param reason The reclaim reason identifier
115+
* @param oldAddress Previous address
116+
* @param newAddress New address
115117
*/
116-
event IndexerEligibilityReclaimAddressSet(address indexed oldReclaimAddress, address indexed newReclaimAddress);
118+
event ReclaimAddressSet(bytes32 indexed reason, address indexed oldAddress, address indexed newAddress);
117119

118120
/**
119-
* @notice Emitted when the subgraph reclaim address is set
120-
* @param oldReclaimAddress Previous subgraph reclaim address
121-
* @param newReclaimAddress New subgraph reclaim address
122-
*/
123-
event SubgraphDeniedReclaimAddressSet(address indexed oldReclaimAddress, address indexed newReclaimAddress);
124-
125-
/**
126-
* @notice Emitted when denied rewards are reclaimed due to eligibility
127-
* @param indexer Address of the indexer whose rewards were denied
128-
* @param allocationID Address of the allocation
121+
* @notice Emitted when rewards are reclaimed to a configured address
122+
* @param reason The reclaim reason identifier
129123
* @param amount Amount of rewards reclaimed
130-
*/
131-
event RewardsReclaimedDueToIndexerEligibility(
132-
address indexed indexer,
133-
address indexed allocationID,
134-
uint256 amount
135-
);
136-
137-
/**
138-
* @notice Emitted when denied rewards are reclaimed due to subgraph denylist
139-
* @param indexer Address of the indexer whose rewards were denied
124+
* @param indexer Address of the indexer
140125
* @param allocationID Address of the allocation
141-
* @param subgraphDeploymentID Subgraph deployment ID that was denied
142-
* @param amount Amount of rewards reclaimed
126+
* @param subgraphDeploymentID Subgraph deployment ID for the allocation
127+
* @param data Additional context data for the reclaim
143128
*/
144-
event RewardsReclaimedDueToSubgraphDenylist(
129+
event RewardsReclaimed(
130+
bytes32 indexed reason,
131+
uint256 amount,
145132
address indexed indexer,
146133
address indexed allocationID,
147-
bytes32 indexed subgraphDeploymentID,
148-
uint256 amount
134+
bytes32 subgraphDeploymentID,
135+
bytes data
149136
);
150137

151138
// -- Modifiers --
@@ -306,27 +293,17 @@ contract RewardsManager is RewardsManagerV6Storage, GraphUpgradeable, IERC165, I
306293

307294
/**
308295
* @inheritdoc IRewardsManager
309-
* @dev Set to zero address to disable eligibility reclaim functionality
296+
* @dev bytes32(0) is reserved as an invalid reason to prevent accidental misconfiguration
297+
* and catch uninitialized reason identifiers.
310298
*/
311-
function setIndexerEligibilityReclaimAddress(address newReclaimAddress) external override onlyGovernor {
312-
address oldReclaimAddress = indexerEligibilityReclaimAddress;
299+
function setReclaimAddress(bytes32 reason, address newAddress) external override onlyGovernor {
300+
require(reason != bytes32(0), "Cannot set reclaim address for (bytes32(0))");
313301

314-
if (oldReclaimAddress != newReclaimAddress) {
315-
indexerEligibilityReclaimAddress = newReclaimAddress;
316-
emit IndexerEligibilityReclaimAddressSet(oldReclaimAddress, newReclaimAddress);
317-
}
318-
}
302+
address oldAddress = reclaimAddresses[reason];
319303

320-
/**
321-
* @inheritdoc IRewardsManager
322-
* @dev Set to zero address to disable subgraph reclaim functionality
323-
*/
324-
function setSubgraphDeniedReclaimAddress(address newReclaimAddress) external override onlyGovernor {
325-
address oldReclaimAddress = subgraphDeniedReclaimAddress;
326-
327-
if (oldReclaimAddress != newReclaimAddress) {
328-
subgraphDeniedReclaimAddress = newReclaimAddress;
329-
emit SubgraphDeniedReclaimAddressSet(oldReclaimAddress, newReclaimAddress);
304+
if (oldAddress != newAddress) {
305+
reclaimAddresses[reason] = newAddress;
306+
emit ReclaimAddressSet(reason, oldAddress, newAddress);
330307
}
331308
}
332309

@@ -561,64 +538,130 @@ contract RewardsManager is RewardsManagerV6Storage, GraphUpgradeable, IERC165, I
561538
}
562539

563540
/**
564-
* @notice Checks for and handles denial and reclaim of rewards due to subgraph deny list
565-
* @dev If denied, emits RewardsDenied event and mints to reclaim address if configured
541+
* @notice Calculate rewards for an allocation
542+
* @param rewardsIssuer Address of the rewards issuer calling the function
543+
* @param allocationID Address of the allocation
544+
* @return rewards Amount of rewards calculated
545+
* @return indexer Address of the indexer
546+
* @return subgraphDeploymentID Subgraph deployment ID
547+
*/
548+
function _calcAllocationRewards(
549+
address rewardsIssuer,
550+
address allocationID
551+
) private returns (uint256 rewards, address indexer, bytes32 subgraphDeploymentID) {
552+
(
553+
bool isActive,
554+
address _indexer,
555+
bytes32 _subgraphDeploymentID,
556+
uint256 tokens,
557+
uint256 accRewardsPerAllocatedToken,
558+
uint256 accRewardsPending
559+
) = IRewardsIssuer(rewardsIssuer).getAllocationData(allocationID);
560+
561+
uint256 updatedAccRewardsPerAllocatedToken = onSubgraphAllocationUpdate(_subgraphDeploymentID);
562+
563+
rewards = isActive
564+
? accRewardsPending.add(
565+
_calcRewards(tokens, accRewardsPerAllocatedToken, updatedAccRewardsPerAllocatedToken)
566+
)
567+
: 0;
568+
569+
indexer = _indexer;
570+
subgraphDeploymentID = _subgraphDeploymentID;
571+
}
572+
573+
/**
574+
* @notice Common function to reclaim rewards to a configured address
575+
* @param reason The reclaim reason identifier
576+
* @param rewards Amount of rewards to reclaim
566577
* @param indexer Address of the indexer
567578
* @param allocationID Address of the allocation
568-
* @param subgraphDeploymentID Subgraph deployment ID
569-
* @param rewards Amount of rewards that would be distributed
570-
* @return True if rewards are denied, false otherwise
579+
* @param subgraphDeploymentID Subgraph deployment ID for the allocation
580+
* @param data Additional context data for the reclaim
581+
* @return reclaimed The amount of rewards that were reclaimed (0 if no reclaim address set)
571582
*/
572-
function _rewardsDeniedDueToSubgraphDenyList(
583+
function _reclaimRewards(
584+
bytes32 reason,
585+
uint256 rewards,
573586
address indexer,
574587
address allocationID,
575588
bytes32 subgraphDeploymentID,
576-
uint256 rewards
577-
) private returns (bool) {
578-
if (isDenied(subgraphDeploymentID)) {
579-
emit RewardsDenied(indexer, allocationID);
580-
581-
// If a reclaim address is set, mint the denied rewards there
582-
if (0 < rewards && subgraphDeniedReclaimAddress != address(0)) {
583-
graphToken().mint(subgraphDeniedReclaimAddress, rewards);
584-
emit RewardsReclaimedDueToSubgraphDenylist(indexer, allocationID, subgraphDeploymentID, rewards);
585-
}
586-
return true;
589+
bytes memory data
590+
) private returns (uint256 reclaimed) {
591+
address target = reclaimAddresses[reason];
592+
if (0 < rewards && target != address(0)) {
593+
graphToken().mint(target, rewards);
594+
emit RewardsReclaimed(reason, rewards, indexer, allocationID, subgraphDeploymentID, data);
595+
reclaimed = rewards;
587596
}
588-
return false;
589597
}
590598

591599
/**
592-
* @notice Checks for and handles denial and reclaim of rewards due to indexer eligibility
593-
* @dev If denied, emits RewardsDeniedDueToEligibility event and mints to reclaim address if configured
600+
* @notice Check if rewards should be denied and attempt to reclaim them
601+
* @param rewards Amount of rewards to check
594602
* @param indexer Address of the indexer
595603
* @param allocationID Address of the allocation
596-
* @param rewards Amount of rewards that would be distributed
597-
* @return True if rewards are denied, false otherwise
598-
*/
599-
function _rewardsDeniedDueToIndexerEligibility(
604+
* @param subgraphDeploymentID Subgraph deployment ID for the allocation
605+
* @return denied True if rewards should be denied (either reclaimed or dropped), false if they should be minted
606+
* @dev First successful reclaim wins - checks performed in order with short-circuit on reclaim:
607+
* 1. Subgraph deny list: emit RewardsDenied. If reclaim address set → reclaim and return (STOP, eligibility not checked)
608+
* 2. Indexer eligibility: Checked if subgraph not denied OR denied without reclaim address. Emit RewardsDeniedDueToEligibility. If reclaim address set → reclaim and return
609+
* Multiple denial events may be emitted only when multiple checks fail without reclaim addresses configured.
610+
* Any failing check without a reclaim address still denies rewards (drops them without minting).
611+
*/
612+
function _deniedRewards(
613+
uint256 rewards,
600614
address indexer,
601615
address allocationID,
602-
uint256 rewards
603-
) private returns (bool) {
616+
bytes32 subgraphDeploymentID
617+
) private returns (bool denied) {
618+
if (isDenied(subgraphDeploymentID)) {
619+
emit RewardsDenied(indexer, allocationID);
620+
if (
621+
0 <
622+
_reclaimRewards(
623+
RewardsReclaim.SUBGRAPH_DENIED,
624+
rewards,
625+
indexer,
626+
allocationID,
627+
subgraphDeploymentID,
628+
""
629+
)
630+
) {
631+
return true; // Successfully reclaimed, deny rewards
632+
}
633+
denied = true; // Denied but no reclaim address
634+
}
635+
604636
if (address(rewardsEligibilityOracle) != address(0) && !rewardsEligibilityOracle.isEligible(indexer)) {
605637
emit RewardsDeniedDueToEligibility(indexer, allocationID, rewards);
606-
607-
// If a reclaim address is set, mint the denied rewards there
608-
if (0 < rewards && indexerEligibilityReclaimAddress != address(0)) {
609-
graphToken().mint(indexerEligibilityReclaimAddress, rewards);
610-
emit RewardsReclaimedDueToIndexerEligibility(indexer, allocationID, rewards);
638+
if (
639+
0 <
640+
_reclaimRewards(
641+
RewardsReclaim.INDEXER_INELIGIBLE,
642+
rewards,
643+
indexer,
644+
allocationID,
645+
subgraphDeploymentID,
646+
""
647+
)
648+
) {
649+
return true; // Successfully reclaimed, deny rewards
611650
}
612-
return true;
651+
denied = true; // Denied but no reclaim address
613652
}
614-
return false;
615653
}
616654

617655
/**
618656
* @inheritdoc IRewardsManager
619657
* @dev This function can only be called by an authorized rewards issuer which are
620658
* the staking contract (for legacy allocations), and the subgraph service (for new allocations).
621659
* Mints 0 tokens if the allocation is not active.
660+
* @dev First successful reclaim wins - short-circuits on reclaim:
661+
* - If subgraph denied with reclaim address → reclaim to SUBGRAPH_DENIED address (eligibility NOT checked)
662+
* - If subgraph not denied OR denied without address, then check eligibility → reclaim to INDEXER_INELIGIBLE if configured
663+
* - Subsequent denial emitted only when earlier denial has no reclaim address
664+
* - Any denial without reclaim address drops rewards (no minting)
622665
*/
623666
function takeRewards(address _allocationID) external override returns (uint256) {
624667
address rewardsIssuer = msg.sender;
@@ -627,39 +670,37 @@ contract RewardsManager is RewardsManagerV6Storage, GraphUpgradeable, IERC165, I
627670
"Caller must be a rewards issuer"
628671
);
629672

630-
(
631-
bool isActive,
632-
address indexer,
633-
bytes32 subgraphDeploymentID,
634-
uint256 tokens,
635-
uint256 accRewardsPerAllocatedToken,
636-
uint256 accRewardsPending
637-
) = IRewardsIssuer(rewardsIssuer).getAllocationData(_allocationID);
638-
639-
uint256 updatedAccRewardsPerAllocatedToken = onSubgraphAllocationUpdate(subgraphDeploymentID);
673+
(uint256 rewards, address indexer, bytes32 subgraphDeploymentID) = _calcAllocationRewards(
674+
rewardsIssuer,
675+
_allocationID
676+
);
640677

641-
uint256 rewards = 0;
642-
if (isActive) {
643-
// Calculate rewards accrued by this allocation
644-
rewards = accRewardsPending.add(
645-
_calcRewards(tokens, accRewardsPerAllocatedToken, updatedAccRewardsPerAllocatedToken)
646-
);
647-
}
678+
if (rewards == 0) return 0;
679+
if (_deniedRewards(rewards, indexer, _allocationID, subgraphDeploymentID)) return 0;
648680

649-
if (_rewardsDeniedDueToSubgraphDenyList(indexer, _allocationID, subgraphDeploymentID, rewards)) return 0;
681+
graphToken().mint(rewardsIssuer, rewards);
682+
emit HorizonRewardsAssigned(indexer, _allocationID, rewards);
650683

651-
if (_rewardsDeniedDueToIndexerEligibility(indexer, _allocationID, rewards)) return 0;
684+
return rewards;
685+
}
652686

653-
// Mint rewards to the rewards issuer
654-
if (rewards > 0) {
655-
// Mint directly to rewards issuer for the reward amount
656-
// The rewards issuer contract will do bookkeeping of the reward and
657-
// assign in proportion to each stakeholder incentive
658-
graphToken().mint(rewardsIssuer, rewards);
659-
}
687+
/**
688+
* @inheritdoc IRewardsManager
689+
* @dev bytes32(0) as a reason is reserved as a no-op and will not be reclaimed.
690+
*/
691+
function reclaimRewards(
692+
bytes32 reason,
693+
address allocationID,
694+
bytes calldata data
695+
) external override returns (uint256) {
696+
address rewardsIssuer = msg.sender;
697+
require(rewardsIssuer == address(subgraphService), "Not a rewards issuer");
660698

661-
emit HorizonRewardsAssigned(indexer, _allocationID, rewards);
699+
(uint256 rewards, address indexer, bytes32 subgraphDeploymentID) = _calcAllocationRewards(
700+
rewardsIssuer,
701+
allocationID
702+
);
662703

663-
return rewards;
704+
return _reclaimRewards(reason, rewards, indexer, allocationID, subgraphDeploymentID, data);
664705
}
665706
}

packages/contracts/contracts/rewards/RewardsManagerStorage.sol

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ contract RewardsManagerV6Storage is RewardsManagerV5Storage {
9090
IRewardsEligibility public rewardsEligibilityOracle;
9191
/// @notice Address of the issuance allocator
9292
IIssuanceAllocationDistribution public issuanceAllocator;
93-
/// @notice Address to receive tokens denied due to indexer eligibility checks, set to zero to disable
94-
address public indexerEligibilityReclaimAddress;
95-
/// @notice Address to receive tokens denied due to subgraph denylist, set to zero to disable
96-
address public subgraphDeniedReclaimAddress;
93+
/// @notice Mapping of reclaim reason identifiers to reclaim addresses
94+
/// @dev Uses bytes32 for extensibility. See RewardsReclaim library for canonical reasons.
95+
mapping(bytes32 => address) public reclaimAddresses;
9796
}

packages/contracts/contracts/tests/MockSubgraphService.sol

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,28 @@ contract MockSubgraphService is IRewardsIssuer {
102102
function getSubgraphAllocatedTokens(bytes32 subgraphDeploymentId) external view override returns (uint256) {
103103
return subgraphAllocatedTokens[subgraphDeploymentId];
104104
}
105+
106+
/**
107+
* @notice Helper function to call reclaimRewards on RewardsManager for testing
108+
* @param rewardsManager Address of the RewardsManager contract
109+
* @param reason Reason identifier for reclaiming rewards
110+
* @param allocationId The allocation ID
111+
* @param contextData Additional context data for the reclaim
112+
* @return Amount of rewards reclaimed
113+
*/
114+
function callReclaimRewards(
115+
address rewardsManager,
116+
bytes32 reason,
117+
address allocationId,
118+
bytes calldata contextData
119+
) external returns (uint256) {
120+
// Call reclaimRewards on the RewardsManager
121+
// solhint-disable-next-line avoid-low-level-calls
122+
(bool success, bytes memory data) = rewardsManager.call(
123+
// solhint-disable-next-line gas-small-strings
124+
abi.encodeWithSignature("reclaimRewards(bytes32,address,bytes)", reason, allocationId, contextData)
125+
);
126+
require(success, "reclaimRewards call failed");
127+
return abi.decode(data, (uint256));
128+
}
105129
}

0 commit comments

Comments
 (0)