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.

Sign up for GitHub

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

Jump to bottom

[coin/fa] Increase max symbol size to 32 bytes #15846

Merged
merged 2 commits into from
Feb 25, 2025
Merged

[coin/fa] Increase max symbol size to 32 bytes #15846

merged 2 commits into from
Feb 25, 2025
+4 −4

Conversation

gregnazario
Copy link
Contributor

@gregnazario gregnazario commented Jan 30, 2025 •
edited
Loading

Description

This supports more emojis in the space, as 8 bytes is the largest emoji

Right now, with 10, sometimes it can only be 1 emoji

This probably needs an AIP

How Has This Been Tested?

Key Areas to Review

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
  • Move Compiler
  • Other (specify)

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 Trunk.io
Copy link

trunk-io bot commented Jan 30, 2025 •
edited
Loading

⏱️ 10m total CI duration on this PR
Job Cumulative Duration Recent Runs
check-dynamic-deps 4m 🟩 🟩 
rust-cargo-deny 3m 🟩 🟩 
general-lints 53s 🟩 🟩 
semgrep/ci 48s 🟩 🟩 
file_change_determinator 20s 🟩 🟩 
permission-check 4s 🟩 🟩 
permission-check 4s 🟩 🟩 

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@gregnazario gregnazario requested review from lightmark and movekevin and removed request for davidiw, wrwg and movekevin January 30, 2025 04:50
@gregnazario gregnazario force-pushed the increase-coin-symbol-length-to-32-characters branch from 9f67fa2 to 69b81d2 Compare February 1, 2025 17:40
@lightmark
Copy link
Contributor

Does this need a feature flag?

@gregnazario
Copy link
Contributor Author

Does this need a feature flag?

There is one

@gregnazario gregnazario force-pushed the increase-coin-symbol-length-to-32-characters branch from 69b81d2 to 0274d3f Compare February 20, 2025 20:09
@gregnazario gregnazario force-pushed the increase-coin-symbol-length-to-32-characters branch from 4d92614 to 966ece5 Compare February 25, 2025 00:36
@gregnazario gregnazario enabled auto-merge (squash) February 25, 2025 00:54
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea

two traffics test: inner traffic : committed: 12371.60 txn/s, submitted: 12381.12 txn/s, expired: 9.52 txn/s, latency: 3163.45 ms, (p50: 2700 ms, p70: 3000, p90: 3900 ms, p99: 10200 ms), latency samples: 4704000
two traffics test : committed: 99.97 txn/s, latency: 2663.77 ms, (p50: 1900 ms, p70: 2300, p90: 5200 ms, p99: 10900 ms), latency samples: 1700
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.347, avg: 1.001", "ConsensusProposalToOrdered: max: 0.322, avg: 0.313", "ConsensusOrderedToCommit: max: 0.528, avg: 0.459", "ConsensusProposalToCommit: max: 0.840, avg: 0.772"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.24s no progress at version 2439990 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.02s no progress at version 2434268 (avg 1.02s) [limit 16].
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea

Compatibility test results for 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea (PR)
1. Check liveness of validators at old version: 51e3afbaf4645c7a8dd03b94e47555c0dbed0366
compatibility::simple-validator-upgrade::liveness-check : committed: 9978.70 txn/s, latency: 3114.76 ms, (p50: 2700 ms, p70: 3100, p90: 4100 ms, p99: 15800 ms), latency samples: 338760
2. Upgrading first Validator to new version: 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 2720.90 txn/s, latency: 10017.33 ms, (p50: 10800 ms, p70: 13200, p90: 13800 ms, p99: 15400 ms), latency samples: 51140
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 2988.63 txn/s, latency: 11170.40 ms, (p50: 12400 ms, p70: 12800, p90: 12900 ms, p99: 13000 ms), latency samples: 115260
3. Upgrading rest of first batch to new version: 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3314.10 txn/s, latency: 8926.25 ms, (p50: 9600 ms, p70: 10800, p90: 11600 ms, p99: 11700 ms), latency samples: 73760
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3344.50 txn/s, latency: 10301.51 ms, (p50: 11100 ms, p70: 11700, p90: 11800 ms, p99: 11800 ms), latency samples: 127500
4. upgrading second batch to new version: 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 4540.76 txn/s, latency: 5444.47 ms, (p50: 5600 ms, p70: 6000, p90: 7500 ms, p99: 10900 ms), latency samples: 106180
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2209.69 txn/s, submitted: 2209.96 txn/s, expired: 0.27 txn/s, latency: 6551.65 ms, (p50: 5900 ms, p70: 8000, p90: 10800 ms, p99: 12800 ms), latency samples: 178138
5. check swarm health
Compatibility test for 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea passed
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea

Compatibility test results for 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea (PR)
Upgrade the nodes to version: 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1556.40 txn/s, submitted: 1561.06 txn/s, failed submission: 4.66 txn/s, expired: 4.66 txn/s, latency: 1898.27 ms, (p50: 1600 ms, p70: 2100, p90: 3000 ms, p99: 4700 ms), latency samples: 140160
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1654.69 txn/s, submitted: 1661.07 txn/s, failed submission: 6.38 txn/s, expired: 6.38 txn/s, latency: 1739.15 ms, (p50: 1500 ms, p70: 1800, p90: 2700 ms, p99: 4200 ms), latency samples: 150441
5. check swarm health
Compatibility test for 51e3afbaf4645c7a8dd03b94e47555c0dbed0366 ==> 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea passed
Upgrade the remaining nodes to version: 966ece5f28f44fcfa9bf8e5eb31902ce72f1f1ea
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 880.07 txn/s, submitted: 882.74 txn/s, failed submission: 2.67 txn/s, expired: 2.67 txn/s, latency: 3574.35 ms, (p50: 2400 ms, p70: 4700, p90: 7500 ms, p99: 9900 ms), latency samples: 79041
Test Ok

@gregnazario gregnazario merged commit bc2244b into aptos-labs:main Feb 25, 2025
87 of 88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lightmark lightmark lightmark approved these changes

@igor-aptos igor-aptos igor-aptos approved these changes

@movekevin movekevin Awaiting requested review from movekevin movekevin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gregnazario @lightmark @igor-aptos