From 5d05ca90d8bef34464549a56ceb1b0fc8e398e9f Mon Sep 17 00:00:00 2001 From: cargopete Date: Tue, 9 Jun 2026 14:14:52 +0300 Subject: [PATCH] feat(horizon): expose released-but-unwithdrawn delegation (slashable derived view) --- .../contracts/staking/HorizonStaking.sol | 195 +++++++++- .../contracts/staking/HorizonStakingBase.sol | 12 + .../HorizonStakingShared.t.sol | 30 +- .../unit/staking/delegation/release.t.sol | 347 ++++++++++++++++++ .../horizon/internal/IHorizonStakingBase.sol | 18 +- .../horizon/internal/IHorizonStakingMain.sol | 73 +++- .../horizon/internal/IHorizonStakingTypes.sol | 16 + 7 files changed, 652 insertions(+), 39 deletions(-) create mode 100644 packages/horizon/test/unit/staking/delegation/release.t.sol diff --git a/packages/horizon/contracts/staking/HorizonStaking.sol b/packages/horizon/contracts/staking/HorizonStaking.sol index bd6ccef70..8c90bcf40 100644 --- a/packages/horizon/contracts/staking/HorizonStaking.sol +++ b/packages/horizon/contracts/staking/HorizonStaking.sol @@ -305,6 +305,16 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { ); } + /// @inheritdoc IHorizonStakingMain + function releaseThawedDelegation( + address serviceProvider, + address verifier, + address delegator, + uint256 nThawRequests + ) external override notPaused returns (uint256) { + return _releaseThawedDelegation(serviceProvider, verifier, delegator, nThawRequests); + } + /// @inheritdoc IHorizonStakingMain function setDelegationFeeCut( address serviceProvider, @@ -419,11 +429,14 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { // If the slashing leaves the thawing shares with no thawing tokens, cancel pending thawings by: // - deleting all thawing shares + // - clearing released-but-not-withdrawn shares (a subset of the thawing shares) // - incrementing the nonce to invalidate pending thaw requests // Note that thawing shares are completely lost, delegators won't get back the corresponding - // delegation pool shares. + // delegation pool shares. Released shares (sharesWithdrawable) are part of the thawing shares and + // are lost too; the nonce bump invalidates each delegator's stale withdrawableThawingNonce. if (pool.sharesThawing != 0 && pool.tokensThawing == 0) { pool.sharesThawing = 0; + pool.sharesWithdrawable = 0; pool.thawingNonce++; } @@ -862,6 +875,7 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { uint256 _nThawRequests ) private { DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier); + DelegationInternal storage delegation = pool.delegators[msg.sender]; // An invalid delegation pool has shares but no tokens require( @@ -869,28 +883,43 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { HorizonStakingInvalidDelegationPoolState(_serviceProvider, _verifier) ); - uint256 tokensThawed = 0; - uint256 sharesThawing = pool.sharesThawing; - uint256 tokensThawing = pool.tokensThawing; + // Revert only when there is genuinely nothing to withdraw: no thaw requests to release and no + // previously-released shares on record. We intentionally check the raw `sharesWithdrawable` (not the + // nonce-validated amount) so that a delegator whose released shares were wiped out by a full slash gets + // the same no-op behaviour as the legacy fully-slashed path rather than a revert. This preserves the + // legacy revert-on-empty behaviour for delegators who never thawed. + require( + _getThawRequestList(ThawRequestType.Delegation, _serviceProvider, _verifier, msg.sender).count != 0 || + delegation.sharesWithdrawable != 0, + HorizonStakingNothingThawing() + ); - FulfillThawRequestsParams memory params = FulfillThawRequestsParams({ - requestType: ThawRequestType.Delegation, - serviceProvider: _serviceProvider, - verifier: _verifier, - owner: msg.sender, - tokensThawing: tokensThawing, - sharesThawing: sharesThawing, - nThawRequests: _nThawRequests, - thawingNonce: pool.thawingNonce - }); - (tokensThawed, tokensThawing, sharesThawing) = _fulfillThawRequests(params); + // Release any completed thaw requests into the withdrawable tally first. This covers the common case + // where the delegator calls withdrawDelegated directly without having called releaseThawedDelegation. + // It also re-syncs withdrawableThawingNonce, so re-read sharesWithdrawable afterwards. + _releaseThawedDelegation(_serviceProvider, _verifier, msg.sender, _nThawRequests); - // The next subtraction should never revert becase: pool.tokens >= pool.tokensThawing and pool.tokensThawing >= tokensThawed - // In the event the pool gets completely slashed tokensThawed will fulfil to 0. - pool.tokens = pool.tokens - tokensThawed; - pool.sharesThawing = sharesThawing; - pool.tokensThawing = tokensThawing; + // Redeem the caller's released shares against the (possibly slashed) thawing pool. Released shares are a + // subset of sharesThawing, so the conversion mirrors {_releaseThawRequest} and remains correct after any + // slash that occurred between release and withdrawal. + uint256 sharesWithdrawable = delegation.withdrawableThawingNonce == pool.thawingNonce + ? delegation.sharesWithdrawable + : 0; + uint256 tokensThawed = 0; + if (sharesWithdrawable != 0) { + // sharesThawing >= sharesWithdrawable > 0 here, so the division is safe. + tokensThawed = (sharesWithdrawable * pool.tokensThawing) / pool.sharesThawing; + + pool.tokensThawing = pool.tokensThawing - tokensThawed; + pool.sharesThawing = pool.sharesThawing - sharesWithdrawable; + pool.sharesWithdrawable = pool.sharesWithdrawable - sharesWithdrawable; + pool.tokens = pool.tokens - tokensThawed; + delegation.sharesWithdrawable = 0; + } + + // tokensThawed can be zero when no requests have matured yet or the pool was fully slashed. In that case + // this is a no-op, matching the legacy behaviour where fully-slashed thaw requests fulfilled to zero. if (tokensThawed != 0) { if (_newServiceProvider != address(0) && _newVerifier != address(0)) { _delegate(_newServiceProvider, _newVerifier, tokensThawed, _minSharesForNewProvider); @@ -901,6 +930,132 @@ contract HorizonStaking is HorizonStakingBase, IHorizonStakingMain { } } + /** + * @notice Mark `_delegator`'s completed delegation thaw requests as released (withdrawable) without + * transferring any tokens. + * @dev Traverses the thaw request linked list (via {LinkedList-traverse}) and processes every request whose + * `thawingUntil` has passed, up to `_nThawRequests` (or all if 0). For each valid-nonce request it records the + * request's thawing-pool shares against `pool.sharesWithdrawable` and `delegation.sharesWithdrawable`, then + * removes the request from the list. Crucially it does NOT decrement `pool.tokensThawing`/`pool.sharesThawing`: + * the released tokens stay in the thawing pool and remain slashable until {withdrawDelegated} is called. This + * is what keeps the bucket free of the slash-evasion and pool-brick problems — releasing changes only + * per-delegator bookkeeping, never the slashable principal. + * + * Stale released shares from a previous nonce epoch (invalidated by a full slash) are dropped before + * accumulating. Emits {ThawRequestFulfilled} per processed request and {DelegationThawReleased} in aggregate + * when anything was released. Never reverts when there is nothing to release — it is permissionless housekeeping. + * + * @param _serviceProvider The service provider address + * @param _verifier The verifier address + * @param _delegator The delegator whose thaw requests to release + * @param _nThawRequests Max requests to process. 0 = release all completed ones. + * @return tokensReleased Informational token-equivalent of the shares released (snapshot at current pool ratio) + */ + function _releaseThawedDelegation( + address _serviceProvider, + address _verifier, + address _delegator, + uint256 _nThawRequests + ) private returns (uint256 tokensReleased) { + DelegationPoolInternal storage pool = _getDelegationPool(_serviceProvider, _verifier); + DelegationInternal storage delegation = pool.delegators[_delegator]; + ILinkedList.List storage thawRequestList = _getThawRequestList( + ThawRequestType.Delegation, + _serviceProvider, + _verifier, + _delegator + ); + + if (thawRequestList.count == 0) { + return 0; + } + + // Drop stale released shares from a prior nonce epoch (a full slash invalidated them) before accumulating. + if (delegation.withdrawableThawingNonce != pool.thawingNonce) { + delegation.sharesWithdrawable = 0; + delegation.withdrawableThawingNonce = pool.thawingNonce; + } + + // Snapshot the thawing pool ratio for the traversal. tokensThawing/sharesThawing are read-only here - + // released tokens are intentionally left in the thawing pool so they stay slashable. + bytes memory acc = abi.encode( + uint256(0), + uint256(0), + pool.tokensThawing, + pool.sharesThawing, + pool.thawingNonce + ); + (uint256 requestsReleased, bytes memory data) = thawRequestList.traverse( + _getNextThawRequest(ThawRequestType.Delegation), + _releaseThawRequest, + _getDeleteThawRequest(ThawRequestType.Delegation), + acc, + _nThawRequests + ); + + uint256 sharesReleased; + (sharesReleased, tokensReleased, , , ) = abi.decode(data, (uint256, uint256, uint256, uint256, uint256)); + + if (sharesReleased == 0) { + return 0; + } + + pool.sharesWithdrawable += sharesReleased; + delegation.sharesWithdrawable += sharesReleased; + + emit DelegationThawReleased(_serviceProvider, _verifier, _delegator, requestsReleased, tokensReleased); + } + + /** + * @notice Traversal callback that releases a single expired delegation thaw request. + * @dev Used as the `processItem` callback in {_releaseThawedDelegation}. Unlike {_fulfillThawRequest} it does + * not mutate the thawing pool totals - it only accumulates the released shares (and an informational token + * snapshot) into the accumulator. The accumulator layout is + * `(sharesReleased, tokensReleased, tokensThawing, sharesThawing, thawingNonce)`. + * @param _thawRequestId The ID of the current thaw request + * @param _acc The accumulator data + * @return Whether to stop traversal (true once the first not-yet-expired request is reached) + * @return The updated accumulator data + */ + function _releaseThawRequest(bytes32 _thawRequestId, bytes memory _acc) private returns (bool, bytes memory) { + ( + uint256 sharesReleased, + uint256 tokensReleased, + uint256 tokensThawing, + uint256 sharesThawing, + uint256 thawingNonce + ) = abi.decode(_acc, (uint256, uint256, uint256, uint256, uint256)); + + ThawRequest storage thawRequest = _getThawRequest(ThawRequestType.Delegation, _thawRequestId); + + // Thaw requests are ordered by creation time; stop at the first one that has not yet expired. + if (thawRequest.thawingUntil > block.timestamp) { + return (true, LinkedList.NULL_BYTES); + } + + // Only requests on the current nonce carry value; stale ones are removed but contribute nothing. + uint256 tokens = 0; + bool validThawRequest = thawRequest.thawingNonce == thawingNonce; + if (validThawRequest) { + // sharesThawing cannot be zero while a valid thaw request exists, so the division is safe. + tokens = (thawRequest.shares * tokensThawing) / sharesThawing; + sharesReleased = sharesReleased + thawRequest.shares; + tokensReleased = tokensReleased + tokens; + } + + emit ThawRequestFulfilled( + ThawRequestType.Delegation, + _thawRequestId, + tokens, + thawRequest.shares, + thawRequest.thawingUntil, + validThawRequest + ); + + _acc = abi.encode(sharesReleased, tokensReleased, tokensThawing, sharesThawing, thawingNonce); + return (false, _acc); + } + /** * @notice Creates a thaw request. * Allows creating thaw requests up to a maximum of `MAX_THAW_REQUESTS` per owner. diff --git a/packages/horizon/contracts/staking/HorizonStakingBase.sol b/packages/horizon/contracts/staking/HorizonStakingBase.sol index 199e894d3..f7c4bfede 100644 --- a/packages/horizon/contracts/staking/HorizonStakingBase.sol +++ b/packages/horizon/contracts/staking/HorizonStakingBase.sol @@ -151,6 +151,18 @@ abstract contract HorizonStakingBase is return _getDelegatedTokensAvailable(serviceProvider, verifier); } + /// @inheritdoc IHorizonStakingBase + function getDelegatedTokensWithdrawable( + address serviceProvider, + address verifier + ) external view override returns (uint256) { + DelegationPoolInternal storage poolInternal = _getDelegationPool(serviceProvider, verifier); + if (poolInternal.sharesThawing == 0) { + return 0; + } + return (poolInternal.sharesWithdrawable * poolInternal.tokensThawing) / poolInternal.sharesThawing; + } + /// @inheritdoc IHorizonStakingBase function getThawRequest( ThawRequestType requestType, diff --git a/packages/horizon/test/unit/shared/horizon-staking/HorizonStakingShared.t.sol b/packages/horizon/test/unit/shared/horizon-staking/HorizonStakingShared.t.sol index c7e50a133..e290a8e0f 100644 --- a/packages/horizon/test/unit/shared/horizon-staking/HorizonStakingShared.t.sol +++ b/packages/horizon/test/unit/shared/horizon-staking/HorizonStakingShared.t.sol @@ -1121,15 +1121,18 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { beforeValues.pool.thawingNonce == thawRequest.thawingNonce ); } - vm.expectEmit(address(staking)); - emit IHorizonStakingMain.ThawRequestsFulfilled( - params.thawRequestType, - params.serviceProvider, - params.verifier, - msgSender, - calcValues.thawRequestsFulfilledList.length, - calcValues.tokensThawed - ); + // Releasing matured delegation thaw requests now emits DelegationThawReleased (in aggregate) rather than + // the legacy ThawRequestsFulfilled. It only fires when at least one valid-nonce request was released. + if (calcValues.sharesThawed != 0) { + vm.expectEmit(address(staking)); + emit IHorizonStakingMain.DelegationThawReleased( + params.serviceProvider, + params.verifier, + msgSender, + calcValues.thawRequestsFulfilledList.length, + calcValues.tokensThawed + ); + } if (calcValues.tokensThawed != 0) { vm.expectEmit(); if (reDelegate) { @@ -1594,6 +1597,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { uint256 sharesThawing; // Thawing nonce uint256 thawingNonce; + // Released-but-not-withdrawn thawing shares + uint256 sharesWithdrawable; } function _getStorageDelegationPoolInternal( @@ -1622,7 +1627,8 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { _gap_delegators_mapping: uint256(vm.load(address(staking), bytes32(baseSlot + 4))), tokensThawing: uint256(vm.load(address(staking), bytes32(baseSlot + 5))), sharesThawing: uint256(vm.load(address(staking), bytes32(baseSlot + 6))), - thawingNonce: uint256(vm.load(address(staking), bytes32(baseSlot + 7))) + thawingNonce: uint256(vm.load(address(staking), bytes32(baseSlot + 7))), + sharesWithdrawable: uint256(vm.load(address(staking), bytes32(baseSlot + 8))) }); return delegationPoolInternal; @@ -1653,7 +1659,9 @@ abstract contract HorizonStakingSharedTest is GraphBaseTest { DelegationInternal memory delegation = DelegationInternal({ shares: uint256(vm.load(address(staking), bytes32(baseSlot))), __DEPRECATED_tokensLocked: uint256(vm.load(address(staking), bytes32(baseSlot + 1))), - __DEPRECATED_tokensLockedUntil: uint256(vm.load(address(staking), bytes32(baseSlot + 2))) + __DEPRECATED_tokensLockedUntil: uint256(vm.load(address(staking), bytes32(baseSlot + 2))), + sharesWithdrawable: uint256(vm.load(address(staking), bytes32(baseSlot + 3))), + withdrawableThawingNonce: uint256(vm.load(address(staking), bytes32(baseSlot + 4))) }); return delegation; diff --git a/packages/horizon/test/unit/staking/delegation/release.t.sol b/packages/horizon/test/unit/staking/delegation/release.t.sol new file mode 100644 index 000000000..b6de0b92d --- /dev/null +++ b/packages/horizon/test/unit/staking/delegation/release.t.sol @@ -0,0 +1,347 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import { IHorizonStakingMain } from "@graphprotocol/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol"; +import { IHorizonStakingTypes } from "@graphprotocol/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol"; +import { ILinkedList } from "@graphprotocol/interfaces/contracts/horizon/internal/ILinkedList.sol"; + +import { HorizonStakingTest } from "../HorizonStaking.t.sol"; + +/** + * @title HorizonStakingReleaseThawedDelegationTest + * @notice Tests for {releaseThawedDelegation} and the released-but-not-withdrawn (withdrawable) accounting. + * @dev The core invariants under test: + * - releasing is permissionless and idempotent, and never reverts on "nothing to do"; + * - releasing only re-buckets per-delegator bookkeeping: it does NOT change the actively-earning base + * ({getDelegatedTokensAvailable}) nor remove tokens from the slashable thawing pool; + * - released tokens remain fully slashable (no slash-evasion, no pool-brick underflow); + * - a full slash invalidates released shares via the thawing nonce. + */ +contract HorizonStakingReleaseThawedDelegationTest is HorizonStakingTest { + /* + * HELPERS + */ + + function _undelegateAll() private returns (uint256 sharesThawing) { + DelegationInternal memory delegation = _getStorageDelegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + resetPrank(users.delegator); + _undelegate(users.indexer, subgraphDataServiceAddress, delegation.shares); + DelegationPoolInternalTest memory pool = _getStorageDelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + return pool.sharesThawing; + } + + /* + * TESTS + */ + + /// @notice Releasing does not change the actively-earning base; it only re-buckets bookkeeping. + function testRelease_DoesNotChangeActiveBase( + uint256 delegationAmount, + uint256 undelegateShares + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(undelegateShares) + { + skip(MAX_THAWING_PERIOD + 1); + + uint256 activeBefore = staking.getDelegatedTokensAvailable(users.indexer, subgraphDataServiceAddress); + + resetPrank(users.delegator); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + + uint256 activeAfter = staking.getDelegatedTokensAvailable(users.indexer, subgraphDataServiceAddress); + assertEq(activeAfter, activeBefore, "release must not change the actively-earning base"); + } + + /// @notice Anyone may release another delegator's matured thaw requests, and it surfaces as withdrawable. + function testRelease_Permissionless( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(type(uint256).max) + { + skip(MAX_THAWING_PERIOD + 1); + + DelegationPoolInternalTest memory poolBefore = _getStorageDelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + assertEq(poolBefore.sharesWithdrawable, 0); + assertEq(staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), 0); + + // A random third party (not the delegator) performs the housekeeping. + address keeper = makeAddr("keeper"); + resetPrank(keeper); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + + DelegationPoolInternalTest memory poolAfter = _getStorageDelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + DelegationInternal memory delegation = _getStorageDelegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + + assertEq(poolAfter.sharesWithdrawable, poolBefore.sharesThawing, "all matured shares released"); + assertEq(delegation.sharesWithdrawable, poolBefore.sharesThawing); + assertEq(delegation.withdrawableThawingNonce, poolAfter.thawingNonce); + // The thawing pool totals are untouched - tokens stay slashable. + assertEq(poolAfter.tokensThawing, poolBefore.tokensThawing); + assertEq(poolAfter.sharesThawing, poolBefore.sharesThawing); + // Withdrawable token-equivalent equals the full thawing balance (single delegator). + assertEq( + staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), + poolBefore.tokensThawing + ); + } + + /// @notice Releasing before the thaw period elapses is a no-op that does not revert. + function testRelease_NoOpWhenNotMatured( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(type(uint256).max) + { + resetPrank(users.delegator); + uint256 released = staking.releaseThawedDelegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + 0 + ); + assertEq(released, 0, "nothing matured -> zero released, no revert"); + assertEq(staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), 0); + } + + /// @notice Releasing with no thaw requests at all is a no-op that does not revert. + function testRelease_NoOpWhenNoRequests( + uint256 delegationAmount + ) public useIndexer useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { + resetPrank(users.delegator); + uint256 released = staking.releaseThawedDelegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + 0 + ); + assertEq(released, 0); + } + + /// @notice After an external release, the delegator can still withdraw even though the request list is empty. + function testRelease_ThenWithdrawByDelegator( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(type(uint256).max) + { + skip(MAX_THAWING_PERIOD + 1); + + // Third-party release empties the thaw request list. + address keeper = makeAddr("keeper"); + resetPrank(keeper); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + + ILinkedList.List memory list = staking.getThawRequestList( + IHorizonStakingTypes.ThawRequestType.Delegation, + users.indexer, + subgraphDataServiceAddress, + users.delegator + ); + assertEq(list.count, 0, "release consumed the thaw requests"); + + uint256 expected = staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress); + uint256 balanceBefore = token.balanceOf(users.delegator); + + // Withdraw must succeed off the pre-released shares (list is empty). + resetPrank(users.delegator); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); + + assertEq(token.balanceOf(users.delegator) - balanceBefore, expected, "withdraws the released balance"); + DelegationInternal memory delegation = _getStorageDelegation( + users.indexer, + subgraphDataServiceAddress, + users.delegator, + false + ); + assertEq(delegation.sharesWithdrawable, 0, "withdrawable cleared after withdraw"); + } + + /// @notice CRITICAL: released-but-not-withdrawn delegation remains fully slashable (no slash-evasion). + function testRelease_ReleasedTokensRemainSlashable() + public + useIndexer + useProvision(1000 ether, 0, MAX_THAWING_PERIOD) + useDelegationSlashing + { + uint256 delegationTokens = 1000 ether; + resetPrank(users.delegator); + _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + + // Undelegate everything so the whole delegation is thawing. + _undelegateAll(); + skip(MAX_THAWING_PERIOD + 1); + + // Release: the delegator "banks" the matured thaw into the withdrawable bucket. + resetPrank(users.delegator); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + assertEq( + staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), + delegationTokens, + "fully withdrawable after release" + ); + + // Now slash. Provision (1000) is consumed first, then 400 of delegation is burned. + uint256 delegationSlash = 400 ether; + resetPrank(subgraphDataServiceAddress); + _slash(users.indexer, subgraphDataServiceAddress, 1000 ether + delegationSlash, 0); + + // The released tokens were NOT exempt: the withdrawable balance scaled down with the slash. + uint256 expectedRemaining = delegationTokens - delegationSlash; // 600 ether + assertEq( + staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), + expectedRemaining, + "released bucket is slashed proportionally" + ); + + // Views never underflow / brick the pool. + assertEq(staking.getDelegatedTokensAvailable(users.indexer, subgraphDataServiceAddress), 0); + + // Withdrawing nets only the post-slash amount - the slash was borne, not evaded. + uint256 balanceBefore = token.balanceOf(users.delegator); + resetPrank(users.delegator); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); + assertEq( + token.balanceOf(users.delegator) - balanceBefore, + expectedRemaining, + "delegator withdraws the slashed amount, not the pre-slash amount" + ); + } + + /// @notice Releasing before a slash yields the same outcome as not releasing (releasing buys no protection). + function testRelease_NoSlashEvasionAdvantage() + public + useIndexer + useProvision(1000 ether, 0, MAX_THAWING_PERIOD) + useDelegationSlashing + { + uint256 delegationTokens = 1000 ether; + resetPrank(users.delegator); + _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + _undelegateAll(); + skip(MAX_THAWING_PERIOD + 1); + + // Path A: release THEN slash. + resetPrank(users.delegator); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + + uint256 delegationSlash = 250 ether; + resetPrank(subgraphDataServiceAddress); + _slash(users.indexer, subgraphDataServiceAddress, 1000 ether + delegationSlash, 0); + + uint256 balanceBefore = token.balanceOf(users.delegator); + resetPrank(users.delegator); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); + uint256 withdrawnAfterRelease = token.balanceOf(users.delegator) - balanceBefore; + + // The control (no release before slash) is exercised by the existing withdraw tests; here we assert the + // economically meaningful property directly: a released delegator still loses exactly the slashed share. + assertEq(withdrawnAfterRelease, delegationTokens - delegationSlash, "no protection gained by releasing early"); + } + + /// @notice A full slash bumps the thawing nonce and invalidates released shares; withdraw nets zero, no revert. + function testRelease_FullSlashInvalidatesReleasedShares() + public + useIndexer + useProvision(1000 ether, 0, MAX_THAWING_PERIOD) + useDelegationSlashing + { + uint256 delegationTokens = 1000 ether; + resetPrank(users.delegator); + _delegate(users.indexer, subgraphDataServiceAddress, delegationTokens, 0); + _undelegateAll(); + skip(MAX_THAWING_PERIOD + 1); + + resetPrank(users.delegator); + staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + + DelegationPoolInternalTest memory poolBeforeSlash = _getStorageDelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + + // Slash the entire provision + delegation. + resetPrank(subgraphDataServiceAddress); + _slash(users.indexer, subgraphDataServiceAddress, 1000 ether + delegationTokens, 0); + + DelegationPoolInternalTest memory poolAfterSlash = _getStorageDelegationPoolInternal( + users.indexer, + subgraphDataServiceAddress, + false + ); + assertEq(poolAfterSlash.tokensThawing, 0, "thawing tokens fully burned"); + assertEq(poolAfterSlash.sharesThawing, 0, "thawing shares reset"); + assertEq(poolAfterSlash.sharesWithdrawable, 0, "withdrawable shares reset"); + assertEq(poolAfterSlash.thawingNonce, poolBeforeSlash.thawingNonce + 1, "nonce bumped"); + + // Withdrawable view returns zero (no thawing shares left), no division-by-zero. + assertEq(staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), 0); + + // Withdraw is a clean no-op: stale shares are invalidated by the nonce, nothing transferred, no revert. + uint256 balanceBefore = token.balanceOf(users.delegator); + resetPrank(users.delegator); + staking.withdrawDelegated(users.indexer, subgraphDataServiceAddress, 0); + assertEq(token.balanceOf(users.delegator), balanceBefore, "fully slashed -> nothing to withdraw"); + } + + /// @notice The withdrawable view returns zero when the pool has no thawing shares. + function testGetDelegatedTokensWithdrawable_ZeroWhenNoThawing( + uint256 delegationAmount + ) public useIndexer useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) useDelegation(delegationAmount) { + assertEq(staking.getDelegatedTokensWithdrawable(users.indexer, subgraphDataServiceAddress), 0); + } + + /// @notice Releasing twice is idempotent - the second call finds nothing to release. + function testRelease_Idempotent( + uint256 delegationAmount + ) + public + useIndexer + useProvision(10_000_000 ether, 0, MAX_THAWING_PERIOD) + useDelegation(delegationAmount) + useUndelegate(type(uint256).max) + { + skip(MAX_THAWING_PERIOD + 1); + resetPrank(users.delegator); + uint256 first = staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + assertGt(first, 0); + uint256 second = staking.releaseThawedDelegation(users.indexer, subgraphDataServiceAddress, users.delegator, 0); + assertEq(second, 0, "second release is a no-op"); + } +} diff --git a/packages/interfaces/contracts/horizon/internal/IHorizonStakingBase.sol b/packages/interfaces/contracts/horizon/internal/IHorizonStakingBase.sol index 4bc81d44f..29c9d19ee 100644 --- a/packages/interfaces/contracts/horizon/internal/IHorizonStakingBase.sol +++ b/packages/interfaces/contracts/horizon/internal/IHorizonStakingBase.sol @@ -133,13 +133,29 @@ interface IHorizonStakingBase { /** * @notice Gets the delegator's tokens available in a provision. - * @dev Calculated as the tokens available minus the tokens thawing. + * @dev Calculated as the tokens available minus the tokens thawing. This already excludes both in-period and + * completed-but-not-yet-withdrawn delegation, since completed thaw requests remain in `tokensThawing` until + * withdrawn. It is therefore the correct actively-earning base for APR/APY denominators. * @param serviceProvider The address of the service provider. * @param verifier The address of the verifier. * @return The amount of tokens available. */ function getDelegatedTokensAvailable(address serviceProvider, address verifier) external view returns (uint256); + /** + * @notice Gets the token-equivalent of delegation that has completed thawing and been released but not yet + * withdrawn by delegators. + * @dev This is a derived view, computed as `sharesWithdrawable * tokensThawing / sharesThawing`. These tokens + * are a subset of `tokensThawing` (they remain slashable until withdrawn) and do not earn rewards. The value + * only reflects thaw requests that have actually been released via {releaseThawedDelegation}; it returns zero + * when the pool has no thawing shares. Use it to split `tokensThawing` into its in-period and + * completed-but-unwithdrawn components for reporting. + * @param serviceProvider The address of the service provider. + * @param verifier The address of the verifier. + * @return The token-equivalent of released, not-yet-withdrawn delegation. + */ + function getDelegatedTokensWithdrawable(address serviceProvider, address verifier) external view returns (uint256); + /** * @notice Gets a thaw request. * @param thawRequestType The type of thaw request. diff --git a/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol b/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol index 1c87fee1e..69f6358e1 100644 --- a/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol +++ b/packages/interfaces/contracts/horizon/internal/IHorizonStakingMain.sol @@ -211,6 +211,27 @@ interface IHorizonStakingMain { uint256 tokens ); + /** + * @notice Emitted when completed (fully-thawed) delegation thaw requests are released for a delegator via + * {releaseThawedDelegation} (or lazily inside {withdrawDelegated}). + * @dev Releasing only marks the requests' thawing-pool shares as withdrawable for the delegator. No tokens are + * moved or transferred: the underlying tokens stay in the pool's `tokensThawing`/`sharesThawing` accounting + * and remain slashable until {withdrawDelegated} is called. `tokens` is an informational snapshot of the + * token-equivalent at the current pool ratio and may change if the pool is slashed before withdrawal. + * @param serviceProvider The address of the service provider + * @param verifier The address of the verifier + * @param delegator The address of the delegator whose thaw requests were released + * @param thawRequestsReleased The number of thaw requests released + * @param tokens Informational token-equivalent of the released shares at the current pool ratio + */ + event DelegationThawReleased( + address indexed serviceProvider, + address indexed verifier, + address indexed delegator, + uint256 thawRequestsReleased, + uint256 tokens + ); + /** * @notice Emitted when `delegator` withdrew delegated `tokens` from `indexer` using `withdrawDelegated`. * @dev This event is for the legacy `withdrawDelegated` function, only emitted for pre-horizon undelegations. @@ -756,23 +777,61 @@ interface IHorizonStakingMain { /** * @notice Withdraw undelegated tokens from a provision after thawing. - * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw - * requests in the event that fulfilling all of them results in a gas limit error. Otherwise, the function - * will attempt to fulfill all thaw requests until the first one that is not yet expired is found. - * @dev If the delegation pool was completely slashed before withdrawing, calling this function will fulfill - * the thaw requests with an amount equal to zero. + * @dev The parameter `nThawRequests` can be set to a non zero value to release a specific number of thaw + * requests in the event that releasing all of them results in a gas limit error. Otherwise, the function + * will release all thaw requests until the first one that is not yet expired is found. + * @dev Internally this first performs the same release step as {releaseThawedDelegation} (so callers do not + * need to call it beforehand), then withdraws every share that has been released for the caller. + * @dev If no thaw request has matured yet this is a no-op and does not revert. If the delegation pool was + * completely slashed before withdrawing, the matured requests resolve to an amount equal to zero. * * Requirements: * - Must have previously initiated a thaw request using {undelegate}. * - * Emits {ThawRequestFulfilled}, {ThawRequestsFulfilled} and {DelegatedTokensWithdrawn} events. + * Emits {ThawRequestFulfilled} per released request, {DelegationThawReleased} when shares are released, and + * {DelegatedTokensWithdrawn} when tokens are transferred. * * @param serviceProvider The service provider address * @param verifier The verifier address - * @param nThawRequests The number of thaw requests to fulfill. Set to 0 to fulfill all thaw requests. + * @param nThawRequests The number of thaw requests to release. Set to 0 to release all matured thaw requests. */ function withdrawDelegated(address serviceProvider, address verifier, uint256 nThawRequests) external; + /** + * @notice Release completed delegation thaw requests, marking them withdrawable, without transferring tokens. + * @dev This is a permissionless state-update function — anyone may call it for any delegator. It traverses + * `delegator`'s thaw request list for `(serviceProvider, verifier)`, finds every request whose `thawingUntil` + * has passed (up to `nThawRequests`, or all if 0), removes each from the linked list, and credits the request's + * thawing-pool shares to `pool.sharesWithdrawable` and `delegation.sharesWithdrawable`. + * + * It deliberately does NOT change `pool.tokensThawing` / `pool.sharesThawing`: the released tokens remain in the + * thawing pool and stay slashable until withdrawn. This avoids exempting matured-but-unwithdrawn delegation from + * slashing. The token-equivalent of the released shares is exposed via + * {IHorizonStakingBase-getDelegatedTokensWithdrawable}. + * + * Calling this before {withdrawDelegated} is optional — {withdrawDelegated} performs the same release step + * internally. Its primary use-case is to let bots or dashboards keep pool state current so that + * {IHorizonStakingBase-getDelegatedTokensWithdrawable} reflects completed-but-not-yet-withdrawn delegation and + * the remainder of `tokensThawing` reflects only in-period thaw requests. + * + * Unlike {withdrawDelegated}, this function does not revert when there is nothing to release; it simply + * returns zero, so it is safe for bots to call speculatively. + * + * Emits {ThawRequestFulfilled} per released request and {DelegationThawReleased} in aggregate. + * + * @param serviceProvider The service provider address + * @param verifier The verifier address + * @param delegator The delegator whose completed thaw requests to release + * @param nThawRequests Max thaw requests to release. Set to 0 to release all completed ones. + * @return Informational token-equivalent of the released shares at the current pool ratio + */ + function releaseThawedDelegation( + address serviceProvider, + address verifier, + address delegator, + uint256 nThawRequests + ) external returns (uint256); + /** * @notice Re-delegate undelegated tokens from a provision after thawing to a `newServiceProvider` and `newVerifier`. * @dev The parameter `nThawRequests` can be set to a non zero value to fulfill a specific number of thaw diff --git a/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol b/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol index 22cdb5b4b..a2c75ae0e 100644 --- a/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol +++ b/packages/interfaces/contracts/horizon/internal/IHorizonStakingTypes.sol @@ -100,6 +100,12 @@ interface IHorizonStakingTypes { * @param tokensThawing Tokens thawing in the pool * @param sharesThawing Shares representing the thawing tokens * @param thawingNonce Value of the current thawing nonce. Thaw requests with older nonces are invalid. + * @param sharesWithdrawable Thawing-pool shares whose thaw period has expired and that have been released + * (via {releaseThawedDelegation} or lazily during {withdrawDelegated}) but not yet withdrawn. These shares + * remain part of `sharesThawing`/`tokensThawing` and therefore remain slashable until withdrawn. The + * token-equivalent is `sharesWithdrawable * tokensThawing / sharesThawing` and is exposed via + * {IHorizonStakingBase-getDelegatedTokensWithdrawable}. Appended after `thawingNonce` to preserve the + * existing storage layout. */ struct DelegationPoolInternal { uint32 __DEPRECATED_cooldownBlocks; @@ -112,6 +118,7 @@ interface IHorizonStakingTypes { uint256 tokensThawing; uint256 sharesThawing; uint256 thawingNonce; + uint256 sharesWithdrawable; } /** @@ -130,11 +137,20 @@ interface IHorizonStakingTypes { * @param shares Shares owned by the delegator in the pool * @param __DEPRECATED_tokensLocked Tokens locked for undelegation * @param __DEPRECATED_tokensLockedUntil Epoch when locked tokens can be withdrawn + * @param sharesWithdrawable Thawing-pool shares belonging to this delegator that have completed thawing and + * been released (via {releaseThawedDelegation} or lazily during {withdrawDelegated}) but not yet withdrawn. + * These shares remain in the pool's `sharesThawing`/`tokensThawing` accounting and stay slashable until + * withdrawn. Appended to preserve the existing storage layout. + * @param withdrawableThawingNonce The pool `thawingNonce` at which `sharesWithdrawable` were released. If it + * no longer matches the pool nonce, a full slash has since invalidated those shares and they are treated as + * zero. */ struct DelegationInternal { uint256 shares; uint256 __DEPRECATED_tokensLocked; uint256 __DEPRECATED_tokensLockedUntil; + uint256 sharesWithdrawable; + uint256 withdrawableThawingNonce; } /**