Skip to content

Conversation

wrwg
Copy link
Contributor

@wrwg wrwg commented Oct 9, 2025

Description

This enables intermediate tools to use $ in identifiers to separate from user identifiers, e.g. names for public struct functions.

How Has This Been Tested?

Existing and new tests

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)

Copy link
Contributor Author

wrwg commented Oct 9, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@rahxephon89
Copy link
Contributor

In an unlikely scenario where we are upgrading the validators with this change, will there be a possibility of divergence if someone submits code with $ used in function names? @wrwg

Copy link
Contributor

@vgao1996 vgao1996 left a comment

Choose a reason for hiding this comment

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

Is my understanding correct that with this change, users will be able to include $ in the bytecode if they hack it directly, right?

Copy link
Contributor Author

wrwg commented Oct 9, 2025

Teng is right, we need a feature flag.

Yes, this will allow $ in identifiers in the VM, the assembler for example can do it.

Copy link
Contributor Author

wrwg commented Oct 10, 2025

Now gated by bytecode version 9

@wrwg wrwg requested a review from vgao1996 October 10, 2025 01:08
This enables intermediate tools to use `$` in identifiers to separate from user identifiers, e.g. names for public struct functions. This is only supported from bytecode version 9 onwards.
@rahxephon89 rahxephon89 enabled auto-merge (squash) October 14, 2025 03:46

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526

Compatibility test results for 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526 (PR)
1. Check liveness of validators at old version: 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f
compatibility::simple-validator-upgrade::liveness-check : committed: 14414.06 txn/s, latency: 2406.60 ms, (p50: 2500 ms, p70: 2600, p90: 2800 ms, p99: 3600 ms), latency samples: 468300
2. Upgrading first Validator to new version: fa3581267286c1be198f5ddb2ebdd25659060526
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5227.96 txn/s, latency: 6590.52 ms, (p50: 7300 ms, p70: 7400, p90: 7400 ms, p99: 7500 ms), latency samples: 181040
3. Upgrading rest of first batch to new version: fa3581267286c1be198f5ddb2ebdd25659060526
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 5225.37 txn/s, latency: 6596.64 ms, (p50: 7300 ms, p70: 7400, p90: 7500 ms, p99: 7500 ms), latency samples: 181160
4. upgrading second batch to new version: fa3581267286c1be198f5ddb2ebdd25659060526
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8272.71 txn/s, latency: 4122.39 ms, (p50: 4400 ms, p70: 4600, p90: 4700 ms, p99: 4900 ms), latency samples: 275760
5. check swarm health
Compatibility test for 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526 passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on fa3581267286c1be198f5ddb2ebdd25659060526

two traffics test: inner traffic : committed: 13803.57 txn/s, latency: 2727.07 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3400 ms), latency samples: 5248480
two traffics test : committed: 99.97 txn/s, latency: 844.70 ms, (p50: 800 ms, p70: 900, p90: 1000 ms, p99: 1900 ms), latency samples: 1700
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.231, avg: 2.045", "ConsensusProposalToOrdered: max: 0.167, avg: 0.164", "ConsensusOrderedToCommit: max: 0.115, avg: 0.097", "ConsensusProposalToCommit: max: 0.280, avg: 0.261"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.83s no progress at version 4843158 (avg 0.07s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.42s no progress at version 2265505 (avg 0.42s) [limit 16].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526

Compatibility test results for 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526 (PR)
Upgrade the nodes to version: fa3581267286c1be198f5ddb2ebdd25659060526
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2113.08 txn/s, submitted: 2121.22 txn/s, failed submission: 8.14 txn/s, expired: 8.14 txn/s, latency: 1398.09 ms, (p50: 1500 ms, p70: 1500, p90: 1800 ms, p99: 2200 ms), latency samples: 192161
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2034.29 txn/s, submitted: 2039.46 txn/s, failed submission: 5.17 txn/s, expired: 5.17 txn/s, latency: 1471.97 ms, (p50: 1500 ms, p70: 1500, p90: 1800 ms, p99: 2100 ms), latency samples: 181100
5. check swarm health
Compatibility test for 75cc050c2c35782fc994c2a7e61a7fb3ea88f71f ==> fa3581267286c1be198f5ddb2ebdd25659060526 passed
Upgrade the remaining nodes to version: fa3581267286c1be198f5ddb2ebdd25659060526
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2138.30 txn/s, submitted: 2145.38 txn/s, failed submission: 7.07 txn/s, expired: 7.07 txn/s, latency: 1388.61 ms, (p50: 1500 ms, p70: 1500, p90: 1800 ms, p99: 2100 ms), latency samples: 193501
Test Ok

@rahxephon89 rahxephon89 merged commit a1ad678 into main Oct 14, 2025
80 of 89 checks passed
@rahxephon89 rahxephon89 deleted the wrwg/ident_ext branch October 14, 2025 04:36
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.

5 participants