Skip to content

Commit

Permalink
[adapter] Ignore deletions for objects that are always wrapped (#7174)
Browse files Browse the repository at this point in the history
If an object has always existed in a wrapped state and is eventually
deleted, don't included it among the deleted (and therefore modified)
objects in the effects for the transaction that deletes its ID.

## Test Plan

Updated an existing unit test to confirm the values for
`modified_at_versions` after deleting an object that was always wrapped
  • Loading branch information
amnn authored Jan 5, 2023
1 parent f7ac229 commit e5739c3
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ written: object(106)

task 3 'run'. lines 43-43:
written: object(108)
deleted: object(_), object(107)
deleted: object(107)
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ written: object(111)

task 6 'run'. lines 52-52:
written: object(112), object(114)
deleted: object(_), object(113)
deleted: object(113)

task 7 'run'. lines 55-55:
created: object(116), object(117)
Expand Down
7 changes: 3 additions & 4 deletions crates/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,10 +628,9 @@ fn process_successful_execution<S: Storage + ParentSync>(
None => match state_view.get_latest_parent_entry_ref(id) {
Ok(Some((_, previous_version, _))) => previous_version,
Ok(None) => {
// TODO we don't really need to mark this as deleted, as the object was not
// created this txn but has never existed in storage. We just need a better
// way of detecting this rather than relying on the parent sync
SequenceNumber::new()
// This object was not created this transaction but has never existed in
// storage, skip it.
continue;
}
_ => {
return Err(ExecutionError::new_with_source(
Expand Down
34 changes: 30 additions & 4 deletions crates/sui-core/src/unit_tests/move_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sui_types::{
messages::ExecutionStatus,
};

use std::path::PathBuf;
use std::{collections::HashSet, path::PathBuf};
use std::{env, str::FromStr};

const MAX_GAS: u64 = 10000;
Expand Down Expand Up @@ -540,19 +540,30 @@ async fn test_create_then_delete_parent_child_wrap() {
.await
.unwrap();
assert!(effects.status.is_ok());
// Modifies the gas object
assert_eq!(effects.mutated.len(), 1);
// Creates 3 objects, the parent, a field, and the child
assert_eq!(effects.created.len(), 2);
// not wrapped as it wasn't first created
assert_eq!(effects.wrapped.len(), 0);
assert_eq!(effects.events.len(), 3);

let gas_ref = effects.mutated[0].0;

let parent = effects
.created
.iter()
.find(|(_, owner)| matches!(owner, Owner::AddressOwner(_)))
.unwrap()
.0;

let field = effects
.created
.iter()
.find(|((id, _, _), _)| id != &parent.0)
.unwrap()
.0;

// Delete the parent and child altogether.
let effects = call_move(
&authority,
Expand All @@ -568,9 +579,24 @@ async fn test_create_then_delete_parent_child_wrap() {
.await
.unwrap();
assert!(effects.status.is_ok());
// Check that both objects were deleted.
assert_eq!(effects.deleted.len(), 3);
assert_eq!(effects.events.len(), 4);

// The parent and field are considered deleted, the child doesn't count because it wasn't
// considered created in the first place.
assert_eq!(effects.deleted.len(), 2);
assert_eq!(effects.events.len(), 3);

assert_eq!(
effects
.modified_at_versions
.iter()
.cloned()
.collect::<HashSet<_>>(),
HashSet::from([
(gas_ref.0, gas_ref.1),
(parent.0, parent.1),
(field.0, field.1)
]),
);
}

#[tokio::test]
Expand Down

0 comments on commit e5739c3

Please sign in to comment.