[aptosvm] Simplify VM flows #11888

georgemitenkov · 2024-02-03T10:15:12Z

Description

This PR simplifies flows inside AptosVM. In particular:

Duplicated logic for creating the gas meter for view functions has been removed.
Duplicated logic for calculating gas used for view functions has been removed.
There was unreachable code in failure transaction cleanup, where the discarded
status has been returned immediately, but then rechecked again. The first check
is shifted inside.
No more default transaction metadata. It has never really been used, or needed.
Scripts are now validated consistently. Previously, we were checking there are no
events in scripts for regular users, but when executing a waypoint script this check
was not performed.
Simplifies transaction execution function signature to avoid Option<String> which
was previously used to specify the sender. The rationale is that the caller knows the
sender, and so there is no need to complicate the signature (what is the purpose?).
Removes duplicated features from AptosVM and keeps them in MoveVMExt. Also,
session does not need features because a bool flag suffices.
[NEW] Fixed a bug when script hash was not computed for RunOnAbort.

Deleted unreachable tests for failure epilogue.
Moved test for non existing account to move-e2e-tests.

Test Plan

replay-verify: https://github.com/aptos-labs/aptos-core/actions/runs/7766224493

trunk-io · 2024-02-03T10:15:16Z

⏱️ 89h 27m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-unit-coverage	12h	🟥
replay-mainnet / replay-verify (16)	5h 18m	🟩
replay-mainnet / replay-verify (13)	4h 34m	🟩
replay-testnet / replay-verify (16)	4h 3m	🟩
replay-mainnet / replay-verify (12)	3h 11m	🟩
rust-move-unit-coverage	3h 7m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-smoke-coverage	2h 58m	🟩
rust-unit-tests	2h 45m	🟩 🟩 🟩 🟩 🟩 (+1 more)
replay-testnet / replay-verify (9)	2h 42m	🟩
rust-smoke-tests	2h 38m	🟩 🟩 🟩 🟩 🟩 (+1 more)
replay-mainnet / replay-verify (11)	2h 19m	🟩
replay-testnet / replay-verify (10)	2h 9m	🟩
execution-performance / single-node-performance	2h 5m	🟩 🟩 🟩 🟩 🟩 (+2 more)
windows-build	2h 1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
replay-mainnet / replay-verify (15)	1h 53m	🟩
replay-testnet / replay-verify (8)	1h 53m	🟩
replay-testnet / replay-verify (11)	1h 44m	🟩
replay-mainnet / replay-verify (4)	1h 38m	🟩
rust-move-tests	1h 37m	🟩 🟩 🟩 🟩 🟩 (+1 more)
replay-testnet / replay-verify (15)	1h 29m	🟩
rust-images / rust-all	1h 25m	🟩 🟩 🟩 🟩 🟩 (+1 more)
forge-e2e-test / forge	1h 21m	🟩 🟩 🟩 🟩 🟩 (+1 more)
forge-compat-test / forge	1h 16m	🟩 🟩 🟩 🟩 🟩 (+1 more)
replay-mainnet / replay-verify (7)	1h 13m	🟩
replay-testnet / replay-verify (0)	1h 12m	🟩
replay-testnet / replay-verify (6)	1h 11m	🟩
replay-mainnet / replay-verify (10)	1h 10m	🟩
replay-testnet / replay-verify (2)	1h 9m	🟩
replay-testnet / replay-verify (12)	1h 8m	🟩
replay-mainnet / replay-verify (14)	1h 3m	🟩
replay-mainnet / replay-verify (8)	1h 1m	🟩
replay-mainnet / replay-verify (9)	1h	🟩
replay-testnet / replay-verify (13)	1h	🟩
replay-testnet / replay-verify (1)	57m	🟩
replay-testnet / replay-verify (4)	56m	🟩
replay-mainnet / replay-verify (0)	53m	🟩
replay-testnet / replay-verify (14)	53m	🟩
replay-testnet / replay-verify (5)	49m	🟩
replay-testnet / replay-verify (3)	45m	🟩
replay-mainnet / replay-verify (5)	45m	🟩
replay-testnet / replay-verify (7)	45m	🟩
replay-mainnet / replay-verify (6)	42m	🟩
cli-e2e-tests / run-cli-tests	40m	🟩 🟩 🟩 🟩 🟩 (+1 more)
run-tests-main-branch	40m	🟩 🟥 🟥 🟥 🟥 (+1 more)
rust-lints	40m	🟩 🟥 🟩 🟩 🟩 (+1 more)
replay-mainnet / replay-verify (1)	36m	🟩
replay-mainnet / replay-verify (3)	36m	🟩
check	24m	🟩 🟩 🟩 🟩 🟩 (+1 more)
replay-mainnet / replay-verify (2)	19m	🟩
check-dynamic-deps	17m	🟩 🟩 🟩 🟩 🟩 (+2 more)
general-lints	15m	🟩 🟩 🟩 🟩 🟩 (+1 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose	10m	🟩 🟩 🟥 🟥 🟩 (+1 more)
node-api-compatibility-tests / node-api-compatibility-tests	5m	🟩 🟩 🟩 🟩 🟩 (+1 more)
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+2 more)
execution-performance / file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	56s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	23s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	21s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	19s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)
determine-docker-build-metadata	17s	🟩 🟩 🟩 🟩 🟩 (+1 more)
determine-test-metadata	6s	🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
run-tests-main-branch	9m	4m
cli-e2e-tests / run-cli-tests	6m	8m

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

codecov · 2024-02-03T11:09:20Z

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (8575537) 69.9% compared to head (2421a1e) 71.4%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
aptos-move/aptos-vm/src/aptos_vm.rs	85.3%	21 Missing ⚠️
aptos-move/aptos-vm/src/zkid_validation.rs	30.0%	7 Missing ⚠️
...tos-move/aptos-vm/src/block_executor/vm_wrapper.rs	33.3%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #11888       +/-   ##
===========================================
+ Coverage    69.9%    71.4%     +1.4%     
===========================================
  Files        2222      809     -1413     
  Lines      419683   184551   -235132     
===========================================
- Hits       293633   131824   -161809     
+ Misses     126050    52727    -73323

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

aptos-move/aptos-vm/src/aptos_vm.rs

github-actions · 2024-02-20T00:55:53Z

✅ Forge suite `realistic_env_max_load` success on `2421a1e40ef3a63016bf3ff91312c5904a9e66fd`

two traffics test: inner traffic : committed: 7723 txn/s, latency: 4968 ms, (p50: 4500 ms, p90: 6000 ms, p99: 12600 ms), latency samples: 3336600
two traffics test : committed: 100 txn/s, latency: 2013 ms, (p50: 2000 ms, p90: 2200 ms, p99: 2500 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.235, avg: 0.205", "QsPosToProposal: max: 0.160, avg: 0.141", "ConsensusProposalToOrdered: max: 0.589, avg: 0.544", "ConsensusOrderedToCommit: max: 0.496, avg: 0.442", "ConsensusProposalToCommit: max: 1.060, avg: 0.986"]
Max round gap was 1 [limit 4] at version 1410661. Max no progress secs was 8.249534 [limit 15] at version 1410661.
Test Ok

github-actions · 2024-02-20T00:56:00Z

✅ Forge suite `compat` success on `aptos-node-v1.9.5` ==> `2421a1e40ef3a63016bf3ff91312c5904a9e66fd`

Compatibility test results for aptos-node-v1.9.5 ==> 2421a1e40ef3a63016bf3ff91312c5904a9e66fd (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 6772 txn/s, latency: 4905 ms, (p50: 4800 ms, p90: 8000 ms, p99: 9200 ms), latency samples: 237020
2. Upgrading first Validator to new version: 2421a1e40ef3a63016bf3ff91312c5904a9e66fd
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 675 txn/s, submitted: 688 txn/s, expired: 12 txn/s, latency: 35779 ms, (p50: 37400 ms, p90: 57400 ms, p99: 59600 ms), latency samples: 58129
3. Upgrading rest of first batch to new version: 2421a1e40ef3a63016bf3ff91312c5904a9e66fd
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 211 txn/s, submitted: 521 txn/s, expired: 309 txn/s, latency: 37146 ms, (p50: 39800 ms, p90: 51100 ms, p99: 69900 ms), latency samples: 16702
4. upgrading second batch to new version: 2421a1e40ef3a63016bf3ff91312c5904a9e66fd
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2304 txn/s, latency: 12081 ms, (p50: 13400 ms, p90: 14800 ms, p99: 19900 ms), latency samples: 112900
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> 2421a1e40ef3a63016bf3ff91312c5904a9e66fd passed
Test Ok

* Fix `iss`-related bug in Groth16 path & refactor (#12017) Co-authored-by: Oliver <[email protected]> * [aptosvm] Simplify VM flows (#11888) * Duplicated logic for creating the gas meter for view functions has been removed. * Duplicated logic for calculating gas used for view functions has been removed. * There was unreachable code in failure transaction cleanup, where the discarded status has been returned immediately, but then re-checked again. The first check is shifted inside. * No more default transaction metadata. * Scripts are now validated consistently. * Simplifies transaction execution function signature to avoid `Option<String>`. * Removes duplicated features from `AptosVM` and keeps them in `MoveVMExt`. * Fixes a bug when script hash was not computed for `RunOnAbort`. Related tests are moved to `move-e2e-tests`. * [Compiler V2] Critical edge elimination (#11894) Implement a pass to eliminate critical edges by splitting them with empty blocks * [consensus configs] reduce sending block size from 2500 to 1900 (#12091) ### Description The block output limit is no longer hit with p2p txns. ### Test Plan Forge `realistic_env_max_load` TPS improves. * [Indexer-grpc] Add profiling support. (#12034) * Minor aggregator cleanup (#12013) * Minor aggregator cleanup * Addressing PR comments * [move] rotate_authentication_key_call should not modify OriginatingAddress (#12108) Co-authored-by: Alin Tomescu <[email protected]> * [Data Streaming Service] Add dynamic prefetching support * [Data Streaming Service] Add dynamic prefetching unit tests. * [Data Streaming Service] Update existing integration tests. * [State Sync] Add backpressure to fast sync receiver. * Update perf baseline for gas charging coverage improvements (reducing throughput) (#12124) * Reduce latency of cloning network sender using Arc pointers (#12103) * Avoid cloning network sender using Arc pointers * Removing a clone * 100 node sweep test * Removing a few clone operations * reset forge test * Removing some clones * Removing clones * adopt AIP-61 terminology for consistency (#12123) adopt AIP-61 terminology for consistency * [Consensus] Remove non-decoupled execution and refactor for cleaner interfaces (#12104) * fix jwk key logging (#12090) * remove spurious error lines (#12137) * randomness #1: types update from randomnet (#12106) * types update from randomnet * update * lint * lint * All validators broadcast commit vote messages (#12059) * All validators broadcast commit messages * Forge testing * Increase timeout for forge * test forge realistic_env_workload_sweep_test * run realistic_env_workload_sweep_test * run realistic_env_workload_sweep_test * run sweep test * increase forge runner duration * forge testing * Letting the proposer also broadcast commit decision for backward compatibility * removing forge changes * Added a TODO * [vm] Resource access control: runtime engine (#10544) * [vm] Resource access control: runtime engine Implements the runtime engine for resource access control: - a representation of access control specifiers in `loaded_data::runtime_access_specifiers`. - a loader for access specifiers in `runtime::loader::access_specifier_loader`. - a new stateful object representing the access control logic in `runtime::access_control`. - finally the use of the `AccessControlState` in `runtime::interpreter`. * Addressing reviewer comments. * Addressing reviewer comments. * typo: PTLA -> PTAL * Rebasing: adjusting to upstream changes * Rebasing * ObjectCodeDeployment API cleanup update (#12133) * ObjectCodeDeployment API cleanup update (#12141) * [Compiler-v2] porting more V1 unit tests to V2 (#12085) * update tests * fix bug * fix-12116 * fix missing space * add expected got * remove live-var tests * fix had_erros * fix * Enable the max object nesting check (#12129) * Resolved the warning for unused variable (#12157) * update * update * update * update * update * update * update * update * update * update * update * update * update * Squashed commit of the following: commit a50ffec Author: Zhoujun Ma <[email protected]> Date: Thu Feb 22 21:10:12 2024 +0000 lint commit 388350f Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 13:04:28 2024 -0800 update commit 76f7eca Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:56:04 2024 -0800 update commit a663542 Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:54:18 2024 -0800 update commit b439449 Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:34:14 2024 -0800 update commit 3378ceb Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:17:06 2024 -0800 update commit 6cd6685 Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:15:05 2024 -0800 update commit 6d89f37 Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:13:51 2024 -0800 update commit 980f257 Author: zhoujun.ma <[email protected]> Date: Thu Feb 22 12:12:04 2024 -0800 update commit 16e9349 Author: Zhoujun Ma <[email protected]> Date: Thu Feb 22 18:25:08 2024 +0000 lint --------- Co-authored-by: Alin Tomescu <[email protected]> Co-authored-by: Oliver <[email protected]> Co-authored-by: George Mitenkov <[email protected]> Co-authored-by: Zekun Wang <[email protected]> Co-authored-by: Brian (Sunghoon) Cho <[email protected]> Co-authored-by: Guoteng Rao <[email protected]> Co-authored-by: Satya Vusirikala <[email protected]> Co-authored-by: David Wolinsky <[email protected]> Co-authored-by: Josh Lind <[email protected]> Co-authored-by: igor-aptos <[email protected]> Co-authored-by: Sital Kedia <[email protected]> Co-authored-by: Wolfgang Grieskamp <[email protected]> Co-authored-by: Teng Zhang <[email protected]> Co-authored-by: Junkil Park <[email protected]>

georgemitenkov added 6 commits February 1, 2024 03:04

get rid of useless optional sender string

c8a9d0e

remove duplicated features, fix types

d692143

cleanup unreachable discards

1bfffd6

cleanup view function gas metering

1345a3c

unify gas used calculation & remove TxnMeta::default()

5a6682c

consolidate script validation

b55bd1b

georgemitenkov added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Feb 3, 2024

georgemitenkov requested review from davidiw, wrwg, zekun000 and vgao1996 as code owners February 3, 2024 10:15

georgemitenkov requested review from runtian-zhou and gelash February 3, 2024 10:15