feat: cumulative factors and delegator shares for O(1) stake computation - #217
feat: cumulative factors and delegator shares for O(1) stake computation#217adamsoffer wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds cumulative reward and fee factors, precise accounting helpers, delegator share tracking with historical snapshots, and transcoder commission fields. Bonding, reward, round, earnings-claim, and ticket-redemption mappings now update these values. ChangesAccounting factors and commission tracking
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BondingManager
participant Pool
participant Transcoder
participant RoundsManager
BondingManager->>Pool: update cumulativeRewardFactor
BondingManager->>Transcoder: accrue reward commission
RoundsManager->>Transcoder: snapshot active cumulative rewards
BondingManager->>Pool: update cumulativeFeeFactor during fee accounting
BondingManager->>Transcoder: accrue fee commission
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3c1cb89 to
91eb05d
Compare
…cient historical stake computation Store cumulativeRewardFactor and cumulativeFeeFactor on Pool entity (matching on-chain PreciseMathUtils), propagate them forward each round, and introduce a shares field on Delegator that enables O(1) stake lookups via `stake = shares * crf[round] / 10^27`. DelegatorSnapshot entities capture state at bond/unbond/rebond events for time-series chart support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ommission Tracks the orchestrator's total rewardCut commission across all rounds, incremented each time reward() is called. Never resets, so clients can read lifetime earnings in a single field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ission Resets Transcoder.cumulativeRewards to zero when the orchestrator claims earnings, so the field represents pending/unclaimed commission rather than lifetime total. This lets clients compute full orchestrator pending stake as: shares * crf / 10^27 + cumulativeRewards Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Separate never-reset counter alongside cumulativeRewards (which resets on claim). Gives clients a single field for lifetime orchestrator commission without summing historical claim events. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename cumulativeRewards/lifetimeRewards to pendingRewardCommission/ lifetimeRewardCommission for clarity. Add pendingFeeCommission and lifetimeFeeCommission on Transcoder, computed in WinningTicketRedeemed handler. Both pending fields reset on claim. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous implementation did two divisions (num * 10^27 / denom, then base * result / 10^27), causing intermediate truncation that compounded each round in the CRF calculation. Solidity's version does one division (base * num / denom). While the CRF ratio error cancelled out for delegator stake, pendingRewardCommission accumulated ~0.23 LPT/round drift. This fix ensures exact match with the contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The contract's cumulativeRewards includes two components: the rewardCut commission plus rewards earned by the transcoder's own staked commission (activeCumulativeRewards). The subgraph was only tracking the first. Add activeCumulativeRewards field on Transcoder, snapshotted from pendingRewardCommission at the start of each round in newRound. The reward handler now computes both components matching the contract's updateTranscoderWithRewards logic. Reset on claim. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
91eb05d to
7b101df
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/mappings/bondingManager.ts (1)
144-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated shares-from-CRF computation across bond/unbond/rebond.
The same ~10-line "load pool → fallback to
PRECISE_PERC_DIVISOR→ compute shares" block is repeated verbatim inbond(),unbond(), andrebond(). Extracting it into a shared helper inutils/helpers.ts(e.g.computeShares(delegateAddress, roundId, bondedAmount)) would remove the duplication and prevent the three copies from silently diverging if the fallback/edge-case logic ever needs to change (e.g. together with the CRF-continuity fix noted inutils/helpers.ts).Also applies to: 296-313, 418-431
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mappings/bondingManager.ts` around lines 144 - 159, Extract the repeated pool-loading, CRF fallback, and shares calculation from bond(), unbond(), and rebond() into a shared helper in utils/helpers.ts, such as computeShares(delegateAddress, roundId, bondedAmount). Update all three call sites to use the helper while preserving the existing PRECISE_PERC_DIVISOR fallback and cumulativeRewardFactor handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@schema.graphql`:
- Around line 231-246: Update DelegatorSnapshot ID construction in the bond,
unbond, and rebond event handlers to include a unique per-event component such
as transaction hash and log index, rather than only delegator and round. Ensure
each state-changing event creates a distinct snapshot while preserving the
existing DelegatorSnapshot fields and save flow.
In `@src/mappings/bondingManager.ts`:
- Around line 166-175: The DelegatorSnapshot identifier in the snapshot creation
block collides with snapshots from unbond() and rebond() within the same round.
Update snapshotId to include a unique event-specific component while retaining
the delegator and round context, ensuring multiple same-round events produce
distinct records without overwriting earlier state.
In `@src/mappings/ticketBroker.ts`:
- Around line 135-145: Update the previous cumulative reward factor lookup in
the ticket-broker flow around prevPoolForFees so it searches backward through
earlier rounds until it finds the transcoder’s most recent Pool with a nonzero
cumulativeRewardFactor, rather than only checking round.id - 1. Preserve
PRECISE_PERC_DIVISOR as the fallback when no prior qualifying Pool exists, and
use the existing pool-loading helpers or lookup pattern from createOrLoadPool.
In `@utils/helpers.ts`:
- Around line 64-79: Update the cumulative-factor initialization in the shown
pool-loading flow to use the transcoder’s persisted last cumulative reward and
fee factors rather than resetting to ZERO_BI when Pool.load for the previous
round returns null. Store or reuse these factors on the Transcoder entity, while
preserving previous-pool values when available and ensuring the updated factors
are persisted for subsequent rounds.
---
Nitpick comments:
In `@src/mappings/bondingManager.ts`:
- Around line 144-159: Extract the repeated pool-loading, CRF fallback, and
shares calculation from bond(), unbond(), and rebond() into a shared helper in
utils/helpers.ts, such as computeShares(delegateAddress, roundId, bondedAmount).
Update all three call sites to use the helper while preserving the existing
PRECISE_PERC_DIVISOR fallback and cumulativeRewardFactor handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c79b66a3-23ff-4bca-a971-3b774b9e83c2
📒 Files selected for processing (5)
schema.graphqlsrc/mappings/bondingManager.tssrc/mappings/roundsManager.tssrc/mappings/ticketBroker.tsutils/helpers.ts
| type DelegatorSnapshot @entity { | ||
| "Unique identifier: delegator address + round number" | ||
| id: ID! | ||
| "The delegator this snapshot belongs to" | ||
| delegator: Delegator! | ||
| "The delegate (orchestrator) at the time of this snapshot, null if fully unbonded" | ||
| delegate: Transcoder | ||
| "Bonded amount at the time of this snapshot" | ||
| bondedAmount: BigDecimal! | ||
| "Proportional claim on the orchestrator's pool. stake = shares * crf[round] / 10^27" | ||
| shares: BigInt! | ||
| "Round when this snapshot was taken" | ||
| round: Round! | ||
| "Timestamp when this snapshot was taken" | ||
| timestamp: Int! | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(schema\.graphql|bondingManager\.ts)$' || true
echo "== schema relevant =="
sed -n '210,260p' schema.graphql 2>/dev/null || true
echo "== bonding manager outline =="
ast-grep outline bondingManager.ts 2>/dev/null | sed -n '1,220p' || true
echo "== bonding manager delegator snapshot usages =="
rg -n "DelegatorSnapshot|delegator.*round|new DelegatorSnapshot|snapshot" bondingManager.ts schema.graphql 2>/dev/null || true
echo "== context bond/unbond snippets =="
rg -n "bond|rebond|unbond|snapshot|DelegatorSnapshot" bondingManager.ts -C 3Repository: livepeer/subgraph
Length of output: 2972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate bonding manager =="
fd -a 'bondingManager\.ts$' . || true
find . -path '*/node_modules' -prune -o -name 'bondingManager.ts' -print
echo "== grep snapshots in repo =="
rg -n "new DelegatorSnapshot|DelegatorSnapshot\\(|delegator.*round|bondingManager|bond\\(|rebond\\(|unbond\\(-" . -g '!node_modules' -g '!dist' -g '!build' || true
echo "== list target file contents sizes and first 120 lines if exists =="
if [ -f src/mappings/bondingManager.ts ]; then
wc -l src/mappings/bondingManager.ts
sed -n '1,180p' src/mappings/bondingManager.ts
echo "== delegation-related occurrences in file =="
rg -n "DelegatorSnapshot|bond|rebond|unbond" src/mappings/bondingManager.ts -C 4
fiRepository: livepeer/subgraph
Length of output: 32746
Make DelegatorSnapshot.id unique per event.
id is fixed to delegator + round, while bond, unbond, and rebond all write new DelegatorSnapshot(...snapshotId).save() for the same key. Multiple state-changing events for the same delegator in one round will overwrite earlier snapshots, contradicting the snapshot “at each state-changing event” semantics. Include a per-event component, e.g. tx hash + log index, in the snapshot ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schema.graphql` around lines 231 - 246, Update DelegatorSnapshot ID
construction in the bond, unbond, and rebond event handlers to include a unique
per-event component such as transaction hash and log index, rather than only
delegator and round. Ensure each state-changing event creates a distinct
snapshot while preserving the existing DelegatorSnapshot fields and save flow.
| // Save delegator snapshot for historical stake/reward computation | ||
| let snapshotId = event.params.delegator.toHex() + "-" + round.id; | ||
| let snapshot = new DelegatorSnapshot(snapshotId); | ||
| snapshot.delegator = event.params.delegator.toHex(); | ||
| snapshot.delegate = event.params.newDelegate.toHex(); | ||
| snapshot.bondedAmount = delegator.bondedAmount; | ||
| snapshot.shares = delegator.shares; | ||
| snapshot.round = round.id; | ||
| snapshot.timestamp = event.block.timestamp.toI32(); | ||
| snapshot.save(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Snapshot id collision on multiple same-round events for a delegator.
snapshotId = delegator + "-" + round.id will collide with unbond()/rebond() snapshots for the same delegator in the same round, silently overwriting earlier state. See consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mappings/bondingManager.ts` around lines 166 - 175, The DelegatorSnapshot
identifier in the snapshot creation block collides with snapshots from unbond()
and rebond() within the same round. Update snapshotId to include a unique
event-specific component while retaining the delegator and round context,
ensuring multiple same-round events produce distinct records without overwriting
earlier state.
| let prevRoundNum = integerFromString(round.id).minus(ONE_BI); | ||
| let prevPoolForFees = Pool.load( | ||
| makePoolId(event.params.recipient.toHex(), prevRoundNum.toString()) | ||
| ); | ||
| let prevCRF = PRECISE_PERC_DIVISOR; // default: 10^27 | ||
| if ( | ||
| prevPoolForFees && | ||
| !prevPoolForFees.cumulativeRewardFactor.equals(ZERO_BI) | ||
| ) { | ||
| prevCRF = prevPoolForFees.cumulativeRewardFactor; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Same exact round-1 lookback fragility as createOrLoadPool.
prevPoolForFees is looked up at exactly round.id - 1; if no Pool exists for that round for this transcoder (e.g. it was out of the active set), this silently falls back to the PRECISE_PERC_DIVISOR default rather than the transcoder's actual last cumulative reward factor. See consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mappings/ticketBroker.ts` around lines 135 - 145, Update the previous
cumulative reward factor lookup in the ticket-broker flow around prevPoolForFees
so it searches backward through earlier rounds until it finds the transcoder’s
most recent Pool with a nonzero cumulativeRewardFactor, rather than only
checking round.id - 1. Preserve PRECISE_PERC_DIVISOR as the fallback when no
prior qualifying Pool exists, and use the existing pool-loading helpers or
lookup pattern from createOrLoadPool.
|
|
||
| // Propagate cumulative factors from the previous round's pool so every | ||
| // pool has valid factors even if the transcoder misses reward() or has | ||
| // no fees in a round. This mirrors the contract's latestCumulativeFactorsPool. | ||
| let prevRoundNum = integerFromString(roundId).minus(ONE_BI); | ||
| let prevPool = Pool.load( | ||
| makePoolId(transcoderAddress, prevRoundNum.toString()) | ||
| ); | ||
| if (prevPool) { | ||
| pool.cumulativeRewardFactor = prevPool.cumulativeRewardFactor; | ||
| pool.cumulativeFeeFactor = prevPool.cumulativeFeeFactor; | ||
| } else { | ||
| pool.cumulativeRewardFactor = ZERO_BI; | ||
| pool.cumulativeFeeFactor = ZERO_BI; | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant helper file and adjacent contract/event schema/data model.
if [ -f utils/helpers.ts ]; then
echo "== utils/helpers.ts outline =="
ast-grep outline utils/helpers.ts || true
echo
echo "== utils/helpers.ts relevant section =="
sed -n '1,120p' utils/helpers.ts | cat -n
else
echo "utils/helpers.ts not found"
fd -i helpers.ts
fi
echo
echo "== search Pool/Transcoder schema and makePoolId/newRound/lastCumulative usages =="
rg -n "export (function)? Pool|type Pool|interface Pool|PoolData|class Pool|newRound|makePoolId|cumulativeRewardFactor|cumulativeFeeFactor|lastCumulativeRewardFactor|lastCumulativeFeeFactor|Transcoder" -S .Repository: livepeer/subgraph
Length of output: 25275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Behavioral/structural invariant probe from local sources only:
# Build a minimal model of the current CRF/CFF propagation branches by
# reading the relevant source as data and checking whether propagation
# only checks roundId-1 Pool and only uses last transcoder fields when present.
from pathlib import Path
import re
path = Path("utils/helpers.ts")
text = path.read_text() if path.exists() else ""
chunks = text.split("}")
print("FILE_EXISTS:", path.exists())
print("HAS_PROXIMITY_TO_ROUND_ID_PREV:", "minus(ONE_BI)" in text)
print("HAS_EXACT_ROUND_MINUS_1_POOL_LOOKUP:", "makePoolId(transcoderAddress, prevRoundNum.toString())" in text)
print("HAS_ZERO_BACKFILL_BRANCH:", "pool.cumulativeRewardFactor = ZERO_BI;" in text and "pool.cumulativeFeeFactor = ZERO_BI;" in text)
# Print exact propagation block with line numbers for evidence.
for i, line in enumerate(text.splitlines(), 1):
if "prevRoundNum = integerFromString(roundId).minus(ONE_BI)" in line or "Pool.load" in line or "lastCumulative" in line:
print(f"{i}: {line}")
# Read-only small behavioral simulation: if a transcoder has a last-known CRF/CFF
# but Pool.load(roundId-1) is None, what must current code do based on observed branches.
prev_pool_exists = False
transcoder_with_last_factors = True
crf = None
cff = None
if prev_pool_exists:
crf = 1200000000000000000000000
cff = 1100000000000000000000000
else:
# current else-branch
crf = 0
cff = 0
print("BEHAVIORAL_SIMULATION_CURRENT_BRANCHES_WITH_ROUND_MINUS_1_POOL_MISSING:", {"prev_pool_exists": prev_pool_exists, "transcoder_with_last_factors": transcoder_with_last_factors, "crf": crf, "cff": cff})
PYRepository: livepeer/subgraph
Length of output: 580
Persist cumulative factors on Transcoder instead of relying on roundId - 1 pools.
If round roundId - 1 has no Pool for this transcoder, prevPool is null and both cumulativeRewardFactor/cumulativeFeeFactor reset to ZERO_BI, breaking the continuous growth from the previous round. Carry forward stored last factors when Pool.load(prevRound) is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@utils/helpers.ts` around lines 64 - 79, Update the cumulative-factor
initialization in the shown pool-loading flow to use the transcoder’s persisted
last cumulative reward and fee factors rather than resetting to ZERO_BI when
Pool.load for the previous round returns null. Store or reuse these factors on
the Transcoder entity, while preserving previous-pool values when available and
ensuring the updated factors are persisted for subsequent rounds.
Problem
The subgraph has no way to compute a delegator's actual total stake. The
bondedAmountfield only reflects the value at the time of the last claim and does not include rewards earned since then. To get a delegator's current stake, clients must call the contract'spendingStake()method directly — and for historical stake at a past round, there is no solution at all.Similarly, computing an orchestrator's full pending stake (including their unclaimed commission) or their lifetime earnings requires fetching every pool since their last claim and summing across all of them.
This also means there's no way to build time-series charts of delegator stake, show per-round reward breakdowns, or compute orchestrator yield — all of which require knowing stake values across rounds.
Summary
cumulativeRewardFactorandcumulativeFeeFactoron thePoolentity, matching on-chain PreciseMathUtils (27-decimal fixed-point)reward()calls)sharesfield toDelegator(bondedAmount * 10^27 / crf[lastClaimRound]) — invariant across claims, only changes on bond/unbond/rebondDelegatorSnapshotentity capturing delegator state at each bond/unbond/rebond for time-series chart supportWinningTicketRedeemedhandler using previous round's CRF (matching contract behavior)Transcoder:pendingRewardCommission— unclaimed reward commission (rewardCut portion), resets on claimlifetimeRewardCommission— total reward commission ever earned, never resetspendingFeeCommission— unclaimed fee commission, resets on claimlifetimeFeeCommission— total fee commission ever earned, never resetsWhat this unlocks
Current pending stake without contract calls — A delegator's up-to-date total stake (equivalent to
pendingStake()on-chain) can now be computed entirely from the subgraph:stake = shares * crf / 10^27. For orchestrators specifically, their full pending stake includes unclaimed commission:stake = shares * crf / 10^27 + pendingRewardCommission. No contract calls needed.Historical stake at any past round — Something previously impossible. Query the delegator's shares (or the relevant
DelegatorSnapshot) and thePool.cumulativeRewardFactorfor that round, then computeshares * crf[round] / 10^27.Per-round reward and fee breakdowns — Rewards earned in a specific round can be derived by comparing cumulative factors between consecutive rounds:
rewards = shares * (crf[round] - crf[round-1]) / 10^27. Same pattern applies to fees usingcumulativeFeeFactor.Time-series charts of delegator stake — The
sharesapproach combined withDelegatorSnapshotentities makes it straightforward to build a chart showing a delegator's total stake over time. Between snapshots, shares are constant, so stake at any round is justshares * crf[round] / 10^27. When a bond/unbond/rebond occurs, a new snapshot captures the updated shares value to use going forward.Orchestrator commission tracking —
pendingRewardCommissionandpendingFeeCommissiongive the orchestrator's unclaimed commission in a single field each, without needing to sum across pools.lifetimeRewardCommissionandlifetimeFeeCommissionprovide total lifetime earnings.Orchestrator pool analytics — Cumulative factors stored per-pool per-round enable computing total rewards distributed, effective yield, and fee revenue for any orchestrator over any time range without replaying events.
All changes are additive — no breaking changes to existing queries.
Test plan
npx graph codegen && npx graph build— verified locallyshares * crf[round] / 10^27matches actual bonded amountspendingStake()return valuependingRewardCommissionandpendingFeeCommissionreset to zero after orchestrator claimslifetimeRewardCommissionandlifetimeFeeCommissionnever reset🤖 Generated with Claude Code
Summary by CodeRabbit