Skip to content

feat(horizon): expose Provision.delegatedTokensActive as the APR/APY denominator#331

Open
cargopete wants to merge 2 commits into
graphprotocol:masterfrom
cargopete:feat/provision-delegated-tokens-active
Open

feat(horizon): expose Provision.delegatedTokensActive as the APR/APY denominator#331
cargopete wants to merge 2 commits into
graphprotocol:masterfrom
cargopete:feat/provision-delegated-tokens-active

Conversation

@cargopete

Copy link
Copy Markdown
Contributor

feat(horizon): expose Provision.delegatedTokensActive as the APR/APY denominator

Summary

Adds an explicit, maintained delegatedTokensActive field to the Provision entity:

delegatedTokensActive = delegatedTokens - delegatedThawingTokens

This is the correct denominator for delegation APR/APY. It already exists implicitly
(the subgraph computes the same subtraction for delegationExchangeRate and indexer
capacity), but consumers computing returns tend to divide by delegatedTokens, which
includes both in-period thawing and completed-but-not-yet-withdrawn delegation —
systematically depressing reported APR/APY.

Why this is the right layer

Thawed-but-unwithdrawn delegation remains counted in delegatedThawingTokens (mirroring
the on-chain tokensThawing, which is only decremented when withdrawDelegated fulfils a
request — not when the thaw clock merely expires). So subtracting delegatedThawingTokens
already excludes it. No protocol change is required; this is purely a reporting fix, which
is the conclusion reached in the forum discussion
(thread).

For consumers who additionally want the in-period vs completed split of thawing
delegation, the existing ThawRequest entity (thawingUntil, fulfilled, type) is
already sufficient to compute it client-side.

Changes

  • schema.graphql — new Provision.delegatedTokensActive: BigInt! with documentation.
  • helpers.ts
    • createOrLoadProvision: initialise the field.
    • updateAdvancedProvisionMetrics: recompute it (covers delegate, add-to-pool,
      undelegate and slash paths, which already call this helper).
  • horizonStaking.tshandleDelegatedTokensWithdrawn: set the field directly (this
    handler doesn't go through updateAdvancedProvisionMetrics).

Validation

  • yarn prepare:arbitrum (codegen) ✅ — schema parses, types regenerate.
  • yarn build (AssemblyScript compile) ✅.

Note: yarn test currently fails to compile on master due to a pre-existing,
unrelated issue in tests/helpers.test.ts (calls createOrLoadIndexer with 2 args; it
takes 3). Not touched by this PR.

Notes / scope

  • Field added to Provision (the Horizon delegation unit). The legacy Indexer-level
    equivalent is derivable the same way (Indexer.delegatedTokens - Indexer.delegatedThawingTokens) and is already used internally for capacity; happy to
    add an Indexer.delegatedTokensActive mirror if wanted.
  • Subgraphs re-index from genesis on deploy, so the new non-null field is populated for
    all provisions via createOrLoadProvision; no migration concern.

@cargopete

Copy link
Copy Markdown
Contributor Author

Context: off-chain fix for the delegation APR/APY distortion

This PR is the off-chain half of the fix discussed in forum: Separate withdrawable bucket for fully-thawed delegation tokens and Accurately Representing Delegated GRT in APR/APY Calculations.

The key point: thawed-but-unwithdrawn delegation stays counted in delegatedThawingTokens (mirroring on-chain tokensThawing, which is only decremented on withdrawDelegated, not when the thaw clock expires). So delegatedTokens - delegatedThawingTokens is already the correct actively-earning base — the distortion comes purely from consumers dividing by delegatedTokens. This PR surfaces that base explicitly as Provision.delegatedTokensActive, requiring no protocol change.

For completeness, a hardened protocol-level variant (exposing the same split on-chain, with the storage/slashing issues from the original attempt fixed) is available at graphprotocol/contracts#1340 — but the recommendation is to fix this off-chain via this subgraph change.

@tmigone tmigone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but please check tests that are failing.

@cargopete cargopete force-pushed the feat/provision-delegated-tokens-active branch from 762c234 to 4e4998b Compare June 12, 2026 14:01
@cargopete

Copy link
Copy Markdown
Contributor Author

Thanks @tmigone. I dug into the failing checks — none of the failures are caused by this PR. Here's what I found and what I've done:

1. Compile error (the blocker that aborts the whole suite)tests/helpers.test.ts called createOrLoadIndexer with 2 args after it gained a third (graphNetwork) parameter. This makes graph test fail to compile before any test runs. Fixed in this PR (passes the loaded graph network through).

2. Four runtime failures in staking.testStakeDelegated (DelegatedStake with id …006-…003 not found). These are pre-existing on master and unrelated to this change. I verified by reverting this PR's mapping/schema changes to master and re-running: identical result, 4 failed, 84 passed. The mismatch is in the legacy DelegatedStake id scheme (the test expects delegator-indexer, the migrated handler now builds a Horizon-style id) — it looks like it belongs with the in-flight legacy-test work rather than here, so I've left it untouched to avoid conflicting with that. Happy to take it on in a separate PR if useful.

So with the compile fix, the suite runs and this PR's own paths are green; the remaining 4 are a pre-existing legacy-migration item. I've also re-signed the commits so the signature verifies.

@cargopete cargopete requested a review from tmigone June 12, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants