fix(sns): Reduce heap memory usage in SNS governance#10223
fix(sns): Reduce heap memory usage in SNS governance#10223jasonz-dfinity wants to merge 1 commit into
Conversation
NNS1-4195. Free ballot capacity after reward distribution (BTreeMap::clear leaves it allocated), and extend check_heap_can_grow to all non-emergency manage_neuron commands, claim_swap_neurons, and proposal validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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 reduces SNS governance heap pressure by freeing proposal ballot storage after reward settlement and broadening low-heap rejection paths for memory-growing operations.
Changes:
- Replaces cleared proposal ballot maps with new
BTreeMaps to release allocated capacity. - Adds low-resource allowlist helpers for manage-neuron commands and proposal actions.
- Moves heap-growth checks toward
manage_neuron,claim_swap_neurons, and proposal validation paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rs/sns/governance/unreleased_changelog.md |
Documents the heap memory optimization changes. |
rs/sns/governance/src/types.rs |
Adds low-resource allowlist logic for manage-neuron commands and AdvanceSnsTargetVersion. |
rs/sns/governance/src/governance.rs |
Adjusts heap checks and releases proposal ballot backing storage after rewards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Err(err) = self.check_heap_can_grow() { | ||
| log!( | ||
| ERROR, | ||
| "Could not claim_swap_neurons due to heap growth limits. Err: {}", | ||
| err.error_message, | ||
| ); | ||
| return ClaimSwapNeuronsResponse::new_with_error(ClaimSwapNeuronsError::Internal); | ||
| } |
| if !command.allowed_when_resources_are_low() { | ||
| self.check_heap_can_grow()?; | ||
| } |
Summary
Two adjustments to optimize SNS governance heap usage.
Free ballot capacity after reward distribution
Once a proposal reward round is complete, the proposal
ballotsmap is jettisoned — butBTreeMap::clear()only removes entries while keeping the underlying capacity allocated, so the memory was not actually released. Replacing the field withBTreeMap::new()deallocates the backing storage.Broaden
check_heap_can_growcoveragecheck_heap_can_growis now enforced in more places, so the canister rejects new allocations earlier when it is close to its heap limit:manage_neuroncommand exceptMakeProposalandRegisterVote. These two must stay available so the SNS can vote on a recovery proposal even when heap is tight.claim_swap_neurons, mirroring the check that used to live inadd_neuron.AdvanceSnsTargetVersionaction (successor ofUpgradeSnsToNextVersion).The check is removed from
add_neuronandsplit_neuronbecause both are now covered transitively:split_neuronis only called frommanage_neuron, andadd_neuronis only called fromsplit_neuron,claim_neuron(also undermanage_neuron), andclaim_swap_neurons(covered directly).