[vm] Fix script hot-state promotion determinism

aptos-move/aptos-vm-types/Cargo.toml

@@ -39,3 +39,6 @@ aptos-gas-schedule = { workspace = true, features = ["testing"] }
 aptos-transaction-simulation = { workspace = true }
 aptos-vm = { workspace = true }
 test-case = { workspace = true }
+
+[lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] }

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

@@ -144,7 +144,10 @@ use move_vm_runtime::{
     InstantiatedFunctionLoader, LegacyLoaderConfig, ModuleStorage, RuntimeEnvironment,
     ScriptLoader, WithRuntimeEnvironment,
 };
-use move_vm_types::gas::{DependencyKind, GasMeter, UnmeteredGasMeter};
+use move_vm_types::{
+    gas::{DependencyKind, GasMeter, UnmeteredGasMeter},
+    sha3_256,
+};
 use num_cpus;
 use once_cell::sync::OnceCell;
 use rand::RngCore;
@@ -969,6 +972,26 @@ impl AptosVM {
         }
 
         dispatch_loader!(code_storage, loader, {
+            // Record the script's declared dependencies before any fallible step: a
+            // kept-but-failed script still feeds its reads into the consensus-visible promotion
+            // set, which must not depend on verified-script-cache warmth. Probing the script
+            // cache is fine (it is keyed by the byte hash), and if deserialization fails,
+            // `load_script` fails the same way below.
+            let hash = sha3_256(serialized_script.code());
+            let compiled_script = match code_storage.get_script(&hash) {
+                Some(code) => Some(Arc::clone(code.deserialized())),
+                None => code_storage
+                    .runtime_environment()
+                    .deserialize_into_script(serialized_script.code())
+                    .ok()
+                    .map(|script| code_storage.insert_deserialized_script(hash, script)),
+            };
+            if let Some(compiled_script) = compiled_script {
+                for (address, module_name) in compiled_script.immediate_dependencies_iter() {
+                    code_storage.record_module_read(address, module_name);
+                }
+            }
+
             let legacy_loader_config = LegacyLoaderConfig {
                 charge_for_dependencies: self.gas_feature_version() >= RELEASE_V1_10,
                 charge_for_ty_tag_dependencies: self.gas_feature_version() >= RELEASE_V1_27,
@@ -986,15 +1009,6 @@ impl AptosVM {
             self.reject_unstable_bytecode_for_script(script)?;
             event_validation::verify_no_event_emission_in_compiled_script(script)?;
 
-            // Record the script's declared module dependencies as reads. These are a function of
-            // the script bytecode, so recording them here keeps the read set independent of the
-            // verified-script cache: its warmth depends on the execution schedule (parallel
-            // interleaving, aborts), so deriving these reads from cache-gated dependency fetches
-            // would make the hot-state promotion set nondeterministic across nodes.
-            for (address, module_name) in script.immediate_dependencies_iter() {
-                code_storage.record_module_read(address, module_name);
-            }
-
             let args = dispatch_transaction_arg_validation!(
                 session,
                 &loader,

aptos-move/block-executor/src/hot_state_op_accumulator.rs

@@ -45,6 +45,10 @@ where
         }
     }
 
+    // TODO(HotState): one transaction can contribute an arbitrarily large read set here (e.g. a
+    // failed-but-kept script recording all its declared dependencies), so a single cheap
+    // transaction can dominate the per-block promotion budget. Consider a per-transaction cap, or
+    // not promoting reads from failed-but-kept transactions.
     pub fn add_transaction<'a>(
         &mut self,
         writes: impl Iterator<Item = &'a Key>,

aptos-move/e2e-move-tests/Cargo.toml

@@ -57,6 +57,7 @@ aptos-aggregator = { workspace = true }
 aptos-block-executor = { workspace = true, features = ["testing"] }
 aptos-mvhashmap = { workspace = true }
 aptos-resource-viewer = { workspace = true }
+aptos-vm-logging = { workspace = true }
 aptos-vm-types = { workspace = true }
 claims = { workspace = true }
 memory-stats = { workspace = true }

aptos-move/e2e-move-tests/src/tests/hot_state.data/emit_pack/Move.toml

@@ -0,0 +1,6 @@
+[package]
+name = "hot_state_emit_test"
+version = "0.0.0"
+
+[dependencies]
+AptosFramework = { local = "../../../../../framework/aptos-framework" }

aptos-move/e2e-move-tests/src/tests/hot_state.data/emit_pack/scripts/emit_script.move

@@ -0,0 +1,14 @@
+script {
+    use aptos_framework::coin;
+    use aptos_framework::aptos_coin::AptosCoin;
+    use aptos_framework::event;
+    use 0xcafe::emitter;
+
+    /// Event emission gets this script rejected (kept-and-failed) only after `load_script` has
+    /// cached the verified script. The dependencies deliberately mix special-address framework
+    /// modules with a non-special user module (`emitter`).
+    fun main() {
+        let _ = coin::supply<AptosCoin>();
+        event::emit(emitter::make());
+    }
+}

aptos-move/e2e-move-tests/src/tests/hot_state.data/emit_pack/sources/emitter.move

@@ -0,0 +1,9 @@
+module 0xcafe::emitter {
+    // Minimal event type so the script can call `0x1::event::emit`.
+    #[event]
+    struct Marker has store, drop { value: u64 }
+
+    public fun make(): Marker {
+        Marker { value: 0 }
+    }
+}

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

@@ -30,17 +30,27 @@ use aptos_types::{
     state_store::{
         state_key::{inner::StateKeyInner, StateKey},
         table::TableHandle,
+        TStateView,
     },
     transaction::{
-        signature_verified_transaction::into_signature_verified_block, ExecutionStatus,
-        Transaction, TransactionArgument, TransactionStatus,
+        signature_verified_transaction::into_signature_verified_block, AuxiliaryInfo,
+        ExecutionStatus, Transaction, TransactionArgument, TransactionStatus,
     },
     utility_coin::AptosCoinType,
 };
-use aptos_vm::{aptos_vm::AptosVMBlockExecutor, VMBlockExecutor};
+use aptos_vm::{
+    aptos_vm::{AptosVM, AptosVMBlockExecutor},
+    data_cache::AsMoveResolver,
+    VMBlockExecutor,
+};
+use aptos_vm_environment::environment::AptosEnvironment;
+use aptos_vm_logging::log_schema::AdapterLogSchema;
+use aptos_vm_types::module_and_script_storage::{
+    read_recording::ReadRecordingCodeStorage, AsAptosCodeStorage,
+};
 use move_core_types::{
     account_address::AccountAddress, ident_str, language_storage::StructTag,
-    parser::parse_struct_tag,
+    parser::parse_struct_tag, vm_status::StatusCode,
 };
 use std::collections::BTreeSet;
 
@@ -706,3 +716,114 @@ fn test_block_promotions_cover_reads_and_exclude_writes() {
         promoted_and_written,
     );
 }
+
+/// Regression test: a kept-but-failed script txn's recorded module reads (which feed the block
+/// epilogue's `to_make_hot`) must not depend on verified-script-cache warmth. The script emits an
+/// event, so `load_script` succeeds and caches the verified script, but the txn is then rejected
+/// and kept-and-charged; without up-front recording, a warm cache skips the special-address
+/// framework dependencies that a cold cache records.
+///
+/// Drives `execute_single_transaction` directly: the block-level helpers above aggregate reads
+/// across transactions, and only a racy parallel schedule can make the per-block script cache
+/// warm for a committing incarnation.
+#[test]
+fn test_failed_script_promotions_independent_of_script_cache() {
+    let mut h = MoveHarness::new();
+    // Skip the compiler's own bailout on event emission so the VM gets to reject the bytecode
+    // instead (as in `tests::module_event`).
+    let mut build_options = BuildOptions::move_2().set_latest_language();
+    build_options
+        .experiments
+        .push("skip-bailout-on-extended-checks".to_string());
+
+    let publisher = h.new_account_at(helper_address());
+    assert_success!(h.publish_package_with_options(
+        &publisher,
+        &common::test_dir_path("hot_state.data/emit_pack"),
+        build_options.clone(),
+    ));
+    let sender = h.new_account_with_key_pair();
+
+    let package = BuiltPackage::build(
+        common::test_dir_path("hot_state.data/emit_pack"),
+        build_options,
+    )
+    .expect("emit_pack must build");
+    let script = package
+        .extract_script_code()
+        .pop()
+        .expect("emit_script must exist");
+    let txn = into_signature_verified_block(vec![Transaction::UserTransaction(h.create_script(
+        &sender,
+        script,
+        vec![],
+        vec![],
+    ))])
+    .pop()
+    .unwrap();
+
+    let state_view = h.executor.get_state_view();
+    let env = AptosEnvironment::new(state_view);
+    let vm = AptosVM::new(&env);
+
+    // Runs the txn against `adapter` (whose verified-script cache persists across calls, unlike
+    // the per-call recording wrapper) and returns the recorded module reads.
+    let run = |adapter: &_| -> BTreeSet<StateKey> {
+        let code_storage = ReadRecordingCodeStorage::new(adapter);
+        let resolver = state_view.as_move_resolver();
+        let log_context = AdapterLogSchema::new(state_view.id(), 0);
+        let (vm_status, output) = vm
+            .execute_single_transaction(
+                &txn,
+                &resolver,
+                &code_storage,
+                &log_context,
+                &AuxiliaryInfo::default(),
+            )
+            .expect("script execution should not hard-error");
+        assert_eq!(
+            vm_status.status_code(),
+            StatusCode::INVALID_OPERATION_IN_SCRIPT,
+            "script must be rejected for emitting an event",
+        );
+        assert!(
+            !output.status().is_discarded(),
+            "the rejected script txn must be kept, else its reads never feed to_make_hot",
+        );
+        code_storage.into_recorded_reads().into_iter().collect()
+    };
+
+    // Cold: a fresh code storage misses the verified-script cache.
+    let cold_adapter = state_view.as_aptos_code_storage(vm.runtime_environment());
+    let cold_reads = run(&cold_adapter);
+
+    // Warm: the first run primes the verified-script cache; keep the second run's reads.
+    let warm_adapter = state_view.as_aptos_code_storage(vm.runtime_environment());
+    let _ = run(&warm_adapter);
+    let warm_reads = run(&warm_adapter);
+
+    let event_module = StateKey::module(&AccountAddress::ONE, ident_str!("event"));
+    let coin_module = StateKey::module(&AccountAddress::ONE, ident_str!("coin"));
+    let emitter_module = StateKey::module(&helper_address(), ident_str!("emitter"));
+
+    // The special-address framework modules are the ones a warm cache would drop; the
+    // non-special `emitter` is a control recorded on both paths.
+    assert!(
+        cold_reads.contains(&event_module),
+        "cold run must record 0x1::event"
+    );
+    assert!(
+        cold_reads.contains(&coin_module),
+        "cold run must record 0x1::coin"
+    );
+    assert!(cold_reads.contains(&emitter_module));
+    assert!(warm_reads.contains(&emitter_module));
+
+    let cold_only: Vec<_> = cold_reads.difference(&warm_reads).collect();
+    assert_eq!(
+        cold_reads, warm_reads,
+        "a kept-failed script txn's recorded module reads must not depend on verified-script-cache \
+         warmth; modules recorded only on the cold path: {:?}",
+        cold_only,
+    );
+}