aptos-core icon indicating copy to clipboard operation
aptos-core copied to clipboard

[WIP][aptos-vm] implement new gas metering system

Open vgao1996 opened this issue 3 years ago • 1 comments

Note: still a WIP

This implements a proper gas metering system for the Aptos blockchain. More specifically, this

  1. Defines a new extensible on-chain gas schedule
  2. Introduces an aptos-gas crate a. Implements a new gas meter and defines the formulae the instruction costs b. Defines a bi-directional mapping between the on-chain gas schedule and the rust representation c. Defines an initial gas schedule (the values are currently just placeholders)
  3. Made framework natives customizable in terms of gas.
  4. Moved code::from_bytes into a new module util so it can be shared within the aptos framework
  5. Moved the native functions transaction_context::get_script_hash and code::request_publish into framework for better organization a. This PR also fixes code::request_publish so that it properly returns NativeResult::err(...) when given invalid user input, instead of raising an internal error.

TODOs:

  1. Documentation
  2. Code formatting
  3. Tests
  4. Lints
  5. Other cleanups

This change is Reviewable

vgao1996 avatar Jul 29 '22 22:07 vgao1996

Any reason why this is still draft? Some folks may not look until you remove the draft status.

wrwg avatar Jul 31 '22 05:07 wrwg

I'm suggesting that we land this now but then follow up on individual gas formulas with the folks responsible for the particular design area. The general design was discussed and approved by the Move maintainers already.

Can you give me a day to review as well? Will look through by end of Monday at the latest.

movekevin avatar Jul 31 '22 14:07 movekevin

@movekevin please take your time! It'll take me some time to deal with test failures and resolve conflicts anyways. @wrwg these are also the main reason I'm still marking this as a draft. You can find a full list of TODOs in the summary.

vgao1996 avatar Aug 01 '22 16:08 vgao1996

@movekevin let me know if you have any concerns. Otherwise I'll try to land it soon.

vgao1996 avatar Aug 02 '22 23:08 vgao1996

@movekevin let me know if you have any concerns. Otherwise I'll try to land it soon.

Looking at it. I can finish the review in ~2 hours (1 hour after this current SEV review)

movekevin avatar Aug 02 '22 23:08 movekevin

@movekevin I made some of the changes you requested to gas_schedule.move. Let me know if those changes look good to you and if I have answered all of your questions!

Again, thanks for the detailed review!

vgao1996 avatar Aug 03 '22 00:08 vgao1996

:white_check_mark: Forge test success on 47966eef0429f326ec80eb5fa87684936dd4cca0

all up : 7484 TPS, 3899 ms latency, 6000 ms p99 latency,no expired txns

github-actions[bot] avatar Aug 04 '22 00:08 github-actions[bot]