Support for advancing to highest supported protocol version (MystenLa…

…bs#11198)

## Description 

Changes voting procedure for determining next protocol version, so that
validators can select the highest supported version instead of the next
consecutive version.

## Test Plan 

cargo simtest

### Type of Change (Check all that apply)

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [x] breaking change for FNs (FN binary must upgrade)
- [x] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

Fullnode operators will need to upgrade to a binary that supports
protocol version 7.

crates/sui-core/src/authority.rs

@@ -53,7 +53,7 @@ use sui_json_rpc_types::{
     SuiMoveValue, SuiObjectDataFilter, SuiTransactionBlockData, SuiTransactionBlockEvents,
 };
 use sui_macros::{fail_point, fail_point_async};
-use sui_protocol_config::SupportedProtocolVersions;
+use sui_protocol_config::{ProtocolConfig, SupportedProtocolVersions};
 use sui_storage::indexes::{CoinInfo, ObjectIndexChanges};
 use sui_storage::IndexStore;
 use sui_types::committee::{EpochId, ProtocolVersion};
@@ -3193,13 +3193,19 @@ impl AuthorityState {
         Some(res)
     }
 
-    fn choose_protocol_version_and_system_packages(
+    fn is_protocol_version_supported(
         current_protocol_version: ProtocolVersion,
+        proposed_protocol_version: ProtocolVersion,
+        protocol_config: &ProtocolConfig,
         committee: &Committee,
         capabilities: Vec<AuthorityCapabilities>,
         mut buffer_stake_bps: u64,
-    ) -> (ProtocolVersion, Vec<ObjectRef>) {
-        let next_protocol_version = current_protocol_version + 1;
+    ) -> Option<(ProtocolVersion, Vec<ObjectRef>)> {
+        if proposed_protocol_version > current_protocol_version + 1
+            && !protocol_config.advance_to_highest_supported_protocol_version()
+        {
+            return None;
+        }
 
         if buffer_stake_bps > 10000 {
             warn!("clamping buffer_stake_bps to 10000");
@@ -3229,7 +3235,7 @@ impl AuthorityState {
                 // against any change, because framework upgrades always require a protocol version
                 // bump.
                 cap.supported_protocol_versions
-                    .is_version_supported(next_protocol_version)
+                    .is_version_supported(proposed_protocol_version)
                     .then_some((cap.available_system_packages, cap.authority))
             })
             .collect();
@@ -3264,16 +3270,39 @@ impl AuthorityState {
                     ?quorum_threshold,
                     ?buffer_stake_bps,
                     ?effective_threshold,
-                    ?next_protocol_version,
+                    ?proposed_protocol_version,
                     ?packages,
                     "support for upgrade"
                 );
 
                 let has_support = total_votes >= effective_threshold;
-                has_support.then_some((next_protocol_version, packages))
+                has_support.then_some((proposed_protocol_version, packages))
             })
-            // if there was no majority, there is no upgrade
-            .unwrap_or((current_protocol_version, vec![]))
+    }
+
+    fn choose_protocol_version_and_system_packages(
+        current_protocol_version: ProtocolVersion,
+        protocol_config: &ProtocolConfig,
+        committee: &Committee,
+        capabilities: Vec<AuthorityCapabilities>,
+        buffer_stake_bps: u64,
+    ) -> (ProtocolVersion, Vec<ObjectRef>) {
+        let mut next_protocol_version = current_protocol_version;
+        let mut system_packages = vec![];
+
+        while let Some((version, packages)) = Self::is_protocol_version_supported(
+            current_protocol_version,
+            next_protocol_version + 1,
+            protocol_config,
+            committee,
+            capabilities.clone(),
+            buffer_stake_bps,
+        ) {
+            next_protocol_version = version;
+            system_packages = packages;
+        }
+
+        (next_protocol_version, system_packages)
     }
 
     /// Creates and execute the advance epoch transaction to effects without committing it to the database.
@@ -3300,8 +3329,11 @@ impl AuthorityState {
         let (next_epoch_protocol_version, next_epoch_system_packages) =
             Self::choose_protocol_version_and_system_packages(
                 epoch_store.protocol_version(),
+                epoch_store.protocol_config(),
                 epoch_store.committee(),
-                epoch_store.get_capabilities()?,
+                epoch_store
+                    .get_capabilities()
+                    .expect("read capabilities from db cannot fail"),
                 buffer_stake_bps,
             );
 

crates/sui-core/src/unit_tests/authority_tests.rs

@@ -4618,6 +4618,7 @@ fn test_choose_next_system_packages() {
     let committee = Committee::new_simple_test_committee().0;
     let v = &committee.voting_rights;
     let mut protocol_config = ProtocolConfig::get_for_max_version();
+    protocol_config.set_advance_to_highest_supported_protocol_version_for_testing(false);
     protocol_config.set_buffer_stake_for_protocol_upgrade_bps_for_testing(7500);
 
     // all validators agree on new system packages, but without a new protocol version, so no
@@ -4633,6 +4634,7 @@ fn test_choose_next_system_packages() {
         (ver(1), vec![]),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4651,6 +4653,7 @@ fn test_choose_next_system_packages() {
         (ver(1), vec![]),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities.clone(),
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4664,6 +4667,7 @@ fn test_choose_next_system_packages() {
         (ver(2), sort(vec![o1, o2])),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4682,6 +4686,7 @@ fn test_choose_next_system_packages() {
         (ver(1), vec![]),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4700,6 +4705,7 @@ fn test_choose_next_system_packages() {
         (ver(2), sort(vec![o1, o2])),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4718,6 +4724,27 @@ fn test_choose_next_system_packages() {
         (ver(1), vec![]),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
+            &committee,
+            capabilities,
+            protocol_config.buffer_stake_for_protocol_upgrade_bps(),
+        )
+    );
+
+    // all validators support 3, but with this protocol config we cannot advance multiple
+    // versions at once.
+    let capabilities = vec![
+        make_capabilities!(3, v[0].0, vec![o1, o2]),
+        make_capabilities!(3, v[1].0, vec![o1, o2]),
+        make_capabilities!(3, v[2].0, vec![o1, o2]),
+        make_capabilities!(3, v[3].0, vec![o1, o2]),
+    ];
+
+    assert_eq!(
+        (ver(2), sort(vec![o1, o2])),
+        AuthorityState::choose_protocol_version_and_system_packages(
+            ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),
@@ -4736,6 +4763,69 @@ fn test_choose_next_system_packages() {
         (ver(1), vec![]),
         AuthorityState::choose_protocol_version_and_system_packages(
             ProtocolVersion::MIN,
+            &protocol_config,
+            &committee,
+            capabilities,
+            protocol_config.buffer_stake_for_protocol_upgrade_bps(),
+        )
+    );
+
+    protocol_config.set_advance_to_highest_supported_protocol_version_for_testing(true);
+
+    // skip straight to version 3
+    let capabilities = vec![
+        make_capabilities!(3, v[0].0, vec![o1, o2]),
+        make_capabilities!(3, v[1].0, vec![o1, o2]),
+        make_capabilities!(3, v[2].0, vec![o1, o2]),
+        make_capabilities!(3, v[3].0, vec![o1, o3]),
+    ];
+
+    assert_eq!(
+        (ver(3), sort(vec![o1, o2])),
+        AuthorityState::choose_protocol_version_and_system_packages(
+            ProtocolVersion::MIN,
+            &protocol_config,
+            &committee,
+            capabilities,
+            protocol_config.buffer_stake_for_protocol_upgrade_bps(),
+        )
+    );
+
+    let capabilities = vec![
+        make_capabilities!(3, v[0].0, vec![o1, o2]),
+        make_capabilities!(3, v[1].0, vec![o1, o2]),
+        make_capabilities!(4, v[2].0, vec![o1, o2]),
+        make_capabilities!(5, v[3].0, vec![o1, o2]),
+    ];
+
+    // packages are identical between all currently supported versions, so we can upgrade to
+    // 3 which is the highest supported version
+    assert_eq!(
+        (ver(3), sort(vec![o1, o2])),
+        AuthorityState::choose_protocol_version_and_system_packages(
+            ProtocolVersion::MIN,
+            &protocol_config,
+            &committee,
+            capabilities,
+            protocol_config.buffer_stake_for_protocol_upgrade_bps(),
+        )
+    );
+
+    let capabilities = vec![
+        make_capabilities!(2, v[0].0, vec![]),
+        make_capabilities!(2, v[1].0, vec![]),
+        make_capabilities!(3, v[2].0, vec![o1, o2]),
+        make_capabilities!(3, v[3].0, vec![o1, o3]),
+    ];
+
+    // Even though 2f+1 validators agree on version 2, we don't have an agreement about the
+    // packages. In this situation it is likely that (v2, []) is a valid upgrade, but we don't have
+    // a way to detect that. The upgrade simply won't happen until everyone moves to 3.
+    assert_eq!(
+        (ver(1), sort(vec![])),
+        AuthorityState::choose_protocol_version_and_system_packages(
+            ProtocolVersion::MIN,
+            &protocol_config,
             &committee,
             capabilities,
             protocol_config.buffer_stake_for_protocol_upgrade_bps(),

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

@@ -24,7 +24,9 @@ const MAX_PROTOCOL_VERSION: u64 = 7;
 // Version 5: Package upgrade compatibility error fix. New gas cost table. New scoring decision
 //            mechanism that includes up to f scoring authorities.
 // Version 6: Change to how bytes are charged in the gas meter, increase buffer stake to 0.5f
-// Version 7: Disallow adding `key` ability during package upgrades.
+// Version 7: Disallow adding `key` ability during package upgrades,
+//            disable_invariant_violation_check_in_swap_loc,
+//            advance_to_hightest_supported_protocol_version
 
 #[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ProtocolVersion(u64);
@@ -151,6 +153,10 @@ struct FeatureFlags {
     // Disables unnecessary invariant check in the Move VM when swapping the value out of a local
     #[serde(skip_serializing_if = "is_false")]
     disable_invariant_violation_check_in_swap_loc: bool,
+    // advance to highest supported protocol version at epoch change, instead of the next consecutive
+    // protocol version.
+    #[serde(skip_serializing_if = "is_false")]
+    advance_to_highest_supported_protocol_version: bool,
 }
 
 fn is_false(b: &bool) -> bool {
@@ -649,6 +655,11 @@ impl ProtocolConfig {
         self.feature_flags
             .disable_invariant_violation_check_in_swap_loc
     }
+
+    pub fn advance_to_highest_supported_protocol_version(&self) -> bool {
+        self.feature_flags
+            .advance_to_highest_supported_protocol_version
+    }
 }
 
 // Special getters
@@ -1052,6 +1063,8 @@ impl ProtocolConfig {
                 cfg.feature_flags.disallow_adding_key_ability = true;
                 cfg.feature_flags
                     .disable_invariant_violation_check_in_swap_loc = true;
+                cfg.feature_flags
+                    .advance_to_highest_supported_protocol_version = true;
                 cfg
             }
             // Use this template when making changes:
@@ -1099,6 +1112,10 @@ impl ProtocolConfig {
     pub fn set_package_upgrades_for_testing(&mut self, val: bool) {
         self.feature_flags.package_upgrades = val
     }
+    pub fn set_advance_to_highest_supported_protocol_version_for_testing(&mut self, val: bool) {
+        self.feature_flags
+            .advance_to_highest_supported_protocol_version = val
+    }
 }
 
 type OverrideFn = dyn Fn(ProtocolVersion, ProtocolConfig) -> ProtocolConfig + Send;

crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_7.snap

@@ -12,6 +12,7 @@ feature_flags:
   consensus_order_end_of_epoch_last: true
   disallow_adding_key_ability: true
   disable_invariant_violation_check_in_swap_loc: true
+  advance_to_highest_supported_protocol_version: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000