feat(horizon): expose released-but-unwithdrawn delegation (slashable, derived view)

[cargopete]

feat(horizon): expose released-but-unwithdrawn delegation (slashable, derived-view)

Summary

Reworks the original "withdrawable bucket" PR to expose the split between in-period and
completed-but-unwithdrawn delegation without changing the actively-earning base, the
slashing semantics, or the public struct ABI.

Context / correction

The original PR assumed getDelegatedTokensAvailable = tokens - tokensThawing failed to
exclude completed-but-unwithdrawn delegation. It does not: completed thaw requests remain
in tokensThawing until withdrawDelegated fulfils them, so the active base is already
correct. This PR therefore does not change that formula. The only new capability is a
permissionless way to surface, and a view to read, how much of tokensThawing has
completed thawing.

Design

releaseThawedDelegation no longer moves tokens. It records, per delegator, how many
thawing-pool shares have matured (sharesWithdrawable), leaving the tokens in the
slashable thawing pool. withdrawDelegated redeems those shares against the live
(post-slash) thawing-pool ratio.

• Released delegation stays slashable → no slash-evasion, no getDelegatedTokensAvailable
  underflow / pool-brick.
• A full slash resets sharesWithdrawable and bumps the existing thawingNonce, which
  invalidates each delegator's stale released shares (withdrawableThawingNonce).
• getDelegatedTokensWithdrawable() is a derived view:
  sharesWithdrawable * tokensThawing / sharesThawing.

Changes

IHorizonStakingTypes.sol — append-only:

• DelegationPoolInternal.sharesWithdrawable (after thawingNonce)
• DelegationInternal.sharesWithdrawable, DelegationInternal.withdrawableThawingNonce
• public DelegationPool struct: unchanged (no ABI shift)

IHorizonStakingMain.sol

• DelegationThawReleased event; releaseThawedDelegation(...) (permissionless, never
  reverts on nothing-to-do); updated withdrawDelegated NatSpec.

IHorizonStakingBase.sol

• getDelegatedTokensWithdrawable getter; getDelegatedTokensAvailable NatSpec clarified
  (formula unchanged).

HorizonStaking.sol

• releaseThawedDelegation external wrapper + _releaseThawedDelegation (uses
  LinkedList.traverse) + _releaseThawRequest callback (emits per-request
  ThawRequestFulfilled).
• _withdrawDelegated redeems released shares; lazy-releases first for backward compat;
  no-op (not revert) when nothing matured or fully slashed.
• slash(): one line — reset sharesWithdrawable on full thaw-pool drain.
• _undelegate: reverted to original tokens - tokensThawing active base.

HorizonStakingBase.sol — derived withdrawable getter; reverted active-base formula.

Tests

New test/unit/staking/delegation/release.t.sol (10 tests): permissionless release,
idempotency, no-op-when-unmatured / no-requests, release-then-withdraw, active-base
invariance, released-tokens-remain-slashable, no-slash-evasion-advantage,
full-slash-invalidates-released-shares, withdrawable-view-zero-when-no-thawing.

All 350 horizon unit tests pass. solhint (--max-warnings=0) and prettier clean.

Backward compatibility

• withdrawDelegated works unchanged for delegators (lazy release internally).
• Public DelegationPool ABI unchanged; new fields are appended internal storage only.
• Event surface: per-request ThawRequestFulfilled still fires (now on the release step);
  the delegation-withdraw aggregate is DelegationThawReleased rather than
  ThawRequestsFulfilled. Network-subgraph handlers should index DelegationThawReleased.

Still required before merge

• audit label (maintainer) to clear the Require-Audit-Label check.
• Rebase onto current main.

[openzeppelin-code]

feat(horizon): add delegation withdrawable bucket (tokensWithdrawable)

Generated at commit: 5d05ca90d8bef34464549a56ceb1b0fc8e398e9f

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	3 5 0 14 38 60
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

---

For more details view the full report in OpenZeppelin Code Inspector

[tmigone]

Hi, thanks for the contribution.

Please see my comments in https://forum.thegraph.com/t/separate-withdrawable-bucket-for-fully-thawed-delegation-tokens/6882/4 about requiring a stronger justification for a change like this to go through, and other requirements for contributions to the repository.

For completeness sake, I did a first quick pass with Claude review tool on the proposed changes and several important issues were identified (all validated by myself):

  🔴 Critical

  1. Storage layout breaking change — DelegationPoolInternal (IHorizonStakingTypes.sol:110-122)

  tokensWithdrawable is inserted before thawingNonce:

  struct DelegationPoolInternal {
      ...
      uint256 sharesThawing;
      uint256 tokensWithdrawable;   // <-- inserted here
      uint256 thawingNonce;
  }

  This struct is stored in _legacyDelegationPools and _delegationPools mappings (HorizonStakingStorage.sol:105, 180). For
  struct-in-mapping storage, inserting a field in the middle shifts every subsequent slot. After upgrade:
  - existing pools' thawingNonce reads from a fresh (zero) slot → all current nonces effectively reset to 0, retroactively
  re-validating thaw requests that prior slashes had invalidated
  - existing pools' tokensWithdrawable reads from the old thawingNonce slot → non-zero garbage gets subtracted from the active
  base in every view and _undelegate call

  Repo convention (see Provision at line 31 and the prior DelegationPoolInternal shape) is to keep thawingNonce last so new
  fields append cleanly. tokensWithdrawable (and any future field) must go after thawingNonce. The public DelegationPool struct
  has the same ordering issue — its ABI position shifts, breaking off-chain consumers that decode by struct layout.

  2. Slashing does not adjust tokensWithdrawable (HorizonStaking.sol:489-506)

  The delegation slash path only scales pool.tokensThawing and decrements pool.tokens. pool.tokensWithdrawable is untouched. Two
   concrete problems:

  - Pool brick via underflow. _getDelegatedTokensAvailable returns pool.tokens - pool.tokensThawing - pool.tokensWithdrawable.
  Example: pool.tokens = 100 (active 20 / thawing 30 / withdrawable 50), slash of 60 → new pool.tokens = 40, scaled
  pool.tokensThawing = 12, pool.tokensWithdrawable = 50. Result: 40 - 12 - 50 = -22, reverting every view and undelegate. Pool
  is bricked.
  - Slash-evasion vector. releaseThawedDelegation is permissionless, so a delegator who anticipates a slash can call it to move
  their tokens from tokensThawing (proportionally slashable) into tokensWithdrawable (untouched). Pre-PR, those tokens stayed in
   tokensThawing and remained slashable. This is a real economic-security change the PR doesn't acknowledge.

  No cheap fix exists — scaling tokensWithdrawable proportionally also requires updating every delegator's
  tokensReleasedPendingWithdrawal, which isn't iterable on-chain. The design likely needs reconsideration: keep released tokens
  slashable, or cap slash at active + thawing and document the exemption.

  🟡 Major

  3. No tests. PR body lists a 9-item testing checklist, none ticked, no test files in the diff.
  4. _undelegate MIN_DELEGATION check (HorizonStaking.sol:946) still uses pool.tokens - pool.tokensThawing; should also subtract
   pool.tokensWithdrawable to match the new active-base definition on line 934.

  🟢 Minor

  5. withdrawDelegated now reverts with HorizonStakingNothingThawing when all requests are unexpired; previously no-op'd.
  Affects the redelegate path too.
  6. _releaseThawedDelegation reimplements LinkedList traversal instead of reusing LinkedList.removeHead / _fulfillThawRequest —
   two divergent paths to maintain.
  7. Per-request ThawRequestFulfilled events are not emitted on the release path; only the aggregate DelegationThawReleased
  fires. Indexers keyed off the per-request event will miss activity.
  8. thawRequestId == thawRequestList.head check on line 1090 is always true — can be unconditional.
  9. Division by sharesThawing on line 1078 is safe today by invariant but undocumented; add an assert or comment tying it to
  the nonce-match guard.
  10. Interface NatSpec on withdrawDelegated doesn't mention the new revert-on-nothing-releasable behavior.

[cargopete]

Closing this in favour of the off-chain approach, per the discussion on the forum and @tmigone's feedback.

The conclusion we reached: the actively-earning delegation base is already derivable on-chain (getDelegatedTokensAvailable() = tokens - tokensThawing, which already excludes completed-but-unwithdrawn delegation), so the APR/APY distortion is a consumer-side issue rather than a protocol one. Edge & Node have patched Graph Explorer accordingly, and the reporting nicety lives in the network subgraph instead (graphprotocol/graph-network-subgraph#331).

This PR served its purpose as a proof that a hardened, slashable, append-only version was buildable — but "buildable" isn't "worth it" given the added storage, accounting and slashing surface for a value that's ultimately consumed off-chain. Thanks @tmigone for the thorough review.