feat: package linkage deploy#16
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements package linkage deployment functionality for Move packages on Sui. The changes introduce support for managing unpublished dependencies, rewriting module addresses, and force-deploying packages at specific addresses. This is a significant enhancement to the package deployment pipeline that adds more flexible dependency management capabilities.
Changes:
- Added support for tracking and deploying unpublished dependencies with automatic address rewriting for zero-address modules
- Implemented linkage table traversal to resolve canonical package versions and transitive dependencies
- Increased gas limits to accommodate more complex deployment scenarios (100x increase from 100B to 10T MIST)
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/movy-sui/Cargo.toml | Added move-core-types dependency for address manipulation |
| crates/movy-sui/src/compile.rs | Added infrastructure for tracking unpublished dependencies, module address rewriting, and immediate dependency resolution |
| crates/movy-replay/src/exec.rs | Implemented canonical dependency resolution with linkage table support and force deployment at specific addresses |
| crates/movy-replay/src/env.rs | Added complex auto-publish logic for unpublished dependencies with zero-address module name rewriting |
| crates/movy/src/sui/fuzz.rs | Increased gas budget for minting from 100B to 10T MIST |
| crates/movy-fuzz/src/operations/sui_replay.rs | Added BackingPackageStore trait bound for package operations |
| crates/movy-fuzz/src/operations/sui_fuzz.rs | Added BackingPackageStore trait bound (with duplicate) |
| Cargo.lock | Removed source references from git dependencies (likely switching to local or different source) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
| .map(|m| m.module.clone()) | ||
| .collect::<Vec<_>>(); | ||
| if modules.len() == 0 { |
There was a problem hiding this comment.
The comparison modules.len() == 0 should use the more idiomatic modules.is_empty() method instead. This is a standard Rust convention that is more readable and expressive.
| if modules.len() == 0 { | |
| if modules.is_empty() { |
| + ObjectSuiStoreCommit | ||
| + BackingStore | ||
| + BackingPackageStore | ||
| + BackingPackageStore |
There was a problem hiding this comment.
The trait bound BackingPackageStore is duplicated. This is redundant and should be removed. Only one declaration of this trait bound is needed.
| + BackingPackageStore |
| let mut canonical_by_original: BTreeMap<ObjectID, (ObjectID, u64)> = BTreeMap::new(); | ||
| for dep in dependencies.iter() { | ||
| let mut original_id = *dep; | ||
| let mut upgraded_id = *dep; | ||
| let mut upgraded_version = 0u64; | ||
| if let Some(object) = self.db.get_object(dep) { | ||
| if let Some(pkg) = object.data.try_as_package() { | ||
| original_id = pkg.original_package_id(); | ||
| if let Some(info) = pkg.linkage_table().get(&original_id) { | ||
| upgraded_id = info.upgraded_id; | ||
| upgraded_version = info.upgraded_version.value(); | ||
| } else { | ||
| upgraded_version = object.version().value(); | ||
| } | ||
| } else { | ||
| upgraded_version = object.version().value(); | ||
| } | ||
| } | ||
| let entry = canonical_by_original | ||
| .entry(original_id) | ||
| .or_insert((upgraded_id, upgraded_version)); | ||
| if upgraded_version > entry.1 { | ||
| *entry = (upgraded_id, upgraded_version); | ||
| } | ||
| } | ||
| let canonical_deps: Vec<ObjectID> = | ||
| canonical_by_original.values().map(|v| v.0).collect(); | ||
| debug!( | ||
| "Publish deps list (normalized): {}", | ||
| canonical_deps.iter().map(|v| v.to_string()).join(", ") | ||
| ); |
There was a problem hiding this comment.
There's duplicated logic for computing canonical_by_original. The code from lines 309-339 computes canonical dependencies from the initial dependencies list, then lines 341-397 recompute the same canonical mapping for an expanded set of dependencies (including transitive ones). The first computation at lines 309-339 appears unnecessary since its result (canonical_deps) is never used - only the second computation's result is used at line 409. Consider removing the first block to eliminate redundant code.
| let mut canonical_by_original: BTreeMap<ObjectID, (ObjectID, u64)> = BTreeMap::new(); | |
| for dep in dependencies.iter() { | |
| let mut original_id = *dep; | |
| let mut upgraded_id = *dep; | |
| let mut upgraded_version = 0u64; | |
| if let Some(object) = self.db.get_object(dep) { | |
| if let Some(pkg) = object.data.try_as_package() { | |
| original_id = pkg.original_package_id(); | |
| if let Some(info) = pkg.linkage_table().get(&original_id) { | |
| upgraded_id = info.upgraded_id; | |
| upgraded_version = info.upgraded_version.value(); | |
| } else { | |
| upgraded_version = object.version().value(); | |
| } | |
| } else { | |
| upgraded_version = object.version().value(); | |
| } | |
| } | |
| let entry = canonical_by_original | |
| .entry(original_id) | |
| .or_insert((upgraded_id, upgraded_version)); | |
| if upgraded_version > entry.1 { | |
| *entry = (upgraded_id, upgraded_version); | |
| } | |
| } | |
| let canonical_deps: Vec<ObjectID> = | |
| canonical_by_original.values().map(|v| v.0).collect(); | |
| debug!( | |
| "Publish deps list (normalized): {}", | |
| canonical_deps.iter().map(|v| v.to_string()).join(", ") | |
| ); |
…36-to-8406cf86 Codex-generated pull request
…36-to-8406cf86-taolmf refactor: deduplicate zero-address module mapping logic
No description provided.