aptos-labs / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptosfoundation.org
Other
6.2k stars 3.67k forks source link

[Bug] local gas simulation fails when publishing module with event emitted in `init_module` #14466

Open meng-xu-cs opened 3 months ago

meng-xu-cs commented 3 months ago

🐛 Bug

As described in the title.

To further illustrate:

Launch local testnet in one terminal session

aptos node run-local-testnet --with-faucet --force-restart

Prepare a demo account

aptos init --network local --profile demo --private-key <some-key> --skip-faucet --assume-yes
aptos account fund-with-faucet --profile demo --account demo --amount 100000

and then try to publish a Move package with the following criteria:

The module publishing will fail with:

{
    "Error": "API error: API error Error(InternalError): Failed to convert transaction data from storage: Module ModuleId { address: 26977f29f5a80a53c3153613555b05146a5e100c4438c647cf95f452b2e0f96f, name: Identifier(\"demo\") } can't be found"
}

However, if you specify both --gas-unit-price and --max-gas during aptos move publish, the transaction is successful.

To reproduce

See the steps above. The Move.toml and Move module is shown below

Code snippet to reproduce

[addresses] demo = '_'

[dependencies] AptosFramework = { git = "https://github.com/aptos-labs/aptos-core.git", subdir = "aptos-move/framework/aptos-framework/", rev = "main" }


- sources/demo.move
```move
module demo::demo {
    use aptos_framework::event;

    #[event]
    struct MyEvent has store, drop {
        value: u64,
    }

    fun init_module(_sender: &signer) {
        event::emit(MyEvent { value: 42 });
    }
}

Stack trace/error message

// Paste the output here

Expected Behavior

See above

System information

Please complete the following information:

georgemitenkov commented 2 weeks ago

I believe this is because we store error info on the side now. So when trying to look it up for the given module, it does not exist (because it has not been published). @areshand can you confirm? I believe we have a few other bug reports with similar behaviour.

For example here: https://github.com/aptos-labs/aptos-core/blob/dff834ce5e553d907d9c31ea04942e404e00ff4c/api/types/src/convert.rs#L1069 this will return the reported error if abort location is set to non-existing module.

areshand commented 2 weeks ago

We didn't enable the feature. All the error info shouldn't be removed. This issue is different. The error message shows the module_id. But somehow cannot find it in storage. I will take a look to see if there is some other issue.

areshand commented 2 weeks ago

is the error expected? Since this is simulation and the new module is not actually committed to storage, the API will return cannot find the new module id in storage. Once you submitted txn and module committed, the error is gone. Please let me know if I am missing sth.

meng-xu-cs commented 2 weeks ago

I don't think this is an expected error:

aptos move publish --profile demo --sender-account demo --package-dir <path-to-dir-with-Move.toml>

If I am not wrong, this is the suggested way of publishing a Move package via the aptos CLI. And this suggested way of module publishing gives the error above.

aptos move publish --profile demo --sender-account demo --package-dir <path-to-dir-with-Move.toml>

Adding both --gas-unit-price and --max-gas flags will enable the publishing, but I don't think this is the recommended route?

areshand commented 1 week ago

tried to reproduce the bug. It seems it doesn't hit the node API at all. I think this is some local simulation of package publishing. shouldn't be relevant to storage. @gregnazario could you please take a look at this issue?

gregnazario commented 1 week ago

If I'm not mistaken, this specifically is running simulation on the node, not locally. I've also had this happen to me, which is new behavior.

The CLI last I checked didn't do anything locally unless it was changed

areshand commented 1 week ago

Simulation execution status is a success. But when MoveConvert tries to convert/render the events (https://github.com/aptos-labs/aptos-core/blob/main/api/types/src/convert.rs#L198), it fails to find the new module id. I don't have much understanding how the MoveConverter cache works. could @georgemitenkov take from here to see why the MoveConverter doesn't have the module ID? should the new module id be cached after simulation and used for later rendering the txns?

"event: ModuleEvent { type: Struct(StructTag { address: 0000000000000000000000000000000000000000000000000000000000000001, module: Identifier("code"), name: Identifier("PublishPackage"), type_args: [] }), event_data: "e7194662d2006cf307eafbb00387137dc95dfdd228ae0c8158cf3dfabb9972a000" }
event: ModuleEvent { type: Struct(StructTag { address: e7194662d2006cf307eafbb00387137dc95dfdd228ae0c8158cf3dfabb9972a0, module: Identifier("demo"), name: Identifier("MyEvent"), type_args: [] }), event_data: "2a00000000000000" }
details = """
panicked at /Users/bowu/projects/aptos-core/api/types/src/convert.rs:576:67:
called `Result::unwrap()` on an `Err` value: Module ModuleId { address: e7194662d2006cf307eafbb00387137dc95dfdd228ae0c8158cf3dfabb9972a0, name: Identifier(\"demo\") } can't be found""""