chore: migrate rust/stake_neuron_from_cli to icp-cli by marc0olo · Pull Request #1417 · dfinity/examples

marc0olo · 2026-06-16T18:04:04Z

Summary

Replace dfx.json with icp.yaml (nns: true) — icp-cli is used only for local network management; the Rust binary connects to the IC directly via ic-agent and does not depend on icp-cli at runtime
Upgrade ic-agent 0.35 → 0.47 (add pem + ring features; ring is required for secp256k1 keys generated by icp-cli)
Upgrade ic-ledger-types 0.15 → 0.16; drop hex and crc32fast (covered by ic-ledger-types)
Update edition to "2024"
Update src/main.rs: rename --amount → --amount-e8s; add --compute-only flag that derives and prints the staking subaccount without making any IC calls — lets users verify the destination before committing funds; update default URL to localhost:8000
Update src/lib.rs: use .unwrap() for constant principal; clarify that the ledger transfer memo is cosmetic and does not need to match the governance memo — what matters is the nonce used to derive the staking subaccount; document that anyone can complete step 2; remove debug noise; expose compute_neuron_staking_subaccount as pub
Update rust-toolchain.toml: remove channel = "1.80" pin (kept targets only)
Replace setup_and_run.sh with test.sh: creates a funded test identity via icp-cli, builds the binary, runs four tests (subaccount determinism, nonce isolation, below-minimum guard, full staking roundtrip)
Add CI workflow .github/workflows/stake_neuron_from_cli.yml (icp-dev-env-rust:1.0.1); remove legacy dfx-based workflow
Rewrite README.md: clarify that src/lib.rs is the primary artifact (ic-agent Rust code); icp-cli is local-dev only and not needed on mainnet; explain why Node.js is needed with link to alternative installation methods; remove "domain-separated" jargon; correct nonce/memo claim; add nns.internetcomputer.org link; note ICRC-2 as safer alternative with link to feat: add ICRC-2 token approve and token allowance commands icp-cli#637
Delete dfx.json, setup_and_run.sh, legacy CI workflow

Test plan

icp network start -d && bash test.sh passes — subaccount determinism, nonce isolation, below-minimum guard, full 1 ICP staking roundtrip verified against local NNS
cargo build --release succeeds (no icp-cli needed)
CI workflow stake_neuron_from_cli passes on push

🤖 Generated with Claude Code

Converts the dfx-based CLI binary into an icp-cli canister example. Replaces ic-agent/tokio/clap with ic-cdk inter-canister calls, adds icp.yaml (rust@v3.3.0), Makefile with local compute_subaccount tests, CI workflow, and updated README preserving the educational NNS staking domain knowledge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ge to 1.0.1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ller/subaccount bug - Add nns: true to icp.yaml so NNS canisters deploy at mainnet IDs locally - Fix bug in stake_neuron: subaccount was computed from msg_caller() but Governance was told controller = canister_self(); Governance recomputes the expected subaccount from the controller field, so these must match — switched both to canister_self() since the canister holds the ICP and owns the neuron - Replace hand-rolled ledger types (AccountIdentifier, TransferArgs, Tokens, Memo, crc32, build_account_identifier) with ic-ledger-types = "0.16"; the transfer() helper handles correct Candid serialization of AccountIdentifier as a blob - Update test.sh: fund backend with 2 ICP via icp token transfer, add Test 3 that calls stake_neuron and asserts a neuron ID is returned from local NNS Governance - Update edition to 2024 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…l NNS testing - Remove stale "mainnet required" callout — nns: true enables full local testing - Update code snippet to match actual ic-ledger-types-based implementation - Add Node.js v18+ and Rust to prerequisites - Fix deploy/test description to reflect that test.sh exercises stake_neuron locally - Fix mainnet canister call flag: --network ic → -e ic - Update stake_neuron docstring: remove "NNS not available locally" note, add nns: true requirement Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ainnet funding Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Migrates the rust/stake_neuron_from_cli example from a dfx-driven CLI binary to an icp-cli-managed Rust canister example, including a new canister API and automated test.sh flow that exercises neuron staking against a local NNS deployment.

Changes:

Replaced the old CLI (src/main.rs + src/lib.rs) with a Rust canister (backend/lib.rs) exposing compute_subaccount (query) and stake_neuron (update).
Switched project configuration from dfx.json to icp.yaml (with nns: true) and added a workspace layout (Cargo.toml + backend/Cargo.toml + rust-toolchain.toml).
Added CI + local testing via test.sh and a dedicated GitHub Actions workflow.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rust/stake_neuron_from_cli/backend/lib.rs	New canister implementation for subaccount computation + staking flow via ledger + governance calls.
rust/stake_neuron_from_cli/backend/Cargo.toml	New backend crate manifest (notably sets Rust edition).
rust/stake_neuron_from_cli/Cargo.toml	Converted to a workspace manifest pointing at `backend/`.
rust/stake_neuron_from_cli/icp.yaml	New icp-cli project config; enables local NNS deployment (`nns: true`).
rust/stake_neuron_from_cli/rust-toolchain.toml	Ensures `wasm32-unknown-unknown` target is available for builds.
rust/stake_neuron_from_cli/test.sh	New automated test script funding the canister and exercising `compute_subaccount` + `stake_neuron`.
rust/stake_neuron_from_cli/README.md	Updated docs to icp-cli workflow + local NNS testing + mainnet notes.
rust/stake_neuron_from_cli/src/main.rs	Removed old clap/tokio CLI entrypoint.
rust/stake_neuron_from_cli/src/lib.rs	Removed old ic-agent based staking implementation.
rust/stake_neuron_from_cli/setup_and_run.sh	Removed dfx-based setup script.
rust/stake_neuron_from_cli/dfx.json	Removed dfx project config.
rust/stake_neuron_from_cli/Cargo.lock	Removed lockfile from the old single-crate CLI build.
.github/workflows/stake_neuron_from_cli.yml	Added new container-based CI workflow using `icp-cli` deploy + `test.sh`.
.github/workflows/rust-stake-neuron-from-cli-example.yaml	Removed legacy dfx-based workflow.

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

…zation note - Reject amounts below 100_000_000 e8s early so no ICP is transferred before Governance rejects the claim and strands funds in the staking subaccount - Add code comment noting stake_neuron is unguarded; production deployments should restrict to an authorized principal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… guard, and cross-validation tests - Test 1: known-value assertion for compute_subaccount against a fixed reference principal (aaaaa-aa, nonce 0) — catches any regression in the SHA-256 domain separation without depending on a deployment-specific canister ID - Test 2: verify different nonces produce different subaccounts (replaces trivial determinism check) - Test 3: verify stake_neuron rejects amounts below 100_000_000 e8s with a clear error - Test 4: cross-validate that subaccount_hex in the stake_neuron result matches compute_subaccount(backend, nonce) — the key safety invariant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…avior in test.sh Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ll, not total neuron balance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

…o with_arg Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

daniel-wong-dfinity-org-twin

We now support ICRC-2, which is much nicer, because if there is a crash between steps 1 and 2, the ICP is still in the user's control. Here is a good starting point: https://sourcegraph.com/r/github.com/dfinity/ic@cc1287f38c519256d7189d0730fb5f2c145fb27a/-/blob/rs/nns/governance/canister/governance.did?L1576-1593

…pply review feedback The original example was a CLI binary using ic-agent — converting it to an icp-cli canister changed its fundamental purpose. This reverts to the original CLI approach while updating everything to current standards: - Revert from canister to CLI binary (src/main.rs + src/lib.rs, no backend/) - icp.yaml: network-only config with nns: true; no canister recipe - Upgrade ic-agent 0.35 → 0.47 (add pem + ring features for secp256k1 support — icp-cli generates secp256k1 keys by default) - Upgrade ic-ledger-types 0.15 → 0.16; drop hex and crc32fast (covered by ic-ledger-types) - edition = "2024" - Add --compute-only flag: prints the staking subaccount without any IC call or transfer — lets users verify the destination before committing funds - Rename amount → amount_e8s throughout; .unwrap() for constant principal - Clarify memo: the ledger transfer memo is cosmetic and does not need to match the governance nonce; what matters is that the nonce used to derive the staking subaccount matches the memo in claim_or_refresh_neuron_from_account - Document that anyone can complete step 2 (neuron controller is determined by the subaccount derived in step 1) - README: remove "domain-separated" jargon, list the 4 SHA-256 inputs plainly; add nns.internetcomputer.org link; note ICRC-2 as a safer alternative with link to dfinity/icp-cli#637 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ADME - State upfront that ic-agent/Rust is the primary artifact; icp-cli is only used for local network management and not required on mainnet - Explain why Node.js is needed (icp-cli installs via npm) and link to alternative installation methods - Add --compute-only usage example to the subaccount section Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marc0olo · 2026-07-03T10:17:18Z

Hi @daniel-wong-dfinity-org-twin — thanks for the detailed review, it was very helpful. Here's what we acted on:

Architectural change: You were right to flag the canister approach. The original example was a CLI binary using ic-agent — we had inadvertently converted it to a canister during migration, which changed its fundamental purpose. We've reverted back to the CLI binary (src/lib.rs + src/main.rs), with icp.yaml used only to start a local NNS replica for testing. The example now correctly shows how to stake ICP from off-chain Rust code.

Code changes applied:

Renamed amount → amount_e8s ✓
Renamed subaccount → new_neuron_subaccount ✓
.unwrap() for the constant governance principal ✓
Clarified the memo field: the ledger transfer memo is cosmetic; what matters is that the nonce used to derive the staking subaccount matches the memo in claim_or_refresh_neuron_from_account ✓
Added --compute-only flag so users can verify the staking subaccount before committing any ICP ✓
Documented that anyone can complete step 2 ✓

README updates:

Removed "domain-separated" jargon; listed the 4 SHA-256 inputs plainly ✓
Corrected the nonce/memo claim ✓
Added nns.internetcomputer.org link ✓
Made clear that src/lib.rs is the primary artifact and icp-cli is local-dev only ✓
Added ICRC-2 note pointing to feat: add ICRC-2 token approve and token allowance commands icp-cli#637 for the safer atomic approach once icp-cli supports it ✓

Not applied: governance types in a separate file — for an example this size, keeping everything in one lib.rs is more readable.

Would you mind having a quick re-review? We'd love your sign-off before merging.

marc0olo force-pushed the chore/migrate-rust-stake-neuron-to-icp-cli branch from c0ff887 to dadf982 Compare June 17, 2026 17:02

marc0olo and others added 2 commits June 19, 2026 08:32

chore(rust/stake_neuron_from_cli): test.sh over Makefile, bump CI ima…

232a3c7

…ge to 1.0.1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marc0olo force-pushed the chore/migrate-rust-stake-neuron-to-icp-cli branch from dadf982 to 232a3c7 Compare June 19, 2026 06:34

marc0olo and others added 2 commits July 2, 2026 16:57

marc0olo requested a review from Copilot July 2, 2026 15:11

Copilot started reviewing on behalf of marc0olo July 2, 2026 15:11 View session

fix(rust/stake_neuron_from_cli): add icp token transfer command for m…

f958688

…ainnet funding Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Comment thread rust/stake_neuron_from_cli/backend/lib.rs Outdated

Comment thread rust/stake_neuron_from_cli/backend/lib.rs Outdated

Comment thread rust/stake_neuron_from_cli/backend/Cargo.toml Outdated

marc0olo and others added 4 commits July 2, 2026 17:20

docs(rust/stake_neuron_from_cli): explain claim_or_refresh re-run beh…

1f4349c

…avior in test.sh Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(rust/stake_neuron_from_cli): clarify staked_amount_e8s is per-ca…

2e95c39

…ll, not total neuron balance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marc0olo requested a review from Copilot July 2, 2026 15:51

Copilot started reviewing on behalf of marc0olo July 2, 2026 15:51 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Comment thread rust/stake_neuron_from_cli/backend/lib.rs Outdated

style(rust/stake_neuron_from_cli): pass &claim_request by reference t…

cc12897

…o with_arg Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marc0olo marked this pull request as ready for review July 2, 2026 16:22

marc0olo requested review from a team as code owners July 2, 2026 16:22

daniel-wong-dfinity-org-twin approved these changes Jul 2, 2026

View reviewed changes

marc0olo and others added 2 commits July 3, 2026 12:01

marc0olo requested a review from daniel-wong-dfinity-org-twin July 3, 2026 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: migrate rust/stake_neuron_from_cli to icp-cli#1417

chore: migrate rust/stake_neuron_from_cli to icp-cli#1417
marc0olo wants to merge 12 commits into
masterfrom
chore/migrate-rust-stake-neuron-to-icp-cli

marc0olo commented Jun 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

daniel-wong-dfinity-org-twin left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc0olo commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

marc0olo commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

daniel-wong-dfinity-org-twin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc0olo commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marc0olo commented Jun 16, 2026 •

edited

Loading