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

[gas] permission #15232

Merged
merged 3 commits into from
Jan 17, 2025
Merged

[gas] permission #15232

merged 3 commits into from
Jan 17, 2025

Conversation

lightmark
Copy link
Contributor

Description

add Gas permission

Copy link

trunk-io bot commented Nov 7, 2024

⏱️ 27m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-move-unit-coverage 12m 🟩
rust-move-tests 10m 🟥
rust-cargo-deny 2m 🟩
check-dynamic-deps 2m 🟩
general-lints 29s 🟩
semgrep/ci 22s 🟩
file_change_determinator 9s 🟩
permission-check 7s 🟩
permission-check 2s 🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.1%. Comparing base (e4239a8) to head (adeff45).

Additional details and impacted files
@@                       Coverage Diff                        @@
##           09-11-permission_for_framework   #15232    +/-   ##
================================================================
  Coverage                            60.0%    60.1%            
================================================================
  Files                                 857      857            
  Lines                              210762   210762            
================================================================
+ Hits                               126586   126726   +140     
+ Misses                              84176    84036   -140     

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

@runtian-zhou runtian-zhou force-pushed the 09-11-permission_for_framework branch from e4239a8 to d92055d Compare November 15, 2024 20:31
@runtian-zhou runtian-zhou force-pushed the 09-11-permission_for_framework branch 14 times, most recently from bef645e to 74c6842 Compare November 25, 2024 20:05
@runtian-zhou runtian-zhou force-pushed the 09-11-permission_for_framework branch 8 times, most recently from 2d97cf3 to afb055a Compare November 26, 2024 22:09
@igor-aptos igor-aptos force-pushed the lightmark/gas_permission branch from 7fee306 to 4def2bd Compare January 17, 2025 16:46
@igor-aptos igor-aptos changed the base branch from token_bucket to main January 17, 2025 16:46
@igor-aptos igor-aptos enabled auto-merge (squash) January 17, 2025 16:47

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@runtian-zhou runtian-zhou left a comment

Choose a reason for hiding this comment

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

Who will pass the gas_payer sender to the prologue? Or in other words, when can the gas_payer be the permissioned signer?

@runtian-zhou
Copy link
Contributor

Just double checked the code. The fee payer can be a permissioned signer when running an AA in aptos_vm. Looks good to me now.

@igor-aptos
Copy link
Contributor

yes, and if transient signer is used - it's a master signer here, so permission is not used - the max_gas from transaction that user explicitly signed serves that purpose

@igor-aptos igor-aptos force-pushed the lightmark/gas_permission branch from 4def2bd to c9d9310 Compare January 17, 2025 19:22

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@igor-aptos igor-aptos force-pushed the lightmark/gas_permission branch from c9d9310 to f5b8dc6 Compare January 17, 2025 20:36

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9

Compatibility test results for 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9 (PR)
1. Check liveness of validators at old version: 17540fad8e88ab5681f3a91190b9f5d37e53d2ef
compatibility::simple-validator-upgrade::liveness-check : committed: 14342.18 txn/s, latency: 2186.39 ms, (p50: 2100 ms, p70: 2300, p90: 2900 ms, p99: 4200 ms), latency samples: 472560
2. Upgrading first Validator to new version: f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 3974.73 txn/s, latency: 7720.69 ms, (p50: 8100 ms, p70: 9000, p90: 9200 ms, p99: 9200 ms), latency samples: 88960
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3928.05 txn/s, latency: 8675.16 ms, (p50: 9600 ms, p70: 9700, p90: 10100 ms, p99: 10200 ms), latency samples: 139520
3. Upgrading rest of first batch to new version: f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 4360.78 txn/s, latency: 7129.20 ms, (p50: 7800 ms, p70: 8500, p90: 9000 ms, p99: 9100 ms), latency samples: 91960
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4408.08 txn/s, latency: 7706.56 ms, (p50: 8700 ms, p70: 8800, p90: 9000 ms, p99: 9200 ms), latency samples: 155500
4. upgrading second batch to new version: f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 7882.79 txn/s, latency: 3839.75 ms, (p50: 4400 ms, p70: 4600, p90: 4900 ms, p99: 5100 ms), latency samples: 147600
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8167.80 txn/s, latency: 4170.42 ms, (p50: 4500 ms, p70: 4700, p90: 4900 ms, p99: 5200 ms), latency samples: 271940
5. check swarm health
Compatibility test for 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9 passed
Test Ok

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on f5b8dc613065c4a14ca3d5cc8ae4f880ecd53dc9

two traffics test: inner traffic : committed: 14159.71 txn/s, submitted: 14190.01 txn/s, expired: 30.30 txn/s, latency: 2681.12 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3600 ms), latency samples: 5383840
two traffics test : committed: 99.97 txn/s, latency: 1502.76 ms, (p50: 1500 ms, p70: 1500, p90: 1700 ms, p99: 3400 ms), latency samples: 1900
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.538, avg: 1.438", "ConsensusProposalToOrdered: max: 0.304, avg: 0.301", "ConsensusOrderedToCommit: max: 0.425, avg: 0.411", "ConsensusProposalToCommit: max: 0.727, avg: 0.712"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.55s no progress at version 67267 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.72s no progress at version 1624862 (avg 0.71s) [limit 16].
Test Ok

@igor-aptos igor-aptos merged commit bb609ba into main Jan 17, 2025
46 checks passed
@igor-aptos igor-aptos deleted the lightmark/gas_permission branch January 17, 2025 22:08
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.

3 participants