GIP-0003: Accumulated subgraph rewards reset to zero on edge case

Description

The RewardsManager contract’s primary responsibility is to calculate rewards accrued for allocations based on time, the total token supply, tokens signaled on subgraphs, accumulated rewards on a subgraph, and allocations. Under specific edge conditions, the calculation could fail, making a closing allocation that would collect rewards to revert.

Problem

There is a particular edge case in the rewards calculation process under the following sequence:

  • A Subgraph is curated.
  • There are running allocations on that Subgraph.
  • The total amount of curation tokens is removed back to zero by burning ALL the signal.
  • An indexer calls closeAllocation() on an allocation related to the Subgraph.

If this happens, the indexer won’t be able to close that allocation unless it is opting for no rewards by sending POI=0x0. The reason for this is that getAccRewardsForSubgraph() is reseting the accumulated rewards for a subgraph when the curation pool is empty. The consequence is that subgraph.accRewardsForSubgraphSnapshot can be larger than subgraph.accRewardsForSubgraph leading to a revert when calculating new accrued rewards.

Workaround

An indexer could close an allocation passing 0x0 as the POI to skip the rewards calculation.

Solution

Remove the early check within getAccRewardsForSubgraph() that returns zero for the subgraph accumulated rewards when the signal is currently zero. That way, the function will always return the accrued amount, as it should be an ever-increasing number.

Implementation

5 Likes

Community voting is up on Snapshot →
https://snapshot.page/#/graphprotocol.eth/proposal/Qmd1kNg7wMUVaL62wQrCwTnno6rmFHt85zMa5f5SGss186

1 Like

The contract changes have been audited by OpenZeppelin, you can see the full report on this link.

4 Likes

Update: the improvement described here (GIP-0003) was approved by The Graph Council in Graph Governance Proposal-0002.

Subsequently, the upgrade was completed in this transaction. Everything appears healthy and no issues have been reported as a result of the upgrade.

2 Likes