[aggregator] Remove aggregator API gating #13316

georgemitenkov · 2024-05-17T11:06:22Z

Description

This PR removes the usage of aggregator API feature flag (it can be therefore re-used in the future) as #13247 changes the Move code to not rely on it either. API has been fully rolled out as well.

The changes are tested by replay (latest):
https://github.com/aptos-labs/aptos-core/actions/runs/9300355540 with few jobs failing because of timeouts.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

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-05-17T11:06:26Z

⏱️ 424h 4m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
replay-testnet / replay-verify (16)	64h 3m	⬜ 🟥 🟥 🟥 🟥 (+6 more)
replay-testnet / replay-verify (18)	30h 17m	⬜ 🟥 🟥 🟩 🟩 (+1 more)
replay-mainnet / replay-verify (16)	29h 21m	⬜ 🟩 🟩 ⬜ 🟩 (+1 more)
replay-mainnet / replay-verify (15)	28h 21m	⬜ 🟩 🟩 ⬜ 🟩 (+1 more)
replay-mainnet / replay-verify (18)	19h 38m	⬜ ⬜ 🟩 🟩 🟩
replay-mainnet / replay-verify (12)	18h 12m	⬜ ⬜ 🟩 🟩 🟩
replay-mainnet / replay-verify (17)	17h 30m	⬜ ⬜ 🟩 🟩 🟩
replay-mainnet / replay-verify (11)	16h 6m	🟩 ⬜ 🟩 🟩 🟩
replay-testnet / replay-verify (9)	13h 11m	🟩 ⬜ 🟩 🟩 🟩
replay-mainnet / replay-verify (10)	11h 39m	🟩 ⬜ 🟩 🟩 🟩
replay-testnet / replay-verify (10)	11h 24m	🟩 ⬜ 🟩 🟩 🟩
replay-mainnet / replay-verify (14)	10h 9m	🟥 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (8)	9h 59m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (3)	8h 51m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (15)	8h 33m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (11)	7h 51m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (9)	6h 45m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (6)	6h 36m	🟥 🟥 🟩 🟩 🟩
replay-testnet / replay-verify (0)	6h 31m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (6)	6h 22m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (12)	6h 15m	🟥 🟥 🟩 🟩 🟩
replay-testnet / replay-verify (13)	5h 46m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (13)	5h 41m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (2)	5h 35m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (17)	5h 26m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (8)	5h 16m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (1)	4h 57m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (4)	4h 46m	🟥 🟥 🟩 🟩 🟩
replay-testnet / replay-verify (14)	4h 45m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (7)	4h 18m	🟩 🟩 🟩 🟥 🟩
replay-testnet / replay-verify (5)	4h 9m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (7)	4h 5m	🟥 🟥 🟩 🟩 🟩
replay-mainnet / replay-verify (4)	4h 2m	🟩 🟩 🟩 🟩 🟩
replay-testnet / replay-verify (3)	3h 52m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (5)	3h 48m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (2)	3h 4m	🟩 🟩 🟩 🟩 🟩
replay-mainnet / replay-verify (0)	2h 58m	🟩 🟩 🟩 🟩 🟩
rust-targeted-unit-tests	2h 21m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-smoke-tests	1h 46m	🟩 🟩 🟩
replay-mainnet / replay-verify (1)	1h 44m	🟩 🟩 🟩 🟩 🟩
rust-move-unit-coverage	1h 26m	🟩 🟩 🟩 🟩 🟩
rust-move-tests	1h 11m	🟩 🟩 🟩 🟩 🟩 (+1 more)
execution-performance / single-node-performance	59m	🟩 🟥 🟥
rust-images / rust-all	38m	🟩 🟩 🟩
rust-lints	38m	🟩 🟩 🟩 🟩 🟩 (+1 more)
forge-compat-test / forge	36m	🟩 🟩 🟩
forge-e2e-test / forge	29m	🟩 🟥 🟥
cli-e2e-tests / run-cli-tests	26m	🟩 🟥 🟥
run-tests-main-branch	26m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-build-cached-packages	16m	🟩 🟩 🟩
check	12m	🟩 🟩 🟩
general-lints	11m	🟩 🟩 🟩 🟩 🟩 (+1 more)
execution-performance / test-target-determinator	11m	🟩 🟩 🟩
check-dynamic-deps	10m	🟩 🟩 🟩 🟩 🟩 (+1 more)
test-target-determinator	10m	🟩 🟩 🟩
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose	5m	🟩 🟩 🟩
node-api-compatibility-tests / node-api-compatibility-tests	2m	🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
determine-test-metadata	37s	🟩 🟩 🟩 🟩 🟩
file_change_determinator	35s	🟩 🟩 🟩
permission-check	21s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	17s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	17s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	16s	🟩 🟩 🟩
permission-check	14s	🟩 🟩 🟩 🟩 🟩 (+1 more)
determine-docker-build-metadata	5s	🟩 🟩 🟩

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

Job	Duration	vs 7d avg	Delta
cli-e2e-tests / run-cli-tests	10m	7m
rust-move-unit-coverage	13m	18m

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

codecov · 2024-05-17T11:26:44Z

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 58.4%. Comparing base (2e0d258) to head (8b68669).
Report is 3 commits behind head on main.

Files	Patch %	Lines
...rk/src/natives/aggregator_natives/aggregator_v2.rs	50.0%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #13316       +/-   ##
===========================================
- Coverage    71.3%    58.4%    -12.9%     
===========================================
  Files        2314      821     -1493     
  Lines      454889   197265   -257624     
===========================================
- Hits       324682   115373   -209309     
+ Misses     130207    81892    -48315

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

igor-aptos · 2024-05-17T14:06:49Z

You need to check replay -verify, whether they're was any transaction submitted that failed on this check. Depending on the volume, we either need to keep this, or list them for exclusion

georgemitenkov · 2024-05-17T14:15:01Z

@igor-aptos yes, already running the replay :)

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

georgemitenkov · 2024-05-30T09:10:32Z

@igor-aptos I think we should be fine: https://github.com/aptos-labs/aptos-core/actions/runs/9194823097, though one job times out and rerunning doesn't help here...

github-actions · 2024-05-30T10:00:15Z

✅ Forge suite `compat` success on `3ffe0986b5fe4acb76544ae7ae85d73b91a6a411` ==> `8b6866925a792480cf9ad09f49105262e8d20ca1`

Compatibility test results for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> 8b6866925a792480cf9ad09f49105262e8d20ca1 (PR)
1. Check liveness of validators at old version: 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411
compatibility::simple-validator-upgrade::liveness-check : committed: 6786.008384686194 txn/s, latency: 4846.271503862246 ms, (p50: 4800 ms, p90: 8100 ms, p99: 8700 ms), latency samples: 248560
2. Upgrading first Validator to new version: 8b6866925a792480cf9ad09f49105262e8d20ca1
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 2523.269262523528 txn/s, latency: 12288.632357970739 ms, (p50: 13600 ms, p90: 16500 ms, p99: 17400 ms), latency samples: 105260
3. Upgrading rest of first batch to new version: 8b6866925a792480cf9ad09f49105262e8d20ca1
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3416.0156629424914 txn/s, latency: 9144.328766233766 ms, (p50: 9400 ms, p90: 13800 ms, p99: 14200 ms), latency samples: 138600
4. upgrading second batch to new version: 8b6866925a792480cf9ad09f49105262e8d20ca1
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6439.555906325881 txn/s, latency: 5115.044622080496 ms, (p50: 4800 ms, p90: 8100 ms, p99: 9300 ms), latency samples: 232060
5. check swarm health
Compatibility test for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> 8b6866925a792480cf9ad09f49105262e8d20ca1 passed
Test Ok

github-actions · 2024-05-30T10:02:39Z

✅ Forge suite `realistic_env_max_load` success on `8b6866925a792480cf9ad09f49105262e8d20ca1`

two traffics test: inner traffic : committed: 8256.7356362637 txn/s, latency: 4739.746642064768 ms, (p50: 4500 ms, p90: 6000 ms, p99: 10500 ms), latency samples: 3571540
two traffics test : committed: 99.99075582424882 txn/s, latency: 1833.030459770115 ms, (p50: 1800 ms, p90: 2100 ms, p99: 3800 ms), latency samples: 1740
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.210, avg: 0.203", "QsPosToProposal: max: 0.243, avg: 0.220", "ConsensusProposalToOrdered: max: 0.447, avg: 0.407", "ConsensusOrderedToCommit: max: 0.364, avg: 0.350", "ConsensusProposalToCommit: max: 0.769, avg: 0.757"]
Max round gap was 1 [limit 4] at version 1765208. Max no progress secs was 4.838858 [limit 15] at version 1765208.
Test Ok

georgemitenkov · 2024-06-03T21:31:16Z

cc @igor-aptos

remove aggregator_api checks and old gas feature gating

cb91b8a

georgemitenkov requested review from gelash, zekun000, davidiw, wrwg and vgao1996 as code owners May 17, 2024 11:06

georgemitenkov requested a review from igor-aptos May 17, 2024 11:06

rollback consistency gating

057fae7

vgao1996 reviewed May 17, 2024

View reviewed changes

aptos-move/aptos-vm/src/aptos_vm.rs Outdated Show resolved Hide resolved

rollback dep gating

885a7d3

georgemitenkov changed the title ~~[gas][aggregator] Remove aggregator API and old gas feature gatings~~ [aggregator] Remove aggregator API gating May 17, 2024

georgemitenkov requested review from vusirikala and removed request for davidiw, wrwg and zekun000 May 18, 2024 14:25

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 May 18, 2024