From 10c82812bb6a6c4ab6594953d8c0e666c9e8b554 Mon Sep 17 00:00:00 2001 From: Rati Date: Fri, 17 Jan 2025 07:20:07 +0400 Subject: [PATCH] test, rename to interrupt requested --- aptos-move/aptos-gas-meter/src/algebra.rs | 37 ++++++++++++++++--- aptos-move/aptos-vm-types/src/resolver.rs | 22 ++++++++++- aptos-move/aptos-vm/src/aptos_vm.rs | 27 ++++++++++---- aptos-move/aptos-vm/src/data_cache.rs | 7 +++- aptos-move/aptos-vm/src/gas.rs | 9 ++++- .../aptos-vm/src/move_vm_ext/resolver.rs | 6 ++- .../session/view_with_change_set.rs | 10 ++++- aptos-move/aptos-vm/src/testing.rs | 1 + .../src/proptest_types/baseline.rs | 1 + .../src/proptest_types/types.rs | 8 ++++ aptos-move/block-executor/src/scheduler.rs | 4 ++ .../block-executor/src/unit_tests/mod.rs | 32 ++++++++++++++++ aptos-move/block-executor/src/view.rs | 14 ++++++- aptos-move/e2e-tests/src/executor.rs | 2 + 14 files changed, 157 insertions(+), 23 deletions(-) diff --git a/aptos-move/aptos-gas-meter/src/algebra.rs b/aptos-move/aptos-gas-meter/src/algebra.rs index adc4d8697d760..2e0c73b65e8da 100644 --- a/aptos-move/aptos-gas-meter/src/algebra.rs +++ b/aptos-move/aptos-gas-meter/src/algebra.rs @@ -5,8 +5,9 @@ use crate::traits::GasAlgebra; use aptos_gas_algebra::{Fee, FeePerGasUnit, Gas, GasExpression, NumBytes, NumModules, Octa}; use aptos_gas_schedule::{gas_feature_versions, VMGasParameters}; use aptos_logger::error; -use aptos_vm_types::storage::{ - io_pricing::IoPricing, space_pricing::DiskSpacePricing, StorageGasParameters, +use aptos_vm_types::{ + resolver::BlockSynchronizationKillSwitch, + storage::{io_pricing::IoPricing, space_pricing::DiskSpacePricing, StorageGasParameters}, }; use move_binary_format::errors::{PartialVMError, PartialVMResult}; use move_core_types::{ @@ -18,7 +19,7 @@ use std::fmt::Debug; /// Base gas algebra implementation that tracks the gas usage using its internal counters. /// /// Abstract gas amounts are always evaluated to concrete values at the spot. -pub struct StandardGasAlgebra { +pub struct StandardGasAlgebra<'a> { feature_version: u64, vm_gas_params: VMGasParameters, storage_gas_params: StorageGasParameters, @@ -40,15 +41,18 @@ pub struct StandardGasAlgebra { num_dependencies: NumModules, total_dependency_size: NumBytes, + + maybe_block_synchronization_view: Option<&'a dyn BlockSynchronizationKillSwitch>, } -impl StandardGasAlgebra { +impl<'a> StandardGasAlgebra<'a> { pub fn new( gas_feature_version: u64, vm_gas_params: VMGasParameters, storage_gas_params: StorageGasParameters, is_approved_gov_script: bool, balance: impl Into, + maybe_block_synchronization_view: Option<&'a dyn BlockSynchronizationKillSwitch>, ) -> Self { let balance = balance.into().to_unit_with_params(&vm_gas_params.txn); @@ -83,11 +87,12 @@ impl StandardGasAlgebra { storage_fee_used: 0.into(), num_dependencies: 0.into(), total_dependency_size: 0.into(), + maybe_block_synchronization_view, } } } -impl StandardGasAlgebra { +impl StandardGasAlgebra<'_> { fn charge(&mut self, amount: InternalGas) -> (InternalGas, PartialVMResult<()>) { match self.balance.checked_sub(amount) { Some(new_balance) => { @@ -106,7 +111,7 @@ impl StandardGasAlgebra { } } -impl GasAlgebra for StandardGasAlgebra { +impl GasAlgebra for StandardGasAlgebra<'_> { fn feature_version(&self) -> u64 { self.feature_version } @@ -165,6 +170,16 @@ impl GasAlgebra for StandardGasAlgebra { &mut self, abstract_amount: impl GasExpression + Debug, ) -> PartialVMResult<()> { + if self + .maybe_block_synchronization_view + .is_some_and(|view| view.interrupt_requested()) + { + return Err( + PartialVMError::new(StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR) + .with_message("Interrupted from block synchronization view".to_string()), + ); + } + let amount = abstract_amount.evaluate(self.feature_version, &self.vm_gas_params); let (actual, res) = self.charge(amount); @@ -187,6 +202,16 @@ impl GasAlgebra for StandardGasAlgebra { &mut self, abstract_amount: impl GasExpression, ) -> PartialVMResult<()> { + if self + .maybe_block_synchronization_view + .is_some_and(|view| view.interrupt_requested()) + { + return Err( + PartialVMError::new(StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR) + .with_message("Interrupted from block synchronization view".to_string()), + ); + } + let amount = abstract_amount.evaluate(self.feature_version, &self.vm_gas_params); let (actual, res) = self.charge(amount); diff --git a/aptos-move/aptos-vm-types/src/resolver.rs b/aptos-move/aptos-vm-types/src/resolver.rs index 38bcef0f69a24..41ce6d9511d5d 100644 --- a/aptos-move/aptos-vm-types/src/resolver.rs +++ b/aptos-move/aptos-vm-types/src/resolver.rs @@ -19,6 +19,13 @@ use move_core_types::{language_storage::StructTag, value::MoveTypeLayout, vm_sta use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID; use std::collections::{BTreeMap, HashMap}; +/// Allows requesting an immediate interrupt to ongoing transaction execution. For example, this +/// allows an early return from a useless speculative execution when block execution has already +/// halted (e.g. due to gas limit, committing only a block prefix). +pub trait BlockSynchronizationKillSwitch { + fn interrupt_requested(&self) -> bool; +} + /// Allows to query resources from the state. pub trait TResourceView { type Key; @@ -204,7 +211,8 @@ pub trait StateStorageView { /// resolve AggregatorV2 via the state-view based default implementation, as it /// doesn't provide a value exchange functionality). pub trait TExecutorView: - TResourceView + BlockSynchronizationKillSwitch + + TResourceView + TModuleView + TAggregatorV1View + TDelayedFieldView @@ -213,7 +221,8 @@ pub trait TExecutorView: } impl TExecutorView for A where - A: TResourceView + A: BlockSynchronizationKillSwitch + + TResourceView + TModuleView + TAggregatorV1View + TDelayedFieldView @@ -293,6 +302,15 @@ where } } +impl BlockSynchronizationKillSwitch for S +where + S: StateView, +{ + fn interrupt_requested(&self) -> bool { + false + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ResourceGroupSize { Concrete(u64), diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 6a9eee602f73e..e97ff65e788f5 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -89,7 +89,7 @@ use aptos_vm_types::{ }, module_write_set::ModuleWriteSet, output::VMOutput, - resolver::{ExecutorView, ResourceGroupView}, + resolver::{BlockSynchronizationKillSwitch, ExecutorView, ResourceGroupView}, storage::{change_set_configs::ChangeSetConfigs, StorageGasParameters}, }; use ark_bn254::Bn254; @@ -2038,9 +2038,9 @@ impl AptosVM { /// Main entrypoint for executing a user transaction that also allows the customization of the /// gas meter to be used. - pub fn execute_user_transaction_with_custom_gas_meter( + pub fn execute_user_transaction_with_custom_gas_meter<'a, G, F>( &self, - resolver: &impl AptosMoveResolver, + resolver: &'a impl AptosMoveResolver, code_storage: &impl AptosCodeStorage, txn: &SignedTransaction, log_context: &AdapterLogSchema, @@ -2048,7 +2048,14 @@ impl AptosVM { ) -> Result<(VMStatus, VMOutput, G), VMStatus> where G: AptosGasMeter, - F: FnOnce(u64, VMGasParameters, StorageGasParameters, bool, Gas) -> G, + F: FnOnce( + u64, + VMGasParameters, + StorageGasParameters, + bool, + Gas, + Option<&'a dyn BlockSynchronizationKillSwitch>, + ) -> G, { let txn_metadata = TransactionMetadata::new(txn); @@ -2061,6 +2068,7 @@ impl AptosVM { self.storage_gas_params(log_context)?.clone(), is_approved_gov_script, balance, + Some(resolver.as_block_synchronization_kill_switch()), ); let (status, output) = self.execute_user_transaction_impl( resolver, @@ -2080,16 +2088,16 @@ impl AptosVM { /// /// This can be useful for off-chain applications that wants to perform additional /// measurements or analysis while preserving the production gas behavior. - pub fn execute_user_transaction_with_modified_gas_meter( + pub fn execute_user_transaction_with_modified_gas_meter<'a, G, F>( &self, - resolver: &impl AptosMoveResolver, + resolver: &'a impl AptosMoveResolver, code_storage: &impl AptosCodeStorage, txn: &SignedTransaction, log_context: &AdapterLogSchema, modify_gas_meter: F, ) -> Result<(VMStatus, VMOutput, G), VMStatus> where - F: FnOnce(ProdGasMeter) -> G, + F: FnOnce(ProdGasMeter<'a>) -> G, G: AptosGasMeter, { self.execute_user_transaction_with_custom_gas_meter( @@ -2101,13 +2109,15 @@ impl AptosVM { vm_gas_params, storage_gas_params, is_approved_gov_script, - meter_balance| { + meter_balance, + _maybe_block_synchronization_kill_switch| { modify_gas_meter(make_prod_gas_meter( gas_feature_version, vm_gas_params, storage_gas_params, is_approved_gov_script, meter_balance, + None, // No block synchronization kill switch )) }, ) @@ -2478,6 +2488,7 @@ impl AptosVM { storage_gas_params, /* is_approved_gov_script */ false, max_gas_amount.into(), + None, // No block synchronization kill switch ); let resolver = state_view.as_move_resolver(); diff --git a/aptos-move/aptos-vm/src/data_cache.rs b/aptos-move/aptos-vm/src/data_cache.rs index 11da1abc1a9ee..1c516a14cbd92 100644 --- a/aptos-move/aptos-vm/src/data_cache.rs +++ b/aptos-move/aptos-vm/src/data_cache.rs @@ -29,7 +29,8 @@ use aptos_vm_environment::{ }; use aptos_vm_types::{ resolver::{ - ExecutorView, ResourceGroupSize, ResourceGroupView, StateStorageView, TResourceGroupView, + BlockSynchronizationKillSwitch, ExecutorView, ResourceGroupSize, ResourceGroupView, + StateStorageView, TResourceGroupView, }, resource_group_adapter::ResourceGroupAdapter, }; @@ -332,6 +333,10 @@ impl<'e, E: ExecutorView> AsExecutorView for StorageAdapter<'e, E> { fn as_executor_view(&self) -> &dyn ExecutorView { self.executor_view } + + fn as_block_synchronization_kill_switch(&self) -> &dyn BlockSynchronizationKillSwitch { + self.executor_view + } } // Allows to extract the view from `StorageAdapter`. diff --git a/aptos-move/aptos-vm/src/gas.rs b/aptos-move/aptos-vm/src/gas.rs index 26d9543e1e846..cc5628fa24da2 100644 --- a/aptos-move/aptos-vm/src/gas.rs +++ b/aptos-move/aptos-vm/src/gas.rs @@ -12,7 +12,10 @@ use aptos_logger::{enabled, Level}; use aptos_memory_usage_tracker::MemoryTrackedGasMeter; use aptos_types::on_chain_config::Features; use aptos_vm_logging::{log_schema::AdapterLogSchema, speculative_log, speculative_warn}; -use aptos_vm_types::storage::{space_pricing::DiskSpacePricing, StorageGasParameters}; +use aptos_vm_types::{ + resolver::BlockSynchronizationKillSwitch, + storage::{space_pricing::DiskSpacePricing, StorageGasParameters}, +}; use move_core_types::vm_status::{StatusCode, VMStatus}; use move_vm_runtime::ModuleStorage; @@ -20,7 +23,7 @@ use move_vm_runtime::ModuleStorage; const MAXIMUM_APPROVED_TRANSACTION_SIZE_LEGACY: u64 = 1024 * 1024; /// Gas meter used in the production (validator) setup. -pub type ProdGasMeter = MemoryTrackedGasMeter>; +pub type ProdGasMeter<'a> = MemoryTrackedGasMeter>>; /// Creates a gas meter intended for executing transactions in the production. /// @@ -31,6 +34,7 @@ pub fn make_prod_gas_meter( storage_gas_params: StorageGasParameters, is_approved_gov_script: bool, meter_balance: Gas, + maybe_block_synchronization_kill_switch: Option<&dyn BlockSynchronizationKillSwitch>, ) -> ProdGasMeter { MemoryTrackedGasMeter::new(StandardGasMeter::new(StandardGasAlgebra::new( gas_feature_version, @@ -38,6 +42,7 @@ pub fn make_prod_gas_meter( storage_gas_params, is_approved_gov_script, meter_balance, + maybe_block_synchronization_kill_switch, ))) } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs b/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs index 938a75be430c0..770d57b56910c 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs @@ -5,7 +5,8 @@ use aptos_aggregator::resolver::{AggregatorV1Resolver, DelayedFieldResolver}; use aptos_table_natives::TableResolver; use aptos_types::{on_chain_config::ConfigStorage, state_store::state_key::StateKey}; use aptos_vm_types::resolver::{ - ExecutorView, ResourceGroupSize, ResourceGroupView, StateStorageView, + BlockSynchronizationKillSwitch, ExecutorView, ResourceGroupSize, ResourceGroupView, + StateStorageView, }; use bytes::Bytes; use move_binary_format::errors::PartialVMResult; @@ -51,6 +52,9 @@ pub trait ResourceGroupResolver { pub trait AsExecutorView { fn as_executor_view(&self) -> &dyn ExecutorView; + + // TODO: remove once trait upcasting coercion is stabilized (rust issue #65991). + fn as_block_synchronization_kill_switch(&self) -> &dyn BlockSynchronizationKillSwitch; } pub trait AsResourceGroupView { diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session/view_with_change_set.rs b/aptos-move/aptos-vm/src/move_vm_ext/session/view_with_change_set.rs index a6309ce35af70..ce57710dec25f 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session/view_with_change_set.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session/view_with_change_set.rs @@ -23,8 +23,8 @@ use aptos_vm_types::{ abstract_write_op::{AbstractResourceWriteOp, WriteWithDelayedFieldsOp}, change_set::{randomly_check_layout_matches, VMChangeSet}, resolver::{ - ExecutorView, ResourceGroupSize, ResourceGroupView, StateStorageView, TModuleView, - TResourceGroupView, TResourceView, + BlockSynchronizationKillSwitch, ExecutorView, ResourceGroupSize, ResourceGroupView, + StateStorageView, TModuleView, TResourceGroupView, TResourceView, }, }; use bytes::Bytes; @@ -176,6 +176,12 @@ impl<'r> TDelayedFieldView for ExecutorViewWithChangeSet<'r> { } } +impl<'r> BlockSynchronizationKillSwitch for ExecutorViewWithChangeSet<'r> { + fn interrupt_requested(&self) -> bool { + self.base_executor_view.interrupt_requested() + } +} + impl<'r> TResourceView for ExecutorViewWithChangeSet<'r> { type Key = StateKey; type Layout = MoveTypeLayout; diff --git a/aptos-move/aptos-vm/src/testing.rs b/aptos-move/aptos-vm/src/testing.rs index 2ee83eea33d1f..6a00acc666a19 100644 --- a/aptos-move/aptos-vm/src/testing.rs +++ b/aptos-move/aptos-vm/src/testing.rs @@ -94,6 +94,7 @@ impl AptosVM { storage_gas_params, false, gas_meter_balance.into(), + None, ); let change_set_configs = &self diff --git a/aptos-move/block-executor/src/proptest_types/baseline.rs b/aptos-move/block-executor/src/proptest_types/baseline.rs index b2d72577fa3c0..890f8b9ad0a8e 100644 --- a/aptos-move/block-executor/src/proptest_types/baseline.rs +++ b/aptos-move/block-executor/src/proptest_types/baseline.rs @@ -213,6 +213,7 @@ impl BaselineOutput { }, } }, + MockTransaction::InterruptRequested => unreachable!("Not tested with outputs"), } } diff --git a/aptos-move/block-executor/src/proptest_types/types.rs b/aptos-move/block-executor/src/proptest_types/types.rs index bc5ee4395e11d..fae9f81c3337f 100644 --- a/aptos-move/block-executor/src/proptest_types/types.rs +++ b/aptos-move/block-executor/src/proptest_types/types.rs @@ -392,6 +392,7 @@ impl MockIncarnation { /// value determines the index for choosing the read & write sets of the particular execution. #[derive(Clone, Debug)] pub(crate) enum MockTransaction { + InterruptRequested, Write { /// Incarnation counter, increased during each mock (re-)execution. Allows tracking the final /// incarnation for each mock transaction, whose behavior should be reproduced for baseline. @@ -430,6 +431,9 @@ impl MockTransaction { } => incarnation_behaviors, Self::SkipRest(_) => unreachable!("SkipRest does not contain incarnation behaviors"), Self::Abort => unreachable!("Abort does not contain incarnation behaviors"), + Self::InterruptRequested => { + unreachable!("InterruptRequested does not contain incarnation behaviors") + }, } } } @@ -1038,6 +1042,10 @@ where ExecutionStatus::SkipRest(mock_output) }, MockTransaction::Abort => ExecutionStatus::Abort(txn_idx as usize), + MockTransaction::InterruptRequested => { + while !view.interrupt_requested() {} + ExecutionStatus::SkipRest(MockOutput::skip_output()) + }, } } diff --git a/aptos-move/block-executor/src/scheduler.rs b/aptos-move/block-executor/src/scheduler.rs index 0d27eb94ba8f5..918d0c722334a 100644 --- a/aptos-move/block-executor/src/scheduler.rs +++ b/aptos-move/block-executor/src/scheduler.rs @@ -652,6 +652,10 @@ impl Scheduler { !self.has_halted.swap(true, Ordering::SeqCst) } + + pub(crate) fn has_halted(&self) -> bool { + self.has_halted.load(Ordering::Relaxed) + } } impl TWaitForDependency for Scheduler { diff --git a/aptos-move/block-executor/src/unit_tests/mod.rs b/aptos-move/block-executor/src/unit_tests/mod.rs index 73c4e4a44d16f..b2d03b30f6ea2 100644 --- a/aptos-move/block-executor/src/unit_tests/mod.rs +++ b/aptos-move/block-executor/src/unit_tests/mod.rs @@ -227,6 +227,38 @@ fn resource_group_bcs_fallback() { scenario.teardown(); } +#[test] +fn interrupt_requested() { + let transactions = Vec::from([MockTransaction::Abort, MockTransaction::InterruptRequested]); + let txn_provider = DefaultTxnProvider::new(transactions); + let mut guard = AptosModuleCacheManagerGuard::none(); + + let data_view = DeltaDataView::> { + phantom: PhantomData, + }; + let executor_thread_pool = Arc::new( + rayon::ThreadPoolBuilder::new() + .num_threads(num_cpus::get()) + .build() + .unwrap(), + ); + let block_executor = BlockExecutor::< + MockTransaction, MockEvent>, + MockTask, MockEvent>, + DeltaDataView>, + NoOpTransactionCommitHook, MockEvent>, usize>, + DefaultTxnProvider, MockEvent>>, + >::new( + BlockExecutorConfig::new_no_block_limit(num_cpus::get()), + executor_thread_pool, + None, + ); + + // MockTransaction::InterruptRequested will only return if interrupt is requested (here, due + // to abort from the first transaction). O.w. the test will hang. + let _ = block_executor.execute_transactions_parallel(&txn_provider, &data_view, &mut guard); +} + #[test] fn block_output_err_precedence() { let incarnation: MockIncarnation, MockEvent> = MockIncarnation::new( diff --git a/aptos-move/block-executor/src/view.rs b/aptos-move/block-executor/src/view.rs index 889e905e49c79..f07795bd601a4 100644 --- a/aptos-move/block-executor/src/view.rs +++ b/aptos-move/block-executor/src/view.rs @@ -46,7 +46,8 @@ use aptos_types::{ }; use aptos_vm_logging::{log_schema::AdapterLogSchema, prelude::*}; use aptos_vm_types::resolver::{ - ResourceGroupSize, StateStorageView, TModuleView, TResourceGroupView, TResourceView, + BlockSynchronizationKillSwitch, ResourceGroupSize, StateStorageView, TModuleView, + TResourceGroupView, TResourceView, }; use bytes::Bytes; use claims::assert_ok; @@ -1449,6 +1450,17 @@ impl<'a, T: Transaction, S: TStateView> LatestView<'a, T, S> { } } +impl<'a, T: Transaction, S: TStateView> BlockSynchronizationKillSwitch + for LatestView<'a, T, S> +{ + fn interrupt_requested(&self) -> bool { + match &self.latest_view { + ViewState::Sync(state) => state.scheduler.has_halted(), + ViewState::Unsync(_) => false, + } + } +} + impl<'a, T: Transaction, S: TStateView> TResourceView for LatestView<'a, T, S> { type Key = T::Key; type Layout = MoveTypeLayout; diff --git a/aptos-move/e2e-tests/src/executor.rs b/aptos-move/e2e-tests/src/executor.rs index c2039a589d6d6..69427013b3af0 100644 --- a/aptos-move/e2e-tests/src/executor.rs +++ b/aptos-move/e2e-tests/src/executor.rs @@ -1072,6 +1072,7 @@ impl FakeExecutor { env.storage_gas_params().as_ref().unwrap().clone(), false, 1_000_000_000_000_000.into(), + None, )), None, ), @@ -1182,6 +1183,7 @@ impl FakeExecutor { env.storage_gas_params().as_ref().unwrap().clone(), false, 10_000_000_000_000, + None, ), shared_buffer: Arc::clone(&a1), }),