Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(iota-framework/move-stdlib): Deprecated fixed_point32 #5193

Open
wants to merge 8 commits into
base: vm-lang/upstream-nov-dic-24
Choose a base branch
from

Conversation

Dkwcs
Copy link
Contributor

@Dkwcs Dkwcs commented Feb 4, 2025

Description of change

  • Deprecated fixed_point32
  • Added uq32_32 to replace it
    Move fixed_point32 has been deprecated for a new uq32_32 module

Links to any relevant issues

Fixes #5137 .

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How the change has been tested

Run in folder:
iota-framework/packages/move-stdlib/
iota-framework/packages/stardust/
iota-framework/packages/iota-framework/
iota move test

iota-framework-tests:
cargo test

@Dkwcs Dkwcs added the vm-language Issues related to the VM & Language Team label Feb 4, 2025
@Dkwcs Dkwcs self-assigned this Feb 4, 2025
@Dkwcs Dkwcs requested a review from a team as a code owner February 4, 2025 13:47
Copy link

vercel bot commented Feb 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
apps-backend ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:30am
apps-ui-kit ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:30am
rebased-explorer ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:30am
wallet-dashboard ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:30am

@iota-ci iota-ci added the sc-platform Issues related to the Smart Contract Platform group. label Feb 4, 2025
Copy link
Contributor

@miker83z miker83z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the CI error:
thread 'compatibility_tests::test_framework_compatibility' panicked at crates/iota-framework-snapshot/tests/compatibility_tests.rs:37:21: The current IOTA framework 0x000000000000000000000000000000000000000000000000000000000000107a is not compatible with version 1
It probably means that we use fixed_point32 there and we need to change it to uq32_32.

@valeriyr
Copy link
Contributor

valeriyr commented Feb 5, 2025

Check the CI error: thread 'compatibility_tests::test_framework_compatibility' panicked at crates/iota-framework-snapshot/tests/compatibility_tests.rs:37:21: The current IOTA framework 0x000000000000000000000000000000000000000000000000000000000000107a is not compatible with version 1 It probably means that we use fixed_point32 there and we need to change it to uq32_32.

@miker83z @Dkwcs Looks like it is because the royalties public function was changed which is not compatible.

royalties: VecMap<address, FixedPoint32>,
royalties: VecMap<address, UQ32_32>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FixedPoint32 and UQ32_32 structures are compatible on the binary level but I think we still need to change the member type in the Irc27Metadata structure on the Rust side.

@valeriyr
Copy link
Contributor

valeriyr commented Feb 6, 2025

Since the Irc27Metadata structure became incompatible after changing the field type, I propose to leave the Stardust implementation as is. The main reason is that this is a deprecated type itself, and we will not create new instances of it.
Otherwise, we will have to put more effort into updating the type, such as creating a new version of the structure and public functions.

@Dkwcs Dkwcs requested a review from a team as a code owner February 7, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sc-platform Issues related to the Smart Contract Platform group. vm-language Issues related to the VM & Language Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[framework] Extend fixed-point numbers module · MystenLabs/sui@a762240
5 participants