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

[trivial] Add intrinsic for limit for aggregator and update movefmt version #15177

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

rahxephon89
Copy link
Contributor

@rahxephon89 rahxephon89 commented Nov 4, 2024

Description

This PR

  1. closes [Bug][move-prover] warning about aggregator.limit from prover tests on framework code #15135 by adding intrinsic implementation of limit method for aggregator
  2. updates installation of movefmt from 1.0.5 to 1.0.6.

How Has This Been Tested?

  1. existing tests pass with no warning;
  2. manual CLI test

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
  • prover and movefmt

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 Nov 4, 2024

@rahxephon89 rahxephon89 changed the title [WIP] Add intrinsic for limit for aggregator and update movefmt version [trivial] Add intrinsic for limit for aggregator and update movefmt version Nov 4, 2024
@rahxephon89 rahxephon89 marked this pull request as ready for review November 4, 2024 19:42
Copy link
Contributor

@brmataptos brmataptos left a comment

Choose a reason for hiding this comment

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

Better not to mix 2 unrelated PRs together, unless these are mutually dependent in some way I can't see. But we don't seem to want an
intelligible commit history, anyway.

@rahxephon89
Copy link
Contributor Author

Better not to mix 2 unrelated PRs together, unless these are mutually dependent in some way I can't see. But we don't seem to want an intelligible commit history, anyway.

Yeah, I thought changing movefmt version might not be worth a standalone PR. Will submit separately next time

@rahxephon89 rahxephon89 enabled auto-merge (squash) November 4, 2024 21:05

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Nov 4, 2024

✅ Forge suite realistic_env_max_load success on 29331886905443bca0af248a5e9fafd495127920

two traffics test: inner traffic : committed: 14338.14 txn/s, latency: 2770.61 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3300 ms), latency samples: 5451820
two traffics test : committed: 100.09 txn/s, latency: 1512.59 ms, (p50: 1400 ms, p70: 1400, p90: 1600 ms, p99: 10900 ms), latency samples: 1780
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.970, avg: 1.573", "ConsensusProposalToOrdered: max: 0.329, avg: 0.296", "ConsensusOrderedToCommit: max: 0.369, avg: 0.358", "ConsensusProposalToCommit: max: 0.662, avg: 0.653"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.01s no progress at version 2560066 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.54s no progress at version 2560064 (avg 8.54s) [limit 15].
Test Ok

Copy link
Contributor

github-actions bot commented Nov 4, 2024

✅ Forge suite framework_upgrade success on 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920

Compatibility test results for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920 (PR)
Upgrade the nodes to version: 29331886905443bca0af248a5e9fafd495127920
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1261.79 txn/s, submitted: 1263.78 txn/s, failed submission: 2.00 txn/s, expired: 2.00 txn/s, latency: 2437.12 ms, (p50: 2400 ms, p70: 2600, p90: 3600 ms, p99: 5500 ms), latency samples: 113700
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1265.14 txn/s, submitted: 1268.26 txn/s, failed submission: 3.11 txn/s, expired: 3.11 txn/s, latency: 2396.97 ms, (p50: 2100 ms, p70: 2500, p90: 4100 ms, p99: 5900 ms), latency samples: 113760
5. check swarm health
Compatibility test for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920 passed
Upgrade the remaining nodes to version: 29331886905443bca0af248a5e9fafd495127920
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 990.73 txn/s, submitted: 993.83 txn/s, failed submission: 3.10 txn/s, expired: 3.10 txn/s, latency: 3005.71 ms, (p50: 2400 ms, p70: 2800, p90: 5400 ms, p99: 11800 ms), latency samples: 89480
Test Ok

Copy link
Contributor

github-actions bot commented Nov 4, 2024

✅ Forge suite compat success on 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920

Compatibility test results for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920 (PR)
1. Check liveness of validators at old version: 1086a5e00d773704731ab84fb4ed3538613b2250
compatibility::simple-validator-upgrade::liveness-check : committed: 13990.25 txn/s, latency: 2381.71 ms, (p50: 2000 ms, p70: 2200, p90: 2500 ms, p99: 7700 ms), latency samples: 485920
2. Upgrading first Validator to new version: 29331886905443bca0af248a5e9fafd495127920
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 5947.74 txn/s, latency: 4816.37 ms, (p50: 5700 ms, p70: 5900, p90: 6000 ms, p99: 6100 ms), latency samples: 111560
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5575.78 txn/s, latency: 5867.72 ms, (p50: 6400 ms, p70: 6800, p90: 6900 ms, p99: 7000 ms), latency samples: 196880
3. Upgrading rest of first batch to new version: 29331886905443bca0af248a5e9fafd495127920
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6528.55 txn/s, latency: 4341.74 ms, (p50: 5100 ms, p70: 5200, p90: 5300 ms, p99: 5400 ms), latency samples: 119080
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6029.77 txn/s, latency: 5288.26 ms, (p50: 5400 ms, p70: 5500, p90: 7100 ms, p99: 7500 ms), latency samples: 202860
4. upgrading second batch to new version: 29331886905443bca0af248a5e9fafd495127920
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 8495.77 txn/s, latency: 3340.52 ms, (p50: 3300 ms, p70: 3600, p90: 4900 ms, p99: 5300 ms), latency samples: 152340
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 7142.09 txn/s, latency: 4236.96 ms, (p50: 3800 ms, p70: 4900, p90: 5600 ms, p99: 8000 ms), latency samples: 268680
5. check swarm health
Compatibility test for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> 29331886905443bca0af248a5e9fafd495127920 passed
Test Ok

@rahxephon89 rahxephon89 merged commit f345832 into main Nov 4, 2024
86 of 92 checks passed
@rahxephon89 rahxephon89 deleted the teng/fix-15135 branch November 4, 2024 21:38
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.

[Bug][move-prover] warning about aggregator.limit from prover tests on framework code
3 participants