[gas] fix gas formulae for natives in aptos-framework (aptos-labs#3274)

Cargo.toml

@@ -19,6 +19,7 @@ members = [
     "aptos-move/e2e-testsuite",
     "aptos-move/framework",
     "aptos-move/framework/cached-packages",
+    "aptos-move/gas-algebra-ext",
     "aptos-move/move-deps",
     "aptos-move/move-examples",
     "aptos-move/mvhashmap",

aptos-move/aptos-gas/Cargo.toml

@@ -17,6 +17,7 @@ move-table-extension = { git = "https://github.com/move-language/move", rev = "5
 move-vm-types = { git = "https://github.com/move-language/move", rev = "5ecf4df61fb2d8afd4881cae14132b4006996476" }
 
 framework = { path = "../framework" }
+gas-algebra-ext = { path = "../gas-algebra-ext" }
 
 [features]
 testing = ["move-stdlib/testing"]

aptos-move/aptos-gas/src/algebra.rs

@@ -1,27 +1,22 @@
 // Copyright (c) Aptos
 // SPDX-License-Identifier: Apache-2.0
 
-use move_core_types::gas_algebra::{Arg, GasQuantity, InternalGasUnit, UnitDiv};
+use move_core_types::gas_algebra::{GasQuantity, InternalGasUnit, UnitDiv};
+
+pub use gas_algebra_ext::{
+    AbstractValueSize, AbstractValueSizePerArg, AbstractValueUnit, InternalGasPerAbstractValueUnit,
+};
 
 /// Unit of (external) gas.
 pub enum GasUnit {}
 
 /// Unit of gas currency. 1 Octa = 10^-8 Aptos coins.
 pub enum Octa {}
 
-/// Unit of abstract value size -- a conceptual measurement of the memory space a Move value occupies.
-pub enum AbstractValueUnit {}
-
 pub type Gas = GasQuantity<GasUnit>;
 
 pub type GasScalingFactor = GasQuantity<UnitDiv<InternalGasUnit, GasUnit>>;
 
 pub type Fee = GasQuantity<Octa>;
 
 pub type FeePerGasUnit = GasQuantity<UnitDiv<Octa, GasUnit>>;
-
-pub type AbstractValueSize = GasQuantity<AbstractValueUnit>;
-
-pub type InternalGasPerAbstractValueUnit = GasQuantity<UnitDiv<InternalGasUnit, AbstractValueUnit>>;
-
-pub type AbstractValueSizePerArg = GasQuantity<UnitDiv<AbstractValueUnit, Arg>>;

aptos-move/aptos-gas/src/aptos_framework.rs

@@ -72,9 +72,9 @@ crate::natives::define_gas_parameters_for_natives!(GasParameters, "aptos_framewo
     [.hash.sip_hash.per_byte, "hash.sip_hash.per_byte", 1],
 
     [.type_info.type_of.base, "type_info.type_of.base", 1],
-    [.type_info.type_of.per_abstract_memory_unit, "type_info.type_of.per_abstract_memory_unit", 1],
+    [.type_info.type_of.per_byte_in_str, "type_info.type_of.per_abstract_memory_unit", 1],
     [.type_info.type_name.base, "type_info.type_name.base", 1],
-    [.type_info.type_name.per_abstract_memory_unit, "type_info.type_name.per_abstract_memory_unit", 1],
+    [.type_info.type_name.per_byte_in_str, "type_info.type_name.per_abstract_memory_unit", 1],
 
     [.util.from_bytes.base, "util.from_bytes.base", 1],
     [.util.from_bytes.per_byte, "util.from_bytes.per_byte", 1],
@@ -85,7 +85,7 @@ crate::natives::define_gas_parameters_for_natives!(GasParameters, "aptos_framewo
     [.code.request_publish.per_byte, "code.request_publish.per_byte", 1],
 
     [.event.write_to_event_store.base, "event.write_to_event_store.base", 1],
-    [.event.write_to_event_store.per_abstract_memory_unit, "event.write_to_event_store.per_abstract_memory_unit", 1],
+    [.event.write_to_event_store.per_abstract_value_unit, "event.write_to_event_store.per_abstract_memory_unit", 1],
 
     [.state_storage.get_usage.base_cost, "state_storage.get_usage.base", 1],
 

aptos-move/aptos-gas/src/lib.rs

@@ -39,7 +39,7 @@ pub use gas_meter::{
     NativeGasParameters, ToOnChainGasSchedule,
 };
 pub use instr::InstructionGasParameters;
-pub use misc::{AbstractMemorySizeGasParameters, MiscGasParameters};
+pub use misc::{AbstractValueSizeGasParameters, MiscGasParameters};
 pub use move_core_types::gas_algebra::{
     Arg, Byte, GasQuantity, InternalGas, InternalGasPerArg, InternalGasPerByte, InternalGasUnit,
     NumArgs, NumBytes, UnitDiv,

aptos-move/aptos-gas/src/misc.rs

@@ -9,7 +9,7 @@ use move_core_types::{account_address::AccountAddress, gas_algebra::NumArgs};
 use move_vm_types::views::{ValueView, ValueVisitor};
 
 crate::params::define_gas_parameters!(
-    AbstractMemorySizeGasParameters,
+    AbstractValueSizeGasParameters,
     "misc.abs_val",
     [
         // abstract value size
@@ -109,12 +109,12 @@ where
 }
 
 struct AbstractValueSizeVisitor<'a> {
-    params: &'a AbstractMemorySizeGasParameters,
+    params: &'a AbstractValueSizeGasParameters,
     size: AbstractValueSize,
 }
 
 impl<'a> AbstractValueSizeVisitor<'a> {
-    fn new(params: &'a AbstractMemorySizeGasParameters) -> Self {
+    fn new(params: &'a AbstractValueSizeGasParameters) -> Self {
         Self {
             params,
             size: 0.into(),
@@ -196,7 +196,7 @@ impl<'a> ValueVisitor for AbstractValueSizeVisitor<'a> {
     }
 }
 
-impl AbstractMemorySizeGasParameters {
+impl AbstractValueSizeGasParameters {
     /// Calculates the abstract size of the given value.
     pub fn abstract_value_size(&self, val: impl ValueView) -> AbstractValueSize {
         let mut visitor = AbstractValueSizeVisitor::new(self);
@@ -216,7 +216,7 @@ impl AbstractMemorySizeGasParameters {
 /// Miscellaneous gas parameters.
 #[derive(Debug, Clone)]
 pub struct MiscGasParameters {
-    pub abs_val: AbstractMemorySizeGasParameters,
+    pub abs_val: AbstractValueSizeGasParameters,
 }
 
 impl FromOnChainGasSchedule for MiscGasParameters {
@@ -236,7 +236,7 @@ impl ToOnChainGasSchedule for MiscGasParameters {
 impl MiscGasParameters {
     pub fn zeros() -> Self {
         Self {
-            abs_val: AbstractMemorySizeGasParameters::zeros(),
+            abs_val: AbstractValueSizeGasParameters::zeros(),
         }
     }
 }

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

@@ -11,7 +11,10 @@ use crate::{
     transaction_metadata::TransactionMetadata,
 };
 use aptos_aggregator::transaction::TransactionOutputExt;
-use aptos_gas::{AptosGasParameters, FromOnChainGasSchedule, Gas, NativeGasParameters};
+use aptos_gas::{
+    AbstractValueSizeGasParameters, AptosGasParameters, FromOnChainGasSchedule, Gas,
+    NativeGasParameters,
+};
 use aptos_logger::prelude::*;
 use aptos_state_view::StateView;
 use aptos_types::transaction::AbortInfo;
@@ -59,12 +62,15 @@ impl AptosVMImpl {
         });
 
         // TODO(Gas): this doesn't look right.
-        let native_gas_params = match &gas_params {
-            Some(gas_params) => gas_params.natives.clone(),
-            None => NativeGasParameters::zeros(),
+        let (native_gas_params, abs_val_size_gas_params) = match &gas_params {
+            Some(gas_params) => (gas_params.natives.clone(), gas_params.misc.abs_val.clone()),
+            None => (
+                NativeGasParameters::zeros(),
+                AbstractValueSizeGasParameters::zeros(),
+            ),
         };
 
-        let inner = MoveVmExt::new(native_gas_params)
+        let inner = MoveVmExt::new(native_gas_params, abs_val_size_gas_params)
             .expect("should be able to create Move VM; check if there are duplicated natives");
 
         let mut vm = Self {
@@ -85,7 +91,7 @@ impl AptosVMImpl {
             AptosGasParameters::from_on_chain_gas_schedule(&gas_schedule.to_btree_map())
                 .expect("failed to get gas parameters");
 
-        let inner = MoveVmExt::new(gas_params.natives.clone())
+        let inner = MoveVmExt::new(gas_params.natives.clone(), gas_params.misc.abs_val.clone())
             .expect("should be able to create Move VM; check if there are duplicated natives");
 
         Self {

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

@@ -5,7 +5,7 @@ use crate::{
     move_vm_ext::{MoveResolverExt, SessionExt, SessionId},
     natives::aptos_natives,
 };
-use aptos_gas::NativeGasParameters;
+use aptos_gas::{AbstractValueSizeGasParameters, NativeGasParameters};
 use framework::natives::{
     aggregator_natives::NativeAggregatorContext, code::NativeCodeContext,
     cryptography::ristretto255_point::NativeRistrettoPointContext,
@@ -24,10 +24,13 @@ pub struct MoveVmExt {
 }
 
 impl MoveVmExt {
-    pub fn new(native_gas_params: NativeGasParameters) -> VMResult<Self> {
+    pub fn new(
+        native_gas_params: NativeGasParameters,
+        abs_val_size_gas_params: AbstractValueSizeGasParameters,
+    ) -> VMResult<Self> {
         Ok(Self {
             inner: MoveVM::new_with_verifier_config(
-                aptos_natives(native_gas_params),
+                aptos_natives(native_gas_params, abs_val_size_gas_params),
                 VerifierConfig {
                     max_loop_depth: Some(5),
                 },

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

@@ -1,7 +1,7 @@
 // Copyright (c) Aptos
 // SPDX-License-Identifier: Apache-2.0
 
-use aptos_gas::NativeGasParameters;
+use aptos_gas::{AbstractValueSizeGasParameters, NativeGasParameters};
 use aptos_types::account_config::CORE_CODE_ADDRESS;
 use framework::natives::cryptography::ristretto255_point::NativeRistrettoPointContext;
 use framework::natives::{
@@ -18,12 +18,16 @@ use once_cell::sync::Lazy;
 
 static DUMMY_RESOLVER: Lazy<BlankStorage> = Lazy::new(|| BlankStorage);
 
-pub fn aptos_natives(gas_params: NativeGasParameters) -> NativeFunctionTable {
+pub fn aptos_natives(
+    gas_params: NativeGasParameters,
+    abs_val_size_gas_params: AbstractValueSizeGasParameters,
+) -> NativeFunctionTable {
     move_stdlib::natives::all_natives(CORE_CODE_ADDRESS, gas_params.move_stdlib)
         .into_iter()
         .chain(framework::natives::all_natives(
             CORE_CODE_ADDRESS,
             gas_params.aptos_framework,
+            move |val| abs_val_size_gas_params.abstract_value_size(val),
         ))
         .chain(move_table_extension::table_natives(
             CORE_CODE_ADDRESS,

aptos-move/e2e-tests/src/executor.rs

@@ -19,7 +19,7 @@ use crate::{
 };
 use aptos_bitvec::BitVec;
 use aptos_crypto::HashValue;
-use aptos_gas::NativeGasParameters;
+use aptos_gas::{AbstractValueSizeGasParameters, NativeGasParameters};
 use aptos_keygen::KeyGen;
 use aptos_state_view::StateView;
 use aptos_types::{
@@ -515,7 +515,11 @@ impl FakeExecutor {
     ) {
         let write_set = {
             // TODO(Gas): we probably want to switch to non-zero costs in the future
-            let vm = MoveVmExt::new(NativeGasParameters::zeros()).unwrap();
+            let vm = MoveVmExt::new(
+                NativeGasParameters::zeros(),
+                AbstractValueSizeGasParameters::zeros(),
+            )
+            .unwrap();
             let remote_view = RemoteStorage::new(&self.data_store);
             let mut session = vm.new_session(&remote_view, SessionId::void());
             session
@@ -554,7 +558,11 @@ impl FakeExecutor {
         args: Vec<Vec<u8>>,
     ) -> Result<WriteSet, VMStatus> {
         // TODO(Gas): we probably want to switch to non-zero costs in the future
-        let vm = MoveVmExt::new(NativeGasParameters::zeros()).unwrap();
+        let vm = MoveVmExt::new(
+            NativeGasParameters::zeros(),
+            AbstractValueSizeGasParameters::zeros(),
+        )
+        .unwrap();
         let remote_view = RemoteStorage::new(&self.data_store);
         let mut session = vm.new_session(&remote_view, SessionId::void());
         session

aptos-move/framework/Cargo.toml

@@ -42,6 +42,7 @@ aptos-module-verifier = { path = "../../aptos-move/aptos-module-verifier" }
 aptos-sdk-builder = { path = "../aptos-sdk-builder" }
 aptos-state-view = { path = "../../storage/state-view" }
 aptos-types = { path = "../../types" }
+gas-algebra-ext =  { path = "../gas-algebra-ext" }
 move-deps = { path = "../move-deps", features = ["address32"] }
 
 [dev-dependencies]

aptos-move/framework/src/natives/event.rs

@@ -2,13 +2,13 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::natives::helpers::make_module_natives;
+use gas_algebra_ext::{AbstractValueSize, InternalGasPerAbstractValueUnit};
 use move_deps::{
     move_binary_format::errors::PartialVMResult,
-    move_core_types::gas_algebra::{InternalGas, InternalGasPerAbstractMemoryUnit},
+    move_core_types::gas_algebra::InternalGas,
     move_vm_runtime::native_functions::{NativeContext, NativeFunction},
     move_vm_types::{
         loaded_data::runtime_types::Type, natives::function::NativeResult, pop_arg, values::Value,
-        views::ValueView,
     },
 };
 use smallvec::smallvec;
@@ -23,12 +23,13 @@ use std::{collections::VecDeque, sync::Arc};
 #[derive(Debug, Clone)]
 pub struct WriteToEventStoreGasParameters {
     pub base: InternalGas,
-    pub per_abstract_memory_unit: InternalGasPerAbstractMemoryUnit,
+    pub per_abstract_value_unit: InternalGasPerAbstractValueUnit,
 }
 
 #[inline]
 fn native_write_to_event_store(
     gas_params: &WriteToEventStoreGasParameters,
+    calc_abstract_val_size: impl FnOnce(&Value) -> AbstractValueSize,
     context: &mut NativeContext,
     mut ty_args: Vec<Type>,
     mut arguments: VecDeque<Value>,
@@ -42,8 +43,7 @@ fn native_write_to_event_store(
     let guid = pop_arg!(arguments, Vec<u8>);
 
     // TODO(Gas): Get rid of abstract memory size
-    let cost =
-        gas_params.base + gas_params.per_abstract_memory_unit * msg.legacy_abstract_memory_size();
+    let cost = gas_params.base + gas_params.per_abstract_value_unit * calc_abstract_val_size(&msg);
 
     if !context.save_event(guid, seq_num, ty, msg)? {
         return Ok(NativeResult::err(cost, 0));
@@ -54,10 +54,17 @@ fn native_write_to_event_store(
 
 pub fn make_native_write_to_event_store(
     gas_params: WriteToEventStoreGasParameters,
+    calc_abstract_val_size: impl Fn(&Value) -> AbstractValueSize + Send + Sync + 'static,
 ) -> NativeFunction {
     Arc::new(
         move |context, ty_args, args| -> PartialVMResult<NativeResult> {
-            native_write_to_event_store(&gas_params, context, ty_args, args)
+            native_write_to_event_store(
+                &gas_params,
+                &calc_abstract_val_size,
+                context,
+                ty_args,
+                args,
+            )
         },
     )
 }
@@ -71,10 +78,13 @@ pub struct GasParameters {
     pub write_to_event_store: WriteToEventStoreGasParameters,
 }
 
-pub fn make_all(gas_params: GasParameters) -> impl Iterator<Item = (String, NativeFunction)> {
+pub fn make_all(
+    gas_params: GasParameters,
+    calc_abstract_val_size: impl Fn(&Value) -> AbstractValueSize + Send + Sync + 'static,
+) -> impl Iterator<Item = (String, NativeFunction)> {
     let natives = [(
         "write_to_event_store",
-        make_native_write_to_event_store(gas_params.write_to_event_store),
+        make_native_write_to_event_store(gas_params.write_to_event_store, calc_abstract_val_size),
     )];
 
     make_module_natives(natives)