feat(protocol_upgrade): nodes won't vote multiple protocol upgrades t…

…ogether (#11515)

A rather trivial solution to a potential inconvenience introduced by
#11368: due to a human error or a very long epoch multiple protocol
upgrades could end up being applied together at an epoch's boundary.

This PR change the protocol upgrade voting to select at most one upgrade
per epoch.

core/primitives/src/upgrade_schedule.rs

@@ -115,15 +115,19 @@ impl ProtocolUpgradeVotingSchedule {
         }
 
         // The datetime values in the schedule are sorted in ascending order.
-        // Find the last datetime value that is less than the current time. The
-        // schedule is sorted and the last value is the client_protocol_version
+        // Find the first datetime value that is less than the current time
+        // and higher than next_epoch_protocol_version.
+        // The schedule is sorted and the last value is the client_protocol_version
         // so we are guaranteed to find a correct protocol version.
         let mut result = next_epoch_protocol_version;
         for (time, version) in &self.schedule {
             if now < *time {
                 break;
             }
             result = *version;
+            if *version > next_epoch_protocol_version {
+                break;
+            }
         }
 
         result
@@ -332,13 +336,55 @@ mod tests {
         let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-15 00:00:00").unwrap();
         assert_eq!(
             current_protocol_version + 2,
-            schedule.get_protocol_version(now, current_protocol_version)
+            schedule.get_protocol_version(now, current_protocol_version + 1)
         );
         let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-20 00:00:00").unwrap();
         assert_eq!(
             current_protocol_version + 2,
+            schedule.get_protocol_version(now, current_protocol_version + 1)
+        );
+    }
+
+    #[test]
+    fn test_upgrades_are_voted_one_at_a_time() {
+        let current_protocol_version = 100;
+        let client_protocol_version = 103;
+        let schedule = vec![
+            ("2000-01-10 00:00:00", current_protocol_version + 1),
+            ("2000-01-10 01:00:00", current_protocol_version + 2),
+            ("2000-01-10 02:00:00", current_protocol_version + 3),
+        ];
+
+        let schedule = make_voting_schedule(client_protocol_version, schedule);
+
+        // Test that the current version is returned before the first upgrade.
+        let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-05 00:00:00").unwrap();
+        assert_eq!(
+            current_protocol_version,
+            schedule.get_protocol_version(now, current_protocol_version)
+        );
+
+        // Upgrades are scheduled very close to each other, but they should be voted on at a time.
+        // Test the first upgrade.
+        let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-10 10:00:00").unwrap();
+        assert_eq!(
+            current_protocol_version + 1,
             schedule.get_protocol_version(now, current_protocol_version)
         );
+
+        // Test the second upgrade.
+        let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-10 10:00:00").unwrap();
+        assert_eq!(
+            current_protocol_version + 2,
+            schedule.get_protocol_version(now, current_protocol_version + 1)
+        );
+
+        // Test the final upgrade.
+        let now = ProtocolUpgradeVotingSchedule::parse_datetime("2000-01-10 10:00:00").unwrap();
+        assert_eq!(
+            current_protocol_version + 3,
+            schedule.get_protocol_version(now, current_protocol_version + 2)
+        );
     }
 
     #[test]

core/primitives/src/version.rs

@@ -67,6 +67,10 @@ pub const PROTOCOL_UPGRADE_SCHEDULE: Lazy<ProtocolUpgradeVotingSchedule> = Lazy:
     // that they are ordered by datetime and version, the last one is the
     // PROTOCOL_VERSION and that there is enough time between subsequent
     // upgrades.
+    //
+    // At most one protocol version upgrade vote can happen per epoch. If, by any
+    // chance, two or more votes get scheduled on the same epoch, the latest upgrades
+    // will be postponed.
 
     // e.g.
     // let v1_protocol_version = 50;

integration-tests/src/tests/client/process_blocks.rs

@@ -2828,14 +2828,8 @@ fn test_epoch_multi_protocol_version_change() {
     let mut seen_v2 = false;
 
     let mut height = 1;
-    while chrono::Utc::now() < end_time {
-        let head = env.clients[0].chain.head().unwrap();
-        let epoch_id = env.clients[0]
-            .epoch_manager
-            .get_epoch_id_from_prev_block(&head.last_block_hash)
-            .unwrap();
-        let protocol_version =
-            env.clients[0].epoch_manager.get_epoch_protocol_version(&epoch_id).unwrap();
+    while chrono::Utc::now() < end_time && (!seen_v0 || !seen_v1 || !seen_v2) {
+        let (epoch_id, protocol_version) = get_epoch_id_and_protocol_version(&env);
 
         produce_chunks(&mut env, &epoch_id, height);
 
@@ -2862,7 +2856,96 @@ fn test_epoch_multi_protocol_version_change() {
     assert!(seen_v2);
 }
 
-// helper for test_epoch_multi_protocol_version_change
+#[test]
+fn test_epoch_multi_protocol_version_change_epoch_overlap() {
+    init_test_logger();
+
+    let v0 = PROTOCOL_VERSION - 2;
+    let v1 = PROTOCOL_VERSION - 1;
+    let v2 = PROTOCOL_VERSION;
+
+    // produce blocks roughly every 500ms
+    // one epoch is roughly 2500ms
+    // at least two epochs are needed for one protocol upgrade
+    // schedule the first protocol upgrade voting at now +1s
+    // arrange the second protocol upgrade voting so that it falls
+    // on the same epoch as the first (now +1s)
+    // assert two protocol upgrades should never happen on the same epoch
+
+    let start_time = chrono::Utc::now();
+    let end_time = start_time + chrono::Duration::seconds(25);
+
+    let v1_upgrade_time = start_time + chrono::Duration::seconds(1);
+    let v2_upgrade_time = start_time + chrono::Duration::seconds(2);
+
+    let v1_upgrade_time = v1_upgrade_time.format("%Y-%m-%d %H:%M:%S").to_string();
+    let v2_upgrade_time = v2_upgrade_time.format("%Y-%m-%d %H:%M:%S").to_string();
+
+    let protocol_version_override =
+        format!("{}={},{}={}", v1_upgrade_time, v1, v2_upgrade_time, v2);
+
+    tracing::debug!(target: "test", ?protocol_version_override, "setting the protocol_version_override");
+    std::env::set_var("NEAR_TESTS_PROTOCOL_UPGRADE_OVERRIDE", protocol_version_override);
+
+    let epoch_length = 5;
+    let mut genesis = Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 2);
+    genesis.config.epoch_length = epoch_length;
+    genesis.config.protocol_version = v0;
+    let mut env = TestEnv::builder(&genesis.config)
+        .clients_count(2)
+        .validator_seats(2)
+        .nightshade_runtimes(&genesis)
+        .build();
+
+    let mut seen_v0 = false;
+    let mut seen_v1 = false;
+    let mut seen_v2 = false;
+
+    let mut height = 1;
+    let (mut current_epoch_id, mut current_protocol_version) =
+        get_epoch_id_and_protocol_version(&env);
+    while chrono::Utc::now() < end_time && (!seen_v0 || !seen_v1 || !seen_v2) {
+        let (epoch_id, protocol_version) = get_epoch_id_and_protocol_version(&env);
+
+        produce_chunks(&mut env, &epoch_id, height);
+
+        produce_block(&mut env, &epoch_id, height);
+
+        if protocol_version == v0 {
+            seen_v0 = true;
+        }
+        if protocol_version == v1 {
+            seen_v1 = true;
+        }
+        if protocol_version == v2 {
+            seen_v2 = true;
+        }
+
+        assert!(
+            protocol_version - current_protocol_version <= 1,
+            "protocol version should never increase twice in one iteration ({current_protocol_version} -> {protocol_version})"
+        );
+        if epoch_id == current_epoch_id {
+            assert_eq!(
+                current_protocol_version, protocol_version,
+                "protocol version shouldn't change during the same epoch"
+            );
+        }
+
+        tracing::debug!(target: "test", ?height, ?protocol_version, "loop iter finished");
+
+        height += 1;
+        current_epoch_id = epoch_id;
+        current_protocol_version = protocol_version;
+        std::thread::sleep(std::time::Duration::from_millis(500));
+    }
+
+    assert!(seen_v0);
+    assert!(seen_v1);
+    assert!(seen_v2);
+}
+
+// helper for test_epoch_multi_protocol_version_change* class of tests
 fn produce_block(env: &mut TestEnv, epoch_id: &EpochId, height: u64) {
     let block_producer = env.clients[0].epoch_manager.get_block_producer(epoch_id, height).unwrap();
     let index = if block_producer == "test0" { 0 } else { 1 };
@@ -2872,7 +2955,7 @@ fn produce_block(env: &mut TestEnv, epoch_id: &EpochId, height: u64) {
     }
 }
 
-// helper for test_epoch_multi_protocol_version_change
+// helper for test_epoch_multi_protocol_version_change* class of tests
 fn produce_chunks(env: &mut TestEnv, epoch_id: &EpochId, height: u64) {
     let shard_layout = env.clients[0].epoch_manager.get_shard_layout(epoch_id).unwrap();
 
@@ -2898,6 +2981,16 @@ fn produce_chunks(env: &mut TestEnv, epoch_id: &EpochId, height: u64) {
     }
 }
 
+// helper for test_epoch_multi_protocol_version_change* class of tests
+fn get_epoch_id_and_protocol_version(env: &TestEnv) -> (EpochId, u32) {
+    let head = env.clients[0].chain.head().unwrap();
+    let epoch_id =
+        env.clients[0].epoch_manager.get_epoch_id_from_prev_block(&head.last_block_hash).unwrap();
+    let protocol_version =
+        env.clients[0].epoch_manager.get_epoch_protocol_version(&epoch_id).unwrap();
+    (epoch_id, protocol_version)
+}
+
 #[test]
 fn test_discard_non_finalizable_block() {
     let genesis = Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 1);