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.18k stars 3.65k forks source link

[Bug][move-compiler-v2] Review pre-defined addresses in `move-compiler/tests/move_check_testsuite.rs` for V2 tests #12798

Open brmataptos opened 7 months ago

brmataptos commented 7 months ago

🐛 Bug

v1's move_check_testsuite.rs defines a set of named addresses for all tests. To port V1 tests we will need some of these, but perhaps adding them all could result in conflicts with other named addresses in V2 tests.

fn default_testing_addresses() -> BTreeMap<String, NumericalAddress> {
    let mapping = [
        ("aptos_std", "0x1"),
        ("std", "0x1"),
        ("M", "0x1"),
        ("A", "0x42"),
        ("B", "0x42"),
        ("K", "0x19"),
        ("Async", "0x20"),
    ];

It appears that -std is used in various tests

But

I think that we should:

brmataptos commented 7 months ago

@rahxephon89 @vineethk please review.

vineethk commented 7 months ago

@brmataptos Given that M, A, B, K all have specific test folders associated with them, what do you think about the alternative: for these named addresses, we only add them through test configs for those specific folders.

It is really easy to forget that these are supposed to be standard "test" addresses.

Another option is to use different unique names (aptos_testing_M, etc) that are harder to inadvertently use in our tests, and then change the existing tests to use these names instead.

brmataptos commented 7 months ago

Note that named addresses in tests are pretty uncommon.