[aggregator] Fix simulation & view for delayed fields (#12063)

api/goldens/aptos_api__tests__view_function__test_view_aggregator.json

@@ -0,0 +1 @@
+["10"]

api/src/tests/move/pack_counter/Move.toml

@@ -0,0 +1,9 @@
+[package]
+name = "pack_counter"
+version = "0.0.0"
+
+[dependencies]
+AptosFramework = { local = "../../../../../aptos-move/framework/aptos-framework" }
+
+[addresses]
+addr = "_"

api/src/tests/move/pack_counter/sources/counter.move

@@ -0,0 +1,26 @@
+module addr::counter {
+    use 0x1::aggregator_v2::{Self, Aggregator};
+
+    struct Counter has key {
+        counter: Aggregator<u64>,
+    }
+
+    fun init_module(account: &signer) {
+        let counter = Counter {
+            counter: aggregator_v2::create_aggregator(100),
+        };
+        move_to(account, counter);
+    }
+
+    public entry fun increment_counter() acquires Counter {
+        let counter = &mut borrow_global_mut<Counter>(@addr).counter;
+        aggregator_v2::add(counter, 1);
+    }
+
+    #[view]
+    public fun add_and_get_counter_value(): u64 acquires Counter {
+        let counter = &mut borrow_global_mut<Counter>(@addr).counter;
+        aggregator_v2::add(counter, 10);
+        aggregator_v2::read(counter)
+    }
+}

api/src/tests/simulation_test.rs

@@ -4,8 +4,12 @@
 use super::new_test_context;
 use aptos_api_test_context::{current_function_name, TestContext};
 use aptos_crypto::ed25519::Ed25519Signature;
-use aptos_types::transaction::authenticator::TransactionAuthenticator;
+use aptos_types::transaction::{
+    authenticator::TransactionAuthenticator, EntryFunction, TransactionPayload,
+};
+use move_core_types::{ident_str, language_storage::ModuleId};
 use serde_json::json;
+use std::path::PathBuf;
 
 async fn simulate_aptos_transfer(
     context: &mut TestContext,
@@ -82,3 +86,57 @@ async fn test_simulate_transaction_with_insufficient_balance() {
     let resp = simulate_aptos_transfer(&mut context, false, LARGE_TRANSFER_AMOUNT, 200).await;
     assert!(!resp[0]["success"].as_bool().is_some_and(|v| v));
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_simulate_txn_with_aggregator() {
+    let mut context = new_test_context(current_function_name!());
+    let account = context.root_account().await;
+
+    let named_addresses = vec![("addr".to_string(), account.address())];
+    let path = PathBuf::from(std::env!("CARGO_MANIFEST_DIR")).join("src/tests/move/pack_counter");
+    let payload = TestContext::build_package(path, named_addresses);
+    let txn = account.sign_with_transaction_builder(context.transaction_factory().payload(payload));
+    context.commit_block(&vec![txn]).await;
+
+    let payload = TransactionPayload::EntryFunction(EntryFunction::new(
+        ModuleId::new(account.address(), ident_str!("counter").to_owned()),
+        ident_str!("increment_counter").to_owned(),
+        vec![],
+        vec![],
+    ));
+    let txn = account.sign_with_transaction_builder(context.transaction_factory().payload(payload));
+    if let TransactionAuthenticator::Ed25519 {
+        public_key,
+        signature: _,
+    } = txn.authenticator_ref()
+    {
+        let function = format!("{}::counter::increment_counter", account.address());
+        let resp = context
+            .expect_status_code(200)
+            .post(
+                "/transactions/simulate",
+                json!({
+                    "sender": txn.sender().to_string(),
+                    "sequence_number": txn.sequence_number().to_string(),
+                    "max_gas_amount": txn.max_gas_amount().to_string(),
+                    "gas_unit_price": txn.gas_unit_price().to_string(),
+                    "expiration_timestamp_secs": txn.expiration_timestamp_secs().to_string(),
+                    "payload": {
+                        "type": "entry_function_payload",
+                        "function": function,
+                        "type_arguments": [],
+                        "arguments": []
+                    },
+                    "signature": {
+                        "type": "ed25519_signature",
+                        "public_key": public_key.to_string(),
+                        "signature": Ed25519Signature::dummy_signature().to_string(),
+                    }
+                }),
+            )
+            .await;
+        assert!(resp[0]["success"].as_bool().is_some_and(|v| v));
+    } else {
+        unreachable!("Simulation uses Ed25519 authenticator.");
+    }
+}

api/src/tests/view_function.rs

@@ -2,12 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use super::{new_test_context, new_test_context_with_config};
-use aptos_api_test_context::current_function_name;
+use aptos_api_test_context::{current_function_name, TestContext};
 use aptos_cached_packages::aptos_stdlib;
 use aptos_config::config::{NodeConfig, ViewFilter, ViewFunctionId};
 use aptos_types::account_address::AccountAddress;
 use serde_json::{json, Value};
-use std::str::FromStr;
+use std::{path::PathBuf, str::FromStr};
 
 fn build_coin_balance_request(address: &AccountAddress) -> Value {
     json!({
@@ -188,7 +188,32 @@ async fn test_view_tuple() {
         .post(
             "/view",
             json!({
-                "function":"0xa550c18::test_module::return_tuple",
+                "function": "0xa550c18::test_module::return_tuple",
+                "arguments": [],
+                "type_arguments": [],
+            }),
+        )
+        .await;
+    context.check_golden_output_no_prune(resp);
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_view_aggregator() {
+    let mut context = new_test_context(current_function_name!());
+    let account = context.root_account().await;
+
+    let named_addresses = vec![("addr".to_string(), account.address())];
+    let path = PathBuf::from(std::env!("CARGO_MANIFEST_DIR")).join("src/tests/move/pack_counter");
+    let payload = TestContext::build_package(path, named_addresses);
+    let txn = account.sign_with_transaction_builder(context.transaction_factory().payload(payload));
+    context.commit_block(&vec![txn]).await;
+
+    let function = format!("{}::counter::add_and_get_counter_value", account.address());
+    let resp = context
+        .post(
+            "/view",
+            json!({
+                "function": function,
                 "arguments": [],
                 "type_arguments": [],
             }),

aptos-move/aptos-debugger/src/aptos_debugger.rs

@@ -78,7 +78,10 @@ impl AptosDebugger {
 
         // TODO(Gas): revisit this.
         let resolver = state_view.as_move_resolver();
-        let vm = AptosVM::new(&resolver);
+        let vm = AptosVM::new(
+            &resolver,
+            /*override_is_delayed_field_optimization_capable=*/ Some(false),
+        );
 
         // Module bundle is deprecated!
         if let TransactionPayload::ModuleBundle(_) = txn.payload() {
@@ -268,6 +271,7 @@ impl AptosDebugger {
             features,
             TimedFeaturesBuilder::enable_all().build(),
             &state_view_storage,
+            /*aggregator_v2_type_tagging*/ false,
         )
         .unwrap();
         let mut session = move_vm.new_session(&state_view_storage, SessionId::Void);

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

@@ -171,7 +171,10 @@ pub struct AptosVM {
 }
 
 impl AptosVM {
-    pub fn new(resolver: &impl AptosMoveResolver) -> Self {
+    pub fn new(
+        resolver: &impl AptosMoveResolver,
+        override_is_delayed_field_optimization_capable: Option<bool>,
+    ) -> Self {
         let _timer = TIMER.timer_with(&["AptosVM::new"]);
 
         let features = Features::fetch_config(resolver).unwrap_or_default();
@@ -191,11 +194,19 @@ impl AptosVM {
             .unwrap_or(0);
 
         let mut timed_features_builder = TimedFeaturesBuilder::new(chain_id, timestamp);
-        if let Some(profile) = crate::AptosVM::get_timed_feature_override() {
+        if let Some(profile) = Self::get_timed_feature_override() {
             timed_features_builder = timed_features_builder.with_override_profile(profile)
         }
         let timed_features = timed_features_builder.build();
 
+        // If aggregator execution is enabled, we need to tag aggregator_v2 types,
+        // so they can be exchanged with identifiers during VM execution.
+        let override_is_delayed_field_optimization_capable =
+            override_is_delayed_field_optimization_capable
+                .unwrap_or_else(|| resolver.is_delayed_field_optimization_capable());
+        let aggregator_v2_type_tagging = override_is_delayed_field_optimization_capable
+            && features.is_aggregator_v2_delayed_fields_enabled();
+
         let move_vm = MoveVmExt::new(
             native_gas_params,
             misc_gas_params,
@@ -204,6 +215,7 @@ impl AptosVM {
             features.clone(),
             timed_features.clone(),
             resolver,
+            aggregator_v2_type_tagging,
         )
         .expect("should be able to create Move VM; check if there are duplicated natives");
 
@@ -218,7 +230,7 @@ impl AptosVM {
         }
     }
 
-    pub(crate) fn new_session<'r, S: AptosMoveResolver>(
+    pub fn new_session<'r, S: AptosMoveResolver>(
         &self,
         resolver: &'r S,
         session_id: SessionId,
@@ -1801,7 +1813,10 @@ impl AptosVM {
         gas_budget: u64,
     ) -> ViewFunctionOutput {
         let resolver = state_view.as_move_resolver();
-        let vm = AptosVM::new(&resolver);
+        let vm = AptosVM::new(
+            &resolver,
+            /*override_is_delayed_field_optimization_capable=*/ Some(false),
+        );
         let log_context = AdapterLogSchema::new(state_view.id(), 0);
         let mut gas_meter = match Self::memory_tracked_gas_meter(&vm, &log_context, gas_budget) {
             Ok(gas_meter) => gas_meter,
@@ -2235,7 +2250,10 @@ pub struct AptosSimulationVM(AptosVM);
 
 impl AptosSimulationVM {
     pub fn new(resolver: &impl AptosMoveResolver) -> Self {
-        let mut vm = AptosVM::new(resolver);
+        let mut vm = AptosVM::new(
+            resolver,
+            /*override_is_delayed_field_optimization_capable=*/ Some(false),
+        );
         vm.is_simulation = true;
         Self(vm)
     }

aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs

@@ -32,7 +32,10 @@ impl<'a, S: 'a + StateView + Sync> ExecutorTask for AptosExecutorTask<'a, S> {
 
     fn init(argument: &'a S) -> Self {
         // AptosVM has to be initialized using configs from storage.
-        let vm = AptosVM::new(&argument.as_move_resolver());
+        let vm = AptosVM::new(
+            &argument.as_move_resolver(),
+            /*override_is_delayed_field_optimization_capable=*/ Some(true),
+        );
 
         Self {
             vm,

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

@@ -72,6 +72,7 @@ impl MoveVmExt {
         timed_features: TimedFeatures,
         gas_hook: Option<F>,
         resolver: &impl AptosMoveResolver,
+        aggregator_v2_type_tagging: bool,
     ) -> VMResult<Self>
     where
         F: Fn(DynamicExpression) + Send + Sync + 'static,
@@ -100,10 +101,6 @@ impl MoveVmExt {
             type_byte_cost = 1;
         }
 
-        // If aggregator execution is enabled, we need to tag aggregator_v2 types,
-        // so they can be exchanged with identifiers during VM execution.
-        let aggregator_v2_type_tagging = features.is_aggregator_v2_delayed_fields_enabled();
-
         let mut builder = SafeNativeBuilder::new(
             gas_feature_version,
             native_gas_params.clone(),
@@ -149,6 +146,7 @@ impl MoveVmExt {
         features: Features,
         timed_features: TimedFeatures,
         resolver: &impl AptosMoveResolver,
+        aggregator_v2_type_tagging: bool,
     ) -> VMResult<Self> {
         Self::new_impl::<fn(DynamicExpression)>(
             native_gas_params,
@@ -159,6 +157,7 @@ impl MoveVmExt {
             timed_features,
             None,
             resolver,
+            aggregator_v2_type_tagging,
         )
     }
 
@@ -171,6 +170,7 @@ impl MoveVmExt {
         timed_features: TimedFeatures,
         gas_hook: Option<F>,
         resolver: &impl AptosMoveResolver,
+        aggregator_v2_type_tagging: bool,
     ) -> VMResult<Self>
     where
         F: Fn(DynamicExpression) + Send + Sync + 'static,
@@ -184,6 +184,7 @@ impl MoveVmExt {
             timed_features,
             gas_hook,
             resolver,
+            aggregator_v2_type_tagging,
         )
     }
 

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

@@ -828,7 +828,10 @@ impl MoveHarness {
     }
 
     pub fn new_vm(&self) -> AptosVM {
-        AptosVM::new(&self.executor.data_store().as_move_resolver())
+        AptosVM::new(
+            &self.executor.data_store().as_move_resolver(),
+            /*override_is_delayed_field_optimization_capable=*/ None,
+        )
     }
 
     pub fn set_default_gas_unit_price(&mut self, gas_unit_price: u64) {

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

@@ -615,7 +615,9 @@ impl FakeExecutor {
 
         // TODO(Gas): revisit this.
         let resolver = self.data_store.as_move_resolver();
-        let vm = AptosVM::new(&resolver);
+        let vm = AptosVM::new(
+            &resolver, /*override_is_delayed_field_optimization_capable=*/ None,
+        );
 
         let (_status, output, gas_profiler) = vm.execute_user_transaction_with_custom_gas_meter(
             &resolver,
@@ -689,8 +691,11 @@ impl FakeExecutor {
     }
 
     /// Verifies the given transaction by running it through the VM verifier.
-    pub fn verify_transaction(&self, txn: SignedTransaction) -> VMValidatorResult {
-        let vm = AptosVM::new(&self.get_state_view().as_move_resolver());
+    pub fn validate_transaction(&self, txn: SignedTransaction) -> VMValidatorResult {
+        let vm = AptosVM::new(
+            &self.get_state_view().as_move_resolver(),
+            /*override_is_delayed_field_optimization_capable=*/ None,
+        );
         vm.validate_transaction(txn, &self.data_store)
     }
 
@@ -809,6 +814,7 @@ impl FakeExecutor {
             self.features.clone(),
             timed_features,
             &resolver,
+            false,
         )
         .unwrap();
 
@@ -886,6 +892,7 @@ impl FakeExecutor {
                     a2.lock().unwrap().push(expression);
                 }),
                 &resolver,
+                /*aggregator_v2_type_tagging=*/ false,
             )
             .unwrap();
             let mut session = vm.new_session(&resolver, SessionId::void());
@@ -959,6 +966,7 @@ impl FakeExecutor {
                 self.features.clone(),
                 timed_features,
                 &resolver,
+                false,
             )
             .unwrap();
             let mut session = vm.new_session(&resolver, SessionId::void());
@@ -1022,6 +1030,7 @@ impl FakeExecutor {
             features.clone(),
             timed_features,
             &resolver,
+            features.is_aggregator_v2_delayed_fields_enabled(),
         )
         .unwrap();
         let mut session = vm.new_session(&resolver, SessionId::void());
@@ -1077,6 +1086,7 @@ impl FakeExecutor {
             // FIXME: should probably read the timestamp from storage.
             TimedFeaturesBuilder::enable_all().build(),
             &resolver,
+            false,
         )
         .unwrap();
         let mut session = vm.new_session(&resolver, SessionId::void());