feat(registry-canister): get_subnet endpoint#10387
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new public query API to the NNS registry canister to fetch a parsed subnet record by subnet ID, primarily to support canister-migration logic that needs to inspect subnet type (e.g., CloudEngine), while keeping the endpoint generic for other future callers.
Changes:
- Introduces
get_subnetquery endpoint and Rust implementation that loads and decodes the registry’sSubnetRecord. - Extends the canister Candid interface (
registry.did/registry_test.did) withGetSubnetRequest,SubnetRecord, andGetSubnetResponse. - Adds Rust integration tests for the new endpoint (registry canister) and a PocketIC feature test for CloudEngine subnet type.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/registry/canister/unreleased_changelog.md | Documents the new get_subnet endpoint addition. |
| rs/registry/canister/tests/get_subnet.rs | Adds PocketIC-based registry-canister tests for success/missing/unknown subnet ID cases. |
| rs/registry/canister/src/lib.rs | Exposes the new get_subnet module from the crate. |
| rs/registry/canister/src/get_subnet.rs | Implements request/response types, SubnetRecord decoding, and registry lookup logic. |
| rs/registry/canister/canister/registry.did | Extends the public registry canister Candid interface with get_subnet. |
| rs/registry/canister/canister/registry_test.did | Mirrors the Candid additions for the test interface. |
| rs/registry/canister/canister/canister.rs | Wires the new get_subnet query method into the canister entrypoints. |
| packages/pocket-ic/tests/icp_features.rs | Adds a feature-gated PocketIC test asserting CloudEngine subnet type via get_subnet. |
| packages/pocket-ic/Cargo.toml | Adds head_nns feature flag and a dev-dependency needed by the new test. |
| packages/pocket-ic/BUILD.bazel | Enables head_nns crate feature for the -head-nns test variant and adds Bazel deps. |
| Cargo.lock | Updates lockfile for the new dependency addition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
|
|
||
| impl Registry { | ||
| pub fn get_subnet_record(&self, request: GetSubnetRequest) -> Result<SubnetRecord, String> { | ||
| let subnet_id = request |
There was a problem hiding this comment.
More a question than anything, but does this type of registry endpoint usually have a rate limit or other protections?
There was a problem hiding this comment.
The new endpoint is analogous to the existing endpoint get_subnet_for_canister in terms of security measures.
This PR introduces a
get_subnetendpoint on the NNS registry canister that returns the SubnetRecord for a given subnet ID. The immediate motivation for this change is to enable the migration canister check if a subnet is a cloud engine (by retrieving the subnet type from the corresponding subnet record), but we implement a genericget_subnetendpoint in this PR to cover future use cases, too.The sizes of subnet records on mainnet vary between 732 bytes and 2103 bytes and take between ~100k and ~150k instructions to produce.