diem/move

[Bug] native functions are bound to a hardcoded address (0x1) in unit tests (invoked through move-cli)

Opened this issue · 3 comments

Currently when the move-cli launches, it associates the MoveStdlib native functions with address 0x1.

fn main() -> Result<()> {
    let error_descriptions: ErrorMapping = bcs::from_bytes(move_stdlib::error_descriptions())?;
    let cost_table = &move_vm_types::gas_schedule::INITIAL_COST_SCHEDULE;
    move_cli::move_cli(
        move_stdlib::natives::all_natives(AccountAddress::from_hex_literal("0x1").unwrap()),
        cost_table,
        &error_descriptions,
    )
}

This can be problematic if a user runs unit tests against a package that sets Std to a different numerical address. Here is an example:

[package]
name = "Foo"
version = "1.0.0"

[dependencies]
MoveStdlib = { local = "../move-stdlib" }

[addresses]
A = "_"

[dev-addresses]
A = "0x2"
Std = "0x2"

This will result in the Move VM failing to load the MoveStdlib modules containing native functions:

[ FAIL    ] 0x2::Foo::foo

Test failures:

Failures in 0x2::Foo:

┌── foo ──────
│ ITE: An unknown error was reported. Location: 
│ VMError (if there is one): VMError {
│     major_status: UNEXPECTED_VERIFIER_ERROR,
│     sub_status: None,
│     message: Some(
│         "Unexpected verifier/deserialization error! This likely means there is code stored on chain that is unverifiable!\nError: VMError { major_status: MISSING_DEPENDENCY, sub_status: None, message: None, stacktrace: None, location: Module(ModuleId { address: 00000000000000000000000000000002, name: Identifier(\"UnitTest\") }), indices: [(FunctionHandle, 0)], offsets: [] }",
│     ),
│     stacktrace: None,
│     location: Module(
│         ModuleId {
│             address: 00000000000000000000000000000002,
│             name: Identifier(
│                 "UnitTest",
│             ),
│         },
│     ),
│     indices: [
│         (
│             FunctionHandle,
│             0,
│         ),
│     ],
│     offsets: [],
│ }
└──────────────────

To be fair, I'm not entirely sure if we should label this as a bug as it doesn't seem too unreasonable to require Std = "0x1" in unit tests, at least in the short term. The error message should definitely be improved though.

Alternatively, I think It's possible to make the move-cli (or the unit test driver) resolve the named address "Std" into a numerical address and use that for native functions, but I don't know if we'll run into some unexpected rough edges.

cc @tnowacki @sblackshear

One fairly lightweight solution might be:

  • Add an optional move-stdlib-address flag that defaults to 0x1
  • In the move package command, sanity check move-stdlib-address against Std in the package
  • If the two don't match, fail with a helpful error message suggesting that the user add --move-stdlib-address to their command

Execution failed with unexpected error UNEXPECTED_VERIFIER_ERROR