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

[aptosvm] Simplify VM flows #11888

Merged
merged 19 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
319 changes: 130 additions & 189 deletions aptos-move/aptos-vm/src/aptos_vm.rs

Large diffs are not rendered by default.

21 changes: 5 additions & 16 deletions aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,12 @@
.vm
.execute_single_transaction(txn, &resolver, &log_context)
{
Ok((vm_status, vm_output, sender)) => {
Ok((vm_status, vm_output)) => {
if vm_output.status().is_discarded() {
match sender {
Some(s) => speculative_trace!(
&log_context,
format!(
"Transaction discarded, sender: {}, error: {:?}",
s, vm_status
),
),
None => {
speculative_trace!(
&log_context,
format!("Transaction malformed, error: {:?}", vm_status),
)
},
};
speculative_trace!(
&log_context,
format!("Transaction discarded, status: {:?}", vm_status),

Check warning on line 63 in aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs#L62-L63

Added lines #L62 - L63 were not covered by tests
);
}
if vm_status.status_code() == StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR {
ExecutionStatus::SpeculativeExecutionAbortError(
Expand Down
43 changes: 35 additions & 8 deletions aptos-move/aptos-vm/src/move_vm_ext/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use aptos_framework::natives::{
use aptos_table_natives::{NativeTableContext, TableChangeSet};
use aptos_types::{
access_path::AccessPath, block_metadata::BlockMetadata, block_metadata_ext::BlockMetadataExt,
contract_event::ContractEvent, on_chain_config::Features, state_store::state_key::StateKey,
contract_event::ContractEvent, state_store::state_key::StateKey,
validator_txn::ValidatorTransaction,
};
use aptos_vm_types::{change_set::VMChangeSet, storage::change_set_configs::ChangeSetConfigs};
Expand Down Expand Up @@ -151,24 +151,54 @@ impl SessionId {
pub fn as_uuid(&self) -> HashValue {
self.hash()
}

pub(crate) fn into_script_hash(self) -> Vec<u8> {
match self {
Self::Txn {
sender: _,
sequence_number: _,
script_hash,
}
| Self::Prologue {
sender: _,
sequence_number: _,
script_hash,
}
| Self::Epilogue {
sender: _,
sequence_number: _,
script_hash,
}
| Self::RunOnAbort {
sender: _,
sequence_number: _,
script_hash,
}
| Self::ValidatorTxn { script_hash } => script_hash,
Self::BlockMeta { id: _ }
| Self::Genesis { id: _ }
| Self::Void
| Self::BlockMetaExt { id: _ } => vec![],
}
}
}

pub struct SessionExt<'r, 'l> {
inner: Session<'r, 'l>,
remote: &'r dyn AptosMoveResolver,
features: Arc<Features>,
is_storage_slot_metadata_enabled: bool,
}

impl<'r, 'l> SessionExt<'r, 'l> {
pub fn new(
inner: Session<'r, 'l>,
remote: &'r dyn AptosMoveResolver,
features: Arc<Features>,
is_storage_slot_metadata_enabled: bool,
) -> Self {
Self {
inner,
remote,
features,
is_storage_slot_metadata_enabled,
}
}

Expand Down Expand Up @@ -209,10 +239,7 @@ impl<'r, 'l> SessionExt<'r, 'l> {
let event_context: NativeEventContext = extensions.remove();
let events = event_context.into_events();

let woc = WriteOpConverter::new(
self.remote,
self.features.is_storage_slot_metadata_enabled(),
);
let woc = WriteOpConverter::new(self.remote, self.is_storage_slot_metadata_enabled);

let change_set = Self::convert_change_set(
&woc,
Expand Down
40 changes: 8 additions & 32 deletions aptos-move/aptos-vm/src/move_vm_ext/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ use aptos_gas_algebra::DynamicExpression;
use aptos_gas_schedule::{MiscGasParameters, NativeGasParameters};
use aptos_native_interface::SafeNativeBuilder;
use aptos_table_natives::NativeTableContext;
use aptos_types::{
chain_id::ChainId,
on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures},
};
use aptos_types::on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures};
use move_binary_format::{
deserializer::DeserializerConfig,
errors::VMResult,
Expand All @@ -28,12 +25,12 @@ use move_bytecode_verifier::VerifierConfig;
use move_vm_runtime::{
config::VMConfig, move_vm::MoveVM, native_extensions::NativeContextExtensions,
};
use std::{ops::Deref, sync::Arc};
use std::ops::Deref;

pub struct MoveVmExt {
inner: MoveVM,
chain_id: u8,
features: Arc<Features>,
features: Features,
}

pub fn get_max_binary_format_version(
Expand Down Expand Up @@ -137,7 +134,7 @@ impl MoveVmExt {
resolver,
)?,
chain_id,
features: Arc::new(features),
features,
})
}

Expand Down Expand Up @@ -203,30 +200,9 @@ impl MoveVmExt {
extensions.add(NativeRistrettoPointContext::new());
extensions.add(AlgebraContext::new());
extensions.add(NativeAggregatorContext::new(txn_hash, resolver, resolver));

let script_hash = match session_id {
SessionId::Txn {
sender: _,
sequence_number: _,
script_hash,
}
| SessionId::Prologue {
sender: _,
sequence_number: _,
script_hash,
}
| SessionId::Epilogue {
sender: _,
sequence_number: _,
script_hash,
} => script_hash,
SessionId::ValidatorTxn { script_hash } => script_hash,
_ => vec![],
};

extensions.add(NativeTransactionContext::new(
txn_hash.to_vec(),
script_hash,
session_id.into_script_hash(),
self.chain_id,
));
extensions.add(NativeCodeContext::default());
Expand All @@ -240,12 +216,12 @@ impl MoveVmExt {
SessionExt::new(
self.inner.new_session_with_extensions(resolver, extensions),
resolver,
self.features.clone(),
self.features.is_storage_slot_metadata_enabled(),
)
}

pub fn get_chain_id(&self) -> ChainId {
ChainId::new(self.chain_id)
pub(crate) fn features(&self) -> &Features {
&self.features
}
}

Expand Down
31 changes: 3 additions & 28 deletions aptos-move/aptos-vm/src/transaction_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
// Parts of the project are originally copyright © Meta Platforms, Inc.
// SPDX-License-Identifier: Apache-2.0

use aptos_crypto::{ed25519::Ed25519PrivateKey, HashValue, PrivateKey};
use aptos_crypto::HashValue;
use aptos_gas_algebra::{FeePerGasUnit, Gas, NumBytes};
use aptos_types::{
account_address::AccountAddress,
chain_id::ChainId,
transaction::{authenticator::AuthenticationKey, SignedTransaction, TransactionPayload},
transaction::{SignedTransaction, TransactionPayload},
};
use std::convert::TryFrom;

pub struct TransactionMetadata {
pub sender: AccountAddress,
Expand Down Expand Up @@ -117,30 +116,6 @@ impl TransactionMetadata {
}

pub fn is_multi_agent(&self) -> bool {
!(self.secondary_signers.is_empty() && self.fee_payer.is_none())
}
}

impl Default for TransactionMetadata {
fn default() -> Self {
let mut buf = [0u8; Ed25519PrivateKey::LENGTH];
buf[Ed25519PrivateKey::LENGTH - 1] = 1;
let public_key = Ed25519PrivateKey::try_from(&buf[..]).unwrap().public_key();
TransactionMetadata {
sender: AccountAddress::ZERO,
authentication_key: AuthenticationKey::ed25519(&public_key).to_vec(),
secondary_signers: vec![],
secondary_authentication_keys: vec![],
sequence_number: 0,
fee_payer: None,
fee_payer_authentication_key: None,
max_gas_amount: 100_000_000.into(),
gas_unit_price: 0.into(),
transaction_size: 0.into(),
expiration_timestamp_secs: 0,
chain_id: ChainId::test(),
script_hash: vec![],
script_size: NumBytes::zero(),
}
!self.secondary_signers.is_empty() || self.fee_payer.is_some()
}
}
25 changes: 25 additions & 0 deletions aptos-move/e2e-move-tests/src/tests/account.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::MoveHarness;
use aptos_cached_packages::aptos_stdlib::aptos_account_transfer;
use aptos_language_e2e_tests::account::Account;
use claims::assert_err_eq;
use move_core_types::vm_status::StatusCode;

#[test]
fn non_existent_sender() {
let mut h = MoveHarness::new();

let sender = Account::new();
let receiver = h.new_account_with_balance_and_sequence_number(100_000, 0);

let txn = sender
.transaction()
.payload(aptos_account_transfer(*receiver.address(), 10))
.sequence_number(0)
.sign();

let status = h.run(txn);
assert_err_eq!(status.status(), StatusCode::SENDING_ACCOUNT_DOES_NOT_EXIST);
}
1 change: 1 addition & 0 deletions aptos-move/e2e-move-tests/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

mod access_path_test;
mod account;
mod aggregator;
mod aggregator_v2;
mod attributes;
Expand Down
118 changes: 0 additions & 118 deletions aptos-move/e2e-testsuite/src/tests/failed_transaction_tests.rs

This file was deleted.

Loading
Loading