[messages][object] Remember version object is shared at (MystenLabs#4878

)

* [sdk][ts] Support initial_shared_version in TS SDK

Also adds a test to make a call involving a shared object via the TS
SDK to confirm correctness of the change (the test passes before and
after the PR, but fails without the rest of this commit).

* [messages][object] Add initial shared object version

- To fix an issue around shared object upgrades, we will now require the initial version of a shared object to be specified in transaction arguments
- This information must be verified when creating a certificate
- This initial version is needed only for shared objects, and as such will be specified in the shared object owner

* [messages][object] Update snapshots

JSON RPC Spec, SUI.yaml, empirical transaction costs, etc.

* [messages][object] Tests for shared object initial versions

Tests for happy and error paths for creating a shared object and
making an owned object shared.

## Test Plan

```
cargo simtest -E 'binary(shared_objects_version_tests)'
```

Co-authored-by: Todd Nowacki <[email protected]>

.changeset/proud-laws-drop.md

@@ -0,0 +1,5 @@
+---
+"@mysten/sui.js": patch
+---
+
+Protocol change to add 'initial shared version' to shared object references.

apps/explorer/src/utils/objectUtils.ts

@@ -48,6 +48,7 @@ export function getOwnerStr(owner: ObjectOwner | string): string {
         if ('AddressOwner' in owner) return owner.AddressOwner;
         if ('ObjectOwner' in owner) return owner.ObjectOwner;
         if ('SingleOwner' in owner) return owner.SingleOwner;
+        if ('Shared' in owner) return 'Shared';
     }
     return owner;
 }

crates/sui-adapter/src/adapter.rs

@@ -110,7 +110,7 @@ pub fn execute<
         .filter_map(|arg| match arg {
             CallArg::Pure(_) => None,
             CallArg::Object(ObjectArg::ImmOrOwnedObject((id, _, _)))
-            | CallArg::Object(ObjectArg::SharedObject(id)) => {
+            | CallArg::Object(ObjectArg::SharedObject { id, .. }) => {
                 Some(vec![(*id, state_view.read_object(id)?)])
             }
             CallArg::ObjVec(vec) => {
@@ -121,7 +121,7 @@ pub fn execute<
                     vec.iter()
                         .filter_map(|obj_arg| match obj_arg {
                             ObjectArg::ImmOrOwnedObject((id, _, _))
-                            | ObjectArg::SharedObject(id) => {
+                            | ObjectArg::SharedObject { id, .. } => {
                                 Some((*id, state_view.read_object(id)?))
                             }
                         })
@@ -517,7 +517,7 @@ fn process_successful_execution<S: Storage + ParentSync>(
     }
     let tx_digest = ctx.digest();
 
-    for (id, (write_kind, recipient, tag, abilities, contents)) in writes {
+    for (id, (write_kind, mut recipient, tag, abilities, contents)) in writes {
         let has_public_transfer = abilities.has_store();
         debug_assert_eq!(
             id,
@@ -568,6 +568,22 @@ fn process_successful_execution<S: Storage + ParentSync>(
         // freshly created, this means that its version will now be 1.
         // thus, all objects in the global object pool have version > 0
         move_obj.increment_version();
+
+        // Remember the version this object was shared at, if this write was the one that shared it.
+        if let Owner::Shared {
+            initial_shared_version,
+        } = &mut recipient
+        {
+            // TODO Consider a distinct Recipient enum within ObjectRuntime to enforce this
+            // invariant at the type level.
+            assert_eq!(
+                *initial_shared_version,
+                SequenceNumber::new(),
+                "Initial version should be blank before this point",
+            );
+            *initial_shared_version = move_obj.version();
+        }
+
         // A to-be-transferred object can come from 3 sources:
         //   1. Passed in by-value (in `by_value_objects`, i.e. old_object is not none)
         //   2. Created in this transaction (in `newly_generated_ids`)
@@ -814,9 +830,15 @@ pub fn resolve_and_type_check(
                     type_check_struct(view, type_args, idx, arg_type, param_type)?;
                     o
                 }
-                CallArg::Object(ObjectArg::SharedObject(id)) => {
+                CallArg::Object(ObjectArg::SharedObject {
+                    id,
+                    initial_shared_version,
+                }) => {
                     let (o, arg_type, param_type) = serialize_object(
-                        InputObjectKind::SharedMoveObject(id),
+                        InputObjectKind::SharedMoveObject {
+                            id,
+                            initial_shared_version,
+                        },
                         idx,
                         param_type,
                         objects,
@@ -842,7 +864,13 @@ pub fn resolve_and_type_check(
                             ObjectArg::ImmOrOwnedObject(ref_) => {
                                 InputObjectKind::ImmOrOwnedMoveObject(ref_)
                             }
-                            ObjectArg::SharedObject(id) => InputObjectKind::SharedMoveObject(id),
+                            ObjectArg::SharedObject {
+                                id,
+                                initial_shared_version,
+                            } => InputObjectKind::SharedMoveObject {
+                                id,
+                                initial_shared_version,
+                            },
                         };
                         let (o, arg_type, param_type) = serialize_object(
                             object_kind,
@@ -1052,7 +1080,7 @@ fn serialize_object<'a>(
                 error,
             ));
         }
-        InputObjectKind::SharedMoveObject(_) if !object.is_shared() => {
+        InputObjectKind::SharedMoveObject { .. } if !object.is_shared() => {
             let error = format!(
                 "Argument at index {} populated with an immutable or owned object id {} \
                         but an shared object was expected",
@@ -1149,7 +1177,7 @@ fn inner_param_type<'a>(
         | t @ SignatureToken::TypeParameter(_) => {
             match &object.owner {
                 Owner::AddressOwner(_) | Owner::ObjectOwner(_) => (),
-                Owner::Shared | Owner::Immutable => {
+                Owner::Shared { .. } | Owner::Immutable => {
                     return Err(ExecutionError::new_with_source(
                         ExecutionErrorKind::entry_argument_error(
                             idx,

crates/sui-adapter/src/object_root_ancestor_map.rs

@@ -44,7 +44,7 @@ impl ObjectRootAncestorMap {
                             );
                         }
                     }
-                    Owner::AddressOwner(_) | Owner::Immutable | Owner::Shared => {
+                    Owner::AddressOwner(_) | Owner::Immutable | Owner::Shared { .. } => {
                         break (cur_id, cur_owner);
                     }
                 };

crates/sui-benchmark/src/workloads/shared_counter.rs

@@ -10,7 +10,7 @@ use sui_core::{
     authority_aggregator::AuthorityAggregator, authority_client::NetworkAuthorityClient,
 };
 use sui_types::{
-    base_types::{ObjectID, ObjectRef, SuiAddress},
+    base_types::{ObjectID, ObjectRef, SequenceNumber, SuiAddress},
     crypto::{get_key_pair, AccountKeyPair, EmptySignInfo},
     messages::TransactionEnvelope,
     object::Owner,
@@ -23,6 +23,7 @@ use test_utils::{
 pub struct SharedCounterTestPayload {
     package_ref: ObjectRef,
     counter_id: ObjectID,
+    counter_initial_shared_version: SequenceNumber,
     gas: Gas,
     sender: SuiAddress,
     keypair: Arc<AccountKeyPair>,
@@ -33,6 +34,7 @@ impl Payload for SharedCounterTestPayload {
         Box::new(SharedCounterTestPayload {
             package_ref: self.package_ref,
             counter_id: self.counter_id,
+            counter_initial_shared_version: self.counter_initial_shared_version,
             gas: (new_gas, self.gas.1),
             sender: self.sender,
             keypair: self.keypair.clone(),
@@ -43,6 +45,7 @@ impl Payload for SharedCounterTestPayload {
             self.gas.0,
             self.package_ref,
             self.counter_id,
+            self.counter_initial_shared_version,
             self.sender,
             &self.keypair,
         )
@@ -164,9 +167,11 @@ impl Workload<dyn Payload> for SharedCounterWorkload {
                     &keypair,
                 );
                 if let Some(effects) = submit_transaction(transaction, agg.clone()).await {
+                    let counter_ref = effects.created[0].0;
                     Box::new(SharedCounterTestPayload {
                         package_ref: self.basics_package_ref.unwrap(),
-                        counter_id: effects.created[0].0 .0,
+                        counter_id: counter_ref.0,
+                        counter_initial_shared_version: counter_ref.1,
                         gas: effects.gas_object,
                         sender,
                         keypair: Arc::new(keypair),

crates/sui-cluster-test/src/test_case/shared_object_test.rs

@@ -30,7 +30,7 @@ impl TestCaseImpl for SharedCounterTest {
 
         let wallet_context: &WalletContext = ctx.get_wallet();
         let address = ctx.get_wallet_address();
-        let (package_ref, counter_id) =
+        let (package_ref, (counter_id, counter_version, _)) =
             publish_basics_package_and_make_counter(wallet_context, address).await;
         let (tx_cert, effects) =
             increment_counter(wallet_context, address, None, package_ref, counter_id).await;
@@ -51,7 +51,9 @@ impl TestCaseImpl for SharedCounterTest {
             .await;
 
         let counter_object = ObjectChecker::new(counter_id)
-            .owner(Owner::Shared)
+            .owner(Owner::Shared {
+                initial_shared_version: counter_version,
+            })
             .check_into_object(ctx.get_fullnode_client())
             .await;
 

crates/sui-core/src/authority.rs

@@ -2105,7 +2105,7 @@ impl AuthorityState {
         certificate: &CertifiedTransaction,
     ) -> SuiResult<bool> {
         let digest = certificate.digest();
-        let shared_inputs = certificate.shared_input_objects();
+        let shared_inputs = certificate.shared_input_objects().map(|(id, _)| id);
         let shared_locks = self
             .database
             .get_assigned_object_versions(digest, shared_inputs)?;

crates/sui-core/src/authority/authority_store.rs

@@ -22,7 +22,7 @@ use sui_storage::{
 };
 use sui_types::batch::{SignedBatch, TxSequenceNumber};
 use sui_types::crypto::{AuthoritySignInfo, EmptySignInfo};
-use sui_types::object::{Owner, OBJECT_START_VERSION};
+use sui_types::object::Owner;
 use sui_types::storage::WriteKind;
 use sui_types::{base_types::SequenceNumber, storage::ParentSync};
 use tokio::sync::Notify;
@@ -316,7 +316,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
         let mut errors = Vec::new();
         for kind in objects {
             let obj = match kind {
-                InputObjectKind::MovePackage(id) | InputObjectKind::SharedMoveObject(id) => {
+                InputObjectKind::MovePackage(id) | InputObjectKind::SharedMoveObject { id, .. } => {
                     self.get_object(id)?
                 }
                 InputObjectKind::ImmOrOwnedMoveObject(objref) => {
@@ -348,7 +348,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
         for kind in objects {
             let obj = match kind {
                 InputObjectKind::MovePackage(id) => self.get_object(id)?,
-                InputObjectKind::SharedMoveObject(id) => match shared_locks.get(id) {
+                InputObjectKind::SharedMoveObject { id, .. } => match shared_locks.get(id) {
                     Some(version) => self.get_object_by_key(id, *version)?,
                     None => {
                         errors.push(SuiError::SharedObjectLockNotSetError);
@@ -1047,7 +1047,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
     ) -> SuiResult {
         let mut sequenced_to_delete = Vec::new();
         let mut schedule_to_delete = Vec::new();
-        for object_id in transaction.shared_input_objects() {
+        for (object_id, _) in transaction.shared_input_objects() {
             sequenced_to_delete.push((*transaction_digest, *object_id));
             if self.get_object(object_id)?.is_none() {
                 schedule_to_delete.push(*object_id);
@@ -1117,30 +1117,27 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
         let transaction_digest = *certificate.digest();
 
         // Make an iterator to update the locks of the transaction's shared objects.
-        let ids = certificate.shared_input_objects();
+        let ids = certificate.shared_input_objects().map(|(id, _)| id);
         let versions = self.epoch_tables().next_object_versions.multi_get(ids)?;
 
-        let ids = certificate.shared_input_objects();
-        type LockWrite = (
-            ((TransactionDigest, ObjectID), SequenceNumber),
-            (ObjectID, SequenceNumber),
-        );
         let mut sequenced_to_write = Vec::new();
         let mut schedule_to_write = Vec::new();
-
-        for (id, v) in ids.zip(versions.iter()) {
-            // The object may have been sequenced in a previous epoch - if so the start version
-            // is the latest available version of the object. Otherwise, this is the first time
-            // the shared object has been sequenced, so we use OBJECT_START_VERSION.
-            // Otherwise use the `scheduled` map to to assign the next sequence number.
+        for ((id, initial_shared_version), v) in
+            certificate.shared_input_objects().zip(versions.iter())
+        {
+            // If it is the first time the shared object has been sequenced, assign it the version
+            // that the object was shared at.  This `initial_shared_version` will be the initial
+            // version for the object if it was created as a shared object, or will be the version
+            // it was upgraded to a shared object.  We can trust this number as validity is checked
+            // when creating a certificate
             let version = match v {
                 Some(v) => *v,
                 None => self
                     // TODO: if we use an eventually consistent object store in the future,
                     // we must make this read strongly consistent somehow!
                     .get_latest_parent_entry(*id)?
                     .map(|(objref, _)| objref.1)
-                    .unwrap_or(OBJECT_START_VERSION),
+                    .unwrap_or_else(|| *initial_shared_version),
             };
             let next_version = version.increment();
 

crates/sui-core/src/execution_engine.rs

@@ -9,6 +9,7 @@ use sui_types::storage::{DeleteKind, ObjectResolver, ParentSync, WriteKind};
 #[cfg(test)]
 use sui_types::temporary_store;
 use sui_types::temporary_store::InnerTemporaryStore;
+use sui_types::SUI_SYSTEM_STATE_OBJECT_SHARED_VERSION;
 
 use crate::authority::TemporaryStore;
 use move_core_types::language_storage::ModuleId;
@@ -227,7 +228,10 @@ fn execute_transaction<S: BackingPackageStore + ParentSync>(
                         &function,
                         vec![],
                         vec![
-                            CallArg::Object(ObjectArg::SharedObject(SUI_SYSTEM_STATE_OBJECT_ID)),
+                            CallArg::Object(ObjectArg::SharedObject {
+                                id: SUI_SYSTEM_STATE_OBJECT_ID,
+                                initial_shared_version: SUI_SYSTEM_STATE_OBJECT_SHARED_VERSION,
+                            }),
                             CallArg::Pure(bcs::to_bytes(&epoch).unwrap()),
                             CallArg::Pure(bcs::to_bytes(&storage_charge).unwrap()),
                             CallArg::Pure(bcs::to_bytes(&computation_charge).unwrap()),

crates/sui-core/src/gateway_state.rs

@@ -1163,10 +1163,16 @@ where
         objects: &mut BTreeMap<ObjectID, Object>,
     ) -> Result<ObjectArg, anyhow::Error> {
         let obj = self.get_object_internal(&id).await?;
-        let arg = if obj.is_shared() {
-            ObjectArg::SharedObject(id)
-        } else {
-            ObjectArg::ImmOrOwnedObject(obj.compute_object_reference())
+        let arg = match obj.owner {
+            Owner::Shared {
+                initial_shared_version,
+            } => ObjectArg::SharedObject {
+                id: obj.id(),
+                initial_shared_version,
+            },
+            Owner::AddressOwner(_) | Owner::ObjectOwner(_) | Owner::Immutable => {
+                ObjectArg::ImmOrOwnedObject(obj.compute_object_reference())
+            }
         };
         objects.insert(id, obj);
         Ok(arg)

crates/sui-core/src/transaction_input_checker.rs

@@ -285,20 +285,36 @@ fn check_one_object(
                         }
                     );
                 }
-                Owner::Shared => {
+                Owner::Shared { .. } => {
                     // This object is a mutable shared object. However the transaction
                     // specifies it as an owned object. This is inconsistent.
                     return Err(SuiError::NotSharedObjectError);
                 }
             };
         }
-        InputObjectKind::SharedMoveObject(..) => {
+        InputObjectKind::SharedMoveObject {
+            initial_shared_version: input_initial_shared_version,
+            ..
+        } => {
             fp_ensure!(
                 object.version() < SequenceNumber::MAX,
                 SuiError::InvalidSequenceNumber
             );
-            // When someone locks an object as shared it must be shared already.
-            fp_ensure!(object.is_shared(), SuiError::NotSharedObjectError);
+
+            match object.owner {
+                Owner::AddressOwner(_) | Owner::ObjectOwner(_) | Owner::Immutable => {
+                    // When someone locks an object as shared it must be shared already.
+                    return Err(SuiError::NotSharedObjectError);
+                }
+                Owner::Shared {
+                    initial_shared_version: actual_initial_shared_version,
+                } => {
+                    fp_ensure!(
+                        input_initial_shared_version == actual_initial_shared_version,
+                        SuiError::SharedObjectStartingVersionMismatch
+                    )
+                }
+            }
         }
     };
     Ok(())

crates/sui-core/src/unit_tests/authority_aggregator_tests.rs

@@ -1415,7 +1415,14 @@ pub fn make_response_from_sui_system_state(
             move_content,
         )
     };
-    let object = Object::new_move(move_object, Owner::Shared, *tx_cert.digest());
+    let initial_shared_version = move_object.version();
+    let object = Object::new_move(
+        move_object,
+        Owner::Shared {
+            initial_shared_version,
+        },
+        *tx_cert.digest(),
+    );
     let obj_digest = object.compute_object_reference();
     Ok(ObjectInfoResponse {
         parent_certificate: Some(tx_cert),