Skip to content

Commit 0df7df9

Browse files
roman1e2f5p8spiotrm50
roman1e2f5p8s
authored and
piotrm50
committed
refactor(iota-core): safe renaming of certificate occurrences to transaction (#10767)
This PR renames internal identifiers used in the certificate-only era to certificate-less transaction names (the new names also stay clear enough in the certificate-only flow). This is a follow-up to the only two renamings made during the review process of #10615. See #10908 for the rationale. All changes are safe and backward-compatible as they are performed on internal identifiers - no changes on serialized types, cross-crate public APIs, or metric names. <details> <summary>Summary of renamings</summary> The following identifiers were renamed in modules: - **Types**: - `CertLockGuard` ➡️ `TxLockGuard` - `CertTxGuard` ➡️ `TxGuard` - **Methods**: - `finish_consensus_certificate_process` ➡️ `finish_consensus_transaction_process` - **Tracing spans**: - Label `cert_digest` ➡️ `tx_digest` on `try_schedule()` - **Tracing spans**: - Field `cert_digest` ➡️ `tx_digest` - **Methods**: - `assign_versions_for_certificate` ➡️ `assign_versions_for_transaction` - **Methods**: - `build_and_try_sequencing_certificate` ➡️ `build_and_try_sequencing_transaction` - `update_data_for_scheduled_certificate` ➡️ `update_data_for_scheduled_transaction` - **Methods**: - `process_certificate` ➡️ `process_transaction` - `commit_certificate` ➡️ `commit_transaction` - `prepare_certificate` ➡️ `prepare_transaction` - `prepare_certificate_for_benchmark` ➡️ `prepare_transaction_for_benchmark` - **Tracing spans**: - `process_certificate` ➡️ `process_transaction` - **Fail points**: - `correlated-crash-process-certificate` ➡️ `correlated-crash-process-transaction` - **Fail points**: - `correlated-crash-process-certificate` ➡️ `correlated-crash-process-transaction` - **Types**: - `PendingCertificate` ➡️ `PendingTransaction` - `PendingCertificateStats` ➡️ `PendingTransactionStats` - **Fields**: - `tx_ready_certificates` ➡️ `tx_ready_transactions` (in `TransactionManager`) - `certificate` ➡️ `transaction` (in now `PendingTransaction`) - `pending_certificates` ➡️ `pending_transactions` (in `Inner`) - `executing_certificates` ➡️ `.executing_transactions` (in `Inner`) - **Methods**: - `certificate_ready` ➡️ `transaction_ready` - **Tracing spans**: - `enqueueing_pending_certificate` ➡️ `enqueueing_pending_transaction` - Label `cert_digest` ➡️ `tx_digest` - **Tracing spans**: - `start_execute_pending_certs` ➡️ `start_execute_pending_transactions` - `executing_pending_cert` ➡️ `executing_pending_transaction` - Label `cert_epoch` ➡️ `tx_epoch` Also renamed variables, parameters, log messages, docstrings, and comments containing `cert` in all of the above modules and in `crates/iota-core/src/unit_tests/transaction_manager_tests.rs`. </details> Closes #10908. - [x] Basic tests (linting, compilation, formatting, unit/integration tests) - [ ] Patch-specific tests (correctness, functionality coverage) - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have checked that new and existing unit tests pass locally with my changes --------- Signed-off-by: Roman Overko <roman.overko@iota.org>
1 parent 96936e7 commit 0df7df9

12 files changed

Lines changed: 672 additions & 615 deletions

‎crates/iota-benchmark/tests/simtest.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ mod test {
169169

170170
register_fail_point_if("correlated-crash-after-consensus-commit-boundary", || true);
171171
// TODO: enable this - right now it causes rocksdb errors when re-opening DBs
172-
// register_fail_point_if("correlated-crash-process-certificate", || true);
172+
// register_fail_point_if("correlated-crash-process-transaction", || true);
173173

174174
let test_cluster = build_test_cluster(4, 10000, 1).await;
175175
test_simulated_load(test_cluster, 60).await;

‎crates/iota-core/src/authority.rs‎

Lines changed: 100 additions & 99 deletions
Large diffs are not rendered by default.

‎crates/iota-core/src/authority/authority_per_epoch_store.rs‎

Lines changed: 74 additions & 63 deletions
Large diffs are not rendered by default.

‎crates/iota-core/src/authority/shared_object_congestion_tracker.rs‎

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -372,22 +372,22 @@ impl SharedObjectCongestionTracker {
372372
/// be scheduled, this returns a `start_time`, and if it should be deferred,
373373
/// this returns the deferral key and the congested objects responsible for
374374
/// the deferral.
375-
#[instrument(level = "trace", skip_all, fields(cert_digest = ?cert.digest()))]
375+
#[instrument(level = "trace", skip_all, fields(tx_digest = ?transaction.digest()))]
376376
pub(super) fn try_schedule(
377377
&self,
378-
cert: &VerifiedExecutableTransaction,
378+
transaction: &VerifiedExecutableTransaction,
379379
previously_deferred_tx_digests: &PreviouslyDeferredTransactions,
380380
commit_round: CommitRound,
381381
) -> SequencingResult {
382382
let tx_duration = self
383383
.congestion_control_parameters
384-
.get_estimated_execution_duration(cert);
384+
.get_estimated_execution_duration(transaction);
385385
if tx_duration == 0 {
386386
// This is a zero-duration transaction, no need to defer.
387387
return SequencingResult::Schedule(0);
388388
}
389389

390-
let shared_input_objects = cert.shared_input_objects();
390+
let shared_input_objects = transaction.shared_input_objects();
391391
if shared_input_objects.is_empty() {
392392
// This is an owned object only transaction. No need to defer.
393393
return SequencingResult::Schedule(0);
@@ -445,7 +445,7 @@ impl SharedObjectCongestionTracker {
445445
assert!(!congested_objects.is_empty());
446446

447447
let deferral_key = if let Some(previous_key_suggested_gas_price_pair) =
448-
previously_deferred_tx_digests.get(cert.digest())
448+
previously_deferred_tx_digests.get(transaction.digest())
449449
{
450450
// This transaction has been deferred in previous consensus commit. Use its
451451
// previous deferred_from_round.
@@ -463,23 +463,23 @@ impl SharedObjectCongestionTracker {
463463
SequencingResult::Defer(deferral_key, congested_objects)
464464
}
465465

466-
/// Update shared objects' execution slots used in `cert` using `cert`'s
467-
/// estimated execution duration. This is called when `cert` is scheduled
468-
/// for execution.
466+
/// Update shared objects' execution slots used in `transaction` using
467+
/// `transaction`'s estimated execution duration. This is called when
468+
/// `transaction` is scheduled for execution.
469469
///
470470
/// `start_time` provides the start time of the execution slot assigned to
471-
/// `cert`.
471+
/// `transaction`.
472472
///
473-
/// Returns `Some(BumpObjectExecutionSlotsResult)` if `cert`'s estimated
474-
/// execution duration is non-zero, else returns `None`.
473+
/// Returns `Some(BumpObjectExecutionSlotsResult)` if `transaction`'s
474+
/// estimated execution duration is non-zero, else returns `None`.
475475
pub(super) fn bump_object_execution_slots(
476476
&mut self,
477-
cert: &VerifiedExecutableTransaction,
477+
transaction: &VerifiedExecutableTransaction,
478478
start_time: ExecutionTime,
479479
) -> Option<BumpObjectExecutionSlotsResult> {
480480
let estimated_execution_duration = self
481481
.congestion_control_parameters
482-
.get_estimated_execution_duration(cert);
482+
.get_estimated_execution_duration(transaction);
483483

484484
if estimated_execution_duration == 0 {
485485
return None;
@@ -489,7 +489,7 @@ impl SharedObjectCongestionTracker {
489489
let occupied_slot = ExecutionSlot::new(start_time, end_time);
490490

491491
// Find IDs of shared objects for which execution slots should be bumped.
492-
let object_ids = cert
492+
let object_ids = transaction
493493
.shared_input_objects()
494494
.into_iter()
495495
.filter_map(|obj| obj.mutable.then_some(obj.id))
@@ -506,7 +506,7 @@ impl SharedObjectCongestionTracker {
506506
object_ids,
507507
start_time,
508508
estimated_execution_duration,
509-
cert.transaction_data().gas_price(),
509+
transaction.transaction_data().gas_price(),
510510
))
511511
}
512512

@@ -771,8 +771,8 @@ pub mod shared_object_test_utils {
771771

772772
pub const TEST_ONLY_GAS_PRICE: u64 = 1_000;
773773

774-
/// Builds a certificate with a list of shared objects and their mutability.
775-
/// The certificate is only used to test the
774+
/// Builds a transaction with a list of shared objects and their mutability.
775+
/// The transaction is only used to test the
776776
/// `SharedObjectCongestionTracker` functions, therefore the content
777777
/// other than shared inputs, gas budget and gas price are not
778778
/// important.
@@ -819,14 +819,14 @@ pub mod shared_object_test_utils {
819819

820820
pub(super) fn initialize_tracker_and_try_schedule(
821821
shared_object_congestion_tracker: &mut SharedObjectCongestionTracker,
822-
cert: &VerifiedExecutableTransaction,
822+
transaction: &VerifiedExecutableTransaction,
823823
previously_deferred_tx_digests: &PreviouslyDeferredTransactions,
824824
commit_round: CommitRound,
825825
) -> SequencingResult {
826-
let shared_input_objects = cert.shared_input_objects();
826+
let shared_input_objects = transaction.shared_input_objects();
827827
shared_object_congestion_tracker.initialize_object_execution_slots(&shared_input_objects);
828828
shared_object_congestion_tracker.try_schedule(
829-
cert,
829+
transaction,
830830
previously_deferred_tx_digests,
831831
commit_round,
832832
)
@@ -1367,22 +1367,22 @@ mod object_cost_tests {
13671367
);
13681368

13691369
// Read two objects should not change the object execution slots.
1370-
let cert = build_transaction(
1370+
let transaction = build_transaction(
13711371
&[(object_id_0, false), (object_id_1, false)],
13721372
10,
13731373
TEST_ONLY_GAS_PRICE,
13741374
);
1375-
let cert_duration = shared_object_congestion_tracker
1375+
let tx_duration = shared_object_congestion_tracker
13761376
.congestion_control_parameters
1377-
.get_estimated_execution_duration(&cert);
1377+
.get_estimated_execution_duration(&transaction);
13781378
let start_time = initialize_tracker_and_compute_tx_start_time(
13791379
&mut shared_object_congestion_tracker,
1380-
&cert.shared_input_objects(),
1381-
cert_duration,
1380+
&transaction.shared_input_objects(),
1381+
tx_duration,
13821382
)
13831383
.expect("start time should be computable");
13841384

1385-
shared_object_congestion_tracker.bump_object_execution_slots(&cert, start_time);
1385+
shared_object_congestion_tracker.bump_object_execution_slots(&transaction, start_time);
13861386
assert_eq!(
13871387
shared_object_congestion_tracker,
13881388
new_congestion_tracker_with_initial_value_for_test(
@@ -1397,21 +1397,21 @@ mod object_cost_tests {
13971397

13981398
// Write to object 0 should only bump object 0's execution slots. The start time
13991399
// should be object 1's duration.
1400-
let cert = build_transaction(
1400+
let transaction = build_transaction(
14011401
&[(object_id_0, true), (object_id_1, false)],
14021402
10,
14031403
TEST_ONLY_GAS_PRICE,
14041404
);
1405-
let cert_duration = shared_object_congestion_tracker
1405+
let tx_duration = shared_object_congestion_tracker
14061406
.congestion_control_parameters
1407-
.get_estimated_execution_duration(&cert);
1407+
.get_estimated_execution_duration(&transaction);
14081408
let start_time = initialize_tracker_and_compute_tx_start_time(
14091409
&mut shared_object_congestion_tracker,
1410-
&cert.shared_input_objects(),
1411-
cert_duration,
1410+
&transaction.shared_input_objects(),
1411+
tx_duration,
14121412
)
14131413
.expect("start time should be computable");
1414-
shared_object_congestion_tracker.bump_object_execution_slots(&cert, start_time);
1414+
shared_object_congestion_tracker.bump_object_execution_slots(&transaction, start_time);
14151415
let expected_object_0_duration = match mode {
14161416
PerObjectCongestionControlMode::None => unreachable!(),
14171417
PerObjectCongestionControlMode::TotalGasBudget => 20,
@@ -1440,7 +1440,7 @@ mod object_cost_tests {
14401440

14411441
// Write to all objects should bump all objects' execution durations, including
14421442
// objects that are seen for the first time.
1443-
let cert = build_transaction(
1443+
let transaction = build_transaction(
14441444
&[
14451445
(object_id_0, true),
14461446
(object_id_1, true),
@@ -1454,16 +1454,16 @@ mod object_cost_tests {
14541454
PerObjectCongestionControlMode::TotalGasBudget => 30,
14551455
PerObjectCongestionControlMode::TotalTxCount => 12,
14561456
};
1457-
let cert_duration = shared_object_congestion_tracker
1457+
let tx_duration = shared_object_congestion_tracker
14581458
.congestion_control_parameters
1459-
.get_estimated_execution_duration(&cert);
1459+
.get_estimated_execution_duration(&transaction);
14601460
let start_time = initialize_tracker_and_compute_tx_start_time(
14611461
&mut shared_object_congestion_tracker,
1462-
&cert.shared_input_objects(),
1463-
cert_duration,
1462+
&transaction.shared_input_objects(),
1463+
tx_duration,
14641464
)
14651465
.expect("start time should be computable");
1466-
shared_object_congestion_tracker.bump_object_execution_slots(&cert, start_time);
1466+
shared_object_congestion_tracker.bump_object_execution_slots(&transaction, start_time);
14671467
assert_eq!(
14681468
shared_object_congestion_tracker
14691469
.object_execution_slots
@@ -1587,14 +1587,14 @@ mod object_cost_tests {
15871587
panic!("transaction is congesting, should defer");
15881588
}
15891589

1590-
let cert_duration = shared_object_congestion_tracker
1590+
let tx_duration = shared_object_congestion_tracker
15911591
.congestion_control_parameters
15921592
.get_estimated_execution_duration(&tx);
15931593
assert!(
15941594
initialize_tracker_and_compute_tx_start_time(
15951595
&mut shared_object_congestion_tracker,
15961596
&tx.shared_input_objects(),
1597-
cert_duration,
1597+
tx_duration,
15981598
)
15991599
.is_none()
16001600
);
@@ -1641,14 +1641,14 @@ mod object_cost_tests {
16411641
panic!("case 2: object 2 is congested, should defer");
16421642
}
16431643

1644-
let cert_duration = shared_object_congestion_tracker
1644+
let tx_duration = shared_object_congestion_tracker
16451645
.congestion_control_parameters
16461646
.get_estimated_execution_duration(&tx);
16471647
assert!(
16481648
initialize_tracker_and_compute_tx_start_time(
16491649
&mut shared_object_congestion_tracker,
16501650
&tx.shared_input_objects(),
1651-
cert_duration,
1651+
tx_duration,
16521652
)
16531653
.is_none()
16541654
);
@@ -1679,14 +1679,14 @@ mod object_cost_tests {
16791679
panic!("case 3: object 0 is congested, should defer");
16801680
}
16811681

1682-
let cert_duration = shared_object_congestion_tracker
1682+
let tx_duration = shared_object_congestion_tracker
16831683
.congestion_control_parameters
16841684
.get_estimated_execution_duration(&tx);
16851685
assert!(
16861686
initialize_tracker_and_compute_tx_start_time(
16871687
&mut shared_object_congestion_tracker,
16881688
&tx.shared_input_objects(),
1689-
cert_duration,
1689+
tx_duration,
16901690
)
16911691
.is_none()
16921692
);

0 commit comments

Comments
 (0)