feat(staking,poll): incremental voter weight view + snapshot at PutPollResult (IIP-59 PR 2/6)#4866
feat(staking,poll): incremental voter weight view + snapshot at PutPollResult (IIP-59 PR 2/6)#4866envestcc wants to merge 2 commits into
Conversation
… (IIP-59 PR 1/6) First PR of the IIP-59 protocol-native voter reward distribution series (spec: iotexproject/iips#73). Introduces the on-chain delegate opt-in surface — a commissionRate field on the candidate schema plus the SetCommissionRate action delegates use to change it. No reward-distribution logic yet: that lands in PR 3, gated on the same feature flag as this PR. Chain behavior is unchanged pre-flag. Schema ------ - stakingpb.Candidate gains commissionRate=11; Go Candidate struct mirrors it. Equal / Clone / toProto / fromProto updated (the PoC at iotexproject#4811 missed Equal — flagged in review #2 there). - state.Candidate gains CommissionRate. Populated per epoch from staking.Candidate by PutPollResult in PR 2; the latest user-set value lives on staking.Candidate, and state.Candidate holds the frozen per-epoch value consumed by GrantEpochReward in PR 3. - iotextypes.Candidate.commissionRate is set/read in candidateToPb / pbToCandidate so the field travels through poll snapshots and RPC. 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). - Named so the bool zero value (false) corresponds to the post-fork activated behavior, matching NoCandidateExitQueue / NotSlashUnproductiveDelegates. Action (SetCommissionRate) -------------------------- - action/set_commission_rate.go: SetCommissionRate{rate uint64} with IntrinsicGas (10000) and SanityCheck (rate in [0, 10000]). - Full Proto / LoadProto / FillAction wiring for the iotex-proto oneof slot (setCommissionRate = 56) shipped in iotex-proto v0.6.7 (iotexproject/iotex-proto#174). - EthCompatibleAction + NewSetCommissionRateFromABIBinary so MetaMask / hardhat can submit via the same path as candidateActivate / candidateDeactivate. - PackCommissionRateSetEvent helper producing keccak-anchored topic-0 + indexed candidate address for the receipt log indexers subscribe to. - action/native_staking_contract_interface.sol + native_staking_contract_abi.json: declare \`function setCommissionRate(uint64 rate)\` and \`event CommissionRateSet(address indexed candidate, uint64 newRate)\` so external tooling has an ABI to bind against. Handler (staking) ----------------- - action/protocol/staking/handler_set_commission_rate.go: resolves the caller to a registered candidate by owner, writes candidate.CommissionRate, emits CommissionRateSet on the receipt. No cooldown enforcement — voters already get a ~1.5 epoch reaction window from the PutPollResult snapshot (§3.4 of the IIP), so a separate per-rate-change gate is not required. - validations.go: rejects SetCommissionRate at Validate when the feature flag is off, so pre-flag the action never leaves mempool. - protocol.go: registers the action in the handler switch. Tests ----- - candidate_test.go / state/candidate_test.go: proto roundtrip + Equal / Clone coverage for the new field. - set_commission_rate_test.go: SanityCheck bounds, IntrinsicGas, ABI encode/decode roundtrip. - handler_set_commission_rate_test.go: owner check, rate cap, successful write path, pre-flag Validate rejection, receipt event encoding. go.mod ------ - Bumps github.com/iotexproject/iotex-proto to v0.6.7 for the new SetCommissionRate action + Candidate.commissionRate fields. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Holding review — reviewer flagged two architectural issues with
Both are the same root fix: move voter-weight aggregation to an incrementally-maintained Will redo PR 2 with that design on the same branch and force-push. Marking as draft in the meantime. |
ddbc478 to
4e15806
Compare
|
Force-pushed the redesigned PR 2. Summary of the delta vs. the first draft:
Updated the PR body + title to reflect the new design. Ready for another look. |
Review: incremental voter-weight view looks sound, but the self-stake state machine isn't hookedOverall the design holds up well. A few things I specifically checked and they're correct:
Blocking: three handlers that flip a bucket between voter-visible and self-stake-excluded don't emit a view deltaThe PR adds
The tell is that if err := csm.deactivate(cand, bucket, ...); err != nil { ... }
csm.ApplyVoterWeightDelta(cand.GetIdentifier(), bucket.Owner, p.calculateVoteWeight(bucket, false))
Why it matters — two layers:
This is all gated behind The fix is mechanical: add the Non-blocking
|
…llResult (IIP-59 PR 2/6) Second PR of the IIP-59 protocol-native voter reward distribution series (spec: iotexproject/iips#73, previous: PR 1 — commissionRate schema + SetCommissionRate action). Adds the per-epoch freeze that PR 3's GrantEpochReward will consume: at PutPollResult (mid-epoch snapshot of the next-epoch active delegate set), we now capture (a) the delegate's currently-set commission rate and (b) the per-voter aggregated vote weight for every delegate that will be paid next epoch. No reward-distribution logic yet — GrantEpochReward still runs the pre-IIP-59 path; the snapshot just sits in state until PR 3 reads it. Behavior is unchanged pre-flag. Design change vs. the earlier draft on this branch -------------------------------------------------- The prior version of this PR scanned all buckets at PutPollResult time to build the per-voter aggregate. That was O(all buckets) inside a hot state.db call and, worse, required reading from the contract staking indexer — a db independent of state.db — inside a consensus mutation path. Both problems are gone in this redesign: - A live in-memory VoterWeightView is maintained incrementally by the native handlers and by the contract-staking receipt-driven event dispatch. PutPollResult reads directly from the view (top-N candidates only, O(voters-per-cand)), sorts by voter address bytes, and writes a per-candidate blob. - The consensus path (Handle → contractsStake.Handle) never touches the contract staking indexer db. The view is fed through the same receipt-driven dispatch that today updates contractStakeView, via a `VoterDeltaSink` plumbed through context (no widening of ContractStakeView interface, no breakage for existing mocks/impls). - The view is memory-only. On protocol Start, `CreateBaseView` rebuilds it once by scanning current native buckets and asking each contract-staking indexer for its current per-voter aggregates. The durable half of the story is the frozen per-epoch snapshot blobs written under `_voterWeightSnap`, not the live view. Storage layout -------------- - New 1-byte namespace tag `_voterWeightSnap = 5` in the staking namespace (protocol.go), following the existing _const / _bucket / _voterIndex / _candIndex / _endorsement pattern. - Per-candidate blob key = `_voterWeightSnap || candIdentifier.Bytes()` → 21 bytes, mirroring `_voterIndex` / `_candIndex` layout. - Blob payload = `stakingpb.VoterWeightSnapshot { repeated VoterWeightEntry (bytes voter, bytes weight) }`. Entries are written pre-sorted by voter address bytes; the marshalled output is byte-deterministic given identical logical state — a load- bearing invariant for the byte-equality skip in the writer and for cross-node consensus. Live view --------- - `action/protocol/staking/voter_weight_view.go`: VoterWeightView interface + baseVoterWeightView (map[candID]map[voterID]weight) + wrapVoterWeightView (change overlay). Wrap layers a change map on top of the receiver so Snapshot returns cheaply and Revert discards the overlay. Fork deep-clones for parallel working sets. Commit folds change into base and prunes zero-weight entries. - Deterministic ordering: `Weights(cand)` returns `[]VoterWeight` sorted by voter address bytes. No map iteration reaches the consensus-visible output. - Nil / zero delta is a no-op; negative deltas model bucket exit (unstake, withdraw, ChangeCandidate off-ramp, TransferStake off- ramp). Native handler hooks -------------------- Every native handler that changes a voter bucket's contribution to a candidate calls `csm.ApplyVoterWeightDelta(cand, voter, Δ)`: - handleCreateStake: +weightedVote - handleUnstake: -weightedVote (skipped for self-stake buckets) - handleChangeCandidate: -w on prev, +w on new (skipped when the bucket was prev's self-stake bucket — it was never in the view) - handleTransferStake: -w on old voter, +w on new voter (skipped when bucket is already unstaked — it exited the view earlier) - handleDepositToStake / handleRestake: +Δw on the same (cand, voter) pair (skipped for self-stake buckets) - handleCandidateEndorsement: no-op on the voter view (self-stake gate flips SelfStakeBucketIdx but does not add/remove voter buckets) - handleStakeMigrate: -native_w on (cand, oldOwner); the new contract bucket enters the view via the contract-staking event sink below Self-stake buckets are never in the view. The exclusion rule is uniform: `ContractAddress == "" && Index == cand.SelfStakeBucketIdx` — fixes review issue iotexproject#5 on the PoC PR (iotexproject#4811) where any Index=0 contract bucket was silently treated as self-stake. Contract staking sink --------------------- - `staking/protocol.go` wraps `contractsStake.Handle` with `WithVoterDeltaSink(ctx, viewData.voterWeights)`. All V1/V2/V3 event processors get the sink through context. - `systemcontractindex/stakingindex/vote_view_handler.go` pulls the sink out of context and emits deltas on PutBucket/DeleteBucket: fresh puts emit +w; owner or candidate changes emit split (-old, +new); deletes emit -w. Muted buckets emit no delta (matching the "not in view" invariant). - All sink calls funnel through `VoterWeightView.Apply` which is the single source of truth for the aggregation. Snapshot entry point (staking) ------------------------------ - `action/protocol/staking/voter_weight_snapshot.go`: `Protocol.SnapshotForEpochReward(ctx, sm, cands)` is the public API poll calls. For each candidate in the next-epoch active list: 1. copies `staking.Candidate.CommissionRate` into `state.Candidate.CommissionRate`. poll then persists this via its existing `NxtCandidateKey` PutState — no extra state key needed, the `shiftCandidates` path already promotes next → current at the epoch boundary. 2. reads `vd.voterWeights.Weights(candID)` (already sorted), encodes with `encodeVoterWeightSnapshot`, and writes an incremental per-candidate blob under `_voterWeightSnap` — with a byte-equality skip against the existing stored blob so steady-state (voter set unchanged epoch-over-epoch) does zero writes. - Guarded on `FeatureCtx.NoVoterRewardDistribution` — a no-op pre-fork. - Empty entries after aggregation → `DelState`, not "write empty blob." `VoterWeightsFromSnapshot` returns (nil, nil) for the ErrStateNotExist path, so PR 3 sees the same result either way, but keeping state clean matches how the other candidate-scoped namespaces behave when nothing is left to store. - State read errors other than ErrStateNotExist propagate wrapped. Fixes review issue iotexproject#4 on the PoC — corruption / IO errors no longer silently drop voter entries. Reader for PR 3 --------------- - `VoterWeightsFromSnapshot(sr, candID) → ([]VoterWeight, error)`: a single sm.State call, decoded from the persisted blob. (nil, nil) for a candidate with no snapshot (either not yet activated, or all voters left). Tests ----- - `voter_weight_view_test.go` (19 cases): base apply/aggregate/read, sorting stability, wrap overlay semantics, Snapshot/Revert parity against a Fork+Apply reference, Commit fold + zero-prune, nested Wrap, per-candidate isolation, sink context plumbing. - `vote_view_handler_test.go` (9 cases, systemcontractindex/ stakingindex): PutBucket fresh / same-cand-same-owner / owner-change / candidate-change; DeleteBucket / missing-bucket / muted-bucket; nil-sink tolerance; sink integration that a fresh-then-delete sequence balances to zero after Commit. - `voter_weight_snapshot_test.go`: updated to construct a populated view rather than seeding buckets, then assert SnapshotForEpochReward writes the expected blob and byte-equality skip triggers on unchanged inputs.
4e15806 to
36e06e4
Compare
|
|
Thanks for the careful read — all three misses are real. Fixed in
Dead code. You're right — The 27 self-stake / deactivate / transfer-ownership handler tests still pass, and the full staking package (354 tests) is green. Please take another look. |



Summary
Second PR of the IIP-59 protocol-native voter reward distribution series (spec: iotexproject/iips#73). Stacked on #4865 — PR 1 introduces the
commissionRateschema +SetCommissionRateaction; this PR keeps a live per-voter weight view up to date and freezes both the commission rate and the per-voter weights per-epoch, materializing what PR 3'sGrantEpochRewardwill consume.FeatureCtx.NoVoterRewardDistributionas PR 1. Reward distribution still runs the legacy path.VoterWeightViewinviewData, kept incrementally consistent by native handlers and by the contract-staking receipt-driven event dispatchPutPollResult(mid-epoch snapshot of next-epoch active delegates), we now (a) copystaking.Candidate.CommissionRate → state.Candidate.CommissionRate— poll's existingNxtCandidateKeyPutState persists it, no extra key — and (b) read the sorted per-voter slice out of the view and write a byte-deterministic per-candidate blob under the new_voterWeightSnapstate-trie namespaceVoterWeightsFromSnapshot(sr, candID) → []VoterWeightreturns the frozen slice for one candidate.Design change vs. the first draft on this branch
The prior version of this PR scanned all buckets at
PutPollResulttime to build the aggregate. That was O(all buckets) inside a hot state.db call and — worse — read from the contract staking indexer, a db independent of state.db, inside a consensus mutation path. Both problems are gone in this redesign:handleCreateStake,handleUnstake,handleChangeCandidate,handleTransferStake,handleDepositToStake,handleRestake,handleCandidateEndorsement,handleStakeMigrate) push(cand, voter, Δw)deltas intoVoterWeightViewas they run. Contract-staking events push deltas through aVoterDeltaSinkplumbed via context (noContractStakeViewinterface widening, no mock breakage).SnapshotForEpochRewardatPutPollResultreads O(voters-per-cand) and does zero bucket iteration.Handle) never reads the contract staking indexer db. The view is fed through the same receipt-driven dispatch that already updatescontractStakeView. The view itself is memory-only; on Start,CreateBaseViewrebuilds it once from current buckets + each indexer's current per-voter aggregates. The durable half of the story is the frozen per-epoch snapshot blobs under_voterWeightSnap, not the live view.Wrap()layers a change map on top of the receiver soRevertthrows the overlay away;Fork()deep-clones for parallel working sets;Commit()folds change into base and prunes zeros).Storage layout
_voterWeightSnap = 5in the staking namespace (protocol.go), following the_const / _bucket / _voterIndex / _candIndex / _endorsementpattern._voterWeightSnap || candIdentifier.Bytes()→ 21 bytes total, mirroring_voterIndex/_candIndexlayout.stakingpb.VoterWeightSnapshot { repeated VoterWeightEntry (bytes voter, bytes weight) }. Entries pre-sorted by voter address bytes; marshalled output is byte-deterministic given identical logical state — a load-bearing invariant for consensus and for the byte-equality skip in the writer.Correctness invariants (deliberate; PoC #4811 review issues folded in)
ContractAddress == "" && Index == cand.SelfStakeBucketIdx. Fixes PoC review issue Docker build fails #5 (all contract buckets withIndex=0erroneously got self-stake weighting). Applied uniformly: self-stake buckets never enter the view, so all handler hooks that touch them skip the delta.handleUnstakeemits the removal delta; buckets already unstaked at the time ofhandleTransferStake/handleChangeCandidateare skipped because they left the view earlier.writeVoterWeightSnapshot: encode → compare to existing stored blob bytes → skip PutState when identical. Cuts write amplification in the steady state.VoterWeightsFromSnapshotreturns(nil, nil)for theErrStateNotExistpath either way, but keeping state clean matches how other candidate-scoped namespaces behave.ErrStateNotExistpropagate (wrapped). Fixes PoC review issue Implement IoTeX blockchain explorer API MVP #4 (all errors were being treated as "no buckets").Weights(cand)returns a slice sorted by voter address bytes. No map iteration reaches the consensus-visible output. Fixes PoC review issue Batch update 2019-04-27 6PM PDT #2 (map iteration would have caused block-hash divergence between nodes).Poll integration
poll/util.go setCandidates: after the shortlist is built but beforesm.PutStateunderNxtCandidateKey, callstaking.FindProtocol(...).SnapshotForEpochReward(...). Must run first — the commission-rate copy mutates the candidate list in place, and the persistedstate.Candidateneeds the frozen value.FindProtocollookup means poll still doesn't hard-depend on staking's concreteProtocoltype; the nil-check keeps legacy tests that skip staking registration working.Tests
voter_weight_view_test.go(19 cases): base apply/aggregate/read, sorting stability, wrap overlay semantics, Snapshot/Revert parity against a Fork+Apply reference, Commit fold + zero-prune, nested Wrap, per-candidate isolation, sink context plumbing, deterministic incremental updates.vote_view_handler_test.go(9 cases,systemcontractindex/stakingindex): PutBucket fresh / same-cand-same-owner / owner-change / candidate-change; DeleteBucket / missing-bucket / muted-bucket; nil-sink tolerance; sink integration that a fresh-then-delete sequence balances to zero afterCommit.voter_weight_snapshot_test.go: updated to construct a populated view directly rather than seeding buckets, then assertSnapshotForEpochRewardwrites the expected blob, the byte-equality skip triggers on unchanged inputs, and empty entriesDelStatethe blob. End-to-end key layout, encoding determinism, reader roundtrip, and error propagation are all covered.Test plan
go build ./...go vet ./action/protocol/staking/... ./action/protocol/poll/... ./systemcontractindex/stakingindex/...go test ./action/protocol/staking/... ./systemcontractindex/stakingindex/... -count=1— 390 passing (2 unrelated pre-existing gomonkey flakes inTestProtocol_FetchBucketAndValidate/validate_owner, confirmed to reproduce without this PR)go test ./action/... ./state/... -count=1— smoke passIsToBeEnabledflipped mid-test — deferred to PR 5's e2e determinism harnessSeries roadmap
SetCommissionRateactionPutPollResultNoVoterRewardDistributiongate)🤖 Generated with Claude Code