Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm VMAdapter #4191

Merged
merged 1 commit into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion vm/vm-runtime/src/parallel_executor/vm_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use crate::{
parallel_executor::{storage_wrapper::VersionedView, StarcoinTransactionOutput},
starcoin_vm::StarcoinVM,
vm_adapter::{PreprocessedTransaction, VMAdapter},
vm_adapter::PreprocessedTransaction,
};

use starcoin_parallel_executor::{
Expand Down
106 changes: 63 additions & 43 deletions vm/vm-runtime/src/starcoin_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ use crate::errors::{
convert_normal_success_epilogue_error, convert_prologue_runtime_error, error_split,
};
use crate::move_vm_ext::{MoveResolverExt, MoveVmExt, SessionId, SessionOutput};
use crate::vm_adapter::{
discard_error_output, discard_error_vm_status, PreprocessedTransaction,
PublishModuleBundleOption, SessionAdapter, VMAdapter,
};
use crate::vm_adapter::{PreprocessedTransaction, PublishModuleBundleOption, SessionAdapter};
use anyhow::{bail, format_err, Error, Result};
use move_core_types::gas_algebra::{InternalGasPerByte, NumBytes};
use move_core_types::vm_status::StatusCode::VALUE_SERIALIZATION_ERROR;
Expand Down Expand Up @@ -61,6 +58,7 @@ use starcoin_vm_types::transaction::{DryRunTransaction, Package, TransactionPayl
use starcoin_vm_types::transaction_metadata::TransactionPayloadMetadata;
use starcoin_vm_types::value::{serialize_values, MoveValue};
use starcoin_vm_types::vm_status::KeptVMStatus;
use starcoin_vm_types::write_set::WriteSet;
use starcoin_vm_types::{
errors::Location,
language_storage::TypeTag,
Expand All @@ -71,6 +69,7 @@ use starcoin_vm_types::{
};
use std::borrow::Borrow;
use std::cmp::min;
use std::collections::BTreeMap;
use std::sync::Arc;

static EXECUTION_CONCURRENCY_LEVEL: OnceCell<usize> = OnceCell::new();
Expand All @@ -79,6 +78,29 @@ static EXECUTION_CONCURRENCY_LEVEL: OnceCell<usize> = OnceCell::new();
use crate::metrics::VMMetrics;
use crate::{verifier, VMExecutor};

pub(crate) fn discard_error_vm_status(err: VMStatus) -> (VMStatus, TransactionOutput) {
let vm_status = err.clone();
let error_code = match err.keep_or_discard() {
Ok(_) => {
debug_assert!(false, "discarding non-discardable error: {:?}", vm_status);
vm_status.status_code()
}
Err(code) => code,
};
(vm_status, discard_error_output(error_code))
}

pub(crate) fn discard_error_output(err: StatusCode) -> TransactionOutput {
// Since this transaction will be discarded, no writeset will be included.
TransactionOutput::new(
BTreeMap::new(),
WriteSet::default(),
vec![],
0,
TransactionStatus::Discard(err),
)
}

#[derive(Clone)]
#[allow(clippy::upper_case_acronyms)]
/// Wrapper of MoveVM
Expand Down Expand Up @@ -1465,6 +1487,43 @@ impl StarcoinVM {
) -> VMResult<Arc<CompiledModule>> {
self.move_vm.load_module(module_id, remote)
}

pub(crate) fn execute_single_transaction<S: MoveResolverExt + StateView>(
&self,
txn: &PreprocessedTransaction,
data_cache: &S,
) -> Result<(VMStatus, TransactionOutput, Option<String>), VMStatus> {
Ok(match txn {
PreprocessedTransaction::UserTransaction(txn) => {
let sender = txn.sender().to_string();
let (vm_status, output) = self.execute_user_transaction(data_cache, *txn.clone());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the TODO comment.

The TODO comment indicates that the sender string logic needs to be fixed.

Do you want me to open a GitHub issue to track this task? I can also help brainstorm potential solutions if needed.

// XXX FIXME YSG
// let gas_unit_price = transaction.gas_unit_price(); think about gas_used OutOfGas
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the TODO comment.

The TODO comment indicates that the gas_used logic needs to be fixed to handle OutOfGas scenarios.

Do you want me to open a GitHub issue to track this task? I can also help brainstorm potential solutions if needed.

(vm_status, output, Some(sender))
}
PreprocessedTransaction::BlockMetadata(block_meta) => {
let (vm_status, output) =
match self.process_block_metadata(data_cache, block_meta.clone()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the TODO comment.

The TODO comment indicates that the match logic needs to be fixed.

Do you want me to open a GitHub issue to track this task? I can also help brainstorm potential solutions if needed.

Ok(output) => (VMStatus::Executed, output),
Err(vm_status) => discard_error_vm_status(vm_status),
};
(vm_status, output, Some("block_meta".to_string()))
}
})
}

pub(crate) fn should_restart_execution(output: &TransactionOutput) -> bool {
// XXX FIXME YSG if GasSchedule.move UpgradeEvent
for event in output.events() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the TODO comment.

The TODO comment indicates that the logic to determine if execution should be restarted needs to be fixed to handle GasSchedule changes.

Do you want me to open a GitHub issue to track this task? I can also help brainstorm potential solutions if needed.

if event.key().get_creator_address() == genesis_address()
&& (event.is::<UpgradeEvent>() || event.is::<ConfigChangeEvent<Version>>())
{
info!("should_restart_execution happen");
return true;
}
}
false
}
}

#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -1631,42 +1690,3 @@ impl VMExecutor for StarcoinVM {
}
}
}

impl VMAdapter for StarcoinVM {
fn should_restart_execution(output: &TransactionOutput) -> bool {
// XXX FIXME YSG if GasSchedule.move UpgradeEvent
for event in output.events() {
if event.key().get_creator_address() == genesis_address()
&& (event.is::<UpgradeEvent>() || event.is::<ConfigChangeEvent<Version>>())
{
info!("should_restart_execution happen");
return true;
}
}
false
}

fn execute_single_transaction<S: MoveResolverExt + StateView>(
&self,
txn: &PreprocessedTransaction,
data_cache: &S,
) -> Result<(VMStatus, TransactionOutput, Option<String>), VMStatus> {
Ok(match txn {
PreprocessedTransaction::UserTransaction(txn) => {
let sender = txn.sender().to_string();
let (vm_status, output) = self.execute_user_transaction(data_cache, *txn.clone());
// XXX FIXME YSG
// let gas_unit_price = transaction.gas_unit_price(); think about gas_used OutOfGas
(vm_status, output, Some(sender))
}
PreprocessedTransaction::BlockMetadata(block_meta) => {
let (vm_status, output) =
match self.process_block_metadata(data_cache, block_meta.clone()) {
Ok(output) => (VMStatus::Executed, output),
Err(vm_status) => discard_error_vm_status(vm_status),
};
(vm_status, output, Some("block_meta".to_string()))
}
})
}
}
65 changes: 0 additions & 65 deletions vm/vm-runtime/src/vm_adapter/adapter_common.rs

This file was deleted.

31 changes: 22 additions & 9 deletions vm/vm-runtime/src/vm_adapter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,25 @@
// SPDX-License-Identifier: Apache-2.0

mod adapter;
mod adapter_common;

pub(crate) use {
adapter::{PublishModuleBundleOption, SessionAdapter},
adapter_common::{
discard_error_output, discard_error_vm_status, preprocess_transaction,
PreprocessedTransaction, VMAdapter,
},
};

pub(crate) use adapter::{PublishModuleBundleOption, SessionAdapter};
use starcoin_vm_types::block_metadata::BlockMetadata;
use starcoin_vm_types::transaction::{SignedUserTransaction, Transaction};

#[derive(Debug)]
pub enum PreprocessedTransaction {
UserTransaction(Box<SignedUserTransaction>),
BlockMetadata(BlockMetadata),
}

#[inline]
pub fn preprocess_transaction(txn: Transaction) -> crate::vm_adapter::PreprocessedTransaction {
match txn {
Transaction::BlockMetadata(b) => {
crate::vm_adapter::PreprocessedTransaction::BlockMetadata(b)
}
Transaction::UserTransaction(txn) => {
crate::vm_adapter::PreprocessedTransaction::UserTransaction(Box::new(txn))
}
}
}
Loading