[framework] Fungible Asset - Mutable Metadata - Max length checks #13917

Alexzander-Stone · 2024-07-03T20:49:11Z

Description

Added checks for max length of mutate_metadata parameters. Similar implementation as add_fungibility.

Type of Change

Which Components or Systems Does This Change Impact?

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-07-03T20:49:14Z

⏱️ 5h 29m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
test-fuzzers	3h 37m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-move-tests	15m	🟩
rust-move-unit-coverage	13m	🟩
rust-move-tests	10m	🟥
rust-move-tests	9m	🟥
rust-move-tests	9m	🟩
rust-move-unit-coverage	9m	🟩
general-lints	9m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-move-unit-coverage	9m	🟩
rust-move-unit-coverage	8m	🟩
check-dynamic-deps	6m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-move-tests	4m	🟥
rust-move-unit-coverage	4m	🟥
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	21s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	16s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	16s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	15s	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-move-tests	1s	⬜

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

codecov · 2024-07-03T21:16:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.0%. Comparing base (7ec5b41) to head (79d6e19).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #13917   +/-   ##
=======================================
  Coverage    59.0%    59.0%           
=======================================
  Files         820      820           
  Lines      197622   197622           
=======================================
+ Hits       116771   116774    +3     
+ Misses      80851    80848    -3

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

movekevin · 2024-07-08T18:27:03Z

aptos-move/framework/aptos-framework/sources/fungible_asset.move

@@ -1311,13 +1321,13 @@ module aptos_framework::fungible_asset {
        mutate_metadata(


Can you add a couple of test cases for trying to exceed the different limits?

github-actions · 2024-08-24T01:53:27Z

This issue is stale because it has been open 45 days with no activity. Remove the stale label, comment or push a commit - otherwise this will be closed in 15 days.

github-actions · 2024-09-11T20:16:21Z

✅ Forge suite `compat` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `c559d202c0d31310f33fa898eb9d4087465d4d26`

Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26 (PR)
1. Check liveness of validators at old version: d1bf834728a0cf166d993f4728dfca54f3086fb0
compatibility::simple-validator-upgrade::liveness-check : committed: 10573.87 txn/s, latency: 3187.98 ms, (p50: 2400 ms, p70: 2700, p90: 4600 ms, p99: 25700 ms), latency samples: 410560
2. Upgrading first Validator to new version: c559d202c0d31310f33fa898eb9d4087465d4d26
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 6576.21 txn/s, latency: 4231.81 ms, (p50: 4900 ms, p70: 5100, p90: 5200 ms, p99: 5400 ms), latency samples: 119820
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6333.22 txn/s, latency: 4946.45 ms, (p50: 4900 ms, p70: 5100, p90: 7500 ms, p99: 7600 ms), latency samples: 210120
3. Upgrading rest of first batch to new version: c559d202c0d31310f33fa898eb9d4087465d4d26
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7422.64 txn/s, latency: 3844.32 ms, (p50: 4200 ms, p70: 4400, p90: 4600 ms, p99: 4800 ms), latency samples: 140140
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6539.39 txn/s, latency: 4904.82 ms, (p50: 4800 ms, p70: 5000, p90: 7400 ms, p99: 7600 ms), latency samples: 237260
4. upgrading second batch to new version: c559d202c0d31310f33fa898eb9d4087465d4d26
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11758.01 txn/s, latency: 2312.41 ms, (p50: 2400 ms, p70: 2600, p90: 2800 ms, p99: 3000 ms), latency samples: 211280
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 9622.85 txn/s, latency: 3409.27 ms, (p50: 2700 ms, p70: 3000, p90: 9600 ms, p99: 10800 ms), latency samples: 337220
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26 passed
Test Ok

github-actions · 2024-09-11T20:17:00Z

✅ Forge suite `realistic_env_max_load` success on `c559d202c0d31310f33fa898eb9d4087465d4d26`

two traffics test: inner traffic : committed: 12510.24 txn/s, latency: 3181.53 ms, (p50: 3000 ms, p70: 3100, p90: 3900 ms, p99: 9000 ms), latency samples: 4756680
two traffics test : committed: 100.01 txn/s, latency: 2802.89 ms, (p50: 2500 ms, p70: 2600, p90: 3900 ms, p99: 8200 ms), latency samples: 1840
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.233, avg: 0.219", "QsPosToProposal: max: 0.534, avg: 0.315", "ConsensusProposalToOrdered: max: 0.319, avg: 0.303", "ConsensusOrderedToCommit: max: 0.474, avg: 0.459", "ConsensusProposalToCommit: max: 0.779, avg: 0.762"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.08s no progress at version 5362906 (avg 0.22s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 7.78s no progress at version 1635730 (avg 7.44s) [limit 15].
Test Ok

github-actions · 2024-09-11T20:20:19Z

✅ Forge suite `framework_upgrade` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `c559d202c0d31310f33fa898eb9d4087465d4d26`

Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26 (PR)
Upgrade the nodes to version: c559d202c0d31310f33fa898eb9d4087465d4d26
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1231.27 txn/s, submitted: 1233.49 txn/s, failed submission: 2.23 txn/s, expired: 2.23 txn/s, latency: 2615.95 ms, (p50: 2400 ms, p70: 2700, p90: 4400 ms, p99: 5700 ms), latency samples: 110560
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1302.36 txn/s, submitted: 1304.43 txn/s, failed submission: 2.06 txn/s, expired: 2.06 txn/s, latency: 2507.26 ms, (p50: 2400 ms, p70: 2700, p90: 3900 ms, p99: 5200 ms), latency samples: 113680
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26 passed
Upgrade the remaining nodes to version: c559d202c0d31310f33fa898eb9d4087465d4d26
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1052.62 txn/s, submitted: 1054.85 txn/s, failed submission: 2.23 txn/s, expired: 2.23 txn/s, latency: 2796.42 ms, (p50: 2400 ms, p70: 3000, p90: 4500 ms, p99: 6300 ms), latency samples: 94440
Test Ok

Added check for max fields, similar to what add_fungibility does.

997ef17

Alexzander-Stone self-assigned this Jul 3, 2024

Alexzander-Stone requested review from davidiw, movekevin and wrwg as code owners July 3, 2024 20:49

Alexzander-Stone added the move-framework Issues related to the Framework modules/libraries label Jul 3, 2024

FA tests for mutable metadata had too long of symbol length.

79d6e19

FA tests for mutable metadata had too long of symbol length.

1473a0b

movekevin reviewed Jul 8, 2024

View reviewed changes

movekevin approved these changes Jul 8, 2024

View reviewed changes

Alexzander-Stone added 2 commits July 8, 2024 12:14

Added new expecting-failure tests for each mutate_metadata parameter.

61214e0

Merge branch 'main' into mutate-metadata-field-max-lengths

b1b5f40

movekevin requested a review from lightmark July 9, 2024 23:01

github-actions bot added the Stale label Aug 24, 2024

github-actions bot closed this Sep 8, 2024

Alexzander-Stone reopened this Sep 11, 2024

Merge branch 'main' into mutate-metadata-field-max-lengths

c559d20

kshitjee approved these changes Sep 11, 2024

View reviewed changes

Alexzander-Stone enabled auto-merge (squash) September 11, 2024 19:47

This comment has been minimized.

Sign in to view

Alexzander-Stone merged commit 1365a99 into aptos-labs:main Sep 11, 2024
84 of 89 checks passed

Alexzander-Stone deleted the mutate-metadata-field-max-lengths branch September 11, 2024 23:32

Alexzander-Stone removed the Stale label Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[framework] Fungible Asset - Mutable Metadata - Max length checks #13917

[framework] Fungible Asset - Mutable Metadata - Max length checks #13917

Alexzander-Stone commented Jul 3, 2024

trunk-io bot commented Jul 3, 2024 •

edited

Loading

codecov bot commented Jul 3, 2024

movekevin Jul 8, 2024

Alexzander-Stone Jul 8, 2024

github-actions bot commented Aug 24, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

		@@ -1311,13 +1321,13 @@ module aptos_framework::fungible_asset {
		mutate_metadata(

[framework] Fungible Asset - Mutable Metadata - Max length checks #13917

[framework] Fungible Asset - Mutable Metadata - Max length checks #13917

Conversation

Alexzander-Stone commented Jul 3, 2024

Description

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Jul 3, 2024 • edited Loading

codecov bot commented Jul 3, 2024

Codecov Report

movekevin Jul 8, 2024

Choose a reason for hiding this comment

Alexzander-Stone Jul 8, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 24, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 11, 2024

✅ Forge suite compat success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26

github-actions bot commented Sep 11, 2024

✅ Forge suite realistic_env_max_load success on c559d202c0d31310f33fa898eb9d4087465d4d26

github-actions bot commented Sep 11, 2024

✅ Forge suite framework_upgrade success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> c559d202c0d31310f33fa898eb9d4087465d4d26

trunk-io bot commented Jul 3, 2024 •

edited

Loading

✅ Forge suite `compat` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `c559d202c0d31310f33fa898eb9d4087465d4d26`

✅ Forge suite `realistic_env_max_load` success on `c559d202c0d31310f33fa898eb9d4087465d4d26`

✅ Forge suite `framework_upgrade` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `c559d202c0d31310f33fa898eb9d4087465d4d26`