Break the dependency from TemporaryAuthorityStore to AuthorityStore (#…

…801)

sui_core/src/authority.rs

@@ -24,7 +24,7 @@ use sui_types::{
     fp_bail, fp_ensure, gas,
     messages::*,
     object::{Data, Object, Owner},
-    storage::{DeleteKind, Storage},
+    storage::{BackingPackageStore, DeleteKind, Storage},
     MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS,
 };
 use tracing::*;
@@ -484,8 +484,9 @@ impl AuthorityState {
         transaction: Transaction,
         mut inputs: Vec<Object>,
         tx_ctx: &mut TxContext,
-    ) -> SuiResult<(AuthorityTemporaryStore, ExecutionStatus)> {
-        let mut temporary_store = AuthorityTemporaryStore::new(self, &inputs, tx_ctx.digest());
+    ) -> SuiResult<(AuthorityTemporaryStore<AuthorityStore>, ExecutionStatus)> {
+        let mut temporary_store =
+            AuthorityTemporaryStore::new(self._database.clone(), &inputs, tx_ctx.digest());
         // unwraps here are safe because we built `inputs`
         let mut gas_object = inputs.pop().unwrap();
 
@@ -573,7 +574,7 @@ impl AuthorityState {
     }
 
     fn transfer(
-        temporary_store: &mut AuthorityTemporaryStore,
+        temporary_store: &mut AuthorityTemporaryStore<AuthorityStore>,
         mut inputs: Vec<Object>,
         recipient: SuiAddress,
         mut gas_object: Object,
@@ -824,7 +825,8 @@ impl AuthorityState {
             .flatten()
             .collect::<Vec<_>>();
 
-        let mut temporary_store = AuthorityTemporaryStore::new(self, &input_objects, ctx.digest());
+        let mut temporary_store =
+            AuthorityTemporaryStore::new(self._database.clone(), &input_objects, ctx.digest());
         let package_id = ObjectID::from(*modules[0].self_id().address());
         let natives = self._native_functions.clone();
         let vm = adapter::verify_and_link(&temporary_store, &modules, package_id, natives)?;
@@ -879,8 +881,7 @@ impl AuthorityState {
 
     async fn update_state(
         &self,
-        temporary_store: AuthorityTemporaryStore,
-
+        temporary_store: AuthorityTemporaryStore<AuthorityStore>,
         certificate: CertifiedTransaction,
         signed_effects: SignedTransactionEffects,
     ) -> Result<(u64, TransactionInfoResponse), SuiError> {

sui_core/src/authority/authority_store.rs

@@ -6,8 +6,8 @@ use rocksdb::Options;
 use std::collections::BTreeSet;
 use std::convert::TryInto;
 use std::path::Path;
-
 use std::sync::atomic::AtomicU64;
+
 use sui_types::base_types::SequenceNumber;
 use sui_types::batch::{SignedBatch, TxSequenceNumber};
 use tracing::warn;
@@ -386,7 +386,7 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
         // This is the critical region: testing the locks and writing the
         // new locks must be atomic, and not writes should happen in between.
         {
-            // Aquire the lock to ensure no one else writes when we are in here.
+            // Acquire the lock to ensure no one else writes when we are in here.
             // MutexGuards are unlocked on drop (ie end of this block)
             let _mutexes = self.acquire_locks(mutable_input_objects);
 
@@ -424,9 +424,9 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     ///
     /// Internally it checks that all locks for active inputs are at the correct
     /// version, and then writes locks, objects, certificates, parents atomically.
-    pub fn update_state(
+    pub fn update_state<S>(
         &self,
-        temporary_store: AuthorityTemporaryStore,
+        temporary_store: AuthorityTemporaryStore<S>,
         certificate: CertifiedTransaction,
         signed_effects: SignedTransactionEffects,
     ) -> Result<(TxSequenceNumber, TransactionInfoResponse), SuiError> {
@@ -470,9 +470,9 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     }
 
     /// Persist temporary storage to DB for genesis modules
-    pub fn update_objects_state_for_genesis(
+    pub fn update_objects_state_for_genesis<S>(
         &self,
-        temporary_store: AuthorityTemporaryStore,
+        temporary_store: AuthorityTemporaryStore<S>,
         transaction_digest: TransactionDigest,
     ) -> Result<(), SuiError> {
         debug_assert_eq!(transaction_digest, TransactionDigest::genesis());
@@ -482,10 +482,10 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     }
 
     /// Helper function for updating the objects in the state
-    fn batch_update_objects(
+    fn batch_update_objects<S>(
         &self,
         mut write_batch: DBBatch,
-        temporary_store: AuthorityTemporaryStore,
+        temporary_store: AuthorityTemporaryStore<S>,
         transaction_digest: TransactionDigest,
         should_sequence: bool,
     ) -> Result<Option<TxSequenceNumber>, SuiError> {
@@ -706,7 +706,7 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     /// Retrieves batches including transactions within a range.
     ///
     /// This function returns all signed batches that enclose the requested transaction
-    /// including the batch preceeding the first requested transaction, the batch including
+    /// including the batch preceding the first requested transaction, the batch including
     /// the last requested transaction (if there is one) and all batches in between.
     ///
     /// Transactions returned include all transactions within the batch that include the
@@ -806,22 +806,40 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     }
 }
 
-impl ModuleResolver for AuthorityStore {
+impl<const ALL_OBJ_VER: bool> BackingPackageStore for SuiDataStore<ALL_OBJ_VER> {
+    fn get_package(&self, package_id: &ObjectID) -> SuiResult<Option<Object>> {
+        let package = self.get_object(package_id)?;
+        if let Some(obj) = &package {
+            fp_ensure!(
+                obj.is_package(),
+                SuiError::BadObjectType {
+                    error: format!("Package expected, Move object found: {}", package_id),
+                }
+            );
+        }
+        Ok(package)
+    }
+}
+
+impl<const ALL_OBJ_VER: bool> ModuleResolver for SuiDataStore<ALL_OBJ_VER> {
     type Error = SuiError;
 
     fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
-        match self.get_object(&ObjectID::from(*module_id.address()))? {
-            Some(o) => match &o.data {
-                Data::Package(c) => Ok(c
+        // TODO: We should cache the deserialized modules to avoid
+        // fetching from the store / re-deserializing them everytime.
+        // https://github.com/MystenLabs/sui/issues/809
+        Ok(self
+            .get_package(&ObjectID::from(*module_id.address()))?
+            .and_then(|package| {
+                // unwrap safe since get_package() ensures it's a package object.
+                package
+                    .data
+                    .try_as_package()
+                    .unwrap()
                     .serialized_module_map()
                     .get(module_id.name().as_str())
                     .cloned()
-                    .map(|m| m.into_vec())),
-                _ => Err(SuiError::BadObjectType {
-                    error: "Expected module object".to_string(),
-                }),
-            },
-            None => Ok(None),
-        }
+                    .map(|m| m.into_vec())
+            }))
     }
 }

sui_core/src/authority/temporary_store.rs

@@ -13,8 +13,11 @@ pub type InnerTemporaryStore = (
     Vec<Event>,
 );
 
-pub struct AuthorityTemporaryStore {
-    object_store: Arc<AuthorityStore>,
+pub struct AuthorityTemporaryStore<S> {
+    // The backing store for retrieving Move packages onchain.
+    // When executing a Move call, the dependent packages are not going to be
+    // in the input objects. They will be feteched from the backing store.
+    package_store: Arc<S>,
     tx_digest: TransactionDigest,
     objects: BTreeMap<ObjectID, Object>,
     active_inputs: Vec<ObjectRef>, // Inputs that are not read only
@@ -31,16 +34,16 @@ pub struct AuthorityTemporaryStore {
     created_object_ids: HashSet<ObjectID>,
 }
 
-impl AuthorityTemporaryStore {
+impl<S> AuthorityTemporaryStore<S> {
     /// Creates a new store associated with an authority store, and populates it with
     /// initial objects.
     pub fn new(
-        authority_state: &AuthorityState,
+        package_store: Arc<S>,
         _input_objects: &'_ [Object],
         tx_digest: TransactionDigest,
-    ) -> AuthorityTemporaryStore {
-        AuthorityTemporaryStore {
-            object_store: authority_state._database.clone(),
+    ) -> Self {
+        Self {
+            package_store,
             tx_digest,
             objects: _input_objects.iter().map(|v| (v.id(), v.clone())).collect(),
             active_inputs: _input_objects
@@ -200,7 +203,7 @@ impl AuthorityTemporaryStore {
     }
 }
 
-impl Storage for AuthorityTemporaryStore {
+impl<S> Storage for AuthorityTemporaryStore<S> {
     /// Resets any mutations and deletions recorded in the store.
     fn reset(&mut self) {
         self.written.clear();
@@ -214,13 +217,7 @@ impl Storage for AuthorityTemporaryStore {
         debug_assert!(self.deleted.get(id) == None);
         match self.written.get(id) {
             Some(x) => Some(x.clone()),
-            None => match self.objects.get(id) {
-                Some(x) => Some(x.clone()),
-                None => match self.object_store.get_object(id) {
-                    Ok(o) => o,
-                    Err(e) => panic!("Could not read object {}", e),
-                },
-            },
+            None => self.objects.get(id).cloned(),
         }
     }
 
@@ -276,26 +273,33 @@ impl Storage for AuthorityTemporaryStore {
     }
 }
 
-impl ModuleResolver for AuthorityTemporaryStore {
+impl<S: BackingPackageStore> ModuleResolver for AuthorityTemporaryStore<S> {
     type Error = SuiError;
     fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
-        match self.read_object(&ObjectID::from(*module_id.address())) {
-            Some(o) => match &o.data {
-                Data::Package(c) => Ok(c
-                    .serialized_module_map()
-                    .get(module_id.name().as_str())
-                    .cloned()
-                    .map(|m| m.into_vec())),
-                _ => Err(SuiError::BadObjectType {
-                    error: "Expected module object".to_string(),
-                }),
+        let package_id = &ObjectID::from(*module_id.address());
+        let package = match self.read_object(package_id) {
+            Some(object) => object,
+            None => match self.package_store.get_package(package_id)? {
+                Some(object) => object,
+                None => {
+                    return Ok(None);
+                }
             },
-            None => Ok(None),
+        };
+        match &package.data {
+            Data::Package(c) => Ok(c
+                .serialized_module_map()
+                .get(module_id.name().as_str())
+                .cloned()
+                .map(|m| m.into_vec())),
+            _ => Err(SuiError::BadObjectType {
+                error: "Expected module object".to_string(),
+            }),
         }
     }
 }
 
-impl ResourceResolver for AuthorityTemporaryStore {
+impl<S> ResourceResolver for AuthorityTemporaryStore<S> {
     type Error = SuiError;
 
     fn get_resource(

sui_programmability/adapter/src/unit_tests/adapter_tests.rs

@@ -15,7 +15,7 @@ use sui_types::{
     error::SuiResult,
     gas_coin::GAS,
     object::{Data, Owner},
-    storage::Storage,
+    storage::{BackingPackageStore, Storage},
     MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS,
 };
 
@@ -41,6 +41,12 @@ struct InMemoryStorage {
     temporary: ScratchPad,
 }
 
+impl BackingPackageStore for InMemoryStorage {
+    fn get_package(&self, package_id: &ObjectID) -> SuiResult<Option<Object>> {
+        Ok(self.persistent.get(package_id).cloned())
+    }
+}
+
 impl InMemoryStorage {
     pub fn new(objects: Vec<Object>) -> Self {
         let mut persistent = BTreeMap::new();

sui_types/src/storage.rs

@@ -5,6 +5,7 @@ use std::collections::HashSet;
 
 use crate::{
     base_types::{ObjectID, SequenceNumber},
+    error::SuiResult,
     event::Event,
     object::Object,
 };
@@ -37,3 +38,7 @@ pub trait Storage {
 
     fn delete_object(&mut self, id: &ObjectID, version: SequenceNumber, kind: DeleteKind);
 }
+
+pub trait BackingPackageStore {
+    fn get_package(&self, package_id: &ObjectID) -> SuiResult<Option<Object>>;
+}