Skip to content

Commit d342c4d

Browse files
NikolaMilosaIDX GitHub Automation
andauthored
chore(engine-controller): remove the subnet admin normalization (#10704)
For ease of use and to protect against unwanted changes, this canister forced the authorized caller to be a subnet admin. We are now removing that requirement. --------- Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
1 parent 020422d commit d342c4d

3 files changed

Lines changed: 4 additions & 60 deletions

File tree

rs/engine_controller/canister/canister.rs

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -296,40 +296,14 @@ fn ensure_only_allowed_fields_set(payload: &UpdateSubnetPayload) -> Result<(), S
296296
}
297297
}
298298

299-
/// Ensures that the configured `AUTHORIZED_CALLER` (the engine controller's
300-
/// "super admin") is always present in the resulting admin list, even if the
301-
/// caller forgot to include it.
302-
fn normalize_subnet_admins(admins: Vec<PrincipalId>) -> Vec<PrincipalId> {
303-
let super_admin = PrincipalId(AUTHORIZED_CALLER.with(|c| *c.borrow()));
304-
let mut admins = admins;
305-
if !admins.contains(&super_admin) {
306-
admins.push(super_admin);
307-
}
308-
admins
309-
}
310-
311299
/// Proxies to the registry's `update_subnet` endpoint. Only `subnet_admins`
312300
/// and `is_halted` may be updated through this path; every other field must be
313301
/// left at its default value (`None` / `false`) or the call is rejected.
314-
///
315-
/// The `subnet_admins` list is always normalized to include the engine
316-
/// controller's authorized caller (the super admin), so callers cannot
317-
/// accidentally lock the controller out of the subnet.
318302
#[update]
319303
async fn update_subnet(payload: UpdateSubnetPayload) -> Result<(), String> {
320304
ensure_authorized()?;
321305
ensure_only_allowed_fields_set(&payload)?;
322306

323-
// Normalize `subnet_admins` so the super admin is always present.
324-
// The caller may omit the field entirely (no change requested), but if
325-
// they do supply one, we treat it as the source of truth and add the
326-
// super admin if missing.
327-
#[allow(unused_mut)]
328-
let mut payload = payload;
329-
if let Some(admins) = payload.subnet_admins {
330-
payload.subnet_admins = Some(normalize_subnet_admins(admins));
331-
}
332-
333307
Call::unbounded_wait(REGISTRY_CANISTER_ID.into(), "update_subnet")
334308
.with_arg(payload)
335309
.await

rs/engine_controller/canister/tests.rs

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -138,37 +138,3 @@ fn ensure_only_allowed_fields_set_rejects_non_default_gossip_flag() {
138138
"error must mention the non-default bool: {err}"
139139
);
140140
}
141-
142-
#[test]
143-
fn normalize_subnet_admins_adds_super_admin_when_missing() {
144-
let other = PrincipalId::new_user_test_id(7);
145-
let normalized = normalize_subnet_admins(vec![other]);
146-
147-
let super_admin = PrincipalId(default_authorized_caller());
148-
assert!(
149-
normalized.contains(&super_admin),
150-
"super admin must be present after normalization"
151-
);
152-
assert!(
153-
normalized.contains(&other),
154-
"other admins must be preserved"
155-
);
156-
}
157-
158-
#[test]
159-
fn normalize_subnet_admins_keeps_list_intact_when_super_admin_present() {
160-
let super_admin = PrincipalId(default_authorized_caller());
161-
let other = PrincipalId::new_user_test_id(8);
162-
let input = vec![other, super_admin];
163-
let normalized = normalize_subnet_admins(input.clone());
164-
assert_eq!(
165-
normalized, input,
166-
"list must not be reordered or duplicated when super admin is already present"
167-
);
168-
}
169-
170-
#[test]
171-
fn normalize_subnet_admins_handles_empty_input() {
172-
let normalized = normalize_subnet_admins(vec![]);
173-
assert_eq!(normalized, vec![PrincipalId(default_authorized_caller())]);
174-
}

rs/engine_controller/unreleased_changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ on the process that this file is part of, see
1515

1616
## Changed
1717

18+
* `update_subnet` no longer forces the engine controller's authorized caller
19+
(super admin) into the `subnet_admins` list. The supplied list is now
20+
forwarded to the registry as-is.
21+
1822
## Deprecated
1923

2024
## Removed

0 commit comments

Comments
 (0)