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

Add create_token method in token.move #8825

Merged
merged 11 commits into from
Jun 26, 2023
Merged

Conversation

vusirikala
Copy link
Contributor

Description

This PR adds create_token method in token.move file.

@@ -106,6 +106,22 @@ module aptos_token_objects::token {
};
}

/// Creates a new token object with a unique address and returns the ConstructorRef
/// for additional specialization.
public fun create_token(
Copy link
Contributor

Choose a reason for hiding this comment

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

Mark create_from_account as deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Can you add a unit test for the new function?

Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Also can you check the docs as well to make sure we recommend the right function?

Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Can you update any uses of create_named_token in move-examples that are not justified as well?

@vusirikala
Copy link
Contributor Author

Can you update any uses of create_named_token in move-examples that are not justified as well?

I've check that right now we are using create_named_token only in test cases.

@@ -199,7 +199,7 @@ module aptos_token_objects::aptos_token {
property_types: vector<String>,
property_values: vector<vector<u8>>,
): ConstructorRef acquires AptosCollection {
let constructor_ref = token::create_from_account(
let constructor_ref = token::create_token(
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be behind the same feature flag as create_guid otherwise this would always fail before the flag is enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If auids are enabled, I am now calling create method. If auids are disabled, I am calling create_from_account method. Is that okay?

@@ -106,6 +106,22 @@ module aptos_token_objects::token {
};
}

/// Creates a new token object with a unique address and returns the ConstructorRef
/// for additional specialization.
public fun create_token(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: How about just name this "create"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@vusirikala
Copy link
Contributor Author

Can you add a unit test for the new function?

I added a unit test now. But it runs only if the AUIDs are enabled.

option::none(),
uri,
);
let constructor_ref = if (std::features::auids_enabled()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you add an import for std::features and just call features::auids_enabled here to keep it consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@movekevin
Copy link
Contributor

Can you add a unit test for the new function?

I added a unit test now. But it runs only if the AUIDs are enabled.

That's fine. It should pass after merging this into the create-guid branch

@vusirikala vusirikala merged commit 18b4e3b into satya/create-guid Jun 26, 2023
@vusirikala vusirikala deleted the satya/token-uuid branch June 26, 2023 18:47
vusirikala added a commit that referenced this pull request Jun 26, 2023
* Add native function create_guid

* Append a constant to hash_arg

* Add move test to verify uniqueness of guids

* Charging gas for create_guid method

* Add gas for create guid

* Add create_guid to object.move

* Change GUID to UUID

* Change GUID to UUID in tests

* Change GUID to UUID in tests

* Add create_object method in object.move

* Fix typo

* Fix typo

* fix trigger condition for build jobs

* Add native_get_txn_hash

* Add native_get_txn_hash

* Deprecate the old create_object_from_object and create_object_from_account

* Bumped the latest_gas_version to 10

* Bumped gas version to 10

* Changed gas feature version to 9

* Moved transaction hash hashing to authenticator.rs

* Add documentation to create_uuid method

* Use AuthenticationKey struct for create_uuid

* Remove deprecated comment

* Updatd comments in object.move

* Changed create_uuid to use only one hash

* Add create_unique_address function

* Add drop, store capabilities to UUID

* Resetting the create_object_from_account function

* Add comment for create_object function

* feature gating

* deprecated tag

* Add transaction context spec

* Add a test case in transaction_context

* Updated move test case

* Add create_token method

* Update test cases

* rust lint

* Changed comments

* Remove create_token method

* Add feature flag for test

* fix a unit test

* Fix unit test

* moved unit test to object.move

* changing uuid to auid

* rust lint

* Minor changes

* Change uuid to auid in tests

* Change uuid to auid in tests

* changed name to generate_unique_address

* Fixed the specs which timeout

* Fixed a unit test

* Shift test from e2e-move-tests to transaction_context.move

* Add a comment in object.move

* rust lint

* Add create_token method in token.move (#8825)

* Add create_token method in token.move

* add deprecated flag to create_token_from_account

* Add test case and rename to create

* Add feature flag condition in test case

* enabled feature flag in the unit test

* Enabling auid feature flag in ambassador unit tests

* Enable auid flag in an ambassador.move unit test

* Changed gas cost

* changed name from generate_unique_address to generate_auid_address

* Updated a comment

* Feature gate get_txn_hash

* changing names in test cases

---------

Co-authored-by: geekflyer <[email protected]>
Co-authored-by: Junkil Park <[email protected]>
gedigi pushed a commit that referenced this pull request Aug 2, 2023
* Add native function create_guid

* Append a constant to hash_arg

* Add move test to verify uniqueness of guids

* Charging gas for create_guid method

* Add gas for create guid

* Add create_guid to object.move

* Change GUID to UUID

* Change GUID to UUID in tests

* Change GUID to UUID in tests

* Add create_object method in object.move

* Fix typo

* Fix typo

* fix trigger condition for build jobs

* Add native_get_txn_hash

* Add native_get_txn_hash

* Deprecate the old create_object_from_object and create_object_from_account

* Bumped the latest_gas_version to 10

* Bumped gas version to 10

* Changed gas feature version to 9

* Moved transaction hash hashing to authenticator.rs

* Add documentation to create_uuid method

* Use AuthenticationKey struct for create_uuid

* Remove deprecated comment

* Updatd comments in object.move

* Changed create_uuid to use only one hash

* Add create_unique_address function

* Add drop, store capabilities to UUID

* Resetting the create_object_from_account function

* Add comment for create_object function

* feature gating

* deprecated tag

* Add transaction context spec

* Add a test case in transaction_context

* Updated move test case

* Add create_token method

* Update test cases

* rust lint

* Changed comments

* Remove create_token method

* Add feature flag for test

* fix a unit test

* Fix unit test

* moved unit test to object.move

* changing uuid to auid

* rust lint

* Minor changes

* Change uuid to auid in tests

* Change uuid to auid in tests

* changed name to generate_unique_address

* Fixed the specs which timeout

* Fixed a unit test

* Shift test from e2e-move-tests to transaction_context.move

* Add a comment in object.move

* rust lint

* Add create_token method in token.move (#8825)

* Add create_token method in token.move

* add deprecated flag to create_token_from_account

* Add test case and rename to create

* Add feature flag condition in test case

* enabled feature flag in the unit test

* Enabling auid feature flag in ambassador unit tests

* Enable auid flag in an ambassador.move unit test

* Changed gas cost

* changed name from generate_unique_address to generate_auid_address

* Updated a comment

* Feature gate get_txn_hash

* changing names in test cases

---------

Co-authored-by: geekflyer <[email protected]>
Co-authored-by: Junkil Park <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants