feat(governance): clear voting power snapshots on Mission 70 activation#9731
feat(governance): clear voting power snapshots on Mission 70 activation#9731jasonz-dfinity wants to merge 1 commit intomasterfrom
Conversation
When Mission 70 activates, voting power distribution changes significantly (convex dissolve delay bonus, reduced max dissolve delay). Stale snapshots from before activation would cause false spike detections, since the new voting power totals will differ substantially from the old ones. - Add VotingPowerSnapshots::clear() method that empties both stable BTreeMaps - Call it during governance initialization when the Mission 70 flag is enabled - Add a log line when a voting power spike is detected for observability - Add unit test for clear() verifying both maps are emptied
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
There was a problem hiding this comment.
Pull request overview
This PR addresses Mission 70 governance rollout by resetting persisted voting power snapshot state to avoid false-positive spike detection when voting power distribution shifts at activation.
Changes:
- Added
VotingPowerSnapshots::clear()to empty both snapshot stable maps. - Cleared voting power snapshots during governance restore when the Mission 70 flag is enabled.
- Added a log line when a voting power spike is detected, plus a unit test for
clear().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rs/nns/governance/src/governance/voting_power_snapshots.rs | Adds spike observability log and a clear() method to reset snapshot storage. |
| rs/nns/governance/src/governance/voting_power_snapshots_tests.rs | Adds a unit test intended to validate clear() behavior. |
| rs/nns/governance/src/governance.rs | Invokes snapshot clearing during Governance::new_restored when Mission 70 is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if is_mission_70_voting_rewards_enabled() { |
There was a problem hiding this comment.
VOTING_POWER_SNAPSHOTS is cleared on every upgrade as long as is_mission_70_voting_rewards_enabled() remains true. That means post‑Mission‑70 snapshots get wiped repeatedly, temporarily disabling spike detection after each upgrade and potentially losing useful history. Consider making this a one-time migration by piggybacking on the existing Mission 70 idempotency guard above (e.g., clear snapshots only inside the block that populates neuron_id_to_pre_clamp_dissolve_state, or add a dedicated persisted flag).
| } | |
| if is_mission_70_voting_rewards_enabled() { |
| // Verify that snapshots are present. | ||
| assert_eq!(snapshots.latest_snapshot_timestamp_seconds(), Some(3)); | ||
|
|
||
| // Clear the snapshots. | ||
| snapshots.clear(); | ||
|
|
||
| // Verify that everything is empty. | ||
| assert_eq!(snapshots.latest_snapshot_timestamp_seconds(), None); | ||
| assert_eq!( | ||
| snapshots.previous_ballots_if_voting_power_spike_detected(u64::MAX, 10), | ||
| None | ||
| ); | ||
| assert!(!snapshots.is_latest_snapshot_a_spike(10)); |
There was a problem hiding this comment.
This test doesn’t actually verify that both underlying stable maps were emptied: latest_snapshot_timestamp_seconds() only reflects voting_power_totals, and previous_ballots_if_voting_power_spike_detected can return None even when maps aren’t full in feature="test" mode. Since this test module can access private fields, add assertions on snapshots.neuron_id_to_voting_power_maps.len() and snapshots.voting_power_totals.len() being 0 after clear() (and/or assert both are non-zero before calling clear()).
Summary
VotingPowerSnapshots::clear()method that empties both stable BTreeMapsclear()verifying both maps are emptiedWhy
When Mission 70 activates, voting power distribution changes significantly (convex dissolve delay bonus, reduced max dissolve delay). Stale snapshots from before activation would cause false spike detections, since the new voting power totals will differ substantially from the old ones.
Test plan
VotingPowerSnapshots::clear()verifying both maps are emptied