fix: ensure invalid thaw requests are ignored by getThawedTokens (OZ L-01)

Signed-off-by: Tomás Migone <[email protected]>

Author: tmigone

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

@@ -254,6 +254,7 @@ interface IHorizonStakingMain {
      * @param shares The amount of shares being thawed
      * @param thawingUntil The timestamp until the stake is thawed
      * @param thawRequestId The ID of the thaw request
+     * @param nonce The nonce of the thaw request
      */
     event ThawRequestCreated(
         IHorizonStakingTypes.ThawRequestType indexed requestType,
@@ -262,7 +263,8 @@ interface IHorizonStakingMain {
         address owner,
         uint256 shares,
         uint64 thawingUntil,
-        bytes32 thawRequestId
+        bytes32 thawRequestId,
+        uint256 nonce
     );
 
     /**

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -1065,7 +1065,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             _owner,
             _shares,
             _thawingUntil,
-            thawRequestId
+            thawRequestId,
+            _thawingNonce
         );
         return thawRequestId;
     }
@@ -1173,6 +1174,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         uint256 tokens = 0;
         bool validThawRequest = thawRequest.thawingNonce == thawingNonce;
         if (validThawRequest) {
+            // sharesThawing cannot be zero if there is a valid thaw request so the next division is safe
             tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
             tokensThawing = tokensThawing - tokens;
             sharesThawing = sharesThawing - thawRequest.shares;

packages/horizon/contracts/staking/HorizonStakingBase.sol

@@ -182,14 +182,18 @@ abstract contract HorizonStakingBase is
         bytes32 thawRequestId = thawRequestList.head;
         while (thawRequestId != bytes32(0)) {
             ThawRequest storage thawRequest = _getThawRequest(requestType, thawRequestId);
-            if (thawRequest.thawingUntil <= block.timestamp) {
-                uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
-                tokensThawing = tokensThawing - tokens;
-                sharesThawing = sharesThawing - thawRequest.shares;
-                thawedTokens = thawedTokens + tokens;
-            } else {
-                break;
+            if (thawRequest.thawingNonce == prov.thawingNonce) {
+                if (thawRequest.thawingUntil <= block.timestamp) {
+                    // sharesThawing cannot be zero if there is a valid thaw request so the next division is safe
+                    uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
+                    tokensThawing = tokensThawing - tokens;
+                    sharesThawing = sharesThawing - thawRequest.shares;
+                    thawedTokens = thawedTokens + tokens;
+                } else {
+                    break;
+                }
             }
+
             thawRequestId = thawRequest.next;
         }
         return thawedTokens;

packages/horizon/test/shared/horizon-staking/HorizonStakingShared.t.sol

@@ -428,7 +428,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
             serviceProvider,
             thawingShares,
             uint64(block.timestamp + beforeProvision.thawingPeriod),
-            expectedThawRequestId
+            expectedThawRequestId,
+            beforeProvision.thawingNonce
         );
         vm.expectEmit(address(staking));
         emit IHorizonStakingMain.ProvisionThawed(serviceProvider, verifier, tokens);
@@ -1019,7 +1020,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest {
             beneficiary,
             calcValues.thawingShares,
             calcValues.thawingUntil,
-            calcValues.thawRequestId
+            calcValues.thawRequestId,
+            beforeValues.pool.thawingNonce
         );
         vm.expectEmit();
         emit IHorizonStakingMain.TokensUndelegated(serviceProvider, verifier, delegator, calcValues.tokens);

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

@@ -162,4 +162,76 @@ contract HorizonStakingThawTest is HorizonStakingTest {
         );
         vm.assertEq(thawedTokens, thawAmount * thawSteps);
     }
+
+    function testThaw_GetThawedTokens_AfterProvisionFullySlashed(
+        uint256 amount,
+        uint64 thawingPeriod,
+        uint256 thawAmount
+    ) public useIndexer useProvision(amount, 0, thawingPeriod) {
+        // thaw some funds so there are some shares thawing and tokens thawing
+        thawAmount = bound(thawAmount, 1, amount);
+        _thaw(users.indexer, subgraphDataServiceAddress, thawAmount);
+
+        // skip to after the thawing period has passed
+        skip(thawingPeriod + 1);
+
+        // slash all of it
+        resetPrank(subgraphDataServiceAddress);
+        _slash(users.indexer, subgraphDataServiceAddress, amount, 0);
+
+        // get the thawed tokens - should be zero now as the pool was reset
+        uint256 thawedTokens = staking.getThawedTokens(
+            ThawRequestType.Provision,
+            users.indexer,
+            subgraphDataServiceAddress,
+            users.indexer
+        );
+        vm.assertEq(thawedTokens, 0);
+    }
+
+    function testThaw_GetThawedTokens_AfterRecoveringProvision(
+        uint256 amount,
+        uint64 thawingPeriod,
+        uint256 thawAmount
+    ) public useIndexer useProvision(amount, 0, thawingPeriod) {
+        // thaw some funds so there are some shares thawing and tokens thawing
+        thawAmount = bound(thawAmount, 1, amount);
+        _thaw(users.indexer, subgraphDataServiceAddress, thawAmount);
+
+        // skip to after the thawing period has passed
+        skip(thawingPeriod + 1);
+
+        // slash all of it
+        resetPrank(subgraphDataServiceAddress);
+        _slash(users.indexer, subgraphDataServiceAddress, amount, 0);
+
+        // get the thawed tokens - should be zero now as the pool was reset
+        uint256 thawedTokens = staking.getThawedTokens(
+            ThawRequestType.Provision,
+            users.indexer,
+            subgraphDataServiceAddress,
+            users.indexer
+        );
+        vm.assertEq(thawedTokens, 0);
+
+        // put some funds back in
+        resetPrank(users.indexer);
+        _stake(amount);
+        _addToProvision(users.indexer, subgraphDataServiceAddress, amount);
+
+        // thaw some more funds
+        _thaw(users.indexer, subgraphDataServiceAddress, thawAmount);
+
+        // skip to after the thawing period has passed
+        skip(block.timestamp + thawingPeriod + 1);
+
+        // get the thawed tokens - should be the amount we thawed
+        thawedTokens = staking.getThawedTokens(
+            ThawRequestType.Provision,
+            users.indexer,
+            subgraphDataServiceAddress,
+            users.indexer
+        );
+        vm.assertEq(thawedTokens, thawAmount);
+    }
 }