Detect Move version mismatch between local binary and devnet/testnet/mainnet

[bholc646]

trafficstars

errors don’t tell you something like version mismatch between binaries and packages, it will just go unbounded type error , so newcomers basically would have no clue what the error is and can’t fix it

Here is one way:

1. First of all, in the Move.toml of the sui framework, we add a version there
2. In the Rust code somewhere, we also define a constant version to indicate the version of the binary.
3. Before compiling/publishing any module, we check that the two versions above actually match.
4. Each time we make a breaking change to the framework or binary, we bump both version numbers

[bholc646]

@lxfind to write up paragraph on what we are looking for on this

[lxfind]

Updated

[awelc]

First of all, in the Move.toml of the sui framework, we add a version there

I started working on this but now I am thinking if we should make the version number mandatory or not - the former will make it pretty disruptive for whoever wrote the code for Sui already, and the latter will diminish effectiveness of this solution (@lxfind, @todd-mystenlabs, @sblackshear, @damirka?)

[awelc]

I started working on this but now I am thinking if we should make the version number mandatory or not - the former will make it pretty disruptive for whoever wrote the code for Sui already, and the latter will diminish effectiveness of this solution

Never mind, it was the before the morning coffee kind of question... Defaulting "no version number in the Move.toml file" to our first initial version number should do the trick and allow people to add the extra version field to their manifest files incrementally.

[lxfind]

To be clear, I am not proposing to add some kind of generic version number of move.toml: here we only care about the version of the Sui framework (which we have full control of). So we only need the version for Sui framework's move.toml. I don't know whether a generic version would ever be useful.

[awelc]

To be clear, I am not proposing to add some kind of generic version number of move.toml: here we only care about the version of the Sui framework (which we have full control of). So we only need the version for Sui framework's move.toml. I don't know whether a generic version would ever be useful.

Yes, the idea was that this is for Sui Move only, but some code for Sui has (presumably) already been written without developers being aware of the extra field and its meaning.

[bholc646]

Though it may not fully solve the problem, I do think we can use release notes and discord announcements to broadcast this change to the developer ecosystem. We need to start doing this more consistently for changes of this nature anyway.

[awelc]

OK, I was actually thinking of introducing the version number to all Move.toml files, so that we have a mechanism to warn developers of some changes that might otherwise go unnoticed, for example changes in the function (also native) implementation, but perhaps it is an overkill at this point. Plus we can always add this later.

[awelc]

OK, I think I need some advice from those who possibly thought this through more thoroughly, as it seems to me that implementing the proposal in the current form is rather non-trivial.

Embedding version info in the framework's manifest file is easy, but getting to this information does not seem so. The thing is that our input is a path to the user package's manifest file, and the framework's manifest file is its dependency (possible a versioned remote dependency). In order to find out what the framework's version info is, we need to resolve this dependency and get to the framework's manifest file. This resolution happens pretty deep in the build process on the core Move side and is not easily available to the adapter. Technically we could parse user package's manifest file and resolve the dependency ourselves, but while it wouldn't be so bad for local dependencies, it's not ideal for the remote ones.

Am I missing something (@lxfind, @tnowacki)?

[tnowacki]

I think it should be easy to get the information with some small changes to the package system.

At what point do we want to warn/fail again?

[awelc]

I think it should be easy to get the information with some small changes to the package system.

That's really the kind of feedback I was hoping for. I wasn't sure which (if any) of the directions is easier to adopt as I don't yet know the package system (and related code) all that well. That's enough for me to start digging into a possible refactoring.

At what point do we want to warn/fail again?

The way I think about it is that when you build/publish a package, most likely before it's even compiled/verified you check a version number in the Sui framework's Move.toml file (which is a dependency of the build/published package) and compare it with a version number hardcoded in the Sui CLI code you use for building/publishing - you warn (fail) on detecting a version mismatch.

[tnowacki]

I think we call build before doing test and other stuff. So build might be a good place to do it

My thoughts on where to slide this in is in CompiledPackage and pub deps_compiled_units: Vec<(PackageName, CompiledUnitWithSource)> We could start keeping other metadata there, like the version

[tnowacki]

Alternate thought that requires no changes to the package system: Hash the source files and then check the hash?

[awelc]

Alternate thought that requires no changes to the package system: Hash the source files and then check the hash?

Smart! This could be a bit noisy, though, as ANY change to the framework code would result in the warning being issues (rather than only when we believe we are introducing a breaking change).

We could possibly mitigate it by deciding what we are hashing, for example extract function signatures and struct/const declarations but:

• we would still generate the signal if adding things to the framework
• we could still potentially introduce a breaking change in the function's code and not catch it

Still it's quite appealing because of its implementation simplicity, though I am quite sure how to automate hash generation upon framework code change, which would be useful so that we don't have to do it (and remember to do it) each time we change something.

[tnowacki]

Maybe we could hash the compiled modules? would at least avoid comment changes

[awelc]

On the other hand, thinking about it a bit more, people really shouldn't use a version of framework code incompatible with the binary they use to build their apps, so the noise may not be much of a problem.

A separate question is whether we should hash (source of?) native method code if we were to adopt this solution.

And is there a trick of embedding this hash in the binary (obtained with cargo install --locked --git https://github.com/MystenLabs/sui.git --branch "devnet" sui sui-gateway) without needing to hardcode it in sources (which would help automation).

Perhaps we could have sui as a (script?) wrapper around the binary? Would we need it for other tricks in the future?