Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vgao1996 opened this issue Mar 22, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@vgao1996
Copy link
Contributor

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: [],
│ }
└──────────────────
@vgao1996 vgao1996 added the bug Something isn't working label Mar 22, 2022
@vgao1996
Copy link
Contributor Author

vgao1996 commented Mar 22, 2022

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

@sblackshear
Copy link
Contributor

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

@BraveAction
Copy link

Execution failed with unexpected error UNEXPECTED_VERIFIER_ERROR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants