From 91cda5619ee96954cfaa7526c24499521e66ee3f Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Wed, 4 Dec 2024 15:15:40 -0300 Subject: [PATCH] fix: added minimum delegation for provisions (TRST-M03) --- .../contracts/staking/HorizonStaking.sol | 11 +++++ packages/horizon/test/escrow/collect.t.sol | 2 +- .../horizon/test/payments/GraphPayments.t.sol | 2 +- .../horizon/test/staking/HorizonStaking.t.sol | 28 ++++++++++++- .../test/staking/delegation/addToPool.t.sol | 5 ++- .../test/staking/delegation/delegate.t.sol | 24 +++++++++-- .../test/staking/delegation/redelegate.t.sol | 13 ------ .../test/staking/delegation/undelegate.t.sol | 41 ++++++++++++++++--- .../test/staking/delegation/withdraw.t.sol | 21 +--------- .../horizon/test/staking/slash/slash.t.sol | 6 +-- packages/horizon/test/utils/Constants.sol | 1 + 11 files changed, 105 insertions(+), 49 deletions(-) diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index 659d0f089..5d7961574 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -41,6 +41,9 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { /// @dev Address of the staking extension contract address private immutable STAKING_EXTENSION_ADDRESS; + /// @dev Minimum amount of delegation. + uint256 private constant MIN_DELEGATION = 1e18; + /** * @notice Checks that the caller is authorized to operate over a provision. * @param serviceProvider The address of the service provider. @@ -842,6 +845,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { * have been done before calling this function. */ function _delegate(address _serviceProvider, address _verifier, uint256 _tokens, uint256 _minSharesOut) private { + require(_tokens >= MIN_DELEGATION, HorizonStakingInsufficientTokens(_tokens, MIN_DELEGATION)); require( _provisions[_serviceProvider][_verifier].createdAt != 0, HorizonStakingInvalidProvision(_serviceProvider, _verifier) @@ -911,6 +915,13 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { pool.shares = pool.shares - _shares; delegation.shares = delegation.shares - _shares; + 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( _requestType, diff --git a/packages/horizon/test/escrow/collect.t.sol b/packages/horizon/test/escrow/collect.t.sol index 72b795ee9..e47c04ec5 100644 --- a/packages/horizon/test/escrow/collect.t.sol +++ b/packages/horizon/test/escrow/collect.t.sol @@ -101,7 +101,7 @@ contract GraphEscrowCollectTest is GraphEscrowTest { uint256 tokensDelegatoion = tokens * delegationFeeCut / MAX_PPM; vm.assume(tokensDataService < tokens - tokensProtocol - tokensDelegatoion); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/payments/GraphPayments.t.sol b/packages/horizon/test/payments/GraphPayments.t.sol index 19028bde1..47740b4cd 100644 --- a/packages/horizon/test/payments/GraphPayments.t.sol +++ b/packages/horizon/test/payments/GraphPayments.t.sol @@ -119,7 +119,7 @@ contract GraphPaymentsTest is HorizonStakingSharedTest { address escrowAddress = address(escrow); // Delegate tokens - tokensDelegate = bound(tokensDelegate, 1, MAX_STAKING_TOKENS); + tokensDelegate = bound(tokensDelegate, MIN_DELEGATION, MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, tokensDelegate, 0); diff --git a/packages/horizon/test/staking/HorizonStaking.t.sol b/packages/horizon/test/staking/HorizonStaking.t.sol index b1b45d118..d57b1c1b8 100644 --- a/packages/horizon/test/staking/HorizonStaking.t.sol +++ b/packages/horizon/test/staking/HorizonStaking.t.sol @@ -31,7 +31,7 @@ contract HorizonStakingTest is HorizonStakingSharedTest { modifier useDelegation(uint256 delegationAmount) { address msgSender; (, msgSender, ) = vm.readCallers(); - vm.assume(delegationAmount > 1); + vm.assume(delegationAmount >= MIN_DELEGATION); vm.assume(delegationAmount <= MAX_STAKING_TOKENS); vm.startPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -56,4 +56,30 @@ contract HorizonStakingTest is HorizonStakingSharedTest { resetPrank(msgSender); _; } + + modifier useUndelegate(uint256 shares) { + resetPrank(users.delegator); + + DelegationPoolInternalTest memory pool = _getStorage_DelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + + shares = bound(shares, 1, delegation.shares); + uint256 tokens = (shares * (pool.tokens - pool.tokensThawing)) / pool.shares; + if (shares < delegation.shares) { + uint256 remainingTokens = (shares * (pool.tokens - pool.tokensThawing - tokens)) / pool.shares; + vm.assume(remainingTokens >= MIN_DELEGATION); + } + + _undelegate(users.indexer, subgraphDataServiceAddress, shares); + _; + } } diff --git a/packages/horizon/test/staking/delegation/addToPool.t.sol b/packages/horizon/test/staking/delegation/addToPool.t.sol index ff5b957ca..2bd73f28f 100644 --- a/packages/horizon/test/staking/delegation/addToPool.t.sol +++ b/packages/horizon/test/staking/delegation/addToPool.t.sol @@ -10,6 +10,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { modifier useValidDelegationAmount(uint256 tokens) { vm.assume(tokens <= MAX_STAKING_TOKENS); + vm.assume(tokens >= MIN_DELEGATION); _; } @@ -93,7 +94,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); @@ -116,7 +117,7 @@ contract HorizonStakingDelegationAddToPoolTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/delegate.t.sol b/packages/horizon/test/staking/delegation/delegate.t.sol index ab58e4bde..5fc6681df 100644 --- a/packages/horizon/test/staking/delegation/delegate.t.sol +++ b/packages/horizon/test/staking/delegation/delegate.t.sol @@ -59,9 +59,25 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { staking.delegate(users.indexer, subgraphDataServiceAddress, 0, 0); } + function testDelegate_RevertWhen_UnderMinDelegation( + uint256 amount, + uint256 delegationAmount + ) public useIndexer useProvision(amount, 0, 0) { + delegationAmount = bound(delegationAmount, 1, MIN_DELEGATION - 1); + vm.startPrank(users.delegator); + token.approve(address(staking), delegationAmount); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + 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, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -72,7 +88,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -96,7 +112,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -126,7 +142,7 @@ contract HorizonStakingDelegateTest is HorizonStakingTest { uint256 recoverAmount ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { recoverAmount = bound(recoverAmount, 1, MAX_STAKING_TOKENS); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // create delegation pool resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/redelegate.t.sol b/packages/horizon/test/staking/delegation/redelegate.t.sol index 605e6601f..6e30348c4 100644 --- a/packages/horizon/test/staking/delegation/redelegate.t.sol +++ b/packages/horizon/test/staking/delegation/redelegate.t.sol @@ -9,19 +9,6 @@ import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } - /* * HELPERS */ diff --git a/packages/horizon/test/staking/delegation/undelegate.t.sol b/packages/horizon/test/staking/delegation/undelegate.t.sol index c824832c4..a2d41c061 100644 --- a/packages/horizon/test/staking/delegation/undelegate.t.sol +++ b/packages/horizon/test/staking/delegation/undelegate.t.sol @@ -32,7 +32,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 undelegateSteps ) public useIndexer useProvision(amount, 0, 0) { undelegateSteps = bound(undelegateSteps, 1, 10); - delegationAmount = bound(delegationAmount, 10 wei, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION * undelegateSteps, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationAmount, 0); @@ -44,9 +44,17 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { ); uint256 undelegateAmount = delegation.shares / undelegateSteps; - for (uint i = 0; i < undelegateSteps; i++) { + for (uint i = 0; i < undelegateSteps - 1; i++) { _undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); } + + delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); } function testUndelegate_WithBeneficiary( @@ -60,6 +68,29 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary); } + function testUndelegate_RevertWhen_InsuficientTokens( + uint256 amount, + uint256 delegationAmount, + uint256 undelegateAmount + ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) { + undelegateAmount = bound(undelegateAmount, 1, delegationAmount); + resetPrank(users.delegator); + DelegationInternal memory delegation = _getStorage_Delegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + undelegateAmount = bound(undelegateAmount, delegation.shares - MIN_DELEGATION + 1, delegation.shares - 1); + bytes memory expectedError = abi.encodeWithSelector( + IHorizonStakingMain.HorizonStakingInsufficientTokens.selector, + delegation.shares - undelegateAmount, + MIN_DELEGATION + ); + vm.expectRevert(expectedError); + staking.undelegate(users.indexer, subgraphDataServiceAddress, undelegateAmount); + } + function testUndelegate_RevertWhen_TooManyUndelegations() public useIndexer @@ -112,7 +143,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { function testUndelegate_LegacySubgraphService(uint256 amount, uint256 delegationAmount) public useIndexer { amount = bound(amount, 1, MAX_STAKING_TOKENS); - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, amount, 0, 0); resetPrank(users.delegator); @@ -131,7 +162,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); @@ -162,7 +193,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); // delegate resetPrank(users.delegator); diff --git a/packages/horizon/test/staking/delegation/withdraw.t.sol b/packages/horizon/test/staking/delegation/withdraw.t.sol index ff3fb3db0..c8ab59122 100644 --- a/packages/horizon/test/staking/delegation/withdraw.t.sol +++ b/packages/horizon/test/staking/delegation/withdraw.t.sol @@ -10,23 +10,6 @@ import { LinkedList } from "../../../contracts/libraries/LinkedList.sol"; import { HorizonStakingTest } from "../HorizonStaking.t.sol"; contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { - /* - * MODIFIERS - */ - - modifier useUndelegate(uint256 shares) { - resetPrank(users.delegator); - DelegationInternal memory delegation = _getStorage_Delegation( - users.indexer, - subgraphDataServiceAddress, - users.delegator, - false - ); - shares = bound(shares, 1, delegation.shares); - - _undelegate(users.indexer, subgraphDataServiceAddress, shares); - _; - } /* * TESTS @@ -81,7 +64,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { } function testWithdrawDelegation_LegacySubgraphService(uint256 delegationAmount) public useIndexer { - delegationAmount = bound(delegationAmount, 1, MAX_STAKING_TOKENS); + delegationAmount = bound(delegationAmount, MIN_DELEGATION, MAX_STAKING_TOKENS); _createProvision(users.indexer, subgraphDataServiceLegacyAddress, 10_000_000 ether, 0, MAX_THAWING_PERIOD); resetPrank(users.delegator); @@ -111,7 +94,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, 0, MAX_THAWING_PERIOD) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 2, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION * 2, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/staking/slash/slash.t.sol b/packages/horizon/test/staking/slash/slash.t.sol index a74403ed7..fc5676b0f 100644 --- a/packages/horizon/test/staking/slash/slash.t.sol +++ b/packages/horizon/test/staking/slash/slash.t.sol @@ -54,7 +54,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) { vm.assume(slashTokens > tokens); - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); verifierCutAmount = bound(verifierCutAmount, 0, MAX_PPM); vm.assume(verifierCutAmount <= tokens); @@ -71,7 +71,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 verifierCutAmount, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing() { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); slashTokens = bound(slashTokens, tokens + 1, tokens + 1 + delegationTokens); verifierCutAmount = bound(verifierCutAmount, 0, tokens); @@ -110,7 +110,7 @@ contract HorizonStakingSlashTest is HorizonStakingTest { uint256 tokens, uint256 delegationTokens ) public useIndexer useProvision(tokens, MAX_PPM, 0) useDelegationSlashing { - delegationTokens = bound(delegationTokens, 1, MAX_STAKING_TOKENS); + delegationTokens = bound(delegationTokens, MIN_DELEGATION, MAX_STAKING_TOKENS); resetPrank(users.delegator); _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); diff --git a/packages/horizon/test/utils/Constants.sol b/packages/horizon/test/utils/Constants.sol index cd5cc2bfb..9af54087d 100644 --- a/packages/horizon/test/utils/Constants.sol +++ b/packages/horizon/test/utils/Constants.sol @@ -14,6 +14,7 @@ abstract contract Constants { uint256 internal constant MAX_THAW_REQUESTS = 100; uint64 internal constant MAX_THAWING_PERIOD = 28 days; uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300; + uint256 internal constant MIN_DELEGATION = 1e18; // Epoch manager uint256 internal constant EPOCH_LENGTH = 1; // Rewards manager