Namespace collision in dependencies

[kcsongor]

With automated address management, the self address defined in a Move.toml must be 0x0. When such a package depends on another package that uses automated address management (i.e. its self-address is 0x0), the compiler seems to merge their namespaces, meaning that if they contain modules with identical names, the compiler fails with an error about duplicate identifiers.

Steps to Reproduce Issue

Minimal repro here: https://github.com/wormholelabs-xyz/sui-addr-bug

sui --version
sui 1.48.2-17a9eb8e1914

[github-actions[bot]]

Thank you for opening this issue, a team member will review it shortly. Until then, please do not interact with any users that claim to be from Sui support and do not click on any links!

[stefan-mysten]

This is a known bug @kcsongor and we're working on a fix. I don't have an ETA yet, but hopefully very soon.

[ali-behjati]

@stefan-mysten any updates here?

[stefan-mysten]

Sorry, I don't have news on this. There's some hacky workarounds, but we don't recommend them. @tzakian has some thoughts on possible fixes, but as always, it's a matter of getting someone with cycles to perform the fix.

[avernikoz]

Hi, we are experiencing the same issue and I'd like to share a few more details, which might help in resolving the fix. The core problem is that the configuration required for sui move build to succeed is different from the configuration required for sui client publish, forcing manual changes to a dependency's Move.toml.

Detailed Scenario

Consider a main package (main_package) that depends on a local package (deepbook). Both packages contain modules with identical names (e.g., math, pool).

1. Configuration for a Successful Build

To get sui move build to pass when run on main_package, I must edit the dependency's manifest file, deepbook/Move.toml, and set its address to a non-0x0 value (e.g., its real on-chain address):

# In packages/deepbook/Move.toml
[addresses]
deepbook = "0xcaf6ba059d539a97646d47f0b9ddf843e138d215e2a12ca1f4585d386f7aec3a"

With this configuration, sui move build works correctly because the compiler sees the modules as being in distinct namespaces (0x0::math from main_package vs. 0xcaf6ba...::math from deepbook) and avoids a name collision.

2. Configuration for a Successful Publish

To publish main_package, I must edit the dependency's manifest file, deepbook/Move.toml, and set its address back to 0x0:

# In packages/deepbook/Move.toml
[addresses]
deepbook = "0x0"

This 0x0 address is required to make sui client publish pass. If I use the real on-chain address in deepbook/Move.toml (the one that makes the build succeed), the publish command fails with the error Dry run completed, execution status: failure due to PublishUpgradeMissingDependency in command 0.

The problem is that this configuration, while necessary for publishing, causes sui move build to fail with a duplicate definition error (e.g., Duplicate definition for module '(main_package=0x0)::math').

The Workflow Dilemma

This creates a mandatory, manual workflow:

1. To develop and build, I must set the dependency's address to its real on-chain value in deepbook/Move.toml.
2. To deploy, I must remember to change the address back to 0x0 in deepbook/Move.toml.

This is inefficient and highly error-prone. The expected behavior is for sui move build to be consistent with sui client publish, allowing a single, valid configuration for both building and publishing. The build tool should be able to resolve namespaces for local dependencies in the same way the publish tool does.

[avernikoz]

Hi, @stefan-mysten , could you please share would the potential fix allow modules of main package to have the same name as a dependency? Or it would be forbidden?

Understanding of that would give us info how to name modules before fix would be implemented to avoid conflicts in upgrade / future development of the package.

[avernikoz]

Similar issue: https://github.com/MystenLabs/sui/issues/20112

[gfusee]

Hey @stefan-mysten! I'm interested in this issue, as in my workflow I'm using tricky workarounds that aren't great in terms of developer experience.

I looked at the Sui code, especially the parts triggered by the sui move build, sui client publish, and sui client upgrade commands. I have a proposed fix, and I'm willing to create the PR if you accept it:

• If chain_id is None: during the package address reading in resolution_graph.rs, I propose inspecting all the zeroed addresses after the Move.lock inspection, except the one from the root package, and turning them into random addresses (or a 256-bit hash of their name to make it more deterministic).
• We resolve the Move.lock of each immediate dependency to read and define their published addresses in the resolving table.
• We make the sui client publish, sui client upgrade and sui move build --dump-bytecode-as-base64 commands fail if chain_id cannot be fetched from the ReadApi. At the moment, if read_api.get_chain_identifier returns a Result::Err, it is converted into an Option::None. The issue is that if read_api.get_chain_identifier fails, we can stop the execution there, as the later call to read_api.get_protocol_config(None).await? will likely fail, causing the command to fail anyway.

The first and second points ensures a super smooth developer experience: third-party protocols will just have to publish the Move.lock in their Git repo, containing the published addresses on every network, and keep 0x0 in their Move.toml. Then developers will just have to import the package, and everything will be handled, whether for local building, testing, publishing, or upgrading.

The third point makes it clear that chain_id is only None in the case of a local build, since the Build's chain_id field is marked with #[clap(skip)]. When chain_id is Some, the current logic won't change, as we don't want to include random addresses in the package bytecode that will be sent to the blockchain.

This would not only solve this issue but also remove headaches for both developers providing their code as dependencies and developers using those dependencies!

Let me know what you think about this! If you agree with the solution, I can start working on the PR 🙏