[sui-framework/package] Publisher checks recognise upgrades
#12026
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
Change the behaviour of functions like: ``` sui::package::from_package<T>(self: &Publisher): bool sui::package::from_module<T>(self: &Publisher): bool ``` to return `true` when `T` is from an upgraded version of the package associated with `Publisher`. Previously it was impossible to check the package ownership of a type introduced in an upgraded package (the upgraded type's type name didn't match the original package's `Publisher` and it was impossible to create a `Publisher` for the upgraded package, because it requires an OTW which we can't create for upgraded packages). This support was added by introducing a new stdlib function: `std::typename::get_original`, as of protocol version 11, which produces a runtime representation of the type that uses original (aka runtime) package IDs instead of defining package IDs (the default behaviour for `std::typename::get` and any other case where a runtime representation of the type is converted into an on-chain or storage representation of a type). Test Plan: New transactional test cases for `std::type_name::get_original` and `sui::package::from_package`: ``` $ cargo nextest run -- upgrade/type_names.move $ cargo nextest run -- upgrade/publisher.move ```
amnn
requested review from
rvantonder,
awelc,
tnowacki,
tzakian,
damirka and
dariorussi
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
amnn
commented
May 16, 2023
| task 11 'view-object'. lines 78-78: | ||
| Owner: Account Address ( A ) | ||
| Version: 8 | ||
| Contents: A0::m::Canary {id: sui::object::UID {id: sui::object::ID {bytes: fake(7,0)}}, addr: vector[98u8, 55u8, 50u8, 55u8, 55u8, 53u8, 48u8, 100u8, 98u8, 50u8, 54u8, 48u8, 49u8, 52u8, 101u8, 53u8, 54u8, 101u8, 99u8, 102u8, 57u8, 50u8, 99u8, 100u8, 51u8, 50u8, 101u8, 52u8, 100u8, 48u8, 54u8, 56u8, 50u8, 51u8, 56u8, 56u8, 50u8, 49u8, 54u8, 50u8, 51u8, 99u8, 48u8, 52u8, 54u8, 57u8, 56u8, 100u8, 57u8, 54u8, 50u8, 57u8, 48u8, 56u8, 100u8, 49u8, 50u8, 48u8, 97u8, 53u8, 49u8, 54u8, 100u8, 48u8]} |
There was a problem hiding this comment.
The output is a little hard to make sense of here -- the key thing is that the last one is different from all the rest.
tnowacki
reviewed
May 16, 2023
Comment on lines
+16
to
+20
| /// Return a value representation of the type `T`. Package IDs | ||
| /// that appear in fully qualified type names in the output from | ||
| /// this function are original IDs (the ID of the first version of | ||
| /// the package, even if the type in question was introduced in a | ||
| /// later upgrade). |
There was a problem hiding this comment.
I think this comment is off, just a bad copy paste?
There was a problem hiding this comment.
Ah yes, thanks for spotting -- this has already landed so I'll fix in a follow-up.
| /// this function are original IDs (the ID of the first version of | ||
| /// the package, even if the type in question was introduced in a | ||
| /// later upgrade). | ||
| public native fun get_original<T>(): TypeName; |
There was a problem hiding this comment.
Thoughts on the naming here? Maybe get_with_original_package_id?
It's a bit of a mouthful I know. But get vs get_original feels maybe a bit off as the original here is not describing the TypeName itself, right?
There was a problem hiding this comment.
@damirka and I went back and forth on names, and couldn't come up with an ideal name, but how about get_with_original_ids (plural because there may be multiple in the case of a generic type instantiation) as a trade-off on precision and succinctness?
There was a problem hiding this comment.
Works for me!
(FWIW I never really liked the name get either so overall I don't care too much :P)
ronny-mysten
pushed a commit
that referenced
this pull request
May 17, 2023
## Description Change the behaviour of functions like: ``` sui::package::from_package<T>(self: &Publisher): bool sui::package::from_module<T>(self: &Publisher): bool ``` to return `true` when `T` is from an upgraded version of the package associated with `Publisher`. Previously it was impossible to check the package ownership of a type introduced in an upgraded package (the upgraded type's type name didn't match the original package's `Publisher` and it was impossible to create a `Publisher` for the upgraded package, because it requires an OTW which we can't create for upgraded packages). This support was added by introducing a new stdlib function: `std::typename::get_original`, as of protocol version 11, which produces a runtime representation of the type that uses original (aka runtime) package IDs instead of defining package IDs (the default behaviour for `std::typename::get` and any other case where a runtime representation of the type is converted into an on-chain or storage representation of a type). ## Test Plan New transactional test cases for `std::type_name::get_original` and `sui::package::from_package`: ``` $ cargo nextest run -- upgrade/type_names.move $ cargo nextest run -- upgrade/publisher.move ``` --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [x] user-visible impact - [ ] breaking change for a client SDKs - [x] breaking change for FNs (FN binary must upgrade) - [x] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes As of protocol version 11: - `sui::package::from_package<T>(self: &Publisher)` and `sui::package::from_module<T>(self: &Publisher)` return `true` when `T` is a type introduced at any upgraded version of the package associated with `self: &Publisher` (rather than just the types originally present when that package was published). - `std::type_name::get_original` is introduced as a new stdlib function to return a representation of the type where package IDs have all been normalized to "original" IDs (the ID of the first version of a package) in contrast to the default behaviour of `std::type_name::get` which distinguishes types introduced at different versions of a package by assigning them the ID of the package that introduced them.
amnn
added a commit
that referenced
this pull request
May 17, 2023
Address follow-ups from #12026, related to a typo in a doc comment, and an improvement to the function name. Test Plan: ``` sui$ cargo simtest sui$ env SUI_SKIP_SIMTESTS=1 cargo nextest run ```
6 tasks
amnn
added a commit
that referenced
this pull request
May 18, 2023
…12047) ## Description Address follow-ups from #12026, related to a typo in a doc comment, and an improvement to the function name. ## Test Plan: ``` sui$ cargo simtest sui$ env SUI_SKIP_SIMTESTS=1 cargo nextest run ``` --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [x] user-visible impact - [ ] breaking change for a client SDKs - [x] breaking change for FNs (FN binary must upgrade) - [x] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes Renaming `std::type_name::get_original` (introduced to the framework as protocol version 11) to `std::type_name::get_with_original_ids`.
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.
Description
Change the behaviour of functions like:
to return
truewhenTis from an upgraded version of the package associated withPublisher. Previously it was impossible to check the package ownership of a type introduced in an upgraded package (the upgraded type's type name didn't match the original package'sPublisherand it was impossible to create aPublisherfor the upgraded package, because it requires an OTW which we can't create for upgraded packages).This support was added by introducing a new stdlib function:
std::typename::get_original, as of protocol version 11, which produces a runtime representation of the type that uses original (aka runtime) package IDs instead of defining package IDs (the default behaviour forstd::typename::getand any other case where a runtime representation of the type is converted into an on-chain or storage representation of a type).Test Plan
New transactional test cases for
std::type_name::get_originalandsui::package::from_package:If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes
As of protocol version 11:
sui::package::from_package<T>(self: &Publisher)andsui::package::from_module<T>(self: &Publisher)returntruewhenTis a type introduced at any upgraded version of the package associated withself: &Publisher(rather than just the types originally present when that package was published).std::type_name::get_originalis introduced as a new stdlib function to return a representation of the type where package IDs have all been normalized to "original" IDs (the ID of the first version of a package) in contrast to the default behaviour ofstd::type_name::getwhich distinguishes types introduced at different versions of a package by assigning them the ID of the package that introduced them.