diff --git a/vm/vm-runtime-types/src/resolver.rs b/vm/vm-runtime-types/src/resolver.rs index c0dd93846b..30ca15e8f6 100644 --- a/vm/vm-runtime-types/src/resolver.rs +++ b/vm/vm-runtime-types/src/resolver.rs @@ -89,14 +89,15 @@ pub trait TResourceGroupView { /// the parallel execution setting, as a wrong value will be (later) caught by validation. /// Thus, R/W conflicts are avoided, as long as the estimates are correct (e.g. updating /// struct members of a fixed size). - fn resource_group_size(&self, group_key: &Self::GroupKey) -> anyhow::Result; + fn resource_group_size(&self, group_key: &Self::GroupKey) + -> PartialVMResult; fn get_resource_from_group( &self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, maybe_layout: Option<&Self::Layout>, - ) -> anyhow::Result>; + ) -> PartialVMResult>; /// Needed for charging storage fees for a resource group write, as that requires knowing /// the size of the resource group AFTER the changeset of the transaction is applied (while @@ -107,7 +108,7 @@ pub trait TResourceGroupView { &self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { Ok(self .get_resource_from_group(group_key, resource_tag, None)? .map_or(0, |bytes| bytes.len())) @@ -127,7 +128,7 @@ pub trait TResourceGroupView { &self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { self.get_resource_from_group(group_key, resource_tag, None) .map(|maybe_bytes| maybe_bytes.is_some()) } diff --git a/vm/vm-runtime-types/src/resource_group_adapter.rs b/vm/vm-runtime-types/src/resource_group_adapter.rs index 5007f73d91..2deda14793 100644 --- a/vm/vm-runtime-types/src/resource_group_adapter.rs +++ b/vm/vm-runtime-types/src/resource_group_adapter.rs @@ -4,10 +4,11 @@ use crate::resolver::{ size_u32_as_uleb128, ResourceGroupSize, ResourceGroupView, TResourceGroupView, TResourceView, }; -use anyhow::Error; use bytes::Bytes; +use move_core_types::vm_status::StatusCode; use move_core_types::{language_storage::StructTag, value::MoveTypeLayout}; use serde::Serialize; +use starcoin_vm_types::errors::{PartialVMError, PartialVMResult}; use starcoin_vm_types::state_store::state_key::StateKey; use std::{ cell::RefCell, @@ -46,20 +47,24 @@ impl GroupSizeKind { pub fn group_tagged_resource_size( tag: &T, value_byte_len: usize, -) -> anyhow::Result { - Ok((bcs::serialized_size(&tag)? + value_byte_len + size_u32_as_uleb128(value_byte_len)) as u64) +) -> PartialVMResult { + Ok((bcs::serialized_size(&tag).map_err(|e| { + PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!( + "Tag serialization error for tag {:?}: {:?}", + tag, e + )) + })? + value_byte_len + + size_u32_as_uleb128(value_byte_len)) as u64) } /// Utility method to compute the size of the group as GroupSizeKind::AsSum. pub fn group_size_as_sum( mut group: impl Iterator, -) -> anyhow::Result { - let (count, len) = group - .try_fold((0, 0), |(count, len), (tag, value_byte_len)| { - let delta = group_tagged_resource_size(&tag, value_byte_len)?; - Ok((count + 1, len + delta)) - }) - .map_err(|_: Error| anyhow::Error::msg("Resource group member tag serialization error"))?; +) -> PartialVMResult { + let (count, len) = group.try_fold((0, 0), |(count, len), (tag, value_byte_len)| { + let delta = group_tagged_resource_size(&tag, value_byte_len)?; + Ok::<(usize, u64), PartialVMError>((count + 1, len + delta)) + })?; Ok(ResourceGroupSize::Combined { num_tagged_resources: count, @@ -157,7 +162,7 @@ impl<'r> ResourceGroupAdapter<'r> { // Ensures that the resource group at state_key is cached in self.group_cache. Ok(true) // means the resource was already cached, while Ok(false) means it just got cached. - fn load_to_cache(&self, group_key: &StateKey) -> anyhow::Result { + fn load_to_cache(&self, group_key: &StateKey) -> PartialVMResult { let already_cached = self.group_cache.borrow().contains_key(group_key); if already_cached { return Ok(true); @@ -165,10 +170,16 @@ impl<'r> ResourceGroupAdapter<'r> { let group_data = self.resource_view.get_resource_bytes(group_key, None)?; let (group_data, blob_len): (BTreeMap, u64) = group_data.map_or_else( - || Ok::<_, Error>((BTreeMap::new(), 0)), + || Ok::<_, PartialVMError>((BTreeMap::new(), 0)), |group_data_blob| { - let group_data = bcs::from_bytes(&group_data_blob) - .map_err(|_| anyhow::Error::msg("Resource group deserialization error"))?; + let group_data = bcs::from_bytes(&group_data_blob).map_err(|e| { + PartialVMError::new(StatusCode::UNEXPECTED_DESERIALIZATION_ERROR).with_message( + format!( + "Failed to deserialize the resource group at {:? }: {:?}", + group_key, e + ), + ) + })?; Ok((group_data, group_data_blob.len() as u64)) }, )?; @@ -199,7 +210,10 @@ impl TResourceGroupView for ResourceGroupAdapter<'_> { self.group_size_kind == GroupSizeKind::AsSum } - fn resource_group_size(&self, group_key: &Self::GroupKey) -> anyhow::Result { + fn resource_group_size( + &self, + group_key: &Self::GroupKey, + ) -> PartialVMResult { if self.group_size_kind == GroupSizeKind::None { return Ok(ResourceGroupSize::zero_concrete()); } @@ -222,7 +236,7 @@ impl TResourceGroupView for ResourceGroupAdapter<'_> { group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, maybe_layout: Option<&MoveTypeLayout>, - ) -> anyhow::Result> { + ) -> PartialVMResult> { if let Some(group_view) = self.maybe_resource_group_view { return group_view.get_resource_from_group(group_key, resource_tag, maybe_layout); } diff --git a/vm/vm-runtime/src/data_cache.rs b/vm/vm-runtime/src/data_cache.rs index 9f13d8d2ce..1303043a10 100644 --- a/vm/vm-runtime/src/data_cache.rs +++ b/vm/vm-runtime/src/data_cache.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 //! Scratchpad for on chain values during the execution. -use crate::move_vm_ext::AsExecutorView; +use crate::move_vm_ext::{AsExecutorView, ResourceGroupResolver}; use bytes::Bytes; use move_binary_format::deserializer::DeserializerConfig; use move_binary_format::CompiledModule; @@ -14,7 +14,7 @@ use move_table_extension::{TableHandle, TableResolver}; use starcoin_gas_schedule::LATEST_GAS_FEATURE_VERSION; use starcoin_logger::prelude::*; use starcoin_types::account_address::AccountAddress; -use starcoin_vm_runtime_types::resolver::ExecutorView; +use starcoin_vm_runtime_types::resolver::{ExecutorView, ResourceGroupSize, TResourceGroupView}; use starcoin_vm_runtime_types::resource_group_adapter::ResourceGroupAdapter; use starcoin_vm_types::state_store::{ errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage, @@ -26,6 +26,7 @@ use starcoin_vm_types::{ vm_status::StatusCode, write_set::{WriteOp, WriteSet}, }; +use std::collections::HashMap; use std::{ cell::RefCell, collections::btree_map::BTreeMap, @@ -53,7 +54,7 @@ pub fn get_resource_group_member_from_metadata( pub struct StorageAdapter<'e, E> { executor_view: &'e E, _deserializer_config: DeserializerConfig, - _resource_group_view: ResourceGroupAdapter<'e>, + resource_group_view: ResourceGroupAdapter<'e>, _accessed_groups: RefCell>, } @@ -166,7 +167,7 @@ impl<'a, S: StateView> StorageAdapter<'a, S> { Self { executor_view: state_store, _deserializer_config: DeserializerConfig::new(0, 0), - _resource_group_view: ResourceGroupAdapter::new( + resource_group_view: ResourceGroupAdapter::new( None, state_store, LATEST_GAS_FEATURE_VERSION, @@ -225,6 +226,36 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { } } +impl<'a, S: StateView> ResourceGroupResolver for StorageAdapter<'a, S> { + fn release_resource_group_cache( + &self, + ) -> Option>> { + self.resource_group_view.release_group_cache() + } + + fn resource_group_size(&self, group_key: &StateKey) -> PartialVMResult { + self.resource_group_view.resource_group_size(group_key) + } + + fn resource_size_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult { + self.resource_group_view + .resource_size_in_group(group_key, resource_tag) + } + + fn resource_exists_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult { + self.resource_group_view + .resource_exists_in_group(group_key, resource_tag) + } +} + impl<'a, S> Deref for StorageAdapter<'a, S> { type Target = S; @@ -321,6 +352,36 @@ impl ResourceResolver for RemoteStorageOwned { } } +impl ResourceGroupResolver for RemoteStorageOwned { + fn release_resource_group_cache( + &self, + ) -> Option>> { + self.as_move_resolver().release_resource_group_cache() + } + + fn resource_group_size(&self, group_key: &StateKey) -> PartialVMResult { + self.as_move_resolver().resource_group_size(group_key) + } + + fn resource_size_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult { + self.as_move_resolver() + .resource_size_in_group(group_key, resource_tag) + } + + fn resource_exists_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult { + self.as_move_resolver() + .resource_exists_in_group(group_key, resource_tag) + } +} + impl TableResolver for RemoteStorageOwned { fn resolve_table_entry_bytes_with_layout( &self, diff --git a/vm/vm-runtime/src/move_vm_ext/mod.rs b/vm/vm-runtime/src/move_vm_ext/mod.rs index 5c6d9ccee4..248547cb29 100644 --- a/vm/vm-runtime/src/move_vm_ext/mod.rs +++ b/vm/vm-runtime/src/move_vm_ext/mod.rs @@ -12,7 +12,7 @@ mod warm_vm_cache; pub(crate) mod write_op_converter; pub use crate::move_vm_ext::{ - resolver::{AsExecutorView, StarcoinMoveResolver}, + resolver::{AsExecutorView, ResourceGroupResolver, StarcoinMoveResolver}, session::{SessionExt, SessionId}, vm::MoveVmExt, }; diff --git a/vm/vm-runtime/src/move_vm_ext/resolver.rs b/vm/vm-runtime/src/move_vm_ext/resolver.rs index bf665b6512..6febe36b36 100644 --- a/vm/vm-runtime/src/move_vm_ext/resolver.rs +++ b/vm/vm-runtime/src/move_vm_ext/resolver.rs @@ -1,12 +1,16 @@ // Copyright (c) The Starcoin Core Contributors // SPDX-License-Identifier: Apache-2.0 -use move_binary_format::errors::PartialVMError; -use move_core_types::resolver::MoveResolver; +use bytes::Bytes; +use move_binary_format::errors::{PartialVMError, PartialVMResult}; +use move_core_types::language_storage::StructTag; +use move_core_types::resolver::{MoveResolver, ResourceResolver}; use move_table_extension::TableResolver; use starcoin_aggregator::resolver::{AggregatorV1Resolver, DelayedFieldResolver}; -use starcoin_vm_runtime_types::resolver::ExecutorView; +use starcoin_vm_runtime_types::resolver::{ExecutorView, ResourceGroupSize}; use starcoin_vm_types::on_chain_config::ConfigStorage; +use starcoin_vm_types::state_store::state_key::StateKey; +use std::collections::{BTreeMap, HashMap}; /// A general resolver used by StarcoinVM. Allows to implement custom hooks on /// top of storage, e.g. get resources from resource groups, etc. @@ -16,6 +20,8 @@ pub trait StarcoinMoveResolver: + DelayedFieldResolver + ConfigStorage + MoveResolver + + ResourceResolver + + ResourceGroupResolver + TableResolver + AsExecutorView { @@ -26,30 +32,32 @@ impl< + ConfigStorage + DelayedFieldResolver + MoveResolver + + ResourceResolver + + ResourceGroupResolver + TableResolver + AsExecutorView, > StarcoinMoveResolver for S { } -//pub trait ResourceGroupResolver { -// fn release_resource_group_cache(&self) -// -> Option>>; -// -// fn resource_group_size(&self, group_key: &StateKey) -> PartialVMResult; -// -// fn resource_size_in_group( -// &self, -// group_key: &StateKey, -// resource_tag: &StructTag, -// ) -> PartialVMResult; -// -// fn resource_exists_in_group( -// &self, -// group_key: &StateKey, -// resource_tag: &StructTag, -// ) -> PartialVMResult; -//} +pub trait ResourceGroupResolver { + fn release_resource_group_cache(&self) + -> Option>>; + + fn resource_group_size(&self, group_key: &StateKey) -> PartialVMResult; + + fn resource_size_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult; + + fn resource_exists_in_group( + &self, + group_key: &StateKey, + resource_tag: &StructTag, + ) -> PartialVMResult; +} pub trait AsExecutorView { fn as_executor_view(&self) -> &dyn ExecutorView; diff --git a/vm/vm-runtime/src/move_vm_ext/session.rs b/vm/vm-runtime/src/move_vm_ext/session.rs index ceaf8852a2..6ad75a19e9 100644 --- a/vm/vm-runtime/src/move_vm_ext/session.rs +++ b/vm/vm-runtime/src/move_vm_ext/session.rs @@ -118,7 +118,6 @@ impl SessionId { #[allow(dead_code)] pub struct SessionExt<'r, 'l> { inner: Session<'r, 'l>, - #[allow(dead_code)] resolver: &'r dyn StarcoinMoveResolver, features: Arc, } @@ -275,7 +274,7 @@ impl<'r, 'l> SessionExt<'r, 'l> { /// merging them into a single op corresponding to the whole resource group (V0). fn split_and_merge_resource_groups( runtime: &MoveVM, - _resolver: &dyn StarcoinMoveResolver, + resolver: &dyn StarcoinMoveResolver, change_set: ChangeSet, ) -> PartialVMResult<(ChangeSet, ResourceGroupChangeSet)> { // The use of this implies that we could theoretically call unwrap with no consequences, @@ -286,16 +285,16 @@ impl<'r, 'l> SessionExt<'r, 'l> { }; let mut change_set_filtered = ChangeSet::new(); - //let mut maybe_resource_group_cache = resolver.release_resource_group_cache().map(|v| { - // v.into_iter() - // .map(|(k, v)| (k, v.into_iter().collect::>())) - // .collect::>() - //}); - //let mut resource_group_change_set = if maybe_resource_group_cache.is_some() { - // ResourceGroupChangeSet::V0(BTreeMap::new()) - //} else { - // ResourceGroupChangeSet::V1(BTreeMap::new()) - //}; + let mut maybe_resource_group_cache = resolver.release_resource_group_cache().map(|v| { + v.into_iter() + .map(|(k, v)| (k, v.into_iter().collect::>())) + .collect::>() + }); + let mut resource_group_change_set = if maybe_resource_group_cache.is_some() { + ResourceGroupChangeSet::V0(BTreeMap::new()) + } else { + ResourceGroupChangeSet::V1(BTreeMap::new()) + }; for (addr, account_changeset) in change_set.into_inner() { let mut resource_groups: BTreeMap< StructTag, @@ -331,57 +330,54 @@ impl<'r, 'l> SessionExt<'r, 'l> { ) .map_err(|_| common_error())?; - //for (resource_group_tag, resources) in resource_groups { - // let state_key = StateKey::resource_group(&addr, &resource_group_tag); - // match &mut resource_group_change_set { - // ResourceGroupChangeSet::V0(v0_changes) => { - // let source_data = maybe_resource_group_cache - // .as_mut() - // .expect("V0 cache must be set") - // .remove(&state_key) - // .unwrap_or_default(); - // Self::populate_v0_resource_group_change_set( - // v0_changes, - // state_key, - // source_data, - // resources, - // )?; - // } - // ResourceGroupChangeSet::V1(v1_changes) => { - // // Maintain the behavior of failing the transaction on resource - // // group member existence invariants. - // for (struct_tag, current_op) in resources.iter() { - // let exists = - // resolver.resource_exists_in_group(&state_key, struct_tag)?; - // if matches!(current_op, MoveStorageOp::New(_)) == exists { - // // Deletion and Modification require resource to exist, - // // while creation requires the resource to not exist. - // return Err(common_error()); - // } - // } - // v1_changes.insert(state_key, resources); - // } - // } - //} + for (resource_group_tag, resources) in resource_groups { + let state_key = StateKey::resource_group(&addr, &resource_group_tag); + match &mut resource_group_change_set { + ResourceGroupChangeSet::V0(v0_changes) => { + let source_data = maybe_resource_group_cache + .as_mut() + .expect("V0 cache must be set") + .remove(&state_key) + .unwrap_or_default(); + Self::populate_v0_resource_group_change_set( + v0_changes, + state_key, + source_data, + resources, + )?; + } + ResourceGroupChangeSet::V1(v1_changes) => { + // Maintain the behavior of failing the transaction on resource + // group member existence invariants. + for (struct_tag, current_op) in resources.iter() { + let exists = + resolver.resource_exists_in_group(&state_key, struct_tag)?; + if matches!(current_op, MoveStorageOp::New(_)) == exists { + // Deletion and Modification require resource to exist, + // while creation requires the resource to not exist. + return Err(common_error()); + } + } + v1_changes.insert(state_key, resources); + } + } + } } - Ok(( - change_set_filtered, - ResourceGroupChangeSet::V0(BTreeMap::new()), - )) + Ok((change_set_filtered, resource_group_change_set)) } fn convert_change_set( woc: &WriteOpConverter, change_set: ChangeSet, - _resource_group_change_set: ResourceGroupChangeSet, + resource_group_change_set: ResourceGroupChangeSet, events: Vec<(ContractEvent, Option)>, table_change_set: TableChangeSet, aggregator_change_set: AggregatorChangeSet, legacy_resource_creation_as_modification: bool, ) -> PartialVMResult<(VMChangeSet, ModuleWriteSet)> { let mut resource_write_set = BTreeMap::new(); - let resource_group_write_set = BTreeMap::new(); + let mut resource_group_write_set = BTreeMap::new(); let mut has_modules_published_to_special_address = false; let mut module_write_ops = BTreeMap::new(); @@ -412,20 +408,20 @@ impl<'r, 'l> SessionExt<'r, 'l> { } } - //match resource_group_change_set { - // ResourceGroupChangeSet::V0(v0_changes) => { - // for (state_key, blob_op) in v0_changes { - // let op = woc.convert_resource(&state_key, blob_op, false)?; - // resource_write_set.insert(state_key, op); - // } - // } - // ResourceGroupChangeSet::V1(v1_changes) => { - // for (state_key, resources) in v1_changes { - // let group_write = woc.convert_resource_group_v1(&state_key, resources)?; - // resource_group_write_set.insert(state_key, group_write); - // } - // } - //} + match resource_group_change_set { + ResourceGroupChangeSet::V0(v0_changes) => { + for (state_key, blob_op) in v0_changes { + let op = woc.convert_resource(&state_key, blob_op, false)?; + resource_write_set.insert(state_key, op); + } + } + ResourceGroupChangeSet::V1(v1_changes) => { + for (state_key, resources) in v1_changes { + let group_write = woc.convert_resource_group_v1(&state_key, resources)?; + resource_group_write_set.insert(state_key, group_write); + } + } + } for (handle, change) in table_change_set.changes { for (key, value_op) in change.entries { diff --git a/vm/vm-runtime/src/move_vm_ext/write_op_converter.rs b/vm/vm-runtime/src/move_vm_ext/write_op_converter.rs index df8f7714de..949d2e04b1 100644 --- a/vm/vm-runtime/src/move_vm_ext/write_op_converter.rs +++ b/vm/vm-runtime/src/move_vm_ext/write_op_converter.rs @@ -4,15 +4,19 @@ use crate::move_vm_ext::{session::BytesWithResourceLayout, StarcoinMoveResolver}; use bytes::Bytes; use move_binary_format::errors::{PartialVMError, PartialVMResult}; +use move_core_types::language_storage::StructTag; use move_core_types::{effects::Op as MoveStorageOp, value::MoveTypeLayout, vm_status::StatusCode}; use move_vm_types::delayed_values::error::code_invariant_error; use starcoin_aggregator::delta_change_set::serialize; +use starcoin_vm_runtime_types::abstract_write_op::GroupWrite; use starcoin_vm_runtime_types::resolver::ResourceGroupSize; +use starcoin_vm_runtime_types::resource_group_adapter::group_tagged_resource_size; use starcoin_vm_types::{ on_chain_config::{CurrentTimeMicroseconds, OnChainConfig}, state_store::{state_key::StateKey, state_value::StateValueMetadata}, write_set::WriteOp, }; +use std::collections::BTreeMap; use std::sync::Arc; pub(crate) struct WriteOpConverter<'r> { @@ -183,74 +187,74 @@ impl<'r> WriteOpConverter<'r> { Ok((write_op, layout)) } - //pub(crate) fn convert_resource_group_v1( - // &self, - // state_key: &StateKey, - // group_changes: BTreeMap>, - //) -> PartialVMResult { - // // Resource group metadata is stored at the group StateKey, and can be obtained via the - // // same interfaces at for a resource at a given StateKey. - // let state_value_metadata = self - // .remote - // .as_executor_view() - // .get_resource_state_value_metadata(state_key)?; - // // Currently, due to read-before-write and a gas charge on the first read that is based - // // on the group size, this should simply re-read a cached (speculative) group size. - // let pre_group_size = self.remote.resource_group_size(state_key)?; - // check_size_and_existence_match(&pre_group_size, state_value_metadata.is_some(), state_key)?; - - // let mut inner_ops = BTreeMap::new(); - // let mut post_group_size = pre_group_size; - - // for (tag, current_op) in group_changes { - // // We take speculative group size prior to the transaction, and update it based on the change-set. - // // For each tagged resource in the change set, we subtract the previous size tagged resource size, - // // and then add new tagged resource size. - // // - // // The reason we do not instead get and add the sizes of the resources in the group, - // // but not in the change-set, is to avoid creating unnecessary R/W conflicts (the resources - // // in the change-set are already read, but the other resources are not). - // if !matches!(current_op, MoveStorageOp::New(_)) { - // let old_tagged_value_size = self.remote.resource_size_in_group(state_key, &tag)?; - // let old_size = group_tagged_resource_size(&tag, old_tagged_value_size)?; - // decrement_size_for_remove_tag(&mut post_group_size, old_size)?; - // } - - // match ¤t_op { - // MoveStorageOp::Modify((data, _)) | MoveStorageOp::New((data, _)) => { - // let new_size = group_tagged_resource_size(&tag, data.len())?; - // increment_size_for_add_tag(&mut post_group_size, new_size)?; - // } - // MoveStorageOp::Delete => {} - // }; - - // let legacy_op = match current_op { - // MoveStorageOp::Delete => (WriteOp::legacy_deletion(), None), - // MoveStorageOp::Modify((data, maybe_layout)) => { - // (WriteOp::legacy_modification(data), maybe_layout) - // } - // MoveStorageOp::New((data, maybe_layout)) => { - // (WriteOp::legacy_creation(data), maybe_layout) - // } - // }; - // inner_ops.insert(tag, legacy_op); - // } - - // // Create an op to encode the proper kind for resource group operation. - // let metadata_op = if post_group_size.get() == 0 { - // MoveStorageOp::Delete - // } else if pre_group_size.get() == 0 { - // MoveStorageOp::New(Bytes::new()) - // } else { - // MoveStorageOp::Modify(Bytes::new()) - // }; - // Ok(GroupWrite::new( - // self.convert(state_value_metadata, metadata_op, false)?, - // inner_ops, - // post_group_size, - // pre_group_size.get(), - // )) - //} + pub(crate) fn convert_resource_group_v1( + &self, + state_key: &StateKey, + group_changes: BTreeMap>, + ) -> PartialVMResult { + // Resource group metadata is stored at the group StateKey, and can be obtained via the + // same interfaces at for a resource at a given StateKey. + let state_value_metadata = self + .remote + .as_executor_view() + .get_resource_state_value_metadata(state_key)?; + // Currently, due to read-before-write and a gas charge on the first read that is based + // on the group size, this should simply re-read a cached (speculative) group size. + let pre_group_size = self.remote.resource_group_size(state_key)?; + check_size_and_existence_match(&pre_group_size, state_value_metadata.is_some(), state_key)?; + + let mut inner_ops = BTreeMap::new(); + let mut post_group_size = pre_group_size; + + for (tag, current_op) in group_changes { + // We take speculative group size prior to the transaction, and update it based on the change-set. + // For each tagged resource in the change set, we subtract the previous size tagged resource size, + // and then add new tagged resource size. + // + // The reason we do not instead get and add the sizes of the resources in the group, + // but not in the change-set, is to avoid creating unnecessary R/W conflicts (the resources + // in the change-set are already read, but the other resources are not). + if !matches!(current_op, MoveStorageOp::New(_)) { + let old_tagged_value_size = self.remote.resource_size_in_group(state_key, &tag)?; + let old_size = group_tagged_resource_size(&tag, old_tagged_value_size)?; + decrement_size_for_remove_tag(&mut post_group_size, old_size)?; + } + + match ¤t_op { + MoveStorageOp::Modify((data, _)) | MoveStorageOp::New((data, _)) => { + let new_size = group_tagged_resource_size(&tag, data.len())?; + increment_size_for_add_tag(&mut post_group_size, new_size)?; + } + MoveStorageOp::Delete => {} + }; + + let legacy_op = match current_op { + MoveStorageOp::Delete => (WriteOp::legacy_deletion(), None), + MoveStorageOp::Modify((data, maybe_layout)) => { + (WriteOp::legacy_modification(data), maybe_layout) + } + MoveStorageOp::New((data, maybe_layout)) => { + (WriteOp::legacy_creation(data), maybe_layout) + } + }; + inner_ops.insert(tag, legacy_op); + } + + // Create an op to encode the proper kind for resource group operation. + let metadata_op = if post_group_size.get() == 0 { + MoveStorageOp::Delete + } else if pre_group_size.get() == 0 { + MoveStorageOp::New(Bytes::new()) + } else { + MoveStorageOp::Modify(Bytes::new()) + }; + Ok(GroupWrite::new( + self.convert(state_value_metadata, metadata_op, false)?, + inner_ops, + post_group_size, + pre_group_size.get(), + )) + } fn convert( &self,