The Graph team found an edge case condition in which the amount of accumulated rewards retrieved from the RewardsManager
for a specific signal can be zero.
The main condition is that, on a subgraph having allocations, the curation signal is removed entirely.
In this case, if someone calls the _closeAllocation
function of the Staking
contract, this will internally call the _distributeRewards
function and finally the takeRewards
function of the RewardsManager
.
The problem is that the takeRewards
function is internally calling the onSubgraphAllocationUpdate
function which will call getAccRewardsPerAllocatedToken
and the getAccRewardsForSubgraph
functions. This last function should return the rewards accumulated over time for a given subgraph. Whenever the execution reach this function call, if there are no tokens in the curation pool the result will be zero and the result of the onSubgraphAllocationUpdate
call will return accRewardsPerAllocatedToken == 0
.
Finally the takeRewards
function will call the _calcRewards
function that will return zero too, and zero rewards will be minted.
The intention of the getAccRewardsForSubgraph
function is to retrieve an ever increasing number, but under such conditions, zero would be returned, no matter the previous returned value.
The Graph team addressed this issue in PR#452.
The changes are the following:
newAccrued
and newValue
variables of the getAccRewardsForSubgraph
function are now newRewardsPerSignal
and newRewards
accordingly. The newAccrued
and newValue
variables of the getAccRewardsPerAllocatedToken
are now newRewardsForSubgraph
and newRewardsPerAllocatedToken
. Finally, the newAccrued
variable of the _calcRewards
function has been renamed to newRewardsPerAllocatedTokens
.getAccRewardsForSubgraph
function is not returning zero anymore on an empty pool. Instead it will return now the old value with no new rewards added.takeRewards
function to avoid minting zero rewards.We are very happy with the small and modular changes that The Graph is performing and we are glad that this edge case has been spotted and solved.
This pull request has been audited during the course of two days by one auditor and reviewed by a reviewer during the course of one day.
The only note to report is the fact that the takeRewards
function is returning zero whenever accRewardsPerAllocatedToken
has the same value as acc.accRewardsPerAllocatedToken
(when there are no new rewards), but before that, the _calcRewards
functions and the if
statement are evaluated even if they will have no effects. To be more gas efficient, consider returning earlier to avoid performing useless operations.