[draft] Storage Deposit

api/src/context.rs

@@ -238,7 +238,7 @@ impl Context {
     pub fn get_state_value(&self, state_key: &StateKey, version: u64) -> Result<Option<Vec<u8>>> {
         self.db
             .state_view_at_version(Some(version))?
-            .get_state_value(state_key)
+            .get_state_value_bytes(state_key)
     }
 
     pub fn get_state_value_poem<E: InternalError>(

api/src/state.rs

@@ -305,7 +305,7 @@ impl StateApi {
         let (ledger_info, ledger_version, state_view) =
             self.preprocess_request(ledger_version.map(|inner| inner.0))?;
         let bytes = state_view
-            .get_state_value(&state_key)
+            .get_state_value_bytes(&state_key)
             .context(format!("Failed to query DB to check for {:?}", state_key))
             .map_err(|err| {
                 BasicErrorWith404::internal_with_code(
@@ -392,7 +392,7 @@ impl StateApi {
         // Retrieve value from the state key
         let state_key = StateKey::table_item(TableHandle(table_handle.into()), raw_key);
         let bytes = state_view
-            .get_state_value(&state_key)
+            .get_state_value_bytes(&state_key)
             .context(format!(
                 "Failed when trying to retrieve table item from the DB with key: {}",
                 key
@@ -446,7 +446,7 @@ impl StateApi {
             table_item_request.key.0.clone(),
         );
         let bytes = state_view
-            .get_state_value(&state_key)
+            .get_state_value_bytes(&state_key)
             .context(format!(
                 "Failed when trying to retrieve table item from the DB with key: {}",
                 table_item_request.key,

api/types/src/convert.rs

@@ -255,8 +255,8 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
         access_path: AccessPath,
         op: WriteOp,
     ) -> Result<WriteSetChange> {
-        let ret = match op {
-            WriteOp::Deletion => match access_path.get_path() {
+        let ret = match op.into_bytes() {
+            None => match access_path.get_path() {
                 Path::Code(module_id) => WriteSetChange::DeleteModule(DeleteModule {
                     address: access_path.address.into(),
                     state_key_hash,
@@ -273,21 +273,21 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
                     resource: typ.into(),
                 }),
             },
-            WriteOp::Modification(val) | WriteOp::Creation(val) => match access_path.get_path() {
+            Some(bytes) => match access_path.get_path() {
                 Path::Code(_) => WriteSetChange::WriteModule(WriteModule {
                     address: access_path.address.into(),
                     state_key_hash,
-                    data: MoveModuleBytecode::new(val).try_parse_abi()?,
+                    data: MoveModuleBytecode::new(bytes).try_parse_abi()?,
                 }),
                 Path::Resource(typ) => WriteSetChange::WriteResource(WriteResource {
                     address: access_path.address.into(),
                     state_key_hash,
-                    data: self.try_into_resource(&typ, &val)?,
+                    data: self.try_into_resource(&typ, &bytes)?,
                 }),
                 Path::ResourceGroup(typ) => WriteSetChange::WriteResource(WriteResource {
                     address: access_path.address.into(),
                     state_key_hash,
-                    data: self.try_into_resource(&typ, &val)?,
+                    data: self.try_into_resource(&typ, &bytes)?,
                 }),
             },
         };
@@ -303,26 +303,26 @@ impl<'a, R: MoveResolverExt + ?Sized> MoveConverter<'a, R> {
     ) -> Result<WriteSetChange> {
         let hex_handle = handle.0.to_vec().into();
         let key: HexEncodedBytes = key.into();
-        let ret = match op {
-            WriteOp::Deletion => {
-                let data = self.try_delete_table_item_into_deleted_table_data(handle, &key.0)?;
+        let ret = match op.into_bytes() {
+            Some(bytes) => {
+                let data =
+                    self.try_write_table_item_into_decoded_table_data(handle, &key.0, &bytes)?;
 
-                WriteSetChange::DeleteTableItem(DeleteTableItem {
+                WriteSetChange::WriteTableItem(WriteTableItem {
                     state_key_hash,
                     handle: hex_handle,
                     key,
+                    value: bytes.into(),
                     data,
                 })
             },
-            WriteOp::Modification(value) | WriteOp::Creation(value) => {
-                let data =
-                    self.try_write_table_item_into_decoded_table_data(handle, &key.0, &value)?;
+            None => {
+                let data = self.try_delete_table_item_into_deleted_table_data(handle, &key.0)?;
 
-                WriteSetChange::WriteTableItem(WriteTableItem {
+                WriteSetChange::DeleteTableItem(DeleteTableItem {
                     state_key_hash,
                     handle: hex_handle,
                     key,
-                    value: value.into(),
                     data,
                 })
             },

aptos-move/aptos-aggregator/src/aggregator_extension.rs

@@ -368,7 +368,7 @@ mod test {
     use aptos_types::{
         account_address::AccountAddress,
         state_store::{
-            state_key::StateKey, state_storage_usage::StateStorageUsage,
+            state_key::StateKey, state_storage_usage::StateStorageUsage, state_value::StateValue,
             table::TableHandle as AptosTableHandle,
         },
     };
@@ -394,8 +394,12 @@ mod test {
     impl TStateView for FakeTestStorage {
         type Key = StateKey;
 
-        fn get_state_value(&self, state_key: &StateKey) -> anyhow::Result<Option<Vec<u8>>> {
-            Ok(self.data.get(state_key).cloned())
+        fn get_state_value(&self, state_key: &StateKey) -> anyhow::Result<Option<StateValue>> {
+            Ok(self
+                .data
+                .get(state_key)
+                .cloned()
+                .map(StateValue::new_legacy))
         }
 
         fn is_genesis(&self) -> bool {
@@ -414,7 +418,7 @@ mod test {
             key: &[u8],
         ) -> Result<Option<Vec<u8>>, anyhow::Error> {
             let state_key = StateKey::table_item(AptosTableHandle::from(*handle), key.to_vec());
-            self.get_state_value(&state_key)
+            self.get_state_value_bytes(&state_key)
         }
     }
 

aptos-move/aptos-aggregator/src/delta_change_set.rs

@@ -170,7 +170,7 @@ impl DeltaOp {
         state_key: &StateKey,
     ) -> anyhow::Result<WriteOp, VMStatus> {
         state_view
-            .get_state_value(state_key)
+            .get_state_value_bytes(state_key)
             .map_err(|_| VMStatus::Error(StatusCode::STORAGE_ERROR))
             .and_then(|maybe_bytes| {
                 match maybe_bytes {
@@ -352,7 +352,9 @@ impl ::std::iter::IntoIterator for DeltaChangeSet {
 mod tests {
     use super::*;
     use aptos_state_view::TStateView;
-    use aptos_types::state_store::state_storage_usage::StateStorageUsage;
+    use aptos_types::state_store::{
+        state_storage_usage::StateStorageUsage, state_value::StateValue,
+    };
     use claims::{assert_err, assert_matches, assert_ok, assert_ok_eq};
     use once_cell::sync::Lazy;
     use std::collections::HashMap;
@@ -540,8 +542,12 @@ mod tests {
     impl TStateView for FakeView {
         type Key = StateKey;
 
-        fn get_state_value(&self, state_key: &StateKey) -> anyhow::Result<Option<Vec<u8>>> {
-            Ok(self.data.get(state_key).cloned())
+        fn get_state_value(&self, state_key: &StateKey) -> anyhow::Result<Option<StateValue>> {
+            Ok(self
+                .data
+                .get(state_key)
+                .cloned()
+                .map(StateValue::new_legacy))
         }
 
         fn is_genesis(&self) -> bool {

aptos-move/aptos-aggregator/src/transaction.rs

@@ -74,15 +74,14 @@ impl ChangeSetExt {
         for (key, mut op) in other.into_iter() {
             if let Some(r) = write_ops.get_mut(&key) {
                 match r {
-                    Creation(data) => {
+                    Creation(data)
+                    | Modification(data)
+                    | CreationWithMetadata { data, .. }
+                    | ModificationWithMetadata { data, .. } => {
                         let val: u128 = bcs::from_bytes(data)?;
-                        *r = Creation(bcs::to_bytes(&op.apply_to(val)?)?);
+                        *data = bcs::to_bytes(&op.apply_to(val)?)?;
                     },
-                    Modification(data) => {
-                        let val: u128 = bcs::from_bytes(data)?;
-                        *r = Modification(bcs::to_bytes(&op.apply_to(val)?)?);
-                    },
-                    Deletion => {
+                    Deletion | DeletionWithMetadata { .. } => {
                         bail!("Failed to apply Aggregator delta -- value already deleted");
                     },
                 }
@@ -110,7 +109,6 @@ impl ChangeSetExt {
 
     pub fn squash_change_set(self, other: ChangeSet) -> anyhow::Result<Self> {
         use btree_map::Entry::*;
-        use WriteOp::*;
 
         let checker = self.checker.clone();
         let (mut delta, change_set) = self.into_inner();
@@ -123,19 +121,8 @@ impl ChangeSetExt {
         for (key, op) in other_write_set.into_iter() {
             match write_ops.entry(key) {
                 Occupied(mut entry) => {
-                    let r = entry.get_mut();
-                    match (&r, op) {
-                        (Modification(_) | Creation(_), Creation(_))
-                        | (Deletion, Deletion | Modification(_)) => {
-                            bail!("The given change sets cannot be squashed")
-                        },
-                        (Modification(_), Modification(data)) => *r = Modification(data),
-                        (Creation(_), Modification(data)) => *r = Creation(data),
-                        (Modification(_), Deletion) => *r = Deletion,
-                        (Deletion, Creation(data)) => *r = Modification(data),
-                        (Creation(_), Deletion) => {
-                            entry.remove();
-                        },
+                    if !WriteOp::squash(entry.get_mut(), op)? {
+                        entry.remove();
                     }
                 },
                 Vacant(entry) => {

aptos-move/aptos-debugger/src/lib.rs

@@ -187,15 +187,14 @@ impl AptosDebugger {
         .unwrap();
         let mut session = move_vm.new_session(&state_view_storage, SessionId::Void);
         f(&mut session).map_err(|err| format_err!("Unexpected VM Error: {:?}", err))?;
-        session
-            .finish()
-            .map_err(|err| format_err!("Unexpected VM Error: {:?}", err))?
-            .into_change_set(
+        let change_set_ext = session
+            .finish(
                 &mut (),
                 &ChangeSetConfigs::unlimited_at_gas_feature_version(LATEST_GAS_FEATURE_VERSION),
             )
-            .map_err(|err| format_err!("Unexpected VM Error: {:?}", err))
-            .map(|res| res.into_inner().1)
+            .map_err(|err| format_err!("Unexpected VM Error: {:?}", err))?;
+        let (_delta_change_set, change_set) = change_set_ext.into_inner();
+        Ok(change_set)
     }
 }
 

aptos-move/aptos-gas/src/transaction/storage.rs

@@ -73,14 +73,14 @@ impl StoragePricingV1 {
             }
 
             match op {
-                Creation(data) => {
+                Creation(data) | CreationWithMetadata { data, .. } => {
                     num_new_items += 1.into();
                     num_bytes_val += NumBytes::new(data.len() as u64);
                 },
-                Modification(data) => {
+                Modification(data) | ModificationWithMetadata { data, .. } => {
                     num_bytes_val += NumBytes::new(data.len() as u64);
                 },
-                Deletion => (),
+                Deletion | DeletionWithMetadata { .. } => (),
             }
         }
 
@@ -183,15 +183,15 @@ impl StoragePricingV2 {
 
         for (key, op) in ops {
             match &op {
-                Creation(data) => {
+                Creation(data) | CreationWithMetadata { data, .. } => {
                     num_items_create += 1.into();
                     num_bytes_create += self.write_op_size(key, data);
                 },
-                Modification(data) => {
+                Modification(data) | ModificationWithMetadata { data, .. } => {
                     num_items_write += 1.into();
                     num_bytes_write += self.write_op_size(key, data);
                 },
-                Deletion => (),
+                Deletion | DeletionWithMetadata { .. } => (),
             }
         }
 
@@ -271,7 +271,12 @@ impl ChangeSetConfigs {
         }
     }
 
-    pub fn creation_as_modification(&self) -> bool {
+    pub fn legacy_resource_creation_as_modification(&self) -> bool {
+        // Bug fixed at gas_feature_version 3 where (non-group) resource creation was converted to
+        // modification.
+        // Modules and table items were not affected (https://github.com/aptos-labs/aptos-core/pull/4722/commits/7c5e52297e8d1a6eac67a68a804ab1ca2a0b0f37).
+        // Resource groups and state values with metadata were not affected because they were
+        // introduced later than feature_version 3 on all networks.
         self.gas_feature_version < 3
     }
 
@@ -301,15 +306,12 @@ impl CheckChangeSet for ChangeSetConfigs {
 
         let mut write_set_size = 0;
         for (key, op) in change_set.write_set() {
-            match op {
-                WriteOp::Creation(data) | WriteOp::Modification(data) => {
-                    let write_op_size = (data.len() + key.size()) as u64;
-                    if write_op_size > self.max_bytes_per_write_op {
-                        return Err(VMStatus::Error(ERR));
-                    }
-                    write_set_size += write_op_size;
-                },
-                WriteOp::Deletion => (),
+            if let Some(bytes) = op.bytes() {
+                let write_op_size = (bytes.len() + key.size()) as u64;
+                if write_op_size > self.max_bytes_per_write_op {
+                    return Err(VMStatus::Error(ERR));
+                }
+                write_set_size += write_op_size;
             }
             if write_set_size > self.max_bytes_all_write_ops_per_transaction {
                 return Err(VMStatus::Error(ERR));

aptos-move/aptos-release-builder/src/components/feature_flags.rs

@@ -26,6 +26,7 @@ pub enum FeatureFlag {
     MultiEd25519PkValidateV2Natives,
     Blake2b256Native,
     ResourceGroups,
+    StorageSlotMetadata,
 }
 
 fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -124,6 +125,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
             },
             FeatureFlag::Blake2b256Native => AptosFeatureFlag::BLAKE2B_256_NATIVE,
             FeatureFlag::ResourceGroups => AptosFeatureFlag::RESOURCE_GROUPS,
+            FeatureFlag::StorageSlotMetadata => AptosFeatureFlag::STORAGE_SLOT_METADATA,
         }
     }
 }
@@ -147,6 +149,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
             },
             AptosFeatureFlag::BLAKE2B_256_NATIVE => FeatureFlag::Blake2b256Native,
             AptosFeatureFlag::RESOURCE_GROUPS => FeatureFlag::ResourceGroups,
+            AptosFeatureFlag::STORAGE_SLOT_METADATA => FeatureFlag::StorageSlotMetadata,
         }
     }
 }

aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs

@@ -368,7 +368,7 @@ impl<'a> AptosTestAdapter<'a> {
             AccessPath::resource_access_path(*signer_addr, AccountResource::struct_tag());
         let account_blob = self
             .storage
-            .get_state_value(&StateKey::access_path(account_access_path))
+            .get_state_value_bytes(&StateKey::access_path(account_access_path))
             .unwrap()
             .ok_or_else(|| {
                 format_err!(
@@ -388,7 +388,7 @@ impl<'a> AptosTestAdapter<'a> {
 
         let balance_blob = self
             .storage
-            .get_state_value(&StateKey::access_path(coin_access_path))
+            .get_state_value_bytes(&StateKey::access_path(coin_access_path))
             .unwrap()
             .ok_or_else(|| {
                 format_err!(
@@ -895,7 +895,7 @@ impl<'a> MoveTestAdapter<'a> for AptosTestAdapter<'a> {
 
                 let bytes = self
                     .storage
-                    .get_state_value(&state_key)
+                    .get_state_value_bytes(&state_key)
                     .unwrap()
                     .ok_or_else(|| format_err!("Failed to fetch table item.",))?;
 

aptos-move/aptos-validator-interface/src/lib.rs

@@ -173,20 +173,21 @@ impl DebuggerStateView {
         &self,
         state_key: &StateKey,
         version: Version,
-    ) -> Result<Option<Vec<u8>>> {
+    ) -> Result<Option<StateValue>> {
         let (tx, rx) = std::sync::mpsc::channel();
         let query_handler_locked = self.query_sender.lock().unwrap();
         query_handler_locked
             .send((state_key.clone(), version, tx))
             .unwrap();
-        Ok(rx.recv()?)
+        let bytes_opt = rx.recv()?;
+        Ok(bytes_opt.map(StateValue::new_legacy))
     }
 }
 
 impl TStateView for DebuggerStateView {
     type Key = StateKey;
 
-    fn get_state_value(&self, state_key: &StateKey) -> Result<Option<Vec<u8>>> {
+    fn get_state_value(&self, state_key: &StateKey) -> Result<Option<StateValue>> {
         self.get_state_value_internal(state_key, self.version)
     }