Merge TransactionBlockExecutor and VMExecutor into VMBlockExecutor

api/test-context/src/test_context.rs

@@ -17,10 +17,7 @@ use aptos_config::{
 };
 use aptos_crypto::{ed25519::Ed25519PrivateKey, hash::HashValue, SigningKey};
 use aptos_db::AptosDB;
-use aptos_executor::{
-    block_executor::{AptosVMBlockExecutor, BlockExecutor},
-    db_bootstrapper,
-};
+use aptos_executor::{block_executor::BlockExecutor, db_bootstrapper};
 use aptos_executor_types::BlockExecutorTrait;
 use aptos_framework::BuiltPackage;
 use aptos_indexer_grpc_table_info::internal_indexer_db_service::MockInternalIndexerDBService;
@@ -50,7 +47,7 @@ use aptos_types::{
         TransactionPayload, TransactionStatus, Version,
     },
 };
-use aptos_vm::AptosVM;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use aptos_vm_validator::vm_validator::PooledVMValidator;
 use bytes::Bytes;
 use hyper::{HeaderMap, Response};
@@ -171,8 +168,12 @@ pub fn new_test_context_inner(
         }
         DbReaderWriter::wrap(aptos_db)
     };
-    let ret =
-        db_bootstrapper::maybe_bootstrap::<AptosVM>(&db_rw, &genesis, genesis_waypoint).unwrap();
+    let ret = db_bootstrapper::maybe_bootstrap::<AptosVMBlockExecutor>(
+        &db_rw,
+        &genesis,
+        genesis_waypoint,
+    )
+    .unwrap();
     assert!(ret.is_some());
 
     let mempool = MockSharedMempool::new_in_runtime(&db_rw, PooledVMValidator::new(db.clone(), 1));

aptos-move/aptos-e2e-comparison-testing/src/data_collection.rs

@@ -17,7 +17,7 @@ use aptos_types::{
     write_set::TOTAL_SUPPLY_STATE_KEY,
 };
 use aptos_validator_interface::{AptosValidatorInterface, FilterCondition, RestDebuggerInterface};
-use aptos_vm::{AptosVM, VMExecutor};
+use aptos_vm::{aptos_vm::AptosVMBlockExecutor, VMBlockExecutor};
 use move_core_types::account_address::AccountAddress;
 use std::{
     collections::HashMap,
@@ -92,7 +92,8 @@ impl DataCollection {
         // FIXME(#10412): remove the assert
         let val = debugger_state_view.get_state_value(TOTAL_SUPPLY_STATE_KEY.deref());
         assert!(val.is_ok() && val.unwrap().is_some());
-        AptosVM::execute_block_no_limit(&sig_verified_txns, debugger_state_view)
+        AptosVMBlockExecutor
+            .execute_block_no_limit(&sig_verified_txns, debugger_state_view)
             .map_err(|err| format_err!("Unexpected VM Error: {:?}", err))
     }
 

aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs

@@ -27,7 +27,7 @@ use aptos_types::{
     },
     AptosCoinType,
 };
-use aptos_vm::{AptosVM, VMExecutor};
+use aptos_vm::{aptos_vm::AptosVMBlockExecutor, VMBlockExecutor};
 use aptos_vm_environment::prod_configs::set_paranoid_type_checks;
 use aptos_vm_genesis::GENESIS_KEYPAIR;
 use clap::Parser;
@@ -522,9 +522,9 @@ impl<'a> AptosTestAdapter<'a> {
             // Or should we just use execute_block_no_limit ?
             block_gas_limit_type: BlockGasLimitType::Limit(30000),
         };
-        let (mut outputs, _) =
-            AptosVM::execute_block(&sig_verified_block, &self.storage.clone(), onchain_config)?
-                .into_inner();
+        let (mut outputs, _) = AptosVMBlockExecutor
+            .execute_block(&sig_verified_block, &self.storage.clone(), onchain_config)?
+            .into_inner();
 
         assert_eq!(outputs.len(), 1);
 

aptos-move/aptos-vm-profiling/src/bins/run_aptos_p2p.rs

@@ -7,7 +7,7 @@ use aptos_types::{
     transaction::{signature_verified_transaction::SignatureVerifiedTransaction, Transaction},
     write_set::WriteSet,
 };
-use aptos_vm::{AptosVM, VMExecutor};
+use aptos_vm::{aptos_vm::AptosVMBlockExecutor, VMBlockExecutor};
 use std::{
     collections::HashMap,
     io::{self, Read},
@@ -48,7 +48,7 @@ fn main() -> Result<()> {
         })
         .collect();
 
-    let res = AptosVM::execute_block_no_limit(&txns, &state_store)?;
+    let res = AptosVMBlockExecutor.execute_block_no_limit(&txns, &state_store)?;
     for i in 0..NUM_TXNS {
         assert!(res[i as usize].status().status().unwrap().is_success());
     }

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

@@ -24,7 +24,7 @@ use crate::{
     transaction_metadata::TransactionMetadata,
     transaction_validation, verifier,
     verifier::randomness::get_randomness_annotation,
-    VMExecutor, VMValidator,
+    VMBlockExecutor, VMValidator,
 };
 use anyhow::anyhow;
 use aptos_block_executor::txn_commit_hook::NoOpTransactionCommitHook;
@@ -2766,14 +2766,27 @@ impl AptosVM {
     }
 }
 
+// TODO - move out from this file?
+pub struct AptosVMBlockExecutor;
+
 // Executor external API
-impl VMExecutor for AptosVM {
+impl VMBlockExecutor for AptosVMBlockExecutor {
+    // NOTE: At the moment there are no persistent caches that live past the end of a block (that's
+    // why AptosVMBlockExecutor has no state)
+    // There are some cache invalidation issues around transactions publishing code that need to be
+    // sorted out before that's possible.
+
+    fn new() -> Self {
+        Self
+    }
+
     /// Execute a block of `transactions`. The output vector will have the exact same length as the
     /// input vector. The discarded transactions will be marked as `TransactionStatus::Discard` and
     /// have an empty `WriteSet`. Also `state_view` is immutable, and does not have interior
     /// mutability. Writes to be applied to the data view are encoded in the write set part of a
     /// transaction output.
     fn execute_block(
+        &self,
         transactions: &[SignatureVerifiedTransaction],
         state_view: &(impl StateView + Sync),
         onchain_config: BlockExecutorConfigFromOnchain,
@@ -2800,9 +2813,9 @@ impl VMExecutor for AptosVM {
             state_view,
             BlockExecutorConfig {
                 local: BlockExecutorLocalConfig {
-                    concurrency_level: Self::get_concurrency_level(),
+                    concurrency_level: AptosVM::get_concurrency_level(),
                     allow_fallback: true,
-                    discard_failed_blocks: Self::get_discard_failed_blocks(),
+                    discard_failed_blocks: AptosVM::get_discard_failed_blocks(),
                 },
                 onchain: onchain_config,
             },

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

@@ -153,14 +153,15 @@ pub trait VMValidator {
 }
 
 /// This trait describes the VM's execution interface.
-pub trait VMExecutor: Send + Sync {
-    // NOTE: At the moment there are no persistent caches that live past the end of a block (that's
-    // why execute_block doesn't take &self.)
-    // There are some cache invalidation issues around transactions publishing code that need to be
-    // sorted out before that's possible.
+pub trait VMBlockExecutor: Send + Sync {
+    /// Be careful if any state is kept in VMBlockExecutor, as all validations are implementers responsibility
+    /// (and state_view passed in execute_block can go both backwards and forwards in time).
+    /// TODO: Currently, production uses new() on every block, and only executor-benchmark reuses across.
+    fn new() -> Self;
 
     /// Executes a block of transactions and returns output for each one of them.
     fn execute_block(
+        &self,
         transactions: &[SignatureVerifiedTransaction],
         state_view: &(impl StateView + Sync),
         onchain_config: BlockExecutorConfigFromOnchain,
@@ -169,10 +170,11 @@ pub trait VMExecutor: Send + Sync {
     /// Executes a block of transactions and returns output for each one of them,
     /// Without applying any block limit
     fn execute_block_no_limit(
+        &self,
         transactions: &[SignatureVerifiedTransaction],
         state_view: &(impl StateView + Sync),
     ) -> Result<Vec<TransactionOutput>, VMStatus> {
-        Self::execute_block(
+        self.execute_block(
             transactions,
             state_view,
             BlockExecutorConfigFromOnchain::new_no_block_limit(),

aptos-move/aptos-vm/tests/sharded_block_executor.rs

@@ -202,8 +202,9 @@ mod test_utils {
         },
     };
     use aptos_vm::{
+        aptos_vm::AptosVMBlockExecutor,
         sharded_block_executor::{executor_client::ExecutorClient, ShardedBlockExecutor},
-        AptosVM, VMExecutor,
+        VMBlockExecutor,
     };
     use move_core_types::account_address::AccountAddress;
     use rand::{rngs::OsRng, Rng};
@@ -307,8 +308,9 @@ mod test_utils {
                 .into_iter()
                 .map(|t| t.into_txn())
                 .collect();
-        let unsharded_txn_output =
-            AptosVM::execute_block_no_limit(&ordered_txns, executor.data_store()).unwrap();
+        let unsharded_txn_output = AptosVMBlockExecutor
+            .execute_block_no_limit(&ordered_txns, executor.data_store())
+            .unwrap();
         compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
     }
 
@@ -356,9 +358,9 @@ mod test_utils {
             )
             .unwrap();
 
-        let unsharded_txn_output =
-            AptosVM::execute_block_no_limit(&execution_ordered_txns, executor.data_store())
-                .unwrap();
+        let unsharded_txn_output = AptosVMBlockExecutor
+            .execute_block_no_limit(&execution_ordered_txns, executor.data_store())
+            .unwrap();
         compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
     }
 
@@ -410,9 +412,9 @@ mod test_utils {
             )
             .unwrap();
 
-        let unsharded_txn_output =
-            AptosVM::execute_block_no_limit(&execution_ordered_txns, executor.data_store())
-                .unwrap();
+        let unsharded_txn_output = AptosVMBlockExecutor
+            .execute_block_no_limit(&execution_ordered_txns, executor.data_store())
+            .unwrap();
         compare_txn_outputs(unsharded_txn_output, sharded_txn_output);
     }
 }

aptos-node/src/state_sync.rs

@@ -33,7 +33,7 @@ use aptos_storage_service_server::{
 use aptos_storage_service_types::StorageServiceMessage;
 use aptos_time_service::TimeService;
 use aptos_types::waypoint::Waypoint;
-use aptos_vm::AptosVM;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use std::sync::Arc;
 use tokio::runtime::Runtime;
 
@@ -152,7 +152,7 @@ pub fn start_state_sync_and_get_notification_handles(
         setup_data_streaming_service(state_sync_config, aptos_data_client.clone())?;
 
     // Create the chunk executor and persistent storage
-    let chunk_executor = Arc::new(ChunkExecutor::<AptosVM>::new(db_rw.clone()));
+    let chunk_executor = Arc::new(ChunkExecutor::<AptosVMBlockExecutor>::new(db_rw.clone()));
     let metadata_storage = PersistentMetadataStorage::new(&node_config.storage.dir());
 
     // Create notification senders and listeners for mempool, consensus and the storage service

aptos-node/src/storage.rs

@@ -13,7 +13,7 @@ use aptos_storage_interface::{DbReader, DbReaderWriter};
 use aptos_types::{
     ledger_info::LedgerInfoWithSignatures, transaction::Version, waypoint::Waypoint,
 };
-use aptos_vm::AptosVM;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use either::Either;
 use std::{fs, path::Path, sync::Arc, time::Instant};
 use tokio::{
@@ -33,8 +33,9 @@ pub(crate) fn maybe_apply_genesis(
         .unwrap_or(&node_config.base.waypoint)
         .genesis_waypoint();
     if let Some(genesis) = get_genesis_txn(node_config) {
-        let ledger_info_opt = maybe_bootstrap::<AptosVM>(db_rw, genesis, genesis_waypoint)
-            .map_err(|err| anyhow!("DB failed to bootstrap {}", err))?;
+        let ledger_info_opt =
+            maybe_bootstrap::<AptosVMBlockExecutor>(db_rw, genesis, genesis_waypoint)
+                .map_err(|err| anyhow!("DB failed to bootstrap {}", err))?;
         Ok(ledger_info_opt)
     } else {
         info ! ("Genesis txn not provided! This is fine only if you don't expect to apply it. Otherwise, the config is incorrect!");

consensus/src/consensus_provider.rs

@@ -29,13 +29,14 @@ use aptos_channels::aptos_channel::Receiver;
 use aptos_config::config::NodeConfig;
 use aptos_consensus_notifications::ConsensusNotificationSender;
 use aptos_event_notifications::{DbBackedOnChainConfig, ReconfigNotificationListener};
-use aptos_executor::block_executor::{AptosVMBlockExecutor, BlockExecutor};
+use aptos_executor::block_executor::BlockExecutor;
 use aptos_logger::prelude::*;
 use aptos_mempool::QuorumStoreRequest;
 use aptos_network::application::interface::{NetworkClient, NetworkServiceEvents};
 use aptos_storage_interface::DbReaderWriter;
 use aptos_time_service::TimeService;
 use aptos_validator_transaction_pool::VTxnPoolState;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use futures::channel::mpsc;
 use move_core_types::account_address::AccountAddress;
 use std::{collections::HashMap, sync::Arc};

crates/aptos-genesis/src/lib.rs

@@ -30,7 +30,7 @@ use aptos_types::{
     transaction::Transaction,
     waypoint::Waypoint,
 };
-use aptos_vm::AptosVM;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use aptos_vm_genesis::Validator;
 use std::convert::TryInto;
 
@@ -171,6 +171,6 @@ impl GenesisInfo {
             None,
         )?;
         let db_rw = DbReaderWriter::new(aptosdb);
-        aptos_executor::db_bootstrapper::generate_waypoint::<AptosVM>(&db_rw, genesis)
+        aptos_executor::db_bootstrapper::generate_waypoint::<AptosVMBlockExecutor>(&db_rw, genesis)
     }
 }

crates/aptos-genesis/src/mainnet.rs

@@ -16,7 +16,7 @@ use aptos_types::{
     transaction::Transaction,
     waypoint::Waypoint,
 };
-use aptos_vm::AptosVM;
+use aptos_vm::aptos_vm::AptosVMBlockExecutor;
 use aptos_vm_genesis::{AccountBalance, EmployeePool, ValidatorWithCommissionRate};
 
 /// Holder object for all pieces needed to generate a genesis transaction
@@ -161,6 +161,6 @@ impl MainnetGenesisInfo {
             None,
         )?;
         let db_rw = DbReaderWriter::new(aptosdb);
-        aptos_executor::db_bootstrapper::generate_waypoint::<AptosVM>(&db_rw, genesis)
+        aptos_executor::db_bootstrapper::generate_waypoint::<AptosVMBlockExecutor>(&db_rw, genesis)
     }
 }

execution/executor-benchmark/src/db_generator.rs

@@ -11,13 +11,10 @@ use aptos_config::{
     utils::get_genesis_txn,
 };
 use aptos_db::AptosDB;
-use aptos_executor::{
-    block_executor::TransactionBlockExecutor,
-    db_bootstrapper::{generate_waypoint, maybe_bootstrap},
-};
+use aptos_executor::db_bootstrapper::{generate_waypoint, maybe_bootstrap};
 use aptos_storage_interface::DbReaderWriter;
 use aptos_types::on_chain_config::Features;
-use aptos_vm::AptosVM;
+use aptos_vm::{aptos_vm::AptosVMBlockExecutor, VMBlockExecutor};
 use std::{fs, path::Path};
 
 pub fn create_db_with_accounts<V>(
@@ -31,7 +28,7 @@ pub fn create_db_with_accounts<V>(
     pipeline_config: PipelineConfig,
     init_features: Features,
 ) where
-    V: TransactionBlockExecutor + 'static,
+    V: VMBlockExecutor + 'static,
 {
     println!("Initializing...");
 
@@ -88,6 +85,9 @@ pub(crate) fn bootstrap_with_genesis(
     );
 
     // Bootstrap db with genesis
-    let waypoint = generate_waypoint::<AptosVM>(&db_rw, get_genesis_txn(&config).unwrap()).unwrap();
-    maybe_bootstrap::<AptosVM>(&db_rw, get_genesis_txn(&config).unwrap(), waypoint).unwrap();
+    let waypoint =
+        generate_waypoint::<AptosVMBlockExecutor>(&db_rw, get_genesis_txn(&config).unwrap())
+            .unwrap();
+    maybe_bootstrap::<AptosVMBlockExecutor>(&db_rw, get_genesis_txn(&config).unwrap(), waypoint)
+        .unwrap();
 }

execution/executor-benchmark/src/ledger_update_stage.rs

@@ -3,8 +3,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::pipeline::{CommitBlockMessage, LedgerUpdateMessage};
-use aptos_executor::block_executor::{BlockExecutor, TransactionBlockExecutor};
+use aptos_executor::block_executor::BlockExecutor;
 use aptos_executor_types::BlockExecutorTrait;
+use aptos_vm::VMBlockExecutor;
 use std::sync::{mpsc, Arc};
 
 pub enum CommitProcessing {
@@ -24,7 +25,7 @@ pub struct LedgerUpdateStage<V> {
 
 impl<V> LedgerUpdateStage<V>
 where
-    V: TransactionBlockExecutor,
+    V: VMBlockExecutor,
 {
     pub fn new(
         executor: Arc<BlockExecutor<V>>,