sui icon indicating copy to clipboard operation
sui copied to clipboard

[1/x][lamport] Get rid of TransactionEffects::is_object_mutated_here

Open amnn opened this issue 2 years ago • 3 comments

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.

Test Plan

$ cargo simtest
$ cargo nextest run

Stacked on #5748

amnn avatar Nov 04 '22 17:11 amnn

@lxfind and I chatted about what is_object_mutated_here is being used for, and we think it may no longer be working correctly (and the code it's used in is also not long for this world).

Note that it's just the top commit that needs a review here, the rest are part of #5748.

amnn avatar Nov 04 '22 17:11 amnn

@amnn is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Nov 09 '22 12:11 vercel[bot]

Bringing forward the removal of is_object_mutated_here while I sort out the rest of lamport timestamps. @lxfind, could you take another look when you get a chance?

amnn avatar Nov 09 '22 12:11 amnn