fix: tweaks to thawing pool management

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

@@ -254,8 +254,15 @@ interface IHorizonStakingMain {
      * @param tokens The amount of tokens being released
      * @param shares The amount of shares being released
      * @param thawingUntil The timestamp until the stake has thawed
+     * @param valid Whether the thaw request was valid at the time of fulfillment
      */
-    event ThawRequestFulfilled(bytes32 indexed thawRequestId, uint256 tokens, uint256 shares, uint64 thawingUntil);
+    event ThawRequestFulfilled(
+        bytes32 indexed thawRequestId,
+        uint256 tokens,
+        uint256 shares,
+        uint64 thawingUntil,
+        bool valid
+    );
 
     /**
      * @notice Emitted when a series of thaw requests are fulfilled.

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

@@ -32,6 +32,8 @@ interface IHorizonStakingTypes {
         uint32 maxVerifierCutPending;
         // Pending value for `thawingPeriod`. Verifier needs to accept it before it becomes active.
         uint64 thawingPeriodPending;
+        // Value of the current thawing nonce. Thaw requests with older nonces are invalid.
+        uint256 thawingNonce;
     }
 
     /**
@@ -75,6 +77,8 @@ interface IHorizonStakingTypes {
         uint256 tokensThawing;
         // Shares representing the thawing tokens
         uint256 sharesThawing;
+        // Value of the current thawing nonce. Thaw requests with older nonces are invalid.
+        uint256 thawingNonce;
     }
 
     /**
@@ -101,6 +105,8 @@ interface IHorizonStakingTypes {
         uint256 tokensThawing;
         // Shares representing the thawing tokens
         uint256 sharesThawing;
+        // Value of the current thawing nonce. Thaw requests with older nonces are invalid.
+        uint256 thawingNonce;
     }
 
     /**
@@ -137,5 +143,7 @@ interface IHorizonStakingTypes {
         uint64 thawingUntil;
         // Id of the next thaw request in the linked list
         bytes32 next;
+        // Used to invalidate unfulfilled thaw requests
+        uint256 thawingNonce;
     }
 }

packages/horizon/contracts/staking/HorizonStaking.sol

@@ -286,7 +286,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
 
         // Delegation pool must exist before adding tokens
         DelegationPoolInternal storage pool = _getDelegationPool(serviceProvider, verifier);
-        require(pool.shares > 0, HorizonStakingInvalidDelegationPool(serviceProvider, verifier));
+        require(
+            pool.shares > 0 || pool.sharesThawing > 0,
+            HorizonStakingInvalidDelegationPool(serviceProvider, verifier)
+        );
 
         pool.tokens = pool.tokens + tokens;
         _graphToken().pullTokens(msg.sender, tokens);
@@ -402,6 +405,12 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
                 (FIXED_POINT_PRECISION);
             prov.tokens = prov.tokens - providerTokensSlashed;
 
+            // Reset provision's thawing pool if provision was fully slashed
+            if (prov.tokens == 0 && prov.tokensThawing == 0) {
+                prov.sharesThawing = 0;
+                prov.thawingNonce++;
+            }
+
             // Service provider accounting
             _serviceProviders[serviceProvider].tokensProvisioned =
                 _serviceProviders[serviceProvider].tokensProvisioned -
@@ -429,6 +438,12 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
                     (pool.tokensThawing * (FIXED_POINT_PRECISION - delegationFractionSlashed)) /
                     FIXED_POINT_PRECISION;
 
+                // Reset delegation pool's thawing pool if pool was fully slashed
+                if (pool.tokens == 0 && pool.tokensThawing == 0) {
+                    pool.sharesThawing = 0;
+                    pool.thawingNonce++;
+                }
+
                 emit DelegationSlashed(serviceProvider, verifier, tokensToSlash);
             } else {
                 emit DelegationSlashingSkipped(serviceProvider, verifier, tokensToSlash);
@@ -638,7 +653,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             thawingPeriod: _thawingPeriod,
             createdAt: uint64(block.timestamp),
             maxVerifierCutPending: _maxVerifierCut,
-            thawingPeriodPending: _thawingPeriod
+            thawingPeriodPending: _thawingPeriod,
+            thawingNonce: 0
         });
 
         ServiceProviderInternal storage sp = _serviceProviders[_serviceProvider];
@@ -666,25 +682,40 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
 
     /**
      * @notice See {IHorizonStakingMain-thaw}.
+     * @dev We use a thawing pool to keep track of tokens thawing for multiple thaw requests.
+     * If due to slashing the thawing pool looses all of it's tokens, the pool is reset and all pending thaw
+     * requests are invalidated.
      */
     function _thaw(address _serviceProvider, address _verifier, uint256 _tokens) private returns (bytes32) {
         require(_tokens != 0, HorizonStakingInvalidZeroTokens());
         uint256 tokensAvailable = _getProviderTokensAvailable(_serviceProvider, _verifier);
         require(tokensAvailable >= _tokens, HorizonStakingInsufficientTokens(tokensAvailable, _tokens));
 
         Provision storage prov = _provisions[_serviceProvider][_verifier];
-        uint256 thawingShares = prov.sharesThawing == 0 ? _tokens : (prov.sharesThawing * _tokens) / prov.tokensThawing;
+
+        // Calculate shares to issue
+        // Thawing pool is reset/initialized in any of the following cases:
+        // - prov.tokensThawing == 0, pool is empty
+        bool initializeThawingPool = prov.tokensThawing == 0;
+        uint256 thawingShares = initializeThawingPool ? _tokens : ((prov.sharesThawing * _tokens) / prov.tokensThawing);
         uint64 thawingUntil = uint64(block.timestamp + uint256(prov.thawingPeriod));
 
-        prov.tokensThawing = prov.tokensThawing + _tokens;
+        // Safety check to ensure pool has no shares when being initialized
+        // This should never happen if the pool was reset due to slashing or it's the first initialization
+        if (initializeThawingPool) {
+            prov.sharesThawing = 0;
+        }
+
         prov.sharesThawing = prov.sharesThawing + thawingShares;
+        prov.tokensThawing = prov.tokensThawing + _tokens;
 
         bytes32 thawRequestId = _createThawRequest(
             _serviceProvider,
             _verifier,
             _serviceProvider,
             thawingShares,
-            thawingUntil
+            thawingUntil,
+            prov.thawingNonce
         );
         emit ProvisionThawed(_serviceProvider, _verifier, _tokens);
         return thawRequestId;
@@ -705,7 +736,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             _serviceProvider,
             tokensThawing,
             sharesThawing,
-            _nThawRequests
+            _nThawRequests,
+            prov.thawingNonce
         );
 
         prov.tokens = prov.tokens - tokensThawed;
@@ -732,15 +764,19 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);
         DelegationInternal storage delegation = pool.delegators[msg.sender];
 
+        // An invalid delegation pool has shares or thawing shares but no tokens
         require(
             pool.tokens != 0 || (pool.shares == 0 && pool.sharesThawing == 0),
             HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier)
         );
 
         // Calculate shares to issue
-        uint256 shares = (pool.tokens == 0 || pool.tokens == pool.tokensThawing)
-            ? _tokens
-            : ((_tokens * pool.shares) / (pool.tokens - pool.tokensThawing));
+        // Delegation pool is reset/initialized in any of the following cases:
+        // - pool.tokens == 0 and pool.shares == 0, pool is completely empty. Note that we don't test shares == 0 because
+        //   the invalid delegation pool check already ensures shares are 0 if tokens are 0
+        // - pool.tokens == pool.tokensThawing, the entire pool is thawing
+        bool initializePool = pool.tokens == 0 || pool.tokens == pool.tokensThawing;
+        uint256 shares = initializePool ? _tokens : ((_tokens * pool.shares) / (pool.tokens - pool.tokensThawing));
         require(shares != 0 && shares >= _minSharesOut, HorizonStakingSlippageProtection(shares, _minSharesOut));
 
         pool.tokens = pool.tokens + _tokens;
@@ -755,22 +791,41 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
      * @notice See {IHorizonStakingMain-undelegate}.
      * @dev To allow delegation to be slashable even while thawing without breaking accounting
      * the delegation pool shares are burned and replaced with thawing pool shares.
+     * @dev Note that due to slashing the delegation pool can enter an invalid state if all it's tokens are slashed.
+     * An invalid pool can only be recovered by adding back tokens into the pool with {IHorizonStakingMain-addToDelegationPool}.
+     * Any time the delegation pool is invalidated, the thawing pool is also reset and any pending undelegate requests get
+     * invalidated. Delegation that is caught thawing when the pool is invalidated will be completely lost.
      */
     function _undelegate(address _serviceProvider, address _verifier, uint256 _shares) private returns (bytes32) {
         require(_shares > 0, HorizonStakingInvalidZeroShares());
         DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);
         DelegationInternal storage delegation = pool.delegators[msg.sender];
         require(delegation.shares >= _shares, HorizonStakingInsufficientShares(delegation.shares, _shares));
+
+        // An invalid delegation pool has shares or thawing shares but no tokens
         require(pool.tokens != 0, HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier));
 
+        // Calculate thawing shares to issue - convert delegation pool shares to thawing pool shares
+        // delegation pool shares -> delegation pool tokens -> thawing pool shares
+        // Thawing pool is reset/initialized in any of the following cases:
+        // - prov.tokensThawing == 0, pool is empty
         uint256 tokens = (_shares * (pool.tokens - pool.tokensThawing)) / pool.shares;
-        uint256 thawingShares = pool.tokensThawing == 0 ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing);
+        bool initializeThawingPool = pool.tokensThawing == 0;
+        uint256 thawingShares = initializeThawingPool ? tokens : ((tokens * pool.sharesThawing) / pool.tokensThawing);
         uint64 thawingUntil = uint64(block.timestamp + uint256(_provisions[_serviceProvider][_verifier].thawingPeriod));
-        pool.shares = pool.shares - _shares;
+
+        // Safety check to ensure thawing pool has no shares when being initialized
+        // This should never happen if the pool was reset due to slashing or it's the first initialization
+        if (initializeThawingPool) {
+            pool.sharesThawing = 0;
+        }
+
         pool.tokensThawing = pool.tokensThawing + tokens;
         pool.sharesThawing = pool.sharesThawing + thawingShares;
 
+        pool.shares = pool.shares - _shares;
         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;
@@ -785,7 +840,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             _verifier,
             msg.sender,
             thawingShares,
-            thawingUntil
+            thawingUntil,
+            pool.thawingNonce
         );
 
         emit TokensUndelegated(_serviceProvider, _verifier, msg.sender, tokens);
@@ -803,6 +859,10 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         uint256 _nThawRequests
     ) private {
         DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier);
+
+        // An invalid delegation pool has shares or thawing shares but no tokens
+        // Note we don't test for shares, the existence of thaw requests means thawing shares also exist
+        // even if they are fulfilled to 0 tokens
         require(pool.tokens != 0, HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier));
 
         uint256 tokensThawed = 0;
@@ -814,7 +874,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             msg.sender,
             tokensThawing,
             sharesThawing,
-            _nThawRequests
+            _nThawRequests,
+            pool.thawingNonce
         );
 
         pool.tokens = pool.tokens - tokensThawed;
@@ -843,20 +904,27 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
      * @param _owner The address of the owner of the thaw request
      * @param _shares The number of shares to thaw
      * @param _thawingUntil The timestamp until which the shares are thawing
+     * @param _thawingNonce Owner's validity nonce for the thaw request
      * @return The ID of the thaw request
      */
     function _createThawRequest(
         address _serviceProvider,
         address _verifier,
         address _owner,
         uint256 _shares,
-        uint64 _thawingUntil
+        uint64 _thawingUntil,
+        uint256 _thawingNonce
     ) private returns (bytes32) {
         LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner];
         require(thawRequestList.count < MAX_THAW_REQUESTS, HorizonStakingTooManyThawRequests());
 
         bytes32 thawRequestId = keccak256(abi.encodePacked(_serviceProvider, _verifier, _owner, thawRequestList.nonce));
-        _thawRequests[thawRequestId] = ThawRequest({ shares: _shares, thawingUntil: _thawingUntil, next: bytes32(0) });
+        _thawRequests[thawRequestId] = ThawRequest({
+            shares: _shares,
+            thawingUntil: _thawingUntil,
+            next: bytes32(0),
+            thawingNonce: _thawingNonce
+        });
 
         if (thawRequestList.count != 0) _thawRequests[thawRequestList.tail].next = thawRequestId;
         thawRequestList.addTail(thawRequestId);
@@ -874,6 +942,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
      * @param _tokensThawing The current amount of tokens already thawing
      * @param _sharesThawing The current amount of shares already thawing
      * @param _nThawRequests The number of thaw requests to fulfill. If set to 0, all thaw requests are fulfilled.
+     * @param _thawingNonce The current valid thawing nonce. Any thaw request with a different nonce is invalid and should be ignored.
      * @return The amount of thawed tokens
      * @return The amount of tokens still thawing
      * @return The amount of shares still thawing
@@ -884,7 +953,8 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         address _owner,
         uint256 _tokensThawing,
         uint256 _sharesThawing,
-        uint256 _nThawRequests
+        uint256 _nThawRequests,
+        uint256 _thawingNonce
     ) private returns (uint256, uint256, uint256) {
         LinkedList.List storage thawRequestList = _thawRequestLists[_serviceProvider][_verifier][_owner];
         require(thawRequestList.count > 0, HorizonStakingNothingThawing());
@@ -894,7 +964,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
             _getNextThawRequest,
             _fulfillThawRequest,
             _deleteThawRequest,
-            abi.encode(tokensThawed, _tokensThawing, _sharesThawing),
+            abi.encode(tokensThawed, _tokensThawing, _sharesThawing, _thawingNonce),
             _nThawRequests
         );
 
@@ -923,20 +993,30 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain {
         }
 
         // decode
-        (uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing) = abi.decode(
+        (uint256 tokensThawed, uint256 tokensThawing, uint256 sharesThawing, uint256 thawingNonce) = abi.decode(
             _acc,
-            (uint256, uint256, uint256)
+            (uint256, uint256, uint256, uint256)
         );
 
-        // process
-        uint256 tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
-        tokensThawing = tokensThawing - tokens;
-        sharesThawing = sharesThawing - thawRequest.shares;
-        tokensThawed = tokensThawed + tokens;
-        emit ThawRequestFulfilled(_thawRequestId, tokens, thawRequest.shares, thawRequest.thawingUntil);
+        // process - only fulfill thaw requests for the current valid nonce
+        uint256 tokens = 0;
+        bool invalidThawRequest = thawRequest.thawingNonce != thawingNonce;
+        if (!invalidThawRequest) {
+            tokens = (thawRequest.shares * tokensThawing) / sharesThawing;
+            tokensThawing = tokensThawing - tokens;
+            sharesThawing = sharesThawing - thawRequest.shares;
+            tokensThawed = tokensThawed + tokens;
+        }
+        emit ThawRequestFulfilled(
+            _thawRequestId,
+            tokens,
+            thawRequest.shares,
+            thawRequest.thawingUntil,
+            !invalidThawRequest
+        );
 
         // encode
-        _acc = abi.encode(tokensThawed, tokensThawing, sharesThawing);
+        _acc = abi.encode(tokensThawed, tokensThawing, sharesThawing, thawingNonce);
         return (false, _acc);
     }
 

packages/horizon/contracts/staking/HorizonStakingBase.sol

@@ -96,6 +96,7 @@ abstract contract HorizonStakingBase is
         pool.shares = poolInternal.shares;
         pool.tokensThawing = poolInternal.tokensThawing;
         pool.sharesThawing = poolInternal.sharesThawing;
+        pool.thawingNonce = poolInternal.thawingNonce;
         return pool;
     }