Merge pull request #549 from graphprotocol/ariel/zero-alloc

abarmat · web-flow · commit 6bfec1a72bd1 · 2022-07-13T11:37:05.000-03:00
Create altruistic allocations and close stale allocations
diff --git a/audits/ConsenSysDiligence/2022-03-graph-altruistic-alloc-and-query-versioning.pdf b/audits/ConsenSysDiligence/2022-03-graph-altruistic-alloc-and-query-versioning.pdf
diff --git a/contracts/epochs/EpochManager.sol b/contracts/epochs/EpochManager.sol
@@ -29,7 +29,9 @@ contract EpochManager is EpochManagerV1Storage, GraphUpgradeable, IEpochManager
 
         Managed._initialize(_controller);
 
-        lastLengthUpdateEpoch = 0;
+        // NOTE: We make the first epoch to be one instead of zero to avoid any issue
+        // with composing contracts that may use zero as an empty value
+        lastLengthUpdateEpoch = 1;
         lastLengthUpdateBlock = blockNum();
         epochLength = _epochLength;
 
diff --git a/contracts/staking/Staking.sol b/contracts/staking/Staking.sol
@@ -135,7 +135,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
      * An amount of `tokens` get unallocated from `subgraphDeploymentID`.
      * The `effectiveAllocation` are the tokens allocated from creation to closing.
      * This event also emits the POI (proof of indexing) submitted by the indexer.
-     * `isDelegator` is true if the sender was one of the indexer's delegators.
+     * `isPublic` is true if the sender was someone other than the indexer.
      */
     event AllocationClosed(
         address indexed indexer,
@@ -146,7 +146,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         uint256 effectiveAllocation,
         address sender,
         bytes32 poi,
-        bool isDelegator
+        bool isPublic
     );
 
     /**
@@ -1106,9 +1106,6 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
     ) private {
         require(_isAuth(_indexer), "!auth");
 
-        // Only allocations with a non-zero token amount are allowed
-        require(_tokens > 0, "!tokens");
-
         // Check allocation
         require(_allocationID != address(0), "!alloc");
         require(_getAllocationState(_allocationID) == AllocationState.Null, "!null");
@@ -1119,8 +1116,16 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         bytes32 digest = ECDSA.toEthSignedMessageHash(messageHash);
         require(ECDSA.recover(digest, _proof) == _allocationID, "!proof");
 
-        // Needs to have free capacity not used for other purposes to allocate
-        require(getIndexerCapacity(_indexer) >= _tokens, "!capacity");
+        if (_tokens > 0) {
+            // Needs to have free capacity not used for other purposes to allocate
+            require(getIndexerCapacity(_indexer) >= _tokens, "!capacity");
+        } else {
+            // Allocating zero-tokens still needs to comply with stake requirements
+            require(
+                stakes[_indexer].tokensSecureStake() >= minimumIndexerStake,
+                "!minimumIndexerStake"
+            );
+        }
 
         // Creates an allocation
         // Allocation identifiers are not reused
@@ -1133,18 +1138,23 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
             0, // closedAtEpoch
             0, // Initialize collected fees
             0, // Initialize effective allocation
-            _updateRewards(_subgraphDeploymentID) // Initialize accumulated rewards per stake allocated
+            (_tokens > 0) ? _updateRewards(_subgraphDeploymentID) : 0 // Initialize accumulated rewards per stake allocated
         );
         allocations[_allocationID] = alloc;
 
-        // Mark allocated tokens as used
-        stakes[_indexer].allocate(alloc.tokens);
+        // -- Rewards Distribution --
 
-        // Track total allocations per subgraph
-        // Used for rewards calculations
-        subgraphAllocations[alloc.subgraphDeploymentID] = subgraphAllocations[
-            alloc.subgraphDeploymentID
-        ].add(alloc.tokens);
+        // Process non-zero-allocation rewards tracking
+        if (_tokens > 0) {
+            // Mark allocated tokens as used
+            stakes[_indexer].allocate(alloc.tokens);
+
+            // Track total allocations per subgraph
+            // Used for rewards calculations
+            subgraphAllocations[alloc.subgraphDeploymentID] = subgraphAllocations[
+                alloc.subgraphDeploymentID
+            ].add(alloc.tokens);
+        }
 
         emit AllocationCreated(
             _indexer,
@@ -1175,24 +1185,26 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         require(epochs > 0, "<epochs");
 
         // Indexer or operator can close an allocation
-        // Delegators are also allowed but only after maxAllocationEpochs passed
+        // Anyone is allowed to close ONLY under two concurrent conditions
+        // - After maxAllocationEpochs passed
+        // - When the allocation is for non-zero amount of tokens
         bool isIndexer = _isAuth(alloc.indexer);
-        if (epochs > maxAllocationEpochs) {
-            require(isIndexer || isDelegator(alloc.indexer, msg.sender), "!auth-or-del");
-        } else {
+        if (epochs <= maxAllocationEpochs || alloc.tokens == 0) {
             require(isIndexer, "!auth");
         }
 
+        // Close the allocation and start counting a period to settle remaining payments from
+        // state channels.
+        allocations[_allocationID].closedAtEpoch = alloc.closedAtEpoch;
+
+        // -- Rebate Pool --
+
         // Calculate effective allocation for the amount of epochs it remained allocated
         alloc.effectiveAllocation = _getEffectiveAllocation(
             maxAllocationEpochs,
             alloc.tokens,
             epochs
         );
-
-        // Close the allocation and start counting a period to settle remaining payments from
-        // state channels.
-        allocations[_allocationID].closedAtEpoch = alloc.closedAtEpoch;
         allocations[_allocationID].effectiveAllocation = alloc.effectiveAllocation;
 
         // Account collected fees and effective allocation in rebate pool for the epoch
@@ -1202,21 +1214,26 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         }
         rebatePool.addToPool(alloc.collectedFees, alloc.effectiveAllocation);
 
-        // Distribute rewards if proof of indexing was presented by the indexer or operator
-        if (isIndexer && _poi != 0) {
-            _distributeRewards(_allocationID, alloc.indexer);
-        } else {
-            _updateRewards(alloc.subgraphDeploymentID);
-        }
+        // -- Rewards Distribution --
 
-        // Free allocated tokens from use
-        stakes[alloc.indexer].unallocate(alloc.tokens);
+        // Process non-zero-allocation rewards tracking
+        if (alloc.tokens > 0) {
+            // Distribute rewards if proof of indexing was presented by the indexer or operator
+            if (isIndexer && _poi != 0) {
+                _distributeRewards(_allocationID, alloc.indexer);
+            } else {
+                _updateRewards(alloc.subgraphDeploymentID);
+            }
 
-        // Track total allocations per subgraph
-        // Used for rewards calculations
-        subgraphAllocations[alloc.subgraphDeploymentID] = subgraphAllocations[
-            alloc.subgraphDeploymentID
-        ].sub(alloc.tokens);
+            // Free allocated tokens from use
+            stakes[alloc.indexer].unallocate(alloc.tokens);
+
+            // Track total allocations per subgraph
+            // Used for rewards calculations
+            subgraphAllocations[alloc.subgraphDeploymentID] = subgraphAllocations[
+                alloc.subgraphDeploymentID
+            ].sub(alloc.tokens);
+        }
 
         emit AllocationClosed(
             alloc.indexer,
@@ -1258,8 +1275,8 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         // Purge allocation data except for:
         // - indexer: used in disputes and to avoid reusing an allocationID
         // - subgraphDeploymentID: used in disputes
-        allocations[_allocationID].tokens = 0; // This avoid collect(), close() and claim() to be called
-        allocations[_allocationID].createdAtEpoch = 0;
+        allocations[_allocationID].tokens = 0;
+        allocations[_allocationID].createdAtEpoch = 0; // This avoid collect(), close() and claim() to be called
         allocations[_allocationID].closedAtEpoch = 0;
         allocations[_allocationID].collectedFees = 0;
         allocations[_allocationID].effectiveAllocation = 0;
@@ -1529,7 +1546,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
         if (alloc.indexer == address(0)) {
             return AllocationState.Null;
         }
-        if (alloc.tokens == 0) {
+        if (alloc.createdAtEpoch == 0) {
             return AllocationState.Claimed;
         }
 
diff --git a/contracts/staking/libs/Rebates.sol b/contracts/staking/libs/Rebates.sol
@@ -14,6 +14,7 @@ import "./Cobbs.sol";
  */
 library Rebates {
     using SafeMath for uint256;
+    using Rebates for Rebates.Pool;
 
     // Tracks stats for allocations closed on a particular epoch for claiming
     // The pool also keeps tracks of total query fees collected and stake used
@@ -86,7 +87,7 @@ library Rebates {
         uint256 rebateReward = 0;
 
         // Calculate the rebate rewards for the indexer
-        if (pool.fees > 0) {
+        if (pool.fees > 0 && pool.effectiveAllocatedStake > 0) {
             rebateReward = LibCobbDouglas.cobbDouglas(
                 pool.fees, // totalRewards
                 _indexerFees,
@@ -98,7 +99,7 @@ library Rebates {
             );
 
             // Under NO circumstance we will reward more than total fees in the pool
-            uint256 _unclaimedFees = pool.fees.sub(pool.claimedRewards);
+            uint256 _unclaimedFees = pool.unclaimedFees();
             if (rebateReward > _unclaimedFees) {
                 rebateReward = _unclaimedFees;
             }
diff --git a/contracts/tests/RebatePoolMock.sol b/contracts/tests/RebatePoolMock.sol
@@ -48,7 +48,7 @@ contract RebatePoolMock {
         uint32 _alphaNumerator,
         uint32 _alphaDenominator
     ) external pure returns (uint256) {
-        if (_totalFees == 0) {
+        if (_totalFees == 0 || _totalStake == 0) {
             return 0;
         }
 
diff --git a/test/epochs.test.ts b/test/epochs.test.ts
@@ -68,6 +68,11 @@ describe('EpochManager', () => {
     })
 
     describe('calculations', () => {
+      it('first epoch should be 1', async function () {
+        const currentEpoch = await epochManager.currentEpoch()
+        expect(currentEpoch).eq(1)
+      })
+
       it('should return correct block number', async function () {
         const currentBlock = await latestBlock()
         expect(await epochManager.blockNum()).eq(currentBlock)
diff --git a/test/lib/testHelpers.ts b/test/lib/testHelpers.ts
@@ -83,6 +83,12 @@ export const advanceToNextEpoch = async (epochManager: EpochManager): Promise<vo
   return advanceBlocks(epochLen.sub(blocksSinceEpoch))
 }
 
+export const advanceEpochs = async (epochManager: EpochManager, n: number): Promise<void> => {
+  for (let i = 0; i < n; i++) {
+    await advanceToNextEpoch(epochManager)
+  }
+}
+
 export const evmSnapshot = async (): Promise<number> => provider().send('evm_snapshot', [])
 export const evmRevert = async (id: number): Promise<boolean> => provider().send('evm_revert', [id])
 
diff --git a/test/staking/allocation.test.ts b/test/staking/allocation.test.ts
diff --git a/test/staking/rebate.test.ts b/test/staking/rebate.test.ts

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ contract RebatePoolMock {`
`48`	`48`	`uint32 _alphaNumerator,`
`49`	`49`	`uint32 _alphaDenominator`
`50`	`50`	`) external pure returns (uint256) {`
`51`		`- if (_totalFees == 0) {`
	`51`	`+ if (_totalFees == 0 \|\| _totalStake == 0) {`
`52`	`52`	`return 0;`
`53`	`53`	`}`
`54`	`54`