Skip to content

Fix missing UnstakingManager subscription; track vault exchange rate and staking positions - #22

Open
lcamargof wants to merge 4 commits into
mainfrom
fix/vault-unstaking-manager-exchange-rate
Open

Fix missing UnstakingManager subscription; track vault exchange rate and staking positions#22
lcamargof wants to merge 4 commits into
mainfrom
fix/vault-unstaking-manager-exchange-rate

Conversation

@lcamargof

@lcamargof lcamargof commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Version 1.10.0 — clean reindex of all three networks (no graft). Deployed as dtf-index-{mainnet,base,bsc}/1.10.0 and syncing; prod tags still on 1.9.8.

Bug: locks never indexed for vaults discovered outside a deployer event

The vlRSR vaults (BSC 0xE744…, mainnet 0xABbD…, base 0x2F0D…) were deployed by the per-chain ReserveOptimisticGovernorDeployer (reserve-governor repo), which the subgraph didn't track. They got discovered through Governor.token(), and that path only subscribed the StakingToken template — the UnstakingManager subscription lived solely in the GovernanceDeployer handler. Result on Goldsky: unstakingManager: null, locks: [] while LockCreated events sat on chain.

  • getOrCreateStakingToken now subscribes both templates (subscribeUnstakingManager), so every discovery path is covered.
  • New OptimisticGovernorDeployer data source per chain (mainnet 0x2acc45e1…, base 0x604d70d1…, bsc 0x1c10e68b…). deployWithNewStakingVault indexes the vault, manager, governance and timelock (the AccessControl vault emits no OwnershipTransferred). deployWithExistingStakingVault (stakingVault = 0x0) only discovers the vault; the DTF governance keeps being created by the DTF role grant, where its timelock and optimisticProposers are known.
  • Account is now saved before Lock / RewardClaim reference it.

Vault exchange rate

The reserve-governor StakingVault streams native RSR rewards into totalAssets(), so vlRSR/RSR is no longer 1:1 and drifts every block. A hardcoded TrackedVault data source per chain (networks.json, the vlRSR vault) runs a polling block handler every ~10 minutes (every derived from secondsPerBlock: mainnet 50, base 300, bsc 1333 blocks) that records StakingToken.exchangeRate / totalAssets, a daily upsert snapshot and a change-only snapshot. Also handles UnstakingDelaySet / RewardRatioSet.

Per-account staking positions

StakingPosition (average-cost principal, realizedRewards, lifetime totals), StakingPositionRecord per Deposit / LOCK_CANCEL / Withdraw / TRANSFER_IN|OUT, and StakingPositionDailySnapshot. Withdrawals release principal pro rata and realize the excess; wallet transfers move principal pro rata; a cancelled lock (Deposit.sender == unstakingManager) re-enters as principal without inflating lifetime totals. Query recipe for the UI chart in docs/staking-vaults.md.

Validation

  • 110 matchstick tests passing, including replays of the real BSC data: the deployer event, both LockCreated events (blocks 116930184 / 116931302), and the locker's full 14-event history checked against an independent BigInt oracle (final shares match balanceOf on chain).
  • graph build green for mainnet, base and bsc; all 19 deployWithExistingStakingVault governances verified on chain to point at their chain's vlRSR; deploy script confirmed not to graft.

…vault exchange rate and staking positions

- Move the UnstakingManager template subscription into getOrCreateStakingToken so
  vaults discovered through Governor.token() or timelock role grants index their
  locks (BSC/mainnet/base vlRSR had unstakingManager = null and no Lock entities)
- Add the ReserveOptimisticGovernorDeployer data source per chain
  (ReserveOptimisticGovernorSystemDeployed, both new-vault and existing-vault flavours)
- Add a hardcoded TrackedVault data source per chain (the vlRSR vault) with a
  polling block handler sampling convertToAssets/totalAssets every ~10 minutes;
  StakingToken.exchangeRate/totalAssets, daily + change-only snapshots
- Add StakingPosition / StakingPositionRecord / StakingPositionDailySnapshot:
  average-cost principal, pro-rata release on withdraw/transfer, realized rewards,
  LOCK_CANCEL re-deposits, accountingComplete for late-discovered positions
- Handle UnstakingDelaySet / RewardRatioSet; save Account before referencing it
  from Lock / RewardClaim
- Tests replay the real BSC events (deployer event, both LockCreated, the locker's
  full 14-event history against an independent oracle)
Shares received from a holder whose own cost basis was unknown were folded into
the receiver's tracked pool with zero principal, so a later withdrawal booked the
full payout as realized rewards. Positions now hold two pools (shares with a
known basis, untrackedShares without); exits take from both pro rata and only the
tracked part releases principal or earns rewards, on any number of hops.
Vaults are discovered at creation (deployer events + TrackedVault), so every
share is seen entering; the quarantine pools and accountingComplete flag were a
safety net for a case that cannot happen. Back to plain average-cost principal.
For deployWithExistingStakingVault the deployer handler created the Governance
before its GovernanceTimelock existed, so getOrCreateGovernance froze
optimisticProposers to [] and nothing backfilled it (prod currently has them via
the role-grant path). The handler now only discovers the vault through
Governor.token(); the DTF role grant creates timelock + governance as on main.
@JuampiRombola

Copy link
Copy Markdown
Contributor

Code review — a few correctness issues worth fixing before merge, plus some smaller notes.

1. handleWithdraw drops the Withdraw event's receiver param (src/staking-token/mappings.ts:170)
_handleVaultWithdraw only receives owner, but ERC-4626 allows receiver != owner and the UnstakingManager keys the resulting lock to the receiver. When Alice withdraws to Bob, totalWithdrawn/realizedRewards are booked to Alice while the lock belongs to Bob — and if Bob later cancels the lock, the LOCK_CANCEL re-deposit lands on Bob's position as principal he never "deposited", while Alice permanently shows realized rewards she never received.

2. Position tracking runs on non-vault StakingTokens (src/staking-token/mappings.ts:149)
_handlePositionTransfer fires for every non-mint/burn transfer on the StakingToken template, but the template also attaches to plain vote tokens discovered via Governor.token() (the code already tolerates this: getOrCreateStakingToken swallows reverting asset(), and _handleExchangeRatePoll guards on unstakingManager === null). For such tokens no Deposit ever fires, so transfers produce structurally wrong StakingPosition rows (sender clamps to 0 shares, receiver gets shares with principal 0, records stamp a meaningless 1:1 rate). Mirroring the poll handler's unstakingManager === null guard in handleTransfer would fix it.

3. Exchange rate freezes for any future vault outside the hardcoded TrackedVault list (subgraph.yaml.mustache / networks.json)
The next deployWithNewStakingVault gets its rate seeded once at discovery and never updated — no polls, no snapshots, stale rates stamped on transfer records — until someone hand-edits networks.json on three chains and reindexes. Consumers can't even tell the value is stale since the field is non-null. Suggestion: also refresh the rate from the vault's own Deposit/Withdraw handlers, keeping the poll as a heartbeat — that covers every discovered vault automatically.

4. !== string identity bug in the legacy-governance guard (src/staking-token/handlers.ts:311, pre-existing but in a PR-touched file)
oldOwner.toHexString() !== GENESIS_ADDRESS uses AssemblyScript's reference-identity !== on a fresh string, so it's always true and every legacy Ownable vault gets a bogus 0x0 entry pushed as its first legacyGovernance element (the constructor's OwnershipTransferred(0x0 → deployer) replays in the template creation block). One-char fix: !==!= (line 139 of the same file already does this correctly).

Smaller notes:

  • LOCK_CANCEL asymmetry: a cancel leaves the withdrawal in totalWithdrawn but excludes the re-deposit from totalDeposited, so deposit→withdraw→cancel→withdraw reads totalDeposited=100, totalWithdrawn=240 and the pair stops working as matched gross-flow counters. Either count the round-trip in both or document the asymmetry on the schema fields.
  • subscribeUnstakingManager is one-shot and silent: if try_unstakingManager() reverts/returns zero at first discovery there's no log and no retry, silently recreating the exact bug this PR fixes. A log.warning on the skip + retrying when the field is null on existing entities would make it self-healing.
  • The getOrCreateAccount(x); account.save() fix is repeated at call sites, but src/governance/handlers.ts:370/389/420 still assign getOrCreateAccount(event.transaction.from).id without a save — same latent dangling-Account bug. Saving on the create branch inside getOrCreateAccount fixes all sites at once (no caller depends on it being unsaved).
  • Dead StakingToken import in src/staking-token/handlers.ts:6; README template-parse list has two steps numbered "4."; _handleOptimisticGovernorSystemDeployed links stakingToken.governance twice (on-chain rediscovery in createGovernanceTimelock + overwrite from the event param) — passing the known governor in would leave one linking path and save ~3 eth_calls per deploy.

Solid PR overall — the test coverage (real BSC replay against an independent oracle) is great. Items 1–4 are the ones I'd address before merge.

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.

3 participants