Skip to content

Commit

Permalink
Create a separate congestion control consensus commit limit for Mysti…
Browse files Browse the repository at this point in the history
…ceti (#18648)

## Description 

Since mysticeti has different commit rate than Narwhal, we want to use
different transaction count limit for shared
object congestion control in consensus commit. Therefore, I created a
different config for mysticeti, and
the consensus handler will choose to use different limit based on the
current consensus algorithm.

We also turn on shared object congestion control on testnet.

## Test plan 

simtest updated
cluster testing

---

## 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): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Eugene Boguslavsky <[email protected]>
  • Loading branch information
halfprice and ebmifa authored Jul 19, 2024
1 parent b677505 commit 4dffddb
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 41 deletions.
22 changes: 14 additions & 8 deletions crates/sui-benchmark/tests/simtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,15 +466,21 @@ mod test {
config.set_per_object_congestion_control_mode_for_testing(mode);
match mode {
PerObjectCongestionControlMode::None => panic!("Congestion control mode cannot be None in test_simulated_load_shared_object_congestion_control"),
PerObjectCongestionControlMode::TotalGasBudget =>
config.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(
checkpoint_budget_factor
PerObjectCongestionControlMode::TotalGasBudget => {
let total_gas_limit = checkpoint_budget_factor
* DEFAULT_VALIDATOR_GAS_PRICE
* TEST_ONLY_GAS_UNIT_FOR_HEAVY_COMPUTATION_STORAGE,
),
PerObjectCongestionControlMode::TotalTxCount => config.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(
txn_count_limit
),
* TEST_ONLY_GAS_UNIT_FOR_HEAVY_COMPUTATION_STORAGE;
config.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(total_gas_limit);
config.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(total_gas_limit);
},
PerObjectCongestionControlMode::TotalTxCount => {
config.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(
txn_count_limit
);
config.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(
txn_count_limit
);
},
}
config.set_max_deferral_rounds_for_congestion_control_for_testing(max_deferral_rounds);
config
Expand Down
21 changes: 16 additions & 5 deletions crates/sui-core/src/authority/authority_per_epoch_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque};
use std::future::Future;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use sui_config::node::ExpensiveSafetyCheckConfig;
use sui_config::node::{ConsensusProtocol, ExpensiveSafetyCheckConfig};
use sui_macros::fail_point_arg;
use sui_types::accumulator::Accumulator;
use sui_types::authenticator_state::{get_authenticator_state, ActiveJwk};
Expand Down Expand Up @@ -66,6 +66,7 @@ use crate::consensus_handler::{
ConsensusCommitInfo, SequencedConsensusTransaction, SequencedConsensusTransactionKey,
SequencedConsensusTransactionKind, VerifiedSequencedConsensusTransaction,
};
use crate::consensus_manager::ConsensusManager;
use crate::epoch::epoch_metrics::EpochMetrics;
use crate::epoch::randomness::{
DkgStatus, RandomnessManager, RandomnessReporter, VersionedProcessedMessage,
Expand Down Expand Up @@ -1748,6 +1749,17 @@ impl AuthorityPerEpochStore {
.collect::<Result<Vec<_>, _>>()?)
}

fn get_max_accumulated_txn_cost_per_object_in_commit(&self) -> Option<u64> {
match ConsensusManager::get_consensus_protocol_in_epoch(self) {
ConsensusProtocol::Narwhal => self
.protocol_config()
.max_accumulated_txn_cost_per_object_in_narwhal_commit_as_option(),
ConsensusProtocol::Mysticeti => self
.protocol_config()
.max_accumulated_txn_cost_per_object_in_mysticeti_commit_as_option(),
}
}

fn should_defer(
&self,
cert: &VerifiedExecutableTransaction,
Expand All @@ -1774,15 +1786,14 @@ impl AuthorityPerEpochStore {
));
}

if let Some(max_accumulated_txn_cost_per_object_in_checkpoint) = self
.protocol_config()
.max_accumulated_txn_cost_per_object_in_checkpoint_as_option()
if let Some(max_accumulated_txn_cost_per_object_in_commit) =
self.get_max_accumulated_txn_cost_per_object_in_commit()
{
// Defer transaction if it uses shared objects that are congested.
if let Some((deferral_key, congested_objects)) = shared_object_congestion_tracker
.should_defer_due_to_object_congestion(
cert,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
previously_deferred_tx_digests,
commit_round,
)
Expand Down
22 changes: 11 additions & 11 deletions crates/sui-core/src/authority/shared_object_congestion_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl SharedObjectCongestionTracker {
pub fn should_defer_due_to_object_congestion(
&self,
cert: &VerifiedExecutableTransaction,
max_accumulated_txn_cost_per_object_in_checkpoint: u64,
max_accumulated_txn_cost_per_object_in_commit: u64,
previously_deferred_tx_digests: &HashMap<TransactionDigest, DeferralKey>,
commit_round: Round,
) -> Option<(DeferralKey, Vec<ObjectID>)> {
Expand All @@ -89,7 +89,7 @@ impl SharedObjectCongestionTracker {
}
let start_cost = self.compute_tx_start_at_cost(&shared_input_objects);

if start_cost + tx_cost <= max_accumulated_txn_cost_per_object_in_checkpoint {
if start_cost + tx_cost <= max_accumulated_txn_cost_per_object_in_commit {
return None;
}

Expand Down Expand Up @@ -273,8 +273,8 @@ mod object_cost_tests {

let tx_gas_budget = 100;

// Set max_accumulated_txn_cost_per_object_in_checkpoint to only allow 1 transaction to go through.
let max_accumulated_txn_cost_per_object_in_checkpoint = match mode {
// Set max_accumulated_txn_cost_per_object_in_commit to only allow 1 transaction to go through.
let max_accumulated_txn_cost_per_object_in_commit = match mode {
PerObjectCongestionControlMode::None => unreachable!(),
PerObjectCongestionControlMode::TotalGasBudget => tx_gas_budget + 1,
PerObjectCongestionControlMode::TotalTxCount => 2,
Expand Down Expand Up @@ -310,7 +310,7 @@ mod object_cost_tests {
if let Some((_, congested_objects)) = shared_object_congestion_tracker
.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&HashMap::new(),
0,
)
Expand All @@ -328,7 +328,7 @@ mod object_cost_tests {
assert!(shared_object_congestion_tracker
.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&HashMap::new(),
0,
)
Expand All @@ -345,7 +345,7 @@ mod object_cost_tests {
if let Some((_, congested_objects)) = shared_object_congestion_tracker
.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&HashMap::new(),
0,
)
Expand All @@ -370,7 +370,7 @@ mod object_cost_tests {
let shared_obj_0 = ObjectID::random();
let tx = build_transaction(&[(shared_obj_0, true)], 100);
// Make should_defer_due_to_object_congestion always defer transactions.
let max_accumulated_txn_cost_per_object_in_checkpoint = 0;
let max_accumulated_txn_cost_per_object_in_commit = 0;
let shared_object_congestion_tracker = SharedObjectCongestionTracker::new(mode);

// Insert a random pre-existing transaction.
Expand All @@ -392,7 +392,7 @@ mod object_cost_tests {
_,
)) = shared_object_congestion_tracker.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&previously_deferred_tx_digests,
10,
) {
Expand All @@ -419,7 +419,7 @@ mod object_cost_tests {
_,
)) = shared_object_congestion_tracker.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&previously_deferred_tx_digests,
10,
) {
Expand Down Expand Up @@ -447,7 +447,7 @@ mod object_cost_tests {
_,
)) = shared_object_congestion_tracker.should_defer_due_to_object_congestion(
&tx,
max_accumulated_txn_cost_per_object_in_checkpoint,
max_accumulated_txn_cost_per_object_in_commit,
&previously_deferred_tx_digests,
10,
) {
Expand Down
6 changes: 4 additions & 2 deletions crates/sui-core/src/consensus_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ impl ConsensusManager {
}

// Picks the consensus protocol based on the protocol config and the epoch.
fn pick_protocol(&self, epoch_store: &AuthorityPerEpochStore) -> ConsensusProtocol {
pub fn get_consensus_protocol_in_epoch(
epoch_store: &AuthorityPerEpochStore,
) -> ConsensusProtocol {
let protocol_config = epoch_store.protocol_config();
if protocol_config.version >= ProtocolVersion::new(36) {
if let Ok(consensus_choice) = std::env::var("CONSENSUS") {
Expand Down Expand Up @@ -205,7 +207,7 @@ impl ConsensusManagerTrait for ConsensusManager {
"Cannot start consensus. ConsensusManager protocol {index} is already running"
);
});
let protocol = self.pick_protocol(&epoch_store);
let protocol = Self::get_consensus_protocol_in_epoch(&epoch_store);
info!("Starting consensus protocol {protocol:?} ...");
match protocol {
ConsensusProtocol::Narwhal => {
Expand Down
16 changes: 13 additions & 3 deletions crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5812,10 +5812,17 @@ async fn test_consensus_handler_per_object_congestion_control(
PerObjectCongestionControlMode::None => unreachable!(),
PerObjectCongestionControlMode::TotalGasBudget => {
protocol_config
.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(200_000_000);
.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(200_000_000);
protocol_config
.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(
200_000_000,
);
}
PerObjectCongestionControlMode::TotalTxCount => {
protocol_config.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(2);
protocol_config
.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(2);
protocol_config
.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(2);
}
}
protocol_config.set_max_deferral_rounds_for_congestion_control_for_testing(1000); // Set to a large number so that we don't hit this limit.
Expand Down Expand Up @@ -6034,7 +6041,10 @@ async fn test_consensus_handler_congestion_control_transaction_cancellation() {
protocol_config.set_per_object_congestion_control_mode_for_testing(
PerObjectCongestionControlMode::TotalGasBudget,
);
protocol_config.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(100_000_000);
protocol_config
.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(100_000_000);
protocol_config
.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(100_000_000);
protocol_config.set_max_deferral_rounds_for_congestion_control_for_testing(2);
let authority = TestAuthorityBuilder::new()
.with_reference_gas_price(1000)
Expand Down
9 changes: 6 additions & 3 deletions crates/sui-core/src/unit_tests/congestion_control_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ impl TestSetup {
);

// Set shared object congestion control such that it only allows 1 transaction to go through.
let max_accumulated_txn_cost_per_object_in_checkpoint =
let max_accumulated_txn_cost_per_object_in_commit =
TEST_ONLY_GAS_PRICE * TEST_ONLY_GAS_UNIT;
protocol_config.set_max_accumulated_txn_cost_per_object_in_checkpoint_for_testing(
max_accumulated_txn_cost_per_object_in_checkpoint,
protocol_config.set_max_accumulated_txn_cost_per_object_in_narwhal_commit_for_testing(
max_accumulated_txn_cost_per_object_in_commit,
);
protocol_config.set_max_accumulated_txn_cost_per_object_in_mysticeti_commit_for_testing(
max_accumulated_txn_cost_per_object_in_commit,
);

// Set max deferral rounds to 0 to testr cancellation. All deferred transactions will be cancelled.
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,8 @@
"hmac_hmac_sha3_256_input_cost_per_byte": {
"u64": "2"
},
"max_accumulated_txn_cost_per_object_in_checkpoint": null,
"max_accumulated_txn_cost_per_object_in_mysticeti_commit": null,
"max_accumulated_txn_cost_per_object_in_narwhal_commit": null,
"max_age_of_jwk_in_epochs": null,
"max_arguments": {
"u32": "512"
Expand Down
27 changes: 23 additions & 4 deletions crates/sui-protocol-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ const MAX_PROTOCOL_VERSION: u64 = 53;
// Enable deny list v2 on testnet and mainnet.
// Version 53: Add feature flag to decide whether to attempt to finalize bridge committee
// Enable consensus commit prologue V3 on testnet.
// Turn on shared object congestion control in testnet.

#[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct ProtocolVersion(u64);
Expand Down Expand Up @@ -1147,9 +1148,11 @@ pub struct ProtocolConfig {
/// The maximum size of transactions included in a consensus proposed block
consensus_max_transactions_in_block_bytes: Option<u64>,

/// The max accumulated txn execution cost per object in a checkpoint. Transactions
/// The max accumulated txn execution cost per object in a Narwhal commit. Transactions
/// in a checkpoint will be deferred once their touch shared objects hit this limit.
max_accumulated_txn_cost_per_object_in_checkpoint: Option<u64>,
/// This config is meant to be used when consensus protocol is Narwhal, where each
/// consensus commit corresponding to 1 checkpoint (or 2 if randomness is enabled)
max_accumulated_txn_cost_per_object_in_narwhal_commit: Option<u64>,

/// The max number of consensus rounds a transaction can be deferred due to shared object congestion.
/// Transactions will be cancelled after this many rounds.
Expand All @@ -1168,6 +1171,12 @@ pub struct ProtocolConfig {
// Note: this is not a feature flag because we want to distinguish between
// `None` and `Some(false)`, as committee was already finalized on Testnet.
bridge_should_try_to_finalize_committee: Option<bool>,

/// The max accumulated txn execution cost per object in a mysticeti. Transactions
/// in a commit will be deferred once their touch shared objects hit this limit.
/// This config plays the same role as `max_accumulated_txn_cost_per_object_in_narwhal_commit`
/// but for mysticeti commits due to that mysticeti has higher commit rate.
max_accumulated_txn_cost_per_object_in_mysticeti_commit: Option<u64>,
}

// feature flags
Expand Down Expand Up @@ -1937,7 +1946,7 @@ impl ProtocolConfig {

consensus_max_transactions_in_block_bytes: None,

max_accumulated_txn_cost_per_object_in_checkpoint: None,
max_accumulated_txn_cost_per_object_in_narwhal_commit: None,

max_deferral_rounds_for_congestion_control: None,

Expand All @@ -1948,6 +1957,8 @@ impl ProtocolConfig {
max_soft_bundle_size: None,

bridge_should_try_to_finalize_committee: None,

max_accumulated_txn_cost_per_object_in_mysticeti_commit: None,
// When adding a new constant, set it to None in the earliest version, like this:
// new_constant: None,
};
Expand Down Expand Up @@ -2470,7 +2481,7 @@ impl ProtocolConfig {

// Turn on shared object congestion control in devnet.
if chain != Chain::Testnet && chain != Chain::Mainnet {
cfg.max_accumulated_txn_cost_per_object_in_checkpoint = Some(100);
cfg.max_accumulated_txn_cost_per_object_in_narwhal_commit = Some(100);
cfg.feature_flags.per_object_congestion_control_mode =
PerObjectCongestionControlMode::TotalTxCount;
}
Expand Down Expand Up @@ -2515,6 +2526,14 @@ impl ProtocolConfig {
if chain == Chain::Unknown {
cfg.feature_flags.authority_capabilities_v2 = true;
}

// Turns on shared object congestion control on testnet.
if chain != Chain::Mainnet {
cfg.max_accumulated_txn_cost_per_object_in_narwhal_commit = Some(100);
cfg.max_accumulated_txn_cost_per_object_in_mysticeti_commit = Some(10);
cfg.feature_flags.per_object_congestion_control_mode =
PerObjectCongestionControlMode::TotalTxCount;
}
}
// Use this template when making changes:
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ feature_flags:
enable_coin_deny_list: true
enable_group_ops_native_functions: true
reject_mutable_random_on_entry_functions: true
per_object_congestion_control_mode: TotalTxCount
consensus_choice: Mysticeti
consensus_network: Tonic
zklogin_max_epoch_upper_bound_delta: 30
Expand Down Expand Up @@ -282,9 +283,11 @@ random_beacon_min_round_interval_ms: 200
random_beacon_dkg_version: 1
consensus_max_transaction_size_bytes: 262144
consensus_max_transactions_in_block_bytes: 6291456
max_accumulated_txn_cost_per_object_in_narwhal_commit: 100
max_deferral_rounds_for_congestion_control: 10
min_checkpoint_interval_ms: 200
checkpoint_summary_version_specific_data: 1
max_soft_bundle_size: 5
bridge_should_try_to_finalize_committee: true
max_accumulated_txn_cost_per_object_in_mysticeti_commit: 10

Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ random_beacon_min_round_interval_ms: 200
random_beacon_dkg_version: 1
consensus_max_transaction_size_bytes: 262144
consensus_max_transactions_in_block_bytes: 6291456
max_accumulated_txn_cost_per_object_in_checkpoint: 100
max_accumulated_txn_cost_per_object_in_narwhal_commit: 100
max_deferral_rounds_for_congestion_control: 10
min_checkpoint_interval_ms: 200
checkpoint_summary_version_specific_data: 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,10 @@ random_beacon_min_round_interval_ms: 200
random_beacon_dkg_version: 1
consensus_max_transaction_size_bytes: 262144
consensus_max_transactions_in_block_bytes: 6291456
max_accumulated_txn_cost_per_object_in_checkpoint: 100
max_accumulated_txn_cost_per_object_in_narwhal_commit: 100
max_deferral_rounds_for_congestion_control: 10
min_checkpoint_interval_ms: 200
checkpoint_summary_version_specific_data: 1
max_soft_bundle_size: 5
bridge_should_try_to_finalize_committee: true

max_accumulated_txn_cost_per_object_in_mysticeti_commit: 10
2 changes: 1 addition & 1 deletion sdk/graphql-transport/src/methods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,7 @@ export const RPC_METHODS: {
const attributes: Record<string, ProtocolConfigValue | null> = {};

const configTypeMap: Record<string, string> = {
max_accumulated_txn_cost_per_object_in_checkpoint: 'u64',
max_accumulated_txn_cost_per_object_in_narwhal_commit: 'u64',
max_arguments: 'u32',
max_gas_payment_objects: 'u32',
max_modules_in_publish: 'u32',
Expand Down

0 comments on commit 4dffddb

Please sign in to comment.