Skip to content

Commit

Permalink
[verify] Limit check for 0x0 addresses to publish command (MystenLabs…
Browse files Browse the repository at this point in the history
…#7000)

Previously this check ran whenever a module was compiled, preventing the
compilation of modules with non-zero addresses, with the intention of
preventing the accidental re-publishing of an already published module.

However, this blocks uses of a compiled package apart from for
publishing such as verifying that it matches an on-chain package, or
running its tests.

Loosening the restriction by only making this check when publishing a
module (where it is relevant, and also double checked by the validator).

Test Plan:

```
sui$ cargo nextest run
sui$ cargo simtest
```
  • Loading branch information
amnn authored Dec 22, 2022
1 parent 29e7a84 commit eaafa19
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 19 deletions.
42 changes: 23 additions & 19 deletions crates/sui-framework-build/src/compiled_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use move_binary_format::{
CompiledModule,
};
use move_bytecode_utils::{layout::SerdeLayoutBuilder, module_cache::GetModule, Modules};
use move_compiler::compiled_unit::CompiledUnitEnum;
use move_compiler::compiled_unit::{CompiledUnitEnum, NamedCompiledModule};
use move_core_types::{
account_address::AccountAddress,
language_storage::{ModuleId, StructTag, TypeTag},
Expand Down Expand Up @@ -69,24 +69,6 @@ impl BuildConfig {
Ok(package) => package,
};
let compiled_modules = package.root_modules_map();
let package_name = package.compiled_package_info.package_name.as_str();
let is_framework =
package_name == SUI_PACKAGE_NAME || package_name == MOVE_STDLIB_PACKAGE_NAME;
if !is_framework {
if let Some(m) = compiled_modules
.iter_modules()
.iter()
.find(|m| m.self_id().address() != &AccountAddress::ZERO)
{
// TODO: this should be a generic build failure, not a Sui module publish failure
return Err(SuiError::ModulePublishFailure {
error: format!(
"Modules must all have 0x0 as their addresses. Violated by module {:?}",
m.self_id()
),
});
}
}
if self.run_bytecode_verifier {
for m in compiled_modules.iter_modules() {
move_bytecode_verifier::verify_module(m).map_err(|err| {
Expand Down Expand Up @@ -326,6 +308,28 @@ impl CompiledPackage {
}
layout_builder.into_registry()
}

/// Checks whether this package corresponds to a built-in framework
pub fn is_framework(&self) -> bool {
let package_name = self.package.compiled_package_info.package_name.as_str();
package_name == SUI_PACKAGE_NAME || package_name == MOVE_STDLIB_PACKAGE_NAME
}

/// Checks for root modules with non-zero package addresses. Returns an arbitrary one, if one
/// can can be found, otherwise returns `None`.
pub fn published_root_module(&self) -> Option<&CompiledModule> {
self.package
.root_compiled_units
.iter()
.find_map(|unit| match &unit.unit {
CompiledUnitEnum::Module(NamedCompiledModule { module, .. })
if module.self_id().address() != &AccountAddress::ZERO =>
{
Some(module)
}
_ => None,
})
}
}

impl Default for BuildConfig {
Expand Down
14 changes: 14 additions & 0 deletions crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use serde::Serialize;
use serde_json::json;
use sui_framework::build_move_package;
use sui_source_validation::BytecodeSourceVerifier;
use sui_types::error::SuiError;
use tokio::sync::RwLock;
use tracing::{info, warn};

Expand Down Expand Up @@ -425,6 +426,19 @@ impl SuiClientCommands {
},
)?;

if !compiled_package.is_framework() {
if let Some(already_published) = compiled_package.published_root_module() {
return Err(SuiError::ModulePublishFailure {
error: format!(
"Modules must all have 0x0 as their addresses. \
Violated by module {:?}",
already_published.self_id(),
),
}
.into());
}
}

let compiled_modules = compiled_package.get_package_bytes();

let client = context.get_client().await?;
Expand Down

0 comments on commit eaafa19

Please sign in to comment.