Skip to content

Commit

Permalink
Cleanup handling of delete id edge case (MystenLabs#752)
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind authored Mar 11, 2022
1 parent 4b74271 commit 19ddffd
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 26 deletions.
8 changes: 0 additions & 8 deletions sui_core/src/authority/temporary_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,6 @@ impl AuthorityTemporaryStore {
entry.0 = entry.0.increment();
}
}
// self.deleted contains all object IDs that were passed through ID::delete_id.
// However that doesn't necessarily indicate an object was deleted, if the object
// didn't show up in the input. There are two cases, one is that the object just got
// unwrapped, and another is just deletion of an ID that doesn't belong to a previous
// existing object. The second case can be filtered out.
self.deleted.retain(|id, (_version, kind)| {
kind != &DeleteKind::NotExistInInput || unwrapped_object_ids.contains(id)
});
}

pub fn to_signed_effects(
Expand Down
27 changes: 12 additions & 15 deletions sui_programmability/adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ fn process_successful_execution<
let tx_digest = ctx.digest();
let mut deleted_ids = HashMap::new();
let mut deleted_child_ids = HashSet::new();
// newly_generated_ids contains all object IDs generated in this transaction.
let newly_generated_ids = ctx.recreate_all_ids();
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 @@ -562,7 +564,11 @@ fn process_successful_execution<
// unwrap safe because this event can only be emitted from processing
// native call delete_id, which guarantees the type of the id.
let id: VersionedID = bcs::from_bytes(&event_bytes).unwrap();
deleted_ids.insert(*id.object_id(), id.version());
// We don't care about IDs that are generated in this same transaction
// but only to be deleted.
if !newly_generated_ids.contains(id.object_id()) {
deleted_ids.insert(*id.object_id(), id.version());
}
Ok(())
}
EventType::ShareObject => Err(SuiError::UnsupportedSharedObjectError),
Expand Down Expand Up @@ -609,28 +615,19 @@ fn process_successful_execution<
return (gas_used, 0, Err(SuiError::DeleteObjectOwnedObject));
}
if deleted_ids.contains_key(id) {
state_view.delete_object(id, object.version(), DeleteKind::ExistInInput);
state_view.delete_object(id, object.version(), DeleteKind::Normal);
} else {
state_view.delete_object(id, object.version(), DeleteKind::Wrap);
}

gas_refund += gas::calculate_object_deletion_refund(object);
}
// The loop above may not cover all ids in deleted_ids, i.e. some of the deleted_ids
// may not show up in by_value_objects.
// This can happen for two reasons:
// 1. The call to ID::delete_id() was not a result of deleting a pre-existing object.
// This can happen either because we were deleting an object that just got created
// in this same transaction; or we have an ID that's created but not associated with
// a real object. In either case, we don't care about this id.
// 2. This object was wrapped in the past, and now is getting deleted. It won't show up
// in the input, but the deletion is also real.
// We cannot distinguish the above two cases here just yet. So we just add it with
// the kind NotExistInInput. They will be eventually filtered out in
// [`AuthorityTemporaryStore::patch_unwrapped_objects`].

// Any id that's not covered in the above loop, i.e. not in input but also deleted,
// must be unwrapped and then deleted.
for (id, version) in deleted_ids {
if !by_value_objects.contains_key(&id) {
state_view.delete_object(&id, version, DeleteKind::NotExistInInput);
state_view.delete_object(&id, version, DeleteKind::UnwrapThenDelete);
}
}

Expand Down
10 changes: 7 additions & 3 deletions sui_types/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ use crate::{

#[derive(Debug, PartialEq)]
pub enum DeleteKind {
ExistInInput,
NotExistInInput,
/// An object is provided in the call input, and gets deleted.
Normal,
/// An object is not provided in the call input, but gets unwrapped
/// from another object, and then gets deleted.
UnwrapThenDelete,
/// An object is provided in the call input, and gets wrapped into another object.
Wrap,
}

Expand All @@ -22,7 +26,7 @@ pub trait Storage {

fn write_object(&mut self, object: Object);

/// Record an event that happened during execution
/// Record an event that happened during execution
fn log_event(&mut self, event: Event);

fn delete_object(&mut self, id: &ObjectID, version: SequenceNumber, kind: DeleteKind);
Expand Down

0 comments on commit 19ddffd

Please sign in to comment.