Skip to content

Commit

Permalink
[1/x][lamport] Get rid of TransactionEffects::is_object_mutated_here (M…
Browse files Browse the repository at this point in the history
…ystenLabs#5851)

Spotted while working on lamport timestamps.  This function is only
called in one place (`AuthorityAggregator::get_object_info_execute`)
to check that a validator that object info is being read from isn't
Byzantine, but it is only used in Gateway, and benchmarks.  The former
is going away soon, and the latter does not need to worry about
byzantine validators.

Furthermore, the logic in this function for deleted and wrapped
objects does not seem correct:  It asserts that an object is deleted
in effects if it appears in the deleted effects at `version - 1`,
but the logic in `TemporaryStore::delete_object` increments an
object's version when it is deleted, so it should match.

Opting to remove the code to re-execute certificates instead of trying
to make it work with lamport timestamps as there's a good chance it's
not currently working and therefore needed.
  • Loading branch information
amnn authored Nov 10, 2022
1 parent 23533a2 commit 1282fc6
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 54 deletions.
30 changes: 4 additions & 26 deletions crates/sui-core/src/authority_aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1959,42 +1959,20 @@ where
}

pub async fn get_object_info_execute(&self, object_id: ObjectID) -> SuiResult<ObjectRead> {
let (object_map, cert_map) = self.get_object_by_id(object_id, false).await?;
let (object_map, _cert_map) = self.get_object_by_id(object_id, false).await?;
let mut object_ref_stack: Vec<_> = object_map.into_iter().collect();

while let Some(((obj_ref, tx_digest), (obj_option, layout_option, authorities))) =
while let Some(((obj_ref, _tx_digest), (obj_option, layout_option, authorities))) =
object_ref_stack.pop()
{
let stake: StakeUnit = authorities
.iter()
.map(|(name, _)| self.committee.weight(name))
.sum();

let mut is_ok = false;
// If we have f+1 stake telling us of the latest version of the object, we just accept
// it.
if stake >= self.committee.validity_threshold() {
// If we have f+1 stake telling us of the latest version of the object, we just accept it.
is_ok = true;
} else if cert_map.contains_key(&tx_digest) {
// If we have less stake telling us about the latest state of an object
// we re-run the certificate on all authorities to ensure it is correct.
if let Ok(effects) = self
.process_certificate(cert_map[&tx_digest].clone().into())
.await
{
if effects.effects.is_object_mutated_here(obj_ref) {
is_ok = true;
} else {
// TODO: Throw a byzantine fault here
error!(
?object_id,
?tx_digest,
"get_object_info_execute. Byzantine failure!"
);
continue;
}
}
}
if is_ok {
match obj_option {
Some(obj) => {
return Ok(ObjectRead::Exists(obj_ref, obj, layout_option));
Expand Down
28 changes: 0 additions & 28 deletions crates/sui-types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1675,34 +1675,6 @@ impl TransactionEffects {
&self.gas_used
}

pub fn is_object_mutated_here(&self, obj_ref: ObjectRef) -> bool {
// The mutated or created case
if self.all_mutated().any(|(oref, _, _)| *oref == obj_ref) {
return true;
}

// The deleted case
if obj_ref.2 == ObjectDigest::OBJECT_DIGEST_DELETED
&& self
.deleted
.iter()
.any(|(id, seq, _)| *id == obj_ref.0 && seq.increment() == obj_ref.1)
{
return true;
}

// The wrapped case
if obj_ref.2 == ObjectDigest::OBJECT_DIGEST_WRAPPED
&& self
.wrapped
.iter()
.any(|(id, seq, _)| *id == obj_ref.0 && seq.increment() == obj_ref.1)
{
return true;
}
false
}

pub fn to_sign_effects(
self,
epoch: EpochId,
Expand Down

0 comments on commit 1282fc6

Please sign in to comment.