diff --git a/crates/sui-core/src/authority.rs b/crates/sui-core/src/authority.rs index 8f7edeeeb7605..2c0a397aa4d9a 100644 --- a/crates/sui-core/src/authority.rs +++ b/crates/sui-core/src/authority.rs @@ -15,7 +15,6 @@ use crate::{ }; use arc_swap::ArcSwap; use async_trait::async_trait; -use itertools::Itertools; use move_binary_format::CompiledModule; use move_bytecode_utils::module_cache::SyncModuleCache; use move_core_types::{ @@ -299,14 +298,14 @@ impl AuthorityState { return Err(SuiError::ValidatorHaltedAtEpochEnd); } - let (_gas_status, all_objects) = transaction_input_checker::check_transaction_input( + let (_gas_status, input_objects) = transaction_input_checker::check_transaction_input( &self.database, &transaction, &self.metrics.shared_obj_tx, ) .await?; - let owned_objects = transaction_input_checker::filter_owned_objects(&all_objects); + let owned_objects = input_objects.filter_owned_objects(); let signed_transaction = SignedTransaction::new( self.committee.load().epoch, @@ -461,7 +460,7 @@ impl AuthorityState { let certificate = confirmation_transaction.certificate; let transaction_digest = *certificate.digest(); - let (gas_status, objects_by_kind) = transaction_input_checker::check_transaction_input( + let (gas_status, input_objects) = transaction_input_checker::check_transaction_input( &self.database, &certificate, &self.metrics.shared_obj_tx, @@ -470,12 +469,7 @@ impl AuthorityState { // At this point we need to check if any shared objects need locks, // and whether they have them. - let shared_object_refs: Vec<_> = objects_by_kind - .iter() - .filter(|(kind, _)| matches!(kind, InputObjectKind::SharedMoveObject(_))) - .map(|(_, obj)| obj.compute_object_reference()) - .sorted() - .collect(); + let shared_object_refs = input_objects.filter_shared_objects(); if !shared_object_refs.is_empty() && !certificate.data.kind.is_system_tx() { // If the transaction contains shared objects, we need to ensure they have been scheduled // for processing by the consensus protocol. @@ -488,7 +482,7 @@ impl AuthorityState { self.metrics .num_input_objs - .observe(objects_by_kind.len() as f64); + .observe(input_objects.len() as f64); self.metrics .num_shared_objects .observe(shared_object_refs.len() as f64); @@ -496,19 +490,13 @@ impl AuthorityState { .batch_size .observe(certificate.data.kind.batch_size() as f64); debug!( - num_inputs = objects_by_kind.len(), + num_inputs = input_objects.len(), "Read inputs for transaction from DB" ); - let transaction_dependencies = objects_by_kind - .iter() - .map(|(_, obj)| obj.previous_transaction) - .collect(); - let mut temporary_store = AuthorityTemporaryStore::new( - self.database.clone(), - objects_by_kind, - transaction_digest, - ); + let transaction_dependencies = input_objects.transaction_dependencies(); + let mut temporary_store = + AuthorityTemporaryStore::new(self.database.clone(), input_objects, transaction_digest); let effects = execution_engine::execute_transaction_to_effects( shared_object_refs, &mut temporary_store, diff --git a/crates/sui-core/src/authority/authority_store.rs b/crates/sui-core/src/authority/authority_store.rs index b35befb4f68c4..4ddc455cf937f 100644 --- a/crates/sui-core/src/authority/authority_store.rs +++ b/crates/sui-core/src/authority/authority_store.rs @@ -3,6 +3,7 @@ use super::*; use crate::epoch::EpochInfoLocals; use crate::gateway_state::GatewayTxSeqNumber; +use crate::transaction_input_checker::InputObjects; use narwhal_executor::ExecutionIndices; use rocksdb::Options; use serde::{Deserialize, Serialize}; @@ -591,7 +592,7 @@ impl Deserialize<'de>> /// need to recreate the temporary store based on the inputs and effects to update it properly. pub async fn update_gateway_state( &self, - active_inputs: Vec<(InputObjectKind, Object)>, + input_objects: InputObjects, mutated_objects: HashMap, certificate: CertifiedTransaction, effects: TransactionEffectsEnvelope, @@ -599,7 +600,7 @@ impl Deserialize<'de>> ) -> SuiResult { let transaction_digest = certificate.digest(); let mut temporary_store = - AuthorityTemporaryStore::new(Arc::new(&self), active_inputs, *transaction_digest); + AuthorityTemporaryStore::new(Arc::new(&self), input_objects, *transaction_digest); for (_, object) in mutated_objects { temporary_store.write_object(object); } @@ -1206,7 +1207,8 @@ pub async fn store_package_and_init_modules_for_genesis< .collect::>(); debug_assert!(ctx.digest() == TransactionDigest::genesis()); - let mut temporary_store = AuthorityTemporaryStore::new(store.clone(), filtered, ctx.digest()); + let mut temporary_store = + AuthorityTemporaryStore::new(store.clone(), InputObjects::new(filtered), ctx.digest()); let package_id = ObjectID::from(*modules[0].self_id().address()); let natives = native_functions.clone(); let mut gas_status = SuiGasStatus::new_unmetered(); @@ -1239,7 +1241,8 @@ pub async fn generate_genesis_system_object< genesis_ctx: &mut TxContext, ) -> SuiResult { let genesis_digest = genesis_ctx.digest(); - let mut temporary_store = AuthorityTemporaryStore::new(store.clone(), vec![], genesis_digest); + let mut temporary_store = + AuthorityTemporaryStore::new(store.clone(), InputObjects::new(vec![]), genesis_digest); let pubkeys: Vec> = committee .expanded_keys .values() diff --git a/crates/sui-core/src/authority/temporary_store.rs b/crates/sui-core/src/authority/temporary_store.rs index a9222bed95ce8..d0018c4a48990 100644 --- a/crates/sui-core/src/authority/temporary_store.rs +++ b/crates/sui-core/src/authority/temporary_store.rs @@ -1,5 +1,6 @@ // Copyright (c) 2022, Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::transaction_input_checker::InputObjects; use move_core_types::account_address::AccountAddress; use sui_types::{event::Event, gas::SuiGasStatus, object::Owner}; @@ -20,7 +21,7 @@ pub struct AuthorityTemporaryStore { package_store: Arc, tx_digest: TransactionDigest, objects: BTreeMap, - active_inputs: Vec, // Inputs that are not read only + mutable_inputs: Vec, // Inputs that are mutable written: BTreeMap, // Objects written /// Objects actively deleted. deleted: BTreeMap, @@ -36,32 +37,16 @@ impl AuthorityTemporaryStore { /// initial objects. pub fn new( package_store: Arc, - input_objects: Vec<(InputObjectKind, Object)>, + input_objects: InputObjects, tx_digest: TransactionDigest, ) -> Self { - let active_inputs = input_objects - .iter() - .filter_map(|(kind, object)| match kind { - InputObjectKind::MovePackage(_) => None, - InputObjectKind::ImmOrOwnedMoveObject(object_ref) => { - if object.is_immutable() { - None - } else { - Some(*object_ref) - } - } - InputObjectKind::SharedMoveObject(_) => Some(object.compute_object_reference()), - }) - .collect(); - let objects = input_objects - .into_iter() - .map(|(_, object)| (object.id(), object)) - .collect(); + let mutable_inputs = input_objects.mutable_inputs(); + let objects = input_objects.into_object_map(); Self { package_store, tx_digest, objects, - active_inputs, + mutable_inputs, written: BTreeMap::new(), deleted: BTreeMap::new(), events: Vec::new(), @@ -90,7 +75,7 @@ impl AuthorityTemporaryStore { } ( self.objects, - self.active_inputs, + self.mutable_inputs, self.written, self.deleted, self.events, @@ -102,7 +87,7 @@ impl AuthorityTemporaryStore { /// sequence number. This is required to achieve safety. /// We skip the gas object, because gas object will be updated separately. pub fn ensure_active_inputs_mutated(&mut self, gas_object_id: &ObjectID) { - for (id, _seq, _) in &self.active_inputs { + for (id, _seq, _) in &self.mutable_inputs { if id == gas_object_id { continue; } @@ -268,7 +253,7 @@ impl AuthorityTemporaryStore { self.written.iter().all(|(elt, _)| used.insert(elt)); self.deleted.iter().all(|elt| used.insert(elt.0)); - self.active_inputs.iter().all(|elt| !used.insert(&elt.0)) + self.mutable_inputs.iter().all(|elt| !used.insert(&elt.0)) }, "Mutable input neither written nor deleted." ); diff --git a/crates/sui-core/src/epoch/tests/reconfiguration_tests.rs b/crates/sui-core/src/epoch/tests/reconfiguration_tests.rs index 1b5345f7e1a3c..68ab5f99378b9 100644 --- a/crates/sui-core/src/epoch/tests/reconfiguration_tests.rs +++ b/crates/sui-core/src/epoch/tests/reconfiguration_tests.rs @@ -20,6 +20,7 @@ use sui_types::{ SUI_SYSTEM_STATE_OBJECT_ID, }; +use crate::transaction_input_checker::InputObjects; use crate::{ authority::AuthorityTemporaryStore, authority_active::ActiveAuthority, authority_aggregator::authority_aggregator_tests::init_local_authorities, @@ -130,13 +131,15 @@ async fn test_start_epoch_change() { let tx_digest = *transaction.digest(); let mut temporary_store = AuthorityTemporaryStore::new( state.database.clone(), - transaction - .data - .input_objects() - .unwrap() - .into_iter() - .zip(genesis_objects) - .collect(), + InputObjects::new( + transaction + .data + .input_objects() + .unwrap() + .into_iter() + .zip(genesis_objects) + .collect(), + ), tx_digest, ); let effects = execution_engine::execute_transaction_to_effects( diff --git a/crates/sui-core/src/gateway_state.rs b/crates/sui-core/src/gateway_state.rs index 9dd33eb61507a..08f437e2981a0 100644 --- a/crates/sui-core/src/gateway_state.rs +++ b/crates/sui-core/src/gateway_state.rs @@ -44,6 +44,7 @@ use crate::{ use sui_json::{resolve_move_function_args, SuiJsonCallArg, SuiJsonValue}; use crate::gateway_types::*; +use crate::transaction_input_checker::InputObjects; #[cfg(test)] #[path = "unit_tests/gateway_state_tests.rs"] @@ -445,7 +446,7 @@ where async fn execute_transaction_impl_inner( &self, - all_objects: Vec<(InputObjectKind, Object)>, + input_objects: InputObjects, transaction: Transaction, ) -> Result<(CertifiedTransaction, TransactionEffects), anyhow::Error> { // If execute_transaction ever fails due to panic, we should fix the panic and make sure it doesn't. @@ -497,7 +498,7 @@ where ); self.store .update_gateway_state( - all_objects, + input_objects, mutated_objects, new_certificate.clone(), effects.clone().to_unsigned_effects(), @@ -520,20 +521,20 @@ where self.sync_input_objects_with_authorities(&transaction) .await?; - let (_gas_status, all_objects) = transaction_input_checker::check_transaction_input( + let (_gas_status, input_objects) = transaction_input_checker::check_transaction_input( &self.store, &transaction, &self.metrics.shared_obj_tx, ) .await?; - let owned_objects = transaction_input_checker::filter_owned_objects(&all_objects); + let owned_objects = input_objects.filter_owned_objects(); self.set_transaction_lock(&owned_objects, transaction.clone()) .instrument(tracing::trace_span!("db_set_transaction_lock")) .await?; let exec_result = self - .execute_transaction_impl_inner(all_objects, transaction) + .execute_transaction_impl_inner(input_objects, transaction) .await; if exec_result.is_err() && is_last_retry { // If we cannot successfully execute this transaction, even after all the retries, diff --git a/crates/sui-core/src/transaction_input_checker.rs b/crates/sui-core/src/transaction_input_checker.rs index 875e1b29a307c..274b52862bd7c 100644 --- a/crates/sui-core/src/transaction_input_checker.rs +++ b/crates/sui-core/src/transaction_input_checker.rs @@ -1,10 +1,11 @@ // Copyright (c) 2022, Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -use std::collections::HashSet; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use prometheus_exporter::prometheus::IntCounter; use serde::{Deserialize, Serialize}; +use sui_types::base_types::TransactionDigest; use sui_types::{ base_types::{ObjectID, ObjectRef, SequenceNumber, SuiAddress}, error::{SuiError, SuiResult}, @@ -17,12 +18,94 @@ use tracing::{debug, instrument}; use crate::authority::SuiDataStore; +pub struct InputObjects { + objects: Vec<(InputObjectKind, Object)>, +} + +impl InputObjects { + pub fn new(objects: Vec<(InputObjectKind, Object)>) -> Self { + Self { objects } + } + + pub fn len(&self) -> usize { + self.objects.len() + } + + pub fn is_empty(&self) -> bool { + self.objects.is_empty() + } + + pub fn filter_owned_objects(&self) -> Vec { + let owned_objects: Vec<_> = self + .objects + .iter() + .filter_map(|(object_kind, object)| match object_kind { + InputObjectKind::MovePackage(_) => None, + InputObjectKind::ImmOrOwnedMoveObject(object_ref) => { + if object.is_immutable() { + None + } else { + Some(*object_ref) + } + } + InputObjectKind::SharedMoveObject(_) => None, + }) + .collect(); + + debug!( + num_mutable_objects = owned_objects.len(), + "Checked locks and found mutable objects" + ); + + owned_objects + } + + pub fn filter_shared_objects(&self) -> Vec { + self.objects + .iter() + .filter(|(kind, _)| matches!(kind, InputObjectKind::SharedMoveObject(_))) + .map(|(_, obj)| obj.compute_object_reference()) + .collect() + } + + pub fn transaction_dependencies(&self) -> BTreeSet { + self.objects + .iter() + .map(|(_, obj)| obj.previous_transaction) + .collect() + } + + pub fn mutable_inputs(&self) -> Vec { + self.objects + .iter() + .filter_map(|(kind, object)| match kind { + InputObjectKind::MovePackage(_) => None, + InputObjectKind::ImmOrOwnedMoveObject(object_ref) => { + if object.is_immutable() { + None + } else { + Some(*object_ref) + } + } + InputObjectKind::SharedMoveObject(_) => Some(object.compute_object_reference()), + }) + .collect() + } + + pub fn into_object_map(self) -> BTreeMap { + self.objects + .into_iter() + .map(|(_, object)| (object.id(), object)) + .collect() + } +} + #[instrument(level = "trace", skip_all)] pub async fn check_transaction_input( store: &SuiDataStore, transaction: &TransactionEnvelope, shared_obj_metric: &IntCounter, -) -> Result<(SuiGasStatus<'static>, Vec<(InputObjectKind, Object)>), SuiError> +) -> Result<(SuiGasStatus<'static>, InputObjects), SuiError> where S: Eq + Serialize + for<'de> Deserialize<'de>, { @@ -34,7 +117,7 @@ where ) .await?; - let objects_by_kind = check_locks(store, &transaction.data).await?; + let input_objects = check_locks(store, &transaction.data).await?; if transaction.contains_shared_object() { shared_obj_metric.inc(); @@ -44,7 +127,7 @@ where gas_status.charge_consensus()?; } - Ok((gas_status, objects_by_kind)) + Ok((gas_status, input_objects)) } /// Checking gas budget by fetching the gas object only from the store, @@ -93,7 +176,7 @@ where async fn check_locks( store: &SuiDataStore, transaction: &TransactionData, -) -> Result, SuiError> +) -> Result where S: Eq + Serialize + for<'de> Deserialize<'de>, { @@ -172,31 +255,7 @@ where } fp_ensure!(!all_objects.is_empty(), SuiError::ObjectInputArityViolation); - Ok(all_objects) -} - -pub fn filter_owned_objects(all_objects: &[(InputObjectKind, Object)]) -> Vec { - let owned_objects: Vec<_> = all_objects - .iter() - .filter_map(|(object_kind, object)| match object_kind { - InputObjectKind::MovePackage(_) => None, - InputObjectKind::ImmOrOwnedMoveObject(object_ref) => { - if object.is_immutable() { - None - } else { - Some(*object_ref) - } - } - InputObjectKind::SharedMoveObject(_) => None, - }) - .collect(); - - debug!( - num_mutable_objects = owned_objects.len(), - "Checked locks and found mutable objects" - ); - - owned_objects + Ok(InputObjects::new(all_objects)) } /// The logic to check one object against a reference, and return the object if all is well diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index ddbb5e533dc37..8842e2932f326 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -6,7 +6,6 @@ use crate::{args::*, in_memory_storage::InMemoryStorage}; use anyhow::{anyhow, bail}; use bimap::btree::BiBTreeMap; -use itertools::Itertools; use move_binary_format::{file_format::CompiledScript, CompiledModule}; use move_bytecode_utils::module_cache::GetModule; use move_command_line_common::{ @@ -38,6 +37,7 @@ use std::{ sync::Arc, }; use sui_adapter::{adapter::new_move_vm, genesis}; +use sui_core::transaction_input_checker::InputObjects; use sui_core::{authority::AuthorityTemporaryStore, execution_engine}; use sui_framework::DEFAULT_FRAMEWORK_PATH; use sui_types::{ @@ -49,9 +49,7 @@ use sui_types::{ error::SuiError, event::Event, gas, - messages::{ - ExecutionStatus, InputObjectKind, Transaction, TransactionData, TransactionEffects, - }, + messages::{ExecutionStatus, Transaction, TransactionData, TransactionEffects}, object::{self, Object, ObjectFormatOptions, GAS_VALUE_FOR_TESTING}, MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS, }; @@ -444,18 +442,11 @@ impl<'a> SuiTestAdapter<'a> { Some((kind, obj)) }) .collect::>(); - let transaction_dependencies = objects_by_kind - .iter() - .map(|(_, obj)| obj.previous_transaction) - .collect(); - let shared_object_refs: Vec<_> = objects_by_kind - .iter() - .filter(|(kind, _)| matches!(kind, InputObjectKind::SharedMoveObject(_))) - .map(|(_, obj)| obj.compute_object_reference()) - .sorted() - .collect(); + let input_objects = InputObjects::new(objects_by_kind); + let transaction_dependencies = input_objects.transaction_dependencies(); + let shared_object_refs: Vec<_> = input_objects.filter_shared_objects(); let mut temporary_store = - AuthorityTemporaryStore::new(self.storage.clone(), objects_by_kind, transaction_digest); + AuthorityTemporaryStore::new(self.storage.clone(), input_objects, transaction_digest); let TransactionEffects { status, events,