feat(staking): IIP-59 VoterWeightView infrastructure (PR 2/5)#4860
feat(staking): IIP-59 VoterWeightView infrastructure (PR 2/5)#4860envestcc wants to merge 5 commits into
Conversation
0ade44a to
3c5e0d7
Compare
Adds the proto-level field and feature gate that the rest of the IIP-59 protocol-native voter reward distribution PRs will hang behavior off of. No runtime behavior changes in this PR — the field is populated as zero on existing chain data, default behavior matches today exactly. Schema: - stakingpb.Candidate gains commissionRate=11; Go Candidate struct mirrors it. Equal/Clone/toProto/fromProto updated (the original PoC missed Equal — flagged in iotexproject#4811 review #2). - state.Candidate gains CommissionRate, snapshotted per epoch from the staking candidate state by PutPollResult (added in PR 4). The latest user-set value lives on staking.Candidate; state.Candidate holds the per-epoch frozen value consumed by GrantEpochReward. - iotextypes.Candidate.commissionRate is set/read in candidateToPb / pbToCandidate so the new field travels through poll snapshots and over the wire. Feature flag: - FeatureCtx.NoVoterRewardDistribution, bound to !g.IsToBeEnabled(height) per AGENTS.md convention for WIP features (the gate will be swapped for a real hardfork height at release time). - Named so that the bool zero value (false) corresponds to the post-fork activated behavior, matching the existing NoCandidateExitQueue / NotSlashUnproductiveDelegates convention. A docstring records this rule next to the field for future readers. Why no separate commissionRateLastEpoch field: - IIP-59 doesn't prescribe a per-rate-change cooldown. Its protection against rapid manipulation is epoch-boundary activation, which our design already provides for free: a SetCommissionRate at any moment only affects rewards in the epoch *after* the next PutPollResult snapshot, giving voters ~1.5 epochs of reaction time. - If the protocol later decides cooldown is needed, adding the field is an additive proto change with no migration cost. Toolchain: - stakingpb regenerated with protoc-gen-go v1.26.0 to match the version recorded in the existing staking.pb.go header. Local dev: - go.mod replace pointing at ../iotex-proto so the build resolves the new iotex-proto fields prior to the proto PR being tagged. Remove the replace once iotex-proto cuts a release containing the SetCommissionRate + Candidate.commissionRate additions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…IIP-59) This commit introduces the data structure that IIP-59's reward-distribution path will consume in PR 4. No callers yet — this is the underlying view type, its mutation entry point, the deterministic hash digest, and the clone/iteration helpers, with unit tests. Design: - Per-candidate sorted slice of (voter, weight) tuples (sorted by voter address). distributeVoterReward iterates the slice directly — never the map — so receipt log order is deterministic across nodes (PoC iotexproject#4811 review finding #2 was exactly this bug, fixed here at the data-structure level rather than at every caller). - Per-candidate index map for O(log n) insert/remove on hot paths. - Multiple buckets from the same voter to the same delegate are aggregated per voter, not per bucket, which avoids per-bucket rounding loss at distribution time. - Hash() walks candidates in sorted hash160 order and voters in already-sorted slice order; two views with the same logical state produce the same digest, independent of Apply() insertion order. PR 2's next commit persists this digest to a new staking namespace tag (_voterWeights = 5) so a restarted node can rebuild from buckets and verify against the last-committed hash. Apply() semantics: - Adds a (cand, voter, delta) tuple. Aggregates with any existing weight. - Withdrawals that drive the per-voter total to zero remove the voter entry; the candidate entry is removed too when its last voter leaves. - Withdrawals against an unknown (cand) or (voter) are silently no-op (rationale: the staking handlers never overdraw; if they ever do, the view-hash check at restart catches the divergence loudly). Coverage: - Tests for add, aggregate, decrease, drive-to-zero, over-withdraw, unknown-key no-op, sortedness, clone deep-copy, hash determinism across insertion orders, hash sensitivity to weight change, realistic-event-stream incremental-vs-rebuild equivalence, and a benchmark for the Apply hot path (73.8 ns/op on M1 Pro). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| // per-action handlers. The build is idempotent — subsequent calls | ||
| // see a non-nil voterWeights and short-circuit. | ||
| if featureCtx.EnableVoterRewardDistribution { | ||
| if err := p.ensureVoterWeightView(ctx, sm); err != nil { |
There was a problem hiding this comment.
I don't think it's necessary to load VoterWeightView every time CreatePreState is called; it only needs to be loaded once at startup. There should be no scenario where the VoterWeightView is missing here.
| amount: new(big.Int).Set(v.snapshots[i].amount), | ||
| count: v.snapshots[i].count, | ||
| contractsStake: v.snapshots[i].contractsStake, | ||
| voterWeights: v.snapshots[i].voterWeights.Clone(), |
There was a problem hiding this comment.
Cloning voterWeights during viewData.Fork is an inefficient implementation that will cause serious performance issues. You can refer to contractStakeView as a reference.
| } | ||
| } | ||
| fork.contractsStake = v.contractsStake.Fork() | ||
| fork.voterWeights = v.voterWeights.Clone() |
… (IIP-59) Wires the IIP-59 voter-weight view into the staking protocol's standard view lifecycle so it follows the same Fork/Snapshot/Revert/Commit rules as bucketPool and contractsStake. No callers consume the view yet — that arrives in PR 4 with distributeVoterReward. viewData: - New voterWeights *VoterWeightView field plus parallel field on Snapshot for deep capture/restore. Fork deep-clones; Snapshot deep-clones (Apply is the single mutation path and mutates in place, so a shallow snapshot would not survive any later change); Revert restores the snapshotted clone; Commit persists the view digest under the new _voterWeights namespace tag when IsDirty. - voterWeightDigest is the on-disk format: a single 32-byte Hash256 rewritten only on dirty commits. Other nodes' digests at the same block height must match byte-for-byte; a mismatch surfaces via the state digest path that already feeds into deltaStateDigest, so any divergence fails the block. CreatePreStates branch: - When EnableVoterRewardDistribution is on and viewData.voterWeights is nil (first block after flag activation, or restart), ensureVoterWeightView scans all active native buckets, computes per-bucket weight via CalculateVoteWeight (using ContractAddress=="" gate to apply the self-stake bonus only to native self-stake buckets — fixes PoC iotexproject#4811 review finding iotexproject#5), and Apply()s each weight into a fresh view. - After the build, the rebuilt hash is checked against the persisted digest. Mismatch is fatal: the staking view has diverged from the on-chain bucket state and continuing would produce invalid receipts. ErrStateNotExist (no record yet — first activation) is the normal path: the view is marked dirty so the next Commit writes the initial digest. Contract-staking buckets are not yet enumerated here; PR 2's next commit adds the indexer-driven enumeration. Until then, the view is built from native buckets only; the persisted hash still anchors determinism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…P-59) Defines the single entry point that subsequent handler hooks (PR 2.5) will use to keep the VoterWeightView in sync with on-chain bucket changes. No callers yet — the helper is no-op when the feature flag is off and when delta is zero, so PR 2.5 can wire it next to existing candidate.AddVote / candidate.SubVote sites without first checking the flag. The helper sits next to ensureVoterWeightView so all IIP-59 view-mutation concerns are co-located. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| if err := v.contractsStake.Commit(ctx, sm); err != nil { | ||
| return err | ||
| } | ||
| if err := v.commitVoterWeights(sm); err != nil { |
There was a problem hiding this comment.
can define a Commit method of the VoterWeightView, and call it instead
| amount: new(big.Int).Set(v.bucketPool.total.amount), | ||
| count: v.bucketPool.total.count, | ||
| contractsStake: v.contractsStake, | ||
| voterWeights: v.voterWeights.Clone(), |
3c5e0d7 to
0cc2d30
Compare
| // | ||
| // The hash is persisted at the _voterWeights namespace tag on every block | ||
| // commit and re-checked at restart against a rebuilt-from-buckets view. | ||
| func (v *VoterWeightView) Hash() hash.Hash256 { |
There was a problem hiding this comment.
We should run a benchmark for Hash(). Currently, there are approximately 100 candidates and 100,000 staking buckets on mainnet.
…eview) Responds to envestcc's review feedback on PR iotexproject#4860. Replaces the eager deep-clone Snapshot/Fork model with the same lazy-overlay pattern that ContractStakeView already uses (voteView / candidateVotesWraper in systemcontractindex/stakingindex), and moves the IIP-59 init out of the per-block CreatePreStates hot path. VoterWeightView is now an interface with three implementations: - voterWeightBase — the terminal layer holding the actual sorted map. - voterWeightWrap — a thin overlay used by viewData.Snapshot. Wraps a parent (base or another wrap) and accumulates Apply deltas locally; Commit replays them into the parent's base directly. No data copy at Snapshot time. - voterWeightFork — commit-in-clone overlay used by viewData.Fork. The parent base is shared until Commit actually flushes, then cloned so the parent stays intact. Forks that never mutate the view pay nothing. Per-snapshot cost drops from O(N) (full map clone of ~100k voters) to O(k) where k is the number of (cand, voter) tuples Apply touched between snapshot and revert — typically a handful per action. Commit and persistence moved onto the interface: - voterWeights.Commit(sm) flattens the overlay AND persists the digest if dirty. viewData.Commit just calls it and installs the returned view. - viewData.commitVoterWeights / VoterWeightView.MarkClean removed. Initial scan moved from CreatePreStates → Protocol.Start: - Protocol.Start now calls loadVoterWeightView once at node startup after both candCenter and contractsStake have loaded. The previous ensureVoterWeightView hook ran inside CreatePreStates (which fires on every block) and only short-circuited on the nil check — wasteful. - Persisted digest is still verified against the rebuilt view at startup; mismatch is fatal. ErrStateNotExist (first activation, no digest yet) marks the view dirty so the next block commit writes the initial hash. New test coverage: - TestVoterWeightView_ForkIsolation — fork mutates without leaking to parent before commit; after commit, fork has the delta and parent unchanged. - TestVoterWeightView_WrapMergesIntoParent — wrap commit lands changes in the shared parent base. - BenchmarkVoterWeightView_Hash — 100 candidates × 100k voters (mainnet-scale): Hash() runs in ~13.2ms on M1 Pro. Block time is 5s, so this is well within the budget; called at most once per block via the commit path. Existing tests retargeted to the interface API (Hash() == ZeroHash256 instead of removed IsEmpty(); Fork/Wrap instead of removed Clone()). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
All six comments addressed in Comment 1 — Removed the CreatePreStates branch. Comments 2, 3, 5 —
Per-snapshot cost drops from O(N) (full map clone of ~100k voters) to O(k) where k is the number of (cand, voter) tuples touched between snapshot and revert — typically a handful per action. Comment 4 — Moved both flatten and persist logic onto the interface: Comment 6 — Added ~13.2ms per call at full mainnet scale, called at most once per block via the viewData commit path. Well within a 5-second block budget. Test sweep
|
| // bucket state and view state cannot silently pass into block | ||
| // production. This is a one-shot full scan — subsequent blocks rely on | ||
| // the incremental Apply hooks in the per-action handlers. | ||
| if !protocol.MustGetFeatureCtx(ctx).NoVoterRewardDistribution { |
There was a problem hiding this comment.
should load voterWeightView even if NoVoterRewardDistribution
| // the enumeration once its handler hooks are wired. Until then, the view is | ||
| // built from native buckets only — the persisted hash still anchors | ||
| // determinism within that scope. | ||
| func (p *Protocol) loadVoterWeightView(c *viewData, sr protocol.StateReader) error { |
There was a problem hiding this comment.
In addition to native buckets, those contract staking buckets should also be counted.
…eview) Responds to envestcc's review feedback on PR iotexproject#4860. Replaces the eager deep-clone Snapshot/Fork model with the same lazy-overlay pattern that ContractStakeView already uses (voteView / candidateVotesWraper in systemcontractindex/stakingindex), and moves the IIP-59 init out of the per-block CreatePreStates hot path. VoterWeightView is now an interface with three implementations: - voterWeightBase — the terminal layer holding the actual sorted map. - voterWeightWrap — a thin overlay used by viewData.Snapshot. Wraps a parent (base or another wrap) and accumulates Apply deltas locally; Commit replays them into the parent's base directly. No data copy at Snapshot time. - voterWeightFork — commit-in-clone overlay used by viewData.Fork. The parent base is shared until Commit actually flushes, then cloned so the parent stays intact. Forks that never mutate the view pay nothing. Per-snapshot cost drops from O(N) (full map clone of ~100k voters) to O(k) where k is the number of (cand, voter) tuples Apply touched between snapshot and revert — typically a handful per action. Commit and persistence moved onto the interface: - voterWeights.Commit(sm) flattens the overlay AND persists the digest if dirty. viewData.Commit just calls it and installs the returned view. - viewData.commitVoterWeights / VoterWeightView.MarkClean removed. Initial scan moved from CreatePreStates → Protocol.Start: - Protocol.Start now calls loadVoterWeightView once at node startup after both candCenter and contractsStake have loaded. The previous ensureVoterWeightView hook ran inside CreatePreStates (which fires on every block) and only short-circuited on the nil check — wasteful. - Persisted digest is still verified against the rebuilt view at startup; mismatch is fatal. ErrStateNotExist (first activation, no digest yet) marks the view dirty so the next block commit writes the initial hash. New test coverage: - TestVoterWeightView_ForkIsolation — fork mutates without leaking to parent before commit; after commit, fork has the delta and parent unchanged. - TestVoterWeightView_WrapMergesIntoParent — wrap commit lands changes in the shared parent base. - BenchmarkVoterWeightView_Hash — 100 candidates × 100k voters (mainnet-scale): Hash() runs in ~13.2ms on M1 Pro. Block time is 5s, so this is well within the budget; called at most once per block via the commit path. Existing tests retargeted to the interface API (Hash() == ZeroHash256 instead of removed IsEmpty(); Fork/Wrap instead of removed Clone()). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2678a6e to
a55e194
Compare
|
Both addressed in Comment 1 —
|
|
|
Closing as part of IIP-59 PR reorganization. The proposal was amended (iotexproject/iips#73) to cover both block reward + epoch reward and to base voter distribution on a per-epoch snapshot. The new PR split is:
The incremental voter weight view (old #4860 + #4863) is dropped in favor of a per-PutPollResult full scan; ~40k buckets sorted+encoded once per epoch fits well inside the mint budget and eliminates the 13-handler-hook consensus-safety surface. Old #4864 (voter reward distribution) is replaced by new PR 3 which reads the snapshot instead of the live view. Superseded by new PRs — will link once opened. |
1 participant
Summary
PR 2 of the IIP-59 series. Introduces the per-(candidate, voter) weighted-votes view data structure that the rest of the IIP-59 implementation hangs off of, integrates it into the staking protocol's standard view lifecycle (Fork/Snapshot/Revert/Commit), and persists its deterministic hash digest to anchor cross-node consensus on the view contents. No runtime behavior changes — the view is built when the flag activates but no callers consume it in this PR.
The actual handler hooks that keep the view in lock-step with each bucket lifecycle event are split into PR 2.5 to keep this PR scoped to the view's data structure and lifecycle wiring (low-risk; no production handler files touched).
Series
What's in this PR
Data structure (
voter_weight_view.go):VoterWeightViewkeys candidates by Hash160 identifier; each entry is a sorted slice of(voter, *big.Int)plus an index map for O(log n) insert/remove.Apply(candID, voter, delta)is the single mutation entry point. Aggregates per-voter, removes the entry on drain-to-zero, removes the candidate when its last voter leaves, and is defensively no-op when a withdrawal targets an unknown (cand) or (voter).VoterWeightsByCandidate(candID)returns a sorted copy of the slice — distribution iterates this slice, never the map, which makes receipt-log order and state-write order identical across nodes (this is the bug class flagged as Batch update 2019-04-27 6PM PDT #2 in the original PoC review at iotex-core#4811).Hash()walks candidates in sorted Hash160 order and voters in their already-sorted slice order; insertion-order-independent (proven by tests).Applyat 73.8 ns/op on M1 Pro.viewData integration (
viewdata.go):voterWeights *VoterWeightViewfield plus matching field onSnapshot.Fork/Snapshotdeep-clone (Apply mutates in place);Revertrestores the cloned snapshot;Commitpersists the digest whenIsDirty.Persistence (
voter_weight_view.go):_voterWeights = 5(next free tag; the existing 0-4 are reserved inprotocol.go).voterWeightDigestserializes as exactly 32 bytes; corruption surfaces as a deserialization error rather than a silent zero.viewData.commitVoterWeightswrites the digest only when dirty, so pre-flag chains are byte-identical to today's behavior.Initial population (
protocol.goCreatePreStates):EnableVoterRewardDistributionis on andviewData.voterWeightsis nil (first block after activation, or restart),ensureVoterWeightViewscans all active native buckets, computes per-bucket weight viaCalculateVoteWeight(withContractAddress == \"\" && Index == cand.SelfStakeBucketIdxfor the self-stake gate — fixes PoC review finding Docker build fails #5), and Apply()s each weight into a fresh view.Helper for PR 2.5 (
voter_weight_view.go):applyVoterWeightDelta(csm, candIdentifier, voter, delta)— the single entry point PR 2.5 will wire into every native + contract-staking handler that changes a bucket's weight contribution. No-op when the flag is off and when delta is zero, so callers can drop it next to any existingcandidate.AddVote/candidate.SubVotewithout first checking the flag.What's not in this PR
task #7in our internal tracker) will instrument:handlers.go:handleCreateStake,handleUnstake,handleChangeCandidate,handleTransferStake(asymmetric: −W on old voter, +W on new voter),handleDepositToStake,handleRestakehandler_candidate_endorsement.go,handler_candidate_selfstake.gohandler_stake_migrate.go(−W native, +W contract)nfteventhandler.go(V1/V2/V3 contract staking events)vote_reviser.go,protocol.goensureVoterWeightViewcurrently scans native buckets only. Contract buckets are read from each indexer via the existingContractStakeView.CandidateStakeVotesboundary, which is per-candidate aggregated. To enumerate per-voter contract bucket contributions at startup, PR 2.5 (or its precursor) will extend the indexer interface with aBuckets()enumerator (or use the existingBucketsByCandidateover the candidate-center list).Why split this way
The data structure and lifecycle are independently reviewable — they compile, they pass tests, and they cannot change runtime behavior because no caller reads from the view. Splitting handler instrumentation into its own PR contains the production-handler-touching change to a separately-scoped review window, where most of the work is mechanical (delta wiring next to existing AddVote/SubVote sites) but each site needs careful attention to corner cases (transfer-style asymmetric updates, self-stake bonus changes, native↔contract migration).
Test plan
go build ./...passesgo test ./action/...— 922 tests pass; no regressionvoter_weight_view_test.go— 14 unit tests + 1 benchmark covering data-structure correctness, determinism, and clone safetymake lint(run before un-drafting)Behavior gating
EnableVoterRewardDistribution = false.voterWeightsstays nil.Applypaths are no-op.Commitis no-op. Persistence path is no-op. Byte-identical to today.🤖 Generated with Claude Code