Skip to content

Commit

Permalink
[fix] Addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Jul 27, 2022
1 parent 7bbad59 commit c90dbbe
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 43 deletions.
17 changes: 8 additions & 9 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/aptos_vm_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,13 +573,13 @@ pub(crate) fn get_transaction_output_ext<A: AccessPathCache, S: MoveResolverExt>
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<A: AccessPathCache, S: MoveResolverExt>(
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/aptos-vm/src/move_vm_ext/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -199,10 +199,10 @@ impl SessionOutput {
self,
ap_cache: &mut C,
) -> Result<ChangeSetExt, VMStatus> {
// 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> {
Expand Down
2 changes: 0 additions & 2 deletions aptos-move/aptos-vm/src/parallel_executor/vm_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
26 changes: 16 additions & 10 deletions types/src/delta_set.rs → types/src/delta_change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
2 changes: 1 addition & 1 deletion types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions types/src/transaction/change_set.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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)
}
}
23 changes: 13 additions & 10 deletions types/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -996,7 +999,7 @@ impl TransactionOutputExt {
impl From<TransactionOutput> for TransactionOutputExt {
fn from(output: TransactionOutput) -> Self {
TransactionOutputExt {
delta_set: DeltaSet::default(),
delta_change_set: DeltaChangeSet::empty(),
output,
}
}
Expand Down

0 comments on commit c90dbbe

Please sign in to comment.