chore(nns): remove eight year gang bonus base migration logic#10108
Conversation
The strict and relaxed eight year gang bonus base migrations have already run on mainnet, so the migration code, the migration_done flags, and supporting constants/tests are no longer needed. Proto field numbers 31 (eight_year_gang_bonus_migration_done) and 33 (relaxed_eight_year_gang_bonus_migration_done) are now reserved.
There was a problem hiding this comment.
Pull request overview
This PR removes now-dead “eight year gang bonus base” one-time migration logic from NNS Governance after confirming the strict and relaxed migrations have already executed on mainnet, and updates the persisted protobuf schema accordingly.
Changes:
- Removed the strict + relaxed migration implementations and their call site during
Governance::new_restored. - Deleted the migration-only helper APIs from the neuron stores and removed migration-only tests.
- Dropped the persisted
*_migration_doneflags and reserved protobuf field numbers31and33to preserve schema compatibility.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
rs/nns/governance/unreleased_changelog.md |
Documents removal of the one-time migrations and protobuf field reservation. |
rs/nns/governance/src/storage/neurons/neurons_tests.rs |
Removes migration-only tests and now-unused import. |
rs/nns/governance/src/storage/neurons.rs |
Deletes stable neuron store migration helpers and related imports. |
rs/nns/governance/src/neuron_store/neuron_store_tests.rs |
Removes relaxed-migration-only test and now-unused imports. |
rs/nns/governance/src/neuron_store.rs |
Deletes migration helper methods and related imports/constants usage. |
rs/nns/governance/src/heap_governance_data.rs |
Removes migration flags from heap governance state split/reassemble paths and tests. |
rs/nns/governance/src/governance/tests/mod.rs |
Removes migration-specific tests and now-unused imports. |
rs/nns/governance/src/governance.rs |
Removes migration constants and the post-upgrade migration entry points. |
rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs |
Updates generated protobuf Rust bindings to drop the deprecated fields. |
rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto |
Removes deprecated fields and reserves tags/names to prevent reuse. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Why
The strict and the relaxed eight year gang bonus base migrations have already
run on mainnet (proposals 141441 and 141565). Migrations don't re-run, so the
migration code and supporting constants/tests are dead weight.
What
Governance::maybe_set_eight_year_gang_bonus_baseandmaybe_set_relaxed_eight_year_gang_bonus_base_e8s_or_panic, plus the callsite in
Governance::new_restored.set_eight_year_gang_bonus_base_e8s_for_all_neurons_or_panicand
set_relaxed_eight_year_gang_bonus_base_e8s_or_panichelpers fromNeuronStoreandStableNeuronStore.RELAXED_EIGHT_YEAR_GANG_MIN_DISSOLVE_DELAY_SECONDSandRELAXED_EIGHT_YEAR_GANG_MAX_AGING_SINCE_TIMESTAMP_SECONDS.(
test_maybe_set_eight_year_gang_bonus_base,test_maybe_set_relaxed_eight_year_gang_bonus_base,test_set_eight_year_gang_bonus_base_e8s_for_all_neurons,test_set_relaxed_eight_year_gang_bonus_base_e8s).eight_year_gang_bonus_base_e8sis still maintained on the regular path: newneurons default it to 0,
start_dissolvingzeroes it, and split/merge keepsit correctly partitioned across parent/child neurons.
Rollback safety
The two
*_migration_doneflags are kept onHeapGovernanceDataand in theproto as persisted-true rollback guards. If we ever rolled back to a release
that still contains the migration code, the strict migration would otherwise
re-run against post-Mission-70 (clamped ≤ 2-year) dissolve delays, hit the
elsebranch for every neuron, and zero out every neuron'seight_year_gang_bonus_base_e8s. Keeping the flags persisted astruemakesthe previous-release migration short-circuit on rollback.