Enforce previous tx set in active input objects

sui_core/src/authority.rs

@@ -358,6 +358,7 @@ impl AuthorityState {
             gas::deduct_gas(&mut gas_object, *gas_used);
             temporary_store.write_object(gas_object);
         }
+        // We must ensure that even in the case of failure, ensure to advance seq# and update previous tx
         temporary_store.ensure_active_inputs_mutated();
         Ok((temporary_store, status))
     }

sui_core/src/authority/temporary_store.rs

@@ -88,6 +88,7 @@ impl AuthorityTemporaryStore {
                 let mut object = self.objects[id].clone();
                 // Active input object must be Move object.
                 object.data.try_as_move_mut().unwrap().increment_version();
+                object.previous_transaction = self.tx_digest;
                 self.written.insert(*id, object);
             }
         }
@@ -220,7 +221,7 @@ impl Storage for AuthorityTemporaryStore {
         caller.
     */
 
-    fn write_object(&mut self, mut object: Object) {
+    fn write_object(&mut self, object: Object) {
         // there should be no write after delete
         debug_assert!(self.deleted.get(&object.id()) == None);
         // Check it is not read-only
@@ -233,9 +234,10 @@ impl Storage for AuthorityTemporaryStore {
             }
         }
 
+        // TODO: do we still need this?
         // 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;
+        // object.previous_transaction = self.tx_digest;
         self.written.insert(object.id(), object);
     }
 

sui_core/src/unit_tests/authority_tests.rs

@@ -1509,6 +1509,165 @@ async fn test_object_owning_another_object() {
     );
 }
 
+#[tokio::test]
+async fn test_object_previous_transaction_field() {
+    let (sender, sender_key) = get_key_pair();
+    let (recipient, _) = get_key_pair();
+
+    let gas_object_id = ObjectID::random();
+    let rec_gas_object_id = ObjectID::random();
+
+    let authority_state = init_state_with_ids(vec![
+        (sender, gas_object_id),
+        (recipient, rec_gas_object_id),
+    ])
+    .await;
+
+    let effects = create_move_object(&authority_state, &gas_object_id, &sender, &sender_key)
+        .await
+        .unwrap();
+    let (new_object_id1, _seq1, _) = effects.created[0].0;
+    let obj1 = authority_state
+        .get_object(&new_object_id1)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(effects.transaction_digest, obj1.previous_transaction);
+
+    let effects = create_move_object(&authority_state, &gas_object_id, &sender, &sender_key)
+        .await
+        .unwrap();
+    let (new_object_id2, _seq2, _) = effects.created[0].0;
+
+    let effects = call_framework_code(
+        &authority_state,
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        "ObjectBasics",
+        "update",
+        vec![],
+        vec![new_object_id1, new_object_id2],
+        vec![],
+    )
+    .await
+    .unwrap();
+    println!("{}", effects);
+    println!("{:?}", effects.transaction_digest);
+
+    let obj1 = authority_state
+        .get_object(&new_object_id1)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(effects.transaction_digest, obj1.previous_transaction);
+    let obj2 = authority_state
+        .get_object(&new_object_id2)
+        .await
+        .unwrap()
+        .unwrap();
+    println!("{:?}", obj1.to_object_reference());
+    println!("{:?}", obj2.to_object_reference());
+
+    assert_eq!(effects.transaction_digest, obj2.previous_transaction);
+
+    // Check entry for object to be deleted is returned
+    let (obj_ref, tx) = authority_state
+        .get_latest_parent_entry(new_object_id1)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(obj_ref.0, new_object_id1);
+    assert_eq!(obj_ref.1, SequenceNumber::from(2));
+    assert_eq!(effects.transaction_digest, tx);
+
+    let _ = call_framework_code(
+        &authority_state,
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        "ObjectBasics",
+        "delete",
+        vec![],
+        vec![new_object_id1],
+        vec![],
+    )
+    .await
+    .unwrap();
+
+    assert!(authority_state
+        .get_object(&new_object_id1)
+        .await
+        .unwrap()
+        .is_none());
+    let effects = create_move_object(&authority_state, &gas_object_id, &sender, &sender_key)
+        .await
+        .unwrap();
+    let (new_object_id3, _seq3, _) = effects.created[0].0;
+
+    // Transfer obj1 to obj2.
+    let effects = call_framework_code(
+        &authority_state,
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        "ObjectBasics",
+        "transfer_to_object",
+        vec![],
+        vec![new_object_id2, new_object_id3],
+        vec![],
+    )
+    .await
+    .unwrap();
+    assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+    assert_eq!(effects.mutated.len(), 3);
+    assert_eq!(
+        authority_state
+            .get_object(&new_object_id2)
+            .await
+            .unwrap()
+            .unwrap()
+            .owner,
+        SuiAddress::from(new_object_id3),
+    );
+
+    let effects = create_move_object(&authority_state, &gas_object_id, &sender, &sender_key)
+        .await
+        .unwrap();
+    let (new_object_id4, _seq4, _) = effects.created[0].0;
+
+    let effects = call_framework_code(
+        &authority_state,
+        &gas_object_id,
+        &sender,
+        &sender_key,
+        "ObjectBasics",
+        "transfer",
+        vec![],
+        vec![new_object_id4],
+        vec![bcs::to_bytes(&AccountAddress::from(recipient)).unwrap()],
+    )
+    .await
+    .unwrap();
+
+    assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+    assert_eq!(effects.mutated.len(), 2);
+
+    let obj4 = authority_state
+        .get_object(&new_object_id4)
+        .await
+        .unwrap()
+        .unwrap();
+    let gas_obj = authority_state
+        .get_object(&gas_object_id)
+        .await
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(effects.transaction_digest, obj4.previous_transaction);
+    assert_eq!(effects.transaction_digest, gas_obj.previous_transaction);
+}
+
 // helpers
 
 #[cfg(test)]

sui_programmability/adapter/src/adapter.rs

@@ -92,6 +92,7 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
         args,
         mutable_ref_objects,
         by_value_objects,
+        read_only_refs,
     } = match resolve_and_type_check(
         package_object,
         module,
@@ -116,6 +117,7 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
         type_args,
         args,
         mutable_ref_objects,
+        read_only_refs,
         by_value_objects,
         object_owner_map,
         gas_budget,
@@ -128,6 +130,7 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
         ExecutionStatus::Success { gas_used } => {
             match gas::try_deduct_gas(&mut gas_object, gas_used) {
                 Ok(()) => {
+                    gas_object.previous_transaction = ctx.digest();
                     state_view.write_object(gas_object);
                     Ok(ExecutionStatus::Success { gas_used })
                 }
@@ -152,6 +155,7 @@ fn execute_internal<
     type_args: Vec<TypeTag>,
     args: Vec<Vec<u8>>,
     mutable_ref_objects: Vec<Object>,
+    read_ref_objects: Vec<Object>,
     by_value_objects: BTreeMap<ObjectID, Object>,
     object_owner_map: HashMap<SuiAddress, SuiAddress>,
     gas_budget: u64, // gas budget for the current call operation
@@ -213,6 +217,7 @@ fn execute_internal<
                 state_view,
                 by_value_objects,
                 mutable_refs,
+                read_ref_objects.into_iter(),
                 events,
                 ctx,
                 object_owner_map,
@@ -330,6 +335,7 @@ pub fn publish<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
                 Vec::new(),
                 args,
                 Vec::new(),
+                Vec::new(),
                 BTreeMap::new(),
                 HashMap::new(),
                 current_gas_budget,
@@ -468,18 +474,45 @@ fn process_successful_execution<
     state_view: &mut S,
     mut by_value_objects: BTreeMap<ObjectID, Object>,
     mutable_refs: impl Iterator<Item = (Object, Vec<u8>)>,
+    read_ref_objects: impl Iterator<Item = Object>,
     events: Vec<MoveEvent>,
     ctx: &TxContext,
     mut object_owner_map: HashMap<SuiAddress, SuiAddress>,
 ) -> (u64, u64, SuiResult) {
+    // Four situations with object arguments
+    // 1. Immutable objects: nothing to do
+    // 2. Mutable objects but not mutated during TX (read ref): update the previous_tx and incr the seq #
+    // 3. Mutable objects mutated during tx (mutable ref): update the contents, previous_tx, incr the seq #
+    // 4. Mutable objects mutated during tx (by value Structs, a.k.a consumed): update the contents/owner, previous_tx, incr the seq #
+
+    // 1. Immutable objects: nothing to do
+    // 2. Mutable objects but not mutated during TX (read ref): update the previous_tx and incr the seq #
+    for mut obj in read_ref_objects {
+        if !obj.is_read_only() {
+            // Update preivous tx for non-readonly objects
+            obj.data
+                .try_as_move_mut()
+                .expect("Mutable objects must be Move objects")
+                .increment_version();
+            obj.previous_transaction = ctx.digest();
+            state_view.write_object(obj)
+        }
+    }
+
+    // 3. Mutable objects mutated during tx (mutable ref): update the contents, previous_tx, incr the seq #
     for (mut obj, new_contents) in mutable_refs {
         // update contents and increment sequence number
         obj.data
             .try_as_move_mut()
             .expect("We previously checked that mutable ref inputs are Move objects")
             .update_contents(new_contents);
+        obj.previous_transaction = ctx.digest();
         state_view.write_object(obj);
     }
+
+    // 4. Mutable objects mutated during tx (by value Structs, a.k.a consumed): update the contents/owner, previous_tx, incr the seq #
+    // This step is handled as part of `handle_transfer`
+
     // process events to identify transfers, freezes
     let mut gas_used = 0;
     let tx_digest = ctx.digest();

sui_programmability/adapter/src/unit_tests/adapter_tests.rs

@@ -321,7 +321,8 @@ fn test_object_basics() {
     )
     .unwrap()
     .unwrap();
-    assert_eq!(storage.updated().len(), 2);
+    // Although obj2 contents are not changed, we update its previous tx and seq #
+    assert_eq!(storage.updated().len(), 3);
     assert!(storage.created().is_empty());
     assert!(storage.deleted().is_empty());
     // test than an event was emitted as expected

sui_types/src/move_package.rs

@@ -33,6 +33,7 @@ pub struct TypeCheckSuccess {
     pub args: Vec<Vec<u8>>,
     pub by_value_objects: BTreeMap<ObjectID, Object>,
     pub mutable_ref_objects: Vec<Object>,
+    pub read_only_refs: Vec<Object>,
 }
 
 // serde_bytes::ByteBuf is an analog of Vec<u8> with built-in fast serialization.
@@ -210,6 +211,7 @@ pub fn resolve_and_type_check(
     let mut num_immutable_objects = 0;
     #[cfg(debug_assertions)]
     let num_objects = object_args.len();
+    let mut read_only_refs = vec![];
 
     let ty_args: Vec<Type> = type_args.iter().map(|t| Type::from(t.clone())).collect();
     for (idx, object) in object_args.into_iter().enumerate() {
@@ -240,6 +242,7 @@ pub fn resolve_and_type_check(
                         {
                             num_immutable_objects += 1
                         }
+                        read_only_refs.push(object);
                     }
                     Type::Struct { .. } => {
                         if object.is_read_only() {
@@ -293,6 +296,7 @@ pub fn resolve_and_type_check(
         args,
         by_value_objects,
         mutable_ref_objects,
+        read_only_refs,
     })
 }