diff --git a/sui_core/src/authority.rs b/sui_core/src/authority.rs index 80f0399cf9491..10a3249649844 100644 --- a/sui_core/src/authority.rs +++ b/sui_core/src/authority.rs @@ -10,7 +10,7 @@ use move_core_types::{ }; use move_vm_runtime::native_functions::NativeFunctionTable; use std::{ - collections::{BTreeMap, BTreeSet, HashSet, VecDeque}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}, pin::Pin, sync::Arc, }; @@ -119,7 +119,7 @@ impl AuthorityState { transaction: &Transaction, object_kind: InputObjectKind, object: &Object, - mutable_object_addresses: &HashSet, + owned_object_authenticators: &HashSet, ) -> SuiResult { match object_kind { InputObjectKind::MovePackage(package_id) => { @@ -184,7 +184,7 @@ impl AuthorityState { Owner::ObjectOwner(owner) => { // Check that the object owner is another mutable object in the input. fp_ensure!( - mutable_object_addresses.contains(&owner), + owned_object_authenticators.contains(&owner), SuiError::IncorrectSigner ); } @@ -193,7 +193,10 @@ impl AuthorityState { } }; } - InputObjectKind::SharedMoveObject(..) => (), + InputObjectKind::SharedMoveObject(..) => { + // When someone locks an object as shared it must be shared already. + fp_ensure!(object.is_shared(), SuiError::NotSharedObjectError); + } }; Ok(()) } @@ -205,18 +208,22 @@ impl AuthorityState { transaction: &Transaction, ) -> Result, SuiError> { let input_objects = transaction.input_objects(); - let mut all_objects = Vec::with_capacity(input_objects.len()); + // These IDs act as authenticators that can own other objects. let objects = self.fetch_objects(&input_objects).await?; - let mutable_object_addresses: HashSet<_> = objects + let owned_object_authenticators: HashSet<_> = objects .iter() .flat_map(|opt_obj| match opt_obj { Some(obj) if !obj.is_read_only() => Some(obj.id().into()), _ => None, }) .collect(); + + // Gather all objects and errors. + let mut all_objects = Vec::with_capacity(input_objects.len()); let mut errors = Vec::new(); for (object_kind, object) in input_objects.into_iter().zip(objects) { + // All objects must exist in the DB. let object = match object { Some(object) => object, None => { @@ -225,8 +232,14 @@ impl AuthorityState { } }; - match self.check_one_lock(transaction, object_kind, &object, &mutable_object_addresses) - { + // Check if the object contents match the type of lock we need for + // this object. + match self.check_one_lock( + transaction, + object_kind, + &object, + &owned_object_authenticators, + ) { Ok(()) => all_objects.push((object_kind, object)), Err(e) => { errors.push(e); @@ -274,7 +287,7 @@ impl AuthorityState { transaction.check_signature()?; let transaction_digest = transaction.digest(); - let mutable_objects: Vec<_> = self + let owned_objects: Vec<_> = self .check_locks(&transaction) .instrument(tracing::trace_span!("tx_check_locks")) .await? @@ -293,7 +306,7 @@ impl AuthorityState { .collect(); debug!( - num_mutable_objects = mutable_objects.len(), + num_mutable_objects = owned_objects.len(), "Checked locks and found mutable objects" ); @@ -303,7 +316,7 @@ impl AuthorityState { // The call to self.set_transaction_lock checks the lock is not conflicting, // and returns ConflictingTransaction error in case there is a lock on a different // existing transaction. - self.set_transaction_lock(&mutable_objects, signed_transaction) + self.set_transaction_lock(&owned_objects, signed_transaction) .instrument(tracing::trace_span!("db_set_transaction_lock")) .await?; @@ -336,59 +349,61 @@ impl AuthorityState { &self, transaction_digest: TransactionDigest, transaction: &Transaction, - ) -> Result, SuiError> { + inputs: &[(InputObjectKind, Object)], + ) -> Result<(), SuiError> { // If the transaction contains shared objects, we need to ensure they have been scheduled // for processing by the consensus protocol. if transaction.contains_shared_object() { debug!("Validating shared object sequence numbers from consensus..."); - let mut lock_errors = Vec::new(); - - let shared_ids = transaction.shared_input_objects(); - let shared_locks = self._database.sequenced(transaction_digest, shared_ids)?; - let shared_objects = self.get_objects(shared_ids).await?; - - let mut inputs = Vec::with_capacity(shared_ids.len()); - - for (lock, object) in shared_locks.into_iter().zip(shared_objects.into_iter()) { - // Check whether the shared objects have already been assigned a sequence number by - // the consensus. Bail if the transaction contains even one shared object that either: - // (i) was not assigned a sequence number, or - // (ii) has a different sequence number than the current one. - // - // Note that if the shared object is not in storage (it has been destroyed), we keep - // processing the transaction to unlock all single-writer objects. The execution engine - // will simply execute no-op. - - match (lock, object) { - (Some(seq), Some(object)) => { - if object.version() != seq { - warn!(object_version =? object.version(), - locked_version =? seq, - "Unexpected version number in locked shared object"); - // TODO: Send a more informative error message here, stating - // that we are waiting for the execution. - lock_errors.push(SuiError::InvalidSequenceNumber); - continue; - } - - inputs.push(object); + + // Collect the version we have for each shared object + let shared_ids: HashSet<_> = inputs + .iter() + .filter_map(|(kind, obj)| match kind { + InputObjectKind::SharedMoveObject(..) if obj.owner.is_shared_mutable() => { + Some((obj.id(), obj.version())) } - _ => { - lock_errors.push(SuiError::InvalidSequenceNumber); + _ => None, + }) + .collect(); + // Internal consistency check + debug_assert!( + !shared_ids.is_empty(), + "we just checked that there are share objects yet none found?" + ); + + // Read the + let shared_locks: HashMap<_, _> = self + ._database + .all_shared_locks(transaction_digest)? + .into_iter() + .collect(); + + let lock_errors: Vec<_> = shared_ids + .iter() + .filter_map(|(object_id, version)| { + if !shared_locks.contains_key(object_id) { + Some(SuiError::SharedObjectLockNotSetObject) + } else if shared_locks[object_id] != *version { + Some(SuiError::UnexpectedSequenceNumber { + object_id: *object_id, + expected_sequence: shared_locks[object_id], + }) + } else { + None } - } - } + }) + .collect(); + fp_ensure!( lock_errors.is_empty(), SuiError::LockErrors { errors: lock_errors } ); - - return Ok(inputs); } - Ok(Vec::new()) + Ok(()) } async fn process_certificate( @@ -399,23 +414,25 @@ impl AuthorityState { let transaction = certificate.transaction.clone(); let transaction_digest = transaction.digest(); - let mut inputs: Vec<_> = self - .check_locks(&transaction) - .await? - .into_iter() - .map(|(_, object)| object) - .collect(); + let objects_by_kind: Vec<_> = self.check_locks(&transaction).await?; - let shared_objects = self - .check_shared_locks(transaction_digest, &transaction) + // At this point we need to check if any shared objects need locks, + // and whether they have them. + let _shared_objects = self + .check_shared_locks(transaction_digest, &transaction, &objects_by_kind) .await?; - inputs.extend(shared_objects); + // inputs.extend(shared_objects); debug!( - num_inputs = inputs.len(), + num_inputs = objects_by_kind.len(), "Read inputs for transaction from DB" ); + let inputs: Vec<_> = objects_by_kind + .into_iter() + .map(|(_, object)| object) + .collect(); + let mut transaction_dependencies: BTreeSet<_> = inputs .iter() .map(|object| object.previous_transaction) diff --git a/sui_core/src/authority/authority_store.rs b/sui_core/src/authority/authority_store.rs index c7165d7d3c0a6..31459c66a488b 100644 --- a/sui_core/src/authority/authority_store.rs +++ b/sui_core/src/authority/authority_store.rs @@ -314,6 +314,20 @@ impl SuiDataStore { self.sequenced.multi_get(&keys[..]).map_err(SuiError::from) } + /// Read a lock for a specific (transaction, shared object) pair. + pub fn all_shared_locks( + &self, + transaction_digest: TransactionDigest, + ) -> Result, SuiError> { + Ok(self + .sequenced + .iter() + .skip_to(&(transaction_digest, ObjectID::ZERO))? + .take_while(|((tx, _objid), _ver)| *tx == transaction_digest) + .map(|((_tx, objid), ver)| (objid, ver)) + .collect()) + } + // Methods to mutate the store /// Insert an object diff --git a/sui_core/tests/staged/sui.yaml b/sui_core/tests/staged/sui.yaml index 8f715b672c257..66c99a10f276f 100644 --- a/sui_core/tests/staged/sui.yaml +++ b/sui_core/tests/staged/sui.yaml @@ -360,208 +360,212 @@ SuiError: 5: UnsupportedSharedObjectError: UNIT 6: - DeleteObjectOwnedObject: UNIT + NotSharedObjectError: UNIT 7: + DeleteObjectOwnedObject: UNIT + 8: + SharedObjectLockNotSetObject: UNIT + 9: InvalidSignature: STRUCT: - error: STR - 8: + 10: IncorrectSigner: UNIT - 9: + 11: UnknownSigner: UNIT - 10: + 12: CertificateRequiresQuorum: UNIT - 11: + 13: UnexpectedSequenceNumber: STRUCT: - object_id: TYPENAME: ObjectID - expected_sequence: TYPENAME: SequenceNumber - 12: + 14: ConflictingTransaction: STRUCT: - pending_transaction: TYPENAME: Transaction - 13: + 15: ErrorWhileProcessingTransaction: UNIT - 14: + 16: ErrorWhileProcessingTransactionTransaction: STRUCT: - err: STR - 15: + 17: ErrorWhileProcessingConfirmationTransaction: STRUCT: - err: STR - 16: + 18: ErrorWhileRequestingCertificate: UNIT - 17: + 19: ErrorWhileProcessingPublish: STRUCT: - err: STR - 18: + 20: ErrorWhileProcessingMoveCall: STRUCT: - err: STR - 19: + 21: ErrorWhileRequestingInformation: UNIT - 20: + 22: ObjectFetchFailed: STRUCT: - object_id: TYPENAME: ObjectID - err: STR - 21: + 23: MissingEalierConfirmations: STRUCT: - object_id: TYPENAME: ObjectID - current_sequence_number: TYPENAME: SequenceNumber - 22: + 24: UnexpectedTransactionIndex: UNIT - 23: + 25: CertificateNotfound: STRUCT: - certificate_digest: TYPENAME: TransactionDigest - 24: + 26: ParentNotfound: STRUCT: - object_id: TYPENAME: ObjectID - sequence: TYPENAME: SequenceNumber - 25: + 27: UnknownSenderAccount: UNIT - 26: + 28: CertificateAuthorityReuse: UNIT - 27: + 29: InvalidSequenceNumber: UNIT - 28: + 30: SequenceOverflow: UNIT - 29: + 31: SequenceUnderflow: UNIT - 30: + 32: WrongShard: UNIT - 31: + 33: InvalidCrossShardUpdate: UNIT - 32: + 34: InvalidAuthenticator: UNIT - 33: + 35: InvalidAddress: UNIT - 34: + 36: InvalidTransactionDigest: UNIT - 35: + 37: InvalidObjectDigest: STRUCT: - object_id: TYPENAME: ObjectID - expected_digest: TYPENAME: ObjectDigest - 36: + 38: InvalidDecoding: UNIT - 37: + 39: UnexpectedMessage: UNIT - 38: + 40: DuplicateObjectRefInput: UNIT - 39: + 41: ClientIoError: STRUCT: - error: STR - 40: + 42: TransferImmutableError: UNIT - 41: + 43: TooManyItemsError: NEWTYPE: U64 - 42: + 44: InvalidSequenceRangeError: UNIT - 43: + 45: NoBatchesFoundError: UNIT - 44: + 46: CannotSendClientMessageError: UNIT - 45: + 47: SubscriptionItemsDropedError: NEWTYPE: U64 - 46: + 48: SubscriptionServiceClosed: UNIT - 47: + 49: ModuleLoadFailure: STRUCT: - error: STR - 48: + 50: ModuleVerificationFailure: STRUCT: - error: STR - 49: + 51: ModuleDeserializationFailure: STRUCT: - error: STR - 50: + 52: ModulePublishFailure: STRUCT: - error: STR - 51: + 53: ModuleBuildFailure: STRUCT: - error: STR - 52: + 54: DependentPackageNotFound: STRUCT: - package_id: TYPENAME: ObjectID - 53: + 55: MoveUnitTestFailure: STRUCT: - error: STR - 54: + 56: FunctionNotFound: STRUCT: - error: STR - 55: + 57: ModuleNotFound: STRUCT: - module_name: STR - 56: + 58: InvalidFunctionSignature: STRUCT: - error: STR - 57: + 59: TypeError: STRUCT: - error: STR - 58: + 60: AbortedExecution: STRUCT: - error: STR - 59: + 61: InvalidMoveEvent: STRUCT: - error: STR - 60: + 62: CircularObjectOwnership: UNIT - 61: + 63: GasBudgetTooHigh: STRUCT: - error: STR - 62: + 64: InsufficientGas: STRUCT: - error: STR - 63: + 65: InvalidTxUpdate: UNIT - 64: + 66: TransactionLockExists: UNIT - 65: + 67: TransactionLockDoesNotExist: UNIT - 66: + 68: TransactionLockReset: UNIT - 67: + 69: ObjectNotFound: STRUCT: - object_id: TYPENAME: ObjectID - 68: + 70: ObjectDeleted: STRUCT: - object_ref: @@ -569,26 +573,26 @@ SuiError: - TYPENAME: ObjectID - TYPENAME: SequenceNumber - TYPENAME: ObjectDigest - 69: + 71: BadObjectType: STRUCT: - error: STR - 70: + 72: MoveExecutionFailure: UNIT - 71: + 73: ObjectInputArityViolation: UNIT - 72: + 74: ExecutionInvariantViolation: UNIT - 73: + 75: AuthorityInformationUnavailable: UNIT - 74: + 76: AuthorityUpdateFailure: UNIT - 75: + 77: ByzantineAuthoritySuspicion: STRUCT: - authority: TYPENAME: PublicKeyBytes - 76: + 78: PairwiseSyncFailed: STRUCT: - xsource: @@ -599,33 +603,33 @@ SuiError: TYPENAME: TransactionDigest - error: TYPENAME: SuiError - 77: + 79: StorageError: NEWTYPE: TYPENAME: TypedStoreError - 78: + 80: BatchErrorSender: UNIT - 79: + 81: GenericAuthorityError: STRUCT: - error: STR - 80: + 82: QuorumNotReached: STRUCT: - errors: SEQ: TYPENAME: SuiError - 81: + 83: ObjectSerializationError: UNIT - 82: + 84: ConcurrentTransactionError: UNIT - 83: + 85: IncorrectRecipientError: UNIT - 84: + 86: TooManyIncorrectAuthorities: UNIT - 85: + 87: IncorrectGasSplit: UNIT - 86: + 88: IncorrectGasMerge: UNIT Transaction: STRUCT: diff --git a/sui_types/src/error.rs b/sui_types/src/error.rs index 38a2be551b1d3..1abca3924e1ec 100644 --- a/sui_types/src/error.rs +++ b/sui_types/src/error.rs @@ -45,8 +45,12 @@ pub enum SuiError { UnexpectedOwnerType, #[error("Shared mutable object not yet supported")] UnsupportedSharedObjectError, + #[error("Object used as shared is not shared.")] + NotSharedObjectError, #[error("An object that's owned by another object cannot be deleted or wrapped. It must be transerred to an account address first before deletion")] DeleteObjectOwnedObject, + #[error("The shared locks for this transaction have not yet been set.")] + SharedObjectLockNotSetObject, // Signature verification #[error("Signature is not valid: {}", error)] diff --git a/sui_types/src/object.rs b/sui_types/src/object.rs index ad7ee757ddc7b..95831c798f931 100644 --- a/sui_types/src/object.rs +++ b/sui_types/src/object.rs @@ -275,6 +275,10 @@ impl Owner { Owner::SharedMutable | Owner::SharedImmutable => true, } } + + pub fn is_shared_mutable(&self) -> bool { + matches!(self, Owner::SharedMutable) + } } impl std::cmp::PartialEq for Owner {