chore: Let PocketIC library depend on public ic-management-canister-types #3986
Conversation
This file contains 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
michael-weigelt
added
the
CI_ALL_BAZEL_TARGETS
mraszyk
reviewed
Feb 18, 2025
Comment on lines
-184
to
-195
| fn candid_equality_test() { | ||
| let declared_interface_str = | ||
| std::fs::read_to_string(std::env::var_os("IC_DID").unwrap()).unwrap(); | ||
| let declared_interface = CandidSource::Text(&declared_interface_str); | ||
|
|
||
| candid::export_service!(); | ||
| let implemented_interface_str = __export_service(); | ||
| let implemented_interface = CandidSource::Text(&implemented_interface_str); | ||
|
|
||
| let result = service_equal(declared_interface, implemented_interface); | ||
| assert!(result.is_ok(), "{:?}\n\n", result.unwrap_err()); | ||
| } |
There was a problem hiding this comment.
Do we have this test somewhere?
mraszyk
reviewed
Feb 18, 2025
Comment on lines
+88
to
125
| /// Types of curves that can be used for threshold key derivation (vetKD). | ||
| /// ```text | ||
| /// (variant { bls12_381_g2; }) | ||
| /// ``` | ||
| #[derive( | ||
| Copy, | ||
| Clone, | ||
| Eq, | ||
| PartialEq, | ||
| Ord, | ||
| PartialOrd, | ||
| Hash, | ||
| Debug, | ||
| CandidType, | ||
| Deserialize, | ||
| EnumIter, | ||
| Serialize, | ||
| )] | ||
| pub enum VetKdCurve { | ||
| #[serde(rename = "bls12_381_g2")] | ||
| #[allow(non_camel_case_types)] | ||
| Bls12_381_G2, | ||
| } | ||
|
|
||
| /// Unique identifier for a key that can be used for threshold key derivation | ||
| /// (vetKD). The name is just an identifier, but it may be used to convey | ||
| /// some information about the key (e.g. that the key is meant to be used for | ||
| /// testing purposes). | ||
| /// ```text | ||
| /// (record { curve: vetkd_curve; name: text}) | ||
| /// ``` | ||
| #[derive( | ||
| Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, CandidType, Deserialize, Serialize, | ||
| )] | ||
| pub struct VetKdKeyId { | ||
| pub curve: VetKdCurve, | ||
| pub name: String, | ||
| } |
There was a problem hiding this comment.
this is not publicly released yet (dfinity/portal#3763)
mraszyk
reviewed
Feb 18, 2025
Comment on lines
+145
to
149
| pub enum MasterPublicKeyId { | ||
| Ecdsa(EcdsaKeyId), | ||
| Schnorr(SchnorrKeyId), | ||
| VetKd(VetKdKeyId), | ||
| } |
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.
This is an imperfect step in the right direction.
Follow-ups and explanations:
management_canistermodule are different from the ones in the public crate. For now, they are importedastheir old name, in order to reduce changes in this PR. They should probably be consistently renamed. Depending on whether these types are exposed to PocketIC users, this might be a breaking change, hence this PR does not perform this renaming step.management_canisterstill contains a few types, because they are not defined YET in the current version of the public crate, or they are more recent versions of those types (e.g., with an additional field). As the public crate is upgraded and contains all necessary types, this module will hopefully disappear, and all necessary management canister types can be imported via the public crate. For experimental pre-releases, theic-management-canister-typescrate and thepocket-iccrate have to both publish beta-releases on crates.io.This PR changes more crates in
ic/rs/...than it should, becauseasthe old name.To the codeowners of these crates:
If you have only imported PocketIC for the management canister types, please remove the dependency and depend on the public crate. If you find yourself depending on ic-management-canister-types-private or
"//rs/types/management_canister_types", please also move to the public crate.