[docs] Update AptosVM readme #15251

georgemitenkov · 2024-11-12T07:58:37Z

Description

Fixes #14606. We had some really ugly/old docs for AptosVM, quickly updated.

How Has This Been Tested?

Key Areas to Review

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-11-12T07:58:41Z

⏱️ 34m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
rust-doc-tests	5m	🟩
test-target-determinator	5m	🟩
check-dynamic-deps	5m	🟩 🟩 🟩 🟩
execution-performance / test-target-determinator	4m	🟩
check	4m	🟩
rust-cargo-deny	2m	🟥 🟥 ⬜ 🟩
rust-move-tests	2m	🟩
rust-move-tests	2m	🟩
fetch-last-released-docker-image-tag	2m	🟩
rust-move-tests	1m	⬜
semgrep/ci	1m	🟩 🟩 🟩 🟩
general-lints	45s	🟩 🟩 ⬜ 🟩
file_change_determinator	45s	🟩 🟩 🟩 🟩
permission-check	12s	🟩 🟩 🟩 🟩
file_change_determinator	11s	🟩

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

georgemitenkov · 2024-11-12T07:58:53Z

[docs] Update AptosVM readme #15251 👈
main

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

Join @georgemitenkov and the rest of your teammates on Graphite

graphite-app · 2024-11-12T07:59:10Z

aptos-move/aptos-vm/README.md

+### AptosSimulationVM
+
+Used to simulate transactions before executing them on a real network.
+Implementation-wise, wraps AptosVM with some minor modifications for multi-signature transactions,


Replace the trailing comma with a period to maintain consistent sentence punctuation throughout the document.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

vineethk

Minor comments to look at.

vineethk · 2024-11-12T17:14:42Z

aptos-move/aptos-vm/README.md

+For every user transaction, the following steps are performed:
+
+1. ***Prologue:***
+   Responsible for checking the structure of the transaction such as signature verification, balance checks on the account paying gas for this transaction, etc.


In the previous version of this document, it is mentioned that bytecode verifier runs in the prologue in verifier mode.

In this new text, the bytecode verifier is not discussed. Perhaps add a brief discussion?

It doesn't necessarily run in prologue, so the old text is wrong (and is probably very old - there used to be no entry functions and scripts were the only way to run code iirc). Bytecode verifier runs for scripts, still, but for entry functions, and this is part of step 2. I added a few sentences explaining this.

vineethk · 2024-11-12T17:18:12Z

aptos-move/aptos-vm/README.md

+   Runs the code specified by the transaction - can be a script or an entry function.
+   Internally, the code execution is done by MoveVM which AptosVM wraps.
+   If execution is successful, produces a change set that can be (but is not yet) applied to the blockchain state.
+3. **Aborted execution (hack):**


The "hack" seems out of place. It is not clear to the reader whether the current implementation is a hack that we intend to fix in some way, or something else. Probably best to explain whatever is the intention in the prose, instead of just mentioning it here.

Removed the "hack" part.

github-actions · 2024-11-13T00:07:24Z

✅ Forge suite `realistic_env_max_load` success on `ea458f9922e610d1ad1e6f1af2fce4ea25f279eb`

two traffics test: inner traffic : committed: 14415.59 txn/s, latency: 2761.47 ms, (p50: 2600 ms, p70: 2700, p90: 3000 ms, p99: 4800 ms), latency samples: 5481160
two traffics test : committed: 100.01 txn/s, latency: 1810.63 ms, (p50: 1300 ms, p70: 1400, p90: 3300 ms, p99: 10400 ms), latency samples: 1800
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.852, avg: 1.358", "ConsensusProposalToOrdered: max: 0.329, avg: 0.294", "ConsensusOrderedToCommit: max: 0.363, avg: 0.350", "ConsensusProposalToCommit: max: 0.656, avg: 0.644"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.94s no progress at version 2276717 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.70s no progress at version 2276715 (avg 8.70s) [limit 15].
Test Ok

github-actions · 2024-11-13T00:10:05Z

✅ Forge suite `compat` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `ea458f9922e610d1ad1e6f1af2fce4ea25f279eb`

Compatibility test results for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> ea458f9922e610d1ad1e6f1af2fce4ea25f279eb (PR)
1. Check liveness of validators at old version: 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9
compatibility::simple-validator-upgrade::liveness-check : committed: 12411.15 txn/s, latency: 2539.92 ms, (p50: 1700 ms, p70: 1800, p90: 2200 ms, p99: 29300 ms), latency samples: 515140
2. Upgrading first Validator to new version: ea458f9922e610d1ad1e6f1af2fce4ea25f279eb
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7245.52 txn/s, latency: 3635.77 ms, (p50: 4000 ms, p70: 4100, p90: 4200 ms, p99: 4900 ms), latency samples: 150120
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7757.07 txn/s, latency: 4064.98 ms, (p50: 4100 ms, p70: 4200, p90: 6500 ms, p99: 6700 ms), latency samples: 259080
3. Upgrading rest of first batch to new version: ea458f9922e610d1ad1e6f1af2fce4ea25f279eb
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 4938.90 txn/s, latency: 5859.12 ms, (p50: 6400 ms, p70: 7100, p90: 7500 ms, p99: 7700 ms), latency samples: 97260
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4836.98 txn/s, latency: 6683.37 ms, (p50: 7200 ms, p70: 7300, p90: 8900 ms, p99: 9100 ms), latency samples: 160100
4. upgrading second batch to new version: ea458f9922e610d1ad1e6f1af2fce4ea25f279eb
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11681.72 txn/s, latency: 2391.41 ms, (p50: 2400 ms, p70: 2700, p90: 3200 ms, p99: 3400 ms), latency samples: 206700
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8455.98 txn/s, latency: 3708.99 ms, (p50: 2600 ms, p70: 2800, p90: 11000 ms, p99: 13400 ms), latency samples: 322140
5. check swarm health
Compatibility test for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> ea458f9922e610d1ad1e6f1af2fce4ea25f279eb passed
Test Ok

georgemitenkov marked this pull request as ready for review November 12, 2024 07:58

georgemitenkov requested review from davidiw, wrwg, zekun000 and vgao1996 as code owners November 12, 2024 07:58

graphite-app bot reviewed Nov 12, 2024

View reviewed changes

[docs] Update AptosVM readme

9681dd5

georgemitenkov force-pushed the george/aptos-vm branch from 738a286 to 9681dd5 Compare November 12, 2024 08:00

georgemitenkov mentioned this pull request Nov 12, 2024

[Bug] Outdated Document in aptos-vm/README file #14606

Closed

heliuchuan approved these changes Nov 12, 2024

View reviewed changes

vineethk approved these changes Nov 12, 2024

View reviewed changes

[comments] Addressing 2 comments

ea458f9

georgemitenkov enabled auto-merge (squash) November 12, 2024 23:39

This comment has been minimized.

Sign in to view

georgemitenkov merged commit d6cbb07 into main Nov 13, 2024
86 of 92 checks passed

georgemitenkov deleted the george/aptos-vm branch November 13, 2024 00:10

rahxephon89 pushed a commit that referenced this pull request Nov 13, 2024

[docs] Update AptosVM readme (#15251)

9fb063f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Update AptosVM readme #15251

[docs] Update AptosVM readme #15251

georgemitenkov commented Nov 12, 2024 •

edited

Loading

trunk-io bot commented Nov 12, 2024 •

edited

Loading

georgemitenkov commented Nov 12, 2024

graphite-app bot Nov 12, 2024

vineethk left a comment

vineethk Nov 12, 2024

georgemitenkov Nov 12, 2024

vineethk Nov 12, 2024

georgemitenkov Nov 12, 2024

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 13, 2024

github-actions bot commented Nov 13, 2024

[docs] Update AptosVM readme #15251

[docs] Update AptosVM readme #15251

Conversation

georgemitenkov commented Nov 12, 2024 • edited Loading

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Nov 12, 2024 • edited Loading

georgemitenkov commented Nov 12, 2024

graphite-app bot Nov 12, 2024

Choose a reason for hiding this comment

vineethk left a comment

Choose a reason for hiding this comment

vineethk Nov 12, 2024

Choose a reason for hiding this comment

georgemitenkov Nov 12, 2024

Choose a reason for hiding this comment

vineethk Nov 12, 2024

Choose a reason for hiding this comment

georgemitenkov Nov 12, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 13, 2024

✅ Forge suite realistic_env_max_load success on ea458f9922e610d1ad1e6f1af2fce4ea25f279eb

github-actions bot commented Nov 13, 2024

✅ Forge suite compat success on 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> ea458f9922e610d1ad1e6f1af2fce4ea25f279eb

georgemitenkov commented Nov 12, 2024 •

edited

Loading

trunk-io bot commented Nov 12, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `ea458f9922e610d1ad1e6f1af2fce4ea25f279eb`

✅ Forge suite `compat` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `ea458f9922e610d1ad1e6f1af2fce4ea25f279eb`