Merge pull request #866 from graphprotocol/pcv/remove-delegation-params-cooldown

pcarranzav · web-flow · commit 51204950c927 · 2023-12-07T15:17:13.000-03:00
feat: remove/deprecate delegation parameters cooldown
diff --git a/cli/commands/contracts/staking.ts b/cli/commands/contracts/staking.ts
@@ -97,9 +97,9 @@ export const setDelegationParameters = async (
   const staking = cli.contracts.Staking
 
   logger.info(`Setting the following delegation parameters for indexer ${cli.walletAddress}
-      indexingRewardCut = ${indexingRewardCut}
-      queryFeeCut       = ${queryFeeCut}
-      cooldownBlocks    = ${cooldownBlocks}
+      indexingRewardCut           = ${indexingRewardCut}
+      queryFeeCut                 = ${queryFeeCut}
+      cooldownBlocks (deprecated) = ${cooldownBlocks}
   `)
   await sendTransaction(cli.wallet, staking, 'setDelegationParameters', [
     indexingRewardCut,
@@ -305,7 +305,7 @@ export const stakingCommand = {
               demandOption: true,
             })
             .option('cooldownBlocks', {
-              description: 'Period that need to pass to update delegation parameters',
+              description: 'Period that need to pass to update delegation parameters (deprecated)',
               type: 'number',
             })
         },
diff --git a/cli/commands/protocol/get.ts b/cli/commands/protocol/get.ts
@@ -13,10 +13,6 @@ export const gettersList = {
   'staking-thawing-period': { contract: 'Staking', name: 'thawingPeriod' },
   'staking-max-allocation-epochs': { contract: 'Staking', name: 'maxAllocationEpochs' },
   'staking-delegation-ratio': { contract: 'Staking', name: 'delegationRatio' },
-  'staking-delegation-parameters-cooldown': {
-    contract: 'Staking',
-    name: 'delegationParametersCooldown',
-  },
   'staking-delegation-unbonding-period': { contract: 'Staking', name: 'delegationUnbondingPeriod' },
   'protocol-percentage': { contract: 'Staking', name: 'protocolPercentage' },
   // Curation
diff --git a/cli/commands/protocol/set.ts b/cli/commands/protocol/set.ts
@@ -14,10 +14,6 @@ export const settersList = {
   'staking-max-allocation-epochs': { contract: 'Staking', name: 'setMaxAllocationEpochs' },
   'staking-protocol-percentage': { contract: 'Staking', name: 'setProtocolPercentage' },
   'staking-delegation-parameters': { contract: 'Staking', name: 'setDelegationParameters' },
-  'staking-delegation-parameters-cooldown': {
-    contract: 'Staking',
-    name: 'setDelegationParametersCooldown',
-  },
   'staking-delegation-unbonding-period': {
     contract: 'Staking',
     name: 'setDelegationUnbondingPeriod',
diff --git a/contracts/discovery/GNS.sol b/contracts/discovery/GNS.sol
@@ -257,7 +257,7 @@ abstract contract GNS is GNSV3Storage, GraphUpgradeable, IGNS, Multicall {
         uint256 subgraphID = _nextSubgraphID(subgraphOwner);
         SubgraphData storage subgraphData = _getSubgraphData(subgraphID);
         subgraphData.subgraphDeploymentID = _subgraphDeploymentID;
-        subgraphData.reserveRatioDeprecated = fixedReserveRatio;
+        subgraphData.__DEPRECATED_reserveRatio = fixedReserveRatio;
 
         // Mint the NFT. Use the subgraphID as tokenID.
         // This function will check the if tokenID already exists.
@@ -371,7 +371,7 @@ abstract contract GNS is GNSV3Storage, GraphUpgradeable, IGNS, Multicall {
         // Deprecate the subgraph and do cleanup
         subgraphData.disabled = true;
         subgraphData.vSignal = 0;
-        subgraphData.reserveRatioDeprecated = 0;
+        subgraphData.__DEPRECATED_reserveRatio = 0;
         // NOTE: We don't reset the following variable as we use it to test if the Subgraph was ever created
         // subgraphData.subgraphDeploymentID = 0;
 
diff --git a/contracts/discovery/IGNS.sol b/contracts/discovery/IGNS.sol
@@ -18,7 +18,7 @@ interface IGNS {
         uint256 nSignal; // The token of the subgraph bonding curve
         mapping(address => uint256) curatorNSignal;
         bytes32 subgraphDeploymentID;
-        uint32 reserveRatioDeprecated; // Ratio for the bonding curve, always 1 in PPM, deprecated.
+        uint32 __DEPRECATED_reserveRatio; // solhint-disable-line var-name-mixedcase
         bool disabled;
         uint256 withdrawableGRT;
     }
diff --git a/contracts/discovery/L1GNS.sol b/contracts/discovery/L1GNS.sol
@@ -107,7 +107,7 @@ contract L1GNS is GNS, L1GNSV1Storage {
             extraData
         );
 
-        subgraphData.reserveRatioDeprecated = 0;
+        subgraphData.__DEPRECATED_reserveRatio = 0;
         _burnNFT(_subgraphID);
         emit SubgraphSentToL2(_subgraphID, msg.sender, _l2Owner, tokensForL2);
     }
diff --git a/contracts/l2/discovery/L2GNS.sol b/contracts/l2/discovery/L2GNS.sol
@@ -282,7 +282,7 @@ contract L2GNS is GNS, L2GNSV1Storage, IL2GNS {
         SubgraphData storage subgraphData = _getSubgraphData(l2SubgraphID);
         IL2GNS.SubgraphL2TransferData storage transferData = subgraphL2TransferData[l2SubgraphID];
 
-        subgraphData.reserveRatioDeprecated = fixedReserveRatio;
+        subgraphData.__DEPRECATED_reserveRatio = fixedReserveRatio;
         // The subgraph will be disabled until finishSubgraphTransferFromL1 is called
         subgraphData.disabled = true;
 
diff --git a/contracts/l2/staking/L2Staking.sol b/contracts/l2/staking/L2Staking.sol
@@ -102,7 +102,7 @@ contract L2Staking is Staking, IL2StakingBase {
 
         // Initialize the delegation pool the first time
         if (__delegationPools[_indexer].updatedAtBlock == 0) {
-            _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM, __delegationParametersCooldown);
+            _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM);
         }
 
         emit StakeDeposited(_indexer, _amount);
diff --git a/contracts/rewards/RewardsManagerStorage.sol b/contracts/rewards/RewardsManagerStorage.sol
@@ -8,7 +8,7 @@ import "../governance/Managed.sol";
 contract RewardsManagerV1Storage is Managed {
     // -- State --
 
-    uint256 private issuanceRateDeprecated;
+    uint256 private __DEPRECATED_issuanceRate; // solhint-disable-line var-name-mixedcase
     uint256 public accRewardsPerSignal;
     uint256 public accRewardsPerSignalLastBlockUpdated;
 
@@ -29,7 +29,7 @@ contract RewardsManagerV2Storage is RewardsManagerV1Storage {
 
 contract RewardsManagerV3Storage is RewardsManagerV2Storage {
     // Snapshot of the total supply of GRT when accRewardsPerSignal was last updated
-    uint256 private tokenSupplySnapshotDeprecated;
+    uint256 private __DEPRECATED_tokenSupplySnapshot; // solhint-disable-line var-name-mixedcase
 }
 
 contract RewardsManagerV4Storage is RewardsManagerV3Storage {
diff --git a/contracts/staking/IStakingBase.sol b/contracts/staking/IStakingBase.sol
@@ -90,7 +90,7 @@ interface IStakingBase is IStakingData {
         address indexed indexer,
         uint32 indexingRewardCut,
         uint32 queryFeeCut,
-        uint32 cooldownBlocks
+        uint32 __DEPRECATED_cooldownBlocks // solhint-disable-line var-name-mixedcase
     );
 
     /**
@@ -260,12 +260,11 @@ interface IStakingBase is IStakingData {
      * @notice Set the delegation parameters for the caller.
      * @param _indexingRewardCut Percentage of indexing rewards left for the indexer
      * @param _queryFeeCut Percentage of query fees left for the indexer
-     * @param _cooldownBlocks Period that need to pass to update delegation parameters
      */
     function setDelegationParameters(
         uint32 _indexingRewardCut,
         uint32 _queryFeeCut,
-        uint32 _cooldownBlocks
+        uint32 // _cooldownBlocks, deprecated
     ) external;
 
     /**
diff --git a/contracts/staking/IStakingData.sol b/contracts/staking/IStakingData.sol
@@ -18,7 +18,7 @@ interface IStakingData {
         uint256 createdAtEpoch; // Epoch when it was created
         uint256 closedAtEpoch; // Epoch when it was closed
         uint256 collectedFees; // Collected fees for the allocation
-        uint256 __DEPRECATED_effectiveAllocation; // Effective allocation when closed
+        uint256 __DEPRECATED_effectiveAllocation; // solhint-disable-line var-name-mixedcase
         uint256 accRewardsPerAllocatedToken; // Snapshot used for reward calc
         uint256 distributedRebates; // Collected rebates that have been rebated
     }
@@ -29,7 +29,7 @@ interface IStakingData {
      * @dev Delegation pool information. One per indexer.
      */
     struct DelegationPool {
-        uint32 cooldownBlocks; // Blocks to wait before updating parameters
+        uint32 __DEPRECATED_cooldownBlocks; // solhint-disable-line var-name-mixedcase
         uint32 indexingRewardCut; // in PPM
         uint32 queryFeeCut; // in PPM
         uint256 updatedAtBlock; // Block when the pool was last updated
diff --git a/contracts/staking/IStakingExtension.sol b/contracts/staking/IStakingExtension.sol
@@ -20,7 +20,7 @@ interface IStakingExtension is IStakingData {
      * the original DelegationPool in IStakingData.sol contains a nested mapping.
      */
     struct DelegationPoolReturn {
-        uint32 cooldownBlocks; // Blocks to wait before updating parameters
+        uint32 __DEPRECATED_cooldownBlocks; // solhint-disable-line var-name-mixedcase
         uint32 indexingRewardCut; // in PPM
         uint32 queryFeeCut; // in PPM
         uint256 updatedAtBlock; // Block when the pool was last updated
@@ -85,14 +85,6 @@ interface IStakingExtension is IStakingData {
      */
     function setDelegationRatio(uint32 _delegationRatio) external;
 
-    /**
-     * @notice Set the minimum time in blocks an indexer needs to wait to change delegation parameters.
-     * Indexers can set a custom amount time for their own cooldown, but it must be greater than this.
-     * @dev This function is only callable by the governor
-     * @param _blocks Number of blocks to set the delegation parameters cooldown period
-     */
-    function setDelegationParametersCooldown(uint32 _blocks) external;
-
     /**
      * @notice Set the time, in epochs, a Delegator needs to wait to withdraw tokens after undelegating.
      * @dev This function is only callable by the governor
@@ -191,13 +183,6 @@ interface IStakingExtension is IStakingData {
      */
     function delegationRatio() external view returns (uint32);
 
-    /**
-     * @notice Getter for delegationParametersCooldown:
-     * Minimum time in blocks an indexer needs to wait to change delegation parameters
-     * @return Delegation parameters cooldown in blocks
-     */
-    function delegationParametersCooldown() external view returns (uint32);
-
     /**
      * @notice Getter for delegationUnbondingPeriod:
      * Time in epochs a delegator needs to wait to withdraw delegated stake
diff --git a/contracts/staking/Staking.sol b/contracts/staking/Staking.sol
@@ -544,14 +544,13 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
      * @notice Set the delegation parameters for the caller.
      * @param _indexingRewardCut Percentage of indexing rewards left for the indexer
      * @param _queryFeeCut Percentage of query fees left for the indexer
-     * @param _cooldownBlocks Period that need to pass to update delegation parameters
      */
     function setDelegationParameters(
         uint32 _indexingRewardCut,
         uint32 _queryFeeCut,
-        uint32 _cooldownBlocks
+        uint32 // _cooldownBlocks, deprecated
     ) public override {
-        _setDelegationParameters(msg.sender, _indexingRewardCut, _queryFeeCut, _cooldownBlocks);
+        _setDelegationParameters(msg.sender, _indexingRewardCut, _queryFeeCut);
     }
 
     /**
@@ -661,41 +660,24 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
      * @param _indexer Indexer to set delegation parameters
      * @param _indexingRewardCut Percentage of indexing rewards left for delegators
      * @param _queryFeeCut Percentage of query fees left for delegators
-     * @param _cooldownBlocks Period that need to pass to update delegation parameters
      */
     function _setDelegationParameters(
         address _indexer,
         uint32 _indexingRewardCut,
-        uint32 _queryFeeCut,
-        uint32 _cooldownBlocks
+        uint32 _queryFeeCut
     ) internal {
         // Incentives must be within bounds
         require(_queryFeeCut <= MAX_PPM, ">queryFeeCut");
         require(_indexingRewardCut <= MAX_PPM, ">indexingRewardCut");
 
-        // Cooldown period set by indexer cannot be below protocol global setting
-        require(_cooldownBlocks >= __delegationParametersCooldown, "<cooldown");
-
-        // Verify the cooldown period passed
         DelegationPool storage pool = __delegationPools[_indexer];
-        require(
-            pool.updatedAtBlock == 0 ||
-                pool.updatedAtBlock.add(uint256(pool.cooldownBlocks)) <= block.number,
-            "!cooldown"
-        );
 
         // Update delegation params
         pool.indexingRewardCut = _indexingRewardCut;
         pool.queryFeeCut = _queryFeeCut;
-        pool.cooldownBlocks = _cooldownBlocks;
         pool.updatedAtBlock = block.number;
 
-        emit DelegationParametersUpdated(
-            _indexer,
-            _indexingRewardCut,
-            _queryFeeCut,
-            _cooldownBlocks
-        );
+        emit DelegationParametersUpdated(_indexer, _indexingRewardCut, _queryFeeCut, 0);
     }
 
     /**
@@ -718,7 +700,7 @@ abstract contract Staking is StakingV4Storage, GraphUpgradeable, IStakingBase, M
 
         // Initialize the delegation pool the first time
         if (__delegationPools[_indexer].updatedAtBlock == 0) {
-            _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM, __delegationParametersCooldown);
+            _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM);
         }
 
         emit StakeDeposited(_indexer, _tokens);
diff --git a/contracts/staking/StakingExtension.sol b/contracts/staking/StakingExtension.sol
@@ -43,18 +43,16 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi
      * initialize() function, so it uses the same access control check to ensure it is
      * being called by the Staking implementation as part of the proxy upgrade process.
      * @param _delegationUnbondingPeriod Delegation unbonding period in blocks
-     * @param _cooldownBlocks Minimum time between changes to delegation parameters, in blocks
      * @param _delegationRatio Delegation capacity multiplier (e.g. 10 means 10x the indexer stake)
      * @param _delegationTaxPercentage Percentage of delegated tokens to burn as delegation tax, expressed in parts per million
      */
     function initialize(
         uint32 _delegationUnbondingPeriod,
-        uint32 _cooldownBlocks,
+        uint32, //_cooldownBlocks, deprecated
         uint32 _delegationRatio,
         uint32 _delegationTaxPercentage
     ) external onlyImpl {
         _setDelegationUnbondingPeriod(_delegationUnbondingPeriod);
-        _setDelegationParametersCooldown(_cooldownBlocks);
         _setDelegationRatio(_delegationRatio);
         _setDelegationTaxPercentage(_delegationTaxPercentage);
     }
@@ -79,16 +77,6 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi
         _setDelegationRatio(_delegationRatio);
     }
 
-    /**
-     * @notice Set the minimum time in blocks an indexer needs to wait to change delegation parameters.
-     * Indexers can set a custom amount time for their own cooldown, but it must be greater than this.
-     * @dev This function is only callable by the governor
-     * @param _blocks Number of blocks to set the delegation parameters cooldown period
-     */
-    function setDelegationParametersCooldown(uint32 _blocks) external override onlyGovernor {
-        _setDelegationParametersCooldown(_blocks);
-    }
-
     /**
      * @notice Set the time, in epochs, a Delegator needs to wait to withdraw tokens after undelegating.
      * @dev This function is only callable by the governor
@@ -242,15 +230,6 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi
         return __delegationRatio;
     }
 
-    /**
-     * @notice Getter for delegationParametersCooldown:
-     * Minimum time in blocks an indexer needs to wait to change delegation parameters
-     * @return Delegation parameters cooldown in blocks
-     */
-    function delegationParametersCooldown() external view override returns (uint32) {
-        return __delegationParametersCooldown;
-    }
-
     /**
      * @notice Getter for delegationUnbondingPeriod:
      * Time in epochs a delegator needs to wait to withdraw delegated stake
@@ -284,7 +263,7 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi
         DelegationPool storage pool = __delegationPools[_indexer];
         return
             DelegationPoolReturn(
-                pool.cooldownBlocks, // Blocks to wait before updating parameters
+                0, // Blocks to wait before updating parameters (deprecated)
                 pool.indexingRewardCut, // in PPM
                 pool.queryFeeCut, // in PPM
                 pool.updatedAtBlock, // Block when the pool was last updated
@@ -499,15 +478,6 @@ contract StakingExtension is StakingV4Storage, GraphUpgradeable, IStakingExtensi
         emit ParameterUpdated("delegationRatio");
     }
 
-    /**
-     * @dev Internal: Set the time in blocks an indexer needs to wait to change delegation parameters.
-     * @param _blocks Number of blocks to set the delegation parameters cooldown period
-     */
-    function _setDelegationParametersCooldown(uint32 _blocks) private {
-        __delegationParametersCooldown = _blocks;
-        emit ParameterUpdated("delegationParametersCooldown");
-    }
-
     /**
      * @dev Internal: Set the period for undelegation of stake from indexer.
      * @param _delegationUnbondingPeriod Period in epochs to wait for token withdrawals after undelegating
diff --git a/contracts/staking/StakingStorage.sol b/contracts/staking/StakingStorage.sol
@@ -32,7 +32,7 @@ contract StakingV1Storage is Managed {
     uint32 internal __protocolPercentage;
 
     /// @dev Period for allocation to be finalized
-    uint32 private __DEPRECATED_channelDisputeEpochs;
+    uint32 private __DEPRECATED_channelDisputeEpochs; // solhint-disable-line var-name-mixedcase
 
     /// @dev Maximum allocation time
     uint32 internal __maxAllocationEpochs;
@@ -55,7 +55,7 @@ contract StakingV1Storage is Managed {
     mapping(bytes32 => uint256) internal __subgraphAllocations;
 
     // Rebate pools : epoch => Pool
-    mapping(uint256 => uint256) private __DEPRECATED_rebates;
+    mapping(uint256 => uint256) private __DEPRECATED_rebates; // solhint-disable-line var-name-mixedcase
 
     // -- Slashing --
 
@@ -69,8 +69,8 @@ contract StakingV1Storage is Managed {
     /// then they can use up to 500 GRT from the delegated stake
     uint32 internal __delegationRatio;
 
-    /// @dev Time in blocks an indexer needs to wait to change delegation parameters
-    uint32 internal __delegationParametersCooldown;
+    /// @dev Time in blocks an indexer needs to wait to change delegation parameters (deprecated)
+    uint32 internal __DEPRECATED_delegationParametersCooldown; // solhint-disable-line var-name-mixedcase
 
     /// @dev Time in epochs a delegator needs to wait to withdraw delegated stake
     uint32 internal __delegationUnbondingPeriod; // in epochs
diff --git a/test/l2/l2GNS.test.ts b/test/l2/l2GNS.test.ts
@@ -307,7 +307,7 @@ describe('L2GNS', () => {
       expect(subgraphData.vSignal).eq(0)
       expect(subgraphData.nSignal).eq(0)
       expect(subgraphData.subgraphDeploymentID).eq(HashZero)
-      expect(subgraphData.reserveRatioDeprecated).eq(DEFAULT_RESERVE_RATIO)
+      expect(subgraphData.__DEPRECATED_reserveRatio).eq(DEFAULT_RESERVE_RATIO)
       expect(subgraphData.disabled).eq(true)
       expect(subgraphData.withdrawableGRT).eq(0) // Important so that it's not the same as a deprecated subgraph!
 
@@ -347,7 +347,7 @@ describe('L2GNS', () => {
       expect(subgraphData.vSignal).eq(0)
       expect(subgraphData.nSignal).eq(0)
       expect(subgraphData.subgraphDeploymentID).eq(HashZero)
-      expect(subgraphData.reserveRatioDeprecated).eq(DEFAULT_RESERVE_RATIO)
+      expect(subgraphData.__DEPRECATED_reserveRatio).eq(DEFAULT_RESERVE_RATIO)
       expect(subgraphData.disabled).eq(true)
       expect(subgraphData.withdrawableGRT).eq(0) // Important so that it's not the same as a deprecated subgraph!
 
@@ -358,7 +358,7 @@ describe('L2GNS', () => {
       expect(l2SubgraphData.vSignal).eq(0)
       expect(l2SubgraphData.nSignal).eq(0)
       expect(l2SubgraphData.subgraphDeploymentID).eq(l2Subgraph.subgraphDeploymentID)
-      expect(l2SubgraphData.reserveRatioDeprecated).eq(DEFAULT_RESERVE_RATIO)
+      expect(l2SubgraphData.__DEPRECATED_reserveRatio).eq(DEFAULT_RESERVE_RATIO)
       expect(l2SubgraphData.disabled).eq(false)
       expect(l2SubgraphData.withdrawableGRT).eq(0)
     })
diff --git a/test/lib/gnsUtils.ts b/test/lib/gnsUtils.ts
diff --git a/test/rewards/rewards.test.ts b/test/rewards/rewards.test.ts
diff --git a/test/staking/delegation.test.ts b/test/staking/delegation.test.ts

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ interface IGNS {`
`18`	`18`	`uint256 nSignal; // The token of the subgraph bonding curve`
`19`	`19`	`mapping(address => uint256) curatorNSignal;`
`20`	`20`	`bytes32 subgraphDeploymentID;`
`21`		`- uint32 reserveRatioDeprecated; // Ratio for the bonding curve, always 1 in PPM, deprecated.`
	`21`	`+ uint32 __DEPRECATED_reserveRatio; // solhint-disable-line var-name-mixedcase`
`22`	`22`	`bool disabled;`
`23`	`23`	`uint256 withdrawableGRT;`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ contract L1GNS is GNS, L1GNSV1Storage {`
`107`	`107`	`extraData`
`108`	`108`	`);`
`109`	`109`
`110`		`- subgraphData.reserveRatioDeprecated = 0;`
	`110`	`+ subgraphData.__DEPRECATED_reserveRatio = 0;`
`111`	`111`	`_burnNFT(_subgraphID);`
`112`	`112`	`emit SubgraphSentToL2(_subgraphID, msg.sender, _l2Owner, tokensForL2);`
`113`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ contract L2Staking is Staking, IL2StakingBase {`
`102`	`102`
`103`	`103`	`// Initialize the delegation pool the first time`
`104`	`104`	`if (__delegationPools[_indexer].updatedAtBlock == 0) {`
`105`		`- _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM, __delegationParametersCooldown);`
	`105`	`+ _setDelegationParameters(_indexer, MAX_PPM, MAX_PPM);`
`106`	`106`	`}`
`107`	`107`
`108`	`108`	`emit StakeDeposited(_indexer, _amount);`