Skip to content

Commit

Permalink
[adapter] Fix bug for unwrapped but never stored object (MystenLabs#4231
Browse files Browse the repository at this point in the history
)

- Fixed a bug where objects where an invariant violation was thrown for unwrapped objects that did not have a last known version
- For deleted unwrapped, we no longer emit a deleted event
- For transferred unwrapped, we now mark it as newly created
  • Loading branch information
tnowacki authored Aug 23, 2022
1 parent c1de7ba commit b98bf4e
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
processed 4 tasks

init:
A: object(100), B: object(101)

task 1 'publish'. lines 8-38:
created: object(105)
written: object(104)

task 2 'run'. lines 40-40:
created: object(107)
written: object(106)

task 3 'run'. lines 42-42:
written: object(108)
deleted: object(107)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// tests deleting a wrapped object that has never been in storage

//# init --addresses test=0x0 --accounts A B

//# publish

module test::m {
use sui::tx_context::{Self, TxContext};

struct S has key, store {
id: sui::object::UID,
}

struct R has key {
id: sui::object::UID,
s: S,
}

public entry fun create(ctx: &mut TxContext) {
let parent = sui::object::new(ctx);
let child = S { id: sui::object::new(ctx) };
sui::transfer::transfer(R { id: parent, s: child }, tx_context::sender(ctx))
}

public entry fun delete(r: R) {
let R { id, s } = r;
sui::object::delete(id);
let S { id } = s;
sui::object::delete(id);
}
}

//
// Test sharing
//

//# run test::m::create --sender A

//# run test::m::delete --args object(107) --sender A
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
processed 4 tasks

init:
A: object(100), B: object(101)

task 1 'publish'. lines 8-37:
created: object(105)
written: object(104)

task 2 'run'. lines 39-39:
created: object(107)
written: object(106)

task 3 'run'. lines 41-41:
created: object(109)
written: object(108)
deleted: object(107)
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// tests transferring a wrapped object that has never previously been in storage

//# init --addresses test=0x0 --accounts A B

//# publish

module test::m {
use sui::tx_context::{Self, TxContext};

struct S has key, store {
id: sui::object::UID,
}

struct R has key {
id: sui::object::UID,
s: S,
}

public entry fun create(ctx: &mut TxContext) {
let parent = sui::object::new(ctx);
let child = S { id: sui::object::new(ctx) };
sui::transfer::transfer(R { id: parent, s: child }, tx_context::sender(ctx))
}

public entry fun unwrap_and_transfer(r: R, ctx: &mut TxContext) {
let R { id, s } = r;
sui::object::delete(id);
sui::transfer::transfer(s, tx_context::sender(ctx));
}
}

//
// Test sharing
//

//# run test::m::create --sender A

//# run test::m::unwrap_and_transfer --args object(107) --sender A
65 changes: 38 additions & 27 deletions crates/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,7 @@ fn process_successful_execution<
// there is one extra object created with ID hardcoded at 0x5, and it won't be included in
// `newly_generated_ids`. To cope with this, we special check the ID inside `handle_transfer`.
// It's a bit hacky. Ideally, we want `newly_generated_ids` to include it. But it's unclear how.
let newly_generated_ids = ctx.recreate_all_ids();
state_view.set_create_object_ids(newly_generated_ids.clone());
let mut unwrap_and_deleted = BTreeSet::new();
let mut newly_generated_ids = ctx.recreate_all_ids();
let mut frozen_object_ids = BTreeSet::new();
let mut child_count_deltas: BTreeMap<ObjectID, i64> = BTreeMap::new();
let mut newly_generated_deleted = BTreeSet::new();
Expand Down Expand Up @@ -504,7 +502,7 @@ fn process_successful_execution<
state_view,
module_id,
&mut object_owner_map,
&newly_generated_ids,
&mut newly_generated_ids,
&mut newly_generated_unused,
&mut frozen_object_ids,
&mut child_count_deltas,
Expand Down Expand Up @@ -557,26 +555,29 @@ fn process_successful_execution<
ctx.sender(),
*obj_id,
));
let previous_version =
match state_view.get_latest_parent_entry_ref(*obj_id) {
Ok(Some((_, previous_version, _))) => previous_version,
// TODO this error is (hopefully) transient and should not be
// a normal execution error
_ => {
return Err(ExecutionError::new_with_source(
ExecutionErrorKind::InvariantViolation,
missing_unwrapped_msg(obj_id),
));
}
};
unwrap_and_deleted.insert(*obj_id);
changes.insert(
*obj_id,
ObjectChange::Delete(
previous_version,
DeleteKind::UnwrapThenDelete,
),
);
match state_view.get_latest_parent_entry_ref(*obj_id) {
Ok(Some((_, previous_version, _))) => {
changes.insert(
*obj_id,
ObjectChange::Delete(
previous_version,
DeleteKind::UnwrapThenDelete,
),
);
}
// if the object is not in parent sync, it was wrapped before
// ever being stored into storage. Thus we don't need to mark it
// as being deleted
Ok(None) => (),
// TODO this error is (hopefully) transient and should not be
// a normal execution error
Err(_) => {
return Err(ExecutionError::new_with_source(
ExecutionErrorKind::InvariantViolation,
missing_unwrapped_msg(obj_id),
));
}
};
}
}
}
Expand Down Expand Up @@ -678,6 +679,7 @@ 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
Expand Down Expand Up @@ -714,7 +716,7 @@ fn handle_transfer<
state_view: &mut S,
module_id: &ModuleId,
object_owner_map: &mut BTreeMap<SuiAddress, SuiAddress>,
newly_generated_ids: &BTreeSet<ObjectID>,
newly_generated_ids: &mut BTreeSet<ObjectID>,
newly_generated_unused: &mut BTreeSet<ObjectID>,
frozen_object_ids: &mut BTreeSet<ObjectID>,
child_count_deltas: &mut BTreeMap<ObjectID, i64>,
Expand All @@ -729,7 +731,7 @@ fn handle_transfer<
.expect("object contents should start with an id");
newly_generated_unused.remove(&id);
let old_object = by_value_objects.remove(&id);
let is_unwrapped = !(newly_generated_ids.contains(&id) || id == SUI_SYSTEM_STATE_OBJECT_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),
// When an object was wrapped at version `v`, we added an record into `parent_sync`
Expand All @@ -739,7 +741,16 @@ fn handle_transfer<
// 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),
_ => {
// 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)
}
Err(_) => {
// TODO this error is (hopefully) transient and should not be
// a normal execution error
return Err(ExecutionError::new_with_source(
Expand Down

0 comments on commit b98bf4e

Please sign in to comment.