Skip to content

fix!: make committing a proposed batch atomic - #4551

Draft
lwshang wants to merge 3 commits into
masterfrom
lwshang/reset-evidence-on-propose
Draft

lwshang wants to merge 3 commits into
masterfrom
lwshang/reset-evidence-on-propose

Conversation

@lwshang

@lwshang lwshang commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Description

Two changes to how the asset canister commits a proposed batch.

1. Commit a proposed batch in a single message. Since #4450, commit_proposed_batch awaits a self-call once it passes 35B instructions, and sets certified_data only after the last operation. If an operation fails after such an await, only the failing message rolls back: the operations applied before it stay live, and the certified tree no longer matches certified_data until the next successful commit. commit_proposed_batch now runs the state machine to completion within one message, so any failure, including running out of instructions, rolls back the whole commit, as before 0.30.2. The cost is that a proposed batch is again bounded by the per-message instruction limit. commit_batch keeps the self-call extension.

This changes ic_certified_assets::commit_proposed_batch from async fn to fn (breaking for direct callers of the crate); the Candid interface is unchanged.

2. Discard computed evidence when a batch's arguments change. propose_commit_batch now clears any evidence previously computed for the batch, and commit_proposed_batch clears it when it takes the proposed arguments, so evidence always describes the arguments the batch currently holds and new arguments need a fresh compute_evidence.

How Has This Been Tested?

  • New unit test evidence_does_not_carry_over_to_reproposed_arguments covers change 2 (fails without it).
  • Change 1 is at the canister entry point and only differs from the previous behaviour once a commit passes 35B instructions, which unit tests can't reach; no automated test covers it.
  • All 111 ic-certified-assets lib tests pass; clippy is clean.
  • src/distributed/assetstorage.wasm.gz rebuilt with ./scripts/update-frontend-canister.sh --release-build.

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

🤖 Generated with Claude Code

lwshang and others added 3 commits October 19, 2026 12:01
propose_commit_batch now clears any evidence previously computed for the batch,
and commit_proposed_batch clears it when it takes the proposed arguments. Since
commit_proposed_batch can span several messages, a failure after the first one
leaves the batch in place without its arguments; its evidence must not validate
arguments proposed for it afterwards.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
commit_proposed_batch no longer awaits self-calls to extend its instruction
budget. A failure after an await rolled back only the failing message, leaving
the operations applied before it live and certified_data stale. Committing in
one message makes the commit all-or-nothing again, as it was before 0.30.2, at
the cost of bounding a proposed batch by the per-message instruction limit.
commit_batch keeps the extension.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lwshang lwshang changed the title fix: discard a batch's computed evidence when its arguments change fix!: make committing a proposed batch atomic Oct 27, 2026
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.

1 participant