diff --git a/crates/sui-core/src/authority.rs b/crates/sui-core/src/authority.rs index 783f911d991c3..5028fc0e0918c 100644 --- a/crates/sui-core/src/authority.rs +++ b/crates/sui-core/src/authority.rs @@ -1265,7 +1265,7 @@ impl AuthorityState { .start_timer(); let input_objects = &certificate.data().transaction_data().input_objects()?; self.input_loader.read_objects_for_execution( - epoch_store.as_ref(), + epoch_store, &certificate.key(), tx_lock, input_objects, diff --git a/crates/sui-core/src/authority/authority_per_epoch_store.rs b/crates/sui-core/src/authority/authority_per_epoch_store.rs index f1ab486dee221..0b65711b185be 100644 --- a/crates/sui-core/src/authority/authority_per_epoch_store.rs +++ b/crates/sui-core/src/authority/authority_per_epoch_store.rs @@ -58,7 +58,6 @@ use sui_types::messages_consensus::{ VersionedDkgConfirmation, }; use sui_types::signature::GenericSignature; -use sui_types::storage::GetSharedLocks; use sui_types::storage::{BackingPackageStore, InputKey, ObjectStore}; use sui_types::sui_system_state::epoch_start_sui_system_state::{ EpochStartSystemState, EpochStartSystemStateTrait, @@ -1413,7 +1412,7 @@ impl AuthorityPerEpochStore { key: &TransactionKey, objects: &[InputObjectKind], ) -> SuiResult> { - let shared_locks = once_cell::unsync::OnceCell::< + let assigned_shared_versions = once_cell::unsync::OnceCell::< Option>, >::new(); objects @@ -1425,17 +1424,17 @@ impl AuthorityPerEpochStore { initial_shared_version, .. } => { - let shared_locks = shared_locks + let assigned_shared_versions = assigned_shared_versions .get_or_init(|| { - self.get_shared_locks(key) - .expect("reading shared locks should not fail") - .map(|locks| locks.into_iter().collect()) + self.get_assigned_shared_object_versions(key) + .expect("reading assigned shared versions should not fail") + .map(|versions| versions.into_iter().collect()) }) .as_ref() // Shared version assignments could have been deleted if the tx just // finished executing concurrently. .ok_or(SuiError::GenericAuthorityError { - error: "no shared locks".to_string(), + error: "no assigned shared versions".to_string(), })?; let initial_shared_version = @@ -1443,16 +1442,16 @@ impl AuthorityPerEpochStore { *initial_shared_version } else { // (before ConsensusV2 objects, we didn't track initial shared - // version for shared object locks) + // version for shared object version assignments) SequenceNumber::UNKNOWN }; - // If we found locks, but they are missing the assignment for this object, - // it indicates a serious inconsistency! - let Some(version) = shared_locks.get(&(*id, initial_shared_version)) else { + // If we found assigned versions, but they are missing the assignment for + // this object, it indicates a serious inconsistency! + let Some(version) = assigned_shared_versions.get(&(*id, initial_shared_version)) else { panic!( - "Shared object locks should have been set. key: {key:?}, obj \ - id: {id:?}, initial_shared_version: {initial_shared_version:?}, \ - shared_locks: {shared_locks:?}", + "Shared object version should have been assigned. key: {key:?}, \ + obj id: {id:?}, initial_shared_version: {initial_shared_version:?}, \ + assigned_shared_versions: {assigned_shared_versions:?}", ) }; InputKey::VersionedObject { @@ -1715,7 +1714,7 @@ impl AuthorityPerEpochStore { // created the shared object originally - which transaction may not yet have been executed on // this node). // - // Because all paths that assign shared locks for a shared object transaction call this + // Because all paths that assign shared versions for a shared object transaction call this // function, it is impossible for parent_sync to be updated before this function completes // successfully for each affected object id. pub(crate) async fn get_or_init_next_object_versions( @@ -1813,6 +1812,26 @@ impl AuthorityPerEpochStore { Ok(ret) } + pub fn get_assigned_shared_object_versions( + &self, + key: &TransactionKey, + ) -> SuiResult>> { + if self.epoch_start_config().use_version_assignment_tables_v3() { + Ok(self.tables()?.assigned_shared_object_versions_v3.get(key)?) + } else { + Ok(self + .tables()? + .assigned_shared_object_versions_v2 + .get(key)? + .map(|result| { + result + .into_iter() + .map(|(id, v)| ((id, SequenceNumber::UNKNOWN), v)) + .collect() + })) + } + } + async fn set_assigned_shared_object_versions_with_db_batch( &self, versions: AssignedTxAndVersions, @@ -2007,12 +2026,12 @@ impl AuthorityPerEpochStore { } } - /// Lock a sequence number for the shared objects of the input transaction based on the effects - /// of that transaction. + /// Assign a sequence number for the shared objects of the input transaction based on the + /// effects of that transaction. /// Used by full nodes who don't listen to consensus, and validators who catch up by state sync. - // TODO: We should be able to pass in a vector of certs/effects and lock them all at once. + // TODO: We should be able to pass in a vector of certs/effects and acquire them all at once. #[instrument(level = "trace", skip_all)] - pub async fn acquire_shared_locks_from_effects( + pub async fn acquire_shared_version_assignments_from_effects( &self, certificate: &VerifiedExecutableTransaction, effects: &TransactionEffects, @@ -4569,32 +4588,6 @@ impl ConsensusCommitOutput { } } -impl GetSharedLocks for AuthorityPerEpochStore { - fn get_shared_locks( - &self, - key: &TransactionKey, - ) -> SuiResult>> { - if self.epoch_start_config().use_version_assignment_tables_v3() { - Ok(self.tables()?.assigned_shared_object_versions_v3.get(key)?) - } else { - Ok(self - .tables()? - .assigned_shared_object_versions_v2 - .get(key)? - .map(|result| { - result - .into_iter() - .map(|(id, v)| ((id, SequenceNumber::UNKNOWN), v)) - .collect() - })) - } - } - - fn is_initial_shared_version_unknown(&self) -> bool { - !self.epoch_start_config().use_version_assignment_tables_v3() - } -} - impl ExecutionComponents { fn new( protocol_config: &ProtocolConfig, diff --git a/crates/sui-core/src/checkpoints/checkpoint_executor/mod.rs b/crates/sui-core/src/checkpoints/checkpoint_executor/mod.rs index 07e196b9999a5..780e32904bf6d 100644 --- a/crates/sui-core/src/checkpoints/checkpoint_executor/mod.rs +++ b/crates/sui-core/src/checkpoints/checkpoint_executor/mod.rs @@ -588,13 +588,13 @@ impl CheckpointExecutor { if change_epoch_tx.contains_shared_object() { epoch_store - .acquire_shared_locks_from_effects( + .acquire_shared_version_assignments_from_effects( &change_epoch_tx, &change_epoch_fx, self.object_cache_reader.as_ref(), ) .await - .expect("Acquiring shared locks for change_epoch tx cannot fail"); + .expect("Acquiring shared version assignments for change_epoch tx cannot fail"); } self.tx_manager.enqueue_with_expected_effects_digest( @@ -1237,7 +1237,7 @@ async fn execute_transactions( for (tx, _) in &executable_txns { if tx.contains_shared_object() { epoch_store - .acquire_shared_locks_from_effects( + .acquire_shared_version_assignments_from_effects( tx, digest_to_effects.get(tx.digest()).unwrap(), object_cache_reader, diff --git a/crates/sui-core/src/transaction_input_loader.rs b/crates/sui-core/src/transaction_input_loader.rs index 13be39d7acddc..016e8b956cf97 100644 --- a/crates/sui-core/src/transaction_input_loader.rs +++ b/crates/sui-core/src/transaction_input_loader.rs @@ -2,7 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - authority::authority_per_epoch_store::CertLockGuard, execution_cache::ObjectCacheRead, + authority::{ + authority_per_epoch_store::{AuthorityPerEpochStore, CertLockGuard}, + epoch_start_configuration::EpochStartConfigTrait, + }, + execution_cache::ObjectCacheRead, }; use itertools::izip; use mysten_common::fatal; @@ -12,7 +16,7 @@ use std::sync::Arc; use sui_types::{ base_types::{EpochId, ObjectRef, SequenceNumber, TransactionDigest}, error::{SuiError, SuiResult, UserInputError}, - storage::{GetSharedLocks, ObjectKey}, + storage::ObjectKey, transaction::{ InputObjectKind, InputObjects, ObjectReadResult, ObjectReadResultKind, ReceivingObjectReadResult, ReceivingObjectReadResultKind, ReceivingObjects, TransactionKey, @@ -112,7 +116,7 @@ impl TransactionInputLoader { /// Read the inputs for a transaction that is ready to be executed. /// - /// shared_lock_store is used to resolve the versions of any shared input objects. + /// epoch_store is used to resolve the versions of any shared input objects. /// /// This function panics if any inputs are not available, as TransactionManager should already /// have verified that the transaction is ready to be executed. @@ -127,13 +131,13 @@ impl TransactionInputLoader { #[instrument(level = "trace", skip_all)] pub fn read_objects_for_execution( &self, - shared_lock_store: &impl GetSharedLocks, + epoch_store: &Arc, tx_key: &TransactionKey, _tx_lock: &CertLockGuard, // see below for why this is needed input_object_kinds: &[InputObjectKind], epoch_id: EpochId, ) -> SuiResult { - let shared_locks_cell: OnceCell>> = OnceCell::new(); + let assigned_shared_versions_cell: OnceCell>> = OnceCell::new(); let mut results = vec![None; input_object_kinds.len()]; let mut object_keys = Vec::with_capacity(input_object_kinds.len()); @@ -161,32 +165,38 @@ impl TransactionInputLoader { initial_shared_version, .. } => { - let shared_locks = shared_locks_cell + let assigned_shared_versions = assigned_shared_versions_cell .get_or_init(|| { - shared_lock_store - .get_shared_locks(tx_key) - .expect("loading shared locks should not fail") - .map(|locks| locks.into_iter().collect()) + epoch_store + .get_assigned_shared_object_versions(tx_key) + .expect("loading assigned shared versions should not fail") + .map(|versions| versions.into_iter().collect()) }) .as_ref() .unwrap_or_else(|| { // Important to hold the _tx_lock here - otherwise it would be possible - // for a concurrent execution of the same tx to enter this point after the - // first execution has finished and the shared locks have been deleted. - fatal!("Failed to get shared locks for transaction {tx_key:?}"); + // for a concurrent execution of the same tx to enter this point after + // the first execution has finished and the assigned shared versions + // have been deleted. + fatal!( + "Failed to get assigned shared versions for transaction {tx_key:?}" + ); }); - let initial_shared_version = - if shared_lock_store.is_initial_shared_version_unknown() { - // (before ConsensusV2 objects, we didn't track initial shared - // version for shared object locks) - SequenceNumber::UNKNOWN - } else { - *initial_shared_version - }; - // If we find a set of locks but an object is missing, it indicates a serious inconsistency: - let version = shared_locks.get(&(*id, initial_shared_version)).unwrap_or_else(|| { - panic!("Shared object locks should have been set. key: {tx_key:?}, obj id: {id:?}") + let initial_shared_version = if epoch_store + .epoch_start_config() + .use_version_assignment_tables_v3() + { + *initial_shared_version + } else { + // (before ConsensusV2 objects, we didn't track initial shared + // version for shared object locks) + SequenceNumber::UNKNOWN + }; + // If we find a set of assigned versions but an object is missing, 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:?}") }); if version.is_cancelled() { // Do not need to fetch shared object for cancelled transaction. diff --git a/crates/sui-core/src/unit_tests/authority_tests.rs b/crates/sui-core/src/unit_tests/authority_tests.rs index 455cda7005df0..dc086129d2bc4 100644 --- a/crates/sui-core/src/unit_tests/authority_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_tests.rs @@ -44,7 +44,6 @@ use sui_types::messages_consensus::{ use sui_types::object::Data; use sui_types::programmable_transaction_builder::ProgrammableTransactionBuilder; use sui_types::randomness_state::get_randomness_state_obj_initial_shared_version; -use sui_types::storage::GetSharedLocks; use sui_types::sui_system_state::SuiSystemStateWrapper; use sui_types::supported_protocol_versions::SupportedProtocolVersions; use sui_types::utils::{ @@ -4582,9 +4581,9 @@ async fn test_shared_object_transaction_ok() { // Verify shared locks are now set for the transaction. let shared_object_version = authority .epoch_store_for_testing() - .get_shared_locks(&certificate.key()) - .expect("Reading shared locks should not fail") - .expect("Locks should be set") + .get_assigned_shared_object_versions(&certificate.key()) + .expect("Reading shared version assignments should not fail") + .expect("Versions should be set") .into_iter() .find_map(|((object_id, initial_shared_version), version)| { if object_id == shared_object_id @@ -4595,7 +4594,7 @@ async fn test_shared_object_transaction_ok() { None } }) - .expect("Shared object must be locked"); + .expect("Shared object must be assigned a version"); assert_eq!(shared_object_version, OBJECT_START_VERSION); // Finally (Re-)execute the contract should succeed. @@ -4700,9 +4699,9 @@ async fn test_consensus_commit_prologue_generation() { let get_assigned_version = |txn_key: &TransactionKey| -> SequenceNumber { authority_state .epoch_store_for_testing() - .get_shared_locks(txn_key) + .get_assigned_shared_object_versions(txn_key) .unwrap() - .expect("locks should be set") + .expect("versions should be set") .iter() .filter_map(|((id, initial_shared_version), seq)| { if id == &SUI_CLOCK_OBJECT_ID @@ -4808,7 +4807,7 @@ async fn test_consensus_message_processed() { } else { let epoch_store = authority2.epoch_store_for_testing(); epoch_store - .acquire_shared_locks_from_effects( + .acquire_shared_version_assignments_from_effects( &VerifiedExecutableTransaction::new_from_certificate(certificate.clone()), &effects1, authority2.get_object_cache_reader().as_ref(), @@ -6071,9 +6070,9 @@ async fn test_consensus_handler_congestion_control_transaction_cancellation() { // Check cancelled transaction shared locks. let shared_object_version = authority .epoch_store_for_testing() - .get_shared_locks(&cancelled_txn.key()) - .expect("Reading shared locks should not fail") - .expect("locks should be set") + .get_assigned_shared_object_versions(&cancelled_txn.key()) + .expect("Reading shared version assignments should not fail") + .expect("Versions should be set") .into_iter() .collect::>(); assert_eq!( @@ -6102,7 +6101,7 @@ async fn test_consensus_handler_congestion_control_transaction_cancellation() { let input_loader = TransactionInputLoader::new(authority.get_object_cache_reader().clone()); let input_objects = input_loader .read_objects_for_execution( - authority.epoch_store_for_testing().as_ref(), + &authority.epoch_store_for_testing(), &cancelled_txn.key(), &CertLockGuard::dummy_for_tests(), &cancelled_txn diff --git a/crates/sui-core/src/unit_tests/congestion_control_tests.rs b/crates/sui-core/src/unit_tests/congestion_control_tests.rs index 9cd6d7e7de492..c14cc7bee7fd8 100644 --- a/crates/sui-core/src/unit_tests/congestion_control_tests.rs +++ b/crates/sui-core/src/unit_tests/congestion_control_tests.rs @@ -350,7 +350,7 @@ async fn test_congestion_control_execution_cancellation() { .unwrap(); authority_state_2 .epoch_store_for_testing() - .acquire_shared_locks_from_effects( + .acquire_shared_version_assignments_from_effects( &VerifiedExecutableTransaction::new_from_certificate(cert.clone()), &effects, authority_state_2.get_object_cache_reader().as_ref(), diff --git a/crates/sui-single-node-benchmark/src/mock_storage.rs b/crates/sui-single-node-benchmark/src/mock_storage.rs index e3d5325e93b57..f16c3acb8675e 100644 --- a/crates/sui-single-node-benchmark/src/mock_storage.rs +++ b/crates/sui-single-node-benchmark/src/mock_storage.rs @@ -8,16 +8,16 @@ use once_cell::unsync::OnceCell; use prometheus::core::{Atomic, AtomicU64}; use std::collections::HashMap; use std::sync::{Arc, RwLock}; +use sui_core::authority::authority_per_epoch_store::AuthorityPerEpochStore; +use sui_core::authority::epoch_start_configuration::EpochStartConfigTrait; use sui_storage::package_object_cache::PackageObjectCache; -use sui_types::base_types::{ - ConsensusObjectSequenceKey, EpochId, ObjectID, ObjectRef, SequenceNumber, VersionNumber, -}; +use sui_types::base_types::{EpochId, ObjectID, ObjectRef, SequenceNumber, VersionNumber}; use sui_types::error::{SuiError, SuiResult}; use sui_types::inner_temporary_store::InnerTemporaryStore; use sui_types::object::{Object, Owner}; use sui_types::storage::{ - get_module_by_id, BackingPackageStore, ChildObjectResolver, GetSharedLocks, ObjectStore, - PackageObject, ParentSync, + get_module_by_id, BackingPackageStore, ChildObjectResolver, ObjectStore, PackageObject, + ParentSync, }; use sui_types::transaction::{InputObjectKind, InputObjects, ObjectReadResult, TransactionKey}; @@ -46,11 +46,11 @@ impl InMemoryObjectStore { // We will need a trait to unify the these functions. (similarly the one in simulacrum) pub(crate) fn read_objects_for_execution( &self, - shared_locks_source: &dyn GetSharedLocks, + epoch_store: &Arc, tx_key: &TransactionKey, input_object_kinds: &[InputObjectKind], ) -> SuiResult { - let shared_locks_cell: OnceCell>> = OnceCell::new(); + let shared_version_assignments_cell: OnceCell>> = OnceCell::new(); let mut input_objects = Vec::new(); for kind in input_object_kinds { let obj: Option = match kind { @@ -64,27 +64,29 @@ impl InMemoryObjectStore { initial_shared_version, .. } => { - let shared_locks = shared_locks_cell + let shared_version_assignments = shared_version_assignments_cell .get_or_init(|| { - shared_locks_source - .get_shared_locks(tx_key) - .expect("get_shared_locks should not fail") + epoch_store + .get_assigned_shared_object_versions(tx_key) + .expect("get_assigned_shared_object_versions should not fail") .map(|l| l.into_iter().collect()) }) .as_ref() .ok_or_else(|| SuiError::GenericAuthorityError { - error: "Shared object locks should have been set.".to_string(), + error: "Shared object versions should have been assigned.".to_string(), })?; - let initial_shared_version = - if shared_locks_source.is_initial_shared_version_unknown() { - // (before ConsensusV2 objects, we didn't track initial shared - // version for shared object locks) - SequenceNumber::UNKNOWN - } else { - *initial_shared_version - }; - let version = shared_locks.get(&(*id, initial_shared_version)).unwrap_or_else(|| { - panic!("Shared object locks should have been set. key: {tx_key:?}, obj id: {id:?}") + let initial_shared_version = if epoch_store + .epoch_start_config() + .use_version_assignment_tables_v3() + { + *initial_shared_version + } else { + // (before ConsensusV2 objects, we didn't track initial shared + // version for shared object locks) + SequenceNumber::UNKNOWN + }; + let version = shared_version_assignments.get(&(*id, initial_shared_version)).unwrap_or_else(|| { + panic!("Shared object version should have been assigned. key: {tx_key:?}, obj id: {id:?}") }); self.get_object_by_key(id, *version) @@ -179,16 +181,3 @@ impl ParentSync for InMemoryObjectStore { unreachable!() } } - -impl GetSharedLocks for InMemoryObjectStore { - fn get_shared_locks( - &self, - _key: &TransactionKey, - ) -> SuiResult>> { - unreachable!() - } - - fn is_initial_shared_version_unknown(&self) -> bool { - unreachable!() - } -} diff --git a/crates/sui-single-node-benchmark/src/single_node.rs b/crates/sui-single-node-benchmark/src/single_node.rs index 5bea4521f65bc..d26b420d92661 100644 --- a/crates/sui-single-node-benchmark/src/single_node.rs +++ b/crates/sui-single-node-benchmark/src/single_node.rs @@ -192,7 +192,7 @@ impl SingleValidator { ) -> TransactionEffects { let input_objects = transaction.transaction_data().input_objects().unwrap(); let objects = store - .read_objects_for_execution(&*self.epoch_store, &transaction.key(), &input_objects) + .read_objects_for_execution(&self.epoch_store, &transaction.key(), &input_objects) .unwrap(); let executable = VerifiedExecutableTransaction::new_from_certificate( diff --git a/crates/sui-types/src/storage/mod.rs b/crates/sui-types/src/storage/mod.rs index a7770a7755828..8b06376387860 100644 --- a/crates/sui-types/src/storage/mod.rs +++ b/crates/sui-types/src/storage/mod.rs @@ -7,12 +7,12 @@ mod read_store; mod shared_in_memory_store; mod write_store; -use crate::base_types::{ConsensusObjectSequenceKey, TransactionDigest, VersionNumber}; +use crate::base_types::{TransactionDigest, VersionNumber}; use crate::committee::EpochId; use crate::error::{ExecutionError, SuiError}; use crate::execution::{DynamicallyLoadedObjectMetadata, ExecutionResults}; use crate::move_package::MovePackage; -use crate::transaction::{SenderSignedData, TransactionDataAPI, TransactionKey}; +use crate::transaction::{SenderSignedData, TransactionDataAPI}; use crate::{ base_types::{ObjectID, ObjectRef, SequenceNumber}, error::SuiResult, @@ -593,12 +593,3 @@ where self } } - -pub trait GetSharedLocks: Send + Sync { - fn get_shared_locks( - &self, - key: &TransactionKey, - ) -> SuiResult>>; - - fn is_initial_shared_version_unknown(&self) -> bool; -}