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.

Sign up for GitHub

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

add perimission checks to object #14582

Open
wants to merge 1 commit into
base: 09-04-add_permission_check_to_account
Choose a base branch
from
+183 −11
Open

add perimission checks to object #14582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
111 changes: 101 additions & 10 deletions aptos-move/framework/aptos-framework/doc/object.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ make it so that a reference to a global object can be returned from a function.
- [Struct `TransferRef`](#0x1_object_TransferRef)
- [Struct `LinearTransferRef`](#0x1_object_LinearTransferRef)
- [Struct `DeriveRef`](#0x1_object_DeriveRef)
- [Struct `TransferPermission`](#0x1_object_TransferPermission)
- [Struct `TransferEvent`](#0x1_object_TransferEvent)
- [Struct `Transfer`](#0x1_object_Transfer)
- [Constants](#@Constants_0)
Expand Down Expand Up @@ -89,6 +90,7 @@ make it so that a reference to a global object can be returned from a function.
- [Function `is_owner`](#0x1_object_is_owner)
- [Function `owns`](#0x1_object_owns)
- [Function `root_owner`](#0x1_object_root_owner)
- [Function `grant_permission`](#0x1_object_grant_permission)
- [Specification](#@Specification_1)
- [High-level Requirements](#high-level-req)
- [Module-level Specification](#module-level-spec)
Expand Down Expand Up @@ -133,6 +135,7 @@ make it so that a reference to a global object can be returned from a function.
- [Function `is_owner`](#@Specification_1_is_owner)
- [Function `owns`](#@Specification_1_owns)
- [Function `root_owner`](#@Specification_1_root_owner)
- [Function `grant_permission`](#@Specification_1_grant_permission)


<pre><code><b>use</b> <a href="account.md#0x1_account">0x1::account</a>;
Expand All @@ -144,6 +147,7 @@ make it so that a reference to a global object can be returned from a function.
<b>use</b> <a href="../../aptos-stdlib/doc/from_bcs.md#0x1_from_bcs">0x1::from_bcs</a>;
<b>use</b> <a href="guid.md#0x1_guid">0x1::guid</a>;
<b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/hash.md#0x1_hash">0x1::hash</a>;
<b>use</b> <a href="permissioned_signer.md#0x1_permissioned_signer">0x1::permissioned_signer</a>;
<b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">0x1::signer</a>;
<b>use</b> <a href="transaction_context.md#0x1_transaction_context">0x1::transaction_context</a>;
<b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">0x1::vector</a>;
Expand Down Expand Up @@ -496,6 +500,34 @@ Used to create derived objects from a given objects.
</dl>


</details>

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

## Struct `TransferPermission`

Permission to transfer object with permissioned signer.


<pre><code><b>struct</b> <a href="object.md#0x1_object_TransferPermission">TransferPermission</a> <b>has</b> <b>copy</b>, drop, store
</code></pre>



<details>
<summary>Fields</summary>


<dl>
<dt>
<code><a href="object.md#0x1_object">object</a>: <b>address</b></code>
</dt>
<dd>

</dd>
</dl>


</details>

<a id="0x1_object_TransferEvent"></a>
Expand Down Expand Up @@ -1999,6 +2031,10 @@ hierarchy.
<b>to</b>: <b>address</b>,
) <b>acquires</b> <a href="object.md#0x1_object_ObjectCore">ObjectCore</a> {
<b>let</b> owner_address = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(owner);
<b>assert</b>!(
<a href="permissioned_signer.md#0x1_permissioned_signer_check_permission_exists">permissioned_signer::check_permission_exists</a>(owner, <a href="object.md#0x1_object_TransferPermission">TransferPermission</a> { <a href="object.md#0x1_object">object</a> }),
<a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="object.md#0x1_object_EOBJECT_NOT_TRANSFERRABLE">EOBJECT_NOT_TRANSFERRABLE</a>)
);
<a href="object.md#0x1_object_verify_ungated_and_descendant">verify_ungated_and_descendant</a>(owner_address, <a href="object.md#0x1_object">object</a>);
<a href="object.md#0x1_object_transfer_raw_inner">transfer_raw_inner</a>(<a href="object.md#0x1_object">object</a>, <b>to</b>);
}
Expand Down Expand Up @@ -2188,6 +2224,10 @@ Allow origin owners to reclaim any objects they previous burnt.
) <b>acquires</b> <a href="object.md#0x1_object_TombStone">TombStone</a>, <a href="object.md#0x1_object_ObjectCore">ObjectCore</a> {
<b>let</b> object_addr = <a href="object.md#0x1_object">object</a>.inner;
<b>assert</b>!(<b>exists</b>&lt;<a href="object.md#0x1_object_TombStone">TombStone</a>&gt;(object_addr), <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_invalid_argument">error::invalid_argument</a>(<a href="object.md#0x1_object_EOBJECT_NOT_BURNT">EOBJECT_NOT_BURNT</a>));
<b>assert</b>!(
<a href="permissioned_signer.md#0x1_permissioned_signer_check_permission_exists">permissioned_signer::check_permission_exists</a>(original_owner, <a href="object.md#0x1_object_TransferPermission">TransferPermission</a> { <a href="object.md#0x1_object">object</a>: object_addr }),
<a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="object.md#0x1_object_EOBJECT_NOT_TRANSFERRABLE">EOBJECT_NOT_TRANSFERRABLE</a>)
);

<b>let</b> <a href="object.md#0x1_object_TombStone">TombStone</a> { original_owner: original_owner_addr } = <b>move_from</b>&lt;<a href="object.md#0x1_object_TombStone">TombStone</a>&gt;(object_addr);
<b>assert</b>!(original_owner_addr == <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(original_owner), <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="object.md#0x1_object_ENOT_OBJECT_OWNER">ENOT_OBJECT_OWNER</a>));
Expand Down Expand Up @@ -2361,6 +2401,38 @@ to determine the identity of the starting point of ownership.



</details>

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

## Function `grant_permission`



<pre><code><b>public</b> <b>fun</b> <a href="object.md#0x1_object_grant_permission">grant_permission</a>&lt;T&gt;(master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, <a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, <a href="object.md#0x1_object">object</a>: <a href="object.md#0x1_object_Object">object::Object</a>&lt;T&gt;)
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b> <b>fun</b> <a href="object.md#0x1_object_grant_permission">grant_permission</a>&lt;T&gt;(
master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
<a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
<a href="object.md#0x1_object">object</a>: <a href="object.md#0x1_object_Object">Object</a>&lt;T&gt;,
) {
<a href="permissioned_signer.md#0x1_permissioned_signer_authorize_unlimited">permissioned_signer::authorize_unlimited</a>(
master,
<a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>,
<a href="object.md#0x1_object_TransferPermission">TransferPermission</a> { <a href="object.md#0x1_object">object</a>: <a href="object.md#0x1_object">object</a>.inner }
)
}
</code></pre>



</details>

<a id="@Specification_1"></a>
Expand Down Expand Up @@ -2437,16 +2509,7 @@ to determine the identity of the starting point of ownership.
### Module-level Specification


<pre><code><b>pragma</b> aborts_if_is_strict;
</code></pre>




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


<pre><code><b>fun</b> <a href="object.md#0x1_object_spec_exists_at">spec_exists_at</a>&lt;T: key&gt;(<a href="object.md#0x1_object">object</a>: <b>address</b>): bool;
<pre><code><b>pragma</b> aborts_if_is_partial;
</code></pre>


Expand Down Expand Up @@ -3402,4 +3465,32 @@ to determine the identity of the starting point of ownership.
</code></pre>



<a id="@Specification_1_grant_permission"></a>

### Function `grant_permission`


<pre><code><b>public</b> <b>fun</b> <a href="object.md#0x1_object_grant_permission">grant_permission</a>&lt;T&gt;(master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, <a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, <a href="object.md#0x1_object">object</a>: <a href="object.md#0x1_object_Object">object::Object</a>&lt;T&gt;)
</code></pre>




<pre><code><b>pragma</b> aborts_if_is_partial;
<b>aborts_if</b> !<a href="permissioned_signer.md#0x1_permissioned_signer_spec_is_permissioned_signer">permissioned_signer::spec_is_permissioned_signer</a>(<a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>);
<b>aborts_if</b> <a href="permissioned_signer.md#0x1_permissioned_signer_spec_is_permissioned_signer">permissioned_signer::spec_is_permissioned_signer</a>(master);
<b>aborts_if</b> <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(master) != <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(<a href="permissioned_signer.md#0x1_permissioned_signer">permissioned_signer</a>);
</code></pre>




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


<pre><code><b>fun</b> <a href="object.md#0x1_object_spec_exists_at">spec_exists_at</a>&lt;T: key&gt;(<a href="object.md#0x1_object">object</a>: <b>address</b>): bool;
</code></pre>


[move-book]: https://aptos.dev/move/book/SUMMARY
70 changes: 70 additions & 0 deletions aptos-move/framework/aptos-framework/sources/object.move
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module aptos_framework::object {
use aptos_framework::create_signer::create_signer;
use aptos_framework::event;
use aptos_framework::guid;
use aptos_framework::permissioned_signer;

friend aptos_framework::coin;
friend aptos_framework::primary_fungible_store;
Expand Down Expand Up @@ -165,6 +166,11 @@ module aptos_framework::object {
self: address,
}

/// Permission to transfer object with permissioned signer.
struct TransferPermission has copy, drop, store {
object: address,
}

/// Emitted whenever the object's owner field is changed.
struct TransferEvent has drop, store {
object: address,
Expand Down Expand Up @@ -540,6 +546,10 @@ module aptos_framework::object {
to: address,
) acquires ObjectCore {
let owner_address = signer::address_of(owner);
assert!(
permissioned_signer::check_permission_exists(owner, TransferPermission { object }),
runtian-zhou marked this conversation as resolved.
Show resolved Hide resolved
error::permission_denied(EOBJECT_NOT_TRANSFERRABLE)
);
verify_ungated_and_descendant(owner_address, object);
transfer_raw_inner(object, to);
}
Expand Down Expand Up @@ -629,6 +639,10 @@ module aptos_framework::object {
) acquires TombStone, ObjectCore {
let object_addr = object.inner;
assert!(exists<TombStone>(object_addr), error::invalid_argument(EOBJECT_NOT_BURNT));
assert!(
permissioned_signer::check_permission_exists(original_owner, TransferPermission { object: object_addr }),
error::permission_denied(EOBJECT_NOT_TRANSFERRABLE)
);

let TombStone { original_owner: original_owner_addr } = move_from<TombStone>(object_addr);
assert!(original_owner_addr == signer::address_of(original_owner), error::permission_denied(ENOT_OBJECT_OWNER));
Expand Down Expand Up @@ -699,6 +713,18 @@ module aptos_framework::object {
obj_owner
}

public fun grant_permission<T>(
master: &signer,
permissioned_signer: &signer,
object: Object<T>,
) {
permissioned_signer::authorize_unlimited(
master,
permissioned_signer,
TransferPermission { object: object.inner }
)
}

#[test_only]
use std::option::{Self, Option};

Expand Down Expand Up @@ -1093,4 +1119,48 @@ module aptos_framework::object {
set_untransferable(&weapon_constructor_ref);
transfer_with_ref(linear_transfer_ref, @0x456);
}

#[test_only]
use aptos_framework::timestamp;

#[test(creator = @0x123)]
fun test_transfer_permission_e2e(
creator: &signer,
) acquires ObjectCore {
let aptos_framework = account::create_signer_for_test(@0x1);
timestamp::set_time_has_started_for_testing(&aptos_framework);

let (_, hero) = create_hero(creator);
let (_, weapon) = create_weapon(creator);

// Create a permissioned signer
let creator_permission_handle = permissioned_signer::create_permissioned_handle(creator);
let creator_permission_signer = permissioned_signer::signer_from_permissioned_handle(&creator_permission_handle);

// Grant aaron_permission_signer permission to transfer weapon object
grant_permission(creator, &creator_permission_signer, weapon);
Copy link

Choose a reason for hiding this comment

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

The comment in the test refers to aaron_permission_signer but the actual variable name used is creator_permission_signer. The variable name in the comment should be updated to match the implementation for accuracy.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

transfer_to_object(&creator_permission_signer, weapon, hero);

permissioned_signer::destroy_permissioned_handle(creator_permission_handle);
}

#[test(creator = @0x123)]
#[expected_failure(abort_code = 327689, location = Self)]
fun test_transfer_no_permission(
creator: &signer,
) acquires ObjectCore {
let aptos_framework = account::create_signer_for_test(@0x1);
timestamp::set_time_has_started_for_testing(&aptos_framework);

let (_, hero) = create_hero(creator);
let (_, weapon) = create_weapon(creator);

// Create a permissioned signer
let creator_permission_handle = permissioned_signer::create_permissioned_handle(creator);
let creator_permission_signer = permissioned_signer::signer_from_permissioned_handle(&creator_permission_handle);

transfer_to_object(&creator_permission_signer, weapon, hero);

permissioned_signer::destroy_permissioned_handle(creator_permission_handle);
}
}
9 changes: 8 additions & 1 deletion aptos-move/framework/aptos-framework/sources/object.spec.move
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ spec aptos_framework::object {
/// </high-level-req>
///
spec module {
pragma aborts_if_is_strict;
pragma aborts_if_is_partial;
}

spec grant_permission {
pragma aborts_if_is_partial;
aborts_if !permissioned_signer::spec_is_permissioned_signer(permissioned_signer);
aborts_if permissioned_signer::spec_is_permissioned_signer(master);
aborts_if signer::address_of(master) != signer::address_of(permissioned_signer);
}

spec fun spec_exists_at<T: key>(object: address): bool;
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/framework/aptos-token-objects/doc/aptos_token.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ With an existing collection, directly mint a soul bound token into the recipient
<a href="token.md#0x4_token_creator">token::creator</a>(*<a href="token.md#0x4_token">token</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_permission_denied">error::permission_denied</a>(<a href="aptos_token.md#0x4_aptos_token_ENOT_CREATOR">ENOT_CREATOR</a>),
);

<b>borrow_global</b>&lt;<a href="aptos_token.md#0x4_aptos_token_AptosToken">AptosToken</a>&gt;(token_address)
}
</code></pre>
Expand Down Expand Up @@ -1561,6 +1562,7 @@ With an existing collection, directly mint a soul bound token into the recipient
<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_permission_denied">error::permission_denied</a>(<a href="aptos_token.md#0x4_aptos_token_ENOT_CREATOR">ENOT_CREATOR</a>),
);

<b>borrow_global</b>&lt;<a href="aptos_token.md#0x4_aptos_token_AptosCollection">AptosCollection</a>&gt;(collection_address)
}
</code></pre>
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/framework/aptos-token-objects/sources/aptos_token.move
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ module aptos_token_objects::aptos_token {
token::creator(*token) == signer::address_of(creator),
error::permission_denied(ENOT_CREATOR),
);

Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any changes in this module? I saw the doc is updated but no code change here.

borrow_global<AptosToken>(token_address)
}

Expand Down Expand Up @@ -614,6 +615,7 @@ module aptos_token_objects::aptos_token {
collection::creator(*collection) == signer::address_of(creator),
error::permission_denied(ENOT_CREATOR),
);

borrow_global<AptosCollection>(collection_address)
}

Expand Down
Loading