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

Migrate token objects into the framework at 0x4 #7277

Merged
merged 13 commits into from
Mar 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/tests/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async fn test_gen_object() {
let collection_addr = account_address::create_collection_address(user_addr, "Hero Quest!");
let token_addr = account_address::create_token_address(user_addr, "Hero Quest!", "Wukong");
let object_resource = "0x1::object::ObjectCore";
let token_resource = format!("0x{}::token::Token", user_addr);
let token_resource = "0x4::token::Token";
let hero_resource = format!("0x{}::hero::Hero", user_addr);

let collection0 = context.gen_all_resources(&collection_addr).await;
Expand All @@ -56,7 +56,7 @@ async fn test_gen_object() {
let hero = context.gen_all_resources(&token_addr).await;
let hero_map = to_object(hero);
assert!(hero_map.contains_key(object_resource));
assert!(hero_map.contains_key(&token_resource));
assert!(hero_map.contains_key(token_resource));
assert!(hero_map.contains_key(&hero_resource));
let owner: AccountAddress = hero_map[object_resource]["owner"]
.as_str()
Expand Down
1 change: 1 addition & 0 deletions aptos-move/aptos-release-builder/data/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ update_sequence:
- multisig_accounts
- delegation_pools
- ed25519_pubkey_validate_return_false_wrong_length
- struct_constructors
disabled: []
- Consensus:
V1:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub fn generate_upgrade_proposals(
("0x1", "aptos-move/framework/aptos-stdlib"),
("0x1", "aptos-move/framework/aptos-framework"),
("0x3", "aptos-move/framework/aptos-token"),
("0x4", "aptos-move/framework/aptos-token-objects"),
];

let mut result: Vec<(String, String)> = vec![];
Expand Down
15 changes: 15 additions & 0 deletions aptos-move/e2e-move-tests/src/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,3 +615,18 @@ macro_rules! assert_vm_status {
);
}};
}

#[macro_export]
macro_rules! assert_move_abort {
($s:expr, $c:ident) => {{
use aptos_types::transaction::*;
assert!(match $s {
TransactionStatus::Keep(ExecutionStatus::MoveAbort {
location: _,
code: _,
info,
}) => info == $c,
_ => false,
});
}};
}
127 changes: 41 additions & 86 deletions aptos-move/e2e-move-tests/src/tests/string_args.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::{assert_success, assert_vm_status, tests::common, MoveHarness};
use aptos_types::account_address::AccountAddress;
use crate::{assert_move_abort, assert_success, assert_vm_status, tests::common, MoveHarness};
use aptos_types::{
account_address::AccountAddress,
transaction::{AbortInfo, TransactionStatus},
};
use move_core_types::{
identifier::Identifier,
language_storage::{StructTag, TypeTag},
Expand Down Expand Up @@ -62,11 +65,27 @@ fn success_generic(ty_args: Vec<TypeTag>, tests: Vec<(&str, Vec<(Vec<Vec<u8>>, &
}
}

fn fail(tests: Vec<(&str, Vec<Vec<u8>>, StatusCode)>) {
fn deserialization_failure() -> impl Fn(TransactionStatus) {
let status_code = StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT;
move |txn_status| assert_vm_status!(txn_status, status_code)
}

fn abort_info() -> impl Fn(TransactionStatus) {
let abort_info = Some(AbortInfo {
reason_name: "EINVALID_UTF8".to_string(),
description: "An invalid UTF8 encoding.".to_string(),
});
move |txn_status| assert_move_abort!(txn_status, abort_info)
}

fn fail(tests: Vec<(&str, Vec<Vec<u8>>, impl Fn(TransactionStatus))>) {
fail_generic(vec![], tests)
}

fn fail_generic(ty_args: Vec<TypeTag>, tests: Vec<(&str, Vec<Vec<u8>>, StatusCode)>) {
fn fail_generic(
ty_args: Vec<TypeTag>,
tests: Vec<(&str, Vec<Vec<u8>>, impl Fn(TransactionStatus))>,
) {
let mut h = MoveHarness::new();

// Load the code
Expand All @@ -81,7 +100,7 @@ fn fail_generic(ty_args: Vec<TypeTag>, tests: Vec<(&str, Vec<Vec<u8>>, StatusCod
for (entry, args, err) in tests {
// Now send hi transaction, after that resource should exist and carry value
let status = h.run_entry_function(&acc, str::parse(entry).unwrap(), ty_args.clone(), args);
assert_vm_status!(status, err);
err(status);
}
}

Expand Down Expand Up @@ -289,38 +308,22 @@ fn string_args_bad_utf8() {

// simple strings
let args = vec![bcs::to_bytes(&vec![0xF0u8, 0x28u8, 0x8Cu8, 0xBCu8]).unwrap()];
tests.push((
"0xcafe::test::hi",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::hi", args, abort_info()));

let args = vec![bcs::to_bytes(&vec![0xC3u8, 0x28u8]).unwrap()];
tests.push((
"0xcafe::test::hi",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::hi", args, abort_info()));

// vector of strings
let bad = vec![0xC3u8, 0x28u8];
let s_vec = vec![&bad[..], "hello".as_bytes(), "world".as_bytes()];
let i = 0u64;
let args = vec![bcs::to_bytes(&s_vec).unwrap(), bcs::to_bytes(&i).unwrap()];
tests.push((
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec", args, abort_info()));

let bad = vec![0xC3u8, 0x28u8];
let s_vec = vec![&bad[..], "hello".as_bytes(), "world".as_bytes()];
let args = vec![bcs::to_bytes(&s_vec).unwrap(), bcs::to_bytes(&i).unwrap()];
tests.push((
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec", args, abort_info()));

// vector of vector of strings
let i = 0u64;
Expand All @@ -345,11 +348,7 @@ fn string_args_bad_utf8() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, abort_info()));

let bad = vec![0xF0u8, 0x28u8, 0x8Cu8, 0x28u8];
let s_vec = vec![
Expand All @@ -370,11 +369,7 @@ fn string_args_bad_utf8() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, abort_info()));

let bad = vec![0x60u8, 0xFFu8];
let s_vec = vec![
Expand All @@ -395,11 +390,7 @@ fn string_args_bad_utf8() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, abort_info()));

fail(tests);
}
Expand All @@ -421,7 +412,7 @@ fn string_args_chopped() {
fail(vec![(
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
deserialization_failure(),
)]);
i /= 2;
}
Expand All @@ -437,20 +428,12 @@ fn string_args_bad_length() {
// length over max size
let mut args = bcs::to_bytes(&vec![0x30u8; 100000]).unwrap();
args.truncate(20);
tests.push((
"0xcafe::test::hi",
vec![args],
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::hi", vec![args], deserialization_failure()));

// length in size but input chopped
let mut args = bcs::to_bytes(&vec![0x30u8; 30000]).unwrap();
args.truncate(300);
tests.push((
"0xcafe::test::hi",
vec![args],
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::hi", vec![args], deserialization_failure()));

// vector of strings

Expand All @@ -461,11 +444,7 @@ fn string_args_bad_length() {
bcs_vec.truncate(200);
let i = 0u64;
let args = vec![bcs_vec, bcs::to_bytes(&i).unwrap()];
tests.push((
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec", args, deserialization_failure()));

// length over max size after 2 big-ish strings
let bad = vec![0x30u8; 100000];
Expand All @@ -474,23 +453,15 @@ fn string_args_bad_length() {
let mut bcs_vec = bcs::to_bytes(&s_vec).unwrap();
bcs_vec.truncate(30000);
let args = vec![bcs_vec, bcs::to_bytes(&i).unwrap()];
tests.push((
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec", args, deserialization_failure()));

// length in size but input chopped
let big = vec![0x30u8; 10000];
let s_vec = vec![&big[..], &big[..], &big[..]];
let mut bcs_vec = bcs::to_bytes(&s_vec).unwrap();
bcs_vec.truncate(20000);
let args = vec![bcs_vec, bcs::to_bytes(&i).unwrap()];
tests.push((
"0xcafe::test::str_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec", args, deserialization_failure()));

// vector of vector of strings

Expand Down Expand Up @@ -518,11 +489,7 @@ fn string_args_bad_length() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, deserialization_failure()));

let bad = vec![0x30u8; 10000];
let s_vec = vec![
Expand All @@ -545,11 +512,7 @@ fn string_args_bad_length() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, deserialization_failure()));

let bad = vec![0x30u8; 100000];
let s_vec = vec![
Expand All @@ -572,11 +535,7 @@ fn string_args_bad_length() {
bcs::to_bytes(&i).unwrap(),
bcs::to_bytes(&j).unwrap(),
];
tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, deserialization_failure()));

// length over max size with 0 length strings
let s_vec = vec![vec!["".as_bytes(); 3]; 100000];
Expand All @@ -603,11 +562,7 @@ fn string_args_bad_length() {
bcs::to_bytes(&j).unwrap(),
];

tests.push((
"0xcafe::test::str_vec_vec",
args,
StatusCode::FAILED_TO_DESERIALIZE_ARGUMENT,
));
tests.push(("0xcafe::test::str_vec_vec", args, deserialization_failure()));

fail(tests);
}
Expand Down
2 changes: 1 addition & 1 deletion aptos-move/e2e-move-tests/src/tests/token_objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn test_basic_token() {
type_params: vec![],
};
let token_obj_tag = StructTag {
address: addr,
address: AccountAddress::from_hex_literal("0x4").unwrap(),
module: Identifier::new("token").unwrap(),
name: Identifier::new("Token").unwrap(),
type_params: vec![],
Expand Down
13 changes: 13 additions & 0 deletions aptos-move/framework/aptos-token-objects/Move.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make this a new package vs merging into aptos-token? This would require making sure aptos-token-objects is supported as a new package in many places (genesis, release-tooling, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the better approach... the constructs at 0x3 share names and would all be but confusing to co-exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call though... I need to actually run a local node and ensure that it is taken in otherwise this change won't impact devnet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest ensuring both genesis and framework upgrade work. Otherwise, this would potentially bypass tests, compat checks, etc. which can somehow break the release process down the line

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new package just because of shared names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry users would end up even more confused.

name = "AptosTokenObjects"
version = "1.0.0"

[addresses]
std = "0x1"
aptos_std = "0x1"
aptos_framework = "0x1"
aptos_token_objects = "0x4"

[dependencies]
MoveStdlib = { local = "../move-stdlib" }
AptosFramework = { local = "../aptos-framework"}
Loading