fix: add minimum tokens amount for undelegate with beneficiary (TRST-H03)

Author: Maikol

packages/horizon/contracts/interfaces/internal/IHorizonStakingMain.sol

@@ -433,6 +433,13 @@ interface IHorizonStakingMain {
      */
     error HorizonStakingInsufficientDelegationTokens(uint256 tokens, uint256 minTokens);
 
+    /**
+     * @notice Thrown when the minimum token amount required for delegation with beneficiary is not met.
+     * @param tokens The actual token amount
+     * @param minTokens The minimum required token amount
+     */
+    error HorizonStakingInsufficientUndelegationTokens(uint256 tokens, uint256 minTokens);
+
     /**
      * @notice Thrown when attempting to undelegate with a beneficiary that is the zero address.
      */

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -36,14 +36,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
     uint256 private constant FIXED_POINT_PRECISION = 1e18;
 
     /// @dev Maximum number of simultaneous stake thaw requests (per provision) or undelegations (per delegation)
-    uint256 private constant MAX_THAW_REQUESTS = 100;
+    uint256 private constant MAX_THAW_REQUESTS = 1_000;
 
     /// @dev Address of the staking extension contract
     address private immutable STAKING_EXTENSION_ADDRESS;
 
     /// @dev Minimum amount of delegation.
     uint256 private constant MIN_DELEGATION = 1e18;
 
+    /// @dev Minimum amount of undelegation with beneficiary.
+    uint256 private constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18;
+
     /**
      * @notice Checks that the caller is authorized to operate over a provision.
      * @param serviceProvider The address of the service provider.
@@ -931,6 +934,17 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         // delegation pool shares -> delegation pool tokens -> thawing pool shares
         // Thawing pool is reset/initialized when the pool is empty: prov.tokensThawing == 0
         uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares;
+
+        // Since anyone can undelegate for any beneficiary, we require a minimum amount to prevent
+        // malicious actors from flooding the thaw request list with tiny amounts and causing a
+        // denial of service attack by hitting the MAX_THAW_REQUESTS limit
+        if (_requestType == ThawRequestType.DelegationWithBeneficiary) {
+            require(
+                tokens >= MIN_UNDELEGATION_WITH_BENEFICIARY,
+                HorizonStakingInsufficientUndelegationTokens(tokens, MIN_UNDELEGATION_WITH_BENEFICIARY)
+            );
+        }
+
         // Thawing shares are rounded down to protect the pool and avoid taking extra tokens from other participants.
         uint256 thawingShares = pool.tokensThawing == 0 ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing);
         uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod));

packages/horizon/test/staking/delegation/undelegate.t.sol

@@ -63,6 +63,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
         address beneficiary
     ) public useIndexer useProvision(amount, 0, 0) useDelegation(delegationAmount) {
         vm.assume(beneficiary != address(0));
+        vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY);
         resetPrank(users.delegator);
         DelegationInternal memory delegation = _getStorage_Delegation(users.indexer, subgraphDataServiceAddress, users.delegator, false);
         _undelegateWithBeneficiary(users.indexer, subgraphDataServiceAddress, delegation.shares, beneficiary);
@@ -95,7 +96,7 @@ contract HorizonStakingUndelegateTest is HorizonStakingTest {
         public
         useIndexer
         useProvision(1000 ether, 0, 0)
-        useDelegation(1000 ether)
+        useDelegation(10000 ether)
     {
         resetPrank(users.delegator);
 

packages/horizon/test/staking/delegation/withdraw.t.sol

@@ -167,6 +167,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
      {
         vm.assume(beneficiary != address(0));
         vm.assume(beneficiary != address(staking));
+        vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY);
         // Skip beneficiary if balance will overflow
         vm.assume(token.balanceOf(beneficiary) < type(uint256).max - delegationAmount);
 
@@ -196,6 +197,7 @@ contract HorizonStakingWithdrawDelegationTest is HorizonStakingTest {
     {
         vm.assume(beneficiary != address(0));
         vm.assume(beneficiary != users.delegator);
+        vm.assume(delegationAmount >= MIN_UNDELEGATION_WITH_BENEFICIARY);
 
         // Delegator undelegates to beneficiary
         resetPrank(users.delegator);

packages/horizon/test/staking/provision/deprovision.t.sol

@@ -17,7 +17,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest {
         uint256 thawCount,
         uint256 deprovisionCount
     ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) {
-        thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS);
+        thawCount = bound(thawCount, 1, 100);
         deprovisionCount = bound(deprovisionCount, 0, thawCount);
         vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step
         uint256 individualThawAmount = amount / thawCount;
@@ -37,7 +37,7 @@ contract HorizonStakingDeprovisionTest is HorizonStakingTest {
         uint64 thawingPeriod,
         uint256 thawCount
     ) public useIndexer useProvision(amount, maxVerifierCut, thawingPeriod) {
-        thawCount = bound(thawCount, 2, MAX_THAW_REQUESTS);
+        thawCount = bound(thawCount, 2, 100);
         vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step
         uint256 individualThawAmount = amount / thawCount;
 

packages/horizon/test/staking/provision/thaw.t.sol

@@ -26,7 +26,7 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         uint64 thawingPeriod,
         uint256 thawCount
     ) public useIndexer useProvision(amount, 0, thawingPeriod) {
-        thawCount = bound(thawCount, 1, MAX_THAW_REQUESTS);
+        thawCount = bound(thawCount, 1, 100);
         vm.assume(amount >= thawCount); // ensure the provision has at least 1 token for each thaw step
         uint256 individualThawAmount = amount / thawCount;
 
@@ -72,13 +72,8 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         staking.thaw(users.indexer, subgraphDataServiceAddress, thawAmount);
     }
 
-    function testThaw_RevertWhen_OverMaxThawRequests(
-        uint256 amount,
-        uint64 thawingPeriod,
-        uint256 thawAmount
-    ) public useIndexer useProvision(amount, 0, thawingPeriod) {
-        vm.assume(amount >= MAX_THAW_REQUESTS + 1);
-        thawAmount = bound(thawAmount, 1, amount / (MAX_THAW_REQUESTS + 1));
+    function testThaw_RevertWhen_OverMaxThawRequests() public useIndexer useProvision(10000 ether, 0, 0) {
+        uint256 thawAmount = 1 ether;
 
         for (uint256 i = 0; i < MAX_THAW_REQUESTS; i++) {
             _thaw(users.indexer, subgraphDataServiceAddress, thawAmount);

packages/horizon/test/utils/Constants.sol

@@ -11,10 +11,11 @@ abstract contract Constants {
     // GraphPayments parameters
     uint256 internal constant protocolPaymentCut = 10000;
     // Staking constants
-    uint256 internal constant MAX_THAW_REQUESTS = 100;
+    uint256 internal constant MAX_THAW_REQUESTS = 1_000;
     uint64 internal constant MAX_THAWING_PERIOD = 28 days;
     uint32 internal constant THAWING_PERIOD_IN_BLOCKS = 300;
     uint256 internal constant MIN_DELEGATION = 1e18;
+    uint256 internal constant MIN_UNDELEGATION_WITH_BENEFICIARY = 10e18;
     // Epoch manager
     uint256 internal constant EPOCH_LENGTH = 1;
     // Rewards manager