feat(sns): support upgrade options (wasm_memory_persistence, skip_pre_upgrade) in UpgradeSnsControlledCanister - #11076
Draft
claude[bot] wants to merge 2 commits into
Draft
feat(sns): support upgrade options (wasm_memory_persistence, skip_pre_upgrade) in UpgradeSnsControlledCanister#11076claude[bot] wants to merge 2 commits into
claude[bot] wants to merge 2 commits into
Conversation
…_upgrade) in UpgradeSnsControlledCanister Ports the NNS Governance/Root "canister upgrade options" plumbing (#10952, #10979, #11027) to SNS Governance, SNS CLI, and the already-shared ic_nervous_system_root::change_canister code. - Adds UpgradeSnsControlledCanister.CanisterUpgradeOptions (skip_pre_upgrade, wasm_memory_persistence) to the SNS Governance proto/candid interface, mirroring InstallCode.CanisterUpgradeOptions in NNS Governance. - Threads the options through proposal validation (UpgradeSnsControlledCanister::upgrade_options), execution (Governance::upgrade_non_root_canister / assemble_mode), and into CanisterInstallModeV2::Upgrade(Some(_)) when calling SNS Root's change_canister (SNS Root's .did/change_canister already gained support for this via the shared ic_nervous_system_root crate in #10952). - Adds --skip-pre-upgrade / --wasm-memory-persistence flags to `sns upgrade-sns-controlled-canister`, mirroring ic-admin's propose-to-change-nns-canister flags. - Adds unit tests for the new validation/conversion logic and for proposal-data redaction (limited_for_get_proposal / limited_for_list_proposals) preserving canister_upgrade_options. Requested by Björn Tackmann. Original NNS work driven by Daniel Wong. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014kJkdE1XdE2VG9YwfbAhqK
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
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.
Requested by Björn Tackmann · Slack thread
What
SNS Governance's
UpgradeSnsControlledCanisterproposal had no way to specify the EOP (Enhanced Orthogonal Persistence) upgrade optionsskip_pre_upgradeandwasm_memory_persistencewhen upgrading an SNS-controlled canister. This PR ports that support from NNS, mirroring the already-merged NNS PRs #10952, #10979, and #11027.Before:
UpgradeSnsControlledCanisterproposals could only specify the target canister, new WASM, and canister upgrade args — no control over pre-upgrade skipping or Wasm memory persistence mode.After: the proposal (and its API/candid/proto representations) carries new optional upgrade-options fields (mirroring NNS's
ChangeCanisterRequestoptions), exposingskip_pre_upgrade: Option<bool>andwasm_memory_persistence: Option<i32>(enum), which are threaded through to the install/upgrade call to the target canister.How
rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.protoandrs/sns/governance/canister/governance.didgain the new upgrade-options fields onUpgradeSnsControlledCanister.rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs(generated),rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs,rs/sns/governance/src/pb/conversions.rs(API <-> internal conversions),rs/sns/governance/src/types.rs(validation/helpers).rs/sns/governance/src/proposal.rsandrs/sns/governance/src/governance.rsvalidate and apply the new options when executing the upgrade against the target canister, including theCanisterInstallMode/CanisterChangeDetailshandling.rs/sns/governance/src/extensions.rsupdated for consistency with the new options where extensions interact with canister upgrades.rs/sns/cli/src/upgrade_sns_controlled_canister.rsandrs/sns/cli/Cargo.toml/rs/sns/cli/BUILD.bazeladd new--skip-pre-upgrade/--wasm-memory-persistenceflags so the SNS CLI can submit proposals using the new options.rs/sns/governance/src/types/tests.rs,rs/sns/governance/src/governance/assorted_governance_tests.rs,rs/sns/integration_tests/src/upgrade_canister.rs,rs/sns/testing/src/sns.rs, andrs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rsare updated/extended to cover the new fields.rs/sns/governance/unreleased_changelog.mddocuments the change.Local validation note
cargo checkforic-sns-governancerequires a private git dependency (dfinity-lab/build-info) this dev environment isn't authorized to fetch — confirmed this also fails identically on a cleanmastercheckout, so it's a pre-existing environment limitation, not caused by this change.bazelisn't installed locally either. Relying on CI to validate the full build/test matrix.As a lighter-weight local check,
rustfmt --checkwas run against every touched.rsfile (using the repo'srustfmt.toml, edition 2024) and reported no formatting issues.Generated by Claude Code