diff --git a/core/primitives/src/upgrade_schedule.rs b/core/primitives/src/upgrade_schedule.rs index a1f0560266c..842db451148 100644 --- a/core/primitives/src/upgrade_schedule.rs +++ b/core/primitives/src/upgrade_schedule.rs @@ -115,8 +115,9 @@ 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 { @@ -124,6 +125,9 @@ impl ProtocolUpgradeVotingSchedule { 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] diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs index 109e30e40a3..195591fda26 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -67,6 +67,10 @@ pub const PROTOCOL_UPGRADE_SCHEDULE: Lazy = 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; diff --git a/integration-tests/src/tests/client/process_blocks.rs b/integration-tests/src/tests/client/process_blocks.rs index 4d2223b6242..3fb03ec0424 100644 --- a/integration-tests/src/tests/client/process_blocks.rs +++ b/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);