chore: migrate rust/composite_query to icp-cli#1398
Open
marc0olo wants to merge 15 commits into
Open
Conversation
1b6a681 to
60a8fd4
Compare
60a8fd4 to
8f077c8
Compare
- Replace dfx.json with icp.yaml using @dfinity/rust@v3.3.0 - Rename canisters: kv_frontend → caller, data_partition → callee - Move each canister into its own subdirectory (caller/, callee/) - Update workspace Cargo.toml to reference new members - Rewrite caller/lib.rs: replace ic_cdk::api::management_canister with ic-cdk-management-canister crate, replace ic_cdk::api::call::call with ic_cdk::call::Call::bounded_wait pattern - Add Makefile with 7 tests covering put, composite query get, get_update, null lookup, multi-partition routing, and lookup composite query - Add rust-toolchain.toml with wasm32-unknown-unknown target - Add rust-composite_query job to composite_query.yml workflow - Delete legacy rust-composite_query-example.yml workflow - Update README with icp-cli deploy instructions and architecture diagram Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1.0.1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fixes - Replace recipe with custom build steps: callee is built first so its WASM is available for include_bytes! in the caller. Only caller is deployed; callee is a compile-time dependency, not a deployed canister. - Add ic_cdk::export_candid!() to both caller and callee - Remove redundant networks block from icp.yaml - Add Node.js to README prerequisites - Fix make topup -> icp canister top-up --amount 30t caller Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aligns with the Motoko example (Map/Bucket) and icp-cli conventions: - caller/ -> backend/ (package: backend) — the Map canister, deployed via icp.yaml - callee/ -> bucket/ (package: bucket) — the Bucket canister, built as a compile-time dependency embedded via include_bytes! in backend Also fixes pre-existing include_bytes! path bug: ../../target/ was pointing one level too high (to rust/target/ instead of rust/composite_query/target/). Corrected to ../target/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ke Motoko Fixed InsufficientLiquidCycleBalance in CI: hardcoded 10T per bucket exceeded the available balance. Now divides evenly like the Motoko example: cycleShare = Cycles.balance() / (NUM_PARTITIONS + 1) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ic-cdk 0.20's candid::<T>() uses decode_one (single value) but a canister method's return is encoded as a Candid argument list, which requires decode_args (used by candid_tuple). Using candid::<T>() caused all inter-canister call responses to decode silently as None, making test 2 fail (put returned null instead of the previous value). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…og messages Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Migrates rust/composite_query from the legacy dfx-based layout to an icp-cli-compatible setup, restructuring the Rust code into a workspace with a deployable backend (Map) canister that dynamically creates Bucket child canisters at runtime, and updating CI/testing accordingly.
Changes:
- Replaced dfx project config and old
src/canister layout with anicp.yaml-driven build and a Rust workspace (backend/+bucket/). - Updated management canister integration to
ic-cdk-management-canisterand modernized inter-canister call decoding (.candid_tuple()). - Added a
test.shsuite and wired a newrust-composite_queryjob into the shared workflow.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/composite_query/test.sh | Adds an icp-cli test script covering key behaviors (put/get/get_update/lookup/routing). |
| rust/composite_query/rust-toolchain.toml | Ensures the wasm target is available for builds. |
| rust/composite_query/README.md | Updates documentation for icp-cli deploy/test flow and explains architecture. |
| rust/composite_query/icp.yaml | Introduces custom build steps to build/embed bucket.wasm and inject candid metadata. |
| rust/composite_query/Cargo.toml | Converts the project into a workspace with backend and bucket members. |
| rust/composite_query/backend/lib.rs | New backend canister that shards keys across dynamically created bucket canisters. |
| rust/composite_query/backend/Cargo.toml | Adds the backend crate dependencies (ic-cdk, ic-cdk-management-canister). |
| rust/composite_query/bucket/lib.rs | Updates bucket canister implementation and adds candid export. |
| rust/composite_query/bucket/Cargo.toml | Adds the bucket crate definition and dependencies. |
| .github/workflows/composite_query.yml | Adds a Rust job (containerized) for the migrated example. |
| .github/workflows/rust-composite_query-example.yml | Removes the legacy dfx-based workflow. |
| rust/composite_query/dfx.json | Removes legacy dfx configuration. |
| rust/composite_query/src/kv_frontend/lib.rs | Removes the legacy frontend canister implementation. |
| rust/composite_query/src/kv_frontend/Cargo.toml | Removes legacy frontend crate config. |
| rust/composite_query/src/kv_frontend.did | Removes legacy frontend candid file. |
| rust/composite_query/src/data_partition/Cargo.toml | Removes legacy partition canister crate config. |
| rust/composite_query/src/data_partition.did | Removes legacy partition candid file. |
| rust/composite_query/Cargo.lock | Removes the previous lockfile from the old workspace structure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…state - get_partition_for_key returns Option<Principal> using .get() instead of direct indexing — safe for empty or partially-initialized CANISTER_IDS - put/get/get_update return None early if the bucket for the key is not yet available (empty state or partial init from concurrent put) - lookup uses .get().map().unwrap_or_default() instead of direct indexing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Compute cycle_share once before the loop so all buckets receive the same amount (fixes the decreasing-allocation bug) - Update ic-wasm install hint to use npm instead of GitHub repo link - Test 1: assert return value is null (new key has no previous value) - Test 7: assert canister ID is non-empty (not just that index 1 appears) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removing the null assertion from test 1 — the return value is the previous value for key 1 (null on first run, a nat on subsequent runs). Tests 2-7 are already idempotent since they overwrite the same keys with the same values each time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-runs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Change lookup from #[query(composite = true)] to plain #[query] — it makes no inter-canister calls so composite = true is misleading; the composite query showcase is fully covered by get() - Fix README: backend creates all 5 Bucket canisters on the first put call (not lazily per key as the old wording implied) - Fix stale 'data partitions' comment -> 'Bucket canisters' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+31
to
35
| ### Prerequisites | ||
|
|
||
| ```bash | ||
| dfx canister call kv_frontend put '(1:nat, 1337:nat)' | ||
| (null) | ||
| dfx canister call kv_frontend put '(1:nat, 42:nat)' | ||
| (opt (1_337 : nat)) | ||
| ``` | ||
| - Node.js | ||
| - icp-cli: `npm install -g @icp-sdk/icp-cli @icp-sdk/ic-wasm` | ||
|
|
candid-extractor is not part of the standard icp-cli prerequisites (npm install -g @icp-sdk/icp-cli @icp-sdk/ic-wasm). The custom build steps are only needed to compile bucket before backend; dropping the Candid interface extraction step removes the extra dependency. The canister is fully functional without the candid:service WASM metadata. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
rust/composite_queryfrom dfx to icp-cli.Removed:
dfx.json,BUILD.md, legacy workflowrust-composite_query-example.yml, oldsrc/layoutChanged:
backend/(the Map canister, deployed) andbucket/(the Bucket canister, compile-time dependency only) with a workspaceCargo.tomlic_cdk::api::management_canistertoic-cdk-management-canister = "0.1.1"ic_cdk::call::Call::bounded_wait+.candid_tuple()—.candid()usesdecode_one(single value) while method responses requiredecode_args, so.candid_tuple()is correct10Tto dynamic (balance / (NUM_PARTITIONS + 1)), mirroring the Motoko approachget_partition_for_keyreturnsOption<Principal>soget,get_update, andputreturnNonegracefully before initialization instead of trappingAdded:
icp.yamlwith custom build steps —bucketis built first so its WASM is available forinclude_bytes!inbackend; onlybackendis deployed,Bucketinstances are created on demand at runtimetest.shwith 7 tests coveringput, composite queryget,get_update, null lookup, multi-partition routing, andlookuprust-composite_queryjob in the existingcomposite_query.ymlworkflowTest plan
icp network start -d && icp deploy --cycles 30t && bash test.shpasses locallyrust-composite_queryjob passes🤖 Generated with Claude Code