Skip to content

feat(NNS): add snapshot visibility settings#9257

Merged
gregorydemay merged 20 commits intomasterfrom
gdemay/DEFI-2667-snapshot-visibility-governance
Mar 25, 2026
Merged

feat(NNS): add snapshot visibility settings#9257
gregorydemay merged 20 commits intomasterfrom
gdemay/DEFI-2667-snapshot-visibility-governance

Conversation

@gregorydemay
Copy link
Copy Markdown
Contributor

@gregorydemay gregorydemay commented Mar 9, 2026

Summary

🤖 Generated with Claude Code

@gregorydemay gregorydemay changed the base branch from master to gdemay/DEFI-2667-snapshot-visibility-settings March 9, 2026 13:03
@gregorydemay gregorydemay force-pushed the gdemay/DEFI-2667-snapshot-visibility-governance branch from a0920dd to 7981151 Compare March 9, 2026 13:11
@gregorydemay gregorydemay force-pushed the gdemay/DEFI-2667-snapshot-visibility-governance branch from 7981151 to 4e09f94 Compare March 9, 2026 13:20
@gregorydemay gregorydemay added the CI_ALL_BAZEL_TARGETS Runs all bazel targets label Mar 9, 2026
Comment thread rs/nervous_system/clients/src/update_settings.rs
Comment thread rs/nns/nns.bzl
github-merge-queue Bot pushed a commit that referenced this pull request Mar 11, 2026
Add the types needed for canisters' snapshot visibility settings, to
allow principals other than the canister's controllers to be able to
list and read canister's snaphots. In short this is like log visibility
settings but for canister's snapshots. This PR only adds the needed
types and does not change any existing logic.

Support for snapshot visibility settings for NNS-owned canisters will be
done in #9257. Note that the changes regarding the CMC are needed in
this PR because the CMC exposes
`ic_management_canister_types_private::CanisterSettingsArgs` via the
type `CreateCanister` which is an argument of the `create_canister`
endpoint and the test `test_candid_interface_compatibility` ensures that
the exposed Canister API is equal to the one declared in `cmc.did`.

Sequence of PRs to implement canister snapshot visibility settings as
specified in dfinity/portal#6195:
1. #9155 
2. #9158 (this)
3.  #9227

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
Base automatically changed from gdemay/DEFI-2667-snapshot-visibility-settings to master March 11, 2026 19:39
…nance

# Conflicts:
#	rs/cycles_account_manager/src/lib.rs
#	rs/nns/cmc/cmc.did
#	rs/types/management_canister_types/src/lib.rs
@gregorydemay gregorydemay marked this pull request as ready for review March 16, 2026 16:41
@gregorydemay gregorydemay requested a review from a team as a code owner March 16, 2026 16:41
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

  5. 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

Copy link
Copy Markdown
Contributor

@daniel-wong-dfinity-org daniel-wong-dfinity-org left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me sad. I mean, the problem is not in this PR itself, ofc. This PR navigates the mess in a locally reasonable way. The reason this makes me sad is that it shows us the insane number of hoops one needs to jump through in order to achieve something simple. I mean, I already knew that, and I have been trying (and failing) to resist this horrible slide into the classic Google meme:

https://imgflip.com/i/amyalx

rant.terminate()

Comment thread rs/nervous_system/clients/src/update_settings.rs
Comment thread rs/nns/governance/src/proposals/update_canister_settings.rs Outdated
Comment thread rs/nns/nns.bzl
Copy link
Copy Markdown
Contributor Author

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @daniel-wong-dfinity-org for the review and sorry for long time between your review and the 2nd iteration.

Comment thread rs/nervous_system/clients/src/update_settings.rs
Comment thread rs/nns/governance/src/proposals/update_canister_settings.rs Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gregorydemay gregorydemay enabled auto-merge March 25, 2026 08:57
@gregorydemay gregorydemay added this pull request to the merge queue Mar 25, 2026
Merged via the queue into master with commit 1855b0f Mar 25, 2026
38 checks passed
@gregorydemay gregorydemay deleted the gdemay/DEFI-2667-snapshot-visibility-governance branch March 25, 2026 10:15
alin-at-dfinity pushed a commit that referenced this pull request Mar 26, 2026
## Summary
- Follow-up on #9158 to add `snapshot_visibility` support to
governance-team owned canisters (NNS, SNS, CMC, registry admin)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants