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

[token] added functions for mutating token data #5017

Merged
merged 1 commit into from
Oct 18, 2022
Merged

Conversation

0xchloe
Copy link
Contributor

@0xchloe 0xchloe commented Oct 13, 2022

Description

^

p.s. didn't delete any existing functions/tests, just moved the order so that it'll be in the order of maximum, uri, royalty, description, and properties

Test Plan

unit tests


This change is Reviewable

@0xchloe 0xchloe requested a review from davidiw October 13, 2022 04:28
@0xchloe 0xchloe requested a review from areshand as a code owner October 13, 2022 04:28
@0xchloe 0xchloe requested a review from movekevin October 13, 2022 04:34
Copy link
Contributor

@areshand areshand left a comment

Choose a reason for hiding this comment

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

Looking great, thanks for reordering the functions

@0xchloe 0xchloe requested a review from areshand October 13, 2022 20:53
@0xchloe 0xchloe force-pushed the mutate_token_data branch 2 times, most recently from 196fc53 to 70fae48 Compare October 14, 2022 20:54
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.

May be refactor the validations into a common function so we don't need to do it multiple times in these mutate functions.

aptos-move/framework/aptos-token/sources/token.move Outdated Show resolved Hide resolved
@0xchloe 0xchloe force-pushed the mutate_token_data branch 8 times, most recently from 8553abc to 1b1542d Compare October 16, 2022 02:09
@0xchloe 0xchloe force-pushed the mutate_token_data branch 2 times, most recently from fa5890f to 61ef496 Compare October 18, 2022 00:28
@0xchloe 0xchloe enabled auto-merge (squash) October 18, 2022 01:19
@0xchloe 0xchloe requested a review from jjleng as a code owner October 18, 2022 01:56
@0xchloe 0xchloe disabled auto-merge October 18, 2022 01:56
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@0xchloe 0xchloe enabled auto-merge (squash) October 18, 2022 06:36
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

❌ Forge suite compat failure on 843b204dce971d98449b82624f4f684c7a18b991 ==> 15fd3884b60dcb0a8c4c517d286fe39e9d4df549

Forge test runner terminated:
Trailing Log Lines:
Error from server (BadRequest): container "genesis" in pod "genesis-aptos-genesis-eforge147-7frhn" is waiting to start: trying and failing to pull image
{"level":"INFO","source":{"package":"forge","file":"testsuite/forge/src/backend/k8s/cluster_helper.rs:81"},"thread_name":"main","hostname":"forge-compat-pr-5017-1666077826-843b204dce971d98449b82624f4f684","timestamp":"2022-10-18T07:34:43.134360Z","message":"Genesis status: JobStatus { active: Some(1), completion_time: None, conditions: None, failed: None, start_time: Some(Time(2022-10-18T07:24:56Z)), succeeded: None }"}
{"level":"INFO","source":{"package":"forge","file":"testsuite/forge/src/backend/k8s/cluster_helper.rs:63"},"thread_name":"main","hostname":"forge-compat-pr-5017-1666077826-843b204dce971d98449b82624f4f684","timestamp":"2022-10-18T07:34:53.142384Z","message":"Genesis status: JobStatus { active: Some(1), completion_time: None, conditions: None, failed: None, start_time: Some(Time(2022-10-18T07:24:56Z)), succeeded: None }"}
Error from server (BadRequest): container "genesis" in pod "genesis-aptos-genesis-eforge147-7frhn" is waiting to start: trying and failing to pull image
{"level":"INFO","source":{"package":"forge","file":"testsuite/forge/src/backend/k8s/cluster_helper.rs:81"},"thread_name":"main","hostname":"forge-compat-pr-5017-1666077826-843b204dce971d98449b82624f4f684","timestamp":"2022-10-18T07:34:53.216849Z","message":"Genesis status: JobStatus { active: Some(1), completion_time: None, conditions: None, failed: None, start_time: Some(Time(2022-10-18T07:24:56Z)), succeeded: None }"}
{"level":"INFO","source":{"package":"forge","file":"testsuite/forge/src/backend/k8s/cluster_helper.rs:280"},"thread_name":"main","hostname":"forge-compat-pr-5017-1666077826-843b204dce971d98449b82624f4f684","timestamp":"2022-10-18T07:34:53.234809Z","message":"Deleting namespace forge-compat-pr-5017: Some(NamespaceStatus { phase: Some(\"Terminating\") })"}
{"level":"INFO","source":{"package":"forge","file":"testsuite/forge/src/backend/k8s/cluster_helper.rs:388"},"thread_name":"main","hostname":"forge-compat-pr-5017-1666077826-843b204dce971d98449b82624f4f684","timestamp":"2022-10-18T07:34:53.234833Z","message":"aptos-node resources for Forge removed in namespace: forge-compat-pr-5017"}
Failed to run tests:
Genesis did not succeed
Error: Genesis did not succeed
Debugging output:
NAME                                    READY   STATUS        RESTARTS   AGE
genesis-aptos-genesis-eforge147-7frhn   0/1     Terminating   0          10m

@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 15fd3884b60dcb0a8c4c517d286fe39e9d4df549

performance benchmark with full nodes : 6497 TPS, 6105 ms latency, 10800 ms p99 latency,(!) expired 1740 out of 2775980 txns
Test Ok

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.

4 participants