Skip to content

Commit

Permalink
fix: remove minimum delegation requirement from delegation flows
Browse files Browse the repository at this point in the history 
Signed-off-by: Tomás Migone <[email protected]>
  • Loading branch information
@tmigone
tmigone committed Oct 1, 2024
1 parent cf03335 commit 41ad6e1
Show file tree
Hide file tree
Showing 13 changed files with 21 additions and 84 deletions.
18 changes: 1 addition & 17 deletions packages/horizon/contracts/staking/HorizonStaking.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import { HorizonStakingBase } from "./HorizonStakingBase.sol";
* It is designed to be deployed as an upgrade to the L2Staking contract from the legacy contracts package.
* @dev It uses a {HorizonStakingExtension} contract to implement the full {IHorizonStaking} interface through delegatecalls.
* This is due to the contract size limit on Arbitrum (24kB). The extension contract implements functionality to support
* the legacy staking functions and the transfer tools. Both can be eventually removed without affecting the main
* staking contract.
* the legacy staking functions. It can be eventually removed without affecting the main staking contract.
* @custom:security-contact Please email [email protected] if you find any
* bugs. We may have an active bug bounty program.
*/
Expand All @@ -42,11 +41,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
/// @dev Maximum number of simultaneous stake thaw requests (per provision) or undelegations (per delegation)
uint256 private constant MAX_THAW_REQUESTS = 100;

/// @dev Minimum amount of delegation to prevent rounding attacks.
/// TODO: remove this after L2 transfer tool for delegation is removed
/// (delegation on L2 has its own slippage protection)
uint256 private constant MIN_DELEGATION = 1e18;

/// @dev Address of the staking extension contract
address private immutable STAKING_EXTENSION_ADDRESS;

Expand Down Expand Up @@ -724,8 +718,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
* have been done before calling this function.
*/
function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private {
// TODO: remove this after L2 transfer tool for delegation is removed
require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientTokens(_tokens, MIN_DELEGATION));
require(
_provisions[_serviceProvider][_verifier].createdAt != 0,
HorizonStakingInvalidProvision(_serviceProvider, _verifier)
Expand Down Expand Up @@ -773,14 +765,6 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
pool.sharesThawing = pool.sharesThawing + thawingShares;

delegation.shares = delegation.shares - _shares;
// TODO: remove this when L2 transfer tools are removed
if (delegation.shares != 0) {
uint256 remainingTokens = (delegation.shares * (pool.tokens - pool.tokensThawing)) / pool.shares;
require(
remainingTokens >= MIN_DELEGATION,
HorizonStakingInsufficientTokens(remainingTokens, MIN_DELEGATION)
);
}

bytes32 thawRequestId = _createThawRequest(
_serviceProvider,
Expand Down
3 changes: 0 additions & 3 deletions packages/horizon/contracts/staking/HorizonStakingExtension.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ contract HorizonStakingExtension is HorizonStakingBase, IHorizonStakingExtension
using TokenUtils for IGraphToken;
using PPMMath for uint256;

/// @dev Minimum amount of tokens that can be delegated
uint256 private constant MINIMUM_DELEGATION = 1e18;

/**
* @dev Checks that the sender is the L2GraphTokenGateway as configured on the Controller.
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/horizon/test/escrow/collect.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest {
uint256 tokensDelegatoion = tokens * delegationFeeCut / MAX_PPM;
vm.assume(tokensDataService < tokens - tokensProtocol - tokensDelegatoion);

vm.assume(delegationTokens > MIN_DELEGATION);
vm.assume(delegationTokens <= MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

Expand Down
3 changes: 1 addition & 2 deletions packages/horizon/test/payments/GraphPayments.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest {
address escrowAddress = address(escrow);

// Delegate tokens
vm.assume(tokensDelegate > MIN_DELEGATION);
vm.assume(tokensDelegate <= MAX_STAKING_TOKENS);
tokensDelegate = bound(tokensDelegate, 1, MAX_STAKING_TOKENS);
vm.startPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, tokensDelegate, 0);

Expand Down
1 change: 0 additions & 1 deletion packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { IGraphPayments } from "../../../contracts/interfaces/IGraphPayments.sol
import { IHorizonStakingBase } from "../../../contracts/interfaces/internal/IHorizonStakingBase.sol";
import { IHorizonStakingMain } from "../../../contracts/interfaces/internal/IHorizonStakingMain.sol";
import { IHorizonStakingExtension } from "../../../contracts/interfaces/internal/IHorizonStakingExtension.sol";
import { IL2StakingBase } from "@graphprotocol/contracts/contracts/l2/staking/IL2StakingBase.sol";

import { LinkedList } from "../../../contracts/libraries/LinkedList.sol";
import { MathUtils } from "../../../contracts/libraries/MathUtils.sol";
Expand Down
2 changes: 1 addition & 1 deletion packages/horizon/test/staking/HorizonStaking.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract HorizonStakingTest is HorizonStakingSharedTest {
modifier useDelegation(uint256 delegationAmount) {
address msgSender;
(, msgSender, ) = vm.readCallers();
vm.assume(delegationAmount > MIN_DELEGATION);
vm.assume(delegationAmount > 1);
vm.assume(delegationAmount <= MAX_STAKING_TOKENS);
vm.startPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0);
Expand Down
2 changes: 1 addition & 1 deletion packages/horizon/test/staking/allocation/close.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ contract HorizonStakingCloseAllocationTest is HorizonStakingTest {

function testCloseAllocation_WithDelegation(uint256 tokens, uint256 delegationTokens, uint32 indexingRewardCut) public useIndexer useAllocation(1 ether) {
tokens = bound(tokens, 2, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 0, MAX_STAKING_TOKENS);
vm.assume(indexingRewardCut <= MAX_PPM);

uint256 legacyAllocationTokens = tokens / 2;
Expand Down
3 changes: 2 additions & 1 deletion packages/horizon/test/staking/delegation/addToPool.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { HorizonStakingTest } from "../HorizonStaking.t.sol";
contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest {

modifier useValidDelegationAmount(uint256 tokens) {
vm.assume(tokens > MIN_DELEGATION);
vm.assume(tokens <= MAX_STAKING_TOKENS);
_;
}
Expand All @@ -29,6 +28,8 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest {
uint256 delegationAmount,
uint256 addToPoolAmount
) public useIndexer useProvision(amount, 0, 0) useValidDelegationAmount(delegationAmount) useValidAddToPoolAmount(addToPoolAmount) {
delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS);

// Initialize delegation pool
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0);
Expand Down
22 changes: 3 additions & 19 deletions packages/horizon/test/staking/delegation/delegate.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,9 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
staking.delegate(users.indexer, subgraphDataServiceAddress, 0, 0);
}

function testDelegate_RevertWhen_BelowMinimum(
uint256 amount,
uint256 delegationAmount
) public useIndexer useProvision(amount, 0, 0) {
vm.startPrank(users.delegator);
delegationAmount = bound(delegationAmount, 1, MIN_DELEGATION - 1);
token.approve(address(staking), delegationAmount);
bytes memory expectedError = abi.encodeWithSignature(
"HorizonStakingInsufficientTokens(uint256,uint256)",
delegationAmount,
MIN_DELEGATION
);
vm.expectRevert(expectedError);
staking.delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0);
}

function testDelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer {
amount = bound(amount, 1 ether, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS);
_createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0);

resetPrank(users.delegator);
Expand All @@ -88,7 +72,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
uint256 tokens,
uint256 delegationTokens
) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

Expand All @@ -109,7 +93,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest {
uint256 tokens,
uint256 delegationTokens
) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);
DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false);
Expand Down
25 changes: 4 additions & 21 deletions packages/horizon/test/staking/delegation/undelegate.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
uint256 undelegateSteps
) public useIndexer useProvision(amount, 0, 0) {
undelegateSteps = bound(undelegateSteps, 1, 10);
delegationAmount = bound(delegationAmount, MIN_DELEGATION + 10 wei, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, 10 wei, MAX_STAKING_TOKENS);

resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0);
DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false);

// there is a min delegation amount of 1 ether after undelegating
uint256 undelegateAmount = (delegation.shares - 1 ether) / undelegateSteps;
uint256 undelegateAmount = delegation.shares / undelegateSteps;
for (uint i = 0; i < undelegateSteps; i++) {
_undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount);
}
Expand Down Expand Up @@ -85,25 +84,9 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
staking.undelegate(users.indexer, subgraphDataServiceAddress, overDelegationShares);
}

function testUndelegate_RevertWhen_UndelegateLeavesInsufficientTokens(
uint256 delegationAmount,
uint256 withdrawShares
) public useIndexer useProvision(10_000_000 ether, 0, 0) useDelegation(delegationAmount) {
resetPrank(users.delegator);
uint256 minShares = delegationAmount - MIN_DELEGATION + 1;
withdrawShares = bound(withdrawShares, minShares, delegationAmount - 1);
bytes memory expectedError = abi.encodeWithSignature(
"HorizonStakingInsufficientTokens(uint256,uint256)",
delegationAmount - withdrawShares,
MIN_DELEGATION
);
vm.expectRevert(expectedError);
staking.undelegate(users.indexer, subgraphDataServiceAddress, withdrawShares);
}

function testUndelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer {
amount = bound(amount, 1, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS);
_createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0);

resetPrank(users.delegator);
Expand All @@ -117,7 +100,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
uint256 tokens,
uint256 delegationTokens
) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);

Expand Down
14 changes: 3 additions & 11 deletions packages/horizon/test/staking/delegation/withdraw.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
vm.startPrank(users.delegator);
DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false);
shares = bound(shares, 1, delegation.shares);

if (shares != delegation.shares) {
DelegationPoolInternalTest memory pool = _getStorage_DelegationPoolInternal(users.indexer, subgraphDataServiceAddress, false);
uint256 tokens = (shares * (pool.tokens - pool.tokensThawing)) / pool.shares;
uint256 newTokensThawing = pool.tokensThawing + tokens;
uint256 remainingTokens = (delegation.shares * (pool.tokens - newTokensThawing)) / pool.shares;
vm.assume(remainingTokens >= MIN_DELEGATION);
}


_undelegate(users.indexer, subgraphDataServiceAddress, shares);
_;
}
Expand Down Expand Up @@ -137,7 +129,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
}

function testWithdrawDelegation_LegacySubgraphService(uint256 delegationAmount) public useIndexer {
delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS);
_createProvision(users.indexer, subgraphDataServiceLegacyAddress, 10_000_000 ether, 0, MAX_THAWING_PERIOD);

resetPrank(users.delegator);
Expand All @@ -157,7 +149,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
uint256 tokens,
uint256 delegationTokens
) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing() {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
resetPrank(users.delegator);
_delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0);
DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false);
Expand Down
8 changes: 4 additions & 4 deletions packages/horizon/test/staking/slash/slash.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
uint256 delegationTokens
) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) {
vm.assume(slashTokens > tokens);
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);
uint256 maxVerifierTokens = (tokens * MAX_MAX_VERIFIER_CUT) / MAX_PPM;
vm.assume(verifierCutAmount <= maxVerifierTokens);
Expand All @@ -72,8 +72,8 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
uint256 verifierCutAmount,
uint256 delegationTokens
) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) useDelegationSlashing() {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
slashTokens = bound(slashTokens, tokens + 1, tokens + delegationTokens);
delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS);
slashTokens = bound(slashTokens, tokens + 1, tokens + 1 + delegationTokens);
verifierCutAmount = bound(verifierCutAmount, 0, MAX_MAX_VERIFIER_CUT);
uint256 maxVerifierTokens = (tokens * MAX_MAX_VERIFIER_CUT) / MAX_PPM;
vm.assume(verifierCutAmount <= maxVerifierTokens);
Expand All @@ -90,7 +90,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest {
uint256 slashTokens,
uint256 delegationTokens
) public useIndexer useProvision(tokens, MAX_MAX_VERIFIER_CUT, 0) {
delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS);
delegationTokens = bound(delegationTokens, 0, MAX_STAKING_TOKENS);
vm.assume(slashTokens > tokens + delegationTokens);

vm.startPrank(subgraphDataServiceAddress);
Expand Down
1 change: 0 additions & 1 deletion packages/horizon/test/utils/Constants.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ abstract contract Constants {
uint256 internal constant MAX_THAW_REQUESTS = 100;
uint32 internal constant MAX_MAX_VERIFIER_CUT = 1000000; // 100% in parts per million
uint64 internal constant MAX_THAWING_PERIOD = 28 days;
uint256 internal constant MIN_DELEGATION = 1 ether;
uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300;
// Epoch manager
uint256 internal constant EPOCH_LENGTH = 1;
Expand Down

0 comments on commit 41ad6e1

Please sign in to comment.