[authority] Compute and store .to_object_reference() once. (MystenL…

…abs#837)

* Compute and store `.to_object_reference()` once.
* A few more to_object_ref eradications
* Changed name to `compute_object_reference`

Co-authored-by: George Danezis <[email protected]>

sui/src/bench.rs

@@ -200,15 +200,15 @@ impl ClientServerBenchmark {
                 };
 
                 assert!(object.version() == SequenceNumber::from(0));
-                let object_ref = object.to_object_reference();
+                let object_ref = object.compute_object_reference();
                 state.init_transaction_lock(object_ref).await;
                 account_objects.push((address, object.clone(), keypair));
                 state.insert_object(object).await;
 
                 let gas_object_id = ObjectID::random();
                 let gas_object = Object::with_id_owner_for_testing(gas_object_id, address);
                 assert!(gas_object.version() == SequenceNumber::from(0));
-                let gas_object_ref = gas_object.to_object_reference();
+                let gas_object_ref = gas_object.compute_object_reference();
                 state.init_transaction_lock(gas_object_ref).await;
                 gas_objects.push(gas_object.clone());
                 state.insert_object(gas_object).await;
@@ -221,8 +221,8 @@ impl ClientServerBenchmark {
         let mut transactions: Vec<Bytes> = Vec::new();
         let mut next_recipient: SuiAddress = get_key_pair().0;
         for ((account_addr, object, secret), gas_obj) in account_objects.iter().zip(gas_objects) {
-            let object_ref = object.to_object_reference();
-            let gas_object_ref = gas_obj.to_object_reference();
+            let object_ref = object.compute_object_reference();
+            let gas_object_ref = gas_obj.compute_object_reference();
 
             let data = if self.use_move {
                 // TODO: authority should not require seq# or digets for package in Move calls. Use dummy values

sui/src/sui_commands.rs

@@ -287,7 +287,7 @@ async fn make_server_with_genesis_ctx(
 
     for object in preload_objects {
         state
-            .init_transaction_lock(object.to_object_reference())
+            .init_transaction_lock(object.compute_object_reference())
             .await;
         state.insert_object(object.clone()).await;
     }

sui/src/unit_tests/cli_tests.rs

@@ -926,8 +926,8 @@ async fn test_package_publish_command() -> Result<(), anyhow::Error> {
     let dumy_obj = Object::with_id_owner_for_testing(ObjectID::random(), address);
     // Get the created objects
     let (mut created_obj1, mut created_obj2) = (
-        dumy_obj.to_object_reference(),
-        dumy_obj.to_object_reference(),
+        dumy_obj.compute_object_reference(),
+        dumy_obj.compute_object_reference(),
     );
 
     if let WalletCommandResult::Publish(
@@ -1044,8 +1044,8 @@ async fn test_native_transfer() -> Result<(), anyhow::Error> {
         } else {
             assert!(false);
             (
-                dumy_obj.to_object_reference(),
-                dumy_obj.to_object_reference(),
+                dumy_obj.compute_object_reference(),
+                dumy_obj.compute_object_reference(),
             )
         };
 

sui/src/wallet_commands.rs

@@ -219,7 +219,7 @@ impl WalletCommands {
                 let gas_object_read = context.gateway.get_object_info(*gas).await?;
                 let gas_object = gas_object_read.object()?;
                 let sender = gas_object.owner.get_owner_address()?;
-                let gas_obj_ref = gas_object.to_object_reference();
+                let gas_obj_ref = gas_object.compute_object_reference();
 
                 let compiled_modules = build_move_package_to_bytes(Path::new(path))?;
                 let (cert, effects) = context
@@ -299,13 +299,13 @@ impl WalletCommands {
                     .get_object_info(*gas)
                     .await?
                     .into_object()?
-                    .to_object_reference();
+                    .compute_object_reference();
 
                 // Fetch the objects for the object args
                 let mut object_args_refs = Vec::new();
                 for obj_id in object_ids {
                     let obj_info = context.gateway.get_object_info(obj_id).await?;
-                    object_args_refs.push(obj_info.object()?.to_object_reference());
+                    object_args_refs.push(obj_info.object()?.compute_object_reference());
                 }
 
                 let (cert, effects) = context

sui_core/src/authority.rs

@@ -599,7 +599,7 @@ impl AuthorityState {
                             // Read only objects have no locks.
                             None
                         } else {
-                            self.get_transaction_lock(&object.to_object_reference())
+                            self.get_transaction_lock(&object.compute_object_reference())
                                 .await?
                         };
                         let layout = match request_layout {

sui_core/src/authority/authority_store.rs

@@ -350,16 +350,17 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
     /// Insert an object
     pub fn insert_object(&self, object: Object) -> Result<(), SuiError> {
         self.objects.insert(&object.id(), &object)?;
+        let object_ref = object.compute_object_reference();
 
         // Update the index
         if let Some(address) = object.get_single_owner() {
             self.owner_index
-                .insert(&(address, object.id()), &object.to_object_reference())?;
+                .insert(&(address, object.id()), &object_ref)?;
         }
 
         // Update the parent
         self.parent_sync
-            .insert(&object.to_object_reference(), &object.previous_transaction)?;
+            .insert(&object_ref, &object.previous_transaction)?;
 
         // We only side load objects with a genesis parent transaction.
         debug_assert!(object.previous_transaction == TransactionDigest::genesis());
@@ -532,7 +533,7 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
             .chain(
                 written
                     .iter()
-                    .filter_map(|(id, new_object)| match objects.get(id) {
+                    .filter_map(|(id, (_, new_object))| match objects.get(id) {
                         Some(old_object) if old_object.owner != new_object.owner => {
                             old_object.get_single_owner_and_id()
                         }
@@ -548,7 +549,7 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
             &self.parent_sync,
             written
                 .iter()
-                .map(|(_, object)| (object.to_object_reference(), transaction_digest)),
+                .map(|(_, (object_ref, _object_))| (object_ref, transaction_digest)),
         )?;
 
         // Index the certificate by the objects deleted
@@ -573,9 +574,9 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
         // Create locks for new objects, if they are not immutable
         write_batch = write_batch.insert_batch(
             &self.transaction_lock,
-            written.iter().filter_map(|(_, new_object)| {
+            written.iter().filter_map(|(_, (object_ref, new_object))| {
                 if !new_object.is_read_only() {
-                    Some((new_object.to_object_reference(), None))
+                    Some((object_ref, None))
                 } else {
                     None
                 }
@@ -588,22 +589,29 @@ impl<const ALL_OBJ_VER: bool> SuiDataStore<ALL_OBJ_VER> {
                 &self.all_object_versions,
                 written
                     .iter()
-                    .map(|(id, object)| ((*id, object.version()), object)),
+                    .map(|(id, (_object_ref, object))| ((*id, object.version()), object)),
             )?;
         }
 
         // Update the indexes of the objects written
         write_batch = write_batch.insert_batch(
             &self.owner_index,
-            written.iter().filter_map(|(_id, new_object)| {
-                new_object
-                    .get_single_owner_and_id()
-                    .map(|owner_id| (owner_id, new_object.to_object_reference()))
-            }),
+            written
+                .iter()
+                .filter_map(|(_id, (object_ref, new_object))| {
+                    new_object
+                        .get_single_owner_and_id()
+                        .map(|owner_id| (owner_id, object_ref))
+                }),
         )?;
 
         // Insert each output object into the stores
-        write_batch = write_batch.insert_batch(&self.objects, written.iter())?;
+        write_batch = write_batch.insert_batch(
+            &self.objects,
+            written
+                .iter()
+                .map(|(object_id, (_, new_object))| (object_id, new_object)),
+        )?;
 
         // Update the indexes of the objects written
 

sui_core/src/authority/temporary_store.rs

@@ -8,7 +8,7 @@ use super::*;
 pub type InnerTemporaryStore = (
     BTreeMap<ObjectID, Object>,
     Vec<ObjectRef>,
-    BTreeMap<ObjectID, Object>,
+    BTreeMap<ObjectID, (ObjectRef, Object)>,
     BTreeMap<ObjectID, (SequenceNumber, DeleteKind)>,
     Vec<Event>,
 );
@@ -21,10 +21,7 @@ pub struct AuthorityTemporaryStore<S> {
     tx_digest: TransactionDigest,
     objects: BTreeMap<ObjectID, Object>,
     active_inputs: Vec<ObjectRef>, // Inputs that are not read only
-    // TODO: We need to study whether it's worth to optimize the lookup of
-    // object reference by caching object reference in the map as well.
-    // Object reference calculation involves hashing which could be expensive.
-    written: BTreeMap<ObjectID, Object>, // Objects written
+    written: BTreeMap<ObjectID, (ObjectRef, Object)>, // Objects written
     /// Objects actively deleted.
     deleted: BTreeMap<ObjectID, (SequenceNumber, DeleteKind)>,
     /// Ordered sequence of events emitted by execution
@@ -49,7 +46,7 @@ impl<S> AuthorityTemporaryStore<S> {
             active_inputs: _input_objects
                 .iter()
                 .filter(|v| !v.is_read_only())
-                .map(|v| v.to_object_reference())
+                .map(|v| v.compute_object_reference())
                 .collect(),
             written: BTreeMap::new(),
             deleted: BTreeMap::new(),
@@ -63,7 +60,7 @@ impl<S> AuthorityTemporaryStore<S> {
         &self.objects
     }
 
-    pub fn written(&self) -> &BTreeMap<ObjectID, Object> {
+    pub fn written(&self) -> &BTreeMap<ObjectID, (ObjectRef, Object)> {
         &self.written
     }
 
@@ -95,7 +92,8 @@ impl<S> AuthorityTemporaryStore<S> {
                 let mut object = self.objects[id].clone();
                 // Active input object must be Move object.
                 object.data.try_as_move_mut().unwrap().increment_version();
-                self.written.insert(*id, object);
+                self.written
+                    .insert(*id, (object.compute_object_reference(), object));
             }
         }
     }
@@ -109,29 +107,29 @@ impl<S> AuthorityTemporaryStore<S> {
         status: ExecutionStatus,
         gas_object_id: &ObjectID,
     ) -> SignedTransactionEffects {
-        let gas_object = &self.written[gas_object_id];
+        let (gas_reference, gas_object) = &self.written[gas_object_id];
         let effects = TransactionEffects {
             status,
             transaction_digest: *transaction_digest,
             created: self
                 .written
                 .iter()
                 .filter(|(id, _)| self.created_object_ids.contains(*id))
-                .map(|(_, object)| (object.to_object_reference(), object.owner))
+                .map(|(_, (object_ref, object))| (*object_ref, object.owner))
                 .collect(),
             mutated: self
                 .written
                 .iter()
                 .filter(|(id, _)| self.objects.contains_key(*id))
-                .map(|(_, object)| (object.to_object_reference(), object.owner))
+                .map(|(_, (object_ref, object))| (*object_ref, object.owner))
                 .collect(),
             unwrapped: self
                 .written
                 .iter()
                 .filter(|(id, _)| {
                     !self.objects.contains_key(*id) && !self.created_object_ids.contains(*id)
                 })
-                .map(|(_, object)| (object.to_object_reference(), object.owner))
+                .map(|(_, (object_ref, object))| (*object_ref, object.owner))
                 .collect(),
             deleted: self
                 .deleted
@@ -155,7 +153,7 @@ impl<S> AuthorityTemporaryStore<S> {
                     }
                 })
                 .collect(),
-            gas_object: (gas_object.to_object_reference(), gas_object.owner),
+            gas_object: (*gas_reference, gas_object.owner),
             events: self.events.clone(),
             dependencies: transaction_dependencies,
         };
@@ -216,7 +214,7 @@ impl<S> Storage for AuthorityTemporaryStore<S> {
         // there should be no read after delete
         debug_assert!(self.deleted.get(id) == None);
         match self.written.get(id) {
-            Some(x) => Some(x.clone()),
+            Some(x) => Some(x.1.clone()),
             None => self.objects.get(id).cloned(),
         }
     }
@@ -247,7 +245,8 @@ impl<S> Storage for AuthorityTemporaryStore<S> {
         // The adapter is not very disciplined at filling in the correct
         // previous transaction digest, so we ensure it is correct here.
         object.previous_transaction = self.tx_digest;
-        self.written.insert(object.id(), object);
+        self.written
+            .insert(object.id(), (object.compute_object_reference(), object));
     }
 
     fn delete_object(&mut self, id: &ObjectID, version: SequenceNumber, kind: DeleteKind) {

sui_core/src/gateway_state.rs

@@ -309,7 +309,7 @@ impl AccountState {
     pub fn insert_object(&self, object: Object) -> SuiResult {
         self.store
             .objects
-            .insert(&object.to_object_reference(), &object)?;
+            .insert(&object.compute_object_reference(), &object)?;
         Ok(())
     }
 
@@ -319,7 +319,7 @@ impl AccountState {
         option_layout: Option<MoveStructLayout>,
         option_cert: Option<CertifiedTransaction>,
     ) -> SuiResult {
-        let object_ref = object.to_object_reference();
+        let object_ref = object.compute_object_reference();
         let (object_id, _seqnum, _) = object_ref;
 
         self.store.object_refs.insert(&object_id, &object_ref)?;
@@ -616,7 +616,7 @@ where
         while let Some(resp) = receiver.recv().await {
             // Persists them to disk
             if let Ok(o) = resp {
-                err_object_refs.remove(&o.to_object_reference());
+                err_object_refs.remove(&o.compute_object_reference());
                 account.insert_object(o)?;
             }
         }

sui_core/src/safe_client.rs

@@ -135,7 +135,7 @@ impl<C> SafeClient<C> {
                     // Since we are requesting the latest version, we should validate that if the object's
                     // reference actually match with the one from the responded object reference.
                     fp_ensure!(
-                        object_and_lock.object.to_object_reference() == obj_ref,
+                        object_and_lock.object.compute_object_reference() == obj_ref,
                         SuiError::ByzantineAuthoritySuspicion {
                             authority: self.address
                         }