Skip to content

Commit 357dbe6

Browse files
authored
Rename contains_shared_object to is_consensus_tx (#22492)
## Description We will no longer be able to simply use shared objects to determine whether a transaction is a consensus transaction, due to withdraws. Removing this interface to make it less error prone. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
1 parent 847b3a0 commit 357dbe6

18 files changed

Lines changed: 62 additions & 94 deletions

File tree

crates/sui-core/src/authority.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,7 @@ impl AuthorityState {
12811281
transaction: &VerifiedExecutableTransaction,
12821282
epoch_store: &Arc<AuthorityPerEpochStore>,
12831283
) -> SuiResult<TransactionEffects> {
1284-
let _metrics_guard = if transaction.contains_shared_object() {
1284+
let _metrics_guard = if transaction.is_consensus_tx() {
12851285
self.metrics
12861286
.execute_certificate_latency_shared_object
12871287
.start_timer()
@@ -1294,8 +1294,7 @@ impl AuthorityState {
12941294

12951295
self.metrics.total_cert_attempts.inc();
12961296

1297-
// TODO(fastpath): use a separate function to check if a transaction should be executed in fastpath.
1298-
if !transaction.contains_shared_object() {
1297+
if !transaction.is_consensus_tx() {
12991298
// Shared object transactions need to be sequenced by the consensus before enqueueing
13001299
// for execution, done in AuthorityPerEpochStore::handle_consensus_transaction().
13011300
// For owned object transactions, they can be enqueued for execution immediately.
@@ -1367,7 +1366,7 @@ impl AuthorityState {
13671366

13681367
let tx_cache_reader = self.get_transaction_cache_reader();
13691368
if epoch_store.protocol_config().mysticeti_fastpath()
1370-
&& !certificate.contains_shared_object()
1369+
&& !certificate.is_consensus_tx()
13711370
&& execution_env.scheduling_source == SchedulingSource::NonFastPath
13721371
{
13731372
// If this transaction is not scheduled from fastpath, it must be either

crates/sui-core/src/authority/authority_per_epoch_store.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4230,7 +4230,7 @@ impl AuthorityPerEpochStore {
42304230
}
42314231

42324232
// If needed we can support owned object system transactions as well...
4233-
assert!(system_transaction.contains_shared_object());
4233+
assert!(system_transaction.is_consensus_tx());
42344234
ConsensusCertificateResult::SuiTransaction(system_transaction.clone())
42354235
}
42364236

@@ -4345,10 +4345,8 @@ impl AuthorityPerEpochStore {
43454345
}
43464346

43474347
// This certificate will be scheduled. Update object execution cost.
4348-
if transaction.contains_shared_object() {
4349-
shared_object_congestion_tracker
4350-
.bump_object_execution_cost(execution_time_estimator, &transaction);
4351-
}
4348+
shared_object_congestion_tracker
4349+
.bump_object_execution_cost(execution_time_estimator, &transaction);
43524350

43534351
Ok(ConsensusCertificateResult::SuiTransaction(transaction))
43544352
}

crates/sui-core/src/authority/shared_object_congestion_tracker.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,10 @@ impl SharedObjectCongestionTracker {
151151
trace!(
152152
"created SharedObjectCongestionTracker with
153153
{} initial object debts,
154-
mode: {mode:?},
155-
max_accumulated_txn_cost_per_object_in_commit: {max_accumulated_txn_cost_per_object_in_commit:?},
156-
gas_budget_based_txn_cost_cap_factor: {gas_budget_based_txn_cost_cap_factor:?},
157-
gas_budget_based_txn_cost_absolute_cap: {gas_budget_based_txn_cost_absolute_cap:?},
154+
mode: {mode:?},
155+
max_accumulated_txn_cost_per_object_in_commit: {max_accumulated_txn_cost_per_object_in_commit:?},
156+
gas_budget_based_txn_cost_cap_factor: {gas_budget_based_txn_cost_cap_factor:?},
157+
gas_budget_based_txn_cost_absolute_cap: {gas_budget_based_txn_cost_absolute_cap:?},
158158
max_txn_cost_overage_per_object_in_commit: {max_txn_cost_overage_per_object_in_commit:?}",
159159
object_execution_cost.len(),
160160
);
@@ -260,7 +260,7 @@ impl SharedObjectCongestionTracker {
260260

261261
let shared_input_objects: Vec<_> = cert.shared_input_objects().collect();
262262
if shared_input_objects.is_empty() {
263-
// This is an owned object only transaction. No need to defer.
263+
// No shared object used by this transaction. No need to defer.
264264
return None;
265265
}
266266
let start_cost = self.compute_tx_start_at_cost(&shared_input_objects);
@@ -281,7 +281,7 @@ impl SharedObjectCongestionTracker {
281281
// Note that the congested objects here may be caused by transaction dependency of other congested objects.
282282
// Consider in a consensus commit, there are many transactions touching object A, and later in processing the
283283
// consensus commit, there is a transaction touching both object A and B. Although there are fewer transactions
284-
// touching object B, becase it's starting execution is delayed due to dependency to other transactions on
284+
// touching object B, because it's starting execution is delayed due to dependency to other transactions on
285285
// object A, it may be shown up as congested objects.
286286
let mut congested_objects = vec![];
287287
for obj in shared_input_objects {
@@ -319,11 +319,15 @@ impl SharedObjectCongestionTracker {
319319
execution_time_estimator: Option<&ExecutionTimeEstimator>,
320320
cert: &VerifiedExecutableTransaction,
321321
) {
322+
let shared_input_objects: Vec<_> = cert.shared_input_objects().collect();
323+
if shared_input_objects.is_empty() {
324+
return;
325+
}
326+
322327
let Some(tx_cost) = self.get_tx_cost(execution_time_estimator, cert) else {
323328
return;
324329
};
325330

326-
let shared_input_objects: Vec<_> = cert.shared_input_objects().collect();
327331
let start_cost = self.compute_tx_start_at_cost(&shared_input_objects);
328332
let end_cost = start_cost.saturating_add(tx_cost);
329333

crates/sui-core/src/authority/shared_object_version_manager.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,6 @@ impl<T> Schedulable<T> {
117117
}
118118
}
119119

120-
pub fn contains_shared_object(&self) -> bool
121-
where
122-
T: AsTx,
123-
{
124-
match self {
125-
Schedulable::Transaction(tx) => tx.as_tx().contains_shared_object(),
126-
Schedulable::RandomnessStateUpdate(_, _) => true,
127-
}
128-
}
129-
130120
pub fn non_shared_input_object_keys(&self) -> Vec<ObjectKey>
131121
where
132122
T: AsTx,
@@ -187,9 +177,6 @@ impl SharedObjVerManager {
187177
)?;
188178
let mut assigned_versions = Vec::new();
189179
for assignable in assignables {
190-
if !assignable.contains_shared_object() {
191-
continue;
192-
}
193180
let cert_assigned_versions = Self::assign_versions_for_certificate(
194181
epoch_store,
195182
assignable,
@@ -260,6 +247,12 @@ impl SharedObjVerManager {
260247
shared_input_next_versions: &mut HashMap<ConsensusObjectSequenceKey, SequenceNumber>,
261248
cancelled_txns: &BTreeMap<TransactionDigest, CancelConsensusCertificateReason>,
262249
) -> AssignedVersions {
250+
let shared_input_objects: Vec<_> = assignable.shared_input_objects(epoch_store).collect();
251+
if shared_input_objects.is_empty() {
252+
// No shared object used by this transaction. No need to assign versions.
253+
return vec![];
254+
}
255+
263256
let tx_key = assignable.key();
264257

265258
// Check if the transaction is cancelled due to congestion.
@@ -276,9 +269,6 @@ impl SharedObjVerManager {
276269
};
277270
let txn_cancelled = cancellation_info.is_some();
278271

279-
// Make an iterator to update the locks of the transaction's shared objects.
280-
let shared_input_objects: Vec<_> = assignable.shared_input_objects(epoch_store).collect();
281-
282272
let mut input_object_keys = assignable.non_shared_input_object_keys();
283273
let mut assigned_versions = Vec::with_capacity(shared_input_objects.len());
284274
let mut is_mutable_input = Vec::with_capacity(shared_input_objects.len());

crates/sui-core/src/authority_server.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -638,13 +638,11 @@ impl ValidatorService {
638638
SuiError::FullNodeCantHandleCertificate.into()
639639
);
640640

641-
let shared_object_tx = certificates
642-
.iter()
643-
.any(|cert| cert.contains_shared_object());
641+
let is_consensus_tx = certificates.iter().any(|cert| cert.is_consensus_tx());
644642

645643
let metrics = if certificates.len() == 1 {
646644
if wait_for_effects {
647-
if shared_object_tx {
645+
if is_consensus_tx {
648646
&self.metrics.handle_certificate_consensus_latency
649647
} else {
650648
&self.metrics.handle_certificate_non_consensus_latency
@@ -853,11 +851,11 @@ impl ValidatorService {
853851
if !wait_for_effects {
854852
// It is useful to enqueue owned object transaction for execution locally,
855853
// even when we are not returning effects to user
856-
let certificates_without_shared_objects = consensus_transactions
854+
let fast_path_certificates = consensus_transactions
857855
.iter()
858856
.filter_map(|tx| {
859857
if let ConsensusTransactionKind::CertifiedTransaction(certificate) = &tx.kind {
860-
(!certificate.contains_shared_object())
858+
(!certificate.is_consensus_tx())
861859
// Certificates already verified by callers of this function.
862860
.then_some((
863861
VerifiedExecutableTransaction::new_from_certificate(
@@ -872,10 +870,10 @@ impl ValidatorService {
872870
})
873871
.map(|(tx, env)| (Schedulable::Transaction(tx), env))
874872
.collect::<Vec<_>>();
875-
if !certificates_without_shared_objects.is_empty() {
873+
if !fast_path_certificates.is_empty() {
876874
self.state
877875
.execution_scheduler()
878-
.enqueue(certificates_without_shared_objects, epoch_store);
876+
.enqueue(fast_path_certificates, epoch_store);
879877
}
880878
return Ok((None, Weight::zero()));
881879
}
@@ -1300,7 +1298,7 @@ impl ValidatorService {
13001298
for certificate in certificates {
13011299
let tx_digest = *certificate.digest();
13021300
fp_ensure!(
1303-
certificate.contains_shared_object(),
1301+
certificate.is_consensus_tx(),
13041302
SuiError::UserInputError {
13051303
error: UserInputError::NoSharedObjectError { digest: tx_digest }
13061304
}

crates/sui-core/src/consensus_handler.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ impl<C> ConsensusHandler<C> {
10051005
pub(crate) fn classify(transaction: &ConsensusTransaction) -> &'static str {
10061006
match &transaction.kind {
10071007
ConsensusTransactionKind::CertifiedTransaction(certificate) => {
1008-
if certificate.contains_shared_object() {
1008+
if certificate.is_consensus_tx() {
10091009
"shared_certificate"
10101010
} else {
10111011
"owned_certificate"
@@ -1020,7 +1020,7 @@ pub(crate) fn classify(transaction: &ConsensusTransaction) -> &'static str {
10201020
ConsensusTransactionKind::RandomnessDkgMessage(_, _) => "randomness_dkg_message",
10211021
ConsensusTransactionKind::RandomnessDkgConfirmation(_, _) => "randomness_dkg_confirmation",
10221022
ConsensusTransactionKind::UserTransaction(tx) => {
1023-
if tx.contains_shared_object() {
1023+
if tx.is_consensus_tx() {
10241024
"shared_user_transaction"
10251025
} else {
10261026
"owned_user_transaction"
@@ -1202,17 +1202,17 @@ impl SequencedConsensusTransaction {
12021202
}
12031203
}
12041204

1205-
pub fn as_shared_object_txn(&self) -> Option<&SenderSignedData> {
1205+
pub fn as_consensus_txn(&self) -> Option<&SenderSignedData> {
12061206
match &self.transaction {
12071207
SequencedConsensusTransactionKind::External(ConsensusTransaction {
12081208
kind: ConsensusTransactionKind::CertifiedTransaction(certificate),
12091209
..
1210-
}) if certificate.contains_shared_object() => Some(certificate.data()),
1210+
}) if certificate.is_consensus_tx() => Some(certificate.data()),
12111211
SequencedConsensusTransactionKind::External(ConsensusTransaction {
12121212
kind: ConsensusTransactionKind::UserTransaction(txn),
12131213
..
1214-
}) if txn.contains_shared_object() => Some(txn.data()),
1215-
SequencedConsensusTransactionKind::System(txn) if txn.contains_shared_object() => {
1214+
}) if txn.is_consensus_tx() => Some(txn.data()),
1215+
SequencedConsensusTransactionKind::System(txn) if txn.is_consensus_tx() => {
12161216
Some(txn.data())
12171217
}
12181218
_ => None,
@@ -1317,8 +1317,7 @@ impl ConsensusBlockHandler {
13171317
.with_label_values(&["certified"])
13181318
.inc();
13191319
if let ConsensusTransactionKind::UserTransaction(tx) = parsed.transaction.kind {
1320-
// TODO(fastpath): use a separate function to check if a transaction should be executed in fastpath.
1321-
if tx.contains_shared_object() {
1320+
if tx.is_consensus_tx() {
13221321
continue;
13231322
}
13241323
let tx = VerifiedTransaction::new_unchecked(*tx);

crates/sui-core/src/mock_consensus.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl MockConsensusClient {
8888
};
8989
match &tx.kind {
9090
ConsensusTransactionKind::CertifiedTransaction(tx) => {
91-
if tx.contains_shared_object() {
91+
if tx.is_consensus_tx() {
9292
validator.execution_scheduler().enqueue(
9393
vec![(
9494
VerifiedExecutableTransaction::new_from_certificate(
@@ -102,7 +102,7 @@ impl MockConsensusClient {
102102
}
103103
}
104104
ConsensusTransactionKind::UserTransaction(tx) => {
105-
if tx.contains_shared_object() {
105+
if tx.is_consensus_tx() {
106106
validator.execution_scheduler().enqueue(
107107
vec![(
108108
VerifiedExecutableTransaction::new_from_consensus(

crates/sui-core/src/quorum_driver/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ where
596596
} = task;
597597
let transaction = &request.transaction;
598598
let tx_digest = *transaction.digest();
599-
let is_single_writer_tx = !transaction.contains_shared_object();
599+
let is_single_writer_tx = !transaction.is_consensus_tx();
600600

601601
let timer = Instant::now();
602602
let (tx_cert, newly_formed) = match tx_cert {

crates/sui-core/src/transaction_driver/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ where
7676
options: SubmitTransactionOptions,
7777
) -> Result<QuorumSubmitTransactionResponse, TransactionDriverError> {
7878
let tx_digest = request.transaction.digest();
79-
let is_single_writer_tx = !request.transaction.contains_shared_object();
79+
let is_single_writer_tx = !request.transaction.is_consensus_tx();
8080
let timer = Instant::now();
8181
loop {
8282
match self.submit_transaction_once(&request, &options).await {

crates/sui-core/src/transaction_orchestrator.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ where
236236
let tx_digest = *transaction.digest();
237237
debug!(?tx_digest, "TO Received transaction execution request.");
238238

239-
let (_e2e_latency_timer, _txn_finality_timer) = if transaction.contains_shared_object() {
239+
let (_e2e_latency_timer, _txn_finality_timer) = if transaction.is_consensus_tx() {
240240
(
241241
self.metrics.request_latency_shared_obj.start_timer(),
242242
self.metrics
@@ -354,7 +354,7 @@ where
354354
in_flight.dec();
355355
});
356356

357-
let _guard = if transaction.contains_shared_object() {
357+
let _guard = if transaction.is_consensus_tx() {
358358
metrics.local_execution_latency_shared_obj.start_timer()
359359
} else {
360360
metrics.local_execution_latency_single_writer.start_timer()
@@ -446,7 +446,7 @@ where
446446
&'_ self,
447447
transaction: &VerifiedTransaction,
448448
) -> (impl Drop, &'_ GenericCounter<AtomicU64>) {
449-
let (in_flight, good_response) = if transaction.contains_shared_object() {
449+
let (in_flight, good_response) = if transaction.is_consensus_tx() {
450450
self.metrics.total_req_received_shared_object.inc();
451451
(
452452
self.metrics.req_in_flight_shared_object.clone(),

0 commit comments

Comments
 (0)