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

[fix] make the max supply unlimited #13913

Merged
merged 1 commit into from
Jul 10, 2024
Merged

[fix] make the max supply unlimited #13913

merged 1 commit into from
Jul 10, 2024

Conversation

lightmark
Copy link
Contributor

Description

Coin has unlimited supply, so does FA.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

How Has This Been Tested?

Key Areas to Review

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

Copy link

trunk-io bot commented Jul 3, 2024

⏱️ 10h 10m total CI duration on this PR
Job Cumulative Duration Recent Runs
test-fuzzers 4h 15m 🟩🟩🟩🟩🟩 (+2 more)
execution-performance / single-node-performance 56m 🟩🟩
forge-framework-upgrade-test / forge 34m 🟩🟩
forge-e2e-test / forge 31m 🟩🟩
forge-compat-test / forge 28m 🟩🟩
rust-images / rust-all 24m 🟩🟩
rust-move-tests 15m 🟩
rust-move-tests 14m 🟩
rust-move-unit-coverage 13m 🟩
general-lints 13m 🟩🟩🟩🟩🟩 (+2 more)
execution-performance / test-target-determinator 12m 🟩🟩
test-target-determinator 12m 🟩🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-unit-coverage 9m 🟩
rust-move-unit-coverage 9m 🟩
rust-cargo-deny 8m 🟩🟩🟩🟩🟩
rust-move-unit-coverage 8m 🟩
rust-move-unit-coverage 8m 🟩
check 7m 🟩🟩
check-dynamic-deps 6m 🟩🟩🟩🟩🟩 (+2 more)
semgrep/ci 3m 🟩🟩🟩🟩🟩 (+2 more)
rust-move-unit-coverage 3m
rust-move-tests 3m
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+2 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+2 more)
permission-check 40s 🟩🟩🟩🟩🟩 (+2 more)
file_change_determinator 24s 🟩🟩
permission-check 24s 🟩🟩🟩🟩🟩 (+2 more)
permission-check 23s 🟩🟩🟩🟩🟩 (+2 more)
permission-check 22s 🟩🟩🟩🟩🟩 (+2 more)
determine-docker-build-metadata 6s 🟩🟩
permission-check 4s 🟩🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.0%. Comparing base (b1b6b05) to head (4ee6962).
Report is 13 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #13913       +/-   ##
===========================================
- Coverage    71.9%    59.0%    -13.0%     
===========================================
  Files        2316      822     -1494     
  Lines      457023   198161   -258862     
===========================================
- Hits       328998   117008   -211990     
+ Misses     128025    81153    -46872     

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

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 test to make sure you can still mint the FA after migration?

@@ -1011,12 +1011,12 @@ module aptos_framework::fungible_asset {

public(friend) fun deposit_internal(store_addr: address, fa: FungibleAsset) acquires FungibleStore, ConcurrentFungibleBalance {
let FungibleAsset { metadata, amount } = fa;
if (amount == 0) return;

assert!(exists<FungibleStore>(store_addr), error::not_found(EFUNGIBLE_STORE_EXISTENCE));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to the first line? Line 1013 is only needed for the subsequent metadata check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is nit 5 fix...

@lightmark lightmark force-pushed the lightmark/fix_supply branch 3 times, most recently from 57d7a68 to 294be9b Compare July 10, 2024 15:39
@sherry-x sherry-x enabled auto-merge (squash) July 10, 2024 16:04

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.

Copy link
Contributor

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 9639.046712787369 txn/s, latency: 3458.638977480219 ms, (p50: 2500 ms, p90: 6300 ms, p99: 22100 ms), latency samples: 328600
2. Upgrading first Validator to new version: 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 3333.207065272424 txn/s, latency: 7154.373614511463 ms, (p50: 8300 ms, p90: 9700 ms, p99: 11700 ms), latency samples: 89860
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3316.495449975346 txn/s, latency: 9362.049896739933 ms, (p50: 9400 ms, p90: 14800 ms, p99: 15100 ms), latency samples: 135580
3. Upgrading rest of first batch to new version: 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3568.078296312023 txn/s, latency: 7279.256916099774 ms, (p50: 8700 ms, p90: 9700 ms, p99: 10200 ms), latency samples: 88200
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3273.1763496020158 txn/s, latency: 9500.784993427778 ms, (p50: 9400 ms, p90: 14800 ms, p99: 15200 ms), latency samples: 136940
4. upgrading second batch to new version: 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 328.1384195260937 txn/s, submitted: 783.5894797543737 txn/s, expired: 455.45106022828 txn/s, latency: 15634.35358281591 ms, (p50: 3400 ms, p90: 55600 ms, p99: 60500 ms), latency samples: 23836
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6540.039884618171 txn/s, latency: 5064.327317348623 ms, (p50: 4900 ms, p90: 8100 ms, p99: 9300 ms), latency samples: 230220
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b

two traffics test: inner traffic : committed: 8495.520623795182 txn/s, latency: 4611.204060437199 ms, (p50: 4500 ms, p90: 5700 ms, p99: 9800 ms), latency samples: 3675220
two traffics test : committed: 100.09249071853199 txn/s, latency: 2044.453888888889 ms, (p50: 2000 ms, p90: 2300 ms, p99: 2900 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.212, avg: 0.208", "QsPosToProposal: max: 0.198, avg: 0.178", "ConsensusProposalToOrdered: max: 0.307, avg: 0.286", "ConsensusOrderedToCommit: max: 0.385, avg: 0.372", "ConsensusProposalToCommit: max: 0.670, avg: 0.659"]
Max round gap was 1 [limit 4] at version 1799094. Max no progress secs was 5.265027 [limit 15] at version 1799094.
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b (PR)
Upgrade the nodes to version: 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1145.6188456728978 txn/s, submitted: 1148.7569133386032 txn/s, failed submission: 3.1380676657054525 txn/s, expired: 3.1380676657054525 txn/s, latency: 2618.2013304637057 ms, (p50: 2100 ms, p90: 4800 ms, p99: 7800 ms), latency samples: 102220
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1104.856036083624 txn/s, submitted: 1108.4352417077055 txn/s, failed submission: 3.57920562408139 txn/s, expired: 3.57920562408139 txn/s, latency: 2719.466956873861 ms, (p50: 1800 ms, p90: 4800 ms, p99: 12700 ms), latency samples: 98780
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b passed
Upgrade the remaining nodes to version: 4ee69620dffaa9af56f35f1343f3bc4fc7f7237b
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1085.3698635345086 txn/s, submitted: 1086.685197645033 txn/s, failed submission: 1.3153341105245508 txn/s, expired: 1.3153341105245508 txn/s, latency: 2870.527741870329 ms, (p50: 2100 ms, p90: 4500 ms, p99: 9600 ms), latency samples: 99020
Test Ok

@sherry-x sherry-x merged commit e5f4b62 into main Jul 10, 2024
56 checks passed
@sherry-x sherry-x deleted the lightmark/fix_supply branch July 10, 2024 17:40
zi0Black pushed a commit that referenced this pull request Jul 12, 2024
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.

None yet

4 participants