Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Conversation

@mraszyk
Copy link
Contributor

@mraszyk mraszyk commented Sep 22, 2022

To avoid the special case of update calls to the Management Canister (aaaaa-aa) with the method name provisional_create_canister_with_cycles and the effective canister id equal to that of the Management Canister (aaaaa-aa), I propose to change the specification and implementation in several steps as follows:

  1. the specification is relaxed (this PR):
  • rename effective canister id to effective principal id
  • effective principal id can be equal to subnet id for requests submitted to nodes in testnets
  • effective principal id can be equal to subnet id in certificate delegation checking
  1. checks in agent-rs are relaxed according to the relaxed specification

  2. IC testnet (and potentially also boundary node) implementation is incrementally updated

  3. the specification is tightened (future PR) to avoid the special case

  4. checks in agent-rs are tightened according to the tightened specification

@mraszyk mraszyk requested a review from a team as a code owner September 22, 2022 08:22
@netlify
Copy link

netlify bot commented Sep 22, 2022

Deploy Preview for ic-interface-spec ready!

Name Link
🔨 Latest commit 1c62d94
🔍 Latest deploy log https://app.netlify.com/sites/ic-interface-spec/deploys/632c1b5a0bffb700080080eb
😎 Deploy Preview https://deploy-preview-94--ic-interface-spec.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Dfinity-Bjoern
Copy link
Contributor

I am very reluctant to do this. The main reason is that provisional_canister_create_with_cycles should not be in the main specification anyway. It is a feature that is not and should never be available in mainnet. Morally, we should remove it from the spec rather than have this artifact bleed into all parts of the spec.

@mraszyk
Copy link
Contributor Author

mraszyk commented Sep 23, 2022

I am very reluctant to do this. The main reason is that provisional_canister_create_with_cycles should not be in the main specification anyway. It is a feature that is not and should never be available in mainnet. Morally, we should remove it from the spec rather than have this artifact bleed into all parts of the spec.

I think the provisional API is necessary to bootstrap canisters after creating a new IC so it must fundamentally be part of the spec. It must have been necessary after Genesis, too, before it was disabled unless the mainnet started off with some canisters hard-coded in the state.

@mraszyk
Copy link
Contributor Author

mraszyk commented Sep 23, 2022

unless the mainnet started off with some canisters hard-coded in the state

in which case we should change the definition of the initial state of the IC because it wouldn't be possible to create any canisters from the current initial state (w/o having the provisional API)

@mraszyk
Copy link
Contributor Author

mraszyk commented Sep 23, 2022

Moreover, allowing subnet id to be effective principal id for read state requests (in step 3 for BNs and in step 4 for Interface Spec) would allow to query subnet information (i.e., the path /subnet/) without providing a (potentially non-existing) canister id from the canister range of a subnet to which such a request is targetted.

@mraszyk mraszyk marked this pull request as draft September 27, 2022 19:33
@mraszyk
Copy link
Contributor Author

mraszyk commented Oct 18, 2022

Not needed anymore to avoid the special case of update calls to the Management Canister (aaaaa-aa) with the method name provisional_create_canister_with_cycles and the effective canister id equal to that of the Management Canister (aaaaa-aa).

@mraszyk mraszyk closed this Oct 18, 2022
@mraszyk mraszyk deleted the mraszyk/allow-subnet-ecid branch December 17, 2022 09:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants