Skip to content

Commit 74e8414

Browse files
mystenmarkmystenmark
authored
[Cherrypick] Refactor consensus commit prologue creation (#24529) (#24542)
- Refactor add_consensus_commit_prologue so that we don't have to recompute version assignments. - Enable include_cancelled_randomness_txns_in_prologue - Remove old version of process_transactions Note: This exposed a pre-existing bug where cancelled randomness transactions were not added to the commit prologue. The bug is fixed, along with a protocol flag.
1 parent 3395488 commit 74e8414

File tree

10 files changed

+106
-38
lines changed

10 files changed

+106
-38
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,6 +2965,7 @@ impl AuthorityPerEpochStore {
29652965
}
29662966
Schedulable::RandomnessStateUpdate(_, _) => None,
29672967
Schedulable::AccumulatorSettlement(_, _) => None,
2968+
Schedulable::ConsensusCommitPrologue(_, _, _) => None,
29682969
})
29692970
.collect();
29702971

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use std::collections::BTreeMap;
99
use std::collections::HashMap;
1010
use std::collections::HashSet;
1111
use sui_types::SUI_ACCUMULATOR_ROOT_OBJECT_ID;
12+
use sui_types::SUI_CLOCK_OBJECT_ID;
13+
use sui_types::SUI_CLOCK_OBJECT_SHARED_VERSION;
1214
use sui_types::base_types::ConsensusObjectSequenceKey;
1315
use sui_types::base_types::TransactionDigest;
1416
use sui_types::committee::EpochId;
@@ -79,6 +81,7 @@ pub enum Schedulable<T = VerifiedExecutableTransaction> {
7981
Transaction(T),
8082
RandomnessStateUpdate(EpochId, RandomnessRound),
8183
AccumulatorSettlement(EpochId, u64 /* checkpoint height */),
84+
ConsensusCommitPrologue(EpochId, u64 /* round */, u32 /* sub_dag_index */),
8285
}
8386

8487
impl From<VerifiedExecutableTransaction> for Schedulable<VerifiedExecutableTransaction> {
@@ -116,6 +119,9 @@ impl Schedulable<&'_ VerifiedExecutableTransaction> {
116119
Schedulable::AccumulatorSettlement(epoch, checkpoint_height) => {
117120
Schedulable::AccumulatorSettlement(*epoch, *checkpoint_height)
118121
}
122+
Schedulable::ConsensusCommitPrologue(epoch, round, sub_dag_index) => {
123+
Schedulable::ConsensusCommitPrologue(*epoch, *round, *sub_dag_index)
124+
}
119125
}
120126
}
121127
}
@@ -129,6 +135,7 @@ impl<T> Schedulable<T> {
129135
Schedulable::Transaction(tx) => Some(tx.as_tx()),
130136
Schedulable::RandomnessStateUpdate(_, _) => None,
131137
Schedulable::AccumulatorSettlement(_, _) => None,
138+
Schedulable::ConsensusCommitPrologue(_, _, _) => None,
132139
}
133140
}
134141

@@ -161,6 +168,13 @@ impl<T> Schedulable<T> {
161168
mutability: SharedObjectMutability::Mutable,
162169
}))
163170
}
171+
Schedulable::ConsensusCommitPrologue(_, _, _) => {
172+
Either::Right(std::iter::once(SharedInputObject {
173+
id: SUI_CLOCK_OBJECT_ID,
174+
initial_shared_version: SUI_CLOCK_OBJECT_SHARED_VERSION,
175+
mutability: SharedObjectMutability::Mutable,
176+
}))
177+
}
164178
}
165179
}
166180

@@ -173,6 +187,7 @@ impl<T> Schedulable<T> {
173187
.expect("Transaction input should have been verified"),
174188
Schedulable::RandomnessStateUpdate(_, _) => vec![],
175189
Schedulable::AccumulatorSettlement(_, _) => vec![],
190+
Schedulable::ConsensusCommitPrologue(_, _, _) => vec![],
176191
}
177192
}
178193

@@ -184,6 +199,7 @@ impl<T> Schedulable<T> {
184199
Schedulable::Transaction(tx) => transaction_receiving_object_keys(tx.as_tx()),
185200
Schedulable::RandomnessStateUpdate(_, _) => vec![],
186201
Schedulable::AccumulatorSettlement(_, _) => vec![],
202+
Schedulable::ConsensusCommitPrologue(_, _, _) => vec![],
187203
}
188204
}
189205

@@ -199,6 +215,9 @@ impl<T> Schedulable<T> {
199215
Schedulable::AccumulatorSettlement(epoch, checkpoint_height) => {
200216
TransactionKey::AccumulatorSettlement(*epoch, *checkpoint_height)
201217
}
218+
Schedulable::ConsensusCommitPrologue(epoch, round, sub_dag_index) => {
219+
TransactionKey::ConsensusCommitPrologue(*epoch, *round, *sub_dag_index)
220+
}
202221
}
203222
}
204223
}

‎crates/sui-core/src/consensus_handler.rs‎

Lines changed: 63 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use serde::{Deserialize, Serialize};
2929
use sui_macros::{fail_point, fail_point_arg, fail_point_if};
3030
use sui_protocol_config::ProtocolConfig;
3131
use sui_types::{
32+
SUI_RANDOMNESS_STATE_OBJECT_ID,
3233
authenticator_state::ActiveJwk,
3334
base_types::{
3435
AuthorityName, ConciseableName, ConsensusObjectSequenceKey, SequenceNumber,
@@ -44,7 +45,7 @@ use sui_types::{
4445
ExecutionTimeObservation,
4546
},
4647
sui_system_state::epoch_start_sui_system_state::EpochStartSystemStateTrait,
47-
transaction::{SenderSignedData, VerifiedCertificate, VerifiedTransaction},
48+
transaction::{SenderSignedData, TransactionKey, VerifiedCertificate, VerifiedTransaction},
4849
};
4950
use tokio::{sync::MutexGuard, task::JoinSet};
5051
use tracing::{debug, error, info, instrument, trace, warn};
@@ -62,7 +63,7 @@ use crate::{
6263
epoch_start_configuration::EpochStartConfigTrait,
6364
execution_time_estimator::ExecutionTimeEstimator,
6465
shared_object_congestion_tracker::SharedObjectCongestionTracker,
65-
shared_object_version_manager::{AssignedTxAndVersions, Schedulable, SharedObjVerManager},
66+
shared_object_version_manager::{AssignedTxAndVersions, Schedulable},
6667
transaction_deferral::{DeferralKey, DeferralReason, transaction_deferral_within_limit},
6768
},
6869
checkpoints::{
@@ -1230,22 +1231,23 @@ impl<C: CheckpointServiceNotify + Send + Sync> ConsensusHandler<C> {
12301231
}
12311232
}
12321233

1233-
let consensus_commit_prologue = self.add_consensus_commit_prologue_transaction(
1234-
state,
1235-
commit_info,
1236-
transactions_to_schedule
1237-
.iter()
1238-
.map(Schedulable::Transaction),
1239-
&cancelled_txns,
1240-
);
1234+
let consensus_commit_prologue = (!commit_info.skip_consensus_commit_prologue_in_test)
1235+
.then_some(Schedulable::ConsensusCommitPrologue(
1236+
epoch,
1237+
commit_info.round,
1238+
commit_info.consensus_commit_ref.index,
1239+
));
12411240

12421241
let schedulables: Vec<_> = itertools::chain!(
12431242
consensus_commit_prologue.into_iter(),
1244-
authenticator_state_update_transaction.into_iter(),
1245-
transactions_to_schedule.into_iter(),
1243+
authenticator_state_update_transaction
1244+
.into_iter()
1245+
.map(Schedulable::Transaction),
1246+
transactions_to_schedule
1247+
.into_iter()
1248+
.map(Schedulable::Transaction),
1249+
settlement,
12461250
)
1247-
.map(Schedulable::Transaction)
1248-
.chain(settlement)
12491251
.collect();
12501252

12511253
let randomness_schedulables: Vec<_> = randomness_state_update_transaction
@@ -1269,6 +1271,24 @@ impl<C: CheckpointServiceNotify + Send + Sync> ConsensusHandler<C> {
12691271
)
12701272
.expect("failed to assign shared object versions");
12711273

1274+
let consensus_commit_prologue =
1275+
self.add_consensus_commit_prologue_transaction(state, commit_info, &assigned_versions);
1276+
1277+
let mut schedulables = schedulables;
1278+
let mut assigned_versions = assigned_versions;
1279+
if let Some(consensus_commit_prologue) = consensus_commit_prologue {
1280+
assert!(matches!(
1281+
schedulables[0],
1282+
Schedulable::ConsensusCommitPrologue(..)
1283+
));
1284+
assert!(matches!(
1285+
assigned_versions.0[0].0,
1286+
TransactionKey::ConsensusCommitPrologue(..)
1287+
));
1288+
assigned_versions.0[0].0 = TransactionKey::Digest(*consensus_commit_prologue.digest());
1289+
schedulables[0] = Schedulable::Transaction(consensus_commit_prologue);
1290+
}
1291+
12721292
self.epoch_store
12731293
.process_user_signatures(schedulables.iter().chain(randomness_schedulables.iter()));
12741294

@@ -1282,35 +1302,40 @@ impl<C: CheckpointServiceNotify + Send + Sync> ConsensusHandler<C> {
12821302
&'a self,
12831303
state: &'a mut CommitHandlerState,
12841304
commit_info: &'a ConsensusCommitInfo,
1285-
schedulables: impl Iterator<Item = Schedulable<&'a VerifiedExecutableTransaction>>,
1286-
cancelled_txns: &'a BTreeMap<TransactionDigest, CancelConsensusCertificateReason>,
1305+
assigned_versions: &AssignedTxAndVersions,
12871306
) -> Option<VerifiedExecutableTransaction> {
12881307
{
12891308
if commit_info.skip_consensus_commit_prologue_in_test {
12901309
return None;
12911310
}
12921311
}
12931312

1294-
let mut version_assignment = Vec::new();
1295-
let mut shared_input_next_version = HashMap::new();
1296-
for txn in schedulables {
1297-
let key = txn.key();
1298-
match key.as_digest().and_then(|d| cancelled_txns.get(d)) {
1299-
Some(CancelConsensusCertificateReason::CongestionOnObjects(_))
1300-
| Some(CancelConsensusCertificateReason::DkgFailed) => {
1301-
assert_reachable!("cancelled transactions");
1302-
let assigned_versions = SharedObjVerManager::assign_versions_for_certificate(
1303-
&self.epoch_store,
1304-
&txn,
1305-
&mut shared_input_next_version,
1306-
cancelled_txns,
1307-
);
1308-
version_assignment.push((
1309-
*key.unwrap_digest(),
1310-
assigned_versions.shared_object_versions,
1311-
));
1312-
}
1313-
None => {}
1313+
let mut cancelled_txn_version_assignment = Vec::new();
1314+
1315+
let protocol_config = self.epoch_store.protocol_config();
1316+
1317+
for (txn_key, assigned_versions) in assigned_versions.0.iter() {
1318+
let Some(d) = txn_key.as_digest() else {
1319+
continue;
1320+
};
1321+
1322+
if !protocol_config.include_cancelled_randomness_txns_in_prologue()
1323+
&& assigned_versions
1324+
.shared_object_versions
1325+
.iter()
1326+
.any(|((id, _), _)| *id == SUI_RANDOMNESS_STATE_OBJECT_ID)
1327+
{
1328+
continue;
1329+
}
1330+
1331+
if assigned_versions
1332+
.shared_object_versions
1333+
.iter()
1334+
.any(|(_, version)| version.is_cancelled())
1335+
{
1336+
assert_reachable!("cancelled transactions");
1337+
cancelled_txn_version_assignment
1338+
.push((*d, assigned_versions.shared_object_versions.clone()));
13141339
}
13151340
}
13161341

@@ -1320,14 +1345,14 @@ impl<C: CheckpointServiceNotify + Send + Sync> ConsensusHandler<C> {
13201345
TransactionDigest,
13211346
Vec<(ConsensusObjectSequenceKey, SequenceNumber)>
13221347
)>| {
1323-
version_assignment.extend(additional_cancelled_txns);
1348+
cancelled_txn_version_assignment.extend(additional_cancelled_txns);
13241349
}
13251350
);
13261351

13271352
let transaction = commit_info.create_consensus_commit_prologue_transaction(
13281353
self.epoch_store.epoch(),
13291354
self.epoch_store.protocol_config(),
1330-
version_assignment,
1355+
cancelled_txn_version_assignment,
13311356
commit_info,
13321357
state.indirect_state_observer.take().unwrap(),
13331358
);

‎crates/sui-core/src/execution_scheduler/execution_scheduler_impl.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,12 @@ impl ExecutionScheduler {
519519
Schedulable::AccumulatorSettlement(_, _) => {
520520
settlement_txns.push((schedulable.key(), env));
521521
}
522+
Schedulable::ConsensusCommitPrologue(_, _, _) => {
523+
// we only use Schedulable::ConsensusCommitPrologue as a temporary placeholder
524+
// during version assignment, by the time we schedule transactions it should be
525+
// converted to Schedulable::Transaction
526+
unreachable!("Schedulable::ConsensusCommitPrologue should not be enqueued");
527+
}
522528
}
523529
}
524530

‎crates/sui-open-rpc/spec/openrpc.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,6 +1365,7 @@
13651365
"generate_df_type_layouts": false,
13661366
"hardened_otw_check": false,
13671367
"ignore_execution_time_observations_after_certs_closed": false,
1368+
"include_cancelled_randomness_txns_in_prologue": false,
13681369
"include_checkpoint_artifacts_digest_in_summary": false,
13691370
"include_consensus_digest_in_prologue": false,
13701371
"loaded_child_object_format": false,

‎crates/sui-protocol-config/src/lib.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,10 @@ struct FeatureFlags {
894894
// If true, skip GC'ed accept votes in CommitFinalizer.
895895
#[serde(skip_serializing_if = "is_false")]
896896
consensus_skip_gced_accept_votes: bool,
897+
898+
// If true, include cancelled randomness txns in the consensus commit prologue.
899+
#[serde(skip_serializing_if = "is_false")]
900+
include_cancelled_randomness_txns_in_prologue: bool,
897901
}
898902

899903
fn is_false(b: &bool) -> bool {
@@ -2408,6 +2412,11 @@ impl ProtocolConfig {
24082412
pub fn consensus_skip_gced_accept_votes(&self) -> bool {
24092413
self.feature_flags.consensus_skip_gced_accept_votes
24102414
}
2415+
2416+
pub fn include_cancelled_randomness_txns_in_prologue(&self) -> bool {
2417+
self.feature_flags
2418+
.include_cancelled_randomness_txns_in_prologue
2419+
}
24112420
}
24122421

24132422
#[cfg(not(msim))]
@@ -4293,6 +4302,9 @@ impl ProtocolConfig {
42934302
cfg.feature_flags
42944303
.enable_nitro_attestation_all_nonzero_pcrs_parsing = true;
42954304
}
4305+
4306+
cfg.feature_flags
4307+
.include_cancelled_randomness_txns_in_prologue = true;
42964308
}
42974309
// Use this template when making changes:
42984310
//

‎crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_104.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ feature_flags:
127127
private_generics_verifier_v2: true
128128
deprecate_global_storage_ops: true
129129
consensus_skip_gced_accept_votes: true
130+
include_cancelled_randomness_txns_in_prologue: true
130131
max_tx_size_bytes: 131072
131132
max_input_objects: 2048
132133
max_size_written_objects: 5000000

‎crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_104.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ feature_flags:
129129
private_generics_verifier_v2: true
130130
deprecate_global_storage_ops: true
131131
consensus_skip_gced_accept_votes: true
132+
include_cancelled_randomness_txns_in_prologue: true
132133
max_tx_size_bytes: 131072
133134
max_input_objects: 2048
134135
max_size_written_objects: 5000000

‎crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_104.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ feature_flags:
132132
private_generics_verifier_v2: true
133133
deprecate_global_storage_ops: true
134134
consensus_skip_gced_accept_votes: true
135+
include_cancelled_randomness_txns_in_prologue: true
135136
max_tx_size_bytes: 131072
136137
max_input_objects: 2048
137138
max_size_written_objects: 5000000

‎crates/sui-types/src/transaction.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4250,6 +4250,7 @@ pub enum TransactionKey {
42504250
Digest(TransactionDigest),
42514251
RandomnessRound(EpochId, RandomnessRound),
42524252
AccumulatorSettlement(EpochId, u64 /* checkpoint height */),
4253+
ConsensusCommitPrologue(EpochId, u64 /* round */, u32 /* sub_dag_index */),
42534254
}
42544255

42554256
impl TransactionKey {

0 commit comments

Comments
 (0)