From b67a883abad2f446695065afb0910fa44b690edd Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Sat, 24 Aug 2024 00:25:22 +0100 Subject: [PATCH] comments: use delegate where possible --- Cargo.lock | 16 ++++- Cargo.toml | 1 + aptos-move/aptos-vm-types/Cargo.toml | 1 + .../state_view_adapter.rs | 70 ++----------------- third_party/move/move-vm/runtime/Cargo.toml | 1 + third_party/move/move-vm/runtime/src/lib.rs | 4 +- .../runtime/src/storage/code_storage.rs | 2 + .../implementations/unsync_code_storage.rs | 60 ++-------------- .../runtime/src/storage/module_storage.rs | 2 + .../move-vm/runtime/src/storage/publishing.rs | 58 ++------------- 10 files changed, 40 insertions(+), 175 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5115458fea0a42..87bd81265cd978 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -158,6 +158,18 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0942ffc6dcaadf03badf6e6a2d0228460359d5e34b57ccdc720b7382dfbd5ec5" +[[package]] +name = "ambassador" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b27ba24e4d8a188489d5a03c7fabc167a60809a383cdb4d15feb37479cd2a48" +dependencies = [ + "itertools 0.10.5", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "android-tzdata" version = "0.1.1" @@ -4425,6 +4437,7 @@ dependencies = [ name = "aptos-vm-types" version = "0.0.1" dependencies = [ + "ambassador", "anyhow", "aptos-aggregator", "aptos-gas-algebra", @@ -11431,6 +11444,7 @@ dependencies = [ name = "move-vm-runtime" version = "0.1.0" dependencies = [ + "ambassador", "anyhow", "better_any", "bytes", @@ -13385,7 +13399,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18bec9b0adc4eba778b33684b7ba3e7137789434769ee3ce3930463ef904cfca" dependencies = [ "anyhow", - "itertools 0.12.1", + "itertools 0.13.0", "proc-macro2", "quote", "syn 2.0.48", diff --git a/Cargo.toml b/Cargo.toml index 175e667c737830..0d7391e8dac902 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -464,6 +464,7 @@ atty = "0.2.14" nalgebra = "0.32" float-cmp = "0.9.0" again = "0.1.2" +ambassador = "0.4.1" anyhow = "1.0.71" anstyle = "1.0.1" arbitrary = { version = "1.3.2", features = ["derive"] } diff --git a/aptos-move/aptos-vm-types/Cargo.toml b/aptos-move/aptos-vm-types/Cargo.toml index b2c5155461224d..d2188639c5e3aa 100644 --- a/aptos-move/aptos-vm-types/Cargo.toml +++ b/aptos-move/aptos-vm-types/Cargo.toml @@ -13,6 +13,7 @@ repository = { workspace = true } rust-version = { workspace = true } [dependencies] +ambassador = { workspace = true } anyhow = { workspace = true } aptos-aggregator = { workspace = true } aptos-gas-algebra = { workspace = true } diff --git a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs index cebc6d376d2f3a..01d57e716550e0 100644 --- a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs +++ b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs @@ -4,6 +4,7 @@ use crate::module_and_script_storage::{ code_storage::AptosCodeStorage, module_storage::AptosModuleStorage, }; +use ambassador::Delegate; use aptos_types::state_store::{state_key::StateKey, state_value::StateValueMetadata, StateView}; use bytes::Bytes; use move_binary_format::{ @@ -13,8 +14,9 @@ use move_binary_format::{ }; use move_core_types::{account_address::AccountAddress, identifier::IdentStr, metadata::Metadata}; use move_vm_runtime::{ - module_storage_error, move_vm::MoveVM, CodeStorage, IntoUnsyncCodeStorage, Module, - ModuleBytesStorage, ModuleStorage, Script, UnsyncCodeStorage, UnsyncModuleStorage, + ambassador_impl_CodeStorage, ambassador_impl_ModuleStorage, module_storage_error, + move_vm::MoveVM, CodeStorage, IntoUnsyncCodeStorage, Module, ModuleBytesStorage, ModuleStorage, + Script, UnsyncCodeStorage, UnsyncModuleStorage, }; use std::sync::Arc; @@ -39,6 +41,9 @@ impl<'s, S: StateView> ModuleBytesStorage for StateViewAdapter<'s, S> { /// A (not thread-safe) implementation of code storage on top of a state view. /// It is never built directly by clients - only via [AsAptosCodeStorage] trait. /// Can be used to resolve both modules and scripts. +#[derive(Delegate)] +#[delegate(ModuleStorage)] +#[delegate(CodeStorage)] pub struct AptosCodeStorageAdapter<'s, S> { storage: UnsyncCodeStorage>>, } @@ -51,57 +56,6 @@ impl<'s, S: StateView> AptosCodeStorageAdapter<'s, S> { } } -impl<'s, S: StateView> ModuleStorage for AptosCodeStorageAdapter<'s, S> { - fn check_module_exists( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult { - self.storage.check_module_exists(address, module_name) - } - - fn fetch_module_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_module_bytes(address, module_name) - } - - fn fetch_module_size_in_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage - .fetch_module_size_in_bytes(address, module_name) - } - - fn fetch_module_metadata( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_module_metadata(address, module_name) - } - - fn fetch_deserialized_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_deserialized_module(address, module_name) - } - - fn fetch_verified_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_verified_module(address, module_name) - } -} - impl<'s, S: StateView> AptosModuleStorage for AptosCodeStorageAdapter<'s, S> { fn fetch_state_value_metadata( &self, @@ -120,16 +74,6 @@ impl<'s, S: StateView> AptosModuleStorage for AptosCodeStorageAdapter<'s, S> { } } -impl<'s, S: StateView> CodeStorage for AptosCodeStorageAdapter<'s, S> { - fn fetch_deserialized_script(&self, serialized_script: &[u8]) -> VMResult> { - self.storage.fetch_deserialized_script(serialized_script) - } - - fn fetch_verified_script(&self, serialized_script: &[u8]) -> VMResult> { - self.storage.fetch_verified_script(serialized_script) - } -} - impl<'s, S: StateView> AptosCodeStorage for AptosCodeStorageAdapter<'s, S> {} /// Allows to treat a state view as a code storage with scripts and modules. The diff --git a/third_party/move/move-vm/runtime/Cargo.toml b/third_party/move/move-vm/runtime/Cargo.toml index 4844e254451ce0..4e96dcc0035e7b 100644 --- a/third_party/move/move-vm/runtime/Cargo.toml +++ b/third_party/move/move-vm/runtime/Cargo.toml @@ -11,6 +11,7 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +ambassador = { workspace = true } better_any = { workspace = true } bytes = { workspace = true } fail = { workspace = true } diff --git a/third_party/move/move-vm/runtime/src/lib.rs b/third_party/move/move-vm/runtime/src/lib.rs index 5bdaeaee7e7f60..e8ab6208748152 100644 --- a/third_party/move/move-vm/runtime/src/lib.rs +++ b/third_party/move/move-vm/runtime/src/lib.rs @@ -34,7 +34,7 @@ mod storage; pub use loader::{LoadedFunction, Module, Script}; pub use storage::{ - code_storage::{script_hash, CodeStorage}, + code_storage::{ambassador_impl_CodeStorage, script_hash, CodeStorage}, dummy::{use_loader_v2_based_on_env, DummyCodeStorage}, environment::RuntimeEnvironment, implementations::{ @@ -44,6 +44,6 @@ pub use storage::{ IntoUnsyncModuleStorage, LocalModuleBytesStorage, UnsyncModuleStorage, }, }, - module_storage::{ModuleBytesStorage, ModuleStorage}, + module_storage::{ambassador_impl_ModuleStorage, ModuleBytesStorage, ModuleStorage}, publishing::TemporaryModuleStorage, }; diff --git a/third_party/move/move-vm/runtime/src/storage/code_storage.rs b/third_party/move/move-vm/runtime/src/storage/code_storage.rs index 51f644c8696e28..365cb2bc51ec2b 100644 --- a/third_party/move/move-vm/runtime/src/storage/code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/code_storage.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{loader::Script, ModuleStorage}; +use ambassador::delegatable_trait; use move_binary_format::{errors::VMResult, file_format::CompiledScript}; use sha3::{Digest, Sha3_256}; use std::sync::Arc; @@ -17,6 +18,7 @@ pub fn script_hash(serialized_script: &[u8]) -> [u8; 32] { /// clients can implement this trait to ensure that even script dependency is /// upgraded, the correct script is still returned. Scripts are cached based /// on their hash. +#[delegatable_trait] pub trait CodeStorage: ModuleStorage { /// Returns a deserialized script, either by directly deserializing it from the /// provided bytes, or fetching it from the storage (if it has been cached). Note diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs index 084a44a0067213..011f48d751a74a 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs @@ -6,10 +6,11 @@ use crate::{ storage::{ environment::WithEnvironment, implementations::unsync_module_storage::IntoUnsyncModuleStorage, - module_storage::ModuleBytesStorage, + module_storage::{ambassador_impl_ModuleStorage, ModuleBytesStorage}, }, CodeStorage, Module, ModuleStorage, RuntimeEnvironment, Script, UnsyncModuleStorage, }; +use ambassador::Delegate; use bytes::Bytes; use move_binary_format::{ access::ScriptAccess, @@ -37,6 +38,8 @@ enum ScriptStorageEntry { } /// Code storage that stores both modules and scripts (not thread-safe). +#[derive(Delegate)] +#[delegate(ModuleStorage, target = "module_storage")] pub struct UnsyncCodeStorage { script_storage: RefCell>, module_storage: M, @@ -167,61 +170,6 @@ impl CodeStorage for UnsyncCodeStorage { } } -impl ModuleStorage for UnsyncCodeStorage { - fn check_module_exists( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult { - self.module_storage - .check_module_exists(address, module_name) - } - - fn fetch_module_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.module_storage.fetch_module_bytes(address, module_name) - } - - fn fetch_module_size_in_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.module_storage - .fetch_module_size_in_bytes(address, module_name) - } - - fn fetch_module_metadata( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.module_storage - .fetch_module_metadata(address, module_name) - } - - fn fetch_deserialized_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.module_storage - .fetch_deserialized_module(address, module_name) - } - - fn fetch_verified_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.module_storage - .fetch_verified_module(address, module_name) - } -} - #[cfg(test)] impl UnsyncCodeStorage { fn matches bool>( diff --git a/third_party/move/move-vm/runtime/src/storage/module_storage.rs b/third_party/move/move-vm/runtime/src/storage/module_storage.rs index d4f8118e064086..03d38ea81e4d45 100644 --- a/third_party/move/move-vm/runtime/src/storage/module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/module_storage.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::loader::Module; +use ambassador::delegatable_trait; use bytes::Bytes; use move_binary_format::{errors::VMResult, CompiledModule}; use move_core_types::{account_address::AccountAddress, identifier::IdentStr, metadata::Metadata}; @@ -9,6 +10,7 @@ use std::sync::Arc; /// Represents module storage backend, abstracting away any caching behaviour. The /// clients can implement their own module storage to pass to the VM to resolve code. +#[delegatable_trait] pub trait ModuleStorage { /// Returns true if the module exists, and false otherwise. An error is returned /// if there is a storage error. diff --git a/third_party/move/move-vm/runtime/src/storage/publishing.rs b/third_party/move/move-vm/runtime/src/storage/publishing.rs index 1141ab3a766f26..3dda19f1c1ca5d 100644 --- a/third_party/move/move-vm/runtime/src/storage/publishing.rs +++ b/third_party/move/move-vm/runtime/src/storage/publishing.rs @@ -2,9 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - IntoUnsyncModuleStorage, Module, ModuleBytesStorage, ModuleStorage, RuntimeEnvironment, - UnsyncModuleStorage, + ambassador_impl_ModuleStorage, IntoUnsyncModuleStorage, Module, ModuleBytesStorage, + ModuleStorage, RuntimeEnvironment, UnsyncModuleStorage, }; +use ambassador::Delegate; use bytes::Bytes; use move_binary_format::{ access::ModuleAccess, @@ -162,6 +163,8 @@ impl<'m, M: ModuleStorage> ModuleBytesStorage for TemporaryModuleBytesStorage<'m /// 2) Published modules satisfy compatibility constraints. /// 3) Published modules are verifiable and can link to existing modules without breaking /// invariants such as cyclic dependencies. +#[derive(Delegate)] +#[delegate(ModuleStorage)] pub struct TemporaryModuleStorage<'a, M> { storage: UnsyncModuleStorage<'a, TemporaryModuleBytesStorage<'a, M>>, } @@ -223,54 +226,3 @@ impl<'a, M: ModuleStorage> TemporaryModuleStorage<'a, M> { .flat_map(|(_, account_storage)| account_storage.into_values()) } } - -impl<'a, M: ModuleStorage> ModuleStorage for TemporaryModuleStorage<'a, M> { - fn check_module_exists( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult { - self.storage.check_module_exists(address, module_name) - } - - fn fetch_module_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_module_bytes(address, module_name) - } - - fn fetch_module_size_in_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage - .fetch_module_size_in_bytes(address, module_name) - } - - fn fetch_module_metadata( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_module_metadata(address, module_name) - } - - fn fetch_deserialized_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_deserialized_module(address, module_name) - } - - fn fetch_verified_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> VMResult> { - self.storage.fetch_verified_module(address, module_name) - } -}