Skip to content

Update transaction input loader logic for ConsensusV2 objects #21634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2025
Merged

Update transaction input loader logic for ConsensusV2 objects #21634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 42 additions & 33 deletions crates/sui-core/src/transaction_input_loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,30 +62,37 @@ impl TransactionInputLoader {
object: ObjectReadResultKind::Object(package),
});
}
InputObjectKind::SharedMoveObject {
id,
initial_shared_version,
..
} => match self.cache.get_object(id) {
Some(object) => {
input_results[i] = Some(ObjectReadResult::new(*kind, object.into()))
}
None => {
if let Some((version, digest)) =
self.cache.get_last_shared_object_deletion_info(
FullObjectID::new(*id, Some(*initial_shared_version)),
epoch_id,
)
{
input_results[i] = Some(ObjectReadResult {
input_object_kind: *kind,
object: ObjectReadResultKind::DeletedSharedObject(version, digest),
});
} else {
return Err(SuiError::from(kind.object_not_found_error()));
InputObjectKind::SharedMoveObject { .. } => {
let input_full_id = kind.full_object_id();

// Load the most current version from the cache.
match self.cache.get_object(&kind.object_id()) {
// If full ID matches, we're done.
// (Full ID may not match if object was transferred in or out of
// consensus. We have to double-check this because cache is keyed
// on ObjectID and not FullObjectID.)
Some(object) if object.full_id() == input_full_id => {
input_results[i] = Some(ObjectReadResult::new(*kind, object.into()))
}
_ => {
// If the full ID doesn't match, check if the object was marked
// as unavailable.
if let Some((version, digest)) = self
.cache
.get_last_shared_object_deletion_info(input_full_id, epoch_id)
{
input_results[i] = Some(ObjectReadResult {
input_object_kind: *kind,
object: ObjectReadResultKind::DeletedSharedObject(
version, digest,
),
});
} else {
return Err(SuiError::from(kind.object_not_found_error()));
}
}
}
},
}
InputObjectKind::ImmOrOwnedMoveObject(objref) => {
object_refs.push(*objref);
fetch_indices.push(i);
Expand Down Expand Up @@ -185,8 +192,8 @@ impl TransactionInputLoader {
);
});

// If we find a set of assigned versions but an object is missing, it indicates
// a serious inconsistency:
// If we find a set of assigned versions but one object's version assignments
// are missing from the set, it indicates a serious inconsistency:
let version = assigned_shared_versions.get(&(*id, *initial_shared_version)).unwrap_or_else(|| {
panic!("Shared object version should have been assigned. key: {tx_key:?}, obj id: {id:?}")
});
Expand Down Expand Up @@ -216,29 +223,31 @@ impl TransactionInputLoader {
fetches.into_iter()
) {
results[index] = Some(match (object, input) {
(Some(obj), input_object_kind) => ObjectReadResult {
input_object_kind: *input_object_kind,
(Some(obj), InputObjectKind::SharedMoveObject { .. }) if obj.full_id() == input.full_object_id() => ObjectReadResult {
input_object_kind: *input,
object: obj.into(),
},
(None, InputObjectKind::SharedMoveObject { id, initial_shared_version, .. }) => {
(_, InputObjectKind::SharedMoveObject { .. }) => {
assert!(key.1.is_valid());
// Check if the object was deleted by a concurrently certified tx
// If the full ID on a shared input doesn't match, check if the object was
// marked as unavailable by a concurrently certified tx.
let version = key.1;
if let Some(dependency) = self.cache.get_deleted_shared_object_previous_tx_digest(
FullObjectKey::new(
FullObjectID::new(*id, Some(*initial_shared_version)),
version,
),
FullObjectKey::new(input.full_object_id(), version),
epoch_id,
) {
ObjectReadResult {
input_object_kind: *input,
object: ObjectReadResultKind::DeletedSharedObject(version, dependency),
}
} else {
panic!("All dependencies of tx {tx_key:?} should have been executed now, but Shared Object id: {}, version: {version} is absent in epoch {epoch_id}", *id);
panic!("All dependencies of tx {tx_key:?} should have been executed now, but Shared Object id: {:?}, version: {version} is absent in epoch {epoch_id}", input.full_object_id());
}
},
(Some(obj), input_object_kind) => ObjectReadResult {
input_object_kind: *input_object_kind,
object: obj.into(),
},
_ => panic!("All dependencies of tx {tx_key:?} should have been executed now, but obj {key:?} is absent"),
});
}
Expand Down
15 changes: 14 additions & 1 deletion crates/sui-types/src/base_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,21 @@ pub fn update_object_ref_for_testing(object_ref: ObjectRef) -> ObjectRef {
)
}

pub type FullObjectRef = (FullObjectID, SequenceNumber, ObjectDigest);
pub struct FullObjectRef(pub FullObjectID, pub SequenceNumber, pub ObjectDigest);

impl FullObjectRef {
pub fn from_fastpath_ref(object_ref: ObjectRef) -> Self {
Self(
FullObjectID::Fastpath(object_ref.0),
object_ref.1,
object_ref.2,
)
}

pub fn as_object_ref(&self) -> ObjectRef {
(self.0.id(), self.1, self.2)
}
}
/// Represents an distinct stream of object versions for a Shared or ConsensusV2 object,
/// based on the object ID and start version.
pub type ConsensusObjectSequenceKey = (ObjectID, SequenceNumber);
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-types/src/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ impl ObjectInner {
}

pub fn compute_full_object_reference(&self) -> FullObjectRef {
(self.full_id(), self.version(), self.digest())
FullObjectRef(self.full_id(), self.version(), self.digest())
}

pub fn digest(&self) -> ObjectDigest {
Expand Down
8 changes: 3 additions & 5 deletions crates/sui-types/src/programmable_transaction_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,9 @@ impl ProgrammableTransactionBuilder {
) -> anyhow::Result<()> {
let rec_arg = self.pure(recipient).unwrap();
let obj_arg = self.obj(match full_object_ref.0 {
FullObjectID::Fastpath(_) => ObjectArg::ImmOrOwnedObject((
full_object_ref.0.id(),
full_object_ref.1,
full_object_ref.2,
)),
FullObjectID::Fastpath(_) => {
ObjectArg::ImmOrOwnedObject(full_object_ref.as_object_ref())
}
FullObjectID::Consensus((id, initial_shared_version)) => ObjectArg::SharedObject {
id,
initial_shared_version,
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-types/src/storage/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ pub enum MarkerValue {
/// An owned object was deleted (or wrapped) at the given version, and is no longer able to be
/// accessed or used in subsequent transactions.
OwnedDeleted,
/// A shared object was deleted by the transaction and is no longer able to be accessed or
/// used in subsequent transactions.
/// A shared object was deleted or removed from consensus by the transaction and is no longer
/// able to be accessed or used in subsequent transactions.
// TODO: Rename this to something more accurate, like "ConsensusUnavailable".
SharedDeleted(TransactionDigest),
}

Expand Down
15 changes: 12 additions & 3 deletions crates/sui-types/src/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2978,10 +2978,18 @@ pub enum InputObjectKind {

impl InputObjectKind {
pub fn object_id(&self) -> ObjectID {
self.full_object_id().id()
}

pub fn full_object_id(&self) -> FullObjectID {
match self {
Self::MovePackage(id) => *id,
Self::ImmOrOwnedMoveObject((id, _, _)) => *id,
Self::SharedMoveObject { id, .. } => *id,
Self::MovePackage(id) => FullObjectID::Fastpath(*id),
Self::ImmOrOwnedMoveObject((id, _, _)) => FullObjectID::Fastpath(*id),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think it would be better to call it QualifiedObjectID instead of FullObjectID, because this stuff isn't really part of the ID itself, its a function of how we are referring to it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed on chat, but response for posterity

The prefix Full* as a term used to refer to the complete object identifier inclusive of both the address and the start_version for consensus objects was introduced a couple months ago already in #20822

Fair enough if you want to reopen the discussion on that naming choice, but it would be a broad rename across like 20 files that is not really related to this particular PR

Self::SharedMoveObject {
id,
initial_shared_version,
..
} => FullObjectID::Consensus((*id, *initial_shared_version)),
}
}

Expand Down Expand Up @@ -3035,6 +3043,7 @@ pub enum ObjectReadResultKind {
Object(Object),
// The version of the object that the transaction intended to read, and the digest of the tx
// that deleted it.
// TODO: Rename this to something more accurate, like "UnavailableConsensusObject".
DeletedSharedObject(SequenceNumber, TransactionDigest),
// A shared object in a cancelled transaction. The sequence number embeds cancellation reason.
CancelledTransactionSharedObject(SequenceNumber),
Expand Down
Loading