[sui framework][verifier] Make Transfer functions restricted (MystenL…

…abs#2396)

* [sui framework][verifier] Make Transfer functions restricted

- Transfer functions are now private to the module that defines the type
- This restriction is lifted for types with store

crates/sui-adapter-transactional-tests/tests/children/child_of_shared_object.move

@@ -10,7 +10,7 @@ module T3::O3 {
     use Sui::Transfer;
     use Sui::TxContext::{Self, TxContext};
 
-    struct O3 has key {
+    struct O3 has key, store {
         id: VersionedID,
     }
 
@@ -28,7 +28,7 @@ module T2::O2 {
     use Sui::TxContext::{Self, TxContext};
     use T3::O3::O3;
 
-    struct O2 has key {
+    struct O2 has key, store {
         id: VersionedID,
         child: ChildRef<O3>,
     }

crates/sui-framework/sources/Bag.move

@@ -74,7 +74,7 @@ module Sui::Bag {
     /// Abort if the object is already in the Bag.
     /// If the object was owned by another object, an `old_child_ref` would be around
     /// and need to be consumed as well.
-    fun add_impl<T: key>(c: &mut Bag, object: T, old_child_ref: Option<ChildRef<T>>) {
+    fun add_impl<T: key + store>(c: &mut Bag, object: T, old_child_ref: Option<ChildRef<T>>) {
         assert!(
             size(c) + 1 <= c.max_capacity,
             Errors::limit_exceeded(EMaxCapacityExceeded)
@@ -89,14 +89,14 @@ module Sui::Bag {
 
     /// Add a new object to the Bag.
     /// Abort if the object is already in the Bag.
-    public fun add<T: key>(c: &mut Bag, object: T) {
+    public fun add<T: key + store>(c: &mut Bag, object: T) {
         add_impl(c, object, Option::none())
     }
 
     /// Transfer a object that was owned by another object to the bag.
     /// Since the object is a child object of another object, an `old_child_ref`
     /// is around and needs to be consumed.
-    public fun add_child_object<T: key>(c: &mut Bag, object: T, old_child_ref: ChildRef<T>) {
+    public fun add_child_object<T: key + store>(c: &mut Bag, object: T, old_child_ref: ChildRef<T>) {
         add_impl(c, object, Option::some(old_child_ref))
     }
 
@@ -108,7 +108,7 @@ module Sui::Bag {
 
     /// Remove and return the object from the Bag.
     /// Abort if the object is not found.
-    public fun remove<T: key>(c: &mut Bag, object: T): T {
+    public fun remove<T: key + store>(c: &mut Bag, object: T): T {
         let idx = find(c, ID::id(&object));
         if (Option::is_none(&idx)) {
             abort EObjectNotFound
@@ -118,16 +118,28 @@ module Sui::Bag {
     }
 
     /// Remove the object from the Bag, and then transfer it to the signer.
-    public(script) fun remove_and_take<T: key>(c: &mut Bag, object: T, ctx: &mut TxContext) {
+    public(script) fun remove_and_take<T: key + store>(c: &mut Bag, object: T, ctx: &mut TxContext) {
         let object = remove(c, object);
         Transfer::transfer(object, TxContext::sender(ctx));
     }
 
     /// Transfer the entire Bag to `recipient`.
-    public(script) fun transfer(c: Bag, recipient: address, _ctx: &mut TxContext) {
+    public(script) fun transfer_(c: Bag, recipient: address) {
         Transfer::transfer(c, recipient)
     }
 
+    /// Transfer the entire Bag to `recipient`.
+    public fun transfer(c: Bag, recipient: address) {
+        Transfer::transfer(c, recipient)
+    }
+
+    public fun transfer_to_object_id(
+        obj: Bag,
+        owner_id: VersionedID,
+    ): (VersionedID, ChildRef<Bag>) {
+        Transfer::transfer_to_object_id(obj, owner_id)
+    }
+
     /// Look for the object identified by `id_bytes` in the Bag.
     /// Returns the index if found, none if not found.
     fun find(c: &Bag, id: &ID): Option<u64> {

crates/sui-framework/sources/Balance.move

@@ -17,6 +17,7 @@ module Sui::Balance {
     /// Storable balance - an inner struct of a Coin type.
     /// Can be used to store coins which don't need to have the
     /// key ability.
+    /// Helpful in representing a Coin without having to create a stand-alone object.
     struct Balance<phantom T> has store {
         value: u64
     }

crates/sui-framework/sources/Coin.move

@@ -8,8 +8,8 @@ module Sui::Coin {
     use Sui::TxContext::{Self, TxContext};
     use Std::Vector;
 
-    /// A coin of type `T` worth `value`. Transferable but not storable
-    struct Coin<phantom T> has key {
+    /// A coin of type `T` worth `value`. Transferable and storable
+    struct Coin<phantom T> has key, store {
         id: VersionedID,
         balance: Balance<T>
     }

crates/sui-framework/sources/Collection.move

@@ -41,19 +41,22 @@ module Sui::Collection {
     // we could use more efficient data structure such as set.
     const DEFAULT_MAX_CAPACITY: u64 = 65536;
 
-    struct Collection<phantom T: key> has key {
+    struct Collection<phantom T: key + store> has key {
         id: VersionedID,
         objects: vector<ChildRef<T>>,
         max_capacity: u64,
     }
 
     /// Create a new Collection and return it.
-    public fun new<T: key>(ctx: &mut TxContext): Collection<T> {
+    public fun new<T: key + store>(ctx: &mut TxContext): Collection<T> {
         new_with_max_capacity(ctx, DEFAULT_MAX_CAPACITY)
     }
 
     /// Create a new Collection with custom size limit and return it.
-    public fun new_with_max_capacity<T: key>(ctx: &mut TxContext, max_capacity: u64): Collection<T> {
+    public fun new_with_max_capacity<T: key + store>(
+        ctx: &mut TxContext,
+        max_capacity: u64,
+    ): Collection<T> {
         assert!(
             max_capacity <= DEFAULT_MAX_CAPACITY && max_capacity > 0 ,
             Errors::limit_exceeded(EInvalidMaxCapacity)
@@ -66,20 +69,24 @@ module Sui::Collection {
     }
 
     /// Create a new Collection and transfer it to the signer.
-    public(script) fun create<T: key>(ctx: &mut TxContext) {
+    public(script) fun create<T: key + store>(ctx: &mut TxContext) {
         Transfer::transfer(new<T>(ctx), TxContext::sender(ctx))
     }
 
     /// Returns the size of the collection.
-    public fun size<T: key>(c: &Collection<T>): u64 {
+    public fun size<T: key + store>(c: &Collection<T>): u64 {
         Vector::length(&c.objects)
     }
 
     /// Add an object to the collection.
     /// If the object was owned by another object, an `old_child_ref` would be around
     /// and need to be consumed as well.
     /// Abort if the object is already in the collection.
-    fun add_impl<T: key>(c: &mut Collection<T>, object: T, old_child_ref: Option<ChildRef<T>>) {
+    fun add_impl<T: key + store>(
+        c: &mut Collection<T>,
+        object: T,
+        old_child_ref: Option<ChildRef<T>>,
+    ) {
         assert!(
             size(c) + 1 <= c.max_capacity,
             Errors::limit_exceeded(EMaxCapacityExceeded)
@@ -98,46 +105,66 @@ module Sui::Collection {
 
     /// Add an object to the collection.
     /// Abort if the object is already in the collection.
-    public fun add<T: key>(c: &mut Collection<T>, object: T) {
+    public fun add<T: key + store>(c: &mut Collection<T>, object: T) {
         add_impl(c, object, Option::none())
     }
 
     /// Transfer an object that was owned by another object to the collection.
     /// Since the object is a child object of another object, an `old_child_ref`
     /// is around and needs to be consumed.
-    public fun add_child_object<T: key>(c: &mut Collection<T>, object: T, old_child_ref: ChildRef<T>) {
+    public fun add_child_object<T: key + store>(
+        c: &mut Collection<T>,
+        object: T,
+        old_child_ref: ChildRef<T>,
+    ) {
         add_impl(c, object, Option::some(old_child_ref))
     }
 
     /// Check whether the collection contains a specific object,
     /// identified by the object id in bytes.
-    public fun contains<T: key>(c: &Collection<T>, id: &ID): bool {
+    public fun contains<T: key + store>(c: &Collection<T>, id: &ID): bool {
         Option::is_some(&find(c, id))
     }
 
     /// Remove and return the object from the collection.
     /// Abort if the object is not found.
-    public fun remove<T: key>(c: &mut Collection<T>, object: T): (T, ChildRef<T>) {
+    public fun remove<T: key + store>(c: &mut Collection<T>, object: T): (T, ChildRef<T>) {
         let idx = find(c, ID::id(&object));
         assert!(Option::is_some(&idx), EObjectNotFound);
         let child_ref = Vector::remove(&mut c.objects, *Option::borrow(&idx));
         (object, child_ref)
     }
 
     /// Remove the object from the collection, and then transfer it to the signer.
-    public(script) fun remove_and_take<T: key>(c: &mut Collection<T>, object: T, ctx: &mut TxContext) {
+    public(script) fun remove_and_take<T: key + store>(
+        c: &mut Collection<T>,
+        object: T,
+        ctx: &mut TxContext,
+    ) {
         let (object, child_ref) = remove(c, object);
         Transfer::transfer_child_to_address(object, child_ref, TxContext::sender(ctx));
     }
 
     /// Transfer the entire collection to `recipient`.
-    public(script) fun transfer<T: key>(c: Collection<T>, recipient: address, _ctx: &mut TxContext) {
+    public(script) fun transfer_<T: key + store>(c: Collection<T>, recipient: address) {
         Transfer::transfer(c, recipient)
     }
 
+    /// Transfer the entire collection to `recipient`.
+    public fun transfer<T: key + store>(c: Collection<T>, recipient: address) {
+        Transfer::transfer(c, recipient)
+    }
+
+    public fun transfer_to_object_id<T: key + store>(
+        obj: Collection<T>,
+        owner_id: VersionedID,
+    ): (VersionedID, ChildRef<Collection<T>>) {
+        Transfer::transfer_to_object_id(obj, owner_id)
+    }
+
     /// Look for the object identified by `id_bytes` in the collection.
     /// Returns the index if found, none if not found.
-    fun find<T: key>(c: &Collection<T>, id: &ID): Option<u64> {
+    fun find<T: key + store>(c: &Collection<T>, id: &ID): Option<u64> {
         let i = 0;
         let len = size(c);
         while (i < len) {
@@ -147,6 +174,6 @@ module Sui::Collection {
             };
             i = i + 1;
         };
-        return Option::none()
+        Option::none()
     }
 }

crates/sui-framework/tests/BagTests.move

@@ -11,11 +11,11 @@ module Sui::BagTests {
     const EBAG_SIZE_MISMATCH: u64 = 0;
     const EOBJECT_NOT_FOUND: u64 = 1;
 
-    struct Object1 has key {
+    struct Object1 has key, store {
         id: VersionedID,
     }
 
-    struct Object2 has key {
+    struct Object2 has key, store {
         id: VersionedID,
     }
 
@@ -60,15 +60,15 @@ module Sui::BagTests {
         // Sui::Bag::DEFAULT_MAX_CAPACITY is not readable outside the module
         let max_capacity = 65536;
         let bag = Bag::new_with_max_capacity(&mut ctx, max_capacity + 1);
-        Bag::transfer(bag, TxContext::sender(&ctx), &mut ctx);
+        Bag::transfer_(bag, TxContext::sender(&ctx));
     }
 
     #[test]
     #[expected_failure(abort_code = 520)]
     public(script) fun test_init_with_zero() {
         let ctx = TxContext::dummy();
         let bag = Bag::new_with_max_capacity(&mut ctx, 0);
-        Bag::transfer(bag, TxContext::sender(&ctx), &mut ctx);
+        Bag::transfer_(bag, TxContext::sender(&ctx));
     }
 
     #[test]
@@ -81,6 +81,6 @@ module Sui::BagTests {
         Bag::add(&mut bag, obj1);
         let obj2 = Object2 { id: TxContext::new_id(&mut ctx) };
         Bag::add(&mut bag, obj2);
-        Bag::transfer(bag, TxContext::sender(&ctx), &mut ctx);
+        Bag::transfer_(bag, TxContext::sender(&ctx));
     }
 }

crates/sui-framework/tests/CollectionTests.move

@@ -9,7 +9,7 @@ module Sui::CollectionTests {
     use Sui::TestScenario;
     use Sui::TxContext;
 
-    struct Object has key {
+    struct Object has key, store {
         id: VersionedID,
     }
 
@@ -114,15 +114,15 @@ module Sui::CollectionTests {
         // Sui::Collection::DEFAULT_MAX_CAPACITY is not readable outside the module
         let max_capacity = 65536;
         let collection = Collection::new_with_max_capacity<Object>(&mut ctx, max_capacity + 1);
-        Collection::transfer(collection, TxContext::sender(&ctx), &mut ctx);
+        Collection::transfer_(collection, TxContext::sender(&ctx));
     }
 
     #[test]
     #[expected_failure(abort_code = 520)]
     public(script) fun test_init_with_zero() {
         let ctx = TxContext::dummy();
         let collection = Collection::new_with_max_capacity<Object>(&mut ctx, 0);
-        Collection::transfer(collection, TxContext::sender(&ctx), &mut ctx);
+        Collection::transfer_(collection, TxContext::sender(&ctx));
     }
 
     #[test]
@@ -135,6 +135,6 @@ module Sui::CollectionTests {
         Collection::add(&mut collection, obj1);
         let obj2 = Object { id: TxContext::new_id(&mut ctx) };
         Collection::add(&mut collection, obj2);
-        Collection::transfer(collection, TxContext::sender(&ctx), &mut ctx);
+        Collection::transfer_(collection, TxContext::sender(&ctx));
     }
 }

crates/sui-transactional-test-runner/src/args.rs

@@ -80,16 +80,16 @@ impl SuiValue {
         }
     }
 
-    pub(crate) fn into_call_args(self, test_adapter: &SuiTestAdapter) -> CallArg {
-        match self {
+    pub(crate) fn into_call_args(self, test_adapter: &SuiTestAdapter) -> anyhow::Result<CallArg> {
+        Ok(match self {
             SuiValue::Object(fake_id) => {
                 let id = match test_adapter.fake_to_real_object_id(fake_id) {
                     Some(id) => id,
-                    None => panic!("Unknown object, object({})", fake_id),
+                    None => bail!("INVALID TEST. Unknown object, object({})", fake_id),
                 };
                 let obj = match test_adapter.storage.get_object(&id) {
                     Some(obj) => obj,
-                    None => panic!("Could not load object argument {}", id),
+                    None => bail!("INVALID TEST. Could not load object argument {}", id),
                 };
                 if obj.is_shared() {
                     CallArg::SharedObject(id)
@@ -99,7 +99,7 @@ impl SuiValue {
                 }
             }
             SuiValue::MoveValue(v) => CallArg::Pure(v.simple_serialize().unwrap()),
-        }
+        })
     }
 }
 

crates/sui-transactional-test-runner/src/test_adapter.rs

@@ -279,7 +279,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter<'a> {
         let arguments = args
             .into_iter()
             .map(|arg| arg.into_call_args(self))
-            .collect();
+            .collect::<anyhow::Result<_>>()?;
         let package_id = ObjectID::from(*module_id.address());
         let package_ref = match self.storage.get_object(&package_id) {
             Some(obj) => obj.compute_object_reference(),

crates/sui-verifier/src/lib.rs

@@ -7,6 +7,7 @@ pub mod entry_points_verifier;
 pub mod global_storage_access_verifier;
 pub mod id_immutable_verifier;
 pub mod id_leak_verifier;
+pub mod private_transfer;
 pub mod struct_with_key_verifier;
 
 use move_binary_format::{