feat(release-runscript): set engine-controller authorized_caller on upgrade#10728
Draft
pietrodimarco-dfinity wants to merge 1 commit into
Draft
feat(release-runscript): set engine-controller authorized_caller on upgrade#10728pietrodimarco-dfinity wants to merge 1 commit into
pietrodimarco-dfinity wants to merge 1 commit into
Conversation
…pgrade
The release-runscript hardcoded `()` as the engine-controller upgrade
argument, which cannot set the canister's authorized caller. Change the
Create Proposal Texts step to prompt for the authorized caller principal
and build the upgrade arg:
(opt record {
authorized_caller = opt principal "<CALLER>";
initial_dkg_subnet_id = null
})
This candid string is passed as CANDID_ARGS to
prepare-nns-upgrade-proposal-text.sh, which encodes it with
`didc encode | xxd -r -p` to compute the proposal's arg_hash and
verification section.
An empty answer emits `authorized_caller = null`, making the canister
fall back to its built-in default caller (rather than the previous
behavior of producing an invalid `opt principal ""`). The canister
re-applies args on every post_upgrade, so the caller must be supplied
each release to retain it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the release-runscript NNS release tool so engine-controller upgrades can set (or retain) authorized_caller by prompting for a principal and constructing a non-empty Candid upgrade argument, instead of hardcoding ().
Changes:
- Prompt for an engine-controller
authorized_callerprincipal during proposal-text generation. - Build a structured
(opt record { ... })Candid upgrade arg (withinitial_dkg_subnet_id = null) for engine-controller upgrades.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+337
to
+344
| let new_caller = input( | ||
| "Authorized caller principal for engine-controller? (leave empty for canister default)", | ||
| )?; | ||
| let authorized_caller = if new_caller.is_empty() { | ||
| "null".to_string() | ||
| } else { | ||
| format!("opt principal \"{new_caller}\"") | ||
| }; |
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.
What
The
release-runscriptrelease tool hardcoded()as the engine-controller upgrade argument in the Create Proposal Texts step.()cannot set the canister'sauthorized_caller, so it was impossible to change the authorized caller through the normal release flow.This changes the engine-controller branch to prompt for the authorized caller principal and build the upgrade arg:
Why
The engine-controller's
post_upgradere-applies its init args on every upgrade (apply_init_argsinrs/engine_controller/canister/canister.rs). Ifauthorized_callerisnull, the canister falls back to its built-inDEFAULT_AUTHORIZED_CALLER. So to set (or retain) a specific authorized caller, the value must be supplied on each release — which the tool previously couldn't do.The candid string is passed as
CANDID_ARGStoprepare-nns-upgrade-proposal-text.sh, which encodes it viadidc encode | xxd -r -pto compute the proposal'sarg_hashand the verification section.Notes for reviewers
authorized_caller = null(→ canister default). Previously an empty answer would have produced an invalidopt principal ""that failsdidc encode.initial_dkg_subnet_idis leftnull(canister falls back toDEFAULT_INITIAL_DKG_SUBNET_ID). It is not currently promptable — say the word if you'd like it exposed too.service : (opt EngineControllerInitArgs)inrs/engine_controller/engine_controller.did.Testing status
rustfmtapplied. Localcargo clippyover the full dep tree hit only a pre-existingcollapsible_iflint in the unrelatedic-artifact-poolcrate (local clippy 1.93.0 is newer than the repo's pinned toolchain); nothing from this change. The bazel build/test on the pinned toolchain was still running when the PR was requested — CI will confirm.🤖 Generated with Claude Code