Skip to content

Commit

Permalink
Update collections permissions to collection owner (#14113)
Browse files Browse the repository at this point in the history
  • Loading branch information
JohnChangUK authored Aug 1, 2024
1 parent 3517ae7 commit 52e4a0e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 18 deletions.
3 changes: 0 additions & 3 deletions aptos-move/framework/aptos-token-objects/doc/collection.md
Original file line number Diff line number Diff line change
Expand Up @@ -980,9 +980,6 @@ TODO: Hide this until we bring back meaningful way to enforce burns
<a href="royalty.md#0x4_royalty_init">royalty::init</a>(&constructor_ref, <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option_extract">option::extract</a>(&<b>mut</b> <a href="royalty.md#0x4_royalty">royalty</a>))
};

<b>let</b> transfer_ref = <a href="../../aptos-framework/doc/object.md#0x1_object_generate_transfer_ref">object::generate_transfer_ref</a>(&constructor_ref);
<a href="../../aptos-framework/doc/object.md#0x1_object_disable_ungated_transfer">object::disable_ungated_transfer</a>(&transfer_ref);

constructor_ref
}
</code></pre>
Expand Down
12 changes: 11 additions & 1 deletion aptos-move/framework/aptos-token-objects/doc/token.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,16 @@ The URI is over the maximum length



<a id="0x4_token_ENOT_OWNER"></a>

The calling signer is not the owner


<pre><code><b>const</b> <a href="token.md#0x4_token_ENOT_OWNER">ENOT_OWNER</a>: u64 = 8;
</code></pre>



<a id="0x4_token_EDESCRIPTION_TOO_LONG"></a>

The description is over the maximum length
Expand Down Expand Up @@ -542,7 +552,7 @@ The token name is over the maximum length
<a href="royalty.md#0x4_royalty">royalty</a>: Option&lt;Royalty&gt;,
uri: String,
) {
<b>assert</b>!(<a href="collection.md#0x4_collection_creator">collection::creator</a>(<a href="collection.md#0x4_collection">collection</a>) == <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(creator), <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_unauthenticated">error::unauthenticated</a>(<a href="token.md#0x4_token_ENOT_CREATOR">ENOT_CREATOR</a>));
<b>assert</b>!(<a href="../../aptos-framework/doc/object.md#0x1_object_owner">object::owner</a>(<a href="collection.md#0x4_collection">collection</a>) == <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(creator), <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_unauthenticated">error::unauthenticated</a>(<a href="token.md#0x4_token_ENOT_OWNER">ENOT_OWNER</a>));

<b>if</b> (<a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option_is_some">option::is_some</a>(&name_with_index_suffix)) {
// Be conservative, <b>as</b> we don't know what length the index will be, and <b>assume</b> worst case (20 chars in MAX_U64)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,6 @@ module aptos_token_objects::collection {
royalty::init(&constructor_ref, option::extract(&mut royalty))
};

let transfer_ref = object::generate_transfer_ref(&constructor_ref);
object::disable_ungated_transfer(&transfer_ref);

constructor_ref
}

Expand Down Expand Up @@ -750,17 +747,19 @@ module aptos_token_objects::collection {
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x50003, location = aptos_framework::object)]
entry fun test_create_and_transfer(creator: &signer, trader: &signer) {
let creator_address = signer::address_of(creator);
let trader_address = signer::address_of(trader);
let collection_name = string::utf8(b"collection name");
create_collection_helper(creator, collection_name);

let collection = object::address_to_object<Collection>(
create_collection_address(&creator_address, &collection_name),
);
assert!(object::owner(collection) == creator_address, 1);
object::transfer(creator, collection, signer::address_of(trader));
// Transferring collections is allowed
object::transfer(creator, collection, trader_address);
assert!(object::owner(collection) == trader_address, 1);
}

#[test(creator = @0x123)]
Expand Down
57 changes: 48 additions & 9 deletions aptos-move/framework/aptos-token-objects/sources/token.move
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ module aptos_token_objects::token {
const EDESCRIPTION_TOO_LONG: u64 = 6;
/// The seed is over the maximum length
const ESEED_TOO_LONG: u64 = 7;
/// The calling signer is not the owner
const ENOT_OWNER: u64 = 8;

const MAX_TOKEN_NAME_LENGTH: u64 = 128;
const MAX_TOKEN_SEED_LENGTH: u64 = 128;
Expand Down Expand Up @@ -154,7 +156,7 @@ module aptos_token_objects::token {
royalty: Option<Royalty>,
uri: String,
) {
assert!(collection::creator(collection) == signer::address_of(creator), error::unauthenticated(ENOT_CREATOR));
assert!(object::owner(collection) == signer::address_of(creator), error::unauthenticated(ENOT_OWNER));

if (option::is_some(&name_with_index_suffix)) {
// Be conservative, as we don't know what length the index will be, and assume worst case (20 chars in MAX_U64)
Expand Down Expand Up @@ -722,8 +724,8 @@ module aptos_token_objects::token {
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)]
fun test_create_token_non_creator(creator: &signer, trader: &signer) {
#[expected_failure(abort_code = 0x40008, location = aptos_token_objects::token)]
fun test_create_token_non_collection_owner(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_token(
Expand All @@ -733,16 +735,16 @@ module aptos_token_objects::token {
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)]
fun test_create_named_token_non_creator(creator: &signer, trader: &signer) {
#[expected_failure(abort_code = 0x40008, location = aptos_token_objects::token)]
fun test_create_named_token_non_collection_owner(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_token_with_collection_helper(trader, collection, string::utf8(b"token name"));
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)]
fun test_create_named_token_object_non_creator(creator: &signer, trader: &signer) {
#[expected_failure(abort_code = 0x40008, location = aptos_token_objects::token)]
fun test_create_named_token_object_non_collection_owner(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_named_token_object(
Expand All @@ -752,8 +754,8 @@ module aptos_token_objects::token {
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)]
fun test_create_named_token_from_seed_non_creator(creator: &signer, trader: &signer) {
#[expected_failure(abort_code = 0x40008, location = aptos_token_objects::token)]
fun test_create_named_token_from_seed_non_collection_owner(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_named_token_object(
Expand Down Expand Up @@ -784,6 +786,43 @@ module aptos_token_objects::token {
assert!(option::some(expected_royalty) == royalty(token), 2);
}

#[test(creator = @0x123, trader = @0x456)]
#[expected_failure(abort_code = 0x40008, location = aptos_token_objects::token)]
fun test_create_token_after_transferring_collection(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_token(
creator, collection, string::utf8(b"token description"), string::utf8(b"token name"),
option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"),
);

object::transfer(creator, collection, signer::address_of(trader));

// This should fail as the collection is no longer owned by the creator.
create_token(
creator, collection, string::utf8(b"token description"), string::utf8(b"token name"),
option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"),
);
}

#[test(creator = @0x123, trader = @0x456)]
fun create_token_works_with_new_collection_owner(creator: &signer, trader: &signer) {
let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5);
let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref));
create_token(
creator, collection, string::utf8(b"token description"), string::utf8(b"token name"),
option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"),
);

object::transfer(creator, collection, signer::address_of(trader));

// This should pass as `trader` is the new collection owner
create_token(
trader, collection, string::utf8(b"token description"), string::utf8(b"token name"),
option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"),
);
}

#[test(creator = @0x123)]
fun test_collection_royalty(creator: &signer) acquires Token {
let collection_name = string::utf8(b"collection name");
Expand Down

0 comments on commit 52e4a0e

Please sign in to comment.