Create token with collection object. #13215

JohnChangUK · 2024-05-06T21:54:48Z

Description

Amend create token functions which take in Object<Collection> to create tokens using the Object<Collection> rather than the collection name.

If the collection name is changed, the token creation will fail as the current token creation implementation still generates the collection with collection_name.
To circumvent this, a new function which directly creates tokens from the Object<Collection> is added.

Extra documentation will be added to the set_name function in collection.move, and the current token creation functions which take in collection_name.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Unit tests added.

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-05-06T21:54:51Z

⏱️ 6h 9m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-move-unit-coverage	1h 55m	🟩 🟩 🟩 🟩 🟩
rust-targeted-unit-tests	1h 49m	🟩 🟩 🟩 🟩 🟩
rust-move-tests	1h 16m	🟩 🟩 🟩 🟩 🟩
rust-lints	36m	🟩 🟩 🟩 🟩
run-tests-main-branch	17m	🟩 🟩 🟩 🟩
general-lints	7m	🟩 🟩 🟩 🟩
check-dynamic-deps	5m	🟩 🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩
file_change_determinator	46s	🟩 🟩 🟩 🟩
file_change_determinator	44s	🟩 🟩 🟩 🟩
permission-check	11s	🟩 🟩 🟩 🟩
permission-check	10s	🟩 🟩 🟩 🟩
permission-check	10s	🟩 🟩 🟩 🟩
permission-check	8s	🟩 🟩 🟩 🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
rust-targeted-unit-tests	26m	21m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

codecov · 2024-05-06T22:21:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.6%. Comparing base (6cdd4c2) to head (b2a5b39).
Report is 4 commits behind head on main.

❗ Current head b2a5b39 differs from pull request most recent head b46bd57. Consider uploading reports for the commit b46bd57 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #13215   +/-   ##
=======================================
  Coverage    57.6%    57.6%           
=======================================
  Files         834      834           
  Lines      198397   198397           
=======================================
  Hits       114318   114318           
  Misses      84079    84079

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aptos-move/framework/aptos-token-objects/sources/token.move

davidiw · 2024-05-15T06:01:21Z

aptos-move/framework/aptos-token-objects/sources/token.move

+        // Create initial token with current collection name
+        create_token_with_collection_helper(creator, collection, token_name);
+
+        // Set new collection name


sorta minor nit, these comments are not helpful, because if you can't understand collection::set_name you will have difficulty with the comment...

Let me remove these comments which don't add context.

davidiw · 2024-05-15T06:01:33Z

aptos-move/framework/aptos-token-objects/sources/token.move

+        // Set new collection name
+        collection::set_name(&mutator_ref, string::utf8(b"new collection name"));
+
+        // Create new token after changing collection name


same here...

if you want, put at the top a brief description of the test.

github-actions · 2024-05-15T20:35:45Z

✅ Forge suite `realistic_env_max_load` success on `b46bd57515ed498f98b3c7413a2d20508a5b3a43`

two traffics test: inner traffic : committed: 7541.22650795059 txn/s, latency: 5189.714788413253 ms, (p50: 5100 ms, p90: 6000 ms, p99: 10600 ms), latency samples: 3265800
two traffics test : committed: 99.98293405642796 txn/s, latency: 1871.2363636363636 ms, (p50: 1800 ms, p90: 2100 ms, p99: 2400 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.208, avg: 0.201", "QsPosToProposal: max: 0.255, avg: 0.234", "ConsensusProposalToOrdered: max: 0.455, avg: 0.409", "ConsensusOrderedToCommit: max: 0.375, avg: 0.354", "ConsensusProposalToCommit: max: 0.773, avg: 0.763"]
Max round gap was 1 [limit 4] at version 1558464. Max no progress secs was 4.518884 [limit 15] at version 1558464.
Test Ok

github-actions · 2024-05-15T20:37:59Z

✅ Forge suite `compat` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `b46bd57515ed498f98b3c7413a2d20508a5b3a43`

Compatibility test results for 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43 (PR)
1. Check liveness of validators at old version: 01b24e7e3548382dd25440b39a0438a993387f12
compatibility::simple-validator-upgrade::liveness-check : committed: 6173.834791780183 txn/s, latency: 5106.562027893277 ms, (p50: 4800 ms, p90: 9300 ms, p99: 12000 ms), latency samples: 230880
2. Upgrading first Validator to new version: b46bd57515ed498f98b3c7413a2d20508a5b3a43
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1524.0237397634417 txn/s, latency: 19073.617655140828 ms, (p50: 17900 ms, p90: 30100 ms, p99: 30500 ms), latency samples: 87340
3. Upgrading rest of first batch to new version: b46bd57515ed498f98b3c7413a2d20508a5b3a43
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1391.2438269792028 txn/s, latency: 20271.336510344827 ms, (p50: 21500 ms, p90: 27400 ms, p99: 29600 ms), latency samples: 72500
4. upgrading second batch to new version: b46bd57515ed498f98b3c7413a2d20508a5b3a43
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3297.8794337093996 txn/s, latency: 9419.294061707524 ms, (p50: 9700 ms, p90: 12700 ms, p99: 12900 ms), latency samples: 141960
5. check swarm health
Compatibility test for 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43 passed
Test Ok

github-actions · 2024-05-15T21:38:22Z

✅ Forge suite `framework_upgrade` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `b46bd57515ed498f98b3c7413a2d20508a5b3a43`

Compatibility test results for 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43 (PR)
Upgrade the nodes to version: b46bd57515ed498f98b3c7413a2d20508a5b3a43
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1094.2673004731967 txn/s, submitted: 1096.245686512622 txn/s, failed submission: 1.978386039425225 txn/s, expired: 1.978386039425225 txn/s, latency: 2757.715096424267 ms, (p50: 2200 ms, p90: 5400 ms, p99: 7800 ms), latency samples: 99560
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1092.6491690930711 txn/s, submitted: 1096.2053909615597 txn/s, failed submission: 3.5562218684884335 txn/s, expired: 3.5562218684884335 txn/s, latency: 2763.405807567128 ms, (p50: 2400 ms, p90: 4500 ms, p99: 6100 ms), latency samples: 98320
5. check swarm health
Compatibility test for 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43 passed
Upgrade the remaining nodes to version: b46bd57515ed498f98b3c7413a2d20508a5b3a43
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1116.3685613139908 txn/s, submitted: 1118.8021828309259 txn/s, failed submission: 2.433621516934978 txn/s, expired: 2.433621516934978 txn/s, latency: 2719.4867419738407 ms, (p50: 2400 ms, p90: 4500 ms, p99: 6800 ms), latency samples: 100920
Test Ok

JohnChangUK requested a review from BriungRi May 6, 2024 21:54

JohnChangUK requested review from davidiw, movekevin and wrwg as code owners May 6, 2024 21:54

BriungRi reviewed May 7, 2024

View reviewed changes

aptos-move/framework/aptos-token-objects/sources/token.move Show resolved Hide resolved

JohnChangUK force-pushed the fix-create-token branch from c20b4fb to 494d06c Compare May 7, 2024 21:07

BriungRi reviewed May 7, 2024

View reviewed changes

aptos-move/framework/aptos-token-objects/sources/token.move Show resolved Hide resolved

aptos-move/framework/aptos-token-objects/sources/token.move Show resolved Hide resolved

JohnChangUK force-pushed the fix-create-token branch from 494d06c to 1b8a5f1 Compare May 8, 2024 00:24

JohnChangUK closed this May 8, 2024

JohnChangUK reopened this May 8, 2024

JohnChangUK requested review from jillxuu and lightmark May 8, 2024 17:35

JohnChangUK force-pushed the fix-create-token branch from 1b8a5f1 to 2634803 Compare May 12, 2024 21:21

aptos-labs deleted a comment May 13, 2024

BriungRi approved these changes May 13, 2024

View reviewed changes

davidiw approved these changes May 15, 2024

View reviewed changes

Create token with collection object.

b2a5b39

JohnChangUK force-pushed the fix-create-token branch from 2634803 to b2a5b39 Compare May 15, 2024 16:14

sherry-x enabled auto-merge (squash) May 15, 2024 16:21

sherry-x disabled auto-merge May 15, 2024 16:21

This comment has been minimized.

Sign in to view

JohnChangUK enabled auto-merge (squash) May 15, 2024 17:01

This comment has been minimized.

Sign in to view

Merge branch 'main' into fix-create-token

b46bd57

This comment has been minimized.

Sign in to view

JohnChangUK merged commit dc1a28c into main May 15, 2024
50 of 52 checks passed

JohnChangUK deleted the fix-create-token branch May 15, 2024 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create token with collection object. #13215

Create token with collection object. #13215

JohnChangUK commented May 6, 2024 •

edited

trunk-io bot commented May 6, 2024 •

edited

codecov bot commented May 6, 2024 •

edited

davidiw May 15, 2024

JohnChangUK May 15, 2024

davidiw May 15, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

Create token with collection object. #13215

Create token with collection object. #13215

Conversation

JohnChangUK commented May 6, 2024 • edited

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Checklist

trunk-io bot commented May 6, 2024 • edited

codecov bot commented May 6, 2024 • edited

Codecov Report

davidiw May 15, 2024

Choose a reason for hiding this comment

JohnChangUK May 15, 2024

Choose a reason for hiding this comment

davidiw May 15, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 15, 2024

✅ Forge suite realistic_env_max_load success on b46bd57515ed498f98b3c7413a2d20508a5b3a43

github-actions bot commented May 15, 2024

✅ Forge suite compat success on 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43

github-actions bot commented May 15, 2024

✅ Forge suite framework_upgrade success on 01b24e7e3548382dd25440b39a0438a993387f12 ==> b46bd57515ed498f98b3c7413a2d20508a5b3a43

JohnChangUK commented May 6, 2024 •

edited

trunk-io bot commented May 6, 2024 •

edited

codecov bot commented May 6, 2024 •

edited

✅ Forge suite `realistic_env_max_load` success on `b46bd57515ed498f98b3c7413a2d20508a5b3a43`

✅ Forge suite `compat` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `b46bd57515ed498f98b3c7413a2d20508a5b3a43`

✅ Forge suite `framework_upgrade` success on `01b24e7e3548382dd25440b39a0438a993387f12` ==> `b46bd57515ed498f98b3c7413a2d20508a5b3a43`