Merge pull request #640 from graphprotocol/pcv/rewards-underflow

fix: prevent underflow when subgraphs go under the minimum signal threshold
graphprotocol · Sep 1, 2022 · 69dcc9a · 69dcc9a
2 parents f9deb7b + 0ec8778
commit 69dcc9a
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 1 deletion.
diff --git a/contracts/rewards/RewardsManager.sol b/contracts/rewards/RewardsManager.sol
@@ -6,6 +6,7 @@ pragma abicoder v2;
 import "@openzeppelin/contracts/math/SafeMath.sol";
 
 import "../upgrades/GraphUpgradeable.sol";
+import "../staking/libs/MathUtils.sol";
 
 import "./RewardsManagerStorage.sol";
 import "./IRewardsManager.sol";
@@ -281,7 +282,8 @@ contract RewardsManager is RewardsManagerV3Storage, GraphUpgradeable, IRewardsMa
         Subgraph storage subgraph = subgraphs[_subgraphDeploymentID];
 
         uint256 accRewardsForSubgraph = getAccRewardsForSubgraph(_subgraphDeploymentID);
-        uint256 newRewardsForSubgraph = accRewardsForSubgraph.sub(
+        uint256 newRewardsForSubgraph = MathUtils.diffOrZero(
+            accRewardsForSubgraph,
             subgraph.accRewardsForSubgraphSnapshot
         );
 

diff --git a/test/rewards/rewards.test.ts b/test/rewards/rewards.test.ts
@@ -537,6 +537,29 @@ describe('Rewards', () => {
           )
       }
 
+      async function setupIndexerAllocationSignalingAfter() {
+        // Setup
+        await epochManager.setEpochLength(10)
+
+        // Allocate
+        const tokensToAllocate = toGRT('12500')
+        await staking.connect(indexer1.signer).stake(tokensToAllocate)
+        await staking
+          .connect(indexer1.signer)
+          .allocateFrom(
+            indexer1.address,
+            subgraphDeploymentID1,
+            tokensToAllocate,
+            allocationID1,
+            metadata,
+            await channelKey1.generateProof(indexer1.address),
+          )
+
+        // Update total signalled
+        const signalled1 = toGRT('1500')
+        await curation.connect(curator1.signer).mint(subgraphDeploymentID1, signalled1, 0)
+      }
+
       async function setupIndexerAllocationWithDelegation(
         tokensToDelegate: BigNumber,
         delegationParams: DelegationParameters,
@@ -636,6 +659,59 @@ describe('Rewards', () => {
         expect(toRound(afterTokenSupply)).eq(toRound(expectedTokenSupply))
       })
 
+      it('does not revert with an underflow if the minimum signal changes', async function () {
+        // Align with the epoch boundary
+        await advanceToNextEpoch(epochManager)
+        // Setup
+        await setupIndexerAllocation()
+
+        await rewardsManager.connect(governor.signer).setMinimumSubgraphSignal(toGRT(14000))
+
+        // Jump
+        await advanceToNextEpoch(epochManager)
+
+        // Close allocation. At this point rewards should be collected for that indexer
+        const tx = staking.connect(indexer1.signer).closeAllocation(allocationID1, randomHexBytes())
+        await expect(tx)
+          .emit(rewardsManager, 'RewardsAssigned')
+          .withArgs(indexer1.address, allocationID1, await epochManager.currentEpoch(), toBN(0))
+      })
+
+      it('does not revert with an underflow if the minimum signal changes, and signal came after allocation', async function () {
+        // Align with the epoch boundary
+        await advanceToNextEpoch(epochManager)
+        // Setup
+        await setupIndexerAllocationSignalingAfter()
+
+        await rewardsManager.connect(governor.signer).setMinimumSubgraphSignal(toGRT(14000))
+
+        // Jump
+        await advanceToNextEpoch(epochManager)
+
+        // Close allocation. At this point rewards should be collected for that indexer
+        const tx = staking.connect(indexer1.signer).closeAllocation(allocationID1, randomHexBytes())
+        await expect(tx)
+          .emit(rewardsManager, 'RewardsAssigned')
+          .withArgs(indexer1.address, allocationID1, await epochManager.currentEpoch(), toBN(0))
+      })
+
+      it('does not revert if signal was already under minimum', async function () {
+        await rewardsManager.connect(governor.signer).setMinimumSubgraphSignal(toGRT(2000))
+        // Align with the epoch boundary
+        await advanceToNextEpoch(epochManager)
+        // Setup
+        await setupIndexerAllocation()
+
+        // Jump
+        await advanceToNextEpoch(epochManager)
+        // Close allocation. At this point rewards should be collected for that indexer
+        const tx = staking.connect(indexer1.signer).closeAllocation(allocationID1, randomHexBytes())
+
+        await expect(tx)
+          .emit(rewardsManager, 'RewardsAssigned')
+          .withArgs(indexer1.address, allocationID1, await epochManager.currentEpoch(), toBN(0))
+      })
+
       it('should distribute rewards on closed allocation and send to destination', async function () {
         const destinationAddress = randomHexBytes(20)
         await staking.connect(indexer1.signer).setRewardsDestination(destinationAddress)