From c90dbbe69c2aa769cf6b61c18cc276d55c39e53e Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Wed, 27 Jul 2022 12:30:22 +0200 Subject: [PATCH] [fix] Addressed comments --- aptos-move/aptos-vm/src/aptos_vm.rs | 17 ++++++------ aptos-move/aptos-vm/src/aptos_vm_impl.rs | 4 +-- .../aptos-vm/src/move_vm_ext/session.rs | 6 ++--- .../src/parallel_executor/vm_wrapper.rs | 2 -- .../src/{delta_set.rs => delta_change_set.rs} | 26 ++++++++++++------- types/src/lib.rs | 2 +- types/src/transaction/change_set.rs | 12 ++++----- types/src/transaction/mod.rs | 23 +++++++++------- 8 files changed, 49 insertions(+), 43 deletions(-) rename types/src/{delta_set.rs => delta_change_set.rs} (84%) diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 7e6d05886e243..cd73797330b69 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -1234,15 +1234,14 @@ impl AptosSimulationVM { if txn_status.is_discarded() { discard_error_vm_status(err) } else { - let (vm_status, empty_output) = - self.0.failed_transaction_cleanup_and_keep_vm_status( - err, - &mut gas_status, - &txn_data, - storage, - log_context, - ); - (vm_status, empty_output.into_transaction_output()) + let (vm_status, output) = self.0.failed_transaction_cleanup_and_keep_vm_status( + err, + &mut gas_status, + &txn_data, + storage, + log_context, + ); + (vm_status, output.into_transaction_output()) } } } diff --git a/aptos-move/aptos-vm/src/aptos_vm_impl.rs b/aptos-move/aptos-vm/src/aptos_vm_impl.rs index 1f1a32dc90fd6..e50017cdda98e 100644 --- a/aptos-move/aptos-vm/src/aptos_vm_impl.rs +++ b/aptos-move/aptos-vm/src/aptos_vm_impl.rs @@ -573,13 +573,13 @@ pub(crate) fn get_transaction_output_ext let gas_used: u64 = txn_data.max_gas_amount().sub(gas_left).get(); let session_out = session.finish().map_err(|e| e.into_vm_status())?; - let (delta_set, change_set) = session_out.into_change_set_ext(ap_cache)?.into_inner(); + let (delta_change_set, change_set) = session_out.into_change_set_ext(ap_cache)?.into_inner(); let (write_set, events) = change_set.into_inner(); let txn_output = TransactionOutput::new(write_set, events, gas_used, TransactionStatus::Keep(status)); - Ok(TransactionOutputExt::new(delta_set, txn_output)) + Ok(TransactionOutputExt::new(delta_change_set, txn_output)) } pub(crate) fn get_transaction_output( diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session.rs b/aptos-move/aptos-vm/src/move_vm_ext/session.rs index fa9aaad90b8a0..54339fdc83116 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session.rs @@ -10,7 +10,7 @@ use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use aptos_types::{ block_metadata::BlockMetadata, contract_event::ContractEvent, - delta_set::DeltaSet, + delta_change_set::DeltaChangeSet, state_store::state_key::StateKey, transaction::{ChangeSet, ChangeSetExt, SignatureCheckedTransaction}, write_set::{WriteOp, WriteSetMut}, @@ -199,10 +199,10 @@ impl SessionOutput { self, ap_cache: &mut C, ) -> Result { - // TODO: extract `DeltaSet` from Aggregator extension (when it lands) + // TODO: extract `DeltaChangeSet` from Aggregator extension (when it lands) // and initialize `ChangeSetExt` properly. self.into_change_set(ap_cache) - .map(|change_set| ChangeSetExt::new(DeltaSet::default(), change_set)) + .map(|change_set| ChangeSetExt::new(DeltaChangeSet::empty(), change_set)) } pub fn squash(&mut self, other: Self) -> Result<(), VMStatus> { diff --git a/aptos-move/aptos-vm/src/parallel_executor/vm_wrapper.rs b/aptos-move/aptos-vm/src/parallel_executor/vm_wrapper.rs index 3999cd9d481fb..83e1b5c75449e 100644 --- a/aptos-move/aptos-vm/src/parallel_executor/vm_wrapper.rs +++ b/aptos-move/aptos-vm/src/parallel_executor/vm_wrapper.rs @@ -83,8 +83,6 @@ impl<'a, S: 'a + StateView> ExecutorTask for AptosVMWrapper<'a, S> { } }; } - - // TODO: here also pass deltas to `AptosTransactionOutput::new`. if AptosVM::should_restart_execution(&output) { ExecutionStatus::SkipRest(AptosTransactionOutput::new(output)) } else { diff --git a/types/src/delta_set.rs b/types/src/delta_change_set.rs similarity index 84% rename from types/src/delta_set.rs rename to types/src/delta_change_set.rs index 927287a7a4950..ca36d7b7cd2bc 100644 --- a/types/src/delta_set.rs +++ b/types/src/delta_change_set.rs @@ -68,28 +68,34 @@ pub fn deserialize(value_bytes: &[u8]) -> u128 { bcs::from_bytes(value_bytes).expect("unexpected deserialization error") } -/// `DeltaSet` contains all access paths that one transaction wants to update with deltas. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -pub struct DeltaSet { - delta_set: Vec<(StateKey, DeltaOp)>, +/// `DeltaChangeSet` contains all access paths that one transaction wants to update with deltas. +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub struct DeltaChangeSet { + delta_change_set: Vec<(StateKey, DeltaOp)>, } -impl DeltaSet { - pub fn new(delta_set: Vec<(StateKey, DeltaOp)>) -> Self { - DeltaSet { delta_set } +impl DeltaChangeSet { + pub fn empty() -> Self { + DeltaChangeSet { + delta_change_set: vec![], + } + } + + pub fn new(delta_change_set: Vec<(StateKey, DeltaOp)>) -> Self { + DeltaChangeSet { delta_change_set } } pub fn push(&mut self, delta: (StateKey, DeltaOp)) { - self.delta_set.push(delta); + self.delta_change_set.push(delta); } pub fn pop(&mut self) { - self.delta_set.pop(); + self.delta_change_set.pop(); } #[inline] pub fn is_empty(&self) -> bool { - self.delta_set.is_empty() + self.delta_change_set.is_empty() } } diff --git a/types/src/lib.rs b/types/src/lib.rs index f523b8d87bb67..1a9e2a3617ada 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -11,7 +11,7 @@ pub mod block_info; pub mod block_metadata; pub mod chain_id; pub mod contract_event; -pub mod delta_set; +pub mod delta_change_set; pub mod epoch_change; pub mod epoch_state; pub mod event; diff --git a/types/src/transaction/change_set.rs b/types/src/transaction/change_set.rs index 937691373c78f..1525e0cb3797d 100644 --- a/types/src/transaction/change_set.rs +++ b/types/src/transaction/change_set.rs @@ -1,7 +1,7 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use crate::{contract_event::ContractEvent, delta_set::DeltaSet, write_set::WriteSet}; +use crate::{contract_event::ContractEvent, delta_change_set::DeltaChangeSet, write_set::WriteSet}; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, Hash, Eq, PartialEq, Serialize, Deserialize)] @@ -30,19 +30,19 @@ impl ChangeSet { /// Extension of `ChangeSet` that also holds deltas. pub struct ChangeSetExt { - delta_set: DeltaSet, + delta_change_set: DeltaChangeSet, change_set: ChangeSet, } impl ChangeSetExt { - pub fn new(delta_set: DeltaSet, change_set: ChangeSet) -> Self { + pub fn new(delta_change_set: DeltaChangeSet, change_set: ChangeSet) -> Self { ChangeSetExt { - delta_set, + delta_change_set, change_set, } } - pub fn into_inner(self) -> (DeltaSet, ChangeSet) { - (self.delta_set, self.change_set) + pub fn into_inner(self) -> (DeltaChangeSet, ChangeSet) { + (self.delta_change_set, self.change_set) } } diff --git a/types/src/transaction/mod.rs b/types/src/transaction/mod.rs index ab8ee713b4996..f376500236295 100644 --- a/types/src/transaction/mod.rs +++ b/types/src/transaction/mod.rs @@ -6,7 +6,7 @@ use crate::{ block_metadata::BlockMetadata, chain_id::ChainId, contract_event::ContractEvent, - delta_set::DeltaSet, + delta_change_set::DeltaChangeSet, ledger_info::LedgerInfo, proof::{ accumulator::InMemoryAccumulator, TransactionInfoListWithProof, TransactionInfoWithProof, @@ -952,20 +952,23 @@ impl TransactionOutput { } } -/// Extension of `TransactionOutput` that also holds `DeltaSet` +/// Extension of `TransactionOutput` that also holds `DeltaChangeSet` #[derive(Clone, Debug, Eq, PartialEq)] pub struct TransactionOutputExt { - delta_set: DeltaSet, + delta_change_set: DeltaChangeSet, output: TransactionOutput, } impl TransactionOutputExt { - pub fn new(delta_set: DeltaSet, output: TransactionOutput) -> Self { - TransactionOutputExt { delta_set, output } + pub fn new(delta_change_set: DeltaChangeSet, output: TransactionOutput) -> Self { + TransactionOutputExt { + delta_change_set, + output, + } } - pub fn delta_set(&self) -> &DeltaSet { - &self.delta_set + pub fn delta_change_set(&self) -> &DeltaChangeSet { + &self.delta_change_set } pub fn write_set(&self) -> &WriteSet { @@ -984,8 +987,8 @@ impl TransactionOutputExt { &self.output.status } - pub fn into(self) -> (DeltaSet, TransactionOutput) { - (self.delta_set, self.output) + pub fn into(self) -> (DeltaChangeSet, TransactionOutput) { + (self.delta_change_set, self.output) } pub fn into_transaction_output(self) -> TransactionOutput { @@ -996,7 +999,7 @@ impl TransactionOutputExt { impl From for TransactionOutputExt { fn from(output: TransactionOutput) -> Self { TransactionOutputExt { - delta_set: DeltaSet::default(), + delta_change_set: DeltaChangeSet::empty(), output, } }