From 7927905e43ab61f77217037d4f00db8d46025184 Mon Sep 17 00:00:00 2001 From: welbon Date: Mon, 26 Jun 2023 18:07:21 +0800 Subject: [PATCH 1/3] [starcoin vm] Add vm_executor and vm_validator --- executor/src/block_executor.rs | 4 +- executor/src/executor.rs | 3 +- vm/types/src/transaction/mod.rs | 49 +++++++++++++ vm/vm-runtime/src/lib.rs | 39 +++++++++++ vm/vm-runtime/src/starcoin_vm.rs | 116 ++++++++++++++++++++++++++----- 5 files changed, 190 insertions(+), 21 deletions(-) diff --git a/executor/src/block_executor.rs b/executor/src/block_executor.rs index 6318b6dbe4..d9d7bf1ed5 100644 --- a/executor/src/block_executor.rs +++ b/executor/src/block_executor.rs @@ -8,6 +8,8 @@ use starcoin_types::error::ExecutorResult; use starcoin_types::transaction::TransactionStatus; use starcoin_types::transaction::{Transaction, TransactionInfo}; use starcoin_vm_runtime::metrics::VMMetrics; +use starcoin_vm_runtime::starcoin_vm::StarcoinVM; +use starcoin_vm_runtime::VMExecutor; use starcoin_vm_types::contract_event::ContractEvent; #[derive(Clone, Debug, Eq, PartialEq)] @@ -34,7 +36,7 @@ pub fn block_execute( vm_metrics: Option, ) -> ExecutorResult { let txn_outputs = - crate::execute_block_transactions(chain_state, txns.clone(), block_gas_limit, vm_metrics) + StarcoinVM::execute_block(chain_state, txns.clone(), block_gas_limit, vm_metrics) .map_err(BlockExecutorError::BlockTransactionExecuteErr)?; let mut executed_data = BlockExecutedData::default(); diff --git a/executor/src/executor.rs b/executor/src/executor.rs index fcf89a777e..2872a9ada7 100644 --- a/executor/src/executor.rs +++ b/executor/src/executor.rs @@ -5,6 +5,7 @@ use anyhow::Result; use starcoin_types::transaction::{SignedUserTransaction, Transaction, TransactionOutput}; use starcoin_vm_runtime::metrics::VMMetrics; use starcoin_vm_runtime::starcoin_vm::StarcoinVM; +use starcoin_vm_runtime::VMValidator; use starcoin_vm_types::identifier::Identifier; use starcoin_vm_types::language_storage::{ModuleId, TypeTag}; use starcoin_vm_types::{state_view::StateView, vm_status::VMStatus}; @@ -52,7 +53,7 @@ pub fn validate_transaction( metrics: Option, ) -> Option { let mut vm = StarcoinVM::new(metrics); - vm.verify_transaction(chain_state, txn) + vm.validate_transaction(txn, chain_state) } pub fn execute_readonly_function( diff --git a/vm/types/src/transaction/mod.rs b/vm/types/src/transaction/mod.rs index 34de8b6ec4..744ab119a9 100644 --- a/vm/types/src/transaction/mod.rs +++ b/vm/types/src/transaction/mod.rs @@ -43,6 +43,7 @@ pub use script::{ }; use starcoin_crypto::hash::SPARSE_MERKLE_PLACEHOLDER_HASH; use std::str::FromStr; +use move_core_types::vm_status::StatusType; pub use transaction_argument::{ parse_transaction_argument, parse_transaction_arguments, TransactionArgument, }; @@ -913,3 +914,51 @@ impl std::fmt::Display for TxStatus { write!(f, "{}", s) } } + +/// The result of running the transaction through the VM validator. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct VMValidatorResult { + /// Result of the validation: `None` if the transaction was successfully validated + /// or `Some(DiscardedVMStatus)` if the transaction should be discarded. + status: Option, + + /// Score for ranking the transaction priority (e.g., based on the gas price). + /// Only used when the status is `None`. Higher values indicate a higher priority. + score: u64, +} + +impl VMValidatorResult { + pub fn new(vm_status: Option, score: u64) -> Self { + debug_assert!( + match vm_status { + None => true, + Some(status) => + status.status_type() == StatusType::Unknown + || status.status_type() == StatusType::Validation + || status.status_type() == StatusType::InvariantViolation, + }, + "Unexpected discarded status: {:?}", + vm_status + ); + Self { + status: vm_status, + score, + } + } + + pub fn error(vm_status: DiscardedVMStatus) -> Self { + Self { + status: Some(vm_status), + score: 0, + } + } + + pub fn status(&self) -> Option { + self.status + } + + pub fn score(&self) -> u64 { + self.score + } +} + diff --git a/vm/vm-runtime/src/lib.rs b/vm/vm-runtime/src/lib.rs index 7b3bcc534d..8ca78f8699 100644 --- a/vm/vm-runtime/src/lib.rs +++ b/vm/vm-runtime/src/lib.rs @@ -6,14 +6,53 @@ pub mod data_cache; pub mod metrics; pub mod natives; pub mod starcoin_vm; + +use move_core_types::vm_status::VMStatus; pub use move_vm_runtime::move_vm; pub use move_vm_runtime::session; + mod access_path_cache; mod errors; pub mod move_vm_ext; + use starcoin_vm_types::access_path::AccessPath; use starcoin_vm_types::account_address::AccountAddress; use starcoin_vm_types::language_storage::StructTag; +use starcoin_vm_types::state_view::StateView; +use starcoin_vm_types::transaction::{SignedUserTransaction, Transaction, TransactionOutput}; +use crate::metrics::VMMetrics; +use anyhow::Result; + + +/// This trait describes the VM's validation interfaces. +pub trait VMValidator { + /// Executes the prologue of the Aptos Account and verifies that the transaction is valid. + fn validate_transaction( + &mut self, + transaction: SignedUserTransaction, + state_view: &impl StateView, + ) -> Option; +} + +/// 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. + + /// Executes a block of transactions and returns output for each one of them. + // fn execute_block( + // transactions: Vec, + // state_view: &impl StateView, + // ) -> Result, VMStatus>; + fn execute_block( + chain_state: &S, + txns: Vec, + block_gas_limit: u64, + metrics: Option, + ) -> Result>; +} /// Get the AccessPath to a resource stored under `address` with type name `tag` fn create_access_path(address: AccountAddress, tag: StructTag) -> AccessPath { diff --git a/vm/vm-runtime/src/starcoin_vm.rs b/vm/vm-runtime/src/starcoin_vm.rs index 48a3088938..f1b220cf0a 100644 --- a/vm/vm-runtime/src/starcoin_vm.rs +++ b/vm/vm-runtime/src/starcoin_vm.rs @@ -67,6 +67,7 @@ use std::sync::Arc; #[cfg(feature = "metrics")] use crate::metrics::VMMetrics; +use crate::{VMExecutor, VMValidator}; #[derive(Clone)] #[allow(clippy::upper_case_acronyms)] @@ -371,7 +372,7 @@ impl StarcoinVM { // NB: MIN_PRICE_PER_GAS_UNIT may equal zero, but need not in the future. Hence why // we turn off the clippy warning. #[allow(clippy::absurd_extreme_comparisons)] - let below_min_bound = txn_data.gas_unit_price() < txn_gas_params.min_price_per_gas_unit; + let below_min_bound = txn_data.gas_unit_price() < txn_gas_params.min_price_per_gas_unit; if below_min_bound { warn!( "[VM] Gas unit error; min {}, submitted {}", @@ -394,7 +395,7 @@ impl StarcoinVM { } fn verify_transaction_impl( - &mut self, + &self, transaction: &SignatureCheckedTransaction, remote_cache: &StateViewCache, ) -> Result<(), VMStatus> { @@ -476,11 +477,11 @@ impl StarcoinVM { pub fn verify_transaction( &mut self, - state_view: &S, txn: SignedUserTransaction, + state_view: &S, ) -> Option { #[cfg(feature = "metrics")] - let _timer = self.metrics.as_ref().map(|metrics| { + let _timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&["verify_transaction"]) @@ -491,10 +492,12 @@ impl StarcoinVM { Ok(t) => t, Err(_) => return Some(VMStatus::Error(StatusCode::INVALID_SIGNATURE)), }; + if let Err(err) = self.load_configs(state_view) { warn!("Load config error at verify_transaction: {}", err); return Some(VMStatus::Error(StatusCode::VM_STARTUP_FAILURE)); } + match self.verify_transaction_impl(&signature_verified_txn, &data_cache) { Ok(_) => None, Err(err) => { @@ -604,7 +607,7 @@ impl StarcoinVM { .map(|m| m.code().to_vec()) .collect(), package.package_address(), // be careful with the sender. - gas_meter, + gas_meter, PublishModuleBundleOption { force_publish: enforced, only_new_module, @@ -705,11 +708,11 @@ impl StarcoinVM { return Err(VMStatus::Error(StatusCode::UNREACHABLE)); } } - .map_err(|e| - { - warn!("[VM] execute_script_function error, status_type: {:?}, status_code:{:?}, message:{:?}, location:{:?}", e.status_type(), e.major_status(), e.message(), e.location()); - e.into_vm_status() - })?; + .map_err(|e| + { + warn!("[VM] execute_script_function error, status_type: {:?}, status_code:{:?}, message:{:?}, location:{:?}", e.status_type(), e.major_status(), e.message(), e.location()); + e.into_vm_status() + })?; charge_global_write_gas_usage(gas_meter, &session, &txn_data.sender())?; self.success_transaction_cleanup(session, gas_meter, txn_data) @@ -1084,12 +1087,12 @@ impl StarcoinVM { let blocks = chunk_block_transactions(transactions); 'outer: for block in blocks { #[cfg(feature = "metrics")] - let txn_type_name = block.type_name().to_string(); + let txn_type_name = block.type_name().to_string(); match block { TransactionBlock::UserTransaction(txns) => { for transaction in txns { #[cfg(feature = "metrics")] - let timer = self.metrics.as_ref().map(|metrics| { + let timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&[txn_type_name.as_str()]) @@ -1138,7 +1141,7 @@ impl StarcoinVM { } TransactionBlock::BlockPrologue(block_metadata) => { #[cfg(feature = "metrics")] - let timer = self.metrics.as_ref().map(|metrics| { + let timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&[txn_type_name.as_str()]) @@ -1213,7 +1216,7 @@ impl StarcoinVM { check_gas: bool, ) -> Result>, VMStatus> { #[cfg(feature = "metrics")] - let _timer = self.metrics.as_ref().map(|metrics| { + let _timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&["execute_readonly_function"]) @@ -1270,7 +1273,7 @@ impl StarcoinVM { events, table_change_set, } - .into_change_set(&mut ())?; + .into_change_set(&mut ())?; if !write_set.is_empty() { warn!("Readonly function {} changes state", function_name); return Err(VMStatus::Error(StatusCode::REJECTED_WRITE_SET)); @@ -1328,8 +1331,7 @@ impl StarcoinVM { gas_meter.balance(), txn_data.max_gas_amount, status, - ) - .unwrap_or_else(|e| discard_error_vm_status(e).1); + ).unwrap_or_else(|e| discard_error_vm_status(e).1); (error_code, txn_output) } TransactionStatus::Discard(status) => { @@ -1448,8 +1450,7 @@ pub(crate) fn get_transaction_output( change_set, events, table_change_set, - } - .into_change_set(ap_cache)?; + }.into_change_set(ap_cache)?; Ok(TransactionOutput::new( write_set, events, @@ -1493,3 +1494,80 @@ pub fn log_vm_status( } } } + + +// Executor external API +impl VMExecutor for StarcoinVM { + /// 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( + // transactions: Vec, + // state_view: &impl StateView, + // ) -> Result, VMStatus> { + // fail_point!("move_adapter::execute_block", |_| { + // Err(VMStatus::Error( + // StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, + // )) + // }); + // + // let concurrency_level = Self::get_concurrency_level(); + // if concurrency_level > 1 { + // let (result, _) = crate::parallel_executor::ParallelAptosVM::execute_block( + // transactions, + // state_view, + // concurrency_level, + // )?; + // Ok(result) + // } else { + // let output = Self::execute_block_and_keep_vm_status(transactions, state_view)?; + // Ok(output + // .into_iter() + // .map(|(_vm_status, txn_output)| txn_output) + // .collect()) + // } + // } + + fn execute_block( + chain_state: &S, + txns: Vec, + block_gas_limit: u64, + metrics: Option, + ) -> Result> { + let mut vm = StarcoinVM::new(metrics); + let result = vm + .execute_block_transactions(&chain_state, txns, Some(block_gas_limit))? + .into_iter() + .map(|(_, output)| { + debug! {"{:?}", output} + output + }) + .collect(); + Ok(result) + } +} + +// VMValidator external API +impl VMValidator for StarcoinVM { + /// Determine if a transaction is valid. Will return `None` if the transaction is accepted, + /// `Some(Err)` if the VM rejects it, with `Err` as an error code. Verification performs the + /// following steps: + /// 1. The signature on the `SignedTransaction` matches the public key included in the + /// transaction + /// 2. The script to be executed is under given specific configuration. + /// 3. Invokes `Account.prologue`, which checks properties such as the transaction has the + /// right sequence number and the sender has enough balance to pay for the gas. + /// TBD: + /// 1. Transaction arguments matches the main function's type signature. + /// We don't check this item for now and would execute the check at execution time. + fn validate_transaction( + &mut self, + transaction: SignedUserTransaction, + state_view: &impl StateView, + ) -> Option { + //validate_signed_transaction(self, transaction, state_view) + self.verify_transaction(transaction, state_view) + } +} \ No newline at end of file From bdc10b2384ed0281992d89b370a7d85e7b200aba Mon Sep 17 00:00:00 2001 From: welbon Date: Mon, 26 Jun 2023 18:44:59 +0800 Subject: [PATCH 2/3] [starcoin vm] commit for cargo fmt --- vm/types/src/transaction/mod.rs | 3 +-- vm/vm-runtime/src/lib.rs | 5 ++--- vm/vm-runtime/src/starcoin_vm.rs | 23 ++++++++++++----------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/vm/types/src/transaction/mod.rs b/vm/types/src/transaction/mod.rs index 744ab119a9..224af7b2db 100644 --- a/vm/types/src/transaction/mod.rs +++ b/vm/types/src/transaction/mod.rs @@ -34,6 +34,7 @@ use crate::write_set::WriteOp; pub use error::CallError; pub use error::Error as TransactionError; pub use module::Module; +use move_core_types::vm_status::StatusType; pub use package::Package; pub use pending_transaction::{Condition, PendingTransaction}; use schemars::{self, JsonSchema}; @@ -43,7 +44,6 @@ pub use script::{ }; use starcoin_crypto::hash::SPARSE_MERKLE_PLACEHOLDER_HASH; use std::str::FromStr; -use move_core_types::vm_status::StatusType; pub use transaction_argument::{ parse_transaction_argument, parse_transaction_arguments, TransactionArgument, }; @@ -961,4 +961,3 @@ impl VMValidatorResult { self.score } } - diff --git a/vm/vm-runtime/src/lib.rs b/vm/vm-runtime/src/lib.rs index 8ca78f8699..0f16497e13 100644 --- a/vm/vm-runtime/src/lib.rs +++ b/vm/vm-runtime/src/lib.rs @@ -15,14 +15,13 @@ mod access_path_cache; mod errors; pub mod move_vm_ext; +use crate::metrics::VMMetrics; +use anyhow::Result; use starcoin_vm_types::access_path::AccessPath; use starcoin_vm_types::account_address::AccountAddress; use starcoin_vm_types::language_storage::StructTag; use starcoin_vm_types::state_view::StateView; use starcoin_vm_types::transaction::{SignedUserTransaction, Transaction, TransactionOutput}; -use crate::metrics::VMMetrics; -use anyhow::Result; - /// This trait describes the VM's validation interfaces. pub trait VMValidator { diff --git a/vm/vm-runtime/src/starcoin_vm.rs b/vm/vm-runtime/src/starcoin_vm.rs index f1b220cf0a..2a167822e1 100644 --- a/vm/vm-runtime/src/starcoin_vm.rs +++ b/vm/vm-runtime/src/starcoin_vm.rs @@ -372,7 +372,7 @@ impl StarcoinVM { // NB: MIN_PRICE_PER_GAS_UNIT may equal zero, but need not in the future. Hence why // we turn off the clippy warning. #[allow(clippy::absurd_extreme_comparisons)] - let below_min_bound = txn_data.gas_unit_price() < txn_gas_params.min_price_per_gas_unit; + let below_min_bound = txn_data.gas_unit_price() < txn_gas_params.min_price_per_gas_unit; if below_min_bound { warn!( "[VM] Gas unit error; min {}, submitted {}", @@ -481,7 +481,7 @@ impl StarcoinVM { state_view: &S, ) -> Option { #[cfg(feature = "metrics")] - let _timer = self.metrics.as_ref().map(|metrics| { + let _timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&["verify_transaction"]) @@ -1087,12 +1087,12 @@ impl StarcoinVM { let blocks = chunk_block_transactions(transactions); 'outer: for block in blocks { #[cfg(feature = "metrics")] - let txn_type_name = block.type_name().to_string(); + let txn_type_name = block.type_name().to_string(); match block { TransactionBlock::UserTransaction(txns) => { for transaction in txns { #[cfg(feature = "metrics")] - let timer = self.metrics.as_ref().map(|metrics| { + let timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&[txn_type_name.as_str()]) @@ -1141,7 +1141,7 @@ impl StarcoinVM { } TransactionBlock::BlockPrologue(block_metadata) => { #[cfg(feature = "metrics")] - let timer = self.metrics.as_ref().map(|metrics| { + let timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&[txn_type_name.as_str()]) @@ -1216,7 +1216,7 @@ impl StarcoinVM { check_gas: bool, ) -> Result>, VMStatus> { #[cfg(feature = "metrics")] - let _timer = self.metrics.as_ref().map(|metrics| { + let _timer = self.metrics.as_ref().map(|metrics| { metrics .vm_txn_exe_time .with_label_values(&["execute_readonly_function"]) @@ -1273,7 +1273,7 @@ impl StarcoinVM { events, table_change_set, } - .into_change_set(&mut ())?; + .into_change_set(&mut ())?; if !write_set.is_empty() { warn!("Readonly function {} changes state", function_name); return Err(VMStatus::Error(StatusCode::REJECTED_WRITE_SET)); @@ -1331,7 +1331,8 @@ impl StarcoinVM { gas_meter.balance(), txn_data.max_gas_amount, status, - ).unwrap_or_else(|e| discard_error_vm_status(e).1); + ) + .unwrap_or_else(|e| discard_error_vm_status(e).1); (error_code, txn_output) } TransactionStatus::Discard(status) => { @@ -1450,7 +1451,8 @@ pub(crate) fn get_transaction_output( change_set, events, table_change_set, - }.into_change_set(ap_cache)?; + } + .into_change_set(ap_cache)?; Ok(TransactionOutput::new( write_set, events, @@ -1495,7 +1497,6 @@ pub fn log_vm_status( } } - // Executor external API impl VMExecutor for StarcoinVM { /// Execute a block of `transactions`. The output vector will have the exact same length as the @@ -1570,4 +1571,4 @@ impl VMValidator for StarcoinVM { //validate_signed_transaction(self, transaction, state_view) self.verify_transaction(transaction, state_view) } -} \ No newline at end of file +} From 87c172619d8f81f2d9d3b38db01875f0812eeb11 Mon Sep 17 00:00:00 2001 From: welbon Date: Mon, 26 Jun 2023 19:07:54 +0800 Subject: [PATCH 3/3] [starcoin vm] commit for cargo fmt --- executor/tests/executor_test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/executor/tests/executor_test.rs b/executor/tests/executor_test.rs index 1df5044770..fd795b3588 100644 --- a/executor/tests/executor_test.rs +++ b/executor/tests/executor_test.rs @@ -151,7 +151,7 @@ fn test_txn_verify_err_case() -> Result<()> { ); let signed_by_bob = bob.sign_txn(txn); - let verify_result = vm.verify_transaction(&NullStateView, signed_by_bob); + let verify_result = vm.verify_transaction(signed_by_bob, &NullStateView); assert!(verify_result.is_some()); assert_eq!( verify_result.unwrap().status_code(), @@ -211,7 +211,7 @@ fn test_package_txn() -> Result<()> { TransactionPayload::Package(package.clone()), None, )); - let verify_result = vm.verify_transaction(&chain_state, txn); + let verify_result = vm.verify_transaction(txn, &chain_state); assert!(verify_result.is_none()); // execute the package txn account_execute_should_success(&alice, &chain_state, TransactionPayload::Package(package)) @@ -231,7 +231,7 @@ fn test_package_txn() -> Result<()> { TransactionPayload::Package(package), None, )); - let verify_result = vm.verify_transaction(&chain_state, txn); + let verify_result = vm.verify_transaction(txn, &chain_state); assert!(verify_result.is_some()); assert_eq!( verify_result.unwrap().status_code(), @@ -252,7 +252,7 @@ fn test_package_txn() -> Result<()> { TransactionPayload::Package(package.clone()), None, )); - let verify_result = vm.verify_transaction(&chain_state, txn); + let verify_result = vm.verify_transaction(txn, &chain_state); assert!(verify_result.is_none()); // execute the package txn account_execute_should_success(&alice, &chain_state, TransactionPayload::Package(package))