Skip to content

Commit

Permalink
[fix] Removed deltas from WriteOps
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Jul 27, 2022
1 parent 7ce6315 commit 56a2750
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 67 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions api/types/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,6 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
data: self.try_into_resource(&typ, &val)?,
}),
},
// Deltas never use access paths.
WriteOp::Delta(..) => unreachable!("unexpected conversion"),
};
Ok(ret)
}
Expand All @@ -287,8 +285,6 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
key,
value: value.into(),
}),
// Deltas are materialized into WriteOP::Value(..) in executor.
WriteOp::Delta(..) => unreachable!("unexpected conversion"),
};
Ok(ret)
}
Expand Down
3 changes: 0 additions & 3 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ impl<'a, S: StateView> StateViewCache<'a, S> {
self.data_map.remove(ap);
self.data_map.insert(ap.clone(), None);
}
WriteOp::Delta(..) => {
unimplemented!("sequential execution is not supported for deltas")
}
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions aptos-move/aptos-vm/src/parallel_executor/storage_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ impl<'a, S: StateView> StateView for VersionedView<'a, S> {
Some(v) => Ok(match v.as_ref() {
WriteOp::Value(w) => Some(w.clone()),
WriteOp::Deletion => None,
WriteOp::Delta(..) => {
unimplemented!("parallel execution is not supported for deltas")
}
}),
None => self.base_view.get_state_value(state_key),
}
Expand Down
1 change: 0 additions & 1 deletion aptos-move/e2e-tests/src/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ impl FakeDataStore {
WriteOp::Deletion => {
self.remove(state_key);
}
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions aptos-move/genesis-viewer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ fn print_modules(ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 0 {
Expand All @@ -214,7 +213,6 @@ fn print_resources(storage: &impl MoveResolverExt, ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 1 {
Expand Down Expand Up @@ -245,7 +243,6 @@ fn print_account_states(storage: &impl MoveResolverExt, ws: &WriteSet) {
AccessPath::try_from(k.clone()).expect("State key can't be converted to access path");
match v {
WriteOp::Deletion => panic!("found WriteOp::Deletion in WriteSet"),
WriteOp::Delta(..) => panic!("found WriteOp::Delta in WriteSet"),
WriteOp::Value(blob) => {
let tag = ap.path.get(0).expect("empty blob in WriteSet");
if *tag == 1 {
Expand Down
2 changes: 0 additions & 2 deletions aptos-move/transaction-replay/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,10 @@ impl AptosDebugger {
access_path::Path::Resource(tag) => match op {
WriteOp::Deletion => state_view.delete_resource(addr, tag)?,
WriteOp::Value(bytes) => state_view.save_resource(addr, tag, bytes)?,
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
},
access_path::Path::Code(module_id) => match op {
WriteOp::Deletion => state_view.delete_module(&module_id)?,
WriteOp::Value(bytes) => state_view.save_module(&module_id, bytes)?,
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
},
}
}
Expand Down
17 changes: 1 addition & 16 deletions documentation/specifications/move_adapter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -531,28 +531,13 @@ pub struct TransactionOutput {
/// `WriteSet` contains all access paths that one transaction modifies.
/// Each of them is a `WriteOp` where `Value(val)` means that serialized
/// representation should be updated to `val`, and `Deletion` means that
/// we are going to delete this access path. A special `WriteOp` used by
/// aggregator - `Delta(op, limit)`, means that `op` should be applied to
/// deserialized representation and a postcondition `result <= limit` must be
/// ensured.
/// we are going to delete this access path.
pub struct WriteSet {
write_set: Vec<(AccessPath, WriteOp)>,
}

/// `DeltaOperation` specifies the partial function which is used to apply
/// delta.
pub enum DeltaOperation {
Addition(u128),
Subtraction(u128),
}

/// Value when delta application overflows, i.e. the postcondition of delta
/// application.
pub struct DeltaLimit(pub u128);

pub enum WriteOp {
Deletion,
Delta(DeltaOperation, DeltaLimit),
Value(Vec<u8>),
}

Expand Down
1 change: 0 additions & 1 deletion execution/executor-types/src/in_memory_state_calculator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ fn process_state_key_write_op(
let state_value = match write_op {
WriteOp::Value(new_value) => StateValue::from(new_value),
WriteOp::Deletion => StateValue::empty(),
WriteOp::Delta(..) => unreachable!("deltas are only used in executor"),
};
match state_cache.entry(state_key.clone()) {
hash_map::Entry::Occupied(mut entry) => {
Expand Down
1 change: 1 addition & 0 deletions types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ aptos-crypto-derive = { path = "../crates/aptos-crypto-derive" }
move-deps = { path = "../aptos-move/move-deps", features = ["address32"] }

[dev-dependencies]
claim = "0.5.0"
proptest = "1.0.0"
proptest-derive = "0.3.0"
regex = "1.5.5"
Expand Down
128 changes: 128 additions & 0 deletions types/src/delta_set.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Copyright (c) Aptos
// SPDX-License-Identifier: Apache-2.0

//! Parallel data aggregation uses a `Delta` op. Every delta is is a state key
//! (for accessing the storage) and an operation: a partial function with a
//! postcondition.
use crate::state_store::state_key::StateKey;

/// Specifies different delta partial function specifications.
#[derive(Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub enum DeltaOp {
/// Addition of `value` which overflows on `limit`.
Addition { value: u128, limit: u128 },
/// Subtraction of `value` which cannot go below zero.
Subtraction { value: u128 },
}

impl DeltaOp {
/// Returns optional result of delta application to `base` (None if
/// postocndition not satisfied).
pub fn apply_to(&self, base: u128) -> Option<u128> {
match self {
DeltaOp::Addition { value, limit } => addition(base, *value, *limit),
DeltaOp::Subtraction { value } => subtraction(base, *value),
}
}
}

/// Implements application of `Addition` to `base`.
fn addition(base: u128, value: u128, limit: u128) -> Option<u128> {
if value > (limit - base) {
None
} else {
Some(base + value)
}
}

/// Implements application of `Subtraction` to `base`.
fn subtraction(base: u128, value: u128) -> Option<u128> {
if value > base {
None
} else {
Some(base - value)
}
}

impl std::fmt::Debug for DeltaOp {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
DeltaOp::Addition { value, limit } => {
write!(f, "+{} ensures result <= {}", value, limit)
}
DeltaOp::Subtraction { value } => {
write!(f, "-{} ensures 0 <= result", value)
}
}
}
}

/// Serializes value after delta application.
pub fn serialize(value: &u128) -> Vec<u8> {
bcs::to_bytes(value).expect("unexpected serialization error")
}

/// Deserializes value for delta application.
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)>,
}

impl DeltaSet {
pub fn new(delta_set: Vec<(StateKey, DeltaOp)>) -> Self {
DeltaSet { delta_set }
}

pub fn push(&mut self, delta: (StateKey, DeltaOp)) {
self.delta_set.push(delta);
}

pub fn pop(&mut self) {
self.delta_set.pop();
}

#[inline]
pub fn is_empty(&self) -> bool {
self.delta_set.is_empty()
}
}

#[cfg(test)]
mod tests {
use super::*;
use claim::{assert_matches, assert_some_eq};

fn addition(value: u128, limit: u128) -> DeltaOp {
DeltaOp::Addition { value, limit }
}

fn subtraction(value: u128) -> DeltaOp {
DeltaOp::Subtraction { value }
}

#[test]
fn test_delta_addition() {
let add5 = addition(5, 100);
assert_some_eq!(add5.apply_to(0), 5);
assert_some_eq!(add5.apply_to(5), 10);
assert_some_eq!(add5.apply_to(95), 100);

assert_matches!(add5.apply_to(96), None);
}

#[test]
fn test_delta_subtraction() {
let sub5 = subtraction(5);
assert_matches!(sub5.apply_to(0), None);
assert_matches!(sub5.apply_to(1), None);

assert_some_eq!(sub5.apply_to(5), 0);
assert_some_eq!(sub5.apply_to(100), 95);
}
}
1 change: 1 addition & 0 deletions types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod block_info;
pub mod block_metadata;
pub mod chain_id;
pub mod contract_event;
pub mod delta_set;
pub mod epoch_change;
pub mod epoch_state;
pub mod event;
Expand Down
35 changes: 1 addition & 34 deletions types/src/write_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,24 @@
// SPDX-License-Identifier: Apache-2.0

//! For each transaction the VM executes, the VM will output a `WriteSet` that contains each access
//! path it updates. For each access path, the VM can either give its new value or delete it. For
//! aggregator, delta updates are used (note: this is a temporary solution and ideally we should
//! modify `ChangeSet` and `TransactionOutput` to keep deltas internal to executor).
//! path it updates. For each access path, the VM can either give its new value or delete it.
use crate::state_store::state_key::StateKey;
use anyhow::Result;
use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
use serde::{Deserialize, Serialize};

/// Specifies partial function such as +X or -X to use with `WriteOp::Delta`.
#[derive(Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub enum DeltaOperation {
Addition(u128),
Subtraction(u128),
}

impl std::fmt::Debug for DeltaOperation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
DeltaOperation::Addition(value) => write!(f, "+{}", value),
DeltaOperation::Subtraction(value) => write!(f, "-{}", value),
}
}
}

#[derive(Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct DeltaLimit(pub u128);

impl std::fmt::Debug for DeltaLimit {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "result <= {}", self.0)
}
}

#[derive(Clone, Eq, Hash, PartialEq, Serialize, Deserialize)]
pub enum WriteOp {
Deletion,
Value(#[serde(with = "serde_bytes")] Vec<u8>),
#[serde(skip)]
Delta(DeltaOperation, DeltaLimit),
}

impl WriteOp {
#[inline]
pub fn is_deletion(&self) -> bool {
match self {
WriteOp::Deletion => true,
WriteOp::Delta(..) => false,
WriteOp::Value(_) => false,
}
}
Expand All @@ -66,9 +36,6 @@ impl std::fmt::Debug for WriteOp {
.map(|byte| format!("{:02x}", byte))
.collect::<String>()
),
WriteOp::Delta(op, limit) => {
write!(f, "Delta({:?} ensures {:?})", op, limit)
}
WriteOp::Deletion => write!(f, "Deletion"),
}
}
Expand Down

0 comments on commit 56a2750

Please sign in to comment.