fix(sns-wasm): actually overwrite existing SNS-specific upgrade path entries - #11151
fix(sns-wasm): actually overwrite existing SNS-specific upgrade path entries#11151Rachit2323 wants to merge 3 commits into
Conversation
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
|
✅ No security or compliance issues detected. Reviewed everything up to 06aec4f. Security Overview
Detected Code Changes
|
|
@Rachit2323 Since this touches governance-owned code, here are answers to the automated checklist so you can dismiss that review:
To clear the bot's review: use the "⋯" menu on its review, choose "Dismiss review", and paste answers to the 4 items (including the changelog snippet above, or your own wording) into the dismissal box. Generated by Claude Code |
|
|
@Rachit2323 quick clarification — item 1 needs an actual commit adding that entry to Generated by Claude Code |
573426c to
f901320
Compare
|
@Rachit2323 thanks for adding the changelog entry — one correction needed: it names the wrong function. The PR fixes Generated by Claude Code |
|
Run on //rs/migration_canister:migration_canister_test URL: https://github.com/dfinity/ic/actions/runs/32058856840 |
|
@Rachit2323 heads up — master moved (a routine changelog release cut the old entries in Generated by Claude Code |
eb54038 to
06aec4f
Compare
Problem
insert_sns_specific_upgrade_path_entryis used to set a one-off emergency upgrade step for a specific SNS (e.g. "if this SNS is stuck at version A, send it to version B instead"). If you try to change that step later (e.g. because B turned out to be wrong, so it should now go to C), the function only logs that it is "being overwritten" but never actually writes the new value. The old entry stays in place, while the caller is told the request succeeded.Fix
Actually write the new value when overwriting an existing entry, the same way the sibling function
insert_upgrade_path_entryalready does a few lines below it.Testing
Added
test_insert_sns_specific_upgrade_path_entry_overwrites_existing_entry, which inserts an entry, overwrites it with a different target, and checks that the second value is the one actually returned. This test fails on the old code and passes with the fix.