feat(staking): wire VoterWeightView hooks into handlers (PR 2.5/5) - #4863
Closed
envestcc wants to merge 6 commits into
Closed
feat(staking): wire VoterWeightView hooks into handlers (PR 2.5/5)#4863envestcc wants to merge 6 commits into
envestcc wants to merge 6 commits into
Conversation
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>
… (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>
…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>
…ndlers (IIP-59)
PR 2.5 in the IIP-59 series. PR 2 added the per-(candidate, voter) view
infrastructure but no handler called into it; this commit instruments
every native + contract staking handler that mutates a bucket's weight
contribution so the view stays in lock-step with on-chain bucket state.
No runtime behavior change pre-flag — applyVoterWeightDelta is a no-op
when the view is nil.
Handler hooks (one applyVoterWeightDelta call per AddVote/SubVote pair):
action/protocol/staking/handlers.go
- handleCreateStake: +W on (cand, staker)
- handleUnstake: -W on (cand, bucket.Owner)
- handleChangeCandidate: -W on (prevCand, voter) and +W on (newCand, voter)
- handleTransferStake: asymmetric — same cand+weight, voter changes
from oldOwner to newOwner. The handler doesn't
call AddVote/SubVote (candidate total stays
the same), so the hook is two explicit Apply
calls instead of mirroring an existing site.
- handleDepositToStake: Δ on (cand, voter), since the bucket amount
changed and the per-voter weight too
- handleRestake: same Δ pattern
action/protocol/staking/handler_candidate_endorsement.go
- clearCandidateSelfStake: same bucket drops the self-stake bonus —
(newWeight − prevWeight) Δ. Signature gained
a csm argument so the hook has a view handle.
action/protocol/staking/handler_candidate_selfstake.go
- handleCandidateActivate: two transitions in one handler — prev
self-stake bucket loses the bonus, new
self-stake bucket gains it. Two Δ Applies.
action/protocol/staking/handler_stake_migrate.go
- handleStakeMigrate: -W on (cand, bucket.Owner) for the burned
native bucket. The matching +W on the freshly
minted contract bucket flows through the
nfteventhandler hook below.
action/protocol/staking/nfteventhandler.go (contract V1/V2/V3 path)
- PutBucket: +W on (cand, contractBucket.Owner)
- DeleteBucket / DeductBucket: -W on (cand, contractBucket.Owner)
These three are the single funnel every contract staking event takes
(via ContractStakeView.Handle), so instrumenting them once covers all
three indexer impls automatically.
action/protocol/staking/protocol.go
- slashCandidate's self-stake-bucket-shrink path: Δ on (cand, bucket.Owner).
action/protocol/staking/candidate_statemanager.go
- candSM.deactivate: deactivation drops the self-stake bonus on the
bucket — Δ on (cand, bucket.Owner).
- candSM.DirtyView: **bug fix discovered while writing the hook tests** —
the returned DirtyView was stripping voterWeights to nil, so every
Apply through csm.DirtyView().voterWeights would have been a silent
no-op. Now propagates the pointer from the base view.
vote_reviser.go was intentionally NOT instrumented: it only fires at
historical fork heights (Greenland, Hawaii) that are long past on
mainnet, before IIP-59 activates. By the time IIP-59 is on, vote_reviser
is a permanent no-op via its NeedRevise/cache short-circuit.
Side fix in viewdata.go:
- viewData.Commit's feature-flag lookup now uses GetFeatureCtx (returns
ok bool) instead of MustGetFeatureCtx — some unit tests commit with
context.Background() (TestProtocol_HandleCandidateEndorsement_*).
Missing FeatureCtx is treated as pre-fork: persistSM stays nil and the
view's dirty flag is cleared without touching the state trie.
Test:
- voter_weight_hooks_test.go (3): smoke test for applyVoterWeightDelta;
load-time view consistency (per-cand per-voter keying with the same
voter on two candidates); defensive over-withdraw is no-op.
- Full action/protocol/staking package passes (252/254 — the two
remaining failures are the pre-existing flaky
TestProtocol_FetchBucketAndValidate iotexproject#4813 unrelated to this PR).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This was referenced Jun 29, 2026
Member
Author
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 2.5 in the IIP-59 series. PR #4860 added the per-(candidate, voter) view infrastructure but no handler called into it; this PR instruments every native + contract staking handler that mutates a bucket's weight contribution so the view stays in lock-step with on-chain bucket state. No runtime behavior change pre-flag —
applyVoterWeightDeltais a no-op when the view is nil (the standard pre-fork state).Series
Hook coverage
One
applyVoterWeightDeltacall per AddVote / SubVote pair already in the existing handlers:handlers.gohandleCreateStakehandlers.gohandleUnstakehandlers.gohandleChangeCandidatehandlers.gohandleTransferStakeapplyVoterWeightDeltacalls instead of mirroring an existing site.handlers.gohandleDepositToStakehandlers.gohandleRestakehandler_candidate_endorsement.goclearCandidateSelfStakecsmargument so the hook has a view handle.handler_candidate_selfstake.gohandleCandidateActivatehandler_stake_migrate.gohandleStakeMigratenfteventhandlerhook below.nfteventhandler.go(contract V1/V2/V3)PutBucketnfteventhandler.goDeleteBucket/DeductBucketContractStakeView.Handle), so instrumenting them once covers all three indexer impls automatically.protocol.goslashCandidateself-stake-bucket-shrink pathcandidate_statemanager.gocandSM.deactivatevote_reviser.gowas intentionally not instrumented: it only fires at historical fork heights (Greenland, Hawaii) that are long past on mainnet, before IIP-59 activates. By the time IIP-59 is on,vote_reviseris a permanent no-op via itsNeedRevise/ cache short-circuit.Bug fix discovered while writing tests
candSM.DirtyView()was strippingvoterWeightsto nil, so everyApplythroughcsm.DirtyView().voterWeightswould have been a silent no-op. The hook tests caught this immediately.DirtyViewnow propagates the pointer from the base view.Side fix in
viewdata.goviewData.Commit's feature-flag lookup now usesGetFeatureCtx(returnsokbool) instead ofMustGetFeatureCtx. Some unit tests commit withcontext.Background()(TestProtocol_HandleCandidateEndorsement_RevokeSelfStakeAfterHardfork, others) — missing FeatureCtx is treated as pre-fork:persistSMstays nil and the view's dirty flag is cleared without touching the state trie.Test plan
go build ./...passesvoter_weight_hooks_test.go(3 tests):applyVoterWeightDelta./action/protocol/staking/package: 252 / 254 pass (the two remaining failures are the pre-existing flakyTestProtocol_FetchBucketAndValidatebug: TestProtocol_FetchBucketAndValidate flaky in staking test suite #4813 unrelated to this PR)./action/...sweep passesBehavior gating
voterWeights == nil; allapplyVoterWeightDeltacalls are no-op; no state-trie writes; no per-block work added. Byte-identical to today.viewData.Commitpersists the digest once the view becomes dirty.distributeVoterReward.🤖 Generated with Claude Code