Skip to content

Commit

Permalink
[Move] Add Transfer::delete_child_object (MystenLabs#723)
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind authored Mar 11, 2022
1 parent fa5ac73 commit 22ee098
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,12 @@ module ObjectOwner::ObjectOwner {
let Child { id } = child;
ID::delete(id);
}

public fun delete_parent_and_child(parent: Parent, child: Child, _ctx: &mut TxContext) {
let Parent { id, child: child_ref_opt } = parent;
let child_ref = Option::extract(&mut child_ref_opt);
Option::destroy_none(child_ref_opt);
Transfer::delete_child_object(child, child_ref);
ID::delete(id);
}
}
31 changes: 27 additions & 4 deletions sui_core/src/unit_tests/move_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,33 @@ async fn test_object_owning_another_object() {
assert!(effects.status.is_ok());
assert_eq!(effects.created.len(), 2);
// Check that one of them is the parent and the other is the child.
assert!(
(effects.created[0].1 == sender && effects.created[1].1 == effects.created[0].0 .0)
|| (effects.created[1].1 == sender && effects.created[0].1 == effects.created[1].0 .0)
);
let (parent, child) = if effects.created[0].1 == sender {
assert_eq!(effects.created[1].1, effects.created[0].0 .0);
(effects.created[0].0, effects.created[1].0)
} else {
assert!(effects.created[1].1 == sender && effects.created[0].1 == effects.created[1].0 .0);
(effects.created[1].0, effects.created[0].0)
};

// Delete the parent and child altogether.
let effects = call_move(
&authority,
&gas,
&sender,
&sender_key,
&package,
"ObjectOwner",
"delete_parent_and_child",
vec![],
vec![parent.0, child.0],
vec![],
vec![],
)
.await
.unwrap();
assert!(effects.status.is_ok());
// Check that both objects were deleted.
assert_eq!(effects.deleted.len(), 2);
}

async fn build_and_publish_test_package(
Expand Down
26 changes: 21 additions & 5 deletions sui_programmability/adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use move_vm_runtime::{native_functions::NativeFunctionTable, session::ExecutionR
use std::{
borrow::Borrow,
cmp,
collections::{BTreeMap, HashMap},
collections::{BTreeMap, HashMap, HashSet},
convert::TryFrom,
fmt::Debug,
sync::Arc,
Expand Down Expand Up @@ -526,6 +526,7 @@ fn process_successful_execution<
let mut gas_used = 0;
let tx_digest = ctx.digest();
let mut deleted_ids = HashMap::new();
let mut deleted_child_ids = HashSet::new();
for e in events {
let (recipient, event_type, type_, event_bytes) = e;
let result = match EventType::try_from(event_type as u8)
Expand Down Expand Up @@ -569,6 +570,19 @@ fn process_successful_execution<
Ok(())
}
EventType::ShareObject => Err(SuiError::UnsupportedSharedObjectError),
EventType::DeleteChildObject => {
match type_ {
TypeTag::Struct(s) => {
let obj = MoveObject::new(s, event_bytes);
deleted_ids.insert(obj.id(), obj.version());
deleted_child_ids.insert(obj.id());
},
_ => unreachable!(
"Native function delete_child_object_internal<T> ensures that T is always bound to structs"
),
}
Ok(())
}
EventType::User => {
match type_ {
TypeTag::Struct(s) => state_view.log_event(Event::new(s, event_bytes)),
Expand All @@ -589,11 +603,13 @@ fn process_successful_execution<
// (2) wrapped inside another object that is in the Sui object pool
let mut gas_refund: u64 = 0;
for (id, object) in by_value_objects.iter() {
// If an object is owned by another object, we are not allowed to delete the child
// If an object is owned by another object, we are not allowed to directly delete the child
// object because this could lead to a dangling reference of the ownership. Such
// dangling reference can never be dropped. To delete this object, it must first
// be transferred to an account address.
if matches!(object.owner, Owner::ObjectOwner { .. }) {
// dangling reference can never be dropped. To delete this object, one must either first transfer
// the child object to an account address, or call through Transfer::delete_child_object(),
// which would consume both the child object and the ChildRef ownership reference,
// and emit the DeleteChildObject event. These child objects can be safely deleted.
if matches!(object.owner, Owner::ObjectOwner { .. }) && !deleted_child_ids.contains(id) {
return (gas_used, 0, Err(SuiError::DeleteObjectOwnedObject));
}
if deleted_ids.contains_key(id) {
Expand Down
12 changes: 12 additions & 0 deletions sui_programmability/framework/sources/Transfer.move
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ module Sui::Transfer {
transfer(child, recipient)
}

/// Delete the child object along with a ownership reference that shows this object
/// is owned by another object. Deleting both the child object and the reference
/// is safe because the ownership will also be destroyed, and hence there won't
/// be dangling reference to the child object through ownership.
public fun delete_child_object<T: key>(child: T, child_ref: ChildRef<T>) {
let ChildRef { parent_id: _, child_id } = child_ref;
assert!(&child_id == ID::id(&child), ECHILD_ID_MISMATCH);
delete_child_object_internal(child);
}

/// Freeze `obj`. After freezing `obj` becomes immutable and can no
/// longer be transferred or mutated.
public native fun freeze_object<T: key>(obj: T);
Expand All @@ -112,4 +122,6 @@ module Sui::Transfer {
public native fun share_object<T: key>(obj: T);

native fun transfer_internal<T: key>(obj: T, recipient: address, to_object: bool);

native fun delete_child_object_internal<T: key>(child: T);
}
2 changes: 2 additions & 0 deletions sui_programmability/framework/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ pub enum EventType {
/// deleted, the object ID must be deleted and this event will be
/// emitted.
DeleteObjectID,
/// System event: a child object is deleted along with a child ref.
DeleteChildObject,
/// User-defined event
User,
}
Expand Down
5 changes: 5 additions & 0 deletions sui_programmability/framework/src/natives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ pub fn all_natives(
"transferred_object_ids",
test_scenario::transferred_object_ids,
),
(
"Transfer",
"delete_child_object_internal",
transfer::delete_child_object_internal,
),
("Transfer", "transfer_internal", transfer::transfer_internal),
("Transfer", "freeze_object", transfer::freeze_object),
("Transfer", "share_object", transfer::share_object),
Expand Down
7 changes: 6 additions & 1 deletion sui_programmability/framework/src/natives/test_scenario.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sui_types::{
object::Owner,
};

use super::get_nested_struct_field;
use super::{get_nested_struct_field, get_nth_struct_field};

type Event = (Vec<u8>, u64, Type, MoveTypeLayout, Value);

Expand Down Expand Up @@ -86,6 +86,11 @@ fn get_global_inventory(events: &[Event]) -> Inventory {
// note: obj_id may or may not be present in `inventory`--a useer can create an ID and delete it without associating it with a transferred object
inventory.remove(&get_deleted_id_bytes(val).into());
}
EventType::DeleteChildObject => {
// val is an Sui object, with the first field as the versioned id.
let versioned_id = get_nth_struct_field(val.copy_value().unwrap(), 0);
inventory.remove(&get_deleted_id_bytes(&versioned_id).into());
}
EventType::User => (),
}
}
Expand Down
22 changes: 22 additions & 0 deletions sui_programmability/framework/src/natives/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,25 @@ pub fn share_object(
Ok(NativeResult::err(cost, 0))
}
}

/// Implementation of Move native function
/// `delete_child_object_internal<T: key>(child: T)`
pub fn delete_child_object_internal(
context: &mut NativeContext,
mut ty_args: Vec<Type>,
mut args: VecDeque<Value>,
) -> PartialVMResult<NativeResult> {
debug_assert!(ty_args.len() == 1);
debug_assert!(args.len() == 1);

let ty = ty_args.pop().unwrap();
let obj = args.pop_back().unwrap();
let event_type = EventType::DeleteChildObject;
// TODO: Decide the cost.
let cost = native_gas(context.cost_table(), NativeCostIndex::EMIT_EVENT, 1);
if context.save_event(vec![], event_type as u64, ty, obj)? {
Ok(NativeResult::ok(cost, smallvec![]))
} else {
Ok(NativeResult::err(cost, 0))
}
}

0 comments on commit 22ee098

Please sign in to comment.