[move] Split bytecode's absint into its own crate #17459
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
There was a problem hiding this comment.
Looks good to me! The trick to re-use v2's version for v0 and v1 is a bit maverick but should work -- generally you can set-up all legacy snapshots however you like, it's just the latest snapshot that needs to be set-up in such a way that you could create a new cut from it (so it has to be uniform).
I spotted a place where we need to add a template string for the new crate, and had a question about whether we needed to tweak dependencies for legacy cuts of sui-verifier but that's it.
| @@ -31,6 +31,8 @@ sui-verifier-v1 = { path = "v1/sui-verifier" } | |||
| sui-verifier-v2 = { path = "v2/sui-verifier" } | |||
| # sui-verifier-$CUT = { path = "$CUT/sui-verifier" } | |||
|
|
|||
| move-abstract-interpreter-latest = { path = "../external-crates/move/crates/move-abstract-interpreter", package = "move-abstract-interpreter" } | |||
There was a problem hiding this comment.
I think you need to put a -$CUT template here for move-abstract-interpreter.
There was a problem hiding this comment.
Good catch.. A bit strange that nothing broke without it. Maybe just that the APIs don't change in the test?
There was a problem hiding this comment.
I suppose if we have no uses of the abstract interpreter from Sui, it won't complain? That might imply that you don't need to add it here at all (until it gets used from the Sui side).
| @@ -15,6 +15,7 @@ move-core-types.workspace = true | |||
| move-vm-config.workspace = true | |||
| move-abstract-stack.workspace = true | |||
|
|
|||
| move-abstract-interpreter = { path = "../../../external-crates/move/crates/move-abstract-interpreter" } | |||
There was a problem hiding this comment.
I would expect something similar in the legacy cuts, no?
There was a problem hiding this comment.
Only the new/latest cuts have this crate. The previous ones have it already through the versioned bytecode verifier
|
The fact that the "cutting a new execution layer" job works despite us not having the |
Description
move-abstract-interpreterabsintmodule was in the bytecode verifier, so this was already versioned for the existing cuts. Howevercontrol_flow_graphwas in the binary format, so this version cut is new. As a result, I have just made one cut for v2 and used it for v0 and v1. I think this is a bit un-orthodox, but preserves the existing behavior just fine (which is the intention of the execution cuts)Test plan
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.