Skip to content

feat(station): support wasm_memory_persistence for external canister upgrades#634

Merged
sea-snake merged 6 commits into
mainfrom
arshavir/romantic-tesla-200e32
May 26, 2026
Merged

feat(station): support wasm_memory_persistence for external canister upgrades#634
sea-snake merged 6 commits into
mainfrom
arshavir/romantic-tesla-200e32

Conversation

@aterga

@aterga aterga commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • ChangeExternalCanisterOperationInput gains two optional fields, wasm_memory_persistence and skip_pre_upgrade, plumbed through the API → internal model → mgmt::CanisterInstallMode::Upgrade(Some(UpgradeFlags { … })).
  • Without wasm_memory_persistence = keep, Motoko canisters that use Enhanced Orthogonal Persistence cannot be safely upgraded through Orbit — the IC defaults to replace, wiping their main memory.
  • Fully backward-compatible: new fields are opt, legacy CanisterUpgradeModeArgs CBOR (empty map) still deserializes via #[serde(default)].

Motivation

The IC management canister's install_code accepts CanisterUpgradeOptions (containing wasm_memory_persistence and skip_pre_upgrade) when mode is upgrade. Until now, Orbit hard-coded this to None, which:

  • Made it impossible to safely upgrade Motoko canisters using Enhanced Orthogonal Persistence — they require wasm_memory_persistence = keep, otherwise the IC clears their main memory on upgrade.
  • Did not expose skip_pre_upgrade, useful for recovery scenarios where the existing pre_upgrade hook traps.

What changed

  • Candid API (core/station/api): new WasmMemoryPersistence variant; two new opt fields on ChangeExternalCanisterOperationInput and ChangeExternalCanisterOperation. spec.did and the wallet-generated station.did{,.js,.d.ts} mirror the additions.
  • Internal model (core/station/impl/src/models/request_operation.rs): CanisterUpgradeModeArgs carries the two flag fields (with #[serde(default)] for backward-compat with legacy stored requests). From<CanisterInstallMode> now translates to mgmt::CanisterInstallMode::Upgrade(Some(UpgradeFlags { … })) when either field is set, or Upgrade(None) otherwise.
  • Mapper (core/station/impl/src/mappers/request_operation.rs): the api↔impl conversions for the input and DTO route the flags through the upgrade variant.
  • dfx-orbit CLI (tools/dfx-orbit): passes None for both fields when constructing requests (the CLI does not yet expose CLI flags for these — that can be a follow-up).
  • Tests: five new unit tests covering mapper round-trip, mgmt translation, install/reinstall ignoring the flags, and legacy CBOR deserialization. The existing check_candid_interface test confirms spec.did matches the generated service.

Test plan

  • cargo test -p station --lib — 388 unit tests pass, including new mapper tests and check_candid_interface.
  • cargo test --workspace --lib --bins --exclude integration-tests — 651 tests pass, 0 failures.
  • cargo clippy --workspace --all-targets — clean.
  • cargo fmt --all -- --check — clean.
  • PocketIC integration tests — not run locally (no PocketIC binary present); CI will exercise them.
  • Manual smoke test: upgrade a Motoko EOP canister via Orbit with wasm_memory_persistence = Keep and confirm stable state is preserved.

🤖 Generated with Claude Code

…upgrades

ChangeExternalCanisterOperationInput gains two optional fields,
`wasm_memory_persistence` and `skip_pre_upgrade`, which are forwarded to
the IC management canister as `CanisterUpgradeOptions`. Without this,
Motoko canisters that use Enhanced Orthogonal Persistence cannot be
safely upgraded through Orbit — the IC defaults `wasm_memory_persistence`
to `replace`, wiping their main memory.

The fields are additive (`opt`) so existing callers and stored requests
remain backward-compatible; the legacy `CanisterUpgradeModeArgs` CBOR
shape (empty map) still deserializes via `#[serde(default)]`. Covered
by new unit tests for mapper round-trip, mgmt translation, and legacy
deserialization.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aterga aterga requested a review from a team as a code owner May 22, 2026 20:44
@aterga aterga requested a review from Copilot May 22, 2026 20:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR exposes the IC management canister’s install_code upgrade options through Orbit’s Station API and internal request model, enabling safe upgrades of Motoko canisters using Enhanced Orthogonal Persistence via wasm_memory_persistence = keep, and supporting skip_pre_upgrade for recovery scenarios.

Changes:

  • Extends the Station Candid API (ChangeExternalCanisterOperation{Input}) with optional wasm_memory_persistence and skip_pre_upgrade, plus a new WasmMemoryPersistence type.
  • Plumbs the new fields through Station’s internal models/mappers and translates them into mgmt::CanisterInstallMode::Upgrade(Some(UpgradeFlags { ... })) only when set.
  • Updates CLI + integration tests to populate the new fields (currently None) and regenerates wallet DID bindings.

Reviewed changes

Copilot reviewed 8 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/dfx-orbit/src/canister/install.rs Populates new optional upgrade flag fields when constructing change-canister requests (defaults to None).
tests/integration/src/external_canister_tests.rs Updates integration test request construction to include the new optional fields.
core/station/impl/src/services/system.rs Uses CanisterUpgradeModeArgs::default() (now a non-unit struct) when issuing an upgrader upgrade.
core/station/impl/src/models/request_operation.rs Adds upgrade flag fields to CanisterUpgradeModeArgs, introduces internal WasmMemoryPersistence, and maps to mgmt upgrade flags when set.
core/station/impl/src/mappers/request_operation.rs Routes new fields between API DTOs and internal models; adds unit tests for round-trip + mgmt translation + legacy CBOR deserialization.
core/station/api/src/external_canister.rs Adds wasm_memory_persistence / skip_pre_upgrade to request input and operation DTO.
core/station/api/src/common.rs Introduces API-level WasmMemoryPersistence enum with stable (lowercase) serde names.
core/station/api/spec.did Updates the canonical Candid spec with the new enum and optional fields.
apps/wallet/src/generated/station/station.did.js Regenerates JS Candid bindings to include the new types/fields.
apps/wallet/src/generated/station/station.did.d.ts Regenerates TypeScript definitions to include the new types/fields and docs.
apps/wallet/src/generated/station/station.did Regenerates wallet DID file to mirror spec.did.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aterga and others added 2 commits May 22, 2026 23:13
Field ordering produced by `dfx generate station` differs from my
hand-edited bindings; matching the generator output so the CI
`validate-did-bindings` check passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uest

CanisterInstallDialog wasn't updated for the new
`wasm_memory_persistence` / `skip_pre_upgrade` fields on
ChangeExternalCanisterOperationInput; passing empty `[]` (None) for
both, matching the existing integration tests and dfx-orbit CLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread core/station/api/spec.did Outdated
aterga and others added 2 commits May 26, 2026 09:28
Per reviewer feedback, the upgrade flags `wasm_memory_persistence` and
`skip_pre_upgrade` now live inside the `upgrade` variant of
`CanisterInstallMode` (as `opt CanisterUpgradeOptionsInput`), matching
the IC management canister's shape. The flat fields previously added to
`ChangeExternalCanisterOperationInput` and `ChangeExternalCanisterOperation`
are removed.

This is a wire-incompatible Candid change to `CanisterInstallMode`
(the `upgrade` tag goes from unit to `opt CanisterUpgradeOptionsInput`)
but the API surface is otherwise unchanged. The internal storage shape
(`CanisterUpgradeModeArgs` with `#[serde(default)]` fields) is
unchanged, so legacy stored requests still deserialize.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per reviewer feedback, the `upgrade` variant in `CanisterInstallMode`
now carries an inline anonymous `record { skip_pre_upgrade; wasm_memory_persistence }`
with an inline `variant { keep; replace }`, matching the IC management
canister's shape and avoiding new named types in the public Candid
surface. Regenerated wallet station.did{,.js,.d.ts} via `dfx generate`.

The station-api Rust types (`CanisterUpgradeOptionsInput`,
`WasmMemoryPersistence`) remain named — Rust requires it — but
`service_equal` confirms structural equivalence with the inline spec.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread core/station/impl/src/models/request_operation.rs Outdated
Comment thread core/station/impl/src/mappers/request_operation.rs Outdated
Per reviewer feedback:
- Widen the internal CanisterInstallMode::Upgrade variant from
  CanisterUpgradeModeArgs to Option<CanisterUpgradeModeArgs>, matching
  the IC management canister's `Upgrade(Option<UpgradeFlags>)` shape.
- Implement From<CanisterUpgradeModeArgs> for mgmt::UpgradeFlags, then
  drop the ad-hoc conditional and just `.map(Into::into)` over the
  Option in both directions (model→mgmt and impl↔api mappers).

Storage migration: legacy `Upgrade(CanisterUpgradeModeArgs {})` CBOR
deserializes as `Upgrade(Some(CanisterUpgradeModeArgs::default()))` —
semantically equivalent to "no flags". Covered by a new test that
encodes the historical enum-variant shape and confirms it round-trips.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sea-snake sea-snake merged commit f262e22 into main May 26, 2026
29 checks passed
@sea-snake sea-snake deleted the arshavir/romantic-tesla-200e32 branch May 26, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants