From 516b5ec1b12ddb3f7b9ab3cc12bed0496b56ec7e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 15:28:01 +0000 Subject: [PATCH 1/2] feat(wallet): show wasm_memory_persistence upgrade options on change canister requests The station (#634), the dfx-orbit CLI and the wallet Install form (#641) already accept `wasm_memory_persistence` and `skip_pre_upgrade` on external canister upgrades, but reviewers had no readable view of them: `ChangeExternalCanister` requests rendered through the generic `UnsupportedOperation` JSON dump in both the request list and the request detail dialog. Add a dedicated `ChangeExternalCanisterOperation` request view that shows the target canister (resolving its name in detail mode when possible), the install mode, the Wasm memory persistence and skip-pre-upgrade options of an upgrade, and the module/argument checksums. The compact list view only surfaces the upgrade options when they were explicitly set, while the detail view always shows the effective values so approvers can see whether main memory will be kept or replaced. Also document the upgrade options in the external canisters user guide. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019wAX3aCg6ov8rMvYeBxbgK --- .../requests/RequestDetailView.spec.ts | 38 +++- .../components/requests/RequestDetailView.vue | 3 +- .../components/requests/RequestListItem.vue | 3 +- .../ChangeExternalCanisterOperation.spec.ts | 196 ++++++++++++++++++ .../ChangeExternalCanisterOperation.vue | 178 ++++++++++++++++ .../content/docs/users/external-canisters.md | 9 +- 6 files changed, 421 insertions(+), 6 deletions(-) create mode 100644 apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts create mode 100644 apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue diff --git a/apps/wallet/src/components/requests/RequestDetailView.spec.ts b/apps/wallet/src/components/requests/RequestDetailView.spec.ts index cc2474220..6aa2706c3 100644 --- a/apps/wallet/src/components/requests/RequestDetailView.spec.ts +++ b/apps/wallet/src/components/requests/RequestDetailView.spec.ts @@ -1,8 +1,11 @@ -import { describe, expect, it } from 'vitest'; +import { Principal } from '@dfinity/principal'; +import { describe, expect, it, vi } from 'vitest'; import { mount } from '~/test.utils'; import RequestDetailView from './RequestDetailView.vue'; +import ChangeExternalCanisterOperation from './operations/ChangeExternalCanisterOperation.vue'; import { variantIs } from '~/utils/helper.utils'; import { useStationStore } from '~/stores/station.store'; +import { services } from '~/plugins/services.plugin'; import { flushPromises } from '@vue/test-utils'; type RequestDetailViewProps = InstanceType['$props']; @@ -250,7 +253,40 @@ const cancelledProps: RequestDetailViewProps = { }, }; +const changeCanisterProps: RequestDetailViewProps = { + details: pendingProps.details, + request: { + ...pendingProps.request, + operation: { + ChangeExternalCanister: { + canister_id: Principal.fromText('rrkah-fqaaa-aaaaa-aaaaq-cai'), + mode: { + upgrade: [{ wasm_memory_persistence: [{ keep: null }], skip_pre_upgrade: [] }], + }, + module_checksum: 'a'.repeat(64), + arg_checksum: [], + }, + }, + }, +}; + describe('RequestDetailView', () => { + it('renders change canister requests with their upgrade options', async () => { + vi.spyOn(services().station, 'getExternalCanisterByCanisterId').mockRejectedValueOnce( + new Error('not found'), + ); + + const wrapper = mount(RequestDetailView, { + props: changeCanisterProps, + }); + await flushPromises(); + + expect(wrapper.findComponent(ChangeExternalCanisterOperation).exists()).toBe(true); + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').text()).toBe( + 'Keep', + ); + }); + it('renders properly', () => { const wrapper = mount(RequestDetailView, { props: pendingProps, diff --git a/apps/wallet/src/components/requests/RequestDetailView.vue b/apps/wallet/src/components/requests/RequestDetailView.vue index 9d37e793c..7432d6fb4 100644 --- a/apps/wallet/src/components/requests/RequestDetailView.vue +++ b/apps/wallet/src/components/requests/RequestDetailView.vue @@ -257,6 +257,7 @@ import AddRequestPolicyOperation from './operations/AddRequestPolicyOperation.vu import AddUserGroupOperation from './operations/AddUserGroupOperation.vue'; import AddUserOperation from './operations/AddUserOperation.vue'; import CallExternalCanisterOperation from './operations/CallExternalCanisterOperation.vue'; +import ChangeExternalCanisterOperation from './operations/ChangeExternalCanisterOperation.vue'; import EditAccountOperation from './operations/EditAccountOperation.vue'; import EditAddressBookEntryOperation from './operations/EditAddressBookEntryOperation.vue'; import EditPermissionOperation from './operations/EditPermissionOperation.vue'; @@ -318,12 +319,12 @@ const componentsMap: { EditAsset: EditAssetOperation, RemoveAsset: RemoveAssetOperation, CallExternalCanister: CallExternalCanisterOperation, + ChangeExternalCanister: ChangeExternalCanisterOperation, AddNamedRule: AddNamedRuleOperation, EditNamedRule: EditNamedRuleOperation, RemoveNamedRule: RemoveNamedRuleOperation, SetDisasterRecovery: SetDisasterRecoveryOperation, - ChangeExternalCanister: UnsupportedOperation, CreateExternalCanister: UnsupportedOperation, ConfigureExternalCanister: UnsupportedOperation, FundExternalCanister: UnsupportedOperation, diff --git a/apps/wallet/src/components/requests/RequestListItem.vue b/apps/wallet/src/components/requests/RequestListItem.vue index a9b1d0fef..02f967f22 100644 --- a/apps/wallet/src/components/requests/RequestListItem.vue +++ b/apps/wallet/src/components/requests/RequestListItem.vue @@ -51,6 +51,7 @@ import AddRequestPolicyOperation from './operations/AddRequestPolicyOperation.vu import AddUserGroupOperation from './operations/AddUserGroupOperation.vue'; import AddUserOperation from './operations/AddUserOperation.vue'; import CallExternalCanisterOperation from './operations/CallExternalCanisterOperation.vue'; +import ChangeExternalCanisterOperation from './operations/ChangeExternalCanisterOperation.vue'; import EditAccountOperation from './operations/EditAccountOperation.vue'; import EditAddressBookEntryOperation from './operations/EditAddressBookEntryOperation.vue'; import EditPermissionOperation from './operations/EditPermissionOperation.vue'; @@ -108,6 +109,7 @@ const componentsMap: { EditPermission: EditPermissionOperation, ManageSystemInfo: ManageSystemInfoOperation, CallExternalCanister: CallExternalCanisterOperation, + ChangeExternalCanister: ChangeExternalCanisterOperation, AddAsset: AddAssetOperation, EditAsset: EditAssetOperation, RemoveAsset: RemoveAssetOperation, @@ -117,7 +119,6 @@ const componentsMap: { SetDisasterRecovery: SetDisasterRecoveryOperation, // below variants are not supported yet - ChangeExternalCanister: UnsupportedOperation, CreateExternalCanister: UnsupportedOperation, ConfigureExternalCanister: UnsupportedOperation, FundExternalCanister: UnsupportedOperation, diff --git a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts new file mode 100644 index 000000000..1d510dbd7 --- /dev/null +++ b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts @@ -0,0 +1,196 @@ +import { Principal } from '@dfinity/principal'; +import { flushPromises } from '@vue/test-utils'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + CanisterInstallMode, + ChangeExternalCanisterOperation as ChangeExternalCanisterOperationDTO, + Request, +} from '~/generated/station/station.did'; +import { services } from '~/plugins/services.plugin'; +import { mount } from '~/test.utils'; +import ChangeExternalCanisterOperation from './ChangeExternalCanisterOperation.vue'; + +type ExternalCanisterLookup = Awaited< + ReturnType['station']['getExternalCanisterByCanisterId']> +>; + +const canisterId = Principal.fromText('rrkah-fqaaa-aaaaa-aaaaq-cai'); +const moduleChecksum = 'a'.repeat(64); +const argChecksum = 'b'.repeat(64); + +const operationWithMode = (mode: CanisterInstallMode): ChangeExternalCanisterOperationDTO => ({ + mode, + canister_id: canisterId, + module_checksum: moduleChecksum, + arg_checksum: [argChecksum], +}); + +const requestWith = (operation: ChangeExternalCanisterOperationDTO): Request => ({ + id: 'request-id', + title: 'Upgrade canister', + summary: [], + status: { Created: null }, + approvals: [], + created_at: '', + execution_plan: { Immediate: null }, + expiration_dt: '', + requested_by: 'requester-id', + tags: [], + deduplication_key: [], + operation: { ChangeExternalCanister: operation }, +}); + +const mountOperation = (operation: ChangeExternalCanisterOperationDTO, mode: 'list' | 'detail') => + mount(ChangeExternalCanisterOperation, { + props: { + request: requestWith(operation), + operation, + mode, + }, + }); + +const keepUpgrade: CanisterInstallMode = { + upgrade: [{ wasm_memory_persistence: [{ keep: null }], skip_pre_upgrade: [] }], +}; + +describe('ChangeExternalCanisterOperation', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('shows the canister id, install mode and checksums', () => { + const wrapper = mountOperation(operationWithMode({ install: null }), 'list'); + + expect(wrapper.find('[data-test-id="change-canister-target"]').text()).toBe( + canisterId.toText(), + ); + expect(wrapper.find('[data-test-id="change-canister-mode"]').text()).toBe('Install'); + expect(wrapper.find('[data-test-id="change-canister-module-checksum"]').exists()).toBe(true); + expect(wrapper.find('[data-test-id="change-canister-arg-checksum"]').exists()).toBe(true); + }); + + it('shows the full checksums in detail mode', async () => { + vi.spyOn(services().station, 'getExternalCanisterByCanisterId').mockRejectedValueOnce( + new Error('not found'), + ); + + const wrapper = mountOperation(operationWithMode({ reinstall: null }), 'detail'); + await flushPromises(); + + expect(wrapper.find('[data-test-id="change-canister-mode"]').text()).toBe('Reinstall'); + expect(wrapper.find('[data-test-id="change-canister-module-checksum"]').text()).toBe( + moduleChecksum, + ); + expect(wrapper.find('[data-test-id="change-canister-arg-checksum"]').text()).toBe(argChecksum); + }); + + it('omits the argument checksum row when the request has no argument', () => { + const wrapper = mountOperation( + { ...operationWithMode({ install: null }), arg_checksum: [] }, + 'list', + ); + + expect(wrapper.find('[data-test-id="change-canister-arg-checksum"]').exists()).toBe(false); + }); + + it('shows the wasm memory persistence of an upgrade in list mode when it is set', () => { + const wrapper = mountOperation(operationWithMode(keepUpgrade), 'list'); + + expect(wrapper.find('[data-test-id="change-canister-mode"]').text()).toBe('Upgrade'); + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').text()).toBe( + 'Keep', + ); + expect(wrapper.text()).toContain('Wasm Memory Persistence'); + }); + + it('shows the skip pre-upgrade flag of an upgrade when it is set', () => { + const wrapper = mountOperation( + operationWithMode({ + upgrade: [{ wasm_memory_persistence: [{ replace: null }], skip_pre_upgrade: [true] }], + }), + 'list', + ); + + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').text()).toBe( + 'Replace', + ); + expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').text()).toBe('Yes'); + }); + + it('hides the upgrade options in list mode when the upgrade does not set them', () => { + const wrapper = mountOperation(operationWithMode({ upgrade: [] }), 'list'); + + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').exists()).toBe( + false, + ); + expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').exists()).toBe(false); + }); + + it('shows the effective upgrade options in detail mode for a plain upgrade', async () => { + vi.spyOn(services().station, 'getExternalCanisterByCanisterId').mockRejectedValueOnce( + new Error('not found'), + ); + + const wrapper = mountOperation(operationWithMode({ upgrade: [] }), 'detail'); + await flushPromises(); + + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').text()).toBe( + 'Default (replace)', + ); + expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').text()).toBe('No'); + }); + + it('does not show upgrade options for install and reinstall requests', async () => { + vi.spyOn(services().station, 'getExternalCanisterByCanisterId').mockRejectedValue( + new Error('not found'), + ); + + for (const mode of [{ install: null }, { reinstall: null }] as CanisterInstallMode[]) { + const wrapper = mountOperation(operationWithMode(mode), 'detail'); + await flushPromises(); + + expect( + wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').exists(), + ).toBe(false); + expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').exists()).toBe( + false, + ); + } + }); + + it('resolves the canister name in detail mode', async () => { + const lookup = vi + .spyOn(services().station, 'getExternalCanisterByCanisterId') + .mockResolvedValueOnce({ canister: { name: 'Backend' } } as ExternalCanisterLookup); + + const wrapper = mountOperation(operationWithMode(keepUpgrade), 'detail'); + await flushPromises(); + + expect(lookup).toHaveBeenCalledWith(canisterId); + expect(wrapper.find('[data-test-id="change-canister-target"]').text()).toBe( + `Backend (${canisterId.toText()})`, + ); + }); + + it('falls back to the canister id when the canister cannot be resolved', async () => { + vi.spyOn(services().station, 'getExternalCanisterByCanisterId').mockRejectedValueOnce( + new Error('not found'), + ); + + const wrapper = mountOperation(operationWithMode(keepUpgrade), 'detail'); + await flushPromises(); + + expect(wrapper.find('[data-test-id="change-canister-target"]').text()).toBe( + canisterId.toText(), + ); + }); + + it('does not look up the canister in list mode', async () => { + const lookup = vi.spyOn(services().station, 'getExternalCanisterByCanisterId'); + + mountOperation(operationWithMode(keepUpgrade), 'list'); + await flushPromises(); + + expect(lookup).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue new file mode 100644 index 000000000..447edf3b7 --- /dev/null +++ b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue @@ -0,0 +1,178 @@ + + + diff --git a/docs/src/content/docs/users/external-canisters.md b/docs/src/content/docs/users/external-canisters.md index 94a780cfd..e9bce2a74 100644 --- a/docs/src/content/docs/users/external-canisters.md +++ b/docs/src/content/docs/users/external-canisters.md @@ -53,9 +53,12 @@ Regularly review canister permissions to prevent unauthorized access. The permis - **Install:** Install a new WASM file into the canister - **Upgrade:** Replace the existing canister code with a new version. - **Reinstall:** Reinstall the canister with the same code. -3. Upload the WASM file and submit the request. -4. Wait for the approval process (if multi-signature policies are enabled). -5. Monitor the canister status to ensure the upgrade completes successfully. +3. For the **Upgrade** mode, review the additional upgrade options: + - **Wasm Memory Persistence:** Controls whether the canister's main memory is kept or replaced during the upgrade. Leave it on _Default (replace)_ for most canisters. Motoko canisters that use [Enhanced Orthogonal Persistence](https://docs.internetcomputer.org/motoko/orthogonal-persistence/enhanced) must use **Keep**, otherwise the Internet Computer rejects the upgrade to protect their memory. + - **Skip pre-upgrade hook:** Skips the canister's `pre_upgrade` hook. Only useful for recovery when the existing hook traps. +4. Upload the WASM file and submit the request. +5. Wait for the approval process (if multi-signature policies are enabled). +6. Monitor the canister status to ensure the upgrade completes successfully. :::caution[Important] Always test new versions in a staging environment before upgrading production canisters. From d52e5cecca74782ce7e72cc2749fa3463ccb531b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 17:07:13 +0000 Subject: [PATCH 2/2] fix(wallet): show explicit skip_pre_upgrade=false and resolve target names with a verified call Address review findings on the change canister request view: - The compact list view decided whether to show the skip-pre-upgrade row from the flag's truthiness, so a request that explicitly set `skip_pre_upgrade = false` hid the row instead of showing "No". Check whether the optional field is present instead, and cover it in the spec. - The target canister name shown to approvers was fetched with a plain query while the request itself is loaded through a verified call. Fetch the name with a verified call as well so a non-consensus response cannot attach a misleading name to the verified canister id. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019wAX3aCg6ov8rMvYeBxbgK --- .../ChangeExternalCanisterOperation.spec.ts | 17 ++++++++++++++++- .../ChangeExternalCanisterOperation.vue | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts index 1d510dbd7..7d3877c86 100644 --- a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts +++ b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.spec.ts @@ -117,6 +117,20 @@ describe('ChangeExternalCanisterOperation', () => { expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').text()).toBe('Yes'); }); + it('shows an explicitly disabled skip pre-upgrade flag in list mode', () => { + const wrapper = mountOperation( + operationWithMode({ + upgrade: [{ wasm_memory_persistence: [], skip_pre_upgrade: [false] }], + }), + 'list', + ); + + expect(wrapper.find('[data-test-id="change-canister-skip-pre-upgrade"]').text()).toBe('No'); + expect(wrapper.find('[data-test-id="change-canister-wasm-memory-persistence"]').exists()).toBe( + false, + ); + }); + it('hides the upgrade options in list mode when the upgrade does not set them', () => { const wrapper = mountOperation(operationWithMode({ upgrade: [] }), 'list'); @@ -166,7 +180,8 @@ describe('ChangeExternalCanisterOperation', () => { const wrapper = mountOperation(operationWithMode(keepUpgrade), 'detail'); await flushPromises(); - expect(lookup).toHaveBeenCalledWith(canisterId); + // The name is resolved with a verified call, like the request itself. + expect(lookup).toHaveBeenCalledWith(canisterId, true); expect(wrapper.find('[data-test-id="change-canister-target"]').text()).toBe( `Backend (${canisterId.toText()})`, ); diff --git a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue index 447edf3b7..d2d886785 100644 --- a/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue +++ b/apps/wallet/src/components/requests/operations/ChangeExternalCanisterOperation.vue @@ -112,7 +112,13 @@ const wasmMemoryPersistence = computed( () => upgradeOptions.value?.wasm_memory_persistence[0], ); -const skipPreUpgrade = computed(() => upgradeOptions.value?.skip_pre_upgrade[0] ?? false); +// `undefined` when the request does not set the option, which differs from an +// explicit `false`. +const skipPreUpgradeOption = computed( + () => upgradeOptions.value?.skip_pre_upgrade[0], +); + +const skipPreUpgrade = computed(() => skipPreUpgradeOption.value ?? false); // The compact list view only surfaces the upgrade options when they were // explicitly set on the request, while the detail view always shows the @@ -122,7 +128,7 @@ const showWasmMemoryPersistence = computed( ); const showSkipPreUpgrade = computed( - () => isUpgrade.value && (!isListMode.value || skipPreUpgrade.value), + () => isUpgrade.value && (!isListMode.value || skipPreUpgradeOption.value !== undefined), ); const wasmMemoryPersistenceLabel = computed(() => { @@ -152,8 +158,12 @@ const canisterLabel = computed(() => { const loadCanisterName = async (): Promise => { try { + // The name is shown to approvers next to the canister id as the target of the + // change, so it is fetched with a verified (certified) call like the request + // itself rather than a plain query. const result = await station.service.getExternalCanisterByCanisterId( props.operation.canister_id, + true, ); canisterName.value = result.canister.name;