Skip to content

feat(minter): snapshot the watchlist before every upgrade - #285

Draft
gregorydemay wants to merge 7 commits into
DEFI-2780-09-rearm-dropped-sweepsfrom
DEFI-2780-10-watchlist-snapshot
Draft

gregorydemay wants to merge 7 commits into
DEFI-2780-09-rearm-dropped-sweepsfrom
DEFI-2780-10-watchlist-snapshot

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

The watchlist of the automatic deposit flow of #272 lives on the heap and registrations are not recorded as events, so a new pre_upgrade hook records a snapshot of it on every upgrade, empty included, and replay restores the latest one. Covered by unit tests and a PocketIC test of a registration surviving an upgrade.

🤖 Generated with Claude Code

@gregorydemay
gregorydemay added this pull request to stack #288 October 14, 2026 08:50
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:04
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 2878ffd to 8494587 Compare October 14, 2026 09:04

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.

🟡 Changes recommended

Snapshot events currently prevent binary rollback and can make event-query pages exceed response limits.

2 open findings
What changed in this PR

Persists the automatic-deposit watchlist across canister upgrades through event-log snapshots.

Changes:

  • Adds watchlist snapshot serialization and replay.
  • Records snapshots during pre_upgrade.
  • Adds unit, Candid, and PocketIC coverage.
File Description
minter/​src/​test_fixtures/​mod.rs Generates snapshot events for property tests.
minter/​src/​state/​watchlist/​tests.rs Tests snapshot reconstruction.
minter/​src/​state/​watchlist/​mod.rs Implements snapshot and restoration.
minter/​src/​state/​tests.rs Tests snapshot replay ordering.
minter/​src/​state/​mod.rs Restores snapshots into state.
minter/​src/​state/​event/​candid/​mod.rs Converts snapshot Candid types.
minter/​src/​state/​event.rs Defines the persisted snapshot event.
minter/​src/​state/​audit.rs Applies snapshot events during replay.
minter/​src/​main.rs Registers the pre_upgrade hook.
minter/​src/​lifecycle/​tests.rs Tests snapshot recording.
minter/​src/​lifecycle.rs Records snapshots before upgrades.
minter/​cksol_minter.did Exposes snapshot debug-event types.
libs/​types-internal/​src/​event.rs Adds public event representations.
integration_tests/​tests/​tests.rs Tests restoration across an upgrade.
integration_tests/​src/​fixtures.rs Adds watchlist test constants.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread minter/src/state/event.rs
Comment on lines +233 to +234
#[n(15)]
WatchedDepositAddresses(#[n(0)] WatchlistSnapshot),
Comment thread minter/src/state/event.rs
Comment on lines +248 to +249
#[n(2)]
pub entries: Vec<WatchedDepositAddress>,
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:18
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 8494587 to 8888936 Compare October 14, 2026 09:18
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 8888936 to 4e94eae Compare October 14, 2026 09:20

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.

🟡 Changes recommended

The first production rollout loses registrations because the outgoing WASM cannot execute the newly added pre-upgrade hook.

3 open findings

🧠 Review effort: Balanced

Comment thread minter/src/main.rs
Comment on lines +49 to +51
#[ic_cdk::pre_upgrade]
fn pre_upgrade() {
cksol_minter::lifecycle::pre_upgrade(IcCanisterRuntime::new());
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:21

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.

🔵 Needs a closer look

Human confirmation is needed for the initial rollout, because the previously installed Wasm cannot execute this newly added hook.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:35
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 4e94eae to 5dde44b Compare October 14, 2026 09:35

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.

🟢 Approval recommended

Snapshot recording, serialization, replay, and upgrade behavior are consistently implemented and tested.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:56
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 5dde44b to 2aa32af Compare October 14, 2026 09:56

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.

🔵 Needs a closer look

The initial rollout loses existing registrations, and recorded snapshot events prevent rollback to the preceding Wasm.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 11:19
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 2aa32af to b97a96b Compare October 14, 2026 11:19

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.

🔵 Needs a closer look

The mandatory new event makes rollback to any binary lacking tag 15 impossible without a documented recovery strategy.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 09:04
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from ec2b60b to c3b8c88 Compare October 15, 2026 09:04

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.

🟢 Approval recommended

No blocking issues were identified in snapshot persistence, replay, interface changes, or the added test coverage.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 09:54
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from c3b8c88 to 1313d8e Compare October 15, 2026 09:54

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.

🔵 Needs a closer look

The first rollout needs human validation because an installed version without this hook cannot snapshot any existing heap registrations.

3 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 11:28
@gregorydemay
gregorydemay force-pushed the DEFI-2780-10-watchlist-snapshot branch from 1313d8e to 7cac93c Compare October 15, 2026 11:28

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.

🔵 Needs a closer look

Event-page response sizes remain unbounded, and registration preservation during the first rollout is unverified.

4 open findings

🧠 Review effort: Balanced

Comment on lines +129 to +133
entries: snapshot
.entries
.into_iter()
.map(CandidWatchedDepositAddress::from)
.collect(),
Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:30

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.

🔵 Needs a closer look

The first rollout needs human validation because preservation of existing registrations depends on the previously installed version’s pre-upgrade behavior.

4 open findings

🧠 Review effort: Balanced

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.

🔵 Needs a closer look

Human validation is needed to establish replay instruction-budget headroom as populated snapshots accumulate across upgrades.

4 open findings

🧠 Review effort: Balanced

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.

🔵 Needs a closer look

Accumulated snapshot replay costs need validation before approving this mainnet persistence change.

4 open findings

🧠 Review effort: Balanced

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.

🔵 Needs a closer look

No blocking defects were identified, but mainnet persistence changes warrant human upgrade validation.

4 open findings

🧠 Review effort: Balanced

gregorydemay and others added 7 commits October 15, 2026 14:07
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…napshot

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…estoring a snapshot

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…shot tests

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

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.

🔵 Needs a closer look

Initial rollout needs human validation because an outgoing version without this hook cannot snapshot existing heap-only registrations.

4 open findings

🧠 Review effort: Balanced

This branch has not been deployed

No deployments
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.

2 participants