From 4dffddbfb51b0aaa70dd35d06fdaec6e38557b29 Mon Sep 17 00:00:00 2001 From: Zhe Wu Date: Thu, 18 Jul 2024 18:02:40 -0700 Subject: [PATCH] Create a separate congestion control consensus commit limit for Mysticeti (#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 --- crates/sui-benchmark/tests/simtest.rs | 22 +++++++++------ .../authority/authority_per_epoch_store.rs | 21 +++++++++++---- .../shared_object_congestion_tracker.rs | 22 +++++++-------- crates/sui-core/src/consensus_manager/mod.rs | 6 +++-- .../src/unit_tests/authority_tests.rs | 16 ++++++++--- .../unit_tests/congestion_control_tests.rs | 9 ++++--- crates/sui-open-rpc/spec/openrpc.json | 3 ++- crates/sui-protocol-config/src/lib.rs | 27 ++++++++++++++++--- ...ocol_config__test__Testnet_version_53.snap | 3 +++ ...sui_protocol_config__test__version_52.snap | 2 +- ...sui_protocol_config__test__version_53.snap | 4 +-- sdk/graphql-transport/src/methods.ts | 2 +- 12 files changed, 96 insertions(+), 41 deletions(-) diff --git a/crates/sui-benchmark/tests/simtest.rs b/crates/sui-benchmark/tests/simtest.rs index f0d1dfa5cbe88..270a2fd6af878 100644 --- a/crates/sui-benchmark/tests/simtest.rs +++ b/crates/sui-benchmark/tests/simtest.rs @@ -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 diff --git a/crates/sui-core/src/authority/authority_per_epoch_store.rs b/crates/sui-core/src/authority/authority_per_epoch_store.rs index e9e10e17555fd..74dabea0ce9c4 100644 --- a/crates/sui-core/src/authority/authority_per_epoch_store.rs +++ b/crates/sui-core/src/authority/authority_per_epoch_store.rs @@ -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}; @@ -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, @@ -1748,6 +1749,17 @@ impl AuthorityPerEpochStore { .collect::, _>>()?) } + fn get_max_accumulated_txn_cost_per_object_in_commit(&self) -> Option { + 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, @@ -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, ) diff --git a/crates/sui-core/src/authority/shared_object_congestion_tracker.rs b/crates/sui-core/src/authority/shared_object_congestion_tracker.rs index 68f95e21fec4b..4732afe4b7399 100644 --- a/crates/sui-core/src/authority/shared_object_congestion_tracker.rs +++ b/crates/sui-core/src/authority/shared_object_congestion_tracker.rs @@ -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, commit_round: Round, ) -> Option<(DeferralKey, Vec)> { @@ -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; } @@ -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, @@ -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, ) @@ -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, ) @@ -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, ) @@ -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. @@ -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, ) { @@ -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, ) { @@ -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, ) { diff --git a/crates/sui-core/src/consensus_manager/mod.rs b/crates/sui-core/src/consensus_manager/mod.rs index 26503aec169f1..1ce7606caba84 100644 --- a/crates/sui-core/src/consensus_manager/mod.rs +++ b/crates/sui-core/src/consensus_manager/mod.rs @@ -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") { @@ -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 => { diff --git a/crates/sui-core/src/unit_tests/authority_tests.rs b/crates/sui-core/src/unit_tests/authority_tests.rs index 266defdd6b4ff..b0b48a4dfedb6 100644 --- a/crates/sui-core/src/unit_tests/authority_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_tests.rs @@ -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. @@ -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) diff --git a/crates/sui-core/src/unit_tests/congestion_control_tests.rs b/crates/sui-core/src/unit_tests/congestion_control_tests.rs index 75bf76d603a6a..4890b2006644b 100644 --- a/crates/sui-core/src/unit_tests/congestion_control_tests.rs +++ b/crates/sui-core/src/unit_tests/congestion_control_tests.rs @@ -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. diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json index 3cf11e10d5275..244d7016f3af5 100644 --- a/crates/sui-open-rpc/spec/openrpc.json +++ b/crates/sui-open-rpc/spec/openrpc.json @@ -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" diff --git a/crates/sui-protocol-config/src/lib.rs b/crates/sui-protocol-config/src/lib.rs index 74cd992249209..aed1c00a616e1 100644 --- a/crates/sui-protocol-config/src/lib.rs +++ b/crates/sui-protocol-config/src/lib.rs @@ -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); @@ -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, - /// 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, + /// 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, /// The max number of consensus rounds a transaction can be deferred due to shared object congestion. /// Transactions will be cancelled after this many rounds. @@ -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, + + /// 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, } // feature flags @@ -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, @@ -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, }; @@ -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; } @@ -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: // diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_53.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_53.snap index 48ae83e570f8e..0419b3548b467 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_53.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_53.snap @@ -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 @@ -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 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_52.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_52.snap index 4bdaa579cb0f5..f3621117fcd26 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_52.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_52.snap @@ -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 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_53.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_53.snap index 24bcdbf70c03b..095c918715c91 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_53.snap +++ b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_53.snap @@ -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 diff --git a/sdk/graphql-transport/src/methods.ts b/sdk/graphql-transport/src/methods.ts index 8957d70289e53..4525d68762217 100644 --- a/sdk/graphql-transport/src/methods.ts +++ b/sdk/graphql-transport/src/methods.ts @@ -1330,7 +1330,7 @@ export const RPC_METHODS: { const attributes: Record = {}; const configTypeMap: Record = { - 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',