diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index 940d1fd62b..c65bbb7dbc 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -671,6 +671,7 @@ impl ChainStateWriter for ChainStateDB { } fn apply_write_set(&self, write_set: WriteSet) -> Result<()> { + info!("ChainStateWriter::apply_write_set | write set: {:?}", write_set); let mut lock_table_handle = self.updates_table_handle.write(); let mut locks = self.updates.write(); for (state_key, write_op) in write_set { diff --git a/vm/vm-runtime-types/src/resolver.rs b/vm/vm-runtime-types/src/resolver.rs index c0dd93846b..1cf5bab06a 100644 --- a/vm/vm-runtime-types/src/resolver.rs +++ b/vm/vm-runtime-types/src/resolver.rs @@ -89,14 +89,14 @@ 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 +107,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 +127,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()) } @@ -249,6 +249,37 @@ where } } +impl TResourceGroupView for S + where S: StateView { + type GroupKey = StateKey; + type ResourceTag = StructTag; + type Layout = MoveTypeLayout; + + fn is_resource_group_split_in_change_set_capable(&self) -> bool { + todo!() + } + + fn resource_group_size(&self, group_key: &Self::GroupKey) -> PartialVMResult { + todo!() + } + + fn get_resource_from_group(&self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, maybe_layout: Option<&Self::Layout>) -> PartialVMResult> { + todo!() + } + + fn resource_size_in_group(&self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag) -> PartialVMResult { + todo!() + } + + fn resource_exists_in_group(&self, group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag) -> PartialVMResult { + todo!() + } + + fn release_group_cache(&self) -> Option>> { + todo!() + } +} + impl TModuleView for S where S: StateView, @@ -273,6 +304,7 @@ where } } + /// Allows to query storage metadata in the VM session. Needed for storage refunds. /// - Result being Err means storage error or some incostistency (e.g. during speculation, /// needing to abort/halt the transaction with an error status). diff --git a/vm/vm-runtime-types/src/resource_group_adapter.rs b/vm/vm-runtime-types/src/resource_group_adapter.rs index 5007f73d91..b4752cf82a 100644 --- a/vm/vm-runtime-types/src/resource_group_adapter.rs +++ b/vm/vm-runtime-types/src/resource_group_adapter.rs @@ -1,20 +1,22 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::resolver::{ - size_u32_as_uleb128, ResourceGroupSize, ResourceGroupView, TResourceGroupView, TResourceView, -}; -use anyhow::Error; -use bytes::Bytes; -use move_core_types::{language_storage::StructTag, value::MoveTypeLayout}; -use serde::Serialize; -use starcoin_vm_types::state_store::state_key::StateKey; use std::{ cell::RefCell, collections::{BTreeMap, HashMap}, fmt::Debug, }; +use bytes::Bytes; +use move_core_types::{language_storage::StructTag, value::MoveTypeLayout, vm_status::StatusCode}; +use serde::Serialize; + +use starcoin_vm_types::{errors::{PartialVMError, PartialVMResult}, state_store::state_key::StateKey}; + +use crate::resolver::{ + ResourceGroupSize, ResourceGroupView, size_u32_as_uleb128, TResourceGroupView, TResourceView, +}; + /// Corresponding to different gas features, methods for counting the 'size' of a /// resource group. None leads to 0, while AsBlob provides the group size as the /// size of the serialized blob of the BTreeMap corresponding to the group. @@ -46,20 +48,23 @@ 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, @@ -67,6 +72,7 @@ pub fn group_size_as_sum( }) } + #[test] fn test_group_size_same_as_bcs() { use move_core_types::identifier::Identifier; @@ -157,7 +163,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 +171,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 +211,7 @@ 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 +234,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); } @@ -262,17 +274,20 @@ impl TResourceGroupView for ResourceGroupAdapter<'_> { #[cfg(test)] mod tests { - use super::*; + use std::cmp::max; + use claims::{assert_gt, assert_none, assert_ok_eq, assert_some, assert_some_eq}; use move_core_types::account_address::AccountAddress; use move_core_types::identifier::Identifier; use move_core_types::language_storage::TypeTag; + use test_case::test_case; + use starcoin_vm_types::state_store::{ errors::StateviewError, state_storage_usage::StateStorageUsage, state_value::StateValue, TStateView, }; - use std::cmp::max; - use test_case::test_case; + + use super::*; fn mock_tag_0() -> StructTag { StructTag { @@ -388,7 +403,7 @@ mod tests { fn resource_group_size( &self, group_key: &Self::GroupKey, - ) -> anyhow::Result { + ) -> PartialVMResult { Ok(self .group .get(group_key) @@ -401,7 +416,7 @@ mod tests { group_key: &Self::GroupKey, resource_tag: &Self::ResourceTag, _maybe_layout: Option<&Self::Layout>, - ) -> anyhow::Result> { + ) -> PartialVMResult> { Ok(self .group .get(group_key) @@ -412,7 +427,7 @@ mod tests { &self, _group_key: &Self::GroupKey, _resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { unimplemented!("Currently resolved by ResourceGroupAdapter"); } @@ -420,7 +435,7 @@ mod tests { &self, _group_key: &Self::GroupKey, _resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { unimplemented!("Currently resolved by ResourceGroupAdapter"); } diff --git a/vm/vm-runtime/src/data_cache.rs b/vm/vm-runtime/src/data_cache.rs index 9f13d8d2ce..9a059b6624 100644 --- a/vm/vm-runtime/src/data_cache.rs +++ b/vm/vm-runtime/src/data_cache.rs @@ -2,23 +2,29 @@ // SPDX-License-Identifier: Apache-2.0 //! Scratchpad for on chain values during the execution. -use crate::move_vm_ext::AsExecutorView; +use std::{ + cell::RefCell, + collections::btree_map::BTreeMap, + collections::HashSet, + ops::{Deref, DerefMut}, +}; +use std::collections::HashMap; + use bytes::Bytes; -use move_binary_format::deserializer::DeserializerConfig; use move_binary_format::CompiledModule; +use move_binary_format::deserializer::DeserializerConfig; use move_bytecode_utils::compiled_module_viewer::CompiledModuleView; use move_core_types::metadata::Metadata; -use move_core_types::resolver::{resource_size, ModuleResolver, ResourceResolver}; +use move_core_types::resolver::{ModuleResolver, resource_size, ResourceResolver}; use move_core_types::value::MoveTypeLayout; 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::resource_group_adapter::ResourceGroupAdapter; -use starcoin_vm_types::state_store::{ - errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage, - state_value::StateValue, StateView, TStateView, +use starcoin_vm_runtime_types::{ + resolver::{ExecutorView, ResourceGroupSize, ResourceGroupView, TResourceGroupView}, + resource_group_adapter::ResourceGroupAdapter, }; use starcoin_vm_types::{ errors::*, @@ -26,11 +32,15 @@ use starcoin_vm_types::{ vm_status::StatusCode, write_set::{WriteOp, WriteSet}, }; -use std::{ - cell::RefCell, - collections::btree_map::BTreeMap, - collections::HashSet, - ops::{Deref, DerefMut}, +use starcoin_vm_types::state_store::{ + errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage, + state_value::StateValue, StateView, TStateView, +}; + +use crate::move_vm_ext::{ + AsExecutorView, + AsResourceGroupView, + ResourceGroupResolver, }; pub fn get_resource_group_member_from_metadata( @@ -52,9 +62,9 @@ pub fn get_resource_group_member_from_metadata( /// for (non-group) resources and subsequent handling in the StorageAdapter itself. pub struct StorageAdapter<'e, E> { executor_view: &'e E, - _deserializer_config: DeserializerConfig, - _resource_group_view: ResourceGroupAdapter<'e>, - _accessed_groups: RefCell>, + deserializer_config: DeserializerConfig, + resource_group_view: ResourceGroupAdapter<'e>, + accessed_groups: RefCell>, } /// A local cache for a given a `StateView`. The cache is private to the Diem layer @@ -146,6 +156,8 @@ impl<'block, S: StateView> ModuleResolver for StateViewCache<'block, S> { StorageAdapter::new(self).get_module(module_id) } } + + impl<'block, S: StateView> ResourceResolver for StateViewCache<'block, S> { type Error = PartialVMError; @@ -165,14 +177,14 @@ impl<'a, S: StateView> StorageAdapter<'a, S> { pub fn new(state_store: &'a S) -> Self { Self { executor_view: state_store, - _deserializer_config: DeserializerConfig::new(0, 0), - _resource_group_view: ResourceGroupAdapter::new( + deserializer_config: DeserializerConfig::new(0, 0), + resource_group_view: ResourceGroupAdapter::new( None, state_store, LATEST_GAS_FEATURE_VERSION, false, ), - _accessed_groups: RefCell::new(HashSet::new()), + accessed_groups: RefCell::new(HashSet::new()), } } @@ -206,6 +218,7 @@ impl<'a, S: StateView> ModuleResolver for StorageAdapter<'a, S> { self.get(&key).map(|r| r.map(|v| v.bytes().clone())) } } + impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { type Error = PartialVMError; @@ -225,6 +238,37 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { } } +impl<'e, E: StateView> ResourceGroupResolver for StorageAdapter<'e, E> { + 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; @@ -249,6 +293,7 @@ impl<'a, S: StateView> TableResolver for StorageAdapter<'a, S> { }) } } + impl<'a, S: StateView> CompiledModuleView for StorageAdapter<'a, S> { type Item = CompiledModule; fn view_compiled_module(&self, id: &ModuleId) -> anyhow::Result> { @@ -276,6 +321,13 @@ impl<'a, S: StateView> AsExecutorView for StorageAdapter<'a, S> { } } +// Allows to extract the view from `StorageAdapter`. +impl<'e, S> AsResourceGroupView for StorageAdapter<'e, S> { + fn as_resource_group_view(&self) -> &dyn ResourceGroupView { + &self.resource_group_view + } +} + pub struct RemoteStorageOwned { state_view: S, } @@ -321,6 +373,25 @@ impl ResourceResolver for RemoteStorageOwned { } } +impl<'e, S: ResourceGroupView> ResourceGroupResolver for RemoteStorageOwned { + fn release_resource_group_cache(&self) -> Option>> { + self.as_resource_group_view().release_group_cache() + } + + fn resource_group_size(&self, group_key: &StateKey) -> PartialVMResult { + self.as_resource_group_view().resource_group_size(group_key) + } + + fn resource_size_in_group(&self, group_key: &StateKey, resource_tag: &StructTag) -> PartialVMResult { + self.as_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.as_resource_group_view().resource_exists_in_group(group_key, resource_tag) + } +} + + impl TableResolver for RemoteStorageOwned { fn resolve_table_entry_bytes_with_layout( &self, @@ -339,6 +410,13 @@ impl AsExecutorView for RemoteStorageOwned { } } +impl AsResourceGroupView for RemoteStorageOwned { + fn as_resource_group_view(&self) -> &dyn ResourceGroupView { + &self.state_view + } +} + + pub trait IntoMoveResolver { fn into_move_resolver(self) -> RemoteStorageOwned; } @@ -349,10 +427,13 @@ impl IntoMoveResolver for S { } } + #[cfg(test)] pub(crate) mod tests { - use super::*; use starcoin_vm_runtime_types::resource_group_adapter::GroupSizeKind; + + use super::*; + //use starcoin_vm_types::on_chain_config::{Features, OnChainConfig}; // Expose a method to create a storage adapter with a provided group size kind. diff --git a/vm/vm-runtime/src/move_vm_ext/mod.rs b/vm/vm-runtime/src/move_vm_ext/mod.rs index 5c6d9ccee4..3274ea5509 100644 --- a/vm/vm-runtime/src/move_vm_ext/mod.rs +++ b/vm/vm-runtime/src/move_vm_ext/mod.rs @@ -3,6 +3,19 @@ //! MoveVM and Session wrapped, to make sure Starcoin natives and extensions are always installed and //! taken care of after session finish. +use move_binary_format::errors::{PartialVMError, PartialVMResult}; +use move_core_types::{ + account_address::AccountAddress, language_storage::StructTag, vm_status::StatusCode, +}; + +use starcoin_vm_types::state_store::state_key::StateKey; + +pub use crate::move_vm_ext::{ + resolver::{AsExecutorView, AsResourceGroupView, ResourceGroupResolver, StarcoinMoveResolver}, + session::{SessionExt, SessionId}, + vm::MoveVmExt, +}; + mod resolver; mod session; mod vm; @@ -11,17 +24,6 @@ mod warm_vm_cache; pub(crate) mod write_op_converter; -pub use crate::move_vm_ext::{ - resolver::{AsExecutorView, StarcoinMoveResolver}, - session::{SessionExt, SessionId}, - vm::MoveVmExt, -}; -use move_binary_format::errors::{PartialVMError, PartialVMResult}; -use move_core_types::{ - account_address::AccountAddress, language_storage::StructTag, vm_status::StatusCode, -}; -use starcoin_vm_types::state_store::state_key::StateKey; - pub(crate) fn resource_state_key( address: &AccountAddress, tag: &StructTag, diff --git a/vm/vm-runtime/src/move_vm_ext/resolver.rs b/vm/vm-runtime/src/move_vm_ext/resolver.rs index bf665b6512..cd32fa0efa 100644 --- a/vm/vm-runtime/src/move_vm_ext/resolver.rs +++ b/vm/vm-runtime/src/move_vm_ext/resolver.rs @@ -1,12 +1,19 @@ // 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 std::collections::{BTreeMap, HashMap}; + +use bytes::Bytes; +use move_binary_format::errors::{PartialVMError, PartialVMResult}; +use move_core_types::{language_storage::StructTag, resolver::MoveResolver}; use move_table_extension::TableResolver; + use starcoin_aggregator::resolver::{AggregatorV1Resolver, DelayedFieldResolver}; -use starcoin_vm_runtime_types::resolver::ExecutorView; -use starcoin_vm_types::on_chain_config::ConfigStorage; +use starcoin_vm_runtime_types::resolver::{ExecutorView, ResourceGroupSize, ResourceGroupView}; +use starcoin_vm_types::{ + on_chain_config::ConfigStorage, + state_store::state_key::StateKey +}; /// A general resolver used by StarcoinVM. Allows to implement custom hooks on /// top of storage, e.g. get resources from resource groups, etc. @@ -16,8 +23,10 @@ pub trait StarcoinMoveResolver: + DelayedFieldResolver + ConfigStorage + MoveResolver + + ResourceGroupResolver + TableResolver + AsExecutorView + + AsResourceGroupView { } @@ -26,31 +35,37 @@ impl< + ConfigStorage + DelayedFieldResolver + MoveResolver + + ResourceGroupResolver + TableResolver - + AsExecutorView, + + AsExecutorView + + AsResourceGroupView > 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; } + +pub trait AsResourceGroupView { + fn as_resource_group_view(&self) -> &dyn ResourceGroupView; +} diff --git a/vm/vm-runtime/src/move_vm_ext/session.rs b/vm/vm-runtime/src/move_vm_ext/session.rs index ceaf8852a2..0e1342c09b 100644 --- a/vm/vm-runtime/src/move_vm_ext/session.rs +++ b/vm/vm-runtime/src/move_vm_ext/session.rs @@ -275,7 +275,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 +286,17 @@ 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,38 +332,38 @@ 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(( @@ -374,14 +375,14 @@ impl<'r, 'l> SessionExt<'r, 'l> { 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 +413,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..528c2a50eb 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 @@ -1,19 +1,25 @@ // Copyright (c) The Starcoin Core Contributors // SPDX-License-Identifier: Apache-2.0 +use std::collections::BTreeMap; use crate::move_vm_ext::{session::BytesWithResourceLayout, StarcoinMoveResolver}; use bytes::Bytes; use move_binary_format::errors::{PartialVMError, PartialVMResult}; 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::resolver::ResourceGroupSize; +use starcoin_vm_runtime_types::{ + resolver::ResourceGroupSize, + abstract_write_op::GroupWrite, + 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::sync::Arc; +use move_core_types::language_storage::StructTag; pub(crate) struct WriteOpConverter<'r> { remote: &'r dyn StarcoinMoveResolver, @@ -183,74 +189,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,