comments: use delegate where possible

aptos-labs · Aug 24, 2024 · b67a883 · b67a883
1 parent fa97425
commit b67a883
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 175 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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<UnsyncModuleStorage<'s, StateViewAdapter<'s, S>>>,
 }
@@ -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<bool> {
-        self.storage.check_module_exists(address, module_name)
-    }
-
-    fn fetch_module_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<Bytes>> {
-        self.storage.fetch_module_bytes(address, module_name)
-    }
-
-    fn fetch_module_size_in_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<usize>> {
-        self.storage
-            .fetch_module_size_in_bytes(address, module_name)
-    }
-
-    fn fetch_module_metadata(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Vec<Metadata>> {
-        self.storage.fetch_module_metadata(address, module_name)
-    }
-
-    fn fetch_deserialized_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<CompiledModule>> {
-        self.storage.fetch_deserialized_module(address, module_name)
-    }
-
-    fn fetch_verified_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<Module>> {
-        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<Arc<CompiledScript>> {
-        self.storage.fetch_deserialized_script(serialized_script)
-    }
-
-    fn fetch_verified_script(&self, serialized_script: &[u8]) -> VMResult<Arc<Script>> {
-        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
@@ -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
@@ -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
@@ -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
@@ -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<M> {
     script_storage: RefCell<HashMap<[u8; 32], ScriptStorageEntry>>,
     module_storage: M,
@@ -167,61 +170,6 @@ impl<M: ModuleStorage + WithEnvironment> CodeStorage for UnsyncCodeStorage<M> {
     }
 }
 
-impl<M: ModuleStorage + WithEnvironment> ModuleStorage for UnsyncCodeStorage<M> {
-    fn check_module_exists(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<bool> {
-        self.module_storage
-            .check_module_exists(address, module_name)
-    }
-
-    fn fetch_module_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<Bytes>> {
-        self.module_storage.fetch_module_bytes(address, module_name)
-    }
-
-    fn fetch_module_size_in_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<usize>> {
-        self.module_storage
-            .fetch_module_size_in_bytes(address, module_name)
-    }
-
-    fn fetch_module_metadata(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Vec<Metadata>> {
-        self.module_storage
-            .fetch_module_metadata(address, module_name)
-    }
-
-    fn fetch_deserialized_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<CompiledModule>> {
-        self.module_storage
-            .fetch_deserialized_module(address, module_name)
-    }
-
-    fn fetch_verified_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<Module>> {
-        self.module_storage
-            .fetch_verified_module(address, module_name)
-    }
-}
-
 #[cfg(test)]
 impl<M: ModuleStorage + WithEnvironment> UnsyncCodeStorage<M> {
     fn matches<P: Fn(&ScriptStorageEntry) -> 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
@@ -2,13 +2,15 @@
 // 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};
 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
@@ -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<bool> {
-        self.storage.check_module_exists(address, module_name)
-    }
-
-    fn fetch_module_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<Bytes>> {
-        self.storage.fetch_module_bytes(address, module_name)
-    }
-
-    fn fetch_module_size_in_bytes(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Option<usize>> {
-        self.storage
-            .fetch_module_size_in_bytes(address, module_name)
-    }
-
-    fn fetch_module_metadata(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Vec<Metadata>> {
-        self.storage.fetch_module_metadata(address, module_name)
-    }
-
-    fn fetch_deserialized_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<CompiledModule>> {
-        self.storage.fetch_deserialized_module(address, module_name)
-    }
-
-    fn fetch_verified_module(
-        &self,
-        address: &AccountAddress,
-        module_name: &IdentStr,
-    ) -> VMResult<Arc<Module>> {
-        self.storage.fetch_verified_module(address, module_name)
-    }
-}