Skip to content

Commit

Permalink
[move] 0x0 subst on publish matches source subst (#6246)
Browse files Browse the repository at this point in the history
Replacing address `0x0` with a fresh address during publish was
previously handled by `ModuleHandleRewriter`, which took a mapping of
new to old module handles (address, name pairs) and visited all
structures that referenced them in a `CompiledModule` replacing old
for new, and allocating new slots in the identifier and address pools
as needed.

This process can be simplified because of the following properties:
- All the identifiers are already allocated.
- There is only one new address, the new Package ID.
- That address is replacing the old `0x0` address in all cases.

The new implementation takes advantage of these properties and the
indirection of the Address Identifier Pool, and substitutes the
address at the index referenced by the module's self_handle (which it
confirms is `0x0`) with the new address.

Gas costs reduce slightly, because published module sizes decrease (by
the size of one address in the pool).

The main motivation for this change is dependency source verification:
The old implementation left a redundant `0x0` in the address pool,
which developers do not get if they set the address of the module in
`Move.toml`, this meant that the following (common) process results in
a source verification failure:

- Write a package `b`, with its address set to `0x0`.
- Publish it, remember its address.
- Substitute its address in `Move.toml` with the published address.
- Distribute its source.
- Depend on this source when writing another module, `a`.
- Publish the new module.

The second publish will fail dependency verification, because the
locally compiled `b` will not contain `0x0` in its address pool, but
the on-chain `b` will.  With this change, that is no longer true, and
source verification passes in this case.

Test Plan:

```
cargo simtest
cargo nextest run
```

+ new source verification tests coming in a follow-up commit.
  • Loading branch information
amnn authored Nov 28, 2022
1 parent 6f60d40 commit 6102b24
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 440 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ created: object(110)
written: object(109)

task 7 'run'. lines 82-82:
events: CoinBalanceChange { package_id: sui, transaction_module: Identifier("gas"), sender: B, change_type: Gas, owner: AddressOwner(B), coin_type: "sui::sui::SUI", coin_object_id: fake(111), version: SequenceNumber(0), amount: -83 }, MutateObject { package_id: test, transaction_module: Identifier("object_basics"), sender: B, object_type: "test::object_basics::Object", object_id: fake(107), version: SequenceNumber(3) }, MutateObject { package_id: sui, transaction_module: Identifier("unused_input_object"), sender: B, object_type: "test::object_basics::Object", object_id: fake(110), version: SequenceNumber(2) }, MoveEvent { package_id: test, transaction_module: Identifier("object_basics"), sender: B, type_: StructTag { address: test, module: Identifier("object_basics"), name: Identifier("NewValueEvent"), type_params: [] }, contents: [20, 0, 0, 0, 0, 0, 0, 0] }
events: CoinBalanceChange { package_id: sui, transaction_module: Identifier("gas"), sender: B, change_type: Gas, owner: AddressOwner(B), coin_type: "sui::sui::SUI", coin_object_id: fake(111), version: SequenceNumber(0), amount: -82 }, MutateObject { package_id: test, transaction_module: Identifier("object_basics"), sender: B, object_type: "test::object_basics::Object", object_id: fake(107), version: SequenceNumber(3) }, MutateObject { package_id: sui, transaction_module: Identifier("unused_input_object"), sender: B, object_type: "test::object_basics::Object", object_id: fake(110), version: SequenceNumber(2) }, MoveEvent { package_id: test, transaction_module: Identifier("object_basics"), sender: B, type_: StructTag { address: test, module: Identifier("object_basics"), name: Identifier("NewValueEvent"), type_params: [] }, contents: [20, 0, 0, 0, 0, 0, 0, 0] }
written: object(107), object(110), object(111)

task 8 'run'. lines 84-84:
Expand Down
44 changes: 19 additions & 25 deletions crates/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ use sui_verifier::{
};
use tracing::instrument;

use crate::bytecode_rewriter::ModuleHandleRewriter;

macro_rules! assert_invariant {
($cond:expr, $msg:expr) => {
if !$cond {
Expand Down Expand Up @@ -497,37 +495,33 @@ pub fn generate_package_id(
modules: &mut [CompiledModule],
ctx: &mut TxContext,
) -> Result<ObjectID, ExecutionError> {
let mut sub_map = BTreeMap::new();
let package_id = ctx.fresh_id();
for module in modules.iter() {
let old_module_id = module.self_id();
let old_address = *old_module_id.address();
if old_address != AccountAddress::ZERO {
let handle = module.module_handle_at(module.self_module_handle_idx);
let name = module.identifier_at(handle.name);
let new_address = AccountAddress::from(package_id);

for module in modules.iter_mut() {
let self_handle = module.self_handle().clone();
let self_address_idx = self_handle.address;

let addrs = &mut module.address_identifiers;
let Some(address_mut) = addrs.get_mut(self_address_idx.0 as usize) else {
let name = module.identifier_at(self_handle.name);
return Err(ExecutionError::new_with_source(
ExecutionErrorKind::PublishErrorNonZeroAddress,
format!("Publishing module {name} with non-zero address is not allowed"),
format!("Publishing module {name} with invalid address index"),
));
}
let new_module_id = ModuleId::new(
AccountAddress::from(package_id),
old_module_id.name().to_owned(),
);
if sub_map.insert(old_module_id, new_module_id).is_some() {
};

if *address_mut != AccountAddress::ZERO {
let name = module.identifier_at(self_handle.name);
return Err(ExecutionError::new_with_source(
ExecutionErrorKind::PublishErrorDuplicateModule,
"Publishing two modules with the same ID",
ExecutionErrorKind::PublishErrorNonZeroAddress,
format!("Publishing module {name} with non-zero address is not allowed"),
));
}
}
};

// Safe to unwrap because we checked for duplicate domain entries above, and range entries are fresh ID's
let rewriter = ModuleHandleRewriter::new(sub_map).unwrap();
for module in modules.iter_mut() {
// rewrite module handles to reflect freshly generated ID's
rewriter.sub_module_ids(module);
*address_mut = new_address;
}

Ok(package_id)
}

Expand Down
143 changes: 0 additions & 143 deletions crates/sui-adapter/src/bytecode_rewriter.rs

This file was deleted.

1 change: 0 additions & 1 deletion crates/sui-adapter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
// SPDX-License-Identifier: Apache-2.0

pub mod adapter;
pub mod bytecode_rewriter;
pub mod genesis;
Loading

0 comments on commit 6102b24

Please sign in to comment.