From e0d5607759fbab773621108cf133001a1a281b20 Mon Sep 17 00:00:00 2001 From: Arun Koshy <97870774+arun-koshy@users.noreply.github.com> Date: Wed, 23 Oct 2024 18:43:53 -0700 Subject: [PATCH] include only ancestors sync'd 90% & no weak links for EXCLUDE ancestors --- consensus/config/src/committee.rs | 4 + consensus/core/src/ancestor.rs | 22 ++- consensus/core/src/core.rs | 43 +---- consensus/core/src/dag_state.rs | 161 +++--------------- crates/sui-open-rpc/spec/openrpc.json | 2 +- ...ests__genesis_config_snapshot_matches.snap | 2 +- ..._populated_genesis_snapshot_matches-2.snap | 30 ++-- 7 files changed, 59 insertions(+), 205 deletions(-) diff --git a/consensus/config/src/committee.rs b/consensus/config/src/committee.rs index 0d941a3aa39fd..3eda129f2aaab 100644 --- a/consensus/config/src/committee.rs +++ b/consensus/config/src/committee.rs @@ -68,6 +68,10 @@ impl Committee { self.total_stake } + pub fn n_percent_stake_threshold(&self, n: u64) -> Stake { + self.total_stake * n / 100 + } + pub fn quorum_threshold(&self) -> Stake { self.quorum_threshold } diff --git a/consensus/core/src/ancestor.rs b/consensus/core/src/ancestor.rs index 1707a2b478ce9..0ef06c639a7bb 100644 --- a/consensus/core/src/ancestor.rs +++ b/consensus/core/src/ancestor.rs @@ -73,8 +73,12 @@ impl AncestorStateManager { #[cfg(test)] const STATE_LOCK_SCORE_UPDATES: u32 = 1; + // Exclusion threshold is based on reputation scores const EXCLUSION_THRESHOLD_PERCENTAGE: u64 = 10; + // Inclusion threshold is based on network quorum round + const INCLUSION_THRESHOLD_PERCENTAGE: u64 = 90; + pub(crate) fn new(context: Arc, propagation_scores: ReputationScores) -> Self { let state_map = vec![AncestorInfo::new(); context.committee.size()]; @@ -103,6 +107,12 @@ impl AncestorStateManager { self.state_map.iter().map(|info| info.state).collect() } + pub(crate) fn get_inclusion_stake_threshold(&self) -> u64 { + self.context + .committee + .n_percent_stake_threshold(Self::INCLUSION_THRESHOLD_PERCENTAGE) + } + /// Updates the state of all ancestors based on the latest scores and quorum rounds pub(crate) fn update_all_ancestors_state(&mut self) { let propagation_scores_by_authority = self @@ -213,12 +223,12 @@ impl AncestorStateManager { self.state_map[authority_id] = ancestor_info; } - /// Calculate the network's low quorum round from 2f+1 authorities by stake, - /// where low quorum round is the highest round a block has been seen by 2f+1 - /// authorities. + /// Calculate the network's quorum round from authorities by inclusion stake + /// threshold, where quorum round is the highest round a block has been seen + /// by a percentage (inclusion threshold) of authorities. fn calculate_network_low_quorum_round(&self) -> u32 { let committee = &self.context.committee; - let quorum_threshold = committee.quorum_threshold(); + let inclusion_stake_threshold = self.get_inclusion_stake_threshold(); let mut low_quorum_rounds_with_stake = self .quorum_round_per_authority .iter() @@ -231,9 +241,9 @@ impl AncestorStateManager { let mut network_low_quorum_round = 0; for (round, stake) in low_quorum_rounds_with_stake.iter().rev() { - let reached_quorum_before = total_stake >= quorum_threshold; + let reached_quorum_before = total_stake >= inclusion_stake_threshold; total_stake += stake; - if !reached_quorum_before && total_stake >= quorum_threshold { + if !reached_quorum_before && total_stake >= inclusion_stake_threshold { network_low_quorum_round = *round; break; } diff --git a/consensus/core/src/core.rs b/consensus/core/src/core.rs index 2648cd8e2dfa4..e1f45dc8ed247 100644 --- a/consensus/core/src/core.rs +++ b/consensus/core/src/core.rs @@ -925,51 +925,11 @@ impl Core { assert!(parent_round_quorum.reached_threshold(&self.context.committee), "Fatal error, quorum not reached for parent round when proposing for round {clock_round}. Possible mismatch between DagState and Core."); - let mut excluded_ancestors_count = 0; - - // Inclusion of weak links for low score ancestors so there is no long - // list of blocks that we need to include later. for (score, ancestor) in excluded_ancestors.iter() { let excluded_author = ancestor.author(); let block_hostname = &self.context.committee.authority(excluded_author).hostname; - let mut network_low_quorum_round = - self.ancestor_state_manager.quorum_round_per_authority[excluded_author].0; - - // If the network quourum round for this ancestor is greater than or equal - // to the clock round then we want to make sure to set it to clock_round - 1 - // as that is the max round we can include as an ancestor. - network_low_quorum_round = network_low_quorum_round.min(quorum_round); - - let last_included_ancestor_round = self.last_included_ancestors[excluded_author] - .map_or(GENESIS_ROUND, |block_ref| block_ref.round); - - // Check if there is a block that is at a round higher than the last included - // ancestor round but less than or equal to the network low quorum round. - if let Some(last_cached_block_in_range) = self - .dag_state - .read() - .get_last_cached_block_for_authority_in_range( - excluded_author, - last_included_ancestor_round + 1, - network_low_quorum_round + 1, - ) - { - // Include the ancestor block as it has been seen by a strong quorum - ancestors_to_propose.push(last_cached_block_in_range.clone()); - trace!("Included low scoring ancestor {last_cached_block_in_range} with score {score} seen between last included round {last_included_ancestor_round} and network quorum round {network_low_quorum_round} to propose for round {clock_round}"); - self.context - .metrics - .node_metrics - .included_excluded_proposal_ancestors_count_by_authority - .with_label_values(&[block_hostname, "weak"]) - .inc(); - continue; - } else { - trace!("No cached block found for low scoring ancestor {ancestor} with score {score} between last included round {last_included_ancestor_round} and network quorum round {network_low_quorum_round} to propose for round {clock_round}"); - } trace!("Excluded low score ancestor {ancestor} with score {score} to propose for round {clock_round}"); - excluded_ancestors_count += 1; self.context .metrics .node_metrics @@ -979,8 +939,9 @@ impl Core { } info!( - "Included {} ancestors & excluded {excluded_ancestors_count} ancestors for proposal in round {clock_round}", + "Included {} ancestors & excluded {} ancestors for proposal in round {clock_round}", ancestors_to_propose.len(), + excluded_ancestors.len() ); ancestors_to_propose diff --git a/consensus/core/src/dag_state.rs b/consensus/core/src/dag_state.rs index 11a5793b6f932..1fc9a30a872e3 100644 --- a/consensus/core/src/dag_state.rs +++ b/consensus/core/src/dag_state.rs @@ -496,65 +496,23 @@ impl DagState { blocks } - /// Returns the last block cached for the authority within a range. - /// This can return None if there is no block within the range provided. - pub(crate) fn get_last_cached_block_for_authority_in_range( - &self, - authority: AuthorityIndex, - start_round: Round, - end_round: Round, - ) -> Option { - let blocks = self.get_last_cached_block_per_authority_in_range(start_round, end_round); - blocks[authority].clone() - } - - /// Returns the last block cached per authority with `round < end_round`. + /// Returns the last block proposed per authority with `round < end_round`. /// The method is guaranteed to return results only when the `end_round` is not earlier of the /// available cached data for each authority, otherwise the method will panic - it's the caller's - /// responsibility to ensure that is not requesting filtering for earlier rounds. + /// responsibility to ensure that is not requesting filtering for earlier rounds . + /// In case of equivocation for an authority's last slot only one block will be returned (the last in order). pub(crate) fn get_last_cached_block_per_authority( &self, end_round: Round, ) -> Vec { - let blocks = self.get_last_cached_block_per_authority_in_range(GENESIS_ROUND, end_round); - - // Ensure every authority has a valid block as we will fall back to genesis - // blocks which should always be available. - blocks - .into_iter() - .enumerate() - .map(|(idx, block_opt)| { - let authority_index = self.context.committee.to_authority_index(idx).unwrap(); - let hostname = &self.context.committee.authority(authority_index).hostname; - block_opt.unwrap_or_else(|| { - panic!("Expected to find a cached block for authority {hostname}") - }) - }) - .collect() - } - - /// Returns the last block cached per authority within `start_round <= round < end_round`. - /// The method is not guranteed to return a block for the authority as None can - /// be returned if the authority cached blocks are outside of the range provided. - /// In case of equivocation for an authority's last slot only one block will be - /// returned (the last in order). - fn get_last_cached_block_per_authority_in_range( - &self, - start_round: u32, - end_round: u32, - ) -> Vec> { - let mut blocks = if start_round != GENESIS_ROUND { - // Init with None, as we will selectively insert cached blocks in range - vec![None; self.context.committee.size()] - } else { - // Init with the genesis blocks as fallback as genesis round is within range. - self.genesis.values().cloned().map(Some).collect::>() - }; + // init with the genesis blocks as fallback + let mut blocks = self.genesis.values().cloned().collect::>(); - assert!( - end_round != GENESIS_ROUND, - "Attempted to retrieve blocks earlier than the genesis round which is not possible" - ); + if end_round == GENESIS_ROUND { + panic!( + "Attempted to retrieve blocks earlier than the genesis round which is not possible" + ); + } if end_round == GENESIS_ROUND + 1 { return blocks; @@ -568,21 +526,14 @@ impl DagState { .unwrap(); let last_evicted_round = self.evicted_rounds[authority_index]; - - // If the last evicted round is after the start round, start from the - // last evicted round + 1. - let first_available_round = start_round.max(last_evicted_round + 1); - - if first_available_round >= end_round { - debug!("Attempted to request for blocks of rounds < {end_round}, when the last evicted round is {last_evicted_round} and the first available round is {first_available_round} for authority {authority_index}"); - blocks[authority_index] = None; - continue; + if end_round.saturating_sub(1) <= last_evicted_round { + panic!("Attempted to request for blocks of rounds < {end_round}, when the last evicted round is {last_evicted_round} for authority {authority_index}", ); } if let Some(block_ref) = block_refs .range(( Included(BlockRef::new( - first_available_round, + last_evicted_round + 1, authority_index, BlockDigest::MIN, )), @@ -595,7 +546,7 @@ impl DagState { .get(block_ref) .expect("Block should exist in recent blocks"); - blocks[authority_index] = Some(block.clone()); + blocks[authority_index] = block.clone(); } } @@ -1996,82 +1947,6 @@ mod test { assert_eq!(cached_blocks[0].round(), 12); } - #[tokio::test] - async fn test_get_cached_last_block_for_authority_in_range() { - // GIVEN - const CACHED_ROUNDS: Round = 2; - let (mut context, _) = Context::new_for_test(4); - context.parameters.dag_state_cached_rounds = CACHED_ROUNDS; - - let context = Arc::new(context); - let store = Arc::new(MemStore::new()); - let mut dag_state = DagState::new(context.clone(), store.clone()); - - // Create no blocks for authority 0 - // Create one block (round 1) for authority 1 - // Create two blocks (rounds 1,2) for authority 2 - // Create three blocks (rounds 1,2,3) for authority 3 - let mut all_blocks = Vec::new(); - for author in 1..=3 { - for round in 1..=author { - let block = VerifiedBlock::new_for_test(TestBlock::new(round, author).build()); - all_blocks.push(block.clone()); - dag_state.accept_block(block); - } - } - - dag_state.add_commit(TrustedCommit::new_for_test( - 1 as CommitIndex, - CommitDigest::MIN, - context.clock.timestamp_utc_ms(), - all_blocks.last().unwrap().reference(), - all_blocks - .into_iter() - .map(|block| block.reference()) - .collect::>(), - )); - - // WHEN search for the latest blocks between rounds (2..4) - let start_round = 2; - let end_round = 4; - let last_block = dag_state.get_last_cached_block_for_authority_in_range( - AuthorityIndex::new_for_test(3), - start_round, - end_round, - ); - - // THEN we should have found a block within the range - assert_eq!(last_block.unwrap().round(), 3); - - // WHEN we flush the DagState - after adding a commit with all the blocks, we expect this to trigger - // a clean up in the internal cache. That will keep the all the blocks with rounds >= authority_commit_round - CACHED_ROUND. - dag_state.flush(); - - // AND we request blocks between rounds (3..3) - let start_round = 3; - let end_round = 3; - let last_block = dag_state.get_last_cached_block_for_authority_in_range( - AuthorityIndex::new_for_test(3), - start_round, - end_round, - ); - - // THEN we hit the case where block is not found - assert!(last_block.is_none()); - - // AND we request blocks between intentionally incorrect rounds (4..3) - let start_round = 4; - let end_round = 3; - let last_block = dag_state.get_last_cached_block_for_authority_in_range( - AuthorityIndex::new_for_test(3), - start_round, - end_round, - ); - - // THEN we hit the case where block is not found - assert!(last_block.is_none()); - } - #[rstest] #[tokio::test] async fn test_get_cached_last_block_per_authority(#[values(0, 1)] gc_depth: u32) { @@ -2154,7 +2029,9 @@ mod test { } #[tokio::test] - #[should_panic(expected = "Expected to find a cached block for authority test_host_2")] + #[should_panic( + expected = "Attempted to request for blocks of rounds < 2, when the last evicted round is 1 for authority C" + )] async fn test_get_cached_last_block_per_authority_requesting_out_of_round_range() { // GIVEN const CACHED_ROUNDS: Round = 1; @@ -2199,7 +2076,9 @@ mod test { } #[tokio::test] - #[should_panic(expected = "Expected to find a cached block for authority test_host_2")] + #[should_panic( + expected = "Attempted to request for blocks of rounds < 2, when the last evicted round is 1 for authority C" + )] async fn test_get_cached_last_block_per_authority_requesting_out_of_round_range_gc_enabled() { // GIVEN const CACHED_ROUNDS: Round = 1; diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json index 071cb7b1cd943..f33a0c653b67b 100644 --- a/crates/sui-open-rpc/spec/openrpc.json +++ b/crates/sui-open-rpc/spec/openrpc.json @@ -1293,7 +1293,7 @@ "name": "Result", "value": { "minSupportedProtocolVersion": "1", - "maxSupportedProtocolVersion": "68", + "maxSupportedProtocolVersion": "69", "protocolVersion": "6", "featureFlags": { "accept_zklogin_in_multisig": false, diff --git a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__genesis_config_snapshot_matches.snap b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__genesis_config_snapshot_matches.snap index 2fcba9da75085..7a81890461ed6 100644 --- a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__genesis_config_snapshot_matches.snap +++ b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__genesis_config_snapshot_matches.snap @@ -6,7 +6,7 @@ ssfn_config_info: ~ validator_config_info: ~ parameters: chain_start_timestamp_ms: 0 - protocol_version: 68 + protocol_version: 69 allow_insertion_of_extra_objects: true epoch_duration_ms: 86400000 stake_subsidy_start_epoch: 0 diff --git a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap index 7e39140036125..ce61493c555c6 100644 --- a/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap +++ b/crates/sui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap @@ -3,7 +3,7 @@ source: crates/sui-swarm-config/tests/snapshot_tests.rs expression: genesis.sui_system_object().into_genesis_version_for_tooling() --- epoch: 0 -protocol_version: 68 +protocol_version: 69 system_state_version: 1 validators: total_stake: 20000000000000000 @@ -240,13 +240,13 @@ validators: next_epoch_worker_address: ~ extra_fields: id: - id: "0x2f1c192f30b36b0d0a47520ff814ace58ce8a73580c9bf86c0fc729781351bcc" + id: "0x0ab85bf1aef18aad179ca34f4d95e34dd60dd1b2bda18d558e7b0bfe14ba8b77" size: 0 voting_power: 10000 - operation_cap_id: "0x0e83ac0a1c9938e12a692c734f8f38dfe5858076b17611402d46afcd5887ba8e" + operation_cap_id: "0x2fd0e88948443e54727efb6d1756d8c39735a421dac7bb92be690abe205b5bfa" gas_price: 1000 staking_pool: - id: "0x222477b804c11404854c3c14cf29a2840472651c91d8870e07ae852a98c0a2e3" + id: "0xdcc39ac807721ee62f3fe0a6c697cbd93389f79b1a6717a95ccd28dca27821c8" activation_epoch: 0 deactivation_epoch: ~ sui_balance: 20000000000000000 @@ -254,14 +254,14 @@ validators: value: 0 pool_token_balance: 20000000000000000 exchange_rates: - id: "0xf532945be4e9eb7ef597867c6dee34dc1d89f55f711d084bc6aa01c7c99ea179" + id: "0xaa706539eb8876b6c6a9b335c8d0ab1275d0a08a8ad29a6e6da68891e59298e4" size: 1 pending_stake: 0 pending_total_sui_withdraw: 0 pending_pool_token_withdraw: 0 extra_fields: id: - id: "0x4b5abcdcefc7404834889f2890b2b626ab85c15a20b19130b56cbee9bbe2b0af" + id: "0x651bc7aee334c01c02043e8caa326beaf7cf3fee5f099dda61cd39a971b1a0e6" size: 0 commission_rate: 200 next_epoch_stake: 20000000000000000 @@ -269,27 +269,27 @@ validators: next_epoch_commission_rate: 200 extra_fields: id: - id: "0xeb9ab0c31391cb533e672f2aa1a919f474a988620c5eac625dab9e28e15a7661" + id: "0x3e9b72b2ab6660a2fd11af8d5f44925fccdb0fd793ec425f6438a1b149f8e3ea" size: 0 pending_active_validators: contents: - id: "0x1e0beb565adb7f908bce1bb65d14b5da4c6e4e0ff281e91e4c79fd7a20947d35" + id: "0xd93aaac7fc3ff26d6f2336ad8f11c15f2e51a0623715fce08742faa1caabdc65" size: 0 pending_removals: [] staking_pool_mappings: - id: "0xabce5d04c1673e4e317e5c0f78bc346c4960d25473e095c9fb668ac32f5e216d" + id: "0xffabe0c370cf34f726bb6132f97dbc7bf7f77c4b5322797f786cbc73b0c0d125" size: 1 inactive_validators: - id: "0x9069998be467d392b0a8ce1f598a353c415729af75bb2ebafbe66d26114ad52f" + id: "0x3d3953f69d3dcb19cbaad3c5372084f72a9290916c5faf5f8f241cb5d5e6c94b" size: 0 validator_candidates: - id: "0x68667de51bea6086d3fd60059841df6da32a6fd475ad52ad10597797ec6a3ca9" + id: "0x66528ea16da7d7286671c3f187a1e68e0acd4fc13ffe45a0e5de12d2e2a825ec" size: 0 at_risk_validators: contents: [] extra_fields: id: - id: "0xfc98b9ca99540332ff24894fd810f83a85e726542c2119bc1325d350b0399434" + id: "0x1dff1b20ab0c400d5e48019f10ded46bdb08927f2e22fa4ced77ee6829b3c618" size: 0 storage_fund: total_object_storage_rebates: @@ -306,7 +306,7 @@ parameters: validator_low_stake_grace_period: 7 extra_fields: id: - id: "0x16212fe3db87d96453a048041166f3f491c06f00c45a4efe181bf7708c292d3f" + id: "0x3730cc216e9af079f9f8955b07b50366ee726b2d95dd102c4420e0e5102ce558" size: 0 reference_gas_price: 1000 validator_report_records: @@ -320,7 +320,7 @@ stake_subsidy: stake_subsidy_decrease_rate: 1000 extra_fields: id: - id: "0x3110ada5ccc4394928c0116629587c1ad110099430f19ea183d799689eb5a8df" + id: "0xdf3d8018bba72f99aee8cfcb7af644e0edbbd86cb02732cf4bd2f3bbc8b45a25" size: 0 safe_mode: false safe_mode_storage_rewards: @@ -332,6 +332,6 @@ safe_mode_non_refundable_storage_fee: 0 epoch_start_timestamp_ms: 10 extra_fields: id: - id: "0x34587a89960874da16d01bb778a02f7603278b0da8ec9258668982948f9b9535" + id: "0xbb67e32883f2542fb09119328da10c216ae19f663b64f024a724ca8c508502fa" size: 0