From fe6ae8484d2df34f7a3b94f3f17ac83deb52fa64 Mon Sep 17 00:00:00 2001 From: Mark Logan <103447440+mystenmark@users.noreply.github.com> Date: Tue, 25 Apr 2023 20:41:30 -0700 Subject: [PATCH] Verify accumulator behavior in sui transactional tests (#11205) Unfortunately, the transactional tests don't use AuthorityStore, so we have to duplicate some logic here, but I think this is well worth it for the increased coverage. --- Cargo.lock | 1 + crates/sui-core/src/state_accumulator.rs | 327 +++++++++++------- .../sui-transactional-test-runner/Cargo.toml | 1 + .../src/test_adapter.rs | 33 +- crates/sui-types/src/in_memory_storage.rs | 22 +- 5 files changed, 250 insertions(+), 134 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40c2011cd01fe..c0251888782af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10109,6 +10109,7 @@ dependencies = [ "bcs", "bimap", "clap 3.2.23", + "fastcrypto", "move-binary-format", "move-bytecode-utils", "move-command-line-common", diff --git a/crates/sui-core/src/state_accumulator.rs b/crates/sui-core/src/state_accumulator.rs index 3b4234f9cb343..7e56776e9ffc1 100644 --- a/crates/sui-core/src/state_accumulator.rs +++ b/crates/sui-core/src/state_accumulator.rs @@ -5,10 +5,12 @@ use itertools::Itertools; use mysten_metrics::monitored_scope; use serde::Serialize; use sui_protocol_config::ProtocolConfig; -use sui_types::base_types::{ObjectID, SequenceNumber}; +use sui_types::base_types::{ObjectID, ObjectRef, SequenceNumber, VersionNumber}; use sui_types::committee::EpochId; use sui_types::digests::{ObjectDigest, TransactionDigest}; -use sui_types::storage::ObjectKey; +use sui_types::in_memory_storage::InMemoryStorage; +use sui_types::object::Object; +use sui_types::storage::{ObjectKey, ObjectStore}; use tracing::debug; use typed_store::Map; @@ -30,18 +32,69 @@ pub struct StateAccumulator { authority_store: Arc, } +pub trait AccumulatorReadStore { + fn multi_get_object_by_key(&self, object_keys: &[ObjectKey]) -> SuiResult>>; + + fn get_object_ref_prior_to_key( + &self, + object_id: &ObjectID, + version: VersionNumber, + ) -> SuiResult>; +} + +impl AccumulatorReadStore for AuthorityStore { + fn multi_get_object_by_key(&self, object_keys: &[ObjectKey]) -> SuiResult>> { + self.multi_get_object_by_key(object_keys) + } + + fn get_object_ref_prior_to_key( + &self, + object_id: &ObjectID, + version: VersionNumber, + ) -> SuiResult> { + self.get_object_ref_prior_to_key(object_id, version) + } +} + +impl AccumulatorReadStore for InMemoryStorage { + fn multi_get_object_by_key(&self, object_keys: &[ObjectKey]) -> SuiResult>> { + let mut objects = Vec::new(); + for key in object_keys { + objects.push(self.get_object_by_key(&key.0, key.1)?); + } + Ok(objects) + } + + fn get_object_ref_prior_to_key( + &self, + object_id: &ObjectID, + version: VersionNumber, + ) -> SuiResult> { + Ok(if let Some(wrapped_version) = self.get_wrapped(object_id) { + assert!(wrapped_version < version); + Some(( + *object_id, + wrapped_version, + ObjectDigest::OBJECT_DIGEST_WRAPPED, + )) + } else { + None + }) + } +} + /// Serializable representation of the ObjectRef of an /// object that has been wrapped /// TODO: This can be replaced with ObjectKey. -#[derive(Serialize)] -struct WrappedObject { +#[derive(Serialize, Debug)] +pub struct WrappedObject { id: ObjectID, wrapped_at: SequenceNumber, digest: ObjectDigest, } impl WrappedObject { - fn new(id: ObjectID, wrapped_at: SequenceNumber) -> Self { + pub fn new(id: ObjectID, wrapped_at: SequenceNumber) -> Self { Self { id, wrapped_at, @@ -50,6 +103,142 @@ impl WrappedObject { } } +pub fn accumulate_effects( + store: S, + effects: Vec, + protocol_config: &ProtocolConfig, +) -> Accumulator +where + S: std::ops::Deref, + T: AccumulatorReadStore, +{ + let mut acc = Accumulator::default(); + + // process insertions to the set + acc.insert_all( + effects + .iter() + .flat_map(|fx| { + fx.created() + .iter() + .map(|(oref, _)| oref.2) + .chain(fx.unwrapped().iter().map(|(oref, _)| oref.2)) + .chain(fx.mutated().iter().map(|(oref, _)| oref.2)) + }) + .collect::>(), + ); + + // insert wrapped tombstones. We use a custom struct in order to contain the tombstone + // against the object id and sequence number, as the tombstone by itself is not unique. + acc.insert_all( + effects + .iter() + .flat_map(|fx| { + fx.wrapped() + .iter() + .map(|oref| { + bcs::to_bytes(&WrappedObject::new(oref.0, oref.1)) + .unwrap() + .to_vec() + }) + .collect::>>() + }) + .collect::>>(), + ); + + let all_unwrapped = effects + .iter() + .flat_map(|fx| { + fx.unwrapped() + .iter() + .map(|(oref, _owner)| (*fx.transaction_digest(), oref.0, oref.1)) + }) + .chain(effects.iter().flat_map(|fx| { + fx.unwrapped_then_deleted() + .iter() + .map(|oref| (*fx.transaction_digest(), oref.0, oref.1)) + })) + .collect::>(); + + let unwrapped_ids: HashMap> = all_unwrapped + .iter() + .map(|(digest, id, _)| (*digest, *id)) + .into_group_map() + .iter() + .map(|(digest, ids)| (*digest, HashSet::from_iter(ids.iter().cloned()))) + .collect(); + + // Collect keys from modified_at_versions to remove from the accumulator. + // Filter all unwrapped objects (from unwrapped or unwrapped_then_deleted effects) + // as these were inserted into the accumulator as a WrappedObject. Will handle these + // separately. + let modified_at_version_keys: Vec = effects + .iter() + .flat_map(|fx| { + fx.modified_at_versions() + .iter() + .map(|(id, seq_num)| (*fx.transaction_digest(), *id, *seq_num)) + }) + .filter_map(|(tx_digest, id, seq_num)| { + // unwrapped tx + if let Some(ids) = unwrapped_ids.get(&tx_digest) { + // object unwrapped in this tx. We handle it later + if ids.contains(&id) { + return None; + } + } + Some(ObjectKey(id, seq_num)) + }) + .collect(); + + let modified_at_digests: Vec<_> = store + .multi_get_object_by_key(&modified_at_version_keys.clone()) + .expect("Failed to get modified_at_versions object from object table") + .into_iter() + .zip(modified_at_version_keys) + .map(|(obj, key)| { + obj.unwrap_or_else(|| panic!("Object for key {:?} from modified_at_versions effects does not exist in objects table", key)) + .compute_object_reference() + .2 + }) + .collect(); + acc.remove_all(modified_at_digests); + + // Process unwrapped and unwrapped_then_deleted effects, which need to be + // removed as WrappedObject using the last sequence number it was tombstoned + // against. Since this happened in a past transaction, and the child object may + // have been modified since (and hence its sequence number incremented), we + // seek the version prior to the unwrapped version from the objects table directly. + // If the tombstone is not found, then assume this is a newly created wrapped object hence + // we don't expect to find it in the table. + let wrapped_objects_to_remove: Vec = all_unwrapped + .iter() + .filter_map(|(_tx_digest, id, seq_num)| { + let objref = store + .get_object_ref_prior_to_key(id, *seq_num) + .expect("read cannot fail"); + + objref.map(|(id, version, digest)| { + assert!( + !protocol_config.loaded_child_objects_fixed() || digest.is_wrapped(), + "{:?}", + id + ); + WrappedObject::new(id, version) + }) + }) + .collect(); + + acc.remove_all( + wrapped_objects_to_remove + .iter() + .map(|wrapped| bcs::to_bytes(wrapped).unwrap().to_vec()) + .collect::>>(), + ); + + acc +} + impl StateAccumulator { pub fn new(authority_store: Arc) -> Self { Self { authority_store } @@ -85,133 +274,7 @@ impl StateAccumulator { effects: Vec, protocol_config: &ProtocolConfig, ) -> Accumulator { - let mut acc = Accumulator::default(); - - // process insertions to the set - acc.insert_all( - effects - .iter() - .flat_map(|fx| { - fx.created() - .iter() - .map(|(oref, _)| oref.2) - .chain(fx.unwrapped().iter().map(|(oref, _)| oref.2)) - .chain(fx.mutated().iter().map(|(oref, _)| oref.2)) - }) - .collect::>(), - ); - - // insert wrapped tombstones. We use a custom struct in order to contain the tombstone - // against the object id and sequence number, as the tombstone by itself is not unique. - acc.insert_all( - effects - .iter() - .flat_map(|fx| { - fx.wrapped() - .iter() - .map(|oref| { - bcs::to_bytes(&WrappedObject::new(oref.0, oref.1)) - .unwrap() - .to_vec() - }) - .collect::>>() - }) - .collect::>>(), - ); - - let all_unwrapped = effects - .iter() - .flat_map(|fx| { - fx.unwrapped() - .iter() - .map(|(oref, _owner)| (*fx.transaction_digest(), oref.0, oref.1)) - }) - .chain(effects.iter().flat_map(|fx| { - fx.unwrapped_then_deleted() - .iter() - .map(|oref| (*fx.transaction_digest(), oref.0, oref.1)) - })) - .collect::>(); - - let unwrapped_ids: HashMap> = all_unwrapped - .iter() - .map(|(digest, id, _)| (*digest, *id)) - .into_group_map() - .iter() - .map(|(digest, ids)| (*digest, HashSet::from_iter(ids.iter().cloned()))) - .collect(); - - // Collect keys from modified_at_versions to remove from the accumulator. - // Filter all unwrapped objects (from unwrapped or unwrapped_then_deleted effects) - // as these were inserted into the accumulator as a WrappedObject. Will handle these - // separately. - let modified_at_version_keys: Vec = effects - .iter() - .flat_map(|fx| { - fx.modified_at_versions() - .iter() - .map(|(id, seq_num)| (*fx.transaction_digest(), *id, *seq_num)) - }) - .filter_map(|(tx_digest, id, seq_num)| { - // unwrapped tx - if let Some(ids) = unwrapped_ids.get(&tx_digest) { - // object unwrapped in this tx. We handle it later - if ids.contains(&id) { - return None; - } - } - Some(ObjectKey(id, seq_num)) - }) - .collect(); - - let modified_at_digests: Vec<_> = self - .authority_store - .multi_get_object_by_key(&modified_at_version_keys.clone()) - .expect("Failed to get modified_at_versions object from object table") - .into_iter() - .zip(modified_at_version_keys) - .map(|(obj, key)| { - obj.unwrap_or_else(|| panic!("Object for key {:?} from modified_at_versions effects does not exist in objects table", key)) - .compute_object_reference() - .2 - }) - .collect(); - acc.remove_all(modified_at_digests); - - // Process unwrapped and unwrapped_then_deleted effects, which need to be - // removed as WrappedObject using the last sequence number it was tombstoned - // against. Since this happened in a past transaction, and the child object may - // have been modified since (and hence its sequence number incremented), we - // seek the version prior to the unwrapped version from the objects table directly. - // If the tombstone is not found, then assume this is a newly created wrapped object hence - // we don't expect to find it in the table. - let wrapped_objects_to_remove: Vec = all_unwrapped - .iter() - .filter_map(|(_tx_digest, id, seq_num)| { - let objref = self - .authority_store - .get_object_ref_prior_to_key(id, *seq_num) - .expect("read cannot fail"); - - objref.map(|(id, version, digest)| { - assert!( - !protocol_config.loaded_child_objects_fixed() || digest.is_wrapped(), - "{:?}", - id - ); - WrappedObject::new(id, version) - }) - }) - .collect(); - - acc.remove_all( - wrapped_objects_to_remove - .iter() - .map(|wrapped| bcs::to_bytes(wrapped).unwrap().to_vec()) - .collect::>>(), - ); - - acc + accumulate_effects(&*self.authority_store, effects, protocol_config) } /// Unions all checkpoint accumulators at the end of the epoch to generate the diff --git a/crates/sui-transactional-test-runner/Cargo.toml b/crates/sui-transactional-test-runner/Cargo.toml index e047f2ec82a92..38066bc9311df 100644 --- a/crates/sui-transactional-test-runner/Cargo.toml +++ b/crates/sui-transactional-test-runner/Cargo.toml @@ -16,6 +16,7 @@ once_cell = "1.16" rand = "0.8.5" prometheus = "0.13.3" +fastcrypto.workspace = true move-binary-format.workspace = true move-bytecode-utils.workspace = true move-command-line-common.workspace = true diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index ae05fc710f740..fa2be11fafc2e 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -6,6 +6,7 @@ use crate::{args::*, programmable_transaction_test_parser::parser::ParsedCommand}; use anyhow::bail; use bimap::btree::BiBTreeMap; +use fastcrypto::hash::MultisetHash; use move_binary_format::{file_format::CompiledScript, CompiledModule}; use move_bytecode_utils::module_cache::GetModule; use move_command_line_common::{ @@ -39,10 +40,14 @@ use std::{ }; use sui_adapter::execution_engine; use sui_adapter::{adapter::new_move_vm, execution_mode}; -use sui_core::transaction_input_checker::check_objects; +use sui_core::{ + state_accumulator::{accumulate_effects, WrappedObject}, + transaction_input_checker::check_objects, +}; use sui_framework::BuiltInFramework; use sui_framework::DEFAULT_FRAMEWORK_PATH; use sui_protocol_config::ProtocolConfig; +use sui_types::accumulator::Accumulator; use sui_types::MOVE_STDLIB_OBJECT_ID; use sui_types::{ base_types::{ObjectID, ObjectRef, SuiAddress, TransactionDigest, SUI_ADDRESS_LENGTH}, @@ -761,6 +766,21 @@ fn merge_output(left: Option, right: Option) -> Option { } } +fn accumulate_in_memory_store(store: &InMemoryStorage) -> Accumulator { + let mut acc = Accumulator::default(); + for (_, obj) in store.objects().iter() { + acc.insert(obj.compute_object_reference().2); + } + + for (id, version) in store.wrapped().iter() { + acc.insert( + bcs::to_bytes(&WrappedObject::new(*id, *version)) + .expect("Failed to serialize WrappedObject"), + ); + } + acc +} + impl<'a> SuiTestAdapter<'a> { fn upgrade_package( &mut self, @@ -1009,11 +1029,22 @@ impl<'a> SuiTestAdapter<'a> { .collect(); let mut wrapped_ids: Vec<_> = effects.wrapped().iter().map(|(id, _, _)| *id).collect(); let gas_summary = effects.gas_cost_summary(); + + let effects_accum = + accumulate_effects(&*self.storage, vec![effects.clone()], &self.protocol_config); + + let mut before = accumulate_in_memory_store(&self.storage); + // update storage Arc::get_mut(&mut self.storage) .unwrap() .finish(inner.written, inner.deleted); + let after = accumulate_in_memory_store(&self.storage); + + before.union(&effects_accum); + assert_eq!(before.digest(), after.digest()); + // make sure objects that have previously not been in storage get assigned a fake id. let mut might_need_fake_id: Vec<_> = created_ids .iter() diff --git a/crates/sui-types/src/in_memory_storage.rs b/crates/sui-types/src/in_memory_storage.rs index a8add988daf74..6678c9603967f 100644 --- a/crates/sui-types/src/in_memory_storage.rs +++ b/crates/sui-types/src/in_memory_storage.rs @@ -22,6 +22,7 @@ use std::collections::BTreeMap; pub struct InMemoryStorage { persistent: BTreeMap, last_entry_for_deleted: BTreeMap, + wrapped: BTreeMap, } impl BackingPackageStore for InMemoryStorage { @@ -146,6 +147,7 @@ impl InMemoryStorage { Self { persistent, last_entry_for_deleted: BTreeMap::new(), + wrapped: BTreeMap::new(), } } @@ -164,6 +166,7 @@ impl InMemoryStorage { pub fn insert_object(&mut self, object: Object) { let id = object.id(); self.last_entry_for_deleted.remove(&id); + self.wrapped.remove(&id); self.persistent.insert(id, object); } @@ -171,6 +174,14 @@ impl InMemoryStorage { &self.persistent } + pub fn wrapped(&self) -> &BTreeMap { + &self.wrapped + } + + pub fn get_wrapped(&self, id: &ObjectID) -> Option { + self.wrapped.get(id).copied() + } + pub fn into_inner(self) -> BTreeMap { self.persistent } @@ -185,11 +196,20 @@ impl InMemoryStorage { debug_assert!(new_object.id() == _id); self.insert_object(new_object); } - for (id, _) in deleted { + for (id, (ver, kind)) in deleted { if let Some(obj) = self.persistent.remove(&id) { self.last_entry_for_deleted .insert(id, obj.compute_object_reference()); } + match kind { + DeleteKind::Wrap => { + self.wrapped.insert(id, ver); + } + DeleteKind::UnwrapThenDelete => { + self.wrapped.remove(&id); + } + _ => (), + } } } }