[adapter] Pass write kind information to temporary store (#4552)

- The temporary storage and Storage trait has to recreate a lot of information already known to the adapter
- This removes some messiness about having to determine what objects were created or wrapped, which is already done inside the adapter

crates/sui-adapter/src/adapter.rs

@@ -35,7 +35,7 @@ use sui_types::{
     id::UID,
     messages::{CallArg, EntryArgumentErrorKind, InputObjectKind, ObjectArg},
     object::{self, Data, MoveObject, Object, Owner, ID_END_INDEX},
-    storage::{DeleteKind, ObjectChange, ParentSync, Storage},
+    storage::{DeleteKind, ObjectChange, ParentSync, Storage, WriteKind},
     SUI_SYSTEM_STATE_OBJECT_ID,
 };
 use sui_verifier::{
@@ -306,8 +306,7 @@ pub fn store_package_and_init_modules<
     // The call to unwrap() will go away once we remove address owner from Immutable objects.
     let package_object = Object::new_package(modules, ctx.digest());
     let id = package_object.id();
-    state_view.set_create_object_ids(BTreeSet::from([id]));
-    let changes = BTreeMap::from([(id, ObjectChange::Write(package_object))]);
+    let changes = BTreeMap::from([(id, ObjectChange::Write(package_object, WriteKind::Create))]);
     state_view.apply_object_changes(changes);
 
     init_modules(state_view, vm, modules_to_init, ctx, gas_status)
@@ -475,7 +474,7 @@ fn process_successful_execution<
             .try_as_move_mut()
             .expect("We previously checked that mutable ref inputs are Move objects")
             .update_contents_and_increment_version(new_contents);
-        changes.insert(obj_id, ObjectChange::Write(obj));
+        changes.insert(obj_id, ObjectChange::Write(obj, WriteKind::Mutate));
     }
     let tx_digest = ctx.digest();
     // newly_generated_ids contains all object IDs generated in this transaction.
@@ -509,7 +508,7 @@ fn process_successful_execution<
                     EventType::ShareObject => Owner::Shared,
                     _ => unreachable!(),
                 };
-                let obj = handle_transfer(
+                let (obj, write_kind) = handle_transfer(
                     ctx.sender(),
                     new_owner,
                     type_,
@@ -525,7 +524,7 @@ fn process_successful_execution<
                     &mut frozen_object_ids,
                     &mut child_count_deltas,
                 )?;
-                changes.insert(obj.id(), ObjectChange::Write(obj));
+                changes.insert(obj.id(), ObjectChange::Write(obj, write_kind));
             }
             EventType::DeleteObjectID => {
                 // unwrap safe because this event can only be emitted from processing
@@ -673,10 +672,10 @@ fn process_successful_execution<
             let mut object = state_view.read_object(&id).unwrap().clone();
             // Active input object must be Move object.
             object.data.try_as_move_mut().unwrap().increment_version();
-            ObjectChange::Write(object)
+            ObjectChange::Write(object, WriteKind::Mutate)
         });
         match change {
-            ObjectChange::Write(object) => {
+            ObjectChange::Write(object, _) => {
                 object
                     .data
                     .try_as_move_mut()
@@ -697,7 +696,6 @@ fn process_successful_execution<
     }
 
     // apply object writes and object deletions
-    state_view.set_create_object_ids(newly_generated_ids);
     state_view.apply_object_changes(changes);
 
     // check all frozen objects have a child count of zero
@@ -738,7 +736,7 @@ fn handle_transfer<
     newly_generated_unused: &mut BTreeSet<ObjectID>,
     frozen_object_ids: &mut BTreeSet<ObjectID>,
     child_count_deltas: &mut BTreeMap<ObjectID, i64>,
-) -> Result<Object, ExecutionError> {
+) -> Result<(Object, WriteKind), ExecutionError> {
     let s_type = match type_ {
         TypeTag::Struct(s_type) => s_type,
         _ => unreachable!("Only structs can be transferred"),
@@ -750,23 +748,23 @@ fn handle_transfer<
     newly_generated_unused.remove(&id);
     let old_object = by_value_objects.remove(&id);
     let mut is_unwrapped = !(newly_generated_ids.contains(&id) || id == SUI_SYSTEM_STATE_OBJECT_ID);
-    let (version, child_count) = match old_object {
-        Some((_, version, child_count)) => (version, child_count),
+    let (write_kind, version, child_count) = match old_object {
+        Some((_, version, child_count)) => (WriteKind::Mutate, version, child_count),
         // When an object was wrapped at version `v`, we added an record into `parent_sync`
         // with version `v+1` along with OBJECT_DIGEST_WRAPPED. Now when the object is unwrapped,
         // it will also have version `v+1`, leading to a violation of the invariant that any
         // object_id and version pair must be unique. We use the version from parent_sync and
         // increment it (below), so we will have `(v+1)+1`, thus preserving the uniqueness
         None if is_unwrapped => match state_view.get_latest_parent_entry_ref(id) {
-            Ok(Some((_, last_version, _))) => (last_version, None),
+            Ok(Some((_, last_version, _))) => (WriteKind::Unwrap, last_version, None),
             // if the object is not in parent sync, it was wrapped before ever being stored into
             // storage.
             // we set is_unwrapped to false since the object has never been in storage
             // and essentially is being created. Similarly, we add it to the newly_generated_ids set
             Ok(None) => {
                 is_unwrapped = false;
                 newly_generated_ids.insert(id);
-                (SequenceNumber::new(), None)
+                (WriteKind::Create, SequenceNumber::new(), None)
             }
             Err(_) => {
                 // TODO this error is (hopefully) transient and should not be
@@ -777,7 +775,7 @@ fn handle_transfer<
                 ));
             }
         },
-        None => (SequenceNumber::new(), None),
+        None => (WriteKind::Create, SequenceNumber::new(), None),
     };
 
     // safe because `has_public_transfer` was properly determined from the abilities
@@ -878,8 +876,7 @@ fn handle_transfer<
             object_owner_map.insert(obj_address, new_owner);
         }
     }
-
-    Ok(obj)
+    Ok((obj, write_kind))
 }
 
 #[cfg(debug_assertions)]

crates/sui-adapter/src/in_memory_storage.rs

@@ -7,7 +7,7 @@ use sui_types::{
     base_types::{ObjectID, ObjectRef, SequenceNumber},
     error::{SuiError, SuiResult},
     object::Object,
-    storage::{BackingPackageStore, DeleteKind, ParentSync},
+    storage::{BackingPackageStore, DeleteKind, ParentSync, WriteKind},
 };
 
 // TODO: We should use AuthorityTemporaryStore instead.
@@ -97,11 +97,11 @@ impl InMemoryStorage {
 
     pub fn finish(
         &mut self,
-        written: BTreeMap<ObjectID, (ObjectRef, Object)>,
+        written: BTreeMap<ObjectID, (ObjectRef, Object, WriteKind)>,
         deleted: BTreeMap<ObjectID, (SequenceNumber, DeleteKind)>,
     ) {
         debug_assert!(written.keys().all(|id| !deleted.contains_key(id)));
-        for (_id, (_, new_object)) in written {
+        for (_id, (_, new_object, _)) in written {
             debug_assert!(new_object.id() == _id);
             self.insert_object(new_object);
         }

crates/sui-adapter/src/temporary_store.rs

@@ -4,15 +4,17 @@
 use move_core_types::account_address::AccountAddress;
 use move_core_types::language_storage::{ModuleId, StructTag};
 use move_core_types::resolver::{ModuleResolver, ResourceResolver};
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeMap;
 use sui_types::base_types::{
     ObjectDigest, ObjectID, ObjectRef, SequenceNumber, SuiAddress, TransactionDigest,
 };
 use sui_types::error::{ExecutionError, SuiError, SuiResult};
 use sui_types::fp_bail;
 use sui_types::messages::{ExecutionStatus, InputObjects, TransactionEffects};
 use sui_types::object::{Data, Object};
-use sui_types::storage::{BackingPackageStore, DeleteKind, ObjectChange, ParentSync, Storage};
+use sui_types::storage::{
+    BackingPackageStore, DeleteKind, ObjectChange, ParentSync, Storage, WriteKind,
+};
 use sui_types::{
     event::Event,
     gas::{GasCostSummary, SuiGasStatus},
@@ -22,7 +24,7 @@ use sui_types::{
 pub struct InnerTemporaryStore {
     pub objects: BTreeMap<ObjectID, Object>,
     pub mutable_inputs: Vec<ObjectRef>,
-    pub written: BTreeMap<ObjectID, (ObjectRef, Object)>,
+    pub written: BTreeMap<ObjectID, (ObjectRef, Object, WriteKind)>,
     pub deleted: BTreeMap<ObjectID, (SequenceNumber, DeleteKind)>,
 }
 
@@ -39,14 +41,11 @@ pub struct TemporaryStore<S> {
     // When an object is being written, we need to ensure that a few invariants hold.
     // It's critical that we always call write_object to update `_written`, instead of writing
     // into _written directly.
-    _written: BTreeMap<ObjectID, Object>, // Objects written
+    _written: BTreeMap<ObjectID, (Object, WriteKind)>, // Objects written
     /// Objects actively deleted.
     deleted: BTreeMap<ObjectID, (SequenceNumber, DeleteKind)>,
     /// Ordered sequence of events emitted by execution
     events: Vec<Event>,
-    // New object IDs created during the transaction, needed for
-    // telling apart unwrapped objects.
-    created_object_ids: BTreeSet<ObjectID>,
 }
 
 impl<S> TemporaryStore<S> {
@@ -63,7 +62,6 @@ impl<S> TemporaryStore<S> {
             _written: BTreeMap::new(),
             deleted: BTreeMap::new(),
             events: Vec::new(),
-            created_object_ids: BTreeSet::new(),
         }
     }
 
@@ -85,7 +83,7 @@ impl<S> TemporaryStore<S> {
         let written = self
             ._written
             .into_iter()
-            .map(|(id, obj)| (id, (obj.compute_object_reference(), obj)))
+            .map(|(id, (obj, kind))| (id, (obj.compute_object_reference(), obj, kind)))
             .collect();
         let deleted = self
             .deleted
@@ -124,7 +122,8 @@ impl<S> TemporaryStore<S> {
             }
         }
         for object in to_be_updated {
-            self.write_object(object);
+            // The object must be mutated as it was present in the input objects
+            self.write_object(object, WriteKind::Mutate);
         }
     }
 
@@ -144,18 +143,14 @@ impl<S> TemporaryStore<S> {
             gas_object_size,
             gas_object.storage_rebate.into(),
         )?;
-        objects_to_update.push(gas_object.clone());
-
-        for (object_id, object) in &mut self._written {
-            let (old_object_size, storage_rebate) =
-                if let Some(old_object) = self.input_objects.get(object_id) {
-                    (
-                        old_object.object_size_for_gas_metering(),
-                        old_object.storage_rebate,
-                    )
-                } else {
-                    (0, 0)
-                };
+        objects_to_update.push((gas_object.clone(), WriteKind::Mutate));
+
+        for (object_id, (object, write_kind)) in &mut self._written {
+            let (old_object_size, storage_rebate) = self
+                .input_objects
+                .get(object_id)
+                .map(|old| (old.object_size_for_gas_metering(), old.storage_rebate))
+                .unwrap_or((0, 0));
             let new_storage_rebate = gas_status.charge_storage_mutation(
                 old_object_size,
                 object.object_size_for_gas_metering(),
@@ -165,7 +160,7 @@ impl<S> TemporaryStore<S> {
                 // We don't need to set storage rebate for immutable objects, as they will
                 // never be deleted.
                 object.storage_rebate = new_storage_rebate;
-                objects_to_update.push(object.clone());
+                objects_to_update.push((object.clone(), *write_kind));
             }
         }
 
@@ -185,8 +180,8 @@ impl<S> TemporaryStore<S> {
 
         // Write all objects at the end only if all previous gas charges succeeded.
         // This avoids polluting the temporary store state if this function failed.
-        for object in objects_to_update {
-            self.write_object(object);
+        for (object, write_kind) in objects_to_update {
+            self.write_object(object, write_kind);
         }
 
         Ok(())
@@ -201,35 +196,31 @@ impl<S> TemporaryStore<S> {
         status: ExecutionStatus,
         gas_object_ref: ObjectRef,
     ) -> (InnerTemporaryStore, TransactionEffects) {
-        let written = self
+        let written: BTreeMap<ObjectID, (ObjectRef, Owner, WriteKind)> = self
             ._written
             .iter()
-            .map(|(id, obj)| (*id, (obj.compute_object_reference(), obj.owner)))
-            .collect::<BTreeMap<_, _>>();
+            .map(|(id, (obj, write_kind))| {
+                let obj_ref = obj.compute_object_reference();
+                (*id, (obj_ref, obj.owner, *write_kind))
+            })
+            .collect();
 
         // In the case of special transactions that don't require a gas object,
         // we don't really care about the effects to gas, just use the input for it.
         let updated_gas_object_info = if gas_object_ref.0 == ObjectID::ZERO {
             (gas_object_ref, Owner::AddressOwner(SuiAddress::default()))
         } else {
-            written[&gas_object_ref.0]
+            let (obj_ref, owner, _kind) = written[&gas_object_ref.0];
+            (obj_ref, owner)
         };
-        let mut created = vec![];
         let mut mutated = vec![];
+        let mut created = vec![];
         let mut unwrapped = vec![];
-        for (id, object_ref_and_owner) in written {
-            match (
-                self.created_object_ids.contains(&id),
-                self.input_objects.contains_key(&id),
-            ) {
-                (true, _) => created.push(object_ref_and_owner),
-                (false, true) => mutated.push(object_ref_and_owner),
-                (false, false) => {
-                    // wrapped objects must have their version set to 1 + the last known version in
-                    // the `parent_sync`
-                    debug_assert!(object_ref_and_owner.0 .1.value() > 1);
-                    unwrapped.push(object_ref_and_owner)
-                }
+        for (_id, (object_ref, owner, kind)) in written {
+            match kind {
+                WriteKind::Mutate => mutated.push((object_ref, owner)),
+                WriteKind::Create => created.push((object_ref, owner)),
+                WriteKind::Unwrap => unwrapped.push((object_ref, owner)),
             }
         }
 
@@ -292,19 +283,11 @@ impl<S> TemporaryStore<S> {
             "Mutable input neither written nor deleted."
         );
 
-        debug_assert!(
-            {
-                let input_ids = self.input_objects.clone().into_keys().collect();
-                self.created_object_ids.is_disjoint(&input_ids)
-            },
-            "Newly created object IDs showed up in the input",
-        );
-
         debug_assert!(
             {
                 self._written
                     .iter()
-                    .all(|(_, obj)| obj.previous_transaction == self.tx_digest)
+                    .all(|(_, (obj, _))| obj.previous_transaction == self.tx_digest)
             },
             "Object previous transaction not properly set",
         );
@@ -314,7 +297,7 @@ impl<S> TemporaryStore<S> {
     // is that an entry is not both added and deleted by the
     // caller.
 
-    pub fn write_object(&mut self, mut object: Object) {
+    pub fn write_object(&mut self, mut object: Object, kind: WriteKind) {
         // there should be no write after delete
         debug_assert!(self.deleted.get(&object.id()) == None);
         // Check it is not read-only
@@ -330,7 +313,7 @@ impl<S> TemporaryStore<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, kind));
     }
 
     pub fn delete_object(&mut self, id: &ObjectID, version: SequenceNumber, kind: DeleteKind) {
@@ -358,26 +341,25 @@ impl<S> Storage for TemporaryStore<S> {
         self._written.clear();
         self.deleted.clear();
         self.events.clear();
-        self.created_object_ids.clear();
     }
 
     fn read_object(&self, id: &ObjectID) -> Option<&Object> {
         // there should be no read after delete
         debug_assert!(self.deleted.get(id) == None);
-        self._written.get(id).or_else(|| self.input_objects.get(id))
-    }
-
-    fn set_create_object_ids(&mut self, ids: BTreeSet<ObjectID>) {
-        self.created_object_ids = ids;
+        self._written
+            .get(id)
+            .map(|(obj, _kind)| obj)
+            .or_else(|| self.input_objects.get(id))
     }
 
     fn log_event(&mut self, event: Event) {
         self.events.push(event)
     }
+
     fn apply_object_changes(&mut self, changes: BTreeMap<ObjectID, ObjectChange>) {
         for (id, change) in changes {
             match change {
-                ObjectChange::Write(new_value) => self.write_object(new_value),
+                ObjectChange::Write(new_value, kind) => self.write_object(new_value, kind),
                 ObjectChange::Delete(version, kind) => self.delete_object(&id, version, kind),
             }
         }

crates/sui-core/src/authority.rs

@@ -866,7 +866,10 @@ impl AuthorityState {
                 .input_objects()?
                 .iter()
                 .map(|o| o.object_id()),
-            effects.effects.all_mutated(),
+            effects
+                .effects
+                .all_mutated()
+                .map(|(obj_ref, owner, _kind)| (*obj_ref, *owner)),
             cert.signed_data
                 .data
                 .move_calls()