[vm] Remove V1 aggregator from execution

Fold AggregatorV1 changes into the same write ops the VM already uses for V2 delayed fields, instead of bespoke variants. A flag-off delta is a Write flagged is_aggregator_v1_delta; a flag-on delta is an InPlaceDelayedFieldChange; a read, create, or delete is a WriteWithDelayedFields. Squashing, fee exclusion, and the in-place id exchange now reuse the V2 paths, removing the AggregatorV1Delta and AggregatorV1MaterializedDelta variants and their squash arms, along with the dormant V1 DeltaOp machinery in the block executor and mvhashmap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: georgemitenkov

aptos-move/aptos-aggregator/Cargo.toml

@@ -23,8 +23,8 @@ claims = { workspace = true }
 move-binary-format = { workspace = true }
 move-core-types = { workspace = true }
 move-vm-types = { workspace = true }
+once_cell = { workspace = true }
 triomphe = { workspace = true }
 
 [dev-dependencies]
-once_cell = { workspace = true }
 test-case = { workspace = true }

aptos-move/aptos-aggregator/src/aggregator_v1_extension.rs

@@ -211,26 +211,7 @@ pub fn delta_add(v: u128, max_value: u128) -> DeltaOp {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::{
-        aggregator_v1_extension::{EADD_OVERFLOW, ESUB_UNDERFLOW},
-        resolver::{TAggregatorV1View, TDelayedFieldView},
-        types::DelayedFieldValue,
-        FakeAggregatorView,
-    };
-    use aptos_types::{
-        error::PanicError,
-        state_store::{
-            state_key::StateKey,
-            state_value::{StateValue, StateValueMetadata},
-        },
-        write_set::WriteOp,
-    };
-    use claims::{assert_err, assert_none, assert_ok, assert_ok_eq, assert_some_eq};
-    use move_binary_format::errors::{PartialVMError, PartialVMResult};
-    use move_core_types::{value::MoveTypeLayout, vm_status::StatusCode};
-    use once_cell::sync::Lazy;
-    use std::collections::{BTreeMap, HashSet};
-    use triomphe::Arc as TriompheArc;
+    use claims::{assert_err, assert_ok, assert_ok_eq};
 
     fn delta_add_with_history(v: u128, max_value: u128, max: u128, min: u128) -> DeltaOp {
         let mut delta = delta_add(v, max_value);
@@ -472,142 +453,4 @@ mod test {
         assert_eq!(b, c);
         assert_eq!(b.update, Negative(1));
     }
-
-    static KEY: Lazy<StateKey> = Lazy::new(|| StateKey::raw(b"test-key"));
-
-    #[test]
-    fn test_failed_write_op_conversion_because_of_empty_storage() {
-        let state_view = FakeAggregatorView::default();
-        let delta_op = delta_add(10, 1000);
-
-        let err =
-            assert_err!(state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &delta_op));
-        assert_eq!(
-            err.major_status(),
-            StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR
-        );
-        assert_none!(err.sub_status());
-    }
-
-    struct BadStorage;
-
-    impl TAggregatorV1View for BadStorage {
-        type Identifier = StateKey;
-
-        fn get_aggregator_v1_state_value(
-            &self,
-            _id: &Self::Identifier,
-        ) -> PartialVMResult<Option<StateValue>> {
-            Err(
-                PartialVMError::new(StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR)
-                    .with_message("Error message from BadStorage.".to_string()),
-            )
-        }
-    }
-
-    impl TDelayedFieldView for BadStorage {
-        type Identifier = ();
-        type ResourceGroupTag = ();
-        type ResourceKey = ();
-
-        fn get_delayed_field_value(
-            &self,
-            _id: &Self::Identifier,
-        ) -> Result<DelayedFieldValue, PanicOr<DelayedFieldsSpeculativeError>> {
-            Err(code_invariant_error("Error message from BadStorage.").into())
-        }
-
-        fn delayed_field_try_add_delta_outcome(
-            &self,
-            _id: &Self::Identifier,
-            _base_delta: &SignedU128,
-            _delta: &SignedU128,
-            _max_value: u128,
-        ) -> Result<bool, PanicOr<DelayedFieldsSpeculativeError>> {
-            Err(code_invariant_error("Error message from BadStorage.").into())
-        }
-
-        fn generate_delayed_field_id(&self, _width: u32) -> Self::Identifier {
-            unimplemented!("Irrelevant for the test")
-        }
-
-        fn validate_delayed_field_id(&self, _id: &Self::Identifier) -> Result<(), PanicError> {
-            unimplemented!("Irrelevant for the test")
-        }
-
-        fn get_reads_needing_exchange(
-            &self,
-            _delayed_write_set_keys: &HashSet<Self::Identifier>,
-            _skip: &HashSet<Self::ResourceKey>,
-        ) -> Result<
-            BTreeMap<Self::ResourceKey, (StateValueMetadata, u64, TriompheArc<MoveTypeLayout>)>,
-            PanicError,
-        > {
-            unimplemented!("Irrelevant for the test")
-        }
-
-        fn get_group_reads_needing_exchange(
-            &self,
-            _delayed_write_set_keys: &HashSet<Self::Identifier>,
-            _skip: &HashSet<Self::ResourceKey>,
-        ) -> PartialVMResult<BTreeMap<Self::ResourceKey, (StateValueMetadata, u64)>> {
-            unimplemented!("Irrelevant for the test")
-        }
-    }
-
-    #[test]
-    fn test_failed_write_op_conversion_because_of_speculative_error() {
-        let state_view = BadStorage;
-        let delta_op = delta_add(10, 1000);
-
-        let err =
-            assert_err!(state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &delta_op));
-        assert_eq!(
-            err.major_status(),
-            StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR
-        );
-        assert_none!(err.sub_status());
-    }
-
-    #[test]
-    fn test_successful_write_op_conversion() {
-        let mut state_view = FakeAggregatorView::default();
-        state_view.set_from_state_key(KEY.clone(), 100);
-
-        // Both addition and subtraction should succeed!
-        let add_op = delta_add(100, 200);
-        let sub_op = delta_sub(100, 200);
-
-        let add_result = state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &add_op);
-        assert_ok_eq!(
-            add_result,
-            WriteOp::legacy_modification(serialize(&200).into())
-        );
-
-        let sub_result = state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &sub_op);
-        assert_ok_eq!(
-            sub_result,
-            WriteOp::legacy_modification(serialize(&0).into())
-        );
-    }
-
-    #[test]
-    fn test_unsuccessful_write_op_conversion() {
-        let mut state_view = FakeAggregatorView::default();
-        state_view.set_from_state_key(KEY.clone(), 100);
-
-        // Both addition and subtraction should fail!
-        let add_op = delta_add(15, 100);
-        let sub_op = delta_sub(101, 1000);
-
-        let err =
-            assert_err!(state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &add_op));
-        assert_eq!(err.major_status(), StatusCode::ABORTED);
-        assert_some_eq!(err.sub_status(), EADD_OVERFLOW);
-
-        let err =
-            assert_err!(state_view.try_convert_aggregator_v1_delta_into_write_op(&KEY, &sub_op));
-        assert_eq!(err.major_status(), StatusCode::ABORTED);
-        assert_some_eq!(err.sub_status(), ESUB_UNDERFLOW);
-    }
 }

aptos-move/aptos-aggregator/src/delta_change_set.rs

@@ -2,19 +2,16 @@
 // Licensed pursuant to the Innovation-Enabling Source Code License, available at https://github.com/aptos-labs/aptos-core/blob/main/LICENSE
 
 use crate::{
-    aggregator_v1_extension::{addition_v1_error, subtraction_v1_error},
     bounded_math::SignedU128,
-    delta_change_set::{serialize, DeltaOp},
-    types::{DelayedFieldValue, DelayedFieldsSpeculativeError, DeltaApplicationFailureReason},
+    types::{DelayedFieldValue, DelayedFieldsSpeculativeError},
 };
 use aptos_types::{
-    error::{code_invariant_error, PanicError, PanicOr},
+    error::{PanicError, PanicOr},
     state_store::{
         state_key::StateKey,
         state_value::{StateValue, StateValueMetadata},
         StateView,
     },
-    write_set::WriteOp,
 };
 use move_binary_format::errors::{PartialVMError, PartialVMResult};
 use move_core_types::{language_storage::StructTag, value::MoveTypeLayout, vm_status::StatusCode};
@@ -38,6 +35,9 @@ pub trait TAggregatorV1View {
     ///   -  Ok(Some(...))    if aggregator value exists in storage,
     ///   -  Err(...)         otherwise (e.g. storage error or failed delta
     ///                       application).
+    ///
+    /// Used when delayed field optimization is not enabled (implementation is
+    /// sequential).
     fn get_aggregator_v1_state_value(
         &self,
         id: &Self::Identifier,
@@ -60,49 +60,20 @@ pub trait TAggregatorV1View {
         &self,
         id: &Self::Identifier,
     ) -> PartialVMResult<Option<StateValueMetadata>> {
-        // When getting state value metadata for aggregator V1, we need to do a
-        // precise read.
         let maybe_state_value = self.get_aggregator_v1_state_value(id)?;
         Ok(maybe_state_value.map(StateValue::into_metadata))
     }
 
-    fn get_aggregator_v1_state_value_size(
-        &self,
-        id: &Self::Identifier,
-    ) -> PartialVMResult<Option<u64>> {
-        let maybe_state_value = self.get_aggregator_v1_state_value(id)?;
-        Ok(maybe_state_value.map(|v| v.size() as u64))
-    }
-
-    /// Consumes a single delta of aggregator V1, and tries to materialize it
-    /// with a given identifier (state key). If materialization succeeds, a
-    /// write op is produced.
-    fn try_convert_aggregator_v1_delta_into_write_op(
+    /// Used **ONLY** if delayed field optimization is enabled.
+    ///
+    /// Returns an ID corresponding to the exchanged value of the aggregator.
+    fn get_aggregator_v1_delayed_field_id(
         &self,
-        id: &Self::Identifier,
-        delta_op: &DeltaOp,
-    ) -> PartialVMResult<WriteOp> {
-        let base = self.get_aggregator_v1_value(id)?.ok_or_else(|| {
-            PartialVMError::new(StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR)
-                .with_message("Cannot convert delta for deleted aggregator".to_string())
-        })?;
-        delta_op
-            .apply_to(base)
-            .map_err(|e| match &e {
-                PanicOr::Or(DelayedFieldsSpeculativeError::DeltaApplication {
-                    reason: DeltaApplicationFailureReason::Overflow,
-                    ..
-                }) => addition_v1_error(e),
-                PanicOr::Or(DelayedFieldsSpeculativeError::DeltaApplication {
-                    reason: DeltaApplicationFailureReason::Underflow,
-                    ..
-                }) => subtraction_v1_error(e),
-                // Because aggregator V1 never underflows or overflows, all other
-                // application errors are bugs.
-                _ => code_invariant_error(format!("Unexpected delta application error: {:?}", e))
-                    .into(),
-            })
-            .map(|result| WriteOp::legacy_modification(serialize(&result).into()))
+        _id: &Self::Identifier,
+    ) -> PartialVMResult<Option<DelayedFieldID>> {
+        // By default - there is no ID (optimization not enabled). Callers
+        // handle this case as an error gracefully.
+        Ok(None)
     }
 }
 

aptos-move/aptos-aggregator/src/resolver.rs

@@ -67,8 +67,7 @@ async fn main() -> Result<()> {
     gas_log.exec_io.check_consistency()?;
     gas_log.storage.check_consistency()?;
 
-    let txn_output =
-        output.try_materialize_into_transaction_output(&debugger.state_view_at_version(version))?;
+    let txn_output = output.try_materialize_into_transaction_output()?;
 
     // Show results to the user
     println!("{:#?}", txn_output);

aptos-move/aptos-debugger/src/bin/remote-gas-profiler.rs

@@ -347,9 +347,8 @@ fn force_end_epoch(state_view: &impl SimulationStateStore) -> Result<()> {
         &mut TraversalContext::new(&traversal_storage),
         &module_storage,
     )?;
-    let mut change_set = sess.finish(&change_set_configs, &module_storage)?;
+    let change_set = sess.finish(&change_set_configs, &module_storage)?;
 
-    change_set.try_materialize_aggregator_v1_delta_set(&resolver)?;
     let (write_set, _events) = change_set
         .try_combine_into_storage_change_set(ModuleWriteSet::empty())
         .expect("Failed to convert to storage ChangeSet")
@@ -530,7 +529,7 @@ pub async fn simulate_multistep_proposal(
         );
 
         let txn_output = vm_output
-            .try_materialize_into_transaction_output(&resolver)
+            .try_materialize_into_transaction_output()
             .context("failed to materialize transaction output")?;
 
         let txn_status = txn_output.status();

aptos-move/aptos-release-builder/src/simulate.rs

@@ -549,7 +549,7 @@ impl Session {
             );
         }
 
-        let txn_output = vm_output.try_materialize_into_transaction_output(&resolver)?;
+        let txn_output = vm_output.try_materialize_into_transaction_output()?;
         self.state_store.apply_write_set(txn_output.write_set())?;
 
         // Re-patch the randomness seed. The block prologue clears it because our
@@ -612,7 +612,7 @@ impl Session {
                     &AuxiliaryInfo::new_timestamp_not_yet_assigned(0),
                 )
                 .map_err(|e| anyhow::anyhow!("transaction execution failed: {:?}", e))?;
-            let txn_output = vm_output.try_materialize_into_transaction_output(&resolver)?;
+            let txn_output = vm_output.try_materialize_into_transaction_output()?;
             // `finish` panics on a consistency failure with the library's generic
             // bug-report message; `finish_without_consistency_check` skips the
             // check entirely. User-facing tools that want a custom error (or to
@@ -632,7 +632,7 @@ impl Session {
                 &log_context,
                 &AuxiliaryInfo::new_timestamp_not_yet_assigned(0),
             );
-            let txn_output = vm_output.try_materialize_into_transaction_output(&resolver)?;
+            let txn_output = vm_output.try_materialize_into_transaction_output()?;
             (vm_status, txn_output, None)
         };