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

[vm-rewrite] Fix up sui-side tests #21178

Draft
wants to merge 1 commit into
base: tzakian/vm-rewrite-adapter-12
Choose a base branch
from

Conversation

tzakian
Copy link
Contributor

@tzakian tzakian commented Feb 11, 2025

Description

Describe the changes or additions included in this PR.

Test plan

How did you test the new or updated feature?


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented Feb 11, 2025

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

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2025 6:29pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2025 6:29pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2025 6:29pm

Fix a number of different things in order to get Sui-side tests passing:
* Handle including the system packages for all linkages outside of
  system transactions (was missing this for publish and upgrade commands
  where this is needed for the resolution of the upgrade cap and the
  like).
* Handle typetags, and converting them to be runtime type tags before
  flowing them into the VM (always). NB: still some work here in the
  object runtime.
* Handle duplicate modules in packages. This is now an error when
  constructing a `MovePackage` and previously it was an error in the VM.
  However duplicate modules in package are no longer expressible in the
  VM's public interface, so moving this into the Move package seemed
  like the right place.
  - Note for the future: This change may need to be protocol versioned
    as it's a change outside of sui types, however I don't think we need
    to version it since we will check this in the old VM before creating
    the package, and therefore would never hit the new error with it.
* Change in graphql test is just a change in the order of the headers
  and not a meaningful change.
* Ignored one test that is failing for an unrelated reason as far as I
  can tell, once we do another rebase I'll re-enable and dig in to see
  if there's still a problem with it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant