WriteOp carries StateValue

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

@@ -111,15 +111,15 @@ impl AbstractResourceWriteOp {
 
     /// Deposit amount is inserted into metadata at a different time than the WriteOp is created.
     /// So this method is needed to be able to update metadata generically across different variants.
-    pub fn get_metadata_mut(&mut self) -> &mut StateValueMetadata {
+    pub fn metadata_mut(&mut self) -> &mut StateValueMetadata {
         use AbstractResourceWriteOp::*;
         match self {
             Write(write_op)
             | WriteWithDelayedFields(WriteWithDelayedFieldsOp { write_op, .. })
             | WriteResourceGroup(GroupWrite {
                 metadata_op: write_op,
                 ..
-            }) => write_op.get_metadata_mut(),
+            }) => write_op.metadata_mut(),
             InPlaceDelayedFieldChange(InPlaceDelayedFieldChangeOp { metadata, .. })
             | ResourceGroupInPlaceDelayedFieldChange(ResourceGroupInPlaceDelayedFieldChangeOp {
                 metadata,

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.
@@ -883,7 +883,7 @@ impl ChangeSetInterface for VMChangeSet {
                 key,
                 op_size: op.materialized_size(),
                 prev_size: op.prev_materialized_size(key, executor_view)?,
-                metadata_mut: op.get_metadata_mut(),
+                metadata_mut: op.metadata_mut(),
             })
         });
         let v1_aggregators = self.aggregator_v1_write_set.iter_mut().map(|(key, op)| {
@@ -893,7 +893,7 @@ impl ChangeSetInterface for VMChangeSet {
                 prev_size: executor_view
                     .get_aggregator_v1_state_value_size(key)?
                     .unwrap_or(0),
-                metadata_mut: op.get_metadata_mut(),
+                metadata_mut: op.metadata_mut(),
             })
         });
 

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

@@ -123,7 +123,7 @@ impl ModuleWriteSet {
                 key,
                 op_size: write.write_op().write_op_size(),
                 prev_size,
-                metadata_mut: write.write_op_mut().get_metadata_mut(),
+                metadata_mut: write.write_op_mut().metadata_mut(),
             })
         })
     }

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

@@ -98,10 +98,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)
 }
 

ecosystem/indexer-grpc/indexer-grpc-fullnode/src/stream_coordinator.rs

@@ -475,7 +475,7 @@ impl IndexerStreamCoordinator {
                 .iter()
                 .map(|(state_key, write_op)| WriteOpSizeInfo {
                     key_bytes: Self::ser_size_u32(state_key),
-                    value_bytes: write_op.size() as u32,
+                    value_bytes: write_op.bytes_size() as u32,
                 })
                 .collect(),
         }

testsuite/generate-format/src/api.rs

@@ -15,10 +15,7 @@ use aptos_types::{
     account_config::{CoinStoreResource, DepositEvent, WithdrawEvent},
     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::{
         self,
         authenticator::{AccountAuthenticator, TransactionAuthenticator},
@@ -95,9 +92,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.
     // 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)?;