Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove/deprecate delegation parameters cooldown #866

Merged
merged 3 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cli/commands/contracts/staking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
})
},
Expand Down
4 changes: 0 additions & 4 deletions cli/commands/protocol/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions cli/commands/protocol/set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions contracts/discovery/GNS.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion contracts/discovery/IGNS.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/discovery/L1GNS.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/l2/discovery/L2GNS.sol
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,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;

Expand Down
2 changes: 1 addition & 1 deletion contracts/l2/staking/L2Staking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,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);
Expand Down
4 changes: 2 additions & 2 deletions contracts/rewards/RewardsManagerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions contracts/staking/IStakingBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
);

/**
Expand Down Expand Up @@ -274,12 +274,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;

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/staking/IStakingData.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
17 changes: 1 addition & 16 deletions contracts/staking/IStakingExtension.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 5 additions & 23 deletions contracts/staking/Staking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -554,14 +554,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);
}

/**
Expand Down Expand Up @@ -671,41 +670,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);
}

/**
Expand All @@ -728,7 +710,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);
Expand Down
34 changes: 2 additions & 32 deletions contracts/staking/StakingExtension.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,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
Copy link
Contributor

@tmigone tmigone Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to keep this? I understand doing it for setDelegationParameters so we don't change the interface but I think we can entirely remove it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I'd rather keep it to keep changes minimal (this is called from Staking.initialize, so we'd have to change that too)

uint32 _delegationRatio,
uint32 _delegationTaxPercentage
) external onlyImpl {
_setDelegationUnbondingPeriod(_delegationUnbondingPeriod);
_setDelegationParametersCooldown(_cooldownBlocks);
_setDelegationRatio(_delegationRatio);
_setDelegationTaxPercentage(_delegationTaxPercentage);
}
Expand All @@ -77,16 +75,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
Expand Down Expand Up @@ -240,15 +228,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
Expand Down Expand Up @@ -282,7 +261,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
Expand Down Expand Up @@ -508,15 +487,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
Expand Down
8 changes: 4 additions & 4 deletions contracts/staking/StakingStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 --

Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test/l2/l2GNS.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,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!

Expand Down Expand Up @@ -334,7 +334,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!

Expand All @@ -345,7 +345,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)
})
Expand Down
Loading