Issuance allocator audit responses (for Graph_IssuanceAllocator_v01)

packages/contracts/contracts/rewards/RewardsManagerStorage.sol

@@ -7,7 +7,7 @@
 
 pragma solidity ^0.7.6 || 0.8.27;
 
 import { IIssuanceAllocationDistribution } from "@graphprotocol/interfaces/contracts/issuance/allocate/IIssuanceAllocationDistribution.sol";
 import { IRewardsEligibility } from "@graphprotocol/interfaces/contracts/issuance/eligibility/IRewardsEligibility.sol";
 import { IRewardsIssuer } from "@graphprotocol/interfaces/contracts/contracts/rewards/IRewardsIssuer.sol";
 import { IRewardsManager } from "@graphprotocol/interfaces/contracts/contracts/rewards/IRewardsManager.sol";
@@ -90,8 +90,7 @@
     IRewardsEligibility public rewardsEligibilityOracle;
     /// @notice Address of the issuance allocator
     IIssuanceAllocationDistribution public issuanceAllocator;
-    /// @notice Address to receive tokens denied due to indexer eligibility checks, set to zero to disable
-    address public indexerEligibilityReclaimAddress;
-    /// @notice Address to receive tokens denied due to subgraph denylist, set to zero to disable
-    address public subgraphDeniedReclaimAddress;
+    /// @notice Mapping of reclaim reason identifiers to reclaim addresses
+    /// @dev Uses bytes32 for extensibility. See RewardsReclaim library for canonical reasons.
+    mapping(bytes32 => address) public reclaimAddresses;
 }

packages/contracts/contracts/tests/MockIssuanceAllocator.sol

@@ -58,9 +58,9 @@ contract MockIssuanceAllocator is IERC165, IIssuanceAllocationDistribution {
             IIssuanceTarget(target).beforeIssuanceAllocationChange();
         }
         _targetIssuance[target] = TargetIssuancePerBlock({
-            allocatorIssuancePerBlock: allocatorIssuance,
+            allocatorIssuanceRate: allocatorIssuance,
             allocatorIssuanceBlockAppliedTo: block.number,
-            selfIssuancePerBlock: selfIssuance,
+            selfIssuanceRate: selfIssuance,
             selfIssuanceBlockAppliedTo: block.number
         });
     }

packages/contracts/contracts/tests/MockSubgraphService.sol

@@ -102,4 +102,28 @@ contract MockSubgraphService is IRewardsIssuer {
     function getSubgraphAllocatedTokens(bytes32 subgraphDeploymentId) external view override returns (uint256) {
         return subgraphAllocatedTokens[subgraphDeploymentId];
     }
+
+    /**
+     * @notice Helper function to call reclaimRewards on RewardsManager for testing
+     * @param rewardsManager Address of the RewardsManager contract
+     * @param reason Reason identifier for reclaiming rewards
+     * @param allocationId The allocation ID
+     * @param contextData Additional context data for the reclaim
+     * @return Amount of rewards reclaimed
+     */
+    function callReclaimRewards(
+        address rewardsManager,
+        bytes32 reason,
+        address allocationId,
+        bytes calldata contextData
+    ) external returns (uint256) {
+        // Call reclaimRewards on the RewardsManager
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, bytes memory data) = rewardsManager.call(
+            // solhint-disable-next-line gas-small-strings
+            abi.encodeWithSignature("reclaimRewards(bytes32,address,bytes)", reason, allocationId, contextData)
+        );
+        require(success, "reclaimRewards call failed");
+        return abi.decode(data, (uint256));
+    }
 }

packages/contracts/test/tests/unit/rewards/rewards-distribution.test.ts

@@ -21,6 +21,10 @@ import { NetworkFixture } from '../lib/fixtures'
 
 const MAX_PPM = 1000000
 
+// TODO: Behavior change - HorizonRewardsAssigned is no longer emitted when rewards == 0
+// Set to true if the old behavior is restored (emitting event for zero rewards)
+const EMIT_EVENT_FOR_ZERO_REWARDS = false
+
 const { HashZero, WeiPerEther } = constants
 
 const toRound = (n: BigNumber) => formatGRT(n.add(toGRT('0.5'))).split('.')[0]
@@ -321,9 +325,13 @@ describe('Rewards - Distribution', () => {
 
         // Close allocation. At this point rewards should be collected for that indexer
         const tx = staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
-        await expect(tx)
-          .emit(rewardsManager, 'HorizonRewardsAssigned')
-          .withArgs(indexer1.address, allocationID1, toBN(0))
+        if (EMIT_EVENT_FOR_ZERO_REWARDS) {
+          await expect(tx)
+            .emit(rewardsManager, 'HorizonRewardsAssigned')
+            .withArgs(indexer1.address, allocationID1, toBN(0))
+        } else {
+          await expect(tx).to.not.emit(rewardsManager, 'HorizonRewardsAssigned')
+        }
       })
 
       it('does not revert with an underflow if the minimum signal changes, and signal came after allocation', async function () {
@@ -339,9 +347,13 @@ describe('Rewards - Distribution', () => {
 
         // Close allocation. At this point rewards should be collected for that indexer
         const tx = staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
-        await expect(tx)
-          .emit(rewardsManager, 'HorizonRewardsAssigned')
-          .withArgs(indexer1.address, allocationID1, toBN(0))
+        if (EMIT_EVENT_FOR_ZERO_REWARDS) {
+          await expect(tx)
+            .emit(rewardsManager, 'HorizonRewardsAssigned')
+            .withArgs(indexer1.address, allocationID1, toBN(0))
+        } else {
+          await expect(tx).to.not.emit(rewardsManager, 'HorizonRewardsAssigned')
+        }
       })
 
       it('does not revert if signal was already under minimum', async function () {
@@ -356,9 +368,13 @@ describe('Rewards - Distribution', () => {
         // Close allocation. At this point rewards should be collected for that indexer
         const tx = staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
 
-        await expect(tx)
-          .emit(rewardsManager, 'HorizonRewardsAssigned')
-          .withArgs(indexer1.address, allocationID1, toBN(0))
+        if (EMIT_EVENT_FOR_ZERO_REWARDS) {
+          await expect(tx)
+            .emit(rewardsManager, 'HorizonRewardsAssigned')
+            .withArgs(indexer1.address, allocationID1, toBN(0))
+        } else {
+          await expect(tx).to.not.emit(rewardsManager, 'HorizonRewardsAssigned')
+        }
       })
 
       it('should distribute rewards on closed allocation and send to destination', async function () {
@@ -499,7 +515,11 @@ describe('Rewards - Distribution', () => {
 
         // Close allocation. At this point rewards should be zero
         const tx = staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
-        await expect(tx).emit(rewardsManager, 'HorizonRewardsAssigned').withArgs(indexer1.address, allocationID1, 0)
+        if (EMIT_EVENT_FOR_ZERO_REWARDS) {
+          await expect(tx).emit(rewardsManager, 'HorizonRewardsAssigned').withArgs(indexer1.address, allocationID1, 0)
+        } else {
+          await expect(tx).to.not.emit(rewardsManager, 'HorizonRewardsAssigned')
+        }
 
         // After state - should be unchanged since no rewards were minted
         const afterTokenSupply = await grt.totalSupply()

packages/contracts/test/tests/unit/rewards/rewards-eligibility-oracle.test.ts

@@ -256,15 +256,20 @@ describe('Rewards - Eligibility Oracle', () => {
       // Jump to next epoch
       await helpers.mineEpoch(epochManager)
 
-      // Close allocation - denylist should be checked first
+      // Close allocation - both checks will be performed
       const tx = staking.connect(indexer1).closeAllocation(allocationID1, randomHexBytes())
 
-      // Verify: Denylist wins (checked first in RewardsManager.takeRewards line 522)
-      // Should emit RewardsDenied (not RewardsDeniedDueToEligibility)
+      const expectedIndexingRewards = toGRT('1400')
+
+      // Verify: Both denial events are emitted (new "first successful reclaim" behavior)
+      // Since neither has a reclaim address configured, both checks run and both events emit
       await expect(tx).emit(rewardsManager, 'RewardsDenied').withArgs(indexer1.address, allocationID1)
+      await expect(tx)
+        .emit(rewardsManager, 'RewardsDeniedDueToEligibility')
+        .withArgs(indexer1.address, allocationID1, expectedIndexingRewards)
 
-      // Verify: REO event is NOT emitted
-      await expect(tx).to.not.emit(rewardsManager, 'RewardsDeniedDueToEligibility')
+      // Rewards are dropped (no reclaim happens since neither has address configured)
+      await expect(tx).to.not.emit(rewardsManager, 'RewardsReclaimed')
     })
 
     it('should check REO when denylist allows but indexer ineligible', async function () {

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

@@ -57,7 +57,7 @@ describe('RewardsManager interfaces', () => {
     })
 
     it('IRewardsManager should have stable interface ID', () => {
-      expect(IRewardsManager__factory.interfaceId).to.equal('0x731e44f0')
+      expect(IRewardsManager__factory.interfaceId).to.equal('0x45dd0aa0')
     })
   })