WriteOp carries StateValue

aptos-move/aptos-vm-types/src/change_set.rs

@@ -411,18 +411,18 @@ impl VMChangeSet {
             if let Some(write_op) = aggregator_v1_write_set.get_mut(&state_key) {
                 // In this case, delta follows a write op.
                 match write_op {
-                    Creation { data, .. } | Modification { data, .. } => {
+                    Creation(v) | Modification(v) => {
                         // Apply delta on top of creation or modification.
                         // TODO[agg_v1](cleanup): This will not be needed anymore once aggregator
                         // change sets carry non-serialized information.
-                        let base: u128 = bcs::from_bytes(data)
+                        let base: u128 = bcs::from_bytes(v.bytes())
                             .expect("Deserializing into an aggregator value always succeeds");
                         let value = additional_delta_op
                             .apply_to(base)
                             .map_err(PartialVMError::from)?;
-                        *data = serialize(&value).into();
+                        v.set_bytes(serialize(&value).into())
                     },
-                    Deletion { .. } => {
+                    Deletion(..) => {
                         // This case (applying a delta to deleted item) should
                         // never happen. Let's still return an error instead of
                         // panicking.

aptos-move/aptos-vm-types/src/tests/test_change_set.rs

@@ -436,10 +436,7 @@ fn test_aggregator_v2_snapshots_and_derived() {
 
 #[test]
 fn test_resource_groups_squashing() {
-    let modification_metadata = WriteOp::Modification {
-        data: Bytes::new(),
-        metadata: raw_metadata(2000),
-    };
+    let modification_metadata = WriteOp::modification(Bytes::new(), raw_metadata(2000));
 
     macro_rules! as_create_op {
         ($val:expr) => {
@@ -593,10 +590,7 @@ fn test_write_and_read_discrepancy_caught() {
         )])
         .try_build());
 
-    let metadata_op = WriteOp::Modification {
-        data: Bytes::new(),
-        metadata: raw_metadata(1000),
-    };
+    let metadata_op = WriteOp::modification(Bytes::new(), raw_metadata(1000));
     let group_size = ResourceGroupSize::Combined {
         num_tagged_resources: 1,
         all_tagged_resources_size: 14,
@@ -637,17 +631,9 @@ mod tests {
 
     pub(crate) fn write_op_with_metadata(type_idx: u8, v: u128) -> WriteOp {
         match type_idx {
-            CREATION => WriteOp::Creation {
-                data: vec![].into(),
-                metadata: raw_metadata(v as u64),
-            },
-            MODIFICATION => WriteOp::Modification {
-                data: vec![].into(),
-                metadata: raw_metadata(v as u64),
-            },
-            DELETION => WriteOp::Deletion {
-                metadata: raw_metadata(v as u64),
-            },
+            CREATION => WriteOp::creation(vec![].into(), raw_metadata(v as u64)),
+            MODIFICATION => WriteOp::modification(vec![].into(), raw_metadata(v as u64)),
+            DELETION => WriteOp::deletion(raw_metadata(v as u64)),
             _ => unreachable!("Wrong type index for test"),
         }
     }
@@ -694,9 +680,7 @@ mod tests {
             None
         );
         assert_group_write_size!(
-            WriteOp::Deletion {
-                metadata: raw_metadata(10)
-            },
+            WriteOp::deletion(raw_metadata(10)),
             ResourceGroupSize::zero_combined(),
             None
         );
@@ -729,10 +713,7 @@ mod tests {
             Some(sizes[0])
         );
         assert_group_write_size!(
-            WriteOp::Creation {
-                data: Bytes::new(),
-                metadata: raw_metadata(20)
-            },
+            WriteOp::creation(Bytes::new(), raw_metadata(20)),
             sizes[1],
             Some(sizes[1])
         );
@@ -742,10 +723,7 @@ mod tests {
             Some(sizes[2])
         );
         assert_group_write_size!(
-            WriteOp::Modification {
-                data: Bytes::new(),
-                metadata: raw_metadata(30)
-            },
+            WriteOp::modification(Bytes::new(), raw_metadata(30)),
             sizes[3],
             Some(sizes[3])
         );

aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs

@@ -210,7 +210,6 @@ impl<'r> WriteOpConverter<'r> {
         legacy_creation_as_modification: bool,
     ) -> PartialVMResult<WriteOp> {
         use MoveStorageOp::*;
-        use WriteOp::*;
         let write_op = match (state_value_metadata, move_storage_op) {
             (None, Modify(_) | Delete) => {
                 // Possible under speculative execution, returning speculative error waiting for re-execution.
@@ -238,18 +237,12 @@ impl<'r> WriteOpConverter<'r> {
                         WriteOp::legacy_creation(data)
                     }
                 },
-                Some(metadata) => Creation {
-                    data,
-                    metadata: metadata.clone(),
-                },
-            },
-            (Some(metadata), Modify(data)) => {
-                // Inherit metadata even if the feature flags is turned off, for compatibility.
-                Modification { data, metadata }
+                Some(metadata) => WriteOp::creation(data, metadata.clone()),
             },
+            (Some(metadata), Modify(data)) => WriteOp::modification(data, metadata),
             (Some(metadata), Delete) => {
                 // Inherit metadata even if the feature flags is turned off, for compatibility.
-                Deletion { metadata }
+                WriteOp::deletion(metadata)
             },
         };
         Ok(write_op)
@@ -270,13 +263,10 @@ impl<'r> WriteOpConverter<'r> {
                 match &self.new_slot_metadata {
                     // n.b. Aggregator writes historically did not distinguish Create vs Modify.
                     None => WriteOp::legacy_modification(data),
-                    Some(metadata) => WriteOp::Creation {
-                        data,
-                        metadata: metadata.clone(),
-                    },
+                    Some(metadata) => WriteOp::creation(data, metadata.clone()),
                 }
             },
-            Some(metadata) => WriteOp::Modification { data, metadata },
+            Some(metadata) => WriteOp::modification(data, metadata),
         };
 
         Ok(op)
@@ -623,7 +613,7 @@ mod tests {
 
         // Deletion should still contain the metadata - for storage refunds.
         assert_eq!(group_write.metadata_op().metadata(), &metadata);
-        assert_eq!(group_write.metadata_op(), &WriteOp::Deletion { metadata });
+        assert_eq!(group_write.metadata_op(), &WriteOp::deletion(metadata));
         assert_none!(group_write.metadata_op().bytes());
     }
 }

aptos-move/e2e-move-tests/src/tests/storage_refund.rs

@@ -159,7 +159,7 @@ fn assert_result(
     for (_state_key, write_op) in txn_out.write_set() {
         match write_op {
             WriteOp::Creation { .. } => creates += 1,
-            WriteOp::Deletion { metadata } => {
+            WriteOp::Deletion(metadata) => {
                 if metadata.is_none() {
                     panic!("This test expects all deletions to have metadata")
                 }

crates/aptos-rosetta/src/test/mod.rs

@@ -100,10 +100,10 @@ fn resource_group_modification_write_op<T: Serialize>(
 ) -> (StateKey, WriteOp) {
     let encoded = bcs::to_bytes(input).unwrap();
     let state_key = StateKey::resource_group(address, resource);
-    let write_op = WriteOp::Modification {
-        data: encoded.into(),
-        metadata: StateValueMetadata::new(0, 0, &CurrentTimeMicroseconds { microseconds: 0 }),
-    };
+    let write_op = WriteOp::modification(
+        encoded.into(),
+        StateValueMetadata::new(0, 0, &CurrentTimeMicroseconds { microseconds: 0 }),
+    );
     (state_key, write_op)
 }
 

testsuite/generate-format/src/api.rs

@@ -95,9 +95,10 @@ pub fn get_registry() -> Result<Registry> {
     // 1. Record samples for types with custom deserializers.
     trace_crypto_values(&mut tracer, &mut samples)?;
     tracer.trace_value(&mut samples, &event::EventKey::random())?;
-    tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion {
-        metadata: StateValueMetadata::none(),
-    })?;
+    tracer.trace_value(
+        &mut samples,
+        &write_set::WriteOp::Deletion(StateValueMetadata::none()),
+    )?;
 
     // 2. Trace the main entry point(s) + every enum separately.
     // stdlib types

testsuite/generate-format/src/aptos.rs

@@ -14,10 +14,7 @@ use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
 use aptos_types::{
     block_metadata_ext::BlockMetadataExt,
     contract_event, event,
-    state_store::{
-        state_key::StateKey,
-        state_value::{PersistedStateValueMetadata, StateValueMetadata},
-    },
+    state_store::{state_key::StateKey, state_value::PersistedStateValueMetadata},
     transaction,
     transaction::block_epilogue::BlockEpiloguePayload,
     validator_txn::ValidatorTransaction,
@@ -90,9 +87,7 @@ pub fn get_registry() -> Result<Registry> {
     // 1. Record samples for types with custom deserializers.
     trace_crypto_values(&mut tracer, &mut samples)?;
     tracer.trace_value(&mut samples, &event::EventKey::random())?;
-    tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion {
-        metadata: StateValueMetadata::none(),
-    })?;
+    tracer.trace_value(&mut samples, &write_set::WriteOp::legacy_deletion())?;
 
     // 2. Trace the main entry point(s) + every enum separately.
     tracer.trace_type::<contract_event::ContractEvent>(&samples)?;

testsuite/generate-format/src/consensus.rs

@@ -14,10 +14,7 @@ use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
 use aptos_types::{
     block_metadata_ext::BlockMetadataExt,
     contract_event, event,
-    state_store::{
-        state_key::StateKey,
-        state_value::{PersistedStateValueMetadata, StateValueMetadata},
-    },
+    state_store::{state_key::StateKey, state_value::PersistedStateValueMetadata},
     transaction,
     transaction::block_epilogue::BlockEpiloguePayload,
     validator_txn::ValidatorTransaction,
@@ -89,9 +86,7 @@ pub fn get_registry() -> Result<Registry> {
         &aptos_consensus_types::block::Block::make_genesis_block(),
     )?;
     tracer.trace_value(&mut samples, &event::EventKey::random())?;
-    tracer.trace_value(&mut samples, &write_set::WriteOp::Deletion {
-        metadata: StateValueMetadata::none(),
-    })?;
+    tracer.trace_value(&mut samples, &write_set::WriteOp::legacy_deletion())?;
 
     // 2. Trace the main entry point(s) + every enum separately.
     tracer.trace_type::<contract_event::ContractEvent>(&samples)?;

types/src/state_store/state_value.rs

@@ -280,6 +280,23 @@ impl StateValue {
         ))
     }
 
+    pub fn into_bytes(self) -> Bytes {
+        self.inner.data
+    }
+
+    pub fn set_bytes(&mut self, data: Bytes) {
+        self.inner.data = data;
+        self.hash = OnceCell::new();
+    }
+
+    pub fn metadata(&self) -> &StateValueMetadata {
+        &self.inner.metadata
+    }
+
+    pub fn metadata_mut(&mut self) -> &mut StateValueMetadata {
+        &mut self.inner.metadata
+    }
+
     pub fn into_metadata(self) -> StateValueMetadata {
         self.inner.metadata
     }