[move lockfile] rename dependency name field to id and add a sepa…

…rate `name` field to store manifest name (#19387)

## Description

As discussed in
#14178 (comment)
(second and third point), a few changes to the lockfile:
- the `name` field in `dependencies` is renamed to `id` to better
reflect the meaning in the dependency graph (the packages are
discriminated by their identifier, as resolved by the hook, which is not
necessarily their manifest name)
- added a `name` field which will store the dependency manifest name
(this is needed to show user-friendly error messages using the package
manifest name instead of identifier which may be confusing)
- bumped lockfile version to 3

This is part of the work to enable compiling against on-chain
dependencies #14178.

cc @rvantonder @amnn

## Test plan 

All changes are covered by existing unit tests.

---

## 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): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [x] CLI: Introduce lock file version 3, which renames a dependency's
`name` field to `id` and introduces a separate `name` field that stores
the package's name from its manifest for improved error reporting. Older
lock files will be ignored when resolving dependencies so that this
information is always available.
- [ ] Rust SDK:
- [ ] REST API:

crates/sui-core/src/unit_tests/move_package_management_tests.rs

@@ -50,7 +50,7 @@ async fn test_manage_package_update() {
         # @generated by Move, please check-in and do not edit manually.
 
         [move]
-        version = 2
+        version = 3
         manifest_digest = "919A5B078B47AD46674F36E1605578927D5BC4536A7646D78D1320A25DDD57CC"
         deps_digest = "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855"
 

crates/sui-core/src/unit_tests/move_package_publish_tests.rs

@@ -218,32 +218,32 @@ async fn test_generate_lock_file() {
         # @generated by Move, please check-in and do not edit manually.
 
         [move]
-        version = 2
+        version = 3
         manifest_digest = "4C5606BF71339416027A58BDB5BA2EF2F5E0929FCE98BAB8AFFCBC447AFE3A23"
         deps_digest = "3C4103934B1E040BB6B23F1D610B4EF9F2F1166A50A104EADCF77467C004C600"
         dependencies = [
-          { name = "Examples" },
-          { name = "Sui" },
+          { id = "Examples", name = "Examples" },
+          { id = "Sui", name = "Sui" },
         ]
 
         [[move.package]]
-        name = "Examples"
+        id = "Examples"
         source = { local = "../object_basics" }
 
         dependencies = [
-          { name = "Sui" },
+          { id = "Sui", name = "Sui" },
         ]
 
         [[move.package]]
-        name = "MoveStdlib"
+        id = "MoveStdlib"
         source = { local = "../../../../../sui-framework/packages/move-stdlib" }
 
         [[move.package]]
-        name = "Sui"
+        id = "Sui"
         source = { local = "../../../../../sui-framework/packages/sui-framework" }
 
         dependencies = [
-          { name = "MoveStdlib" },
+          { id = "MoveStdlib", name = "MoveStdlib" },
         ]
 
         [move.toolchain-version]

crates/sui-source-validation/src/toolchain.rs

@@ -40,7 +40,7 @@ use tracing::{debug, info};
 
 pub(crate) const CURRENT_COMPILER_VERSION: &str = env!("CARGO_PKG_VERSION");
 const LEGACY_COMPILER_VERSION: &str = CURRENT_COMPILER_VERSION; // TODO: update this when Move 2024 is released
-const PRE_TOOLCHAIN_MOVE_LOCK_VERSION: u64 = 0; // Used to detect lockfiles pre-toolchain versioning support
+const PRE_TOOLCHAIN_MOVE_LOCK_VERSION: u16 = 0; // Used to detect lockfiles pre-toolchain versioning support
 const CANONICAL_UNIX_BINARY_NAME: &str = "sui";
 const CANONICAL_WIN_BINARY_NAME: &str = "sui.exe";
 

external-crates/move/crates/move-package/src/lock_file/schema.rs

@@ -31,7 +31,8 @@ use super::LockFile;
 /// V0: Base version.
 /// V1: Adds toolchain versioning support.
 /// V2: Adds support for managing addresses on package publish and upgrades.
-pub const VERSION: u64 = 2;
+/// V3: Renames dependency `name` field to `id` and adds a `name` field to store the name from the manifest.
+pub const VERSION: u16 = 3;
 
 /// Table for storing package info under an environment.
 const ENV_TABLE_NAME: &str = "env";
@@ -56,8 +57,8 @@ pub struct Packages {
 
 #[derive(Deserialize)]
 pub struct Package {
-    /// The name of the package (corresponds to the name field from its source manifest).
-    pub name: String,
+    /// Package identifier (as resolved by the package hook).
+    pub id: String,
 
     /// Where to find this dependency.  Schema is not described in terms of serde-compatible
     /// structs, so it is deserialized into a generic data structure.
@@ -73,6 +74,9 @@ pub struct Package {
 
 #[derive(Deserialize)]
 pub struct Dependency {
+    /// Package identifier (as resolved by the package hook).
+    pub id: String,
+
     /// The name of the dependency (corresponds to the key for the dependency in the depending
     /// package's source manifest).
     pub name: String,
@@ -110,7 +114,7 @@ pub struct ManagedPackage {
 
 #[derive(Serialize, Deserialize)]
 pub struct Header {
-    pub version: u64,
+    pub version: u16,
     /// A hash of the manifest file content this lock file was generated from computed using SHA-256
     /// hashing algorithm.
     pub manifest_digest: String,
@@ -199,9 +203,9 @@ impl Header {
         let Schema { move_: header } =
             toml::de::from_str::<Schema<Header>>(contents).context("Deserializing lock header")?;
 
-        if header.version > VERSION {
+        if header.version != VERSION {
             bail!(
-                "Lock file format is too new, expected version {} or below, found {}",
+                "Lock file format mismatch, expected version {}, found {}",
                 VERSION,
                 header.version
             );
@@ -252,7 +256,8 @@ pub fn update_dependency_graph(
         .as_table_mut()
         .ok_or_else(|| anyhow!("Could not find or create move table in Move.lock"))?;
 
-    // Update `manifest_digest` and `deps_digest` in `[move]` table section.
+    // Update `version`, `manifest_digest`, and `deps_digest` in `[move]` table section.
+    move_table["version"] = value(VERSION as i64);
     move_table["manifest_digest"] = value(manifest_digest);
     move_table["deps_digest"] = value(deps_digest);
 

external-crates/move/crates/move-package/src/resolution/dependency_graph.rs

@@ -232,6 +232,7 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
         let lock_file = File::open(lock_path);
         let digest_and_lock_contents = lock_file
             .map(|mut lock_file| match schema::Header::read(&mut lock_file) {
+                Ok(header) if header.version < schema::VERSION => None, // outdated lock file - regenerate
                 Ok(header) => Some((header.manifest_digest, header.deps_digest, lock_string_opt)),
                 Err(_) => None, // malformed header - regenerate lock file
             })
@@ -1066,70 +1067,72 @@ impl DependencyGraph {
         package_graph.add_node(root_package_id);
 
         for schema::Dependency {
-            name,
+            id: dep_id,
+            name: dep_name,
             subst,
             digest,
         } in packages.root_dependencies.into_iter().flatten()
         {
             package_graph.add_edge(
                 root_package_id,
-                Symbol::from(name.as_str()),
+                PackageIdentifier::from(dep_id.as_str()),
                 Dependency {
                     mode: DependencyMode::Always,
                     subst: subst.map(parse_substitution).transpose()?,
                     digest: digest.map(Symbol::from),
                     dep_override: false,
-                    dep_name: PM::PackageName::from(name),
+                    dep_name: PM::PackageName::from(dep_name),
                 },
             );
         }
 
         for schema::Dependency {
-            name,
+            id: dep_id,
+            name: dep_name,
             subst,
             digest,
         } in packages.root_dev_dependencies.into_iter().flatten()
         {
             package_graph.add_edge(
                 root_package_id,
-                Symbol::from(name.as_str()),
+                PackageIdentifier::from(dep_id.as_str()),
                 Dependency {
                     mode: DependencyMode::DevOnly,
                     subst: subst.map(parse_substitution).transpose()?,
                     digest: digest.map(Symbol::from),
                     dep_override: false,
-                    dep_name: PM::PackageName::from(name.as_str()),
+                    dep_name: PM::PackageName::from(dep_name),
                 },
             );
         }
 
         // Fill in the remaining dependencies, and the package source information from the lock
         // file.
         for schema::Package {
-            name: pkg_name,
+            id: pkg_id,
             source,
             version,
             dependencies,
             dev_dependencies,
         } in packages.packages.into_iter().flatten()
         {
-            let pkg_name = PM::PackageName::from(pkg_name.as_str());
-            let source = parse_dependency(pkg_name.as_str(), source)
-                .with_context(|| format!("Deserializing dependency '{pkg_name}'"))?;
+            let pkg_id = PackageIdentifier::from(pkg_id.as_str());
+            let source = parse_dependency(pkg_id.as_str(), source)
+                .with_context(|| format!("Deserializing dependency '{pkg_id}'"))?;
 
             let source = match source {
                 PM::Dependency::Internal(source) => source,
                 PM::Dependency::External(resolver) => {
-                    bail!("Unexpected dependency '{pkg_name}' resolved externally by '{resolver}'");
+                    bail!("Unexpected dependency '{pkg_id}' resolved externally by '{resolver}'");
                 }
             };
 
             if source.subst.is_some() {
-                bail!("Unexpected 'addr_subst' in source for '{pkg_name}'")
+                bail!("Unexpected 'addr_subst' in source for '{pkg_id}'")
             }
 
             if source.digest.is_some() {
-                bail!("Unexpected 'digest' in source for '{pkg_name}'")
+                bail!("Unexpected 'digest' in source for '{pkg_id}'")
             }
 
             let pkg = Package {
@@ -1138,7 +1141,7 @@ impl DependencyGraph {
                 version: version.map(Symbol::from),
             };
 
-            match package_table.entry(pkg_name) {
+            match package_table.entry(pkg_id) {
                 Entry::Vacant(entry) => {
                     entry.insert(pkg);
                 }
@@ -1148,22 +1151,23 @@ impl DependencyGraph {
                 Entry::Occupied(entry) => {
                     bail!(
                         "Conflicting dependencies found:\n{0} = {1}\n{0} = {2}",
-                        pkg_name,
+                        pkg_id,
                         PackageWithResolverTOML(entry.get()),
                         PackageWithResolverTOML(&pkg),
                     );
                 }
             };
 
             for schema::Dependency {
+                id: dep_id,
                 name: dep_name,
                 subst,
                 digest,
             } in dependencies.into_iter().flatten()
             {
                 package_graph.add_edge(
-                    pkg_name,
-                    PM::PackageName::from(dep_name.as_str()),
+                    pkg_id,
+                    PackageIdentifier::from(dep_id.as_str()),
                     Dependency {
                         mode: DependencyMode::Always,
                         subst: subst.map(parse_substitution).transpose()?,
@@ -1175,14 +1179,15 @@ impl DependencyGraph {
             }
 
             for schema::Dependency {
+                id: dep_id,
                 name: dep_name,
                 subst,
                 digest,
             } in dev_dependencies.into_iter().flatten()
             {
                 package_graph.add_edge(
-                    pkg_name,
-                    PM::PackageName::from(dep_name.as_str()),
+                    pkg_id,
+                    PackageIdentifier::from(dep_id.as_str()),
                     Dependency {
                         mode: DependencyMode::DevOnly,
                         subst: subst.map(parse_substitution).transpose()?,
@@ -1232,7 +1237,7 @@ impl DependencyGraph {
         for (id, pkg) in &self.package_table {
             writeln!(writer, "\n[[move.package]]")?;
 
-            writeln!(writer, "name = {}", str_escape(id.as_str())?)?;
+            writeln!(writer, "id = {}", str_escape(id.as_str())?)?;
             writeln!(writer, "source = {}", PackageTOML(pkg))?;
             if let Some(version) = &pkg.version {
                 writeln!(writer, "version = {}", str_escape(version.as_str())?)?;
@@ -1566,20 +1571,23 @@ impl<'a> fmt::Display for PackageWithResolverTOML<'a> {
 impl<'a> fmt::Display for DependencyTOML<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let DependencyTOML(
-            name,
+            id,
             Dependency {
                 mode: _,
                 subst,
                 digest,
                 dep_override: _,
-                dep_name: _,
+                dep_name,
             },
         ) = self;
 
         f.write_str("{ ")?;
 
-        write!(f, "name = ")?;
-        f.write_str(&str_escape(name.as_str())?)?;
+        write!(f, "id = ")?;
+        f.write_str(&str_escape(id.as_str())?)?;
+
+        write!(f, ", name = ")?;
+        f.write_str(&str_escape(dep_name.as_str())?)?;
 
         if let Some(digest) = digest {
             write!(f, ", digest = ")?;

external-crates/move/crates/move-package/src/source_package/manifest_parser.rs

@@ -329,7 +329,7 @@ fn parse_address_literal(address_str: &str) -> Result<AccountAddress, AccountAdd
     AccountAddress::from_hex_literal(address_str)
 }
 
-pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency> {
+pub fn parse_dependency(dep_id: &str, mut tval: TV) -> Result<PM::Dependency> {
     let Some(table) = tval.as_table_mut() else {
         bail!("Malformed dependency {}", tval);
     };
@@ -432,7 +432,7 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
                     .ok_or_else(|| anyhow!("'subdir' not a string"))?,
             };
 
-            let package_name = Symbol::from(dep_name);
+            let package_name = Symbol::from(dep_id);
 
             PM::DependencyKind::Custom(PM::CustomDepInfo {
                 node_url,