dependency auto deploy by lbr77 · Pull Request #19 · BitsLabSec/movy

lbr77 · 2026-02-13T04:52:05Z

No description provided.

Copilot

Pull request overview

This pull request implements automatic dependency deployment for Move packages in the Sui blockchain testing environment. The changes enable the system to automatically deploy unpublished local dependencies in the correct topological order before deploying the root package, handling address rewrites and dependency resolution.

Changes:

Added dependency auto-deployment infrastructure including storage for unpublished dependencies and their deployment order
Implemented module rewriting functions to update package addresses and dependency references during deployment
Enhanced deployment flow to automatically deploy local dependencies before the root package

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 14 comments.

File	Description
crates/movy-sui/src/compile.rs	Added fields to track unpublished dependencies, implemented module rewriting functions, enhanced package compilation to extract dependency information
crates/movy-replay/src/exec.rs	Added dependency closure expansion, module self-id rewriting, and force deployment method for packages at specific addresses
crates/movy-replay/src/env.rs	Implemented automatic dependency deployment loop with address mapping and rewriting before root package deployment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T04:56:51Z

crates/movy-sui/src/compile.rs

+fn rewrite_modules_by_id(
+    modules: &mut [CompiledModule],
+    package_id_map: &BTreeMap<ObjectID, ObjectID>,
+) -> Result<(), MovyError> {
+    for module in modules.iter_mut() {
+        rewrite_module_by_id(module, package_id_map)?;
+    }
+    Ok(())
+}
+
+fn rewrite_module_by_id(
+    module: &mut CompiledModule,
+    package_id_map: &BTreeMap<ObjectID, ObjectID>,
+) -> Result<(), MovyError> {
+    let mut addr_index_map: BTreeMap<ObjectID, AddressIdentifierIndex> = BTreeMap::new();
+    for (idx, addr) in module.address_identifiers.iter().enumerate() {
+        addr_index_map.insert(ObjectID::from(*addr), AddressIdentifierIndex(idx as u16));
+    }
+
+    let self_handle_idx = module.self_handle_idx();
+    for idx in 0..module.module_handles.len() {
+        let handle_idx = ModuleHandleIndex(idx as u16);
+        if handle_idx == self_handle_idx {
+            continue;
+        }
+        let handle = module.module_handles[idx].clone();
+        let current_id = ObjectID::from(*module.address_identifier_at(handle.address));
+        let Some(new_id) = package_id_map.get(&current_id) else {
+            continue;
+        };
+        if *new_id == current_id {
+            continue;
+        }
+        let addr_idx = if let Some(existing) = addr_index_map.get(new_id) {
+            *existing
+        } else {
+            let addr = AccountAddress::from(*new_id);
+            module.address_identifiers.push(addr);
+            let new_idx = AddressIdentifierIndex((module.address_identifiers.len() - 1) as u16);
+            addr_index_map.insert(*new_id, new_idx);
+            new_idx
+        };
+        module.module_handles[idx].address = addr_idx;
+    }
+    Ok(())
+}
+
+fn rewrite_modules_by_name(
+    modules: &mut [CompiledModule],
+    module_addr_map: &BTreeMap<String, ObjectID>,
+) -> Result<(), MovyError> {
+    for module in modules.iter_mut() {
+        rewrite_module_by_name(module, module_addr_map)?;
+    }
+    Ok(())
+}
+
+fn rewrite_module_by_name(
+    module: &mut CompiledModule,
+    module_addr_map: &BTreeMap<String, ObjectID>,
+) -> Result<(), MovyError> {
+    let mut addr_index_map: BTreeMap<ObjectID, AddressIdentifierIndex> = BTreeMap::new();
+    for (idx, addr) in module.address_identifiers.iter().enumerate() {
+        addr_index_map.insert(ObjectID::from(*addr), AddressIdentifierIndex(idx as u16));
+    }
+
+    let self_handle_idx = module.self_handle_idx();
+    for idx in 0..module.module_handles.len() {
+        let handle_idx = ModuleHandleIndex(idx as u16);
+        if handle_idx == self_handle_idx {
+            continue;
+        }
+        let handle = module.module_handles[idx].clone();
+        let current_addr = *module.address_identifier_at(handle.address);
+        if current_addr != AccountAddress::ZERO {
+            continue;
+        }
+        let name = module.identifier_at(handle.name).to_string();
+        let Some(new_addr) = module_addr_map.get(&name) else {
+            continue;
+        };
+        let addr_idx = if let Some(existing) = addr_index_map.get(new_addr) {
+            *existing
+        } else {
+            let addr = AccountAddress::from(*new_addr);
+            module.address_identifiers.push(addr);
+            let new_idx = AddressIdentifierIndex((module.address_identifiers.len() - 1) as u16);
+            addr_index_map.insert(*new_addr, new_idx);
+            new_idx
+        };
+        module.module_handles[idx].address = addr_idx;
+    }
+    Ok(())
+}


Missing test coverage: The new functions rewrite_modules_by_id, rewrite_module_by_id, rewrite_modules_by_name, and rewrite_module_by_name lack test coverage. Given that these functions perform critical module rewriting operations that could break package deployment if incorrect, tests should be added to verify their behavior with various scenarios including edge cases.

Copilot · 2026-02-13T04:56:51Z

crates/movy-replay/src/exec.rs

+    fn expand_dependency_closure(&self, dependencies: &mut Vec<ObjectID>) -> Result<(), MovyError> {
+        let mut dep_set: BTreeSet<ObjectID> = dependencies.iter().copied().collect();
+        let mut pending: Vec<ObjectID> = dep_set.iter().copied().collect();
+
+        while let Some(package_id) = pending.pop() {
+            let Some(package) = self.db.get_package_object(&package_id)? else {
+                tracing::warn!(
+                    "dependency package {} not found while expanding publish closure",
+                    package_id
+                );
+                continue;
+            };
+
+            for dep in package.move_package().linkage_table().values() {
+                let dep_id = dep.upgraded_id;
+                if dep_set.insert(dep_id) {
+                    pending.push(dep_id);
+                }
+            }
+        }
+
+        *dependencies = dep_set.into_iter().collect();
+        Ok(())
+    }


Potential infinite loop risk: The expand_dependency_closure method walks the dependency graph but doesn't protect against circular dependencies or enforce a maximum iteration count. While dep_set.insert() should prevent revisiting the same package, consider adding an iteration limit or depth check to prevent potential infinite loops in case of graph corruption or unexpected cycles.

Copilot · 2026-02-13T04:56:52Z

crates/movy-sui/src/compile.rs

        };

-        let deps: BTreeSet<ObjectID> = self.published_dependencies.clone().into_iter().collect();
+        let deps: BTreeSet<ObjectID> = self.dependencies.clone().into_iter().collect();


Semantic change: The movy_mock method now uses self.dependencies instead of self.published_dependencies when initializing the deps BTreeSet. This changes the behavior of which dependencies are included in the mocked package. Ensure this is intentional and that all dependencies (not just published ones) should be included in the mocked package's dependency list.

Suggested change

let deps: BTreeSet<ObjectID> = self.dependencies.clone().into_iter().collect();

let deps: BTreeSet<ObjectID> = self.published_dependencies.clone().into_iter().collect();

Copilot · 2026-02-13T04:56:52Z

crates/movy-replay/src/env.rs

+            if modules.is_empty() {
+                return Err(eyre!("empty modules for dependency {}", dep_name).into());
+            }
+
+            let dep_self_addr = ObjectID::from(*modules[0].address());


Logic error: The code checks if modules.is_empty() at line 190-192, but then immediately accesses modules[0] at line 194 without verifying non-emptiness again. While the empty check prevents panic, the error message at line 191 says "empty modules" but this code path is only reached after the check passes. The indexing at line 194 should be safe due to the check, but consider using modules.first() for safer and more idiomatic Rust code.

Suggested change

if modules.is_empty() {

return Err(eyre!("empty modules for dependency {}", dep_name).into());

}

let dep_self_addr = ObjectID::from(*modules[0].address());

let dep_self_addr = ObjectID::from(

*modules

.first()

.ok_or_else(|| eyre!("empty modules for dependency {}", dep_name))?

.address(),

);

crates/movy-sui/src/compile.rs

Copilot · 2026-02-13T04:56:53Z

crates/movy-replay/src/exec.rs

+    pub fn force_deploy_contract_at(
+        &mut self,
+        package_id: ObjectID,
+        project: SuiCompiledPackage,
+    ) -> Result<ObjectID, MovyError> {
+        let (mut modules, mut dependencies) = project.into_deployment();
+        self.expand_dependency_closure(&mut dependencies)?;
+        Self::rewrite_modules_self_id(&mut modules, package_id);
+        let object = Object::new_package_from_data(
+            Data::Package(MovePackage::new_system(
+                SequenceNumber::new(),
+                &modules,
+                dependencies,
+            )),
+            TransactionDigest::genesis_marker(),
+        );
+        if object.id() != package_id {
+            return Err(eyre!(
+                "force publish id mismatch: expected {}, got {}",
+                package_id,
+                object.id()
+            )
+            .into());
+        }
+        self.db.commit_single_object(object)?;
+        Ok(package_id)
+    }


Missing documentation: This new public method lacks documentation explaining its purpose, parameters, and behavior. Consider adding a doc comment that describes what "force deploying" means and when this should be used instead of the regular deploy_contract method.

Copilot · 2026-02-13T04:56:53Z

crates/movy-replay/src/env.rs

+fn record_zero_address_modules(
+    module_addr_map: &mut BTreeMap<String, ObjectID>,
+    modules: &[CompiledModule],
+    package_addr: ObjectID,
+) -> Result<(), MovyError> {
+    for module in modules.iter() {
+        let name = module.name().to_string();
+        if let Some(prev) = module_addr_map.get(&name).copied() {
+            if prev != package_addr {
+                tracing::debug!(
+                    "duplicate module name {} mapped to both {} and {}, keep {}",
+                    name,
+                    prev,
+                    package_addr,
+                    prev
+                );
+            }
+            continue;
+        }
+        module_addr_map.insert(name, package_addr);
+    }
+    Ok(())
+}


Unnecessary Result return type: The record_zero_address_modules function returns Result but never returns an error - it only returns Ok at line 62. Consider changing the return type from Result to unit type () since there are no error conditions.

crates/movy-sui/src/compile.rs

Copilot · 2026-02-13T04:56:54Z

crates/movy-sui/src/compile.rs

+        } else {
+            let addr = AccountAddress::from(*new_addr);
+            module.address_identifiers.push(addr);
+            let new_idx = AddressIdentifierIndex((module.address_identifiers.len() - 1) as u16);


Potential integer overflow: casting the result of (address_identifiers.len() - 1) to u16 without bounds checking. If the collection grows beyond 65535 entries, this will silently overflow. Consider adding a check to ensure the length fits within u16::MAX before casting, or return an error if it exceeds this limit.

Suggested change

let new_idx = AddressIdentifierIndex((module.address_identifiers.len() - 1) as u16);

let new_idx = AddressIdentifierIndex(

u16::try_from(module.address_identifiers.len() - 1)

.expect("address_identifiers length exceeded u16::MAX"),

);

Copilot AI review requested due to automatic review settings February 13, 2026 04:52

Copilot started reviewing on behalf of lbr77 February 13, 2026 04:52 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

lbr77 closed this Feb 20, 2026

lbr77 force-pushed the v1.65.2-pending branch from 5d9d0b4 to 225bf69 Compare February 20, 2026 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dependency auto deploy#19

dependency auto deploy#19
lbr77 wants to merge 0 commit intoBitsLabSec:v1.65.2-pendingfrom
lbr77:v1.65.2-pending

lbr77 commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	let deps: BTreeSet<ObjectID> = self.dependencies.clone().into_iter().collect();
	let deps: BTreeSet<ObjectID> = self.published_dependencies.clone().into_iter().collect();

Conversation

lbr77 commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants