fix StorageGasParameters::unlimited()

1. make it use latest logic, easier to maintain going forward 2. use latest() instead of unlimited() when proper
aptos-labs · Feb 29, 2024 · a1f4d02 · a1f4d02
1 parent 184df6e
commit a1f4d02
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 41 deletions.
diff --git a/aptos-move/aptos-vm-types/src/storage/mod.rs b/aptos-move/aptos-vm-types/src/storage/mod.rs
@@ -2,17 +2,14 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::storage::{
-    change_set_configs::ChangeSetConfigs,
-    io_pricing::{IoPricing, IoPricingV3},
-    space_pricing::DiskSpacePricing,
+    change_set_configs::ChangeSetConfigs, io_pricing::IoPricing, space_pricing::DiskSpacePricing,
 };
 use aptos_gas_schedule::{AptosGasParameters, InitialGasSchedule, LATEST_GAS_FEATURE_VERSION};
 use aptos_types::{
     access_path::AccessPath,
     on_chain_config::{ConfigStorage, Features},
 };
 use bytes::Bytes;
-use move_core_types::gas_algebra::NumBytes;
 use std::fmt::Debug;
 
 pub mod change_set_configs;
@@ -33,44 +30,56 @@ impl StorageGasParameters {
         gas_params: &AptosGasParameters,
         config_storage: &impl ConfigStorage,
     ) -> Self {
-        let io_pricing = IoPricing::new(gas_feature_version, gas_params, config_storage);
-        let space_pricing = DiskSpacePricing::new(gas_feature_version, features);
-        let change_set_configs = ChangeSetConfigs::new(gas_feature_version, gas_params);
-
-        Self {
-            io_pricing,
-            space_pricing,
-            change_set_configs,
-        }
+        Self::new_impl(
+            gas_feature_version,
+            features,
+            gas_params,
+            config_storage,
+            ChangeSetConfigs::new(gas_feature_version, gas_params),
+        )
     }
 
-    pub fn unlimited(free_write_bytes_quota: NumBytes) -> Self {
-        Self {
-            io_pricing: IoPricing::V3(IoPricingV3 {
-                feature_version: LATEST_GAS_FEATURE_VERSION,
-                legacy_free_write_bytes_quota: free_write_bytes_quota,
-            }),
-            space_pricing: DiskSpacePricing::latest(),
-            change_set_configs: ChangeSetConfigs::unlimited_at_gas_feature_version(
-                LATEST_GAS_FEATURE_VERSION,
-            ),
-        }
+    pub fn unlimited() -> Self {
+        Self::new_impl(
+            LATEST_GAS_FEATURE_VERSION,
+            &Features::default(),
+            &AptosGasParameters::zeros(), // free of charge
+            &DummyConfigStorage,
+            ChangeSetConfigs::unlimited_at_gas_feature_version(LATEST_GAS_FEATURE_VERSION), // no limits
+        )
     }
 
     pub fn latest() -> Self {
-        struct DummyConfigStorage;
-
-        impl ConfigStorage for DummyConfigStorage {
-            fn fetch_config(&self, _access_path: AccessPath) -> Option<Bytes> {
-                unreachable!("Not supposed to be called from latest() / tests.")
-            }
-        }
-
         Self::new(
             LATEST_GAS_FEATURE_VERSION,
             &Features::default(),
             &AptosGasParameters::initial(),
             &DummyConfigStorage,
         )
     }
+
+    fn new_impl(
+        gas_feature_version: u64,
+        features: &Features,
+        gas_params: &AptosGasParameters,
+        config_storage: &impl ConfigStorage,
+        change_set_configs: ChangeSetConfigs,
+    ) -> Self {
+        let io_pricing = IoPricing::new(gas_feature_version, gas_params, config_storage);
+        let space_pricing = DiskSpacePricing::new(gas_feature_version, features);
+
+        Self {
+            io_pricing,
+            space_pricing,
+            change_set_configs,
+        }
+    }
+}
+
+struct DummyConfigStorage;
+
+impl ConfigStorage for DummyConfigStorage {
+    fn fetch_config(&self, _access_path: AccessPath) -> Option<Bytes> {
+        unreachable!("Not supposed to be called from latest() / tests.")
+    }
 }
diff --git a/aptos-move/aptos-vm-types/src/storage/space_pricing.rs b/aptos-move/aptos-vm-types/src/storage/space_pricing.rs
@@ -42,10 +42,6 @@ impl DiskSpacePricing {
         }
     }
 
-    pub fn latest() -> Self {
-        Self::V2
-    }
-
     /// Calculates the storage fee for a state slot allocation.
     pub fn charge_refund_write_op(
         &self,

diff --git a/aptos-move/e2e-move-tests/src/tests/fee_payer.rs b/aptos-move/e2e-move-tests/src/tests/fee_payer.rs
@@ -14,6 +14,7 @@ use aptos_types::{
     on_chain_config::FeatureFlag,
     transaction::{EntryFunction, ExecutionStatus, Script, TransactionPayload, TransactionStatus},
 };
+use aptos_vm_types::storage::StorageGasParameters;
 use move_core_types::{move_resource::MoveStructType, vm_status::StatusCode};
 use once_cell::sync::Lazy;
 
@@ -400,13 +401,11 @@ impl FeePayerPricingInfo {
 }
 
 static PRICING: Lazy<FeePayerPricingInfo> = Lazy::new(|| {
-    use aptos_vm_types::storage::space_pricing::DiskSpacePricing;
-
     let h = MoveHarness::new();
 
     let (_feature_version, params) = h.get_gas_params();
     let params = params.vm.txn;
-    let pricing = DiskSpacePricing::latest();
+    let pricing = StorageGasParameters::latest().space_pricing;
 
     FeePayerPricingInfo {
         estimated_per_new_account_fee_octas: u64::from(

diff --git a/aptos-move/e2e-tests/src/executor.rs b/aptos-move/e2e-tests/src/executor.rs
@@ -830,7 +830,7 @@ impl FakeExecutor {
             ),
             GasMeterType::UnmeteredGasMeter => (
                 AptosGasParameters::zeros(),
-                StorageGasParameters::unlimited(0.into()),
+                StorageGasParameters::unlimited(),
             ),
         };
 
@@ -978,7 +978,7 @@ impl FakeExecutor {
                         //// TODO: fill in these with proper values
                         LATEST_GAS_FEATURE_VERSION,
                         InitialGasSchedule::initial(),
-                        StorageGasParameters::unlimited(0.into()),
+                        StorageGasParameters::latest(),
                         10000000000000,
                     ),
                     // coeff_buffer: BTreeMap::new(),