feat!: length-prefix every variable-length field in asset canister evidence - #4545
Conversation
…idence The encoding hashed for the evidence of a proposed batch, and for the state hash, now length-prefixes every variable-length field -- asset keys, content types, content encodings, header names and values, the declared sha256, and asset content -- and hashes the number of entries in a header map. This is the framing that crates/state-hash uses in dfinity/certified-assets. Every operation in the encoding is self-delimiting, which makes the digest an injective function of the batch it is computed over. The encoding is now specified under `compute_evidence` in docs/design/asset-canister-interface.md, so that tooling which verifies a proposal can reimplement it, and `ic-asset` and `ic-certified-assets` each pin the same test vector for a fixed batch: the two implementations have to agree for the evidence of a proposed batch to be comparable with what dfx computes from source. The evidence of a SetAssetContent operation now covers `last_chunk` whether or not `chunk_ids` is empty, matching the content that `commit_batch` stores for the same operation. The asset canister reports api_version 3. `dfx deploy --by-proposal` and `dfx deploy --compute-evidence` now report an error against an asset canister that reports a lower version, instead of computing a value that cannot be compared with it. Evidence and state hash values differ from the ones earlier versions compute over the same assets, so the asset canister and dfx have to be upgraded together. The e2e assertions no longer pin project-dependent evidence values; they compare the value dfx computes with the value the canister computed, which is the property that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilt with ./scripts/update-frontend-canister.sh --release-build, so that the distributed asset canister carries the framed evidence and state hash encoding. assetstorage.did is unchanged: api_version already has this signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The evidence/state-hash encoding change and API-version gate are compatibility- and integrity-critical and should receive final human verification across upgrade and cross-implementation comparability paths.
Pull request overview
This PR updates the asset canister “evidence” and state-hash encoding to be self-delimiting (length-prefixing every variable-length field, plus hashing header-map entry counts) so the digest becomes an injective function of the encoded batch/state, and aligns the framing with crates/state-hash in dfinity/certified-assets. It also bumps the asset canister api_version to 3 and teaches ic-asset to refuse proposal/evidence workflows against older canisters where computed values would not be comparable.
Changes:
- Switch evidence/state-hash hashing to a length-prefixed, tagged, domain-separated encoding (and update tests + pinned vectors in both implementations).
- Ensure
SetAssetContentevidence coverslast_chunkeven whenchunk_idsis non-empty (matchingcommit_batch’s assembled content). - Gate
dfx/ic-assetproposal + evidence computation on asset canister API version ≥ 3; update docs, changelog, and e2e expectations accordingly.
File summaries
| File | Description |
|---|---|
| src/canisters/frontend/ic-certified-assets/src/tests.rs | Adds targeted tests for boundary/partition injectivity, chunk-boundary independence, last_chunk coverage, and pins a cross-crate test vector. |
| src/canisters/frontend/ic-certified-assets/src/lib.rs | Bumps asset canister api_version() from 2 to 3. |
| src/canisters/frontend/ic-certified-assets/src/evidence.rs | Implements domain-separated, length-prefixed encoding; hashes header-map lengths; length-prefixes content via total content length; includes last_chunk alongside chunk_ids. |
| src/canisters/frontend/ic-asset/src/sync.rs | Rejects --by-proposal preparation when canister API version is too low for comparable evidence. |
| src/canisters/frontend/ic-asset/src/evidence/mod.rs | Implements matching encoding + domain separator, pins a known-batch vector, and rejects evidence computation against canisters with API version < 3. |
| src/canisters/frontend/ic-asset/src/error/prepare_sync_for_proposal.rs | Adds a clear error for “evidence API version too low” in proposal preparation. |
| src/canisters/frontend/ic-asset/src/error/compute_evidence.rs | Adds API-version query failure + “evidence API version too low” errors for compute-evidence. |
| e2e/tests-dfx/assetscanister.bash | Makes e2e evidence assertions robust by extracting evidence from output and comparing computed vs canister-provided evidence dynamically. |
| docs/design/asset-canister-interface.md | Specifies the hashed encoding under compute_evidence and documents API version 3 semantics/compatibility constraints. |
| CHANGELOG.md | Documents the breaking evidence/state-hash change, upgrade requirements, and related behavior changes. |
Review details
- Files reviewed: 10/11 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
vite-plugin-environment 1.1.4, published 2026-09-09, raises its peer dependency from `vite >= 2.7` to `vite >= 8.0`. The templates ask for `^1.1.3` and pin vite at ^4.3.9 (^5.4.3 for svelte), so npm resolves the plugin to 1.1.4 and `npm install` fails with ERESOLVE in a newly created project. That leaves node_modules empty, and the build step then fails with `tsc: command not found`. Ask for exactly 1.1.3 in all four frontend templates. Verified that each of the four resolves with the pin and that vanilla_js fails without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bitcoin.org no longer serves the bitcoin-core binaries and returns 404, which fails the bitcoin e2e job before any test runs. bitcoincore.org serves the same release; the existing sha256 check on the tarball is unchanged and still verifies what is downloaded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ic-mops 3.0.0, published 2026-08-20, removed the `toolchain init` and `toolchain reset` subcommands. setup_playground calls `mops toolchain init` to install the moc version the playground backend builds with, so an unpinned `npm i -g ic-mops` now installs 3.x and every playground test fails in setup with "unknown command 'init'". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 6cfb276. Security OverviewDetected Code Changes
|
marc0olo
left a comment
marc0olo
left a comment
There was a problem hiding this comment.
The encoding change looks right and the cross-crate vector is the right way to
pin it. One thing needs fixing before merge.
The spec says each operation is a tag "followed by its fields, in the order they
are listed in the operation". The documented CreateAssetArguments lists
enable_aliasing before allow_raw_access, but hash_create_asset hashes
allow_raw_access first. I reimplemented the encoding from the document alone
and checked it against the vector this PR pins:
pinned in both crates : 984350ce...3c778ca6
spec as written : 1f872096...b736f48d mismatch
code order : 984350ce...3c778ca6 match
The pinned batch has enable_aliasing: Some(true) and
allow_raw_access: Some(false), so the two orders diverge on this very vector.
Since verifying tooling reimplementing from the doc is the reason it exists, I'd
swap the two hash_opt_bool calls in both implementations rather than document
an exception — that keeps the field-order rule uniform, and the encoding is
already breaking here so it costs nothing.
Everything else in the spec reproduced first try: tags, big-endian nat64 length
prefixes, the header-map entry count, opt/bool tagging, content-length prefix,
and SetAssetProperties field order. Nice document.
Also verified: both implementations are structurally identical across all nine
operations; the domain separator is applied exactly once on each of the three
entry paths, including a state-hash restart; the empty-batch e2e constant is
sha256("ic-certified-assets v2"); the last_chunk gap is real and correctly
closed; the version gate is on both entry points, and an older dfx against a new
canister fails closed at commit time. 110 + 30 tests pass locally.
Two nits:
ENCODING_DOMAINsaysv2whileapi_versionis 3. Two numbers that have to
move together, currently off by one — worth a comment tying them.- The spec calls the encoding "an injective function of the proposed
commit_batcharguments" and then says chunking doesn't affect the digest.
Both can't hold; it's injective over the content, not the arguments. Worth
stating precisely in a document meant for reimplementers.
+1 on splitting the three CI commits out — the encoding change deserves its own
bisect point.
The three external e2e regressions are split out into their own PR so each gets its own bisect point, leaving this branch to the evidence encoding alone. Reverted rather than rewritten, so the branch history reviewers have already seen stays intact. This reverts commit 8ceb0c7 (ci: pin ic-mops to 2.24.0 in the playground e2e test), ad05f0a (ci: download bitcoin core from bitcoincore.org) and 0d181ea (fix: pin vite-plugin-environment to 1.1.3 in frontend templates). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assets.did, CreateAssetArguments and the interface document all list enable_aliasing before allow_raw_access; hash_create_asset hashed them the other way round, so a reimplementation that followed the document produced a different digest for the same batch. Every other operation already hashes its fields in declaration order, so this was the one exception to the rule the document states, on top of the content handling it spells out for SetAssetContent. Swap the two calls in both implementations and move the pinned cross-crate vector with them. Reimplementing the encoding from the document alone now reproduces that vector. Also, from review: - State precisely what the encoding is injective over -- the change a batch applies, with each SetAssetContent's content taken as one byte string -- and say explicitly that it is not injective over the batch arguments, since two batches that differ only in how they split content across chunks encode identically. The document claimed injectivity over the arguments and then noted that chunking does not affect the digest, which cannot both hold. - Note that the `v2` in the domain separator versions the encoding and is not the API version, which is at 3, and that the two move independently. - Record that the domain separator is hashed as its 22 bytes with no length prefix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Picks up the CreateAsset field-order fix. Rebuilt with ./scripts/update-frontend-canister.sh --release-build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…framing # Conflicts: # CHANGELOG.md
The pinned vector stopped at CreateAsset, SetAssetContent and DeleteAsset. It now covers UnsetAssetContent, SetAssetProperties and Clear as well. SetAssetProperties is the one that most needed it: it is the only operation whose arguments differ in type between the two crates -- headers arrive in ic-asset as a Vec<(String, String)> and in the canister as a BTreeMap, with a hand-written conversion between them -- and assemble_batch_operations emits it on the by-proposal path. The vector lists those headers out of order on purpose, so the conversion has to sort them the way the canister does. Its four opt opt fields also exercise all three shapes: set, explicitly cleared, absent. Reimplementing the encoding from the interface document reproduces the new vector, so all three implementations agree. Also from review: - Drop the claim that this is the framing crates/state-hash uses in dfinity/certified-assets. That crate versions with a u8 rather than a string domain, length-prefixes little-endian, sorts headers by (lowercase(name), value), folds content in as stored sha256 hashes rather than raw bytes, and models served state with no batch-operation concept. The two share the technique, not the framing, and the digests are not comparable. - State the SetAssetContent exception where the field-order rule is given, not two paragraphs below it. - Collapse the content-chunking branch in ic-asset: chunks() yields the whole slice when it is shorter than MAX_CHUNK_SIZE and sha256 is streaming, so both arms hashed the same bytes. No shipped canister code changed, so assetstorage.wasm.gz is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed the encoding had been versioned from 1 and the API from 0. Nothing was ever versioned: no digest anywhere carries a `v1` separator, since the encoding that preceded this one had no separator at all. Keep `v2` -- the number counts encodings of this data, and there are two in the wild: the unframed one that every asset canister installed before API version 3 still computes, and this one -- and say that, rather than inventing a history. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit's comment shifted line numbers in evidence.rs, and the wasm embeds source locations through panic metadata, so the binary is not byte-identical even though no code changed. Rebuilt with ./scripts/update-frontend-canister.sh --release-build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
marc0olo
left a comment
marc0olo
left a comment
There was a problem hiding this comment.
Approving. The encoding reproduces from the document alone: I reimplemented all six
operations from docs/design/asset-canister-interface.md in a separate language and got
the pinned 5e8a8c1c...c8b28089 first try. The extended vector now machine-checks the
SetAssetProperties conversion on both sides, which was the one thing previously only
traced by hand.
Two things for afterwards, neither blocking.
The version gate is a lower bound only. api_version >= 3 catches a canister too old
to produce a comparable digest, but not a future one on a later encoding — that would
still satisfy >= 3, dfx would report a v2 digest, and the mismatch would surface only
when commit_proposed_batch rejects it, after a reviewer has already compared two numbers
that were never comparable. Since the encoding version and api_version move
independently by design, the fix is for the canister to report the encoding version and
for ic-asset to require equality rather than a floor. Correct as shipped; it bites at the
next encoding change. Worth a follow-up issue.
ic-asset needs a major bump at release. ComputeEvidenceError and
PrepareSyncForProposalError both gain variants here and neither is #[non_exhaustive].
Not worth changing in this PR — none of the crate's 21 error enums is marked, so doing it
for these two alone would be arbitrary and doing it crate-wide is separate scope. Just
needs catching when versions are set.
Description
The encoding hashed for the evidence of a proposed batch, and for the state hash, now
length-prefixes every variable-length field — asset keys, content types, content encodings,
header names and values, the declared
sha256, and asset content — and hashes the number ofentries in a header map. This is the framing that
crates/state-hashuses indfinity/certified-assets. Each operation in the
encoding is self-delimiting, which makes the digest an injective function of the batch it is
computed over.
The encoding is now specified under
compute_evidenceindocs/design/asset-canister-interface.md, so that
tooling which verifies a proposal can reimplement it, and
ic-assetandic-certified-assetseach pin the same test vector for a fixed batch — the two implementations have to agree for
dfx deploy --compute-evidenceto be comparable with what the asset canister computes.The evidence of a
SetAssetContentoperation now coverslast_chunkwhether or notchunk_idsis empty, matching the content that
commit_batchstores for the same operation.The asset canister reports
api_version3.dfx deploy --by-proposalanddfx deploy --compute-evidencereport an error against an asset canister that reports a lowerversion, instead of computing a value that cannot be compared with it.
Upgrading
Evidence and state hash values that this change computes differ from the values earlier versions
compute over the same assets, so the asset canister and dfx have to be upgraded together:
proposed batch does not survive a canister upgrade);
Crate version bumps (
ic-certified-assets,ic-asset,icx-asset) are left to the release.Unrelated CI fixes carried here
CI was red for two reasons that predate this branch, both external. They are fixed here so this
PR can go green; happy to split them into their own PR if preferred.
fix: pin vite-plugin-environment to 1.1.3— 1.1.4, published 2026-09-09, raises its peerdependency from
vite >= 2.7tovite >= 8.0. The frontend templates ask for^1.1.3againstvite^4.3.9 (^5.4.3 for svelte), sonpm installin a new project fails with ERESOLVE,node_modulesstays empty, and the build then fails withtsc: command not found. This brokeevery job that deploys a frontend project:
smoke,ui_test,dfx/new,dfx/info,dfx/telemetry,dfx/playground,dfx/frontend,dfx/deploy, and onedfx/assetscanistercase.
ci: download bitcoin core from bitcoincore.org— bitcoin.org now 404s for thebitcoin-core binaries, failing the
dfx/bitcoinjobs during provisioning. The existing sha256check on the tarball is unchanged.
Master's last e2e run was 2026-06-23, before either regression, which is why master appears green.
How Has This Been Tested?
cargo test -p ic-certified-assets -p ic-asset— 110 + 30 tests. New cases cover header-mappartitioning, the key/content-type and key/content-encoding boundaries, a key or content region
run on into the operations that follow it, chunk-boundary independence, and
last_chunkalongside
chunk_ids. Each new case was checked to fail against the previous encoding, ratherthan pass vacuously.
evidence_of_known_batch, pinned independently in both crates, checks thatic-assetandic-certified-assetshash the same batch to the same value.cargo clippy -p dfx -p ic-asset -p ic-certified-assets --all-targets,cargo fmt --check,shellcheck e2e/**/*.*sh— all clean.value the canister computed and assert
dfx deploy --compute-evidenceproduces the same one.Not run locally — relying on CI.
src/distributed/assetstorage.wasm.gzwas rebuilt with./scripts/update-frontend-canister.sh --release-build. Relying on thefrontend-canister-up-to-datejob to confirm the reproducible build.Checklist:
🤖 Generated with Claude Code