Separate "Withdrawable" Bucket for Fully-Thawed Delegation Tokens

Governance & GIPs
1

Currently on Horizon, when a delegator undelegate and completes the thaw period, their tokens remain counted in delegatedTokens on the indexer until the delegator explicitly calls withdraw. This creates two problems:

  1. APY distortion — Dashboards using raw delegatedTokens understate actual returns, since idle fully-thawed tokens inflate the capital base without earning rewards.
  2. Delegation capacity distortion — Indexer profiles show inflated “Delegation Received” figures, which misrepresents true active delegation and available capacity.

Current Behavior

Thawing + fully-thawed tokens stay in delegatedTokens until the delegator calls withdraw. There is no way to avoid this in the current contract — it’s baked into how Horizon staking works, since the tokens need to live somewhere until gas is paid to move them. Not likely but delegators sometimes forget about withdrawal so this sits there for elongated periods of time.

Proposed Fix

Add a separate withdrawable bucket in a future contract upgrade. Once the thaw period completes, tokens automatically move out of delegatedTokens and into this withdrawable state. This means:

  • No dashboard needs to manually subtract thawing/thawed tokens
  • APY calculations reflect actual earning capital
  • Indexer delegation metrics are accurate
  • Delegators could receive reminders/alerts when tokens are sitting idle and ready to withdraw

Impact

This is a relatively low-frequency issue (delegators who forget to withdraw), but it has a persistent effect on protocol transparency and data accuracy for anyone relying on on-chain delegation metrics.

Below is example of 11.5M GRT that is fully thawed out but yet counted in our indexer pool of delegation.

image
image629×165 29.4 KB

image
image1280×571 69.4 KB

1 Like
2

can someone fix this please? its a really annoying issue since delegators see skewed APR and subgraph ratio on explorers..

1 Like
3

just fyi theres the same problem even if they arent thawned yet, but the inexer reallocated, which means theyre also not available for the indexer to allocate those thawning funds again ut the data shows like it is available (lower apr, lower subgraph ratios etc)

4

I proposed a fix 2 weeks ago but there’s been only radio silence. typical for e&n

5

Hi all, thanks for raising this issue and prompting the discussion. The request in this post is legitimate and deserves a response, I’m sorry we couldn’t get to it before, we’ll try to be more responsive going forward.

About the proposed fix feat(horizon): add delegation withdrawable bucket (tokensWithdrawable) by cargopete · Pull Request #1340 · graphprotocol/contracts · GitHub , we did notice the PR when it landed in the repository. However we considered it a work in progress rather than something ready for review. For protocol changes, especially in a system that secures millions of dollars in user funds, we expect a minimum level of completeness before beginning the review process. That includes passing CI and, at a minimum basic unit test coverage. With modern AI tools, it’s easier than ever to produce code quickly. That does not reduce the engineering effort required to review, validate, test, and audit protocol changes. In fact, this contributor/reviewer effort asymmetry makes it even more important that contributions arrive with a clear specification, passing CI, and adequate test coverage rather than expecting reviewers to fill in the gaps.

Nonetheless, I’ve taken a look and there are a number of issues with the contract changes being proposed, including two critical protocol breaking ones. These were all identified by Claude’s review tool (and subsequently validated manually), which further illustrates how readily detectable these issues were. I’ll provide more detailed feedback directly on the PR, but I believe there is a more fundamental problem to address.

In my opinion this is primarily an off-chain problem and therefore should be solved off-chain. The requested values should already be derivable from events and by reading the thaw request list and excluding expired entries. This is precisely the type of derived metric that subgraphs were designed to compute, which suggests the root issue may lie in the network subgraph rather than the protocol itself. Fixing this at the protocol layer introduces additional storage, new entry points, more complex accounting, and an expanded attack surface, all to expose information that is ultimately consumed off-chain. If the current events are insufficient to derive the desired data, then emitting additional information may be worth discussing. However, modifying protocol accounting and introducing new state solely to support a derived metric would require a much stronger justification than has been presented so far.

I’ll spend some time this week looking at the issue more holistically and report back, it’s likely a network subgraph or client side patch make more sense here.

3 Likes
6

thanks @tmigone , on it, will make that pr ready for merge

7

Indeed I see the merit in both the protocol-breaking issues and the “should this be on-chain at all?” question are legitimate, and I’ve taken both on board. Two things to share: a correction that I think reframes the whole problem, and a hardened version of the PR for completeness.

1. The actively-earning base is already correct on-chain

While reworking this I went back and checked the assumption the original PR was built on, and it turns out to be wrong in a way that matters.

Completed-but-not-yet-withdrawn delegation never leaves pool.tokensThawing. Nothing decrements tokensThawing when the 28-day thaw clock merely expires — only withdrawDelegated fulfilling the request does. So:

getDelegatedTokensAvailable() = pool.tokens - pool.tokensThawing

already excludes both in-period and completed-but-unwithdrawn delegation. It is the correct APR/APY denominator today, with no protocol change. The distortion described in the original post comes from consumers dividing rewards by pool.tokens (the raw reserve) instead of by getDelegatedTokensAvailable().

So you’re right — this is fundamentally an off-chain / consumer issue. The only genuinely new information a stored bucket would add is the split of tokensThawing into “still in its window” vs “thaw complete, awaiting withdrawal”, and that split is fully derivable from the existing ThawRequestCreated / ThawRequestFulfilled events (they already carry shares, thawingUntil and nonce). No new storage or events are strictly required.

My recommendation is to fix this off-chain — most cheaply by having dashboards divide by getDelegatedTokensAvailable(), and optionally surfacing the in-period/completed split in the network subgraph for nicer reporting. I’ve written up the derivation (a standalone reference plus a network-subgraph mapping sketch) and am happy to open a subgraph PR if that’s the preferred route.

2. Hardened PR, in case a protocol-level split is still wanted

If there’s appetite for exposing the split on-chain anyway, I’ve reworked the PR to be correct and complete. The redesign is materially smaller and resolves every issue raised:

  • Storage layout (critical): new fields are strictly appended after thawingNonce (and after the deprecated Delegation fields). No mid-struct insertion, no nonce reset, and the public DelegationPool struct is left untouched — the withdrawable amount is exposed via a derived view rather than a stored field.
  • Slashing (critical): the redesign never moves tokens out of the thawing pool. “Releasing” only records, per delegator, how many thawing-pool shares are withdrawable; the underlying tokens stay in tokensThawing/sharesThawing and remain fully slashable until withdrawn. Consequences:
    • No getDelegatedTokensAvailable underflow / pool-brick — the formula is unchanged.
    • No slash-evasion — moving a request from “thawing” to “released” is purely bookkeeping; slash() scales the same principal either way. There’s a test that delegates → releases → slashes and asserts the released delegator bears the full slash.
    • A full slash bumps the existing thawingNonce, which invalidates released shares (one extra line in slash()).
  • getDelegatedTokensAvailable / _undelegate: reverted to the original tokens - tokensThawing (already correct), so the line-934/946 inconsistency is gone.
  • Reuse: release now uses LinkedList.traverse with a dedicated callback rather than hand-rolled pointer surgery; per-request ThawRequestFulfilled events are emitted on the release path so indexers keyed on them don’t miss activity.
  • Tests + CI: full Foundry coverage added (permissionless release, idempotency, no-op-when-unmatured, the slashability / no-evasion / full-slash cases, the withdrawable view). All unit tests pass; solhint (--max-warnings=0) and prettier are clean; the stale DelegationInternal test helper that broke the build is fixed. Rebased onto current main.

The PR still needs the audit label from a maintainer to clear that CI check.

Either way, happy to go whichever direction the team prefers — the off-chain route is my recommendation, and the hardened PR is there if a protocol-level split is judged worth the (now much smaller) surface.

8

I’ve taken a deeper look at the network subgraph and graph explorer code and the issues that @PaulieB described initially:

  1. APY distortion — Dashboards using raw delegatedTokens understate actual returns, since idle fully-thawed tokens inflate the capital base without earning rewards.
  2. Delegation capacity distortion — Indexer profiles show inflated “Delegation Received” figures, which misrepresents true active delegation and available capacity.

The root cause for this happening in Graph Explorer is what @AxiomaticAardvark has explained above (thanks for checking that):

The distortion described in the original post comes from consumers dividing rewards by pool.tokens (the raw reserve) instead of by getDelegatedTokensAvailable().

I’ve submitted a patch to fix this in Graph Explorer, the repository is not public but the fix is exactly as described above. I’ll report back once we get it deployed.

@AxiomaticAardvark as for the contract changes, even with a reduced scope I don’t think it’s worth having this on chain for the added complexity and risk.

optionally surfacing the in-period/completed split in the network subgraph for nicer reporting.

This however seems like a nice addition!

9

Agreed on the contract side: a protocol-level bucket isn’t worth the added storage, accounting and slashing surface for a value that’s already derivable off-chain. I’ve closed #1340 on that basis — it was worth proving a hardened, slashable, append-only version was possible, but “possible” isn’t “worth it” here, and off-chain is plainly the right call.

To put that off-chain conclusion into practice, two PRs:

  • Network subgraph — #331: exposes the actively-earning base as a first-class field, Provision.delegatedTokensActive = delegatedTokens − delegatedThawingTokens. It’s the same quantity your Explorer patch computes, surfaced once so any other consumer gets it for free instead of re-deriving it. Cheap, no new indexing cost.
  • Docs — graphprotocol/docs#1109: a short note in the Delegating guide steering integrators to divide rewards by the actively-earning base rather than the raw delegated total, so the next person doesn’t hit the same footgun.

On the in-period vs completed split you flagged as a nice addition — one clarification so I build the right thing:

  • It’s already fully queryable from the existing ThawRequest entities (thawingUntil, fulfilled, type); a consumer can classify each request against the current block time. So nothing extra is strictly required.
  • If you’d specifically like the pool-level completed/in-period amounts as stored fields (a single read rather than a per-request aggregation), that needs a periodic block handler to re-evaluate thawingUntil against block.timestamp — which carries an indexing-cost tradeoff. Happy to add it, but I’d rather you make that call than assume it.

My default, unless you’d prefer the stored split, is to keep #331 to the lightweight delegatedTokensActive field plus a schema doc-comment pointing at ThawRequest. Just say which you’d like and I’ll shape it accordingly.