diff --git a/audits/Trust/2023-02-operator-decentralization-pr749.pdf b/audits/Trust/2023-02-operator-decentralization-pr749.pdf new file mode 100644 index 000000000..ebfe2ba98 Binary files /dev/null and b/audits/Trust/2023-02-operator-decentralization-pr749.pdf differ diff --git a/config/graph.arbitrum-goerli.yml b/config/graph.arbitrum-goerli.yml index 9fa2ab0be..ec7167713 100644 --- a/config/graph.arbitrum-goerli.yml +++ b/config/graph.arbitrum-goerli.yml @@ -123,9 +123,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/config/graph.arbitrum-localhost.yml b/config/graph.arbitrum-localhost.yml index 6a8216fcc..7bee87748 100644 --- a/config/graph.arbitrum-localhost.yml +++ b/config/graph.arbitrum-localhost.yml @@ -123,9 +123,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/config/graph.arbitrum-one.yml b/config/graph.arbitrum-one.yml index 0525a6d2e..f9dae1862 100644 --- a/config/graph.arbitrum-one.yml +++ b/config/graph.arbitrum-one.yml @@ -123,9 +123,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/config/graph.goerli.yml b/config/graph.goerli.yml index f76c7e215..d09610334 100644 --- a/config/graph.goerli.yml +++ b/config/graph.goerli.yml @@ -126,9 +126,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/config/graph.localhost.yml b/config/graph.localhost.yml index 883feb5a1..578809842 100644 --- a/config/graph.localhost.yml +++ b/config/graph.localhost.yml @@ -131,9 +131,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/config/graph.mainnet.yml b/config/graph.mainnet.yml index fd1dd62d1..ff2c9124f 100644 --- a/config/graph.mainnet.yml +++ b/config/graph.mainnet.yml @@ -126,9 +126,6 @@ contracts: - fn: "setSlasher" slasher: "${{DisputeManager.address}}" allowed: true - - fn: "setAssetHolder" - assetHolder: "${{AllocationExchange.address}}" - allowed: true - fn: "syncAllContracts" RewardsManager: proxy: true diff --git a/contracts/l2/curation/IL2Curation.sol b/contracts/l2/curation/IL2Curation.sol index f3d6a0927..235eec28a 100644 --- a/contracts/l2/curation/IL2Curation.sol +++ b/contracts/l2/curation/IL2Curation.sol @@ -29,4 +29,17 @@ interface IL2Curation { external view returns (uint256); + + /** + * @notice Calculate the amount of tokens that would be recovered if minting signal with + * the input tokens and then burning it. This can be used to compute rounding error. + * This function does not account for curation tax. + * @param _subgraphDeploymentID Subgraph deployment for which to mint signal + * @param _tokensIn Amount of tokens used to mint signal + * @return Amount of tokens that would be recovered after minting and burning signal + */ + function tokensToSignalToTokensNoTax(bytes32 _subgraphDeploymentID, uint256 _tokensIn) + external + view + returns (uint256); } diff --git a/contracts/l2/curation/L2Curation.sol b/contracts/l2/curation/L2Curation.sol index 6e32f2112..c18c10423 100644 --- a/contracts/l2/curation/L2Curation.sol +++ b/contracts/l2/curation/L2Curation.sol @@ -421,6 +421,28 @@ contract L2Curation is CurationV2Storage, GraphUpgradeable, IL2Curation { return _tokensToSignal(_subgraphDeploymentID, _tokensIn); } + /** + * @notice Calculate the amount of tokens that would be recovered if minting signal with + * the input tokens and then burning it. This can be used to compute rounding error. + * This function does not account for curation tax. + * @param _subgraphDeploymentID Subgraph deployment for which to mint signal + * @param _tokensIn Amount of tokens used to mint signal + * @return Amount of tokens that would be recovered after minting and burning signal + */ + function tokensToSignalToTokensNoTax(bytes32 _subgraphDeploymentID, uint256 _tokensIn) + external + view + override + returns (uint256) + { + require(_tokensIn != 0, "Can't calculate with 0 tokens"); + uint256 signal = _tokensToSignal(_subgraphDeploymentID, _tokensIn); + CurationPool memory curationPool = pools[_subgraphDeploymentID]; + uint256 poolSignalAfter = getCurationPoolSignal(_subgraphDeploymentID).add(signal); + uint256 poolTokensAfter = curationPool.tokens.add(_tokensIn); + return poolTokensAfter.mul(signal).div(poolSignalAfter); + } + /** * @notice Calculate number of tokens to get when burning signal from a curation pool. * @param _subgraphDeploymentID Subgraph deployment for which to burn signal diff --git a/contracts/l2/discovery/IL2GNS.sol b/contracts/l2/discovery/IL2GNS.sol index 3a8763274..4864e9bc1 100644 --- a/contracts/l2/discovery/IL2GNS.sol +++ b/contracts/l2/discovery/IL2GNS.sol @@ -46,4 +46,11 @@ interface IL2GNS is ICallhookReceiver { * @return L2 subgraph ID */ function getAliasedL2SubgraphID(uint256 _l1SubgraphID) external pure returns (uint256); + + /** + * @notice Return the unaliased L1 subgraph ID from a transferred L2 subgraph ID + * @param _l2SubgraphID L2 subgraph ID + * @return L1subgraph ID + */ + function getUnaliasedL1SubgraphID(uint256 _l2SubgraphID) external pure returns (uint256); } diff --git a/contracts/l2/discovery/L2GNS.sol b/contracts/l2/discovery/L2GNS.sol index c8d952135..1de677079 100644 --- a/contracts/l2/discovery/L2GNS.sol +++ b/contracts/l2/discovery/L2GNS.sol @@ -26,9 +26,17 @@ import { IL2Curation } from "../curation/IL2Curation.sol"; contract L2GNS is GNS, L2GNSV1Storage, IL2GNS { using SafeMathUpgradeable for uint256; + /// Offset added to an L1 subgraph ID to compute the L2 subgraph ID alias uint256 public constant SUBGRAPH_ID_ALIAS_OFFSET = uint256(0x1111000000000000000000000000000000000000000000000000000000001111); + /// Maximum rounding error when receiving signal tokens from L1, in parts-per-million. + /// If the error from minting signal is above this, tokens will be sent back to the curator. + uint256 public constant MAX_ROUNDING_ERROR = 1000; + + /// @dev 100% expressed in parts-per-million + uint256 private constant MAX_PPM = 1000000; + /// @dev Emitted when a subgraph is received from L1 through the bridge event SubgraphReceivedFromL1( uint256 indexed _l1SubgraphID, @@ -122,10 +130,34 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS { require(_subgraphDeploymentID != 0, "GNS: deploymentID != 0"); IL2Curation curation = IL2Curation(address(curation())); - // Update pool: constant nSignal, vSignal can change (w/no slippage protection) - // Buy all signal from the new deployment - uint256 vSignal = curation.mintTaxFree(_subgraphDeploymentID, transferData.tokens); - uint256 nSignal = vSignalToNSignal(_l2SubgraphID, vSignal); + + uint256 vSignal; + uint256 nSignal; + uint256 roundingError; + uint256 tokens = transferData.tokens; + { + // This can't revert because the bridge ensures that _tokensIn is > 0, + // and the minimum curation in L2 is 1 wei GRT + uint256 tokensAfter = curation.tokensToSignalToTokensNoTax( + _subgraphDeploymentID, + tokens + ); + roundingError = tokens.sub(tokensAfter).mul(MAX_PPM).div(tokens); + } + if (roundingError <= MAX_ROUNDING_ERROR) { + vSignal = curation.mintTaxFree(_subgraphDeploymentID, tokens); + nSignal = vSignalToNSignal(_l2SubgraphID, vSignal); + emit SignalMinted(_l2SubgraphID, msg.sender, nSignal, vSignal, tokens); + emit SubgraphUpgraded(_l2SubgraphID, vSignal, tokens, _subgraphDeploymentID); + } else { + graphToken().transfer(msg.sender, tokens); + emit CuratorBalanceReturnedToBeneficiary( + getUnaliasedL1SubgraphID(_l2SubgraphID), + msg.sender, + tokens + ); + emit SubgraphUpgraded(_l2SubgraphID, vSignal, 0, _subgraphDeploymentID); + } subgraphData.disabled = false; subgraphData.vSignal = vSignal; @@ -134,16 +166,8 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS { subgraphData.subgraphDeploymentID = _subgraphDeploymentID; // Set the token metadata _setSubgraphMetadata(_l2SubgraphID, _subgraphMetadata); - emit SubgraphPublished(_l2SubgraphID, _subgraphDeploymentID, fixedReserveRatio); - emit SubgraphUpgraded( - _l2SubgraphID, - subgraphData.vSignal, - transferData.tokens, - _subgraphDeploymentID - ); emit SubgraphVersionUpdated(_l2SubgraphID, _subgraphDeploymentID, _versionMetadata); - emit SignalMinted(_l2SubgraphID, msg.sender, nSignal, vSignal, transferData.tokens); emit SubgraphL2TransferFinalized(_l2SubgraphID); } @@ -227,6 +251,20 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS { return _l1SubgraphID + SUBGRAPH_ID_ALIAS_OFFSET; } + /** + * @notice Return the unaliased L1 subgraph ID from a transferred L2 subgraph ID + * @param _l2SubgraphID L2 subgraph ID + * @return L1subgraph ID + */ + function getUnaliasedL1SubgraphID(uint256 _l2SubgraphID) + public + pure + override + returns (uint256) + { + return _l2SubgraphID - SUBGRAPH_ID_ALIAS_OFFSET; + } + /** * @dev Receive a subgraph from L1. * This function will initialize a subgraph received through the bridge, @@ -278,13 +316,23 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS { IL2GNS.SubgraphL2TransferData storage transferData = subgraphL2TransferData[l2SubgraphID]; SubgraphData storage subgraphData = _getSubgraphData(l2SubgraphID); + IL2Curation curation = IL2Curation(address(curation())); + uint256 roundingError; + if (transferData.l2Done && !subgraphData.disabled) { + // This can't revert because the bridge ensures that _tokensIn is > 0, + // and the minimum curation in L2 is 1 wei GRT + uint256 tokensAfter = curation.tokensToSignalToTokensNoTax( + subgraphData.subgraphDeploymentID, + _tokensIn + ); + roundingError = _tokensIn.sub(tokensAfter).mul(MAX_PPM).div(_tokensIn); + } // If subgraph transfer wasn't finished, we should send the tokens to the curator - if (!transferData.l2Done || subgraphData.disabled) { + if (!transferData.l2Done || subgraphData.disabled || roundingError > MAX_ROUNDING_ERROR) { graphToken().transfer(_curator, _tokensIn); emit CuratorBalanceReturnedToBeneficiary(_l1SubgraphID, _curator, _tokensIn); } else { // Get name signal to mint for tokens deposited - IL2Curation curation = IL2Curation(address(curation())); uint256 vSignal = curation.mintTaxFree(subgraphData.subgraphDeploymentID, _tokensIn); uint256 nSignal = vSignalToNSignal(l2SubgraphID, vSignal); diff --git a/contracts/l2/staking/L2Staking.sol b/contracts/l2/staking/L2Staking.sol index 2923dc04e..ca1c7c82d 100644 --- a/contracts/l2/staking/L2Staking.sol +++ b/contracts/l2/staking/L2Staking.sol @@ -19,6 +19,9 @@ contract L2Staking is Staking, IL2StakingBase { using SafeMath for uint256; using Stakes for Stakes.Indexer; + /// @dev Minimum amount of tokens that can be delegated + uint256 private constant MINIMUM_DELEGATION = 1e18; + /** * @dev Emitted when `delegator` delegated `tokens` to the `indexer`, the delegator * gets `shares` for the delegation pool proportionally to the tokens staked. @@ -124,8 +127,10 @@ contract L2Staking is Staking, IL2StakingBase { // Calculate shares to issue (without applying any delegation tax) uint256 shares = (pool.tokens == 0) ? _amount : _amount.mul(pool.shares).div(pool.tokens); - if (shares == 0) { - // If no shares would be issued (probably a rounding issue or attack), return the tokens to the delegator + if (shares == 0 || _amount < MINIMUM_DELEGATION) { + // If no shares would be issued (probably a rounding issue or attack), + // or if the amount is under the minimum delegation (which could be part of a rounding attack), + // return the tokens to the delegator graphToken().transfer(_delegationData.delegator, _amount); emit TransferredDelegationReturnedToDelegator( _delegationData.indexer, diff --git a/contracts/staking/IStakingBase.sol b/contracts/staking/IStakingBase.sol index acc8ac678..086e29a7d 100644 --- a/contracts/staking/IStakingBase.sol +++ b/contracts/staking/IStakingBase.sol @@ -93,12 +93,6 @@ interface IStakingBase is IStakingData { uint32 cooldownBlocks ); - /** - * @dev Emitted when `caller` set `assetHolder` address as `allowed` to send funds - * to staking contract. - */ - event AssetHolderUpdate(address indexed caller, address indexed assetHolder, bool allowed); - /** * @dev Emitted when `indexer` set `operator` access. */ @@ -219,14 +213,6 @@ interface IStakingBase is IStakingData { uint32 _lambdaDenominator ) external; - /** - * @notice Set an address as allowed asset holder. - * @dev This function can only be called by the governor. - * @param _assetHolder Address of allowed source for state channel funds - * @param _allowed True if asset holder is allowed - */ - function setAssetHolder(address _assetHolder, bool _allowed) external; - /** * @notice Authorize or unauthorize an address to be an operator for the caller. * @param _operator Address to authorize or unauthorize diff --git a/contracts/staking/IStakingExtension.sol b/contracts/staking/IStakingExtension.sol index 815577e2f..3499aa135 100644 --- a/contracts/staking/IStakingExtension.sol +++ b/contracts/staking/IStakingExtension.sol @@ -237,15 +237,6 @@ interface IStakingExtension is IStakingData { */ function rewardsDestination(address _indexer) external view returns (address); - /** - * @notice Getter for assetHolders[_maybeAssetHolder]: - * returns true if the address is an asset holder, i.e. an entity that can collect - * query fees into the Staking contract. - * @param _maybeAssetHolder The address that may or may not be an asset holder - * @return True if the address is an asset holder - */ - function assetHolders(address _maybeAssetHolder) external view returns (bool); - /** * @notice Getter for subgraphAllocations[_subgraphDeploymentId]: * returns the amount of tokens allocated to a subgraph deployment. diff --git a/contracts/staking/Staking.sol b/contracts/staking/Staking.sol index 589141210..ad9102d29 100644 --- a/contracts/staking/Staking.sol +++ b/contracts/staking/Staking.sol @@ -220,17 +220,6 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M ); } - /** - * @notice Set an address as allowed asset holder. - * @param _assetHolder Address of allowed source for state channel funds - * @param _allowed True if asset holder is allowed - */ - function setAssetHolder(address _assetHolder, bool _allowed) external override onlyGovernor { - require(_assetHolder != address(0), "!assetHolder"); - __assetHolders[_assetHolder] = _allowed; - emit AssetHolderUpdate(msg.sender, _assetHolder, _allowed); - } - /** * @notice Authorize or unauthorize an address to be an operator for the caller. * @param _operator Address to authorize or unauthorize @@ -354,7 +343,6 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M /** * @dev Collect and rebate query fees from state channels to the indexer - * Funds received are only accepted from a valid sender. * To avoid reverting on the withdrawal from channel flow this function will accept calls with zero tokens. * We use an exponential rebate formula to calculate the amount of tokens to rebate to the indexer. * This implementation allows collecting multiple times on the same allocation, keeping track of the @@ -366,13 +354,16 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M // Allocation identifier validation require(_allocationID != address(0), "!alloc"); - // The contract caller must be an authorized asset holder - require(__assetHolders[msg.sender] == true, "!assetHolder"); - // Allocation must exist AllocationState allocState = _getAllocationState(_allocationID); require(allocState != AllocationState.Null, "!collect"); + // If the query fees are zero, we don't want to revert + // but we also don't need to do anything, so just return + if (_tokens == 0) { + return; + } + Allocation storage alloc = __allocations[_allocationID]; bytes32 subgraphDeploymentID = alloc.subgraphDeploymentID; @@ -382,9 +373,8 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M uint256 queryRebates = 0; // Tokens to distribute to indexer uint256 delegationRewards = 0; // Tokens to distribute to delegators - // Process query fees only if non-zero amount - if (queryFees > 0) { - // -- Pull tokens from the authorized sender -- + { + // -- Pull tokens from the sender -- IGraphToken graphToken = graphToken(); TokenUtils.pullTokens(graphToken, msg.sender, queryFees); @@ -789,7 +779,7 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M // Creates an allocation // Allocation identifiers are not reused - // The assetHolder address can send collected funds to the allocation + // Anyone can send collected funds to the allocation using collect() Allocation memory alloc = Allocation( _indexer, _subgraphDeploymentID, @@ -955,7 +945,13 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M bool isCurationEnabled = _curationPercentage > 0 && address(curation) != address(0); if (isCurationEnabled && curation.isCurated(_subgraphDeploymentID)) { - uint256 curationFees = uint256(_curationPercentage).mul(_tokens).div(MAX_PPM); + // Calculate the tokens after curation fees first, and subtact that, + // to prevent curation fees from rounding down to zero + uint256 tokensAfterCurationFees = uint256(MAX_PPM) + .sub(_curationPercentage) + .mul(_tokens) + .div(MAX_PPM); + uint256 curationFees = _tokens.sub(tokensAfterCurationFees); if (curationFees > 0) { // Transfer and call collect() // This function transfer tokens to a trusted protocol contracts @@ -981,7 +977,10 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M uint256 _tokens, uint256 _percentage ) private returns (uint256) { - uint256 tax = uint256(_percentage).mul(_tokens).div(MAX_PPM); + // Calculate tokens after tax first, and subtract that, + // to prevent the tax from rounding down to zero + uint256 tokensAfterTax = uint256(MAX_PPM).sub(_percentage).mul(_tokens).div(MAX_PPM); + uint256 tax = _tokens.sub(tokensAfterTax); TokenUtils.burnTokens(_graphToken, tax); // Burn tax if any return tax; } diff --git a/contracts/staking/StakingExtension.sol b/contracts/staking/StakingExtension.sol index 0667b89fb..9c4bdd642 100644 --- a/contracts/staking/StakingExtension.sol +++ b/contracts/staking/StakingExtension.sol @@ -26,6 +26,8 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi /// @dev 100% in parts per million uint32 private constant MAX_PPM = 1000000; + /// @dev Minimum amount of tokens that can be delegated + uint256 private constant MINIMUM_DELEGATION = 1e18; /** * @dev Check if the caller is the slasher. @@ -301,17 +303,6 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi return __rewardsDestination[_indexer]; } - /** - * @notice Getter for assetHolders[_maybeAssetHolder]: - * returns true if the address is an asset holder, i.e. an entity that can collect - * query fees into the Staking contract. - * @param _maybeAssetHolder The address that may or may not be an asset holder - * @return True if the address is an asset holder - */ - function assetHolders(address _maybeAssetHolder) external view override returns (bool) { - return __assetHolders[_maybeAssetHolder]; - } - /** * @notice Getter for operatorAuth[_indexer][_maybeOperator]: * returns true if the operator is authorized to operate on behalf of the indexer. @@ -539,8 +530,8 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi address _indexer, uint256 _tokens ) private returns (uint256) { - // Only delegate a non-zero amount of tokens - require(_tokens > 0, "!tokens"); + // Only allow delegations over a minimum, to prevent rounding attacks + require(_tokens >= MINIMUM_DELEGATION, "!minimum-delegation"); // Only delegate to non-empty address require(_indexer != address(0), "!indexer"); // Only delegate to staked indexer @@ -599,15 +590,26 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi _withdrawDelegated(_delegator, _indexer, address(0)); } + uint256 poolTokens = pool.tokens; + uint256 poolShares = pool.shares; + // Calculate tokens to get in exchange for the shares - uint256 tokens = _shares.mul(pool.tokens).div(pool.shares); + uint256 tokens = _shares.mul(poolTokens).div(poolShares); // Update the delegation pool - pool.tokens = pool.tokens.sub(tokens); - pool.shares = pool.shares.sub(_shares); + poolTokens = poolTokens.sub(tokens); + poolShares = poolShares.sub(_shares); + pool.tokens = poolTokens; + pool.shares = poolShares; // Update the delegation delegation.shares = delegation.shares.sub(_shares); + // Enforce more than the minimum delegation is left, + // to prevent rounding attacks + if (delegation.shares > 0) { + uint256 remainingDelegation = delegation.shares.mul(poolTokens).div(poolShares); + require(remainingDelegation >= MINIMUM_DELEGATION, "!minimum-delegation"); + } delegation.tokensLocked = delegation.tokensLocked.add(tokens); delegation.tokensLockedUntil = epochManager().currentEpoch().add( __delegationUnbondingPeriod diff --git a/contracts/staking/StakingStorage.sol b/contracts/staking/StakingStorage.sol index 88c149e22..340709f2b 100644 --- a/contracts/staking/StakingStorage.sol +++ b/contracts/staking/StakingStorage.sol @@ -89,8 +89,8 @@ contract StakingV1Storage is Managed { // -- Asset Holders -- - /// @dev Allowed AssetHolders that can collect query fees: assetHolder => is allowed - mapping(address => bool) internal __assetHolders; + /// @dev DEPRECATED: Allowed AssetHolders: assetHolder => is allowed + mapping(address => bool) private __DEPRECATED_assetHolders; // solhint-disable-line var-name-mixedcase } /** diff --git a/e2e/deployment/config/staking.test.ts b/e2e/deployment/config/staking.test.ts index e162f0676..3759c2567 100644 --- a/e2e/deployment/config/staking.test.ts +++ b/e2e/deployment/config/staking.test.ts @@ -26,11 +26,6 @@ describe('Staking configuration', () => { expect(isSlasher).eq(true) }) - it('should allow AllocationExchange to collect query fees', async function () { - const allowed = await Staking.assetHolders(AllocationExchange.address) - expect(allowed).eq(true) - }) - it('minimumIndexerStake should match "minimumIndexerStake" in the config file', async function () { const value = await Staking.minimumIndexerStake() const expected = getItemValue(graphConfig, `contracts/${contractName}/init/minimumIndexerStake`) diff --git a/test/disputes/poi.test.ts b/test/disputes/poi.test.ts index e0129411a..fdb6bb954 100644 --- a/test/disputes/poi.test.ts +++ b/test/disputes/poi.test.ts @@ -105,9 +105,6 @@ describe('DisputeManager:POI', async () => { // Give some funds to the fisherman await grt.connect(governor.signer).mint(fisherman.address, fishermanTokens) await grt.connect(fisherman.signer).approve(disputeManager.address, fishermanTokens) - - // Allow the asset holder - await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true) }) beforeEach(async function () { diff --git a/test/disputes/query.test.ts b/test/disputes/query.test.ts index b4ef9bcdf..10edc39f2 100644 --- a/test/disputes/query.test.ts +++ b/test/disputes/query.test.ts @@ -153,9 +153,6 @@ describe('DisputeManager:Query', async () => { await grt.connect(dst.signer).approve(disputeManager.address, fishermanTokens) } - // Allow the asset holder - await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true) - // Create an attestation const attestation = await buildAttestation(receipt, indexer1ChannelKey.privKey) diff --git a/test/l2/l2GNS.test.ts b/test/l2/l2GNS.test.ts index 728decacf..ad188ecb5 100644 --- a/test/l2/l2GNS.test.ts +++ b/test/l2/l2GNS.test.ts @@ -10,6 +10,7 @@ import { getL2SignerFromL1, setAccountBalance, latestBlock, + deriveChannelKey, } from '../lib/testHelpers' import { L2FixtureContracts, NetworkFixture } from '../lib/fixtures' import { toBN } from '../lib/testHelpers' @@ -30,6 +31,7 @@ import { } from '../lib/gnsUtils' import { L2Curation } from '../../build/types/L2Curation' import { GraphToken } from '../../build/types/GraphToken' +import { IL2Staking } from '../../build/types/IL2Staking' const { HashZero } = ethers.constants @@ -43,6 +45,7 @@ interface L1SubgraphParams { describe('L2GNS', () => { let me: Account let other: Account + let attacker: Account let governor: Account let mockRouter: Account let mockL1GRT: Account @@ -56,6 +59,7 @@ describe('L2GNS', () => { let gns: L2GNS let curation: L2Curation let grt: GraphToken + let staking: IL2Staking let newSubgraph0: PublishSubgraph let newSubgraph1: PublishSubgraph @@ -114,12 +118,21 @@ describe('L2GNS', () => { before(async function () { newSubgraph0 = buildSubgraph() - ;[me, other, governor, mockRouter, mockL1GRT, mockL1Gateway, mockL1GNS, mockL1Staking] = - await getAccounts() + ;[ + me, + other, + governor, + mockRouter, + mockL1GRT, + mockL1Gateway, + mockL1GNS, + mockL1Staking, + attacker, + ] = await getAccounts() fixture = new NetworkFixture() fixtureContracts = await fixture.loadL2(governor.signer) - ;({ l2GraphTokenGateway, gns, curation, grt } = fixtureContracts) + ;({ l2GraphTokenGateway, gns, curation, grt, staking } = fixtureContracts) await grt.connect(governor.signer).mint(me.address, toGRT('10000')) await fixture.configureL2Bridge( @@ -398,6 +411,67 @@ describe('L2GNS', () => { .emit(gns, 'SignalMinted') .withArgs(l2SubgraphId, me.address, expectedNSignal, expectedSignal, curatedTokens) }) + it('protects the owner against a rounding attack', async function () { + const { l1SubgraphId, curatedTokens, subgraphMetadata, versionMetadata } = + await defaultL1SubgraphParams() + const collectTokens = curatedTokens.mul(20) + + await staking.connect(governor.signer).setCurationPercentage(100000) + + // Set up an indexer account with some stake + await grt.connect(governor.signer).mint(attacker.address, toGRT('1000000')) + // Curate 1 wei GRT by minting 1 GRT and burning most of it + await grt.connect(attacker.signer).approve(curation.address, toBN(1)) + await curation.connect(attacker.signer).mint(newSubgraph0.subgraphDeploymentID, toBN(1), 0) + + // Check this actually gave us 1 wei signal + expect(await curation.getCurationPoolTokens(newSubgraph0.subgraphDeploymentID)).eq(1) + await grt.connect(attacker.signer).approve(staking.address, toGRT('1000000')) + await staking.connect(attacker.signer).stake(toGRT('100000')) + const channelKey = deriveChannelKey() + // Allocate to the same deployment ID + await staking + .connect(attacker.signer) + .allocateFrom( + attacker.address, + newSubgraph0.subgraphDeploymentID, + toGRT('100000'), + channelKey.address, + randomHexBytes(32), + await channelKey.generateProof(attacker.address), + ) + // Spoof some query fees, 10% of which will go to the Curation pool + await staking.connect(attacker.signer).collect(collectTokens, channelKey.address) + // The curation pool now has 1 wei shares and a lot of tokens, so the rounding attack is prepared + // But L2GNS will protect the owner by sending the tokens + const callhookData = defaultAbiCoder.encode( + ['uint8', 'uint256', 'address'], + [toBN(0), l1SubgraphId, me.address], + ) + await gatewayFinalizeTransfer(mockL1GNS.address, gns.address, curatedTokens, callhookData) + + const l2SubgraphId = await gns.getAliasedL2SubgraphID(l1SubgraphId) + const tx = gns + .connect(me.signer) + .finishSubgraphTransferFromL1( + l2SubgraphId, + newSubgraph0.subgraphDeploymentID, + subgraphMetadata, + versionMetadata, + ) + await expect(tx) + .emit(gns, 'SubgraphPublished') + .withArgs(l2SubgraphId, newSubgraph0.subgraphDeploymentID, DEFAULT_RESERVE_RATIO) + await expect(tx).emit(gns, 'SubgraphMetadataUpdated').withArgs(l2SubgraphId, subgraphMetadata) + await expect(tx).emit(gns, 'CuratorBalanceReturnedToBeneficiary') + await expect(tx) + .emit(gns, 'SubgraphUpgraded') + .withArgs(l2SubgraphId, 0, 0, newSubgraph0.subgraphDeploymentID) + await expect(tx) + .emit(gns, 'SubgraphVersionUpdated') + .withArgs(l2SubgraphId, newSubgraph0.subgraphDeploymentID, versionMetadata) + await expect(tx).emit(gns, 'SubgraphL2TransferFinalized').withArgs(l2SubgraphId) + }) it('cannot be called by someone other than the subgraph owner', async function () { const { l1SubgraphId, curatedTokens, subgraphMetadata, versionMetadata } = await defaultL1SubgraphParams() @@ -741,6 +815,63 @@ describe('L2GNS', () => { expect(gnsBalanceAfter).eq(gnsBalanceBefore) }) + it('protects the curator against a rounding attack', async function () { + // Transfer a subgraph from L1 with only 1 wei GRT of curated signal + const { l1SubgraphId, subgraphMetadata, versionMetadata } = await defaultL1SubgraphParams() + const curatedTokens = toBN('1') + await transferMockSubgraphFromL1( + l1SubgraphId, + curatedTokens, + subgraphMetadata, + versionMetadata, + ) + // Prepare the rounding attack by setting up an indexer and collecting a lot of query fees + const curatorTokens = toGRT('10000') + const collectTokens = curatorTokens.mul(20) + await staking.connect(governor.signer).setCurationPercentage(100000) + // Set up an indexer account with some stake + await grt.connect(governor.signer).mint(attacker.address, toGRT('1000000')) + + await grt.connect(attacker.signer).approve(staking.address, toGRT('1000000')) + await staking.connect(attacker.signer).stake(toGRT('100000')) + const channelKey = deriveChannelKey() + // Allocate to the same deployment ID + await staking + .connect(attacker.signer) + .allocateFrom( + attacker.address, + newSubgraph0.subgraphDeploymentID, + toGRT('100000'), + channelKey.address, + randomHexBytes(32), + await channelKey.generateProof(attacker.address), + ) + // Spoof some query fees, 10% of which will go to the Curation pool + await staking.connect(attacker.signer).collect(collectTokens, channelKey.address) + + const callhookData = defaultAbiCoder.encode( + ['uint8', 'uint256', 'address'], + [toBN(1), l1SubgraphId, me.address], + ) + const curatorTokensBefore = await grt.balanceOf(me.address) + const gnsBalanceBefore = await grt.balanceOf(gns.address) + const tx = gatewayFinalizeTransfer( + mockL1GNS.address, + gns.address, + curatorTokens, + callhookData, + ) + await expect(tx) + .emit(gns, 'CuratorBalanceReturnedToBeneficiary') + .withArgs(l1SubgraphId, me.address, curatorTokens) + const curatorTokensAfter = await grt.balanceOf(me.address) + expect(curatorTokensAfter).eq(curatorTokensBefore.add(curatorTokens)) + const gnsBalanceAfter = await grt.balanceOf(gns.address) + // gatewayFinalizeTransfer will mint the tokens that are sent to the curator, + // so the GNS balance should be the same + expect(gnsBalanceAfter).eq(gnsBalanceBefore) + }) + it('if a subgraph was deprecated after transfer, it returns the tokens to the beneficiary', async function () { const mockL1GNSL2Alias = await getL2SignerFromL1(mockL1GNS.address) // Eth for gas: diff --git a/test/l2/l2Staking.test.ts b/test/l2/l2Staking.test.ts index cba54df50..a8edcdb2d 100644 --- a/test/l2/l2Staking.test.ts +++ b/test/l2/l2Staking.test.ts @@ -295,7 +295,8 @@ describe('L2Staking', () => { .setIssuancePerBlock(toGRT('114')) await staking.connect(me.signer).stake(tokens100k) - await staking.connect(me.signer).delegate(me.address, toBN(1)) // 1 weiGRT == 1 share + // Initialize the delegation pool to allow delegating less than 1 GRT + await staking.connect(me.signer).delegate(me.address, tokens10k) await staking.connect(me.signer).setDelegationParameters(1000, 1000, 1000) await grt.connect(me.signer).approve(fixtureContracts.curation.address, tokens10k) @@ -336,6 +337,88 @@ describe('L2Staking', () => { const delegatorGRTBalanceAfter = await grt.balanceOf(other.address) expect(delegatorGRTBalanceAfter.sub(delegatorGRTBalanceBefore)).to.equal(toBN(1)) }) + it('returns delegation to the delegator if it initializes the pool with less than the minimum delegation', async function () { + await fixtureContracts.rewardsManager + .connect(governor.signer) + .setIssuancePerBlock(toGRT('114')) + + await staking.connect(me.signer).stake(tokens100k) + + await staking.connect(me.signer).setDelegationParameters(1000, 1000, 1000) + await grt.connect(me.signer).approve(fixtureContracts.curation.address, tokens10k) + await fixtureContracts.curation.connect(me.signer).mint(subgraphDeploymentID, tokens10k, 0) + + await allocate(tokens100k) + await advanceToNextEpoch(fixtureContracts.epochManager) + await advanceToNextEpoch(fixtureContracts.epochManager) + await staking.connect(me.signer).closeAllocation(allocationID, randomHexBytes(32)) + // Now there are some rewards sent to delegation pool, so 1 weiGRT is less than 1 share + + const functionData = defaultAbiCoder.encode( + ['tuple(address,address)'], + [[me.address, other.address]], + ) + + const callhookData = defaultAbiCoder.encode( + ['uint8', 'bytes'], + [toBN(1), functionData], // code = 1 means RECEIVE_DELEGATION_CODE + ) + const delegatorGRTBalanceBefore = await grt.balanceOf(other.address) + const tx = gatewayFinalizeTransfer( + mockL1Staking.address, + staking.address, + toGRT('0.1'), // Less than 1 GRT! + callhookData, + ) + + await expect(tx) + .emit(l2GraphTokenGateway, 'DepositFinalized') + .withArgs(mockL1GRT.address, mockL1Staking.address, staking.address, toGRT('0.1')) + const delegation = await staking.getDelegation(me.address, other.address) + await expect(tx) + .emit(staking, 'TransferredDelegationReturnedToDelegator') + .withArgs(me.address, other.address, toGRT('0.1')) + + expect(delegation.shares).to.equal(0) + const delegatorGRTBalanceAfter = await grt.balanceOf(other.address) + expect(delegatorGRTBalanceAfter.sub(delegatorGRTBalanceBefore)).to.equal(toGRT('0.1')) + }) + it('returns delegation under the minimum if the pool is initialized', async function () { + await staking.connect(me.signer).stake(tokens100k) + + // Initialize the delegation pool to allow delegating less than 1 GRT + await staking.connect(me.signer).delegate(me.address, tokens10k) + + const functionData = defaultAbiCoder.encode( + ['tuple(address,address)'], + [[me.address, other.address]], + ) + + const callhookData = defaultAbiCoder.encode( + ['uint8', 'bytes'], + [toBN(1), functionData], // code = 1 means RECEIVE_DELEGATION_CODE + ) + const delegatorGRTBalanceBefore = await grt.balanceOf(other.address) + const tx = gatewayFinalizeTransfer( + mockL1Staking.address, + staking.address, + toGRT('0.1'), + callhookData, + ) + + await expect(tx) + .emit(l2GraphTokenGateway, 'DepositFinalized') + .withArgs(mockL1GRT.address, mockL1Staking.address, staking.address, toGRT('0.1')) + + const delegation = await staking.getDelegation(me.address, other.address) + await expect(tx) + .emit(staking, 'TransferredDelegationReturnedToDelegator') + .withArgs(me.address, other.address, toGRT('0.1')) + + expect(delegation.shares).to.equal(0) + const delegatorGRTBalanceAfter = await grt.balanceOf(other.address) + expect(delegatorGRTBalanceAfter.sub(delegatorGRTBalanceBefore)).to.equal(toGRT('0.1')) + }) }) describe('onTokenTransfer with invalid messages', function () { it('reverts if the code is invalid', async function () { diff --git a/test/payments/allocationExchange.test.ts b/test/payments/allocationExchange.test.ts index fba672e03..e057ef061 100644 --- a/test/payments/allocationExchange.test.ts +++ b/test/payments/allocationExchange.test.ts @@ -14,8 +14,10 @@ import { randomHexBytes, toGRT, Account, + advanceToNextEpoch, } from '../lib/testHelpers' import { arrayify, joinSignature, SigningKey, solidityKeccak256 } from 'ethers/lib/utils' +import { EpochManager } from '../../build/types/EpochManager' const { AddressZero, MaxUint256 } = constants @@ -35,6 +37,7 @@ describe('AllocationExchange', () => { let grt: GraphToken let staking: IStaking let allocationExchange: AllocationExchange + let epochManager: EpochManager async function createVoucher( allocationID: string, @@ -57,7 +60,7 @@ describe('AllocationExchange', () => { authority = Wallet.createRandom() fixture = new NetworkFixture() - ;({ grt, staking } = await fixture.load(governor.signer)) + ;({ grt, staking, epochManager } = await fixture.load(governor.signer)) allocationExchange = (await deployment.deployContract( 'AllocationExchange', governor.signer, @@ -77,7 +80,6 @@ describe('AllocationExchange', () => { await grt.connect(governor.signer).mint(allocationExchange.address, exchangeTokens) // Ensure the exchange is correctly setup - await staking.connect(governor.signer).setAssetHolder(allocationExchange.address, true) await allocationExchange.connect(governor.signer).setAuthority(authority.address, true) await allocationExchange.approveAll() }) @@ -192,6 +194,7 @@ describe('AllocationExchange', () => { // Setup an active allocation const allocationID = await setupAllocation() + await advanceToNextEpoch(epochManager) // Initiate a withdrawal const actualAmount = toGRT('2000') // <- withdraw amount @@ -213,6 +216,7 @@ describe('AllocationExchange', () => { it('reject double spending of a voucher', async function () { // Setup an active allocation const allocationID = await setupAllocation() + await advanceToNextEpoch(epochManager) // Initiate a withdrawal const actualAmount = toGRT('2000') // <- withdraw amount diff --git a/test/rewards/rewards.test.ts b/test/rewards/rewards.test.ts index 7fa60d5f6..e80767a80 100644 --- a/test/rewards/rewards.test.ts +++ b/test/rewards/rewards.test.ts @@ -1013,7 +1013,6 @@ describe('Rewards', () => { // allow the asset holder const tokensToCollect = toGRT('10000') - await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true) // signal in two subgraphs in the same block const subgraphs = [subgraphDeploymentID1, subgraphDeploymentID2] @@ -1043,7 +1042,7 @@ describe('Rewards', () => { ]) // move time fwd - await advanceBlock() + await advanceToNextEpoch(epochManager) // collect funds into staking for that sub await staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID1) diff --git a/test/staking/allocation.test.ts b/test/staking/allocation.test.ts index fa7b71765..948446c1e 100644 --- a/test/staking/allocation.test.ts +++ b/test/staking/allocation.test.ts @@ -117,9 +117,13 @@ describe('Staking:Allocation', () => { // This function tests collect with state updates const shouldCollect = async ( tokensToCollect: BigNumber, - _allocationID?: string, + options: { + allocationID?: string + expectEvent?: boolean + } = {}, ): Promise<{ queryRebates: BigNumber; queryFeesBurnt: BigNumber }> => { - const alloID = _allocationID ?? allocationID + const expectEvent = options.expectEvent ?? true + const alloID = options.allocationID ?? allocationID const alloStateBefore = await staking.getAllocationState(alloID) // Should have a particular state before collecting expect(alloStateBefore).to.be.oneOf([AllocationState.Active, AllocationState.Closed]) @@ -174,21 +178,26 @@ describe('Staking:Allocation', () => { // Collect tokens from allocation const tx = staking.connect(assetHolder.signer).collect(tokensToCollect, alloID) - await expect(tx) - .emit(staking, 'RebateCollected') - .withArgs( - assetHolder.address, - indexer.address, - subgraphDeploymentID, - alloID, - await epochManager.currentEpoch(), - tokensToCollect, - protocolFees, - curationFees, - queryFees, - queryRebates, - delegationRewards, - ) + if (expectEvent) { + await expect(tx) + .emit(staking, 'RebateCollected') + .withArgs( + assetHolder.address, + indexer.address, + subgraphDeploymentID, + alloID, + await epochManager.currentEpoch(), + tokensToCollect, + protocolFees, + curationFees, + queryFees, + queryRebates, + delegationRewards, + ) + } else { + await expect(tx).to.not.be.reverted + await expect(tx).to.not.emit(staking, 'RebateCollected') + } // After state const afterTokenSupply = await grt.totalSupply() @@ -255,8 +264,9 @@ describe('Staking:Allocation', () => { ) // Collect `tokensToCollect` with a single voucher - const rebatedAmountFull = (await shouldCollect(totalTokensToCollect, anotherAllocationID)) - .queryRebates + const rebatedAmountFull = ( + await shouldCollect(totalTokensToCollect, { allocationID: anotherAllocationID }) + ).queryRebates // Check rebated amounts match, allow a small error margin of 5 wei // Due to rounding it's not possible to guarantee an exact match in case of multiple collections @@ -322,9 +332,6 @@ describe('Staking:Allocation', () => { // Give some funds to the delegator and approve staking contract to use funds on delegator behalf await grt.connect(governor.signer).mint(delegator.address, tokensToDelegate) await grt.connect(delegator.signer).approve(staking.address, tokensToDelegate) - - // Allow the asset holder - await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true) }) beforeEach(async function () { @@ -526,6 +533,7 @@ describe('Staking:Allocation', () => { beforeEach(async function () { // Create the allocation await staking.connect(indexer.signer).stake(tokensToStake) + await advanceToNextEpoch(epochManager) await allocate(tokensToAllocate) // Add some signal to the subgraph to enable curation fees @@ -593,7 +601,7 @@ describe('Staking:Allocation', () => { }) it('should collect zero tokens', async function () { - await shouldCollect(toGRT('0')) + await shouldCollect(toGRT('0'), { expectEvent: false }) }) it('should allow multiple collections on the same allocation', async function () { @@ -641,7 +649,7 @@ describe('Staking:Allocation', () => { ) // Collect from closed allocation, should get no rebates - const rebates = await shouldCollect(tokensToCollect, anotherAllocationID) + const rebates = await shouldCollect(tokensToCollect, { allocationID: anotherAllocationID }) expect(rebates.queryRebates).eq(BigNumber.from(0)) expect(rebates.queryFeesBurnt).eq(tokensToCollect) }) @@ -665,7 +673,9 @@ describe('Staking:Allocation', () => { // First collection // Indexer gets 100% of the query fees due to α = 0 - const firstRebates = await shouldCollect(firstTokensToCollect, anotherAllocationID) + const firstRebates = await shouldCollect(firstTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(firstRebates.queryRebates).eq(firstTokensToCollect) expect(firstRebates.queryFeesBurnt).eq(BigNumber.from(0)) @@ -675,14 +685,18 @@ describe('Staking:Allocation', () => { // Second collection // Indexer gets 0% of the query fees // Parameters changed so now they are over-rebated and should get "negative rebates", instead they get 0 - const secondRebates = await shouldCollect(secondTokensToCollect, anotherAllocationID) + const secondRebates = await shouldCollect(secondTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(secondRebates.queryRebates).eq(BigNumber.from(0)) expect(secondRebates.queryFeesBurnt).eq(secondTokensToCollect) // Third collection // Previous collection plus this new one tip the balance and indexer is no longer over-rebated // They get rebates and burn again - const thirdRebates = await shouldCollect(thirdTokensToCollect, anotherAllocationID) + const thirdRebates = await shouldCollect(thirdTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(thirdRebates.queryRebates).gt(BigNumber.from(0)) expect(thirdRebates.queryFeesBurnt).gt(BigNumber.from(0)) }) @@ -706,7 +720,9 @@ describe('Staking:Allocation', () => { // First collection // Indexer gets rebates and burn - const firstRebates = await shouldCollect(firstTokensToCollect, anotherAllocationID) + const firstRebates = await shouldCollect(firstTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(firstRebates.queryRebates).gt(BigNumber.from(0)) expect(firstRebates.queryFeesBurnt).gt(BigNumber.from(0)) @@ -716,18 +732,26 @@ describe('Staking:Allocation', () => { // Second collection // Indexer gets 100% of the query fees // Parameters changed so now they are under-rebated and should get more than the available amount but we cap it - const secondRebates = await shouldCollect(secondTokensToCollect, anotherAllocationID) + const secondRebates = await shouldCollect(secondTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(secondRebates.queryRebates).eq(secondTokensToCollect) expect(secondRebates.queryFeesBurnt).eq(BigNumber.from(0)) // Third collection // Previous collection plus this new one tip the balance and indexer is no longer under-rebated // They get rebates and burn again - const thirdRebates = await shouldCollect(thirdTokensToCollect, anotherAllocationID) + const thirdRebates = await shouldCollect(thirdTokensToCollect, { + allocationID: anotherAllocationID, + }) expect(thirdRebates.queryRebates).gt(BigNumber.from(0)) expect(thirdRebates.queryFeesBurnt).gt(BigNumber.from(0)) }) + it('should collect zero tokens', async function () { + await shouldCollect(toGRT('0'), { expectEvent: false }) + }) + it('should get stuck under-rebated if alpha is changed to zero', async function () { // Set up a new allocation with `tokensToAllocate` staked await staking.connect(indexer.signer).stake(tokensToStake) @@ -742,7 +766,9 @@ describe('Staking:Allocation', () => { // First collection // Indexer gets rebates and burn - const firstRebates = await shouldCollect(tokensToCollect, anotherAllocationID) + const firstRebates = await shouldCollect(tokensToCollect, { + allocationID: anotherAllocationID, + }) expect(firstRebates.queryRebates).gt(BigNumber.from(0)) expect(firstRebates.queryFeesBurnt).gt(BigNumber.from(0)) @@ -754,7 +780,9 @@ describe('Staking:Allocation', () => { // Parameters changed so now they are under-rebated and should get more than the available amount but we cap it // Distributed amount will never catch up due to the initial collection which was less than 100% for (const _i of [...Array(10).keys()]) { - const succesiveRebates = await shouldCollect(tokensToCollect, anotherAllocationID) + const succesiveRebates = await shouldCollect(tokensToCollect, { + allocationID: anotherAllocationID, + }) expect(succesiveRebates.queryRebates).eq(tokensToCollect) expect(succesiveRebates.queryFeesBurnt).eq(BigNumber.from(0)) } diff --git a/test/staking/configuration.test.ts b/test/staking/configuration.test.ts index 4e4a2ed7a..53a0b312c 100644 --- a/test/staking/configuration.test.ts +++ b/test/staking/configuration.test.ts @@ -88,35 +88,6 @@ describe('Staking:Config', () => { }) }) - describe('setAssetHolder', function () { - it('should set `assetHolder`', async function () { - expect(await staking.assetHolders(me.address)).eq(false) - - const tx1 = staking.connect(governor.signer).setAssetHolder(me.address, true) - await expect(tx1) - .emit(staking, 'AssetHolderUpdate') - .withArgs(governor.address, me.address, true) - expect(await staking.assetHolders(me.address)).eq(true) - - const tx2 = staking.connect(governor.signer).setAssetHolder(me.address, false) - await expect(tx2) - .emit(staking, 'AssetHolderUpdate') - .withArgs(governor.address, me.address, false) - await staking.connect(governor.signer).setAssetHolder(me.address, false) - expect(await staking.assetHolders(me.address)).eq(false) - }) - - it('reject set `assetHolder` if not allowed', async function () { - const tx = staking.connect(other.signer).setAssetHolder(me.address, true) - await expect(tx).revertedWith('Only Controller governor') - }) - - it('reject set `assetHolder` to address zero', async function () { - const tx = staking.connect(governor.signer).setAssetHolder(AddressZero, true) - await expect(tx).revertedWith('!assetHolder') - }) - }) - describe('curationPercentage', function () { it('should set `curationPercentage`', async function () { const newValue = toBN('5') diff --git a/test/staking/delegation.test.ts b/test/staking/delegation.test.ts index 35a65f997..811b84cae 100644 --- a/test/staking/delegation.test.ts +++ b/test/staking/delegation.test.ts @@ -20,6 +20,7 @@ import { const { AddressZero, HashZero } = constants const MAX_PPM = toBN('1000000') +const tokensToCollect = toGRT('50000000000000000000') describe('Staking::Delegation', () => { let me: Account @@ -190,13 +191,12 @@ describe('Staking::Delegation', () => { } // Distribute test funds - for (const wallet of [me, indexer, indexer2, assetHolder]) { + for (const wallet of [me, indexer, indexer2]) { await grt.connect(governor.signer).mint(wallet.address, toGRT('1000000')) await grt.connect(wallet.signer).approve(staking.address, toGRT('1000000')) } - - // Allow the asset holder - await staking.connect(governor.signer).setAssetHolder(assetHolder.address, true) + await grt.connect(governor.signer).mint(assetHolder.address, tokensToCollect) + await grt.connect(assetHolder.signer).approve(staking.address, tokensToCollect) }) beforeEach(async function () { @@ -372,7 +372,20 @@ describe('Staking::Delegation', () => { it('reject delegate with zero tokens', async function () { const tokensToDelegate = toGRT('0') const tx = staking.connect(delegator.signer).delegate(indexer.address, tokensToDelegate) - await expect(tx).revertedWith('!tokens') + await expect(tx).revertedWith('!minimum-delegation') + }) + + it('reject delegate with less than 1 GRT when the pool is not initialized', async function () { + const tokensToDelegate = toGRT('0.5') + const tx = staking.connect(delegator.signer).delegate(indexer.address, tokensToDelegate) + await expect(tx).revertedWith('!minimum-delegation') + }) + + it('reject delegating under 1 GRT when the pool is initialized', async function () { + await shouldDelegate(delegator, toGRT('1')) + const tokensToDelegate = toGRT('0.5') + const tx = staking.connect(delegator.signer).delegate(indexer.address, tokensToDelegate) + await expect(tx).revertedWith('!minimum-delegation') }) it('reject delegate to empty address', async function () { @@ -471,6 +484,12 @@ describe('Staking::Delegation', () => { await advanceToNextEpoch(epochManager) // epoch 2 await shouldUndelegate(delegator, toGRT('10')) }) + + it('reject undelegate if remaining tokens are less than the minimum', async function () { + await shouldDelegate(delegator, toGRT('100')) + const tx = staking.connect(delegator.signer).undelegate(indexer.address, toGRT('99.5')) + await expect(tx).revertedWith('!minimum-delegation') + }) }) describe('withdraw', function () { @@ -526,7 +545,6 @@ describe('Staking::Delegation', () => { // Test values const tokensToStake = toGRT('200') const tokensToAllocate = toGRT('2000') - const tokensToCollect = toGRT('500') const tokensToDelegate = toGRT('1800') const subgraphDeploymentID = randomHexBytes() const channelKey = deriveChannelKey() @@ -596,19 +614,24 @@ describe('Staking::Delegation', () => { it('revert if it cannot assign the smallest amount of shares', async function () { // Init the delegation pool - await shouldDelegate(delegator, tokensToDelegate) - + await shouldDelegate(delegator, toGRT('1')) + const tokensToAllocate = toGRT('1') // Collect funds thru full allocation cycle + // Set rebate alpha to 0 to ensure all fees are collected + await staking.connect(governor.signer).setRebateParameters(0, 1, 1, 1) await staking.connect(governor.signer).setDelegationRatio(10) await staking.connect(indexer.signer).setDelegationParameters(0, 0, 0) await setupAllocation(tokensToAllocate) + await advanceToNextEpoch(epochManager) await staking.connect(assetHolder.signer).collect(tokensToCollect, allocationID) await advanceToNextEpoch(epochManager) await staking.connect(indexer.signer).closeAllocation(allocationID, poi) + // We've callected 5e18 GRT (a ridiculous amount), + // which means the price of a share is now 5 GRT - // Delegate with such small amount of tokens (1 wei) that we do not have enough precision - // to even assign 1 wei of shares - const tx = staking.connect(delegator.signer).delegate(indexer.address, toBN(1)) + // Delegate with such small amount of tokens (1 GRT) that we do not have enough precision + // to even assign 1 share + const tx = staking.connect(delegator.signer).delegate(indexer.address, toGRT('1')) await expect(tx).revertedWith('!shares') }) })