fix: merge issuance-audit fixes (TRST-R-1/R-2/R-3) into dips-issuance-merge

Author: Maikol

docs/RewardAccountingSafety.md

@@ -82,7 +82,7 @@ _allocations.snapshotRewards(..., onSubgraphAllocationUpdate()); // ③ Updates
 
 **Why this order matters**:
 
-- Step ① with zero allocations → triggers NO_ALLOCATION reclaim for gap period
+- Step ① with zero allocations → triggers NO_ALLOCATED_TOKENS reclaim for gap period
 - Step ② creates allocation → now allocatedTokens > 0
 - Step ③ same block → newRewards ≈ 0, just confirms snapshot
 
@@ -112,7 +112,7 @@ Every reward path that cannot reach an allocation has a reclaim handler:
 | No global signal     | `updateAccRewardsPerSignal()` with signalledTokens = 0       | `NO_SIGNAL`              |
 | Subgraph denied      | `onSubgraphSignalUpdate()` or `onSubgraphAllocationUpdate()` | `SUBGRAPH_DENIED`        |
 | Below minimum signal | `onSubgraphSignalUpdate()` or `onSubgraphAllocationUpdate()` | `BELOW_MINIMUM_SIGNAL`   |
-| No allocations       | `onSubgraphSignalUpdate()` or `onSubgraphAllocationUpdate()` | `NO_ALLOCATION`          |
+| No allocations       | `onSubgraphSignalUpdate()` or `onSubgraphAllocationUpdate()` | `NO_ALLOCATED_TOKENS`    |
 | Indexer ineligible   | `takeRewards()`                                              | `INDEXER_INELIGIBLE`     |
 | Stale/zero POI       | `_presentPoi()`                                              | `STALE_POI` / `ZERO_POI` |
 | Allocation close     | `_closeAllocation()`                                         | `CLOSE_ALLOCATION`       |
@@ -124,7 +124,7 @@ Every reward path that cannot reach an allocation has a reclaim handler:
 | Failure Mode                 | How Prevented                                                                        |
 | ---------------------------- | ------------------------------------------------------------------------------------ |
 | Double-mint same rewards     | Snapshot updated after every claim; same-block calls return ~0                       |
-| Rewards stuck in accumulator | NO_ALLOCATION reclaim before allocation creation                                     |
+| Rewards stuck in accumulator | NO_ALLOCATED_TOKENS reclaim before allocation creation                               |
 | Gap period loss              | `_getAllocationData` calls `onSubgraphAllocationUpdate` before allocation exists     |
 | Denial-period accumulation   | `accRewardsForSubgraph` tracks; `accRewardsPerAllocatedToken` frozen; diff reclaimed |
 | Signal change mid-period     | `onSubgraphSignalUpdate` hook called before signal changes                           |
@@ -148,8 +148,8 @@ RewardsManager and issuers share responsibility for correct reward accounting:
 **Example - Subgraph Denial** (see [RewardConditions.md](./RewardConditions.md#subgraph_denied) for full details):
 
 - RM: Reclaims new rewards; freezes `accRewardsPerAllocatedToken`
-- AM: Defers claim; preserves pre-denial rewards in allocation snapshot
-- After undeny: AM can claim the preserved pre-denial rewards
+- AM: Defers claim; preserves uncollected rewards (no snapshot update)
+- After undeny: AM can claim preserved uncollected rewards
 
 ## Issuer Requirements
 

docs/RewardConditions.md

@@ -4,18 +4,18 @@ Quick reference for all reward conditions and how they are handled across Reward
 
 ## Summary Table
 
-| Condition              | Identifier                          | Handled By        | Action                    | Rewards Outcome                       |
-| ---------------------- | ----------------------------------- | ----------------- | ------------------------- | ------------------------------------- |
-| `NONE`                 | `bytes32(0)`                        | —                 | Normal path               | Claimed by indexer                    |
-| `NO_SIGNAL`            | `keccak256("NO_SIGNAL")`            | RewardsManager    | Reclaim                   | To reclaim address                    |
-| `SUBGRAPH_DENIED`      | `keccak256("SUBGRAPH_DENIED")`      | Both              | Reclaim (RM) / Defer (AM) | New: reclaimed; Pre-denial: preserved |
-| `BELOW_MINIMUM_SIGNAL` | `keccak256("BELOW_MINIMUM_SIGNAL")` | RewardsManager    | Reclaim                   | To reclaim address                    |
-| `NO_ALLOCATION`        | `keccak256("NO_ALLOCATION")`        | RewardsManager    | Reclaim                   | To reclaim address                    |
-| `INDEXER_INELIGIBLE`   | `keccak256("INDEXER_INELIGIBLE")`   | RewardsManager    | Reclaim                   | To reclaim address                    |
-| `STALE_POI`            | `keccak256("STALE_POI")`            | AllocationManager | Reclaim                   | To reclaim address                    |
-| `ZERO_POI`             | `keccak256("ZERO_POI")`             | AllocationManager | Reclaim                   | To reclaim address                    |
-| `ALLOCATION_TOO_YOUNG` | `keccak256("ALLOCATION_TOO_YOUNG")` | AllocationManager | Defer                     | Preserved for later                   |
-| `CLOSE_ALLOCATION`     | `keccak256("CLOSE_ALLOCATION")`     | AllocationManager | Reclaim                   | To reclaim address                    |
+| Condition              | Identifier                          | Handled By        | Action                    | Rewards Outcome                        |
+| ---------------------- | ----------------------------------- | ----------------- | ------------------------- | -------------------------------------- |
+| `NONE`                 | `bytes32(0)`                        | —                 | Normal path               | Claimed by indexer                     |
+| `NO_SIGNAL`            | `keccak256("NO_SIGNAL")`            | RewardsManager    | Reclaim                   | To reclaim address                     |
+| `SUBGRAPH_DENIED`      | `keccak256("SUBGRAPH_DENIED")`      | Both              | Reclaim (RM) / Defer (AM) | New: reclaimed; Uncollected: preserved |
+| `BELOW_MINIMUM_SIGNAL` | `keccak256("BELOW_MINIMUM_SIGNAL")` | RewardsManager    | Reclaim                   | To reclaim address                     |
+| `NO_ALLOCATED_TOKENS`  | `keccak256("NO_ALLOCATED_TOKENS")`  | RewardsManager    | Reclaim                   | To reclaim address                     |
+| `INDEXER_INELIGIBLE`   | `keccak256("INDEXER_INELIGIBLE")`   | RewardsManager    | Reclaim                   | To reclaim address                     |
+| `STALE_POI`            | `keccak256("STALE_POI")`            | AllocationManager | Reclaim                   | To reclaim address                     |
+| `ZERO_POI`             | `keccak256("ZERO_POI")`             | AllocationManager | Reclaim                   | To reclaim address                     |
+| `ALLOCATION_TOO_YOUNG` | `keccak256("ALLOCATION_TOO_YOUNG")` | AllocationManager | Defer                     | Preserved for later                    |
+| `CLOSE_ALLOCATION`     | `keccak256("CLOSE_ALLOCATION")`     | AllocationManager | Reclaim                   | To reclaim address                     |
 
 ## Reward Distribution Levels
 
@@ -36,7 +36,7 @@ Rewards flow through three levels, with reclaim possible at each:
 │  ─────────────────────────────────────────────────────────────────  │
 │  onSubgraphSignalUpdate() / onSubgraphAllocationUpdate()            │
 │                                                                     │
-│  Reclaim: SUBGRAPH_DENIED, BELOW_MINIMUM_SIGNAL, NO_ALLOCATION      │
+│  Reclaim: SUBGRAPH_DENIED, BELOW_MINIMUM_SIGNAL, NO_ALLOCATED_TOKENS      │
 │                                                                     │
 │  Behavior:                                                          │
 │  - accRewardsForSubgraph only increases when claimable              │
@@ -73,20 +73,20 @@ Rewards flow through three levels, with reclaim possible at each:
 
 - **Trigger**: `isDenied(subgraphDeploymentId)` returns true
 - **Effect**: `accRewardsPerAllocatedToken` stops increasing
-- **Handling**: New rewards reclaimed; pre-denial rewards preserved in allocation snapshots
+- **Handling**: New rewards reclaimed; accumulator frozen (uncollected rewards preserved)
 - **Note**: If no SUBGRAPH_DENIED reclaim address AND signal < minimum, reclaims as BELOW_MINIMUM_SIGNAL instead
 
 **Reward disposition by period:**
 
 | Period        | Disposition                                              |
 | ------------- | -------------------------------------------------------- |
-| Pre-denial    | Claimable after undeny                                   |
+| Before denial | Claimable after undeny                                   |
 | During denial | Reclaimed to protocol (or dropped if no reclaim address) |
 | Post-undeny   | Claimable normally                                       |
 
 **Effect on allocations:**
 
-- _Existing allocations_: Pre-denial rewards preserved; cannot claim while denied; claimable after undeny
+- _Existing allocations_: Uncollected rewards preserved (accumulator frozen, snapshot unchanged); cannot claim while denied; claimable after undeny
 - _New allocations (created while denied)_: Start with frozen baseline; only earn rewards after undeny
 - _POI presentation_: Indexers should continue presenting POIs to prevent staleness (returns 0 but maintains allocation health)
 
@@ -104,7 +104,7 @@ Rewards flow through three levels, with reclaim possible at each:
 - **Effect**: `accRewardsPerAllocatedToken` stops increasing
 - **Handling**: Rewards reclaimed to configured address
 
-#### NO_ALLOCATION
+#### NO_ALLOCATED_TOKENS
 
 - **Trigger**: Subgraph has signal but zero allocated tokens
 - **Effect**: Rewards cannot be distributed to allocations
@@ -146,7 +146,7 @@ Conditions checked in order (first match wins):
 
 - **Trigger**: `isDenied(subgraphDeploymentId)` at POI presentation
 - **Effect**: Cannot claim while denied
-- **Handling**: **Deferred** (returns 0, no snapshot update, pre-denial rewards preserved)
+- **Handling**: **Deferred** (returns 0, no snapshot update, uncollected rewards preserved)
 
 #### CLOSE_ALLOCATION
 
@@ -198,6 +198,33 @@ defaultReclaimAddress = treasuryAddress;  // Catch-all
 
 **Important**: Changes apply retroactively to all future reclaims.
 
+## Parameter Changes: minimumSubgraphSignal
+
+### Retroactive Application Risk
+
+When `minimumSubgraphSignal` is changed via `setMinimumSubgraphSignal()`, existing subgraphs are NOT automatically updated. When subgraphs are later updated (via signal/allocation changes), the **current** threshold is applied to ALL pending rewards since their last update, regardless of historical threshold values.
+
+**Impact:**
+
+| Change Direction    | Effect                                                                   |
+| ------------------- | ------------------------------------------------------------------------ |
+| Threshold increases | Pending rewards on previously eligible subgraphs are reclaimed           |
+| Threshold decreases | Previously ineligible subgraphs retroactively accumulate pending rewards |
+
+### Required Mitigation Process
+
+To prevent retroactive application to long historical periods:
+
+1. **Communicate** the planned threshold change with a specific future date
+2. **Wait** - notice period allows participants to adjust signal if desired
+3. **Identify** affected subgraphs off-chain (those crossing the threshold)
+4. **Call** `onSubgraphSignalUpdate()` for all affected subgraphs to accumulate pending rewards under current eligibility rules
+5. **Execute** threshold change via `setMinimumSubgraphSignal()` (promptly after step 4, ideally same block)
+
+**Responsibility:** Governance handles steps 3-5; participants may optionally adjust signal in step 2.
+
+For implementation details, see NatSpec documentation on `RewardsManager.setMinimumSubgraphSignal()`.
+
 ## Key Behaviors
 
 ### Snapshot Updates

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

@@ -402,7 +402,7 @@ describe('Rewards - Calculations', () => {
 
         // Prepare expected results
         // Option B model: accRewardsForSubgraph only tracks distributable rewards
-        // 2 blocks before allocation = reclaimed (NO_ALLOCATION), 5 blocks after = distributable
+        // 2 blocks before allocation = reclaimed (NO_ALLOCATED_TOKENS), 5 blocks after = distributable
         const expectedSubgraphRewards = toGRT('1000') // 5 blocks × 200 GRT/block
         const expectedRewardsAT = toGRT('0.08') // 1000 GRT / 12500 allocated tokens
 

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

@@ -30,7 +30,7 @@ function expectApproxEq(actual: BigNumber, expected: BigNumber, message: string)
     `${message}: difference ${diff.toString()} exceeds tolerance ${REWARDS_TOLERANCE}`,
   ).to.be.true
 }
-const NO_ALLOCATION = utils.id('NO_ALLOCATION')
+const NO_ALLOCATED_TOKENS = utils.id('NO_ALLOCATED_TOKENS')
 const BELOW_MINIMUM_SIGNAL = utils.id('BELOW_MINIMUM_SIGNAL')
 
 describe('Rewards - Reclaim Addresses', () => {
@@ -879,10 +879,10 @@ describe('Rewards - Reclaim Addresses', () => {
     })
   })
 
-  describe('reclaim NO_ALLOCATION - signal but no allocations', function () {
-    it('should reclaim when signal exists but no allocations and NO_ALLOCATION address set', async function () {
-      // Set reclaim address for NO_ALLOCATION
-      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATION, reclaimWallet.address)
+  describe('reclaim NO_ALLOCATED_TOKENS - signal but no allocations', function () {
+    it('should reclaim when signal exists but no allocations and NO_ALLOCATED_TOKENS address set', async function () {
+      // Set reclaim address for NO_ALLOCATED_TOKENS
+      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATED_TOKENS, reclaimWallet.address)
 
       // Create signal but NO allocation
       const signalled1 = toGRT('1500')

packages/contracts-test/tests/unit/rewards/rewards-signal-allocation-update.test.ts

@@ -449,23 +449,23 @@ describe('Rewards: Signal and Allocation Update Accounting', () => {
   })
 
   describe('onSubgraphSignalUpdate with no allocations', function () {
-    it('should reclaim as NO_ALLOCATION when signal exists but no allocations', async function () {
+    it('should reclaim as NO_ALLOCATED_TOKENS when signal exists but no allocations', async function () {
       // Setup: only signal, no allocation
       await grt.connect(governor).mint(curator.address, tokensToSignal)
       await grt.connect(curator).approve(curation.address, tokensToSignal)
       await curation.connect(curator).mint(subgraphDeploymentID, tokensToSignal, 0)
 
-      // Configure reclaim address for NO_ALLOCATION
-      const NO_ALLOCATION = hre.ethers.utils.id('NO_ALLOCATION')
-      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATION, governor.address)
+      // Configure reclaim address for NO_ALLOCATED_TOKENS
+      const NO_ALLOCATED_TOKENS = hre.ethers.utils.id('NO_ALLOCATED_TOKENS')
+      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATED_TOKENS, governor.address)
 
       // Record initial state
       const initialState = await rewardsManager.subgraphs(subgraphDeploymentID)
 
       // Advance blocks - rewards should accumulate
       await helpers.mine(100)
 
-      // Call onSubgraphSignalUpdate - should reclaim as NO_ALLOCATION
+      // Call onSubgraphSignalUpdate - should reclaim as NO_ALLOCATED_TOKENS
       const tx = await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID)
       const receipt = await tx.wait()
       const afterSignalUpdate = await rewardsManager.subgraphs(subgraphDeploymentID)
@@ -476,10 +476,10 @@ describe('Rewards: Signal and Allocation Update Accounting', () => {
         'accRewardsForSubgraph should not change when no allocations (rewards reclaimed)',
       )
 
-      // Verify reclaim event was emitted with NO_ALLOCATION reason
+      // Verify reclaim event was emitted with NO_ALLOCATED_TOKENS reason
       const reclaimEvent = receipt.events?.find((e) => e.event === 'RewardsReclaimed')
       expect(reclaimEvent).to.not.be.undefined
-      expect(reclaimEvent!.args![0]).to.equal(NO_ALLOCATION, 'Should reclaim with NO_ALLOCATION reason')
+      expect(reclaimEvent!.args![0]).to.equal(NO_ALLOCATED_TOKENS, 'Should reclaim with NO_ALLOCATED_TOKENS reason')
       expect(reclaimEvent!.args![1]).to.be.gt(0, 'Should have reclaimed rewards')
     })
 
@@ -496,8 +496,8 @@ describe('Rewards: Signal and Allocation Update Accounting', () => {
       await helpers.mine(100)
 
       // Configure reclaim and call signal update
-      const NO_ALLOCATION = hre.ethers.utils.id('NO_ALLOCATION')
-      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATION, governor.address)
+      const NO_ALLOCATED_TOKENS = hre.ethers.utils.id('NO_ALLOCATED_TOKENS')
+      await rewardsManager.connect(governor).setReclaimAddress(NO_ALLOCATED_TOKENS, governor.address)
       await rewardsManager.connect(governor).onSubgraphSignalUpdate(subgraphDeploymentID)
 
       // View should remain stable (rewards reclaimed)

packages/contracts-test/tests/unit/rewards/rewards-subgraph-service.test.ts

@@ -380,7 +380,7 @@ describe('Rewards - SubgraphService', () => {
         // Mine blocks
         await helpers.mine(5)
 
-        // Get rewards - Option B: with no allocations, rewards are reclaimed (NO_ALLOCATION)
+        // Get rewards - Option B: with no allocations, rewards are reclaimed (NO_ALLOCATED_TOKENS)
         // so both accRewardsPerAllocatedToken and accRewardsForSubgraph remain 0
         const [accRewardsPerAllocatedToken, accRewardsForSubgraph] =
           await rewardsManager.getAccRewardsPerAllocatedToken(subgraphDeploymentID1)

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

@@ -348,7 +348,7 @@ describe('Rewards', () => {
         await curation.connect(curator1).mint(subgraphDeploymentID1, signalled1, 0)
         await curation.connect(curator2).mint(subgraphDeploymentID2, signalled2, 0)
 
-        // Create allocations for both subgraphs so rewards are accumulated (not reclaimed as NO_ALLOCATION)
+        // Create allocations for both subgraphs so rewards are accumulated (not reclaimed as NO_ALLOCATED_TOKENS)
         await grt.connect(governor).mint(indexer1.address, tokensToStake)
         await grt.connect(indexer1).approve(staking.address, tokensToStake)
         await staking.connect(indexer1).stake(tokensToStake)
@@ -426,7 +426,7 @@ describe('Rewards', () => {
         const signalled1 = toGRT('1500')
         await curation.connect(curator1).mint(subgraphDeploymentID1, signalled1, 0)
 
-        // Create an allocation so rewards are accumulated (not reclaimed as NO_ALLOCATION)
+        // Create an allocation so rewards are accumulated (not reclaimed as NO_ALLOCATED_TOKENS)
         const tokensToStake = toGRT('100000')
         const tokensToAllocate = toGRT('10000')
         await grt.connect(governor).mint(indexer1.address, tokensToStake)