You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
with_sns_subnet() unverified — likely nonexistent: The PocketIC code example at the "Using PocketIC for SNS integration tests" section (line 122) uses PocketIcBuilder::new().with_nns_subnet().with_sns_subnet().with_application_subnet(). The with_sns_subnet() method is not found anywhere in .sources/ (not in cdk-rs e2e tests, not in examples/, not in the pocket-ic.md reference page). The pocket-ic crate examples in .sources/ only use with_nns_subnet() and with_application_subnet(). This call will fail to compile at version 9.0.2 if the method doesn't exist. Add <!-- Needs human verification: with_sns_subnet() existence in pocket-ic crate --> or replace with the verified pattern from the pocket-ic.md page. The pocket-ic.md page says "Named subnets (NNS, SNS, II) carry the same canister ID ranges as mainnet" but does NOT use with_sns_subnet() — suggesting SNS subnet topology is achieved differently (possibly automatically when NNS is present, or not yet exposed in the public API).
Stage numbering error in the automated stages table: The table in "Steps 4–10: Automated" maps stages incorrectly relative to the portal source (testing-locally.mdx). The portal shows stage 8 = "SNS swap starts" with ./participate_in_sns_swap.sh listed as the action at that stage, and stage 9 = "SNS swap ends." The page instead shows stage 9 = "Participate in the swap" and entirely omits the "Swap ends" event. The correct mapping should be:
dfx identity whoami used without icp-cli equivalent note: Line 224 uses $(dfx identity whoami) in the PEM file path without any note that an icp-cli equivalent exists. The icp-cli equivalent is icp identity default (no argument prints the current identity name). The command should be updated to: export PEM_FILE="$HOME/.config/dfx/identity/$(icp identity default)/identity.pem" or a note should explain the dfx dependency clearly, as is done for other dfx commands in this page. (Note: the .config/dfx/identity/ path itself may also be wrong if a developer has only set up identities with icp-cli rather than dfx — consider noting this.)
WASM path uses dfx artifact structure: Line 269 sets WASM_PATH="./.dfx/ic/canisters/test/test.wasm" — this is a dfx build output path. For developers using icp-cli, the WASM is at target/wasm32-unknown-unknown/release/test.wasm (or via $ICP_WASM_OUTPUT_PATH per icp-cli recipes). There is no note explaining this is a dfx-specific path. Since the testflight section explicitly notes that dfx sns commands are used, this can be framed the same way — but it needs a note or correction so icp-cli users know where to find their WASM.
dfx sns init-config-file validate in checklist without note: Line 410 in the pre-launch checklist says "validates successfully with dfx sns init-config-file validate" but, unlike the step-by-step sections, there is no :::note[Requires dfx sns extension] callout here. The checklist item should either include the same extension note or be rewritten to not imply icp-cli support.
Suggestions
DFX_IC_COMMIT hash has no guidance on how to find the current value: Lines 181–182 use DFX_IC_COMMIT=94bbea43c7585a1ef970bd569a447c269af9650b which is copied verbatim from the portal source. This commit hash may be outdated, and the page gives no indication to the reader that it should be updated or where to find the current value. Consider adding a note: <!-- TODO: verify current DFX_IC_COMMIT hash — check dfinity/sdk releases --> or link to the sns-testing repository for the canonical current hash.
The page correctly marks this as an expected output excerpt (copying from the portal), but a reader may not understand what they're looking at. A brief inline note explaining this is a truncated excerpt of the actual deploy output (the actual neuron ID appears on the next line after the colon) would improve clarity.
Aborting the testflight — recovery information is incomplete: The "Aborting the testflight" section (line 394–403) says: "you can recover access by reinstalling the SNS root canister with recovery code. See the sns-testing repository for the recovery pattern." This is vague. The portal source (testing-on-mainnet.mdx) actually includes the full Rust recovery code inline. The page chose to link out rather than reproduce it, which is reasonable, but should either link to the specific section of the sns-testing repository or flag this as a limitation of the guide.
Generic proposal Rust example: ic_cdk::update without import: The generic proposal code block (line 305) uses #[ic_cdk::update] as a macro attribute but the imports at the top only show use candid::CandidType; use serde::Deserialize;. In a real file, ic_cdk itself needs to be in Cargo.toml. The example doesn't show Cargo.toml dependencies — unlike the sns-launch skill's Rust examples which show them. Consider adding a brief note about dependencies or a minimal Cargo.toml snippet, as done in the skill.
PocketIC section could note version dependency: The PocketIC code example at line 112 uses PocketIcBuilder and with_nns_subnet(). The pocket-ic.md page establishes version 9 context. Since the governance testing page references pocket-ic.md for full setup, this is acceptable, but adding // pocket-ic = "9" as a comment or reference would help developers who land on this page directly.
Verified
All internal links resolve: launching.md, managing.md, ../testing/pocket-ic.md, ../../languages/rust/testing.md — all files exist.
icp canister settings update --add-controller flag verified against .sources/icp-cli/docs/reference/cli.md (line 457).
icp canister call, icp canister status, icp canister id with -e flag all verified against .sources/icp-cli/docs/reference/cli.md.
create_canister_on_subnet(None, None, subnet) signature verified against .sources/examples/rust/parallel_calls/ and .sources/examples/motoko/parallel_calls/.
pic.topology().get_app_subnets() verified against the same examples.
with_nns_subnet() and with_application_subnet() verified against .sources/examples/rust/unit_testable_rust_canister/ and .sources/cdk-rs/e2e-tests/.
SNS testflight step commands (dfx sns import, dfx sns download, dfx sns deploy-testflight, quill sns make-proposal, quill sns make-upgrade-canister-proposal) are consistent with the portal source (testing-on-mainnet.mdx).
Candid arguments in list_sns_canisters and list_proposals calls match the portal source exactly.
DFX_IC_COMMIT hash in Steps 1 matches the portal source value.
Generic proposal function ID constraint (≥ 1000) is consistent with portal source.
:::note[Requires dfx sns extension] callouts are present for all dfx sns command groups where no icp-cli equivalent exists.
Page ends with <!-- Upstream: --> comment (CI requirement satisfied).
Frontmatter is complete: title, description, and sidebar.order are present.
No mo:base imports (Motoko is not used on this page).
No internetcomputer.org/docs/ links.
## Next steps section present at end of page (content-authoring.md requirement satisfied).
Diataxis content type (guides) matches the section location.
with_sns_subnet() — added human verification flag: The method with_sns_subnet() was not found in any .sources/ file (not in cdk-rs e2e tests, not in examples/). The reviewer is correct — this method cannot be verified against available source material. Added <!-- Needs human verification: with_sns_subnet() existence in pocket-ic crate --> HTML comment before the code block, and added an inline comment // requires human verification: check pocket-ic 9.x API on the .with_sns_subnet() call. Also added a // pocket-ic = "9" comment to the code block for version context.
Stage numbering table corrected: Fixed the "Steps 4–10: Automated" table to match the portal source accurately. Stage 8 now reads "Swap opens; participate: ./participate_in_sns_swap.sh" and stage 9 now reads "Swap closes" (previously the participate action was at stage 9 and the swap-closes event was omitted entirely).
dfx identity whoami replaced with icp identity default: Updated the PEM_FILE export to use $(icp identity default) instead of $(dfx identity whoami). Added an inline comment explaining that icp identity default prints the current identity name and noting that developers who created identities with icp-cli rather than dfx may find their PEM files at a different path (~/.config/icp/identity/ or the path shown by icp identity export).
WASM path — added icp-cli note: Added a comment in the code block at WASM_PATH="./.dfx/ic/canisters/test/test.wasm" explaining this is a dfx build output path and that icp-cli projects find WASMs at target/wasm32-unknown-unknown/release/test.wasm or the path set by $ICP_WASM_OUTPUT_PATH in icp.yaml.
Pre-launch checklist — added :::note[Requires dfx sns extension] callout: Added the same callout pattern used in the step-by-step sections directly before the "Pre-launch verification checklist" section, so dfx sns init-config-file validate is properly marked as requiring the extension.
Suggestion items applied:
DFX_IC_COMMIT — added TODO comment: Added <!-- TODO: verify current DFX_IC_COMMIT hash — check dfinity/sdk releases or the sns-testing repository for the canonical current value --> before the dfx sns import/dfx sns download commands.
"Developer neuron IDs:" output clarified: Expanded the output block to show <neuron-id> as a placeholder on the next line and added a sentence explaining that the actual neuron ID appears after the colon.
Changes skipped
Aborting testflight — recovery code not inlined: The reviewer notes the portal source includes inline Rust recovery code, and suggests linking to the specific sns-testing section. The current approach (linking to the sns-testing repository) is maintained — inlining complex recovery code would significantly expand the page scope beyond its testing focus, and the recovery scenario is rare. The note is clear that the repository has the pattern. Reason: judgment call — link is sufficient for an edge-case recovery path; inlining full recovery code would bloat the page.
Generic proposal Rust example — Cargo.toml not added: The reviewer suggests adding a Cargo.toml snippet showing ic-cdk, candid, and serde dependencies. These are standard crate dependencies that every Rust canister already has; adding a partial Cargo.toml here would duplicate guidance better placed in a project setup guide and would complicate the example with boilerplate. Reason: standard crate dependencies, not specific to SNS testing; adding would duplicate setup content.
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
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
Sync recommendation
informed by dfinity/portal — docs/building-apps/governing-apps/testing/*; dfinity/icskills — sns-launch; dfinity/icp-js-sdk-docs — pic-js/latest.zip