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 new file mode 100644 index 000000000..cb9f1329f Binary files /dev/null and b/audits/ConsenSysDiligence/2022-03-graph-altruistic-alloc-and-query-versioning.pdf differ diff --git a/contracts/epochs/EpochManager.sol b/contracts/epochs/EpochManager.sol index 9ac49474a..3f022d4af 100644 --- 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 index 0e582fc83..2bcc8d74d 100644 --- 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, " 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 index c41db407d..9323a9747 100644 --- 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 index b1a46a1ac..5a30f4767 100644 --- 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 index 7ac178c33..2ff87084a 100644 --- 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 index ea07ba449..ee7c87322 100644 --- a/test/lib/testHelpers.ts +++ b/test/lib/testHelpers.ts @@ -83,6 +83,12 @@ export const advanceToNextEpoch = async (epochManager: EpochManager): Promise => { + for (let i = 0; i < n; i++) { + await advanceToNextEpoch(epochManager) + } +} + export const evmSnapshot = async (): Promise => provider().send('evm_snapshot', []) export const evmRevert = async (id: number): Promise => provider().send('evm_revert', [id]) diff --git a/test/rewards/rewards.test.ts b/test/rewards/rewards.test.ts index 5bed9b83f..7c6d45c12 100644 --- a/test/rewards/rewards.test.ts +++ b/test/rewards/rewards.test.ts @@ -573,7 +573,6 @@ describe('Rewards', () => { } it('should distribute rewards on closed allocation and stake', async function () { - // Align with the epoch boundary await advanceToNextEpoch(epochManager) // Setup diff --git a/test/staking/allocation.test.ts b/test/staking/allocation.test.ts index 96b6ca332..67e9120e6 100644 --- a/test/staking/allocation.test.ts +++ b/test/staking/allocation.test.ts @@ -15,6 +15,7 @@ import { toBN, toGRT, Account, + advanceEpochs, } from '../lib/testHelpers' const { AddressZero, HashZero } = constants @@ -53,6 +54,7 @@ describe('Staking:Allocation', () => { let staking: Staking // Test values + const indexerTokens = toGRT('1000') const tokensToStake = toGRT('100') const tokensToAllocate = toGRT('100') @@ -64,6 +66,7 @@ describe('Staking:Allocation', () => { const poi = randomHexBytes() // Helpers + const allocate = async (tokens: BigNumber) => { return staking .connect(indexer.signer) @@ -77,6 +80,177 @@ describe('Staking:Allocation', () => { ) } + const shouldAllocate = async (tokensToAllocate: BigNumber) => { + // Before state + const beforeStake = await staking.stakes(indexer.address) + + // Allocate + const currentEpoch = await epochManager.currentEpoch() + const tx = allocate(tokensToAllocate) + await expect(tx) + .emit(staking, 'AllocationCreated') + .withArgs( + indexer.address, + subgraphDeploymentID, + currentEpoch, + tokensToAllocate, + allocationID, + metadata, + ) + + // After state + const afterStake = await staking.stakes(indexer.address) + const afterAlloc = await staking.getAllocation(allocationID) + + // Stake updated + expect(afterStake.tokensAllocated).eq(beforeStake.tokensAllocated.add(tokensToAllocate)) + // Allocation updated + expect(afterAlloc.indexer).eq(indexer.address) + expect(afterAlloc.subgraphDeploymentID).eq(subgraphDeploymentID) + expect(afterAlloc.tokens).eq(tokensToAllocate) + expect(afterAlloc.createdAtEpoch).eq(currentEpoch) + expect(afterAlloc.collectedFees).eq(toGRT('0')) + expect(afterAlloc.closedAtEpoch).eq(toBN('0')) + expect(afterAlloc.effectiveAllocation).eq(toGRT('0')) + } + + // Claim and perform checks + const shouldClaim = async function (allocationID: string, restake: boolean) { + // Should have a particular state before claiming + expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized) + + // Advance blocks to get the allocation in epoch where it can be claimed + await advanceToNextEpoch(epochManager) + + // Before state + const beforeStake = await staking.stakes(indexer.address) + const beforeAlloc = await staking.allocations(allocationID) + const beforeRebatePool = await staking.rebates(beforeAlloc.closedAtEpoch) + const beforeIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + const beforeIndexerTokens = await grt.balanceOf(indexer.address) + + // Claim rebates + const tokensToClaim = beforeAlloc.effectiveAllocation.eq(0) + ? toBN(0) + : beforeAlloc.collectedFees + const currentEpoch = await epochManager.currentEpoch() + const tx = staking.connect(indexer.signer).claim(allocationID, restake) + await expect(tx) + .emit(staking, 'RebateClaimed') + .withArgs( + indexer.address, + subgraphDeploymentID, + allocationID, + currentEpoch, + beforeAlloc.closedAtEpoch, + tokensToClaim, + beforeRebatePool.unclaimedAllocationsCount - 1, + toGRT('0'), + ) + + // After state + const afterBalance = await grt.balanceOf(indexer.address) + const afterStake = await staking.stakes(indexer.address) + const afterAlloc = await staking.allocations(allocationID) + const afterRebatePool = await staking.rebates(beforeAlloc.closedAtEpoch) + + // Funds distributed to indexer + if (restake) { + expect(afterBalance).eq(beforeIndexerTokens) + } else { + expect(afterBalance).eq(beforeIndexerTokens.add(tokensToClaim)) + } + // Stake updated + if (restake) { + expect(afterStake.tokensStaked).eq(beforeStake.tokensStaked.add(tokensToClaim)) + } else { + expect(afterStake.tokensStaked).eq(beforeStake.tokensStaked) + } + // Allocation updated (purged) + expect(afterAlloc.tokens).eq(toGRT('0')) + expect(afterAlloc.createdAtEpoch).eq(toGRT('0')) + expect(afterAlloc.closedAtEpoch).eq(toGRT('0')) + expect(afterAlloc.collectedFees).eq(toGRT('0')) + expect(afterAlloc.effectiveAllocation).eq(toGRT('0')) + expect(afterAlloc.accRewardsPerAllocatedToken).eq(toGRT('0')) + // Rebate updated + expect(afterRebatePool.unclaimedAllocationsCount).eq( + beforeRebatePool.unclaimedAllocationsCount - 1, + ) + if (afterRebatePool.unclaimedAllocationsCount === 0) { + // Rebate pool is empty and then pruned + expect(afterRebatePool.effectiveAllocatedStake).eq(toGRT('0')) + expect(afterRebatePool.fees).eq(toGRT('0')) + } else { + // There are still more unclaimed allocations in the rebate pool + expect(afterRebatePool.effectiveAllocatedStake).eq(beforeRebatePool.effectiveAllocatedStake) + expect(afterRebatePool.fees).eq(beforeRebatePool.fees.sub(tokensToClaim)) + } + + if (restake) { + // Verify that the claimed tokens are restaked + const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + expect(afterIndexerStake).eq(beforeIndexerStake.add(tokensToClaim)) + } else { + // Verify that the claimed tokens are transferred to the indexer + const afterIndexerTokens = await grt.balanceOf(indexer.address) + expect(afterIndexerTokens).eq(beforeIndexerTokens.add(tokensToClaim)) + } + } + + // This function tests collect with state updates + const shouldCollect = async (tokensToCollect: BigNumber) => { + // Before state + const beforeTokenSupply = await grt.totalSupply() + const beforePool = await curation.pools(subgraphDeploymentID) + const beforeAlloc = await staking.getAllocation(allocationID) + + // Advance blocks to get the allocation in epoch where it can be closed + await advanceToNextEpoch(epochManager) + + // Collect fees and calculate expected results + let rebateFees = tokensToCollect + const protocolPercentage = await staking.protocolPercentage() + const protocolFees = rebateFees.mul(protocolPercentage).div(MAX_PPM) + rebateFees = rebateFees.sub(protocolFees) + + const curationPercentage = await staking.curationPercentage() + const curationFees = rebateFees.mul(curationPercentage).div(MAX_PPM) + rebateFees = rebateFees.sub(curationFees) + + // Collect tokens from allocation + const tx = staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID) + await expect(tx) + .emit(staking, 'AllocationCollected') + .withArgs( + indexer.address, + subgraphDeploymentID, + await epochManager.currentEpoch(), + tokensToCollect, + allocationID, + assetHolder.address, + curationFees, + rebateFees, + ) + + // After state + const afterTokenSupply = await grt.totalSupply() + const afterPool = await curation.pools(subgraphDeploymentID) + const afterAlloc = await staking.getAllocation(allocationID) + + // Check that protocol fees are burnt + expect(afterTokenSupply).eq(beforeTokenSupply.sub(protocolFees)) + // Check that curation reserves increased for the SubgraphDeployment + expect(afterPool.tokens).eq(beforePool.tokens.add(curationFees)) + // Verify allocation is updated and allocation cleaned + expect(afterAlloc.tokens).eq(beforeAlloc.tokens) + expect(afterAlloc.createdAtEpoch).eq(beforeAlloc.createdAtEpoch) + expect(afterAlloc.closedAtEpoch).eq(toBN('0')) + expect(afterAlloc.collectedFees).eq(beforeAlloc.collectedFees.add(rebateFees)) + } + + // -- Tests -- + before(async function () { ;[me, governor, indexer, slasher, assetHolder] = await getAccounts() @@ -164,46 +338,6 @@ describe('Staking:Allocation', () => { * Allocate */ describe('allocate', function () { - const shouldAllocate = async (tokensToAllocate: BigNumber) => { - // Before state - const beforeStake = await staking.stakes(indexer.address) - - // Allocate - const currentEpoch = await epochManager.currentEpoch() - const tx = allocate(tokensToAllocate) - await expect(tx) - .emit(staking, 'AllocationCreated') - .withArgs( - indexer.address, - subgraphDeploymentID, - currentEpoch, - tokensToAllocate, - allocationID, - metadata, - ) - - // After state - const afterStake = await staking.stakes(indexer.address) - const afterAlloc = await staking.getAllocation(allocationID) - - // Stake updated - expect(afterStake.tokensAllocated).eq(beforeStake.tokensAllocated.add(tokensToAllocate)) - // Allocation updated - expect(afterAlloc.indexer).eq(indexer.address) - expect(afterAlloc.subgraphDeploymentID).eq(subgraphDeploymentID) - expect(afterAlloc.tokens).eq(tokensToAllocate) - expect(afterAlloc.createdAtEpoch).eq(currentEpoch) - expect(afterAlloc.collectedFees).eq(toGRT('0')) - expect(afterAlloc.closedAtEpoch).eq(toBN('0')) - expect(afterAlloc.effectiveAllocation).eq(toGRT('0')) - } - - it('reject allocate zero tokens', async function () { - const zeroTokens = toGRT('0') - const tx = allocate(zeroTokens) - await expect(tx).revertedWith('!tokens') - }) - it('reject allocate with invalid allocationID', async function () { const tx = staking .connect(indexer.signer) @@ -219,11 +353,15 @@ describe('Staking:Allocation', () => { }) it('reject allocate if no tokens staked', async function () { - const tokensOverCapacity = tokensToStake.add(toBN('1')) - const tx = allocate(tokensOverCapacity) + const tx = allocate(toBN('1')) await expect(tx).revertedWith('!capacity') }) + it('reject allocate zero tokens if no minimum stake', async function () { + const tx = allocate(toBN('0')) + await expect(tx).revertedWith('!minimumIndexerStake') + }) + context('> when staked', function () { beforeEach(async function () { await staking.connect(indexer.signer).stake(tokensToStake) @@ -239,6 +377,12 @@ describe('Staking:Allocation', () => { await shouldAllocate(tokensToAllocate) }) + it('should allow allocation of zero tokens', async function () { + const zeroTokens = toGRT('0') + const tx = allocate(zeroTokens) + await tx + }) + it('should allocate on behalf of indexer', async function () { const proof = await channelKey.generateProof(indexer.address) @@ -313,57 +457,6 @@ describe('Staking:Allocation', () => { * Collect */ describe('collect', function () { - // This function tests collect with state updates - const shouldCollect = async (tokensToCollect: BigNumber) => { - // Before state - const beforeTokenSupply = await grt.totalSupply() - const beforePool = await curation.pools(subgraphDeploymentID) - const beforeAlloc = await staking.getAllocation(allocationID) - - // Advance blocks to get the allocation in epoch where it can be closed - await advanceToNextEpoch(epochManager) - - // Collect fees and calculate expected results - let rebateFees = tokensToCollect - const protocolPercentage = await staking.protocolPercentage() - const protocolFees = rebateFees.mul(protocolPercentage).div(MAX_PPM) - rebateFees = rebateFees.sub(protocolFees) - - const curationPercentage = await staking.curationPercentage() - const curationFees = rebateFees.mul(curationPercentage).div(MAX_PPM) - rebateFees = rebateFees.sub(curationFees) - - // Collect tokens from allocation - const tx = staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID) - await expect(tx) - .emit(staking, 'AllocationCollected') - .withArgs( - indexer.address, - subgraphDeploymentID, - await epochManager.currentEpoch(), - tokensToCollect, - allocationID, - assetHolder.address, - curationFees, - rebateFees, - ) - - // After state - const afterTokenSupply = await grt.totalSupply() - const afterPool = await curation.pools(subgraphDeploymentID) - const afterAlloc = await staking.getAllocation(allocationID) - - // Check that protocol fees are burnt - expect(afterTokenSupply).eq(beforeTokenSupply.sub(protocolFees)) - // Check that curation reserves increased for the SubgraphDeployment - expect(afterPool.tokens).eq(beforePool.tokens.add(curationFees)) - // Verify allocation is updated and allocation cleaned - expect(afterAlloc.tokens).eq(beforeAlloc.tokens) - expect(afterAlloc.createdAtEpoch).eq(beforeAlloc.createdAtEpoch) - expect(afterAlloc.closedAtEpoch).eq(toBN('0')) - expect(afterAlloc.collectedFees).eq(beforeAlloc.collectedFees.add(rebateFees)) - } - beforeEach(async function () { // Create the allocation await staking.connect(indexer.signer).stake(tokensToStake) @@ -484,193 +577,206 @@ describe('Staking:Allocation', () => { beforeEach(async function () { // Stake and allocate await staking.connect(indexer.signer).stake(tokensToStake) - await allocate(tokensToAllocate) }) - it('reject close a non-existing allocation', async function () { - const invalidAllocationID = randomHexBytes(20) - const tx = staking.connect(indexer.signer).closeAllocation(invalidAllocationID, poi) - await expect(tx).revertedWith('!active') - }) - - it('reject close before at least one epoch has passed', async function () { - const tx = staking.connect(indexer.signer).closeAllocation(allocationID, poi) - await expect(tx).revertedWith(' with ${tokensToAllocate} allocated tokens`, async function () { + beforeEach(async function () { + await allocate(tokensToAllocate) + }) - // Close allocation - const tx = staking.connect(me.signer).closeAllocation(allocationID, poi) - await expect(tx).revertedWith('!auth') - }) + it('reject close a non-existing allocation', async function () { + const invalidAllocationID = randomHexBytes(20) + const tx = staking.connect(indexer.signer).closeAllocation(invalidAllocationID, poi) + await expect(tx).revertedWith('!active') + }) - it('reject close if allocation is already closed', async function () { - // Move at least one epoch to be able to close - await advanceToNextEpoch(epochManager) + it('reject close before at least one epoch has passed', async function () { + const tx = staking.connect(indexer.signer).closeAllocation(allocationID, poi) + await expect(tx).revertedWith(' 0 + const alloc = await staking.getAllocation(allocationID) + if (alloc.tokens.gt(0)) { + // Calculations + const beforeAlloc = await staking.getAllocation(allocationID) + const currentEpoch = await epochManager.currentEpoch() + const epochs = currentEpoch.sub(beforeAlloc.createdAtEpoch) + const effectiveAllocation = calculateEffectiveAllocation( + beforeAlloc.tokens, + epochs, + toBN(maxAllocationEpochs), + ) - it('should close many allocations in batch', async function () { - // Setup a second allocation - await staking.connect(indexer.signer).stake(tokensToAllocate) - const channelKey2 = deriveChannelKey() - const allocationID2 = channelKey2.address - await staking - .connect(indexer.signer) - .allocateFrom( - indexer.address, - subgraphDeploymentID, - tokensToAllocate, - allocationID2, - metadata, - await channelKey2.generateProof(indexer.address), - ) + // Setup + await grt.connect(governor.signer).mint(me.address, toGRT('1')) + await grt.connect(me.signer).approve(staking.address, toGRT('1')) + + // Should close by public + const tx = staking.connect(me.signer).closeAllocation(allocationID, poi) + await expect(tx) + .emit(staking, 'AllocationClosed') + .withArgs( + indexer.address, + subgraphDeploymentID, + currentEpoch, + beforeAlloc.tokens, + allocationID, + effectiveAllocation, + me.address, + poi, + true, + ) + } else { + // closing by the public on a zero allocation is not authorized + const tx = staking.connect(me.signer).closeAllocation(allocationID, poi) + await expect(tx).revertedWith('!auth') + } + }) - // Move at least one epoch to be able to close - await advanceToNextEpoch(epochManager) - await advanceToNextEpoch(epochManager) + it('should close many allocations in batch', async function () { + // Setup a second allocation + await staking.connect(indexer.signer).stake(tokensToStake) + const channelKey2 = deriveChannelKey() + const allocationID2 = channelKey2.address + await staking + .connect(indexer.signer) + .allocate( + subgraphDeploymentID, + tokensToAllocate, + allocationID2, + metadata, + await channelKey2.generateProof(indexer.address), + ) - // Close multiple allocations in one tx - const requests = await Promise.all( - [ - { - allocationID: allocationID, - poi: poi, - }, - { - allocationID: allocationID2, - poi: poi, - }, - ].map(({ allocationID, poi }) => - staking.connect(indexer.signer).populateTransaction.closeAllocation(allocationID, poi), - ), - ).then((e) => e.map((e: PopulatedTransaction) => e.data)) - await staking.connect(indexer.signer).multicall(requests) - }) + // Move at least one epoch to be able to close + await advanceToNextEpoch(epochManager) + await advanceToNextEpoch(epochManager) + + // Close multiple allocations in one tx + const requests = await Promise.all( + [ + { + allocationID: allocationID, + poi: poi, + }, + { + allocationID: allocationID2, + poi: poi, + }, + ].map(({ allocationID, poi }) => + staking + .connect(indexer.signer) + .populateTransaction.closeAllocation(allocationID, poi), + ), + ).then((e) => e.map((e: PopulatedTransaction) => e.data)) + await staking.connect(indexer.signer).multicall(requests) + }) + }) + } }) describe('closeAndAllocate', function () { @@ -710,78 +816,9 @@ describe('Staking:Allocation', () => { * Claim */ describe('claim', function () { - // Claim and perform checks - const shouldClaim = async function (allocationID: string, restake: boolean) { - // Advance blocks to get the allocation in epoch where it can be claimed - await advanceToNextEpoch(epochManager) - - // Before state - const beforeBalance = await grt.balanceOf(indexer.address) - const beforeStake = await staking.stakes(indexer.address) - const beforeAlloc = await staking.allocations(allocationID) - const beforeRebatePool = await staking.rebates(beforeAlloc.closedAtEpoch) - - // Claim rebates - const tokensToClaim = beforeAlloc.collectedFees - const currentEpoch = await epochManager.currentEpoch() - const tx = staking.connect(indexer.signer).claim(allocationID, restake) - await expect(tx) - .emit(staking, 'RebateClaimed') - .withArgs( - indexer.address, - subgraphDeploymentID, - allocationID, - currentEpoch, - beforeAlloc.closedAtEpoch, - tokensToClaim, - beforeRebatePool.unclaimedAllocationsCount - 1, - toGRT('0'), - ) - - // After state - const afterBalance = await grt.balanceOf(indexer.address) - const afterStake = await staking.stakes(indexer.address) - const afterAlloc = await staking.allocations(allocationID) - const afterRebatePool = await staking.rebates(beforeAlloc.closedAtEpoch) - - // Funds distributed to indexer - if (restake) { - expect(afterBalance).eq(beforeBalance) - } else { - expect(afterBalance).eq(beforeBalance.add(tokensToClaim)) - } - // Stake updated - if (restake) { - expect(afterStake.tokensStaked).eq(beforeStake.tokensStaked.add(tokensToClaim)) - } else { - expect(afterStake.tokensStaked).eq(beforeStake.tokensStaked) - } - // Allocation updated (purged) - expect(afterAlloc.tokens).eq(toGRT('0')) - expect(afterAlloc.createdAtEpoch).eq(toGRT('0')) - expect(afterAlloc.closedAtEpoch).eq(toGRT('0')) - expect(afterAlloc.collectedFees).eq(toGRT('0')) - expect(afterAlloc.effectiveAllocation).eq(toGRT('0')) - expect(afterAlloc.accRewardsPerAllocatedToken).eq(toGRT('0')) - // Rebate updated - expect(afterRebatePool.unclaimedAllocationsCount).eq( - beforeRebatePool.unclaimedAllocationsCount - 1, - ) - if (afterRebatePool.unclaimedAllocationsCount === 0) { - // Rebate pool is empty and then pruned - expect(afterRebatePool.effectiveAllocatedStake).eq(toGRT('0')) - expect(afterRebatePool.fees).eq(toGRT('0')) - } else { - // There are still more unclaimed allocations in the rebate pool - expect(afterRebatePool.effectiveAllocatedStake).eq(beforeRebatePool.effectiveAllocatedStake) - expect(afterRebatePool.fees).eq(beforeRebatePool.fees.sub(tokensToClaim)) - } - } - beforeEach(async function () { - // Stake and allocate + // Stake await staking.connect(indexer.signer).stake(tokensToStake) - await allocate(tokensToAllocate) // Set channel dispute period to one epoch await staking.connect(governor.signer).setChannelDisputeEpochs(toBN('1')) @@ -791,125 +828,129 @@ describe('Staking:Allocation', () => { await grt.connect(assetHolder.signer).approve(staking.address, tokensToCollect) }) - it('reject claim for non-existing allocation', async function () { - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Active) - const invalidAllocationID = randomHexBytes(20) - const tx = staking.connect(indexer.signer).claim(invalidAllocationID, false) - await expect(tx).revertedWith('!finalized') - }) - - it('reject claim if allocation is not closed', async function () { - expect(await staking.getAllocationState(allocationID)).not.eq(AllocationState.Closed) - const tx = staking.connect(indexer.signer).claim(allocationID, false) - await expect(tx).revertedWith('!finalized') - }) - - context('> when allocation closed', function () { - beforeEach(async function () { - // Collect some funds - await staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID) - - // Advance blocks to get the allocation in epoch where it can be closed - await advanceToNextEpoch(epochManager) - - // Close the allocation - await staking.connect(indexer.signer).closeAllocation(allocationID, poi) - }) - - it('reject claim if closed but channel dispute epochs has not passed', async function () { - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Closed) - const tx = staking.connect(indexer.signer).claim(allocationID, false) - await expect(tx).revertedWith('!finalized') - }) - - it('should claim rebate', async function () { - // Advance blocks to get the allocation finalized - await advanceToNextEpoch(epochManager) - - // Before state - const beforeIndexerTokens = await grt.balanceOf(indexer.address) - - // Claim with no restake - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized) - await shouldClaim(allocationID, false) - - // Verify that the claimed tokens are transferred to the indexer - const afterIndexerTokens = await grt.balanceOf(indexer.address) - expect(afterIndexerTokens).eq(beforeIndexerTokens.add(tokensToCollect)) - }) - - it('should claim rebate with restake', async function () { - // Advance blocks to get the allocation finalized - await advanceToNextEpoch(epochManager) - - // Before state - const beforeIndexerStake = await staking.getIndexerStakedTokens(indexer.address) - - // Claim with restake - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized) - await shouldClaim(allocationID, true) - - // Verify that the claimed tokens are restaked - const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address) - expect(afterIndexerStake).eq(beforeIndexerStake.add(tokensToCollect)) - }) - - it('should claim rebate (by public)', async function () { - // Advance blocks to get the allocation in epoch where it can be claimed - await advanceToNextEpoch(epochManager) - - // Should claim by public, but cannot restake - const beforeIndexerStake = await staking.stakes(indexer.address) - await staking.connect(me.signer).claim(allocationID, true) - const afterIndexerStake = await staking.stakes(indexer.address) - expect(afterIndexerStake.tokensStaked).eq(beforeIndexerStake.tokensStaked) - }) - - it('should claim rebate (by operator)', async function () { - // Advance blocks to get the allocation in epoch where it can be claimed - await advanceToNextEpoch(epochManager) - - // Add as operator - await staking.connect(indexer.signer).setOperator(me.address, true) - - // Should claim if given operator auth and can do restake - const beforeIndexerStake = await staking.stakes(indexer.address) - await staking.connect(me.signer).claim(allocationID, true) - const afterIndexerStake = await staking.stakes(indexer.address) - expect(afterIndexerStake.tokensStaked).not.eq(beforeIndexerStake.tokensStaked) - }) - - it('should claim many rebates with restake', async function () { - // Advance blocks to get the allocation in epoch where it can be claimed - await advanceToNextEpoch(epochManager) - - // Before state - const beforeIndexerStake = await staking.getIndexerStakedTokens(indexer.address) - - // Claim with restake - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized) - const tx = await staking - .connect(indexer.signer) - .populateTransaction.claim(allocationID, true) - await staking.connect(indexer.signer).multicall([tx.data]) - - // Verify that the claimed tokens are restaked - const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address) - expect(afterIndexerStake).eq(beforeIndexerStake.add(tokensToCollect)) - }) + for (const tokensToAllocate of [toBN(100), toBN(0)]) { + context(`> with ${tokensToAllocate} allocated tokens`, async function () { + beforeEach(async function () { + // Allocate + await allocate(tokensToAllocate) + }) - it('reject claim if already claimed', async function () { - // Advance blocks to get the allocation finalized - await advanceToNextEpoch(epochManager) + it('reject claim for non-existing allocation', async function () { + expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Active) + const invalidAllocationID = randomHexBytes(20) + const tx = staking.connect(indexer.signer).claim(invalidAllocationID, false) + await expect(tx).revertedWith('!finalized') + }) - // First claim - await shouldClaim(allocationID, false) + it('reject claim if allocation is not closed', async function () { + expect(await staking.getAllocationState(allocationID)).not.eq(AllocationState.Closed) + const tx = staking.connect(indexer.signer).claim(allocationID, false) + await expect(tx).revertedWith('!finalized') + }) - // Try to claim again - expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Claimed) - const tx = staking.connect(indexer.signer).claim(allocationID, false) - await expect(tx).revertedWith('!finalized') + context('> when allocation closed', function () { + beforeEach(async function () { + // Collect some funds + await staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID) + + // Advance blocks to get the allocation in epoch where it can be closed + await advanceToNextEpoch(epochManager) + + // Close the allocation + await staking.connect(indexer.signer).closeAllocation(allocationID, poi) + }) + + it('reject claim if closed but channel dispute epochs has not passed', async function () { + expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Closed) + const tx = staking.connect(indexer.signer).claim(allocationID, false) + await expect(tx).revertedWith('!finalized') + }) + + it('should claim rebate', async function () { + // Advance blocks to get the allocation finalized + await advanceToNextEpoch(epochManager) + + // Claim with no restake + await shouldClaim(allocationID, false) + }) + + it('should claim rebate with restake', async function () { + // Advance blocks to get the allocation finalized + await advanceToNextEpoch(epochManager) + + // Claim with restake + await shouldClaim(allocationID, true) + }) + + it('should claim rebate (by public)', async function () { + // Advance blocks to get the allocation in epoch where it can be claimed + await advanceToNextEpoch(epochManager) + + // Should claim by public, but cannot restake + const beforeIndexerStake = await staking.stakes(indexer.address) + await staking.connect(me.signer).claim(allocationID, true) + const afterIndexerStake = await staking.stakes(indexer.address) + expect(afterIndexerStake.tokensStaked).eq(beforeIndexerStake.tokensStaked) + }) + + it('should claim rebate (by operator)', async function () { + // Advance blocks to get the allocation in epoch where it can be claimed + await advanceToNextEpoch(epochManager) + + // Before state + const beforeIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + const beforeAlloc = await staking.allocations(allocationID) + + // Add as operator + // Should claim if given operator auth and can do restake + await staking.connect(indexer.signer).setOperator(me.address, true) + await staking.connect(me.signer).claim(allocationID, true) + + // Verify that the claimed tokens are restaked + const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + const tokensToClaim = beforeAlloc.effectiveAllocation.eq(0) + ? toBN(0) + : beforeAlloc.collectedFees + expect(afterIndexerStake).eq(beforeIndexerStake.add(tokensToClaim)) + }) + + it('should claim many rebates with restake', async function () { + // Advance blocks to get the allocation in epoch where it can be claimed + await advanceToNextEpoch(epochManager) + + // Before state + const beforeIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + const beforeAlloc = await staking.allocations(allocationID) + + // Claim with restake + expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized) + const tx = await staking + .connect(indexer.signer) + .populateTransaction.claim(allocationID, true) + await staking.connect(indexer.signer).multicall([tx.data]) + + // Verify that the claimed tokens are restaked + const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address) + const tokensToClaim = beforeAlloc.effectiveAllocation.eq(0) + ? toBN(0) + : beforeAlloc.collectedFees + expect(afterIndexerStake).eq(beforeIndexerStake.add(tokensToClaim)) + }) + + it('reject claim if already claimed', async function () { + // Advance blocks to get the allocation finalized + await advanceToNextEpoch(epochManager) + + // First claim + await shouldClaim(allocationID, false) + + // Try to claim again + expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Claimed) + const tx = staking.connect(indexer.signer).claim(allocationID, false) + await expect(tx).revertedWith('!finalized') + }) + }) }) - }) + } }) }) diff --git a/test/staking/rebate.test.ts b/test/staking/rebate.test.ts index 416992716..2668b29f4 100644 --- a/test/staking/rebate.test.ts +++ b/test/staking/rebate.test.ts @@ -44,6 +44,14 @@ describe('Staking:Rebate', () => { { totalRewards: 0, fees: 0, totalFees: 0, stake: 1200, totalStake: 7300 }, ] + // Edge case #2 - Closed allocations with queries but no stake + const edgeCases2: RebateTestCase[] = [ + { totalRewards: 1300, fees: 100, totalFees: 1300, stake: 0, totalStake: 0 }, + { totalRewards: 1300, fees: 0, totalFees: 1300, stake: 0, totalStake: 0 }, + { totalRewards: 1300, fees: 200, totalFees: 1300, stake: 0, totalStake: 0 }, + { totalRewards: 1300, fees: 1000, totalFees: 1300, stake: 0, totalStake: 0 }, + ] + // This function calculates the Cobb-Douglas formula in Typescript so we can compare against // the Solidity implementation // TODO: consider using bignumber.js to get extra precision @@ -56,7 +64,7 @@ describe('Staking:Rebate', () => { alphaNumerator: number, alphaDenominator: number, ) { - if (totalFees === 0) { + if (totalFees === 0 || totalStake === 0) { return 0 } const feeRatio = fees / totalFees @@ -201,6 +209,10 @@ describe('Staking:Rebate', () => { describe('edge #1 test case', function () { testAlphas(shouldMatchFormulas, edgeCases1) }) + + describe('edge #2 test case', function () { + testAlphas(shouldMatchFormulas, edgeCases2) + }) }) describe('should match rewards out from rebates', function () { @@ -211,6 +223,10 @@ describe('Staking:Rebate', () => { describe('edge #1 test case', function () { testAlphas(shouldMatchOut, edgeCases1) }) + + describe('edge #2 test case', function () { + testAlphas(shouldMatchFormulas, edgeCases2) + }) }) describe('should always be that sum of rebate rewards obtained <= to total rewards', function () { @@ -221,5 +237,9 @@ describe('Staking:Rebate', () => { describe('edge #1 test case', function () { testAlphas(shouldConserveBalances, edgeCases1) }) + + describe('edge #2 test case', function () { + testAlphas(shouldMatchFormulas, edgeCases2) + }) }) })