From 915ea62caa43469d9660fadcbd22f5176f4c1e14 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Wed, 15 Jan 2025 18:34:50 +0800 Subject: [PATCH 01/11] support resource group in vm-runtime --- vm/vm-runtime-types/src/resolver.rs | 9 +- .../src/resource_group_adapter.rs | 46 ++++-- vm/vm-runtime/src/data_cache.rs | 69 ++++++++- vm/vm-runtime/src/move_vm_ext/mod.rs | 2 +- vm/vm-runtime/src/move_vm_ext/resolver.rs | 50 ++++--- vm/vm-runtime/src/move_vm_ext/session.rs | 124 ++++++++-------- .../src/move_vm_ext/write_op_converter.rs | 140 +++++++++--------- 7 files changed, 262 insertions(+), 178 deletions(-) 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, From 27be287206131b5a900dc7b69424095978b54c24 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Wed, 15 Jan 2025 20:46:19 +0800 Subject: [PATCH 02/11] support ResourceGroup StateKey --- chain/src/chain.rs | 4 ++-- cmd/starcoin/src/state/get_proof_cmd.rs | 4 +++- state/statedb/src/lib.rs | 16 +++++++++++----- vm/e2e-tests/src/data_store.rs | 8 ++++++-- vm/types/src/access_path.rs | 2 +- vm/types/src/state_store/state_key/mod.rs | 1 + 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 5ba2b9d2d1..b36441ea8b 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -2099,8 +2099,8 @@ impl ChainReader for BlockChain { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, &struct_tag)? } - DataPath::ResourceGroup(_) => { - bail!("ResourceGroup is not supported in get_transaction_proof") + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, &struct_tag) } }; Some(statedb.get_with_proof(&state_key)?) diff --git a/cmd/starcoin/src/state/get_proof_cmd.rs b/cmd/starcoin/src/state/get_proof_cmd.rs index fbf7e58981..55e5a1b3f2 100644 --- a/cmd/starcoin/src/state/get_proof_cmd.rs +++ b/cmd/starcoin/src/state/get_proof_cmd.rs @@ -72,7 +72,9 @@ impl CommandAction for GetProofCommand { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, &struct_tag)? } - DataPath::ResourceGroup(_) => Err(anyhow::anyhow!("ResourceGroup is not supported."))?, + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, &struct_tag) + } }; let (proof, result) = if opt.raw { diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index 940d1fd62b..1715902cbf 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -112,7 +112,7 @@ impl AccountStateObject { .transpose()? .flatten()), DataPath::Resource(struct_tag) => self.resource_tree.lock().get(struct_tag), - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(_) => unimplemented!("ResourceGroup not support get"), } } @@ -131,7 +131,9 @@ impl AccountStateObject { .transpose()? .unwrap_or((None, SparseMerkleProof::new(None, vec![])))), DataPath::Resource(struct_tag) => self.resource_tree.lock().get_with_proof(struct_tag), - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(_) => { + unimplemented!("ResourceGroup not support get_with_proof") + } } } @@ -151,7 +153,7 @@ impl AccountStateObject { DataPath::Resource(struct_tag) => { self.resource_tree.lock().put(struct_tag, value); } - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(_) => unimplemented!("ResourceGroup not support set"), } } @@ -617,7 +619,9 @@ impl ChainStateWriter for ChainStateDB { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, struct_tag)? } - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, struct_tag) + } } }; self.apply_write_set( @@ -634,7 +638,9 @@ impl ChainStateWriter for ChainStateDB { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, struct_tag)? } - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, struct_tag) + } } }; self.apply_write_set( diff --git a/vm/e2e-tests/src/data_store.rs b/vm/e2e-tests/src/data_store.rs index 30023a1825..7cd0645dbb 100644 --- a/vm/e2e-tests/src/data_store.rs +++ b/vm/e2e-tests/src/data_store.rs @@ -127,7 +127,9 @@ impl ChainStateWriter for FakeDataStore { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, struct_tag)? } - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, struct_tag) + } } }; self.set(state_key, value); @@ -142,7 +144,9 @@ impl ChainStateWriter for FakeDataStore { DataPath::Resource(struct_tag) => { StateKey::resource(&access_path.address, struct_tag)? } - DataPath::ResourceGroup(_) => unimplemented!(), + DataPath::ResourceGroup(struct_tag) => { + StateKey::resource_group(&access_path.address, struct_tag) + } } }; self.remove(&state_key); diff --git a/vm/types/src/access_path.rs b/vm/types/src/access_path.rs index 217aecccde..82b3c1394e 100644 --- a/vm/types/src/access_path.rs +++ b/vm/types/src/access_path.rs @@ -365,7 +365,7 @@ impl fmt::Display for DataPath { write!(f, "{}/{}", storage_index, module_name) } DataPath::ResourceGroup(struct_tag) => { - write!(f, "ResourceGroup({})", struct_tag) + write!(f, "{}/{}", storage_index, struct_tag) } } } diff --git a/vm/types/src/state_store/state_key/mod.rs b/vm/types/src/state_store/state_key/mod.rs index ddcb496182..e534cf927b 100644 --- a/vm/types/src/state_store/state_key/mod.rs +++ b/vm/types/src/state_store/state_key/mod.rs @@ -126,6 +126,7 @@ impl StateKey { Ok(myself) } + // todo: remove Result pub fn resource(address: &AccountAddress, struct_tag: &StructTag) -> Result { Ok(Self(REGISTRY.resource(struct_tag, address).get_or_add( struct_tag, From 07b56dfaf6167580c43321b6f017e5f8023d30a1 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Thu, 16 Jan 2025 07:39:58 +0800 Subject: [PATCH 03/11] fix build errors --- vm/vm-runtime-types/src/resource_group_adapter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vm/vm-runtime-types/src/resource_group_adapter.rs b/vm/vm-runtime-types/src/resource_group_adapter.rs index 2deda14793..91a3491b84 100644 --- a/vm/vm-runtime-types/src/resource_group_adapter.rs +++ b/vm/vm-runtime-types/src/resource_group_adapter.rs @@ -402,7 +402,7 @@ mod tests { fn resource_group_size( &self, group_key: &Self::GroupKey, - ) -> anyhow::Result { + ) -> PartialVMResult { Ok(self .group .get(group_key) @@ -415,7 +415,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) @@ -426,7 +426,7 @@ mod tests { &self, _group_key: &Self::GroupKey, _resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { unimplemented!("Currently resolved by ResourceGroupAdapter"); } @@ -434,7 +434,7 @@ mod tests { &self, _group_key: &Self::GroupKey, _resource_tag: &Self::ResourceTag, - ) -> anyhow::Result { + ) -> PartialVMResult { unimplemented!("Currently resolved by ResourceGroupAdapter"); } From 796642a90e7251d0caa66670ffd9a4a36e1076a6 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Thu, 16 Jan 2025 07:58:10 +0800 Subject: [PATCH 04/11] avoid panic for some resource group operations --- state/statedb/src/lib.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index 1715902cbf..c75893a345 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -112,7 +112,10 @@ impl AccountStateObject { .transpose()? .flatten()), DataPath::Resource(struct_tag) => self.resource_tree.lock().get(struct_tag), - DataPath::ResourceGroup(_) => unimplemented!("ResourceGroup not support get"), + DataPath::ResourceGroup(_) => { + debug!("resource_group_tree not support get"); + Ok(None) + } } } @@ -132,7 +135,8 @@ impl AccountStateObject { .unwrap_or((None, SparseMerkleProof::new(None, vec![])))), DataPath::Resource(struct_tag) => self.resource_tree.lock().get_with_proof(struct_tag), DataPath::ResourceGroup(_) => { - unimplemented!("ResourceGroup not support get_with_proof") + debug!("resource_group_tree not support get"); + Ok((None, SparseMerkleProof::new(None, vec![]))) } } } @@ -153,7 +157,9 @@ impl AccountStateObject { DataPath::Resource(struct_tag) => { self.resource_tree.lock().put(struct_tag, value); } - DataPath::ResourceGroup(_) => unimplemented!("ResourceGroup not support set"), + DataPath::ResourceGroup(_) => { + debug!("resource_group_tree not support set") + } } } From 38f9ad6e7337c2dff10a91c3b99aba0cf75e1ca4 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Fri, 17 Jan 2025 17:17:44 +0800 Subject: [PATCH 05/11] fix parsing Accesspath from string --- vm/types/src/access_path.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vm/types/src/access_path.rs b/vm/types/src/access_path.rs index 82b3c1394e..d3d7d5b84c 100644 --- a/vm/types/src/access_path.rs +++ b/vm/types/src/access_path.rs @@ -331,6 +331,10 @@ impl DataPath { pub fn is_resource(&self) -> bool { matches!(self, Self::Resource(_)) } + pub fn is_resource_group(&self) -> bool { + matches!(self, Self::ResourceGroup(_)) + } + // todo: handle ResourceGroup pub fn as_struct_tag(&self) -> Option<&StructTag> { match self { Self::Resource(struct_tag) => Some(struct_tag), @@ -384,7 +388,7 @@ impl FromStr for AccessPath { let data_path = match data_type { DataType::CODE => Self::code_data_path(Identifier::new(parts[2])?), DataType::RESOURCE => Self::resource_data_path(parse_struct_tag(parts[2])?), - DataType::ResourceGroup => Self::resource_data_path(parse_struct_tag(parts[2])?), + DataType::ResourceGroup => Self::resource_group_data_path(parse_struct_tag(parts[2])?), }; Ok(Self::new(address, data_path)) } From a46367f310693b41540208a31b9f60b77a9f3a9b Mon Sep 17 00:00:00 2001 From: simonjiao Date: Fri, 17 Jan 2025 17:35:39 +0800 Subject: [PATCH 06/11] store ResourceGroup in resource_tree --- state/statedb/src/lib.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index c75893a345..11fa6c3d78 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -113,8 +113,7 @@ impl AccountStateObject { .flatten()), DataPath::Resource(struct_tag) => self.resource_tree.lock().get(struct_tag), DataPath::ResourceGroup(_) => { - debug!("resource_group_tree not support get"); - Ok(None) + bail!("resource_group_tree not support get"); } } } @@ -135,8 +134,7 @@ impl AccountStateObject { .unwrap_or((None, SparseMerkleProof::new(None, vec![])))), DataPath::Resource(struct_tag) => self.resource_tree.lock().get_with_proof(struct_tag), DataPath::ResourceGroup(_) => { - debug!("resource_group_tree not support get"); - Ok((None, SparseMerkleProof::new(None, vec![]))) + bail!("resource_group_tree not support get"); } } } @@ -157,8 +155,9 @@ impl AccountStateObject { DataPath::Resource(struct_tag) => { self.resource_tree.lock().put(struct_tag, value); } - DataPath::ResourceGroup(_) => { - debug!("resource_group_tree not support set") + DataPath::ResourceGroup(struct_tag) => { + eprintln!("treat resource_group as resource"); + self.resource_tree.lock().put(struct_tag, value); } } } From 6caa2026319eecb4fd1f54dee5d2bb704baa1a80 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Mon, 20 Jan 2025 10:06:16 +0800 Subject: [PATCH 07/11] get resource from group if necessary --- vm/vm-runtime/src/data_cache.rs | 64 +++++++++++++++++++----- vm/vm-runtime/src/move_vm_ext/session.rs | 4 ++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/vm/vm-runtime/src/data_cache.rs b/vm/vm-runtime/src/data_cache.rs index 1303043a10..f6246b7c60 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, ResourceGroupResolver}; +use crate::move_vm_ext::{resource_state_key, AsExecutorView, ResourceGroupResolver}; use bytes::Bytes; use move_binary_format::deserializer::DeserializerConfig; use move_binary_format::CompiledModule; @@ -14,7 +14,9 @@ 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, ResourceGroupSize, TResourceGroupView}; +use starcoin_vm_runtime_types::resolver::{ + ExecutorView, ResourceGroupSize, TResourceGroupView, TResourceView, +}; use starcoin_vm_runtime_types::resource_group_adapter::ResourceGroupAdapter; use starcoin_vm_types::state_store::{ errors::StateviewError, state_key::StateKey, state_storage_usage::StateStorageUsage, @@ -38,7 +40,19 @@ pub fn get_resource_group_member_from_metadata( struct_tag: &StructTag, metadata: &[Metadata], ) -> Option { + eprintln!( + "get_resource_group_member_from_metadata {} origin metadata count {}", + struct_tag, + metadata.len() + ); + if metadata.is_empty() && struct_tag.name.as_ident_str().as_str() == "ObjectCore" { + panic!("let's see the backtrace"); + } let metadata = starcoin_framework::get_metadata(metadata)?; + eprintln!( + "get_resource_group_member_from_metadata {} metadata struct_attributes {:?}", + struct_tag, metadata.struct_attributes + ); metadata .struct_attributes .get(struct_tag.name.as_ident_str().as_str())? @@ -55,7 +69,7 @@ pub struct StorageAdapter<'e, E> { executor_view: &'e E, _deserializer_config: DeserializerConfig, resource_group_view: ResourceGroupAdapter<'e>, - _accessed_groups: RefCell>, + accessed_groups: RefCell>, } /// A local cache for a given a `StateView`. The cache is private to the Diem layer @@ -173,7 +187,7 @@ impl<'a, S: StateView> StorageAdapter<'a, S> { LATEST_GAS_FEATURE_VERSION, false, ), - _accessed_groups: RefCell::new(HashSet::new()), + accessed_groups: RefCell::new(HashSet::new()), } } @@ -210,19 +224,45 @@ impl<'a, S: StateView> ModuleResolver for StorageAdapter<'a, S> { impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { type Error = PartialVMError; - // TODO(simon): don't ignore metadata and layout fn get_resource_bytes_with_metadata_and_layout( &self, address: &AccountAddress, struct_tag: &StructTag, - _metadata: &[Metadata], - _layout: Option<&MoveTypeLayout>, + metadata: &[Metadata], + maybe_layout: Option<&MoveTypeLayout>, ) -> Result<(Option, usize), Self::Error> { - let key = StateKey::resource(address, struct_tag) - .map_err(|_| PartialVMError::new(StatusCode::STORAGE_ERROR))?; - let buf = self.get(&key)?.map(|v| v.bytes().clone()); - let size = resource_size(&buf); - Ok((buf, size)) + let resource_group = get_resource_group_member_from_metadata(struct_tag, metadata); + if let Some(resource_group) = resource_group { + eprintln!( + "get_resource_bytes_with_metadata_and_layout {} from group", + struct_tag + ); + let key = StateKey::resource_group(address, &resource_group); + let buf = + self.resource_group_view + .get_resource_from_group(&key, struct_tag, maybe_layout)?; + + let first_access = self.accessed_groups.borrow_mut().insert(key.clone()); + let group_size = if first_access { + self.resource_group_view.resource_group_size(&key)?.get() + } else { + 0 + }; + + let buf_size = resource_size(&buf); + Ok((buf, buf_size + group_size as usize)) + } else { + eprintln!( + "get_resource_bytes_with_metadata_and_layout {} from executor_view", + struct_tag + ); + let state_key = resource_state_key(address, struct_tag)?; + let buf = self + .executor_view + .get_resource_bytes(&state_key, maybe_layout)?; + let buf_size = resource_size(&buf); + Ok((buf, buf_size)) + } } } diff --git a/vm/vm-runtime/src/move_vm_ext/session.rs b/vm/vm-runtime/src/move_vm_ext/session.rs index 6ad75a19e9..94f9008872 100644 --- a/vm/vm-runtime/src/move_vm_ext/session.rs +++ b/vm/vm-runtime/src/move_vm_ext/session.rs @@ -309,6 +309,10 @@ impl<'r, 'l> SessionExt<'r, 'l> { get_resource_group_member_from_metadata(&struct_tag, md) }); + eprintln!( + "struct_tag: {:?} resource group tag {:?}", + struct_tag, resource_group_tag + ); if let Some(resource_group_tag) = resource_group_tag { if resource_groups .entry(resource_group_tag) From 72901aa5b4418a1d9bf2cd3fdb3d7028da90e94f Mon Sep 17 00:00:00 2001 From: simonjiao Date: Mon, 20 Jan 2025 10:19:41 +0800 Subject: [PATCH 08/11] add more codes for debugging --- Cargo.toml | 2 +- state/statedb/src/lib.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dd3ef3394d..8ac521ea7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -525,7 +525,7 @@ starcoin-crypto = { git = "https://github.com/starcoinorg/starcoin-crypto", rev starcoin-decrypt = { path = "commons/decrypt" } starcoin-dev = { path = "vm/dev" } starcoin-executor = { path = "executor" } -starcoin-framework = { path = "vm/framework" } +starcoin-framework = { path = "vm/framework", features = ["testing"] } starcoin-sdk-builder = { path = "vm/starcoin-sdk-builder" } starcoin-genesis = { path = "genesis" } starcoin-logger = { path = "commons/logger" } diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index 11fa6c3d78..d491c7e0db 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -112,8 +112,12 @@ impl AccountStateObject { .transpose()? .flatten()), DataPath::Resource(struct_tag) => self.resource_tree.lock().get(struct_tag), - DataPath::ResourceGroup(_) => { - bail!("resource_group_tree not support get"); + DataPath::ResourceGroup(struct_tag) => { + eprintln!( + "redirect getting resource_group_tree to resource_tree {}", + data_path + ); + self.resource_tree.lock().get(struct_tag) } } } From 09ab27effee489129d07f7d8dcebf8fe820f1570 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Tue, 21 Jan 2025 13:23:22 +0800 Subject: [PATCH 09/11] add a test for resource group --- executor/tests/script_function_test.rs | 37 ++++++++++++++++++++++++-- vm/vm-runtime/src/data_cache.rs | 3 --- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/executor/tests/script_function_test.rs b/executor/tests/script_function_test.rs index 131a2e452a..3feefb03f1 100644 --- a/executor/tests/script_function_test.rs +++ b/executor/tests/script_function_test.rs @@ -21,8 +21,8 @@ use starcoin_vm_types::transaction::{ use starcoin_vm_types::vm_status::KeptVMStatus; use std::ops::Sub; use test_helper::executor::{ - compile_ir_script, compile_modules_with_address, compile_script, execute_and_apply, - prepare_genesis, + association_execute_should_success, compile_ir_script, compile_modules_with_address, + compile_script, execute_and_apply, prepare_genesis, }; use test_helper::txn::create_account_txn_sent_as_association; @@ -463,3 +463,36 @@ fn test_transaction_arg_verify() -> Result<()> { ); Ok(()) } + +// this test aims to check if resource group works well +#[stest::test] +fn test_object_metadata() -> Result<()> { + let (chain_state, net) = prepare_genesis(); + + let script = prepare_script( + SyntaxChoice::Source, + r#" + script{ + use std::option; + use std::string; + use starcoin_std::debug; + use starcoin_framework::object; + use starcoin_framework::coin; + use starcoin_framework::starcoin_coin::STC; + + fun test_metadata(_account: &signer) { + debug::print(&string::utf8(b"test_metadata | entered")); + let metadata = coin::paired_metadata(); + assert!(option::is_some(&metadata), 10000); + let metdata_obj = option::destroy_some(metadata); + assert!(object::is_object(object::object_address(&metdata_obj)), 10001); + debug::print(&string::utf8(b"test_metadata | exited")); + } + } + "#, + )?; + let payload = TransactionPayload::Script(Script::new(script, vec![], vec![])); + let output = association_execute_should_success(&net, &chain_state, payload)?; + assert_eq!(KeptVMStatus::Executed, output.status().status().unwrap()); + Ok(()) +} diff --git a/vm/vm-runtime/src/data_cache.rs b/vm/vm-runtime/src/data_cache.rs index f6246b7c60..d579ca75c5 100644 --- a/vm/vm-runtime/src/data_cache.rs +++ b/vm/vm-runtime/src/data_cache.rs @@ -45,9 +45,6 @@ pub fn get_resource_group_member_from_metadata( struct_tag, metadata.len() ); - if metadata.is_empty() && struct_tag.name.as_ident_str().as_str() == "ObjectCore" { - panic!("let's see the backtrace"); - } let metadata = starcoin_framework::get_metadata(metadata)?; eprintln!( "get_resource_group_member_from_metadata {} metadata struct_attributes {:?}", From 4c54a7e1f347e976a44f891d13c18e55d2260b79 Mon Sep 17 00:00:00 2001 From: simonjiao Date: Tue, 21 Jan 2025 14:05:53 +0800 Subject: [PATCH 10/11] remove debug message --- Cargo.toml | 2 +- state/statedb/src/lib.rs | 14 ++++---------- vm/types/src/access_path.rs | 2 +- vm/vm-runtime/src/data_cache.rs | 17 ----------------- vm/vm-runtime/src/move_vm_ext/session.rs | 4 ---- 5 files changed, 6 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8ac521ea7e..dd3ef3394d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -525,7 +525,7 @@ starcoin-crypto = { git = "https://github.com/starcoinorg/starcoin-crypto", rev starcoin-decrypt = { path = "commons/decrypt" } starcoin-dev = { path = "vm/dev" } starcoin-executor = { path = "executor" } -starcoin-framework = { path = "vm/framework", features = ["testing"] } +starcoin-framework = { path = "vm/framework" } starcoin-sdk-builder = { path = "vm/starcoin-sdk-builder" } starcoin-genesis = { path = "genesis" } starcoin-logger = { path = "commons/logger" } diff --git a/state/statedb/src/lib.rs b/state/statedb/src/lib.rs index d491c7e0db..28ba9563ba 100644 --- a/state/statedb/src/lib.rs +++ b/state/statedb/src/lib.rs @@ -75,6 +75,7 @@ struct AccountStateObject { // refactor AccountStateObject to a readonly object. code_tree: Mutex>>, resource_tree: Mutex>, + // todo(simon) add resource_group_tree store: Arc, } @@ -112,13 +113,7 @@ impl AccountStateObject { .transpose()? .flatten()), DataPath::Resource(struct_tag) => self.resource_tree.lock().get(struct_tag), - DataPath::ResourceGroup(struct_tag) => { - eprintln!( - "redirect getting resource_group_tree to resource_tree {}", - data_path - ); - self.resource_tree.lock().get(struct_tag) - } + DataPath::ResourceGroup(struct_tag) => self.resource_tree.lock().get(struct_tag), } } @@ -137,8 +132,8 @@ impl AccountStateObject { .transpose()? .unwrap_or((None, SparseMerkleProof::new(None, vec![])))), DataPath::Resource(struct_tag) => self.resource_tree.lock().get_with_proof(struct_tag), - DataPath::ResourceGroup(_) => { - bail!("resource_group_tree not support get"); + DataPath::ResourceGroup(struct_tag) => { + self.resource_tree.lock().get_with_proof(struct_tag) } } } @@ -160,7 +155,6 @@ impl AccountStateObject { self.resource_tree.lock().put(struct_tag, value); } DataPath::ResourceGroup(struct_tag) => { - eprintln!("treat resource_group as resource"); self.resource_tree.lock().put(struct_tag, value); } } diff --git a/vm/types/src/access_path.rs b/vm/types/src/access_path.rs index d3d7d5b84c..294243d4b5 100644 --- a/vm/types/src/access_path.rs +++ b/vm/types/src/access_path.rs @@ -334,7 +334,7 @@ impl DataPath { pub fn is_resource_group(&self) -> bool { matches!(self, Self::ResourceGroup(_)) } - // todo: handle ResourceGroup + // todo(simon): handle ResourceGroup pub fn as_struct_tag(&self) -> Option<&StructTag> { match self { Self::Resource(struct_tag) => Some(struct_tag), diff --git a/vm/vm-runtime/src/data_cache.rs b/vm/vm-runtime/src/data_cache.rs index d579ca75c5..2a25732626 100644 --- a/vm/vm-runtime/src/data_cache.rs +++ b/vm/vm-runtime/src/data_cache.rs @@ -40,16 +40,7 @@ pub fn get_resource_group_member_from_metadata( struct_tag: &StructTag, metadata: &[Metadata], ) -> Option { - eprintln!( - "get_resource_group_member_from_metadata {} origin metadata count {}", - struct_tag, - metadata.len() - ); let metadata = starcoin_framework::get_metadata(metadata)?; - eprintln!( - "get_resource_group_member_from_metadata {} metadata struct_attributes {:?}", - struct_tag, metadata.struct_attributes - ); metadata .struct_attributes .get(struct_tag.name.as_ident_str().as_str())? @@ -230,10 +221,6 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { ) -> Result<(Option, usize), Self::Error> { let resource_group = get_resource_group_member_from_metadata(struct_tag, metadata); if let Some(resource_group) = resource_group { - eprintln!( - "get_resource_bytes_with_metadata_and_layout {} from group", - struct_tag - ); let key = StateKey::resource_group(address, &resource_group); let buf = self.resource_group_view @@ -249,10 +236,6 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { let buf_size = resource_size(&buf); Ok((buf, buf_size + group_size as usize)) } else { - eprintln!( - "get_resource_bytes_with_metadata_and_layout {} from executor_view", - struct_tag - ); let state_key = resource_state_key(address, struct_tag)?; let buf = self .executor_view diff --git a/vm/vm-runtime/src/move_vm_ext/session.rs b/vm/vm-runtime/src/move_vm_ext/session.rs index 94f9008872..6ad75a19e9 100644 --- a/vm/vm-runtime/src/move_vm_ext/session.rs +++ b/vm/vm-runtime/src/move_vm_ext/session.rs @@ -309,10 +309,6 @@ impl<'r, 'l> SessionExt<'r, 'l> { get_resource_group_member_from_metadata(&struct_tag, md) }); - eprintln!( - "struct_tag: {:?} resource group tag {:?}", - struct_tag, resource_group_tag - ); if let Some(resource_group_tag) = resource_group_tag { if resource_groups .entry(resource_group_tag) From e914d5620cba4942c9ba4f1078b1abe6dddb65df Mon Sep 17 00:00:00 2001 From: simonjiao Date: Thu, 23 Jan 2025 11:32:56 +0800 Subject: [PATCH 11/11] update generated genesis --- genesis/generated/barnard/genesis | Bin 272591 -> 272591 bytes genesis/generated/halley/genesis | Bin 270303 -> 270303 bytes genesis/generated/main/genesis | Bin 272640 -> 272640 bytes genesis/generated/proxima/genesis | Bin 273989 -> 273989 bytes genesis/generated/vega/genesis | Bin 272640 -> 272640 bytes 5 files changed, 0 insertions(+), 0 deletions(-) diff --git a/genesis/generated/barnard/genesis b/genesis/generated/barnard/genesis index 53f089b9ea6a658e6153010d555d5d7ff45b346c..7ab78950ed132bc2b5cd5c830d56ad443baa7efa 100644 GIT binary patch delta 100 zcmV-q0Gt2M(Gbtk5RgJ3KE@`WEig7ZE-(#+!W`I37H5Lt-$){XgqpJ}^FNX}kzyhs zeo8dAE{9fURFsL}SMHG*TJ)G})Y?cxr3fI0wyp3VlL3kyk-!Oo%7w}Sg~|ej$^*5^ G1O+IR7c1xh delta 100 zcmV-q0Gt2M(Gbtk5RgJ3W`zMn%`1g3tjrPf5fhqr9I}xBrW?H0l_CEHKdnE$kzyhs zjK##~G1KZ{jl)jQqIyZ#v{*svp?k!fWtki5@Cq8PlL3kyk-!Oo%7w}Sg~|ej$^*5^ G1O+Hr6fao- diff --git a/genesis/generated/halley/genesis b/genesis/generated/halley/genesis index 99a484af2086aff04e507aa734f980bf24dedac3..d1fb32b5285fe6feb668131e6648f603846413b6 100644 GIT binary patch delta 100 zcmV-q0Gt2czYyQQ5RgJ3=TdDs8xk$lXIx%5>-Z;RRq@&w*QFmzF4!8>#00k?kzyhs z_xZb7jRlqlS}1z>2RU<{98t*0S-)WfMBvN!V-ld=lL3kyk-!Rp%Z1AUh06kk%LBE` G1QZ_-YAs^` delta 100 zcmV-q0Gt2czYyQQ5RgJ3L%r=0L$M`e;(WyuA_M#;g{_gzW6i-{@??MpE1+j!kzyhs zHd2`EfwP+@8Dfhkkl;bCwA~=BkQ$j7k_6XOu$L#qlL3kyk-!Rp%Z1AUh06kk%LBE` G1QZ`SVJ(pW diff --git a/genesis/generated/main/genesis b/genesis/generated/main/genesis index 9778cb16b79cd80d9d96874acd7278e1e298ca18..278333d7390b97fc4354394f7a017c8ded40b42f 100644 GIT binary patch delta 100 zcmV-q0Gt1S(hz{s5RgJ3WBbflj5U8_S5|{oku}NlNI5fvTUg6)?`B^&h$NmMkzyhs zkFTx0y04(Uj$ABHwA$Ol6e4FmZJFB>+noKix@yFrlL3kyk-!Ln$%V-Qg~0!_f=K9~%Q9ZV`ynUAr!Wl`kzyhs zyk=$rQ*Y{_9Oik^G-8LxPAm!LWBol#Uur)B?|G`ulL3kyk-!Ln$%V-Qg~Tz$l2kzyhs z!L{tfLRk#WEJwCR)3vQ7$~+)m81m;AV>>h)_15T5lL3kyk-!Rp%Z1AUh06kk%LBE` G1bQtVw=p09 delta 100 zcmV-q0Gt2C+z`dw5RgJ3$HA3`N3I{8O9ijD1qlx8aNLt~a8XL+{+XrorYs-7kzyhs zD2@XIZ(R!|3SPrSumL#{VsMHQEA?}w&*buN+uZTAlL3kyk-!Rp%Z1AUh06kk%LBE` G1bQtNIW6}9 diff --git a/genesis/generated/vega/genesis b/genesis/generated/vega/genesis index e431c07bbdafa42a53f3968c89c477131aa960fd..65ac505b4c24a3e6e85e60250ad00dde138ba457 100644 GIT binary patch delta 100 zcmV-q0Gt1S(hz{s5RgJ3hg<*Y@o4TG>bBc`>%h}*!KJF@MW?YV#5(gXf^0Q`kzyhs zeymn_zZeX(N?yq6YCX>j3(iChS(0;cRg{xXTIyC#lL3kyk-!Rp%Z1AUh06kk%LBE` G1T`o&VlYbp delta 100 zcmV-q0Gt1S(hz{s5RgJ3#a%f|Zg`02w3pR;E~}HGKd*9}cd`36(P;c&w^cFYkzyhs zO|{DU{fVUHtqNrc_oevkUK0iB{q<3gB64NEL?bt{lL3kyk-!Rp%Z1AUh06kk%LBE` G1T`osW-