fix(upgradability): use last voted protocol version (#3297)

Address one of the issues mentioned in #3288: use the last voted protocol version instead of current epoch protocol version when there is no change.

Test plan
---------
`test_protocol_version_switch_after_switch`
`test_epoch_protocol_version_change`

Author: bowenwang1996

chain/client/tests/process_blocks.rs

@@ -2061,3 +2061,82 @@ fn test_catchup_gas_price_change() {
     // The chunk extra of the prev block of sync block should be the same as the node that it is syncing from
     assert_eq!(chunk_extra_after_sync, expected_chunk_extra);
 }
+
+#[test]
+fn test_epoch_protocol_version_change() {
+    init_test_logger();
+    let epoch_length = 5;
+    let mut genesis = Genesis::test(vec!["test0", "test1"], 2);
+    genesis.config.epoch_length = epoch_length;
+    genesis.config.protocol_version = PROTOCOL_VERSION;
+    let genesis_height = genesis.config.genesis_height;
+    let runtimes: Vec<Arc<dyn RuntimeAdapter>> = vec![
+        Arc::new(neard::NightshadeRuntime::new(
+            Path::new("."),
+            create_test_store(),
+            Arc::new(genesis.clone()),
+            vec![],
+            vec![],
+        )),
+        Arc::new(neard::NightshadeRuntime::new(
+            Path::new("."),
+            create_test_store(),
+            Arc::new(genesis.clone()),
+            vec![],
+            vec![],
+        )),
+    ];
+    let chain_genesis = ChainGenesis::from(Arc::new(genesis));
+    let mut env = TestEnv::new_with_runtime(chain_genesis, 2, 2, runtimes);
+    for i in 1..=16 {
+        let head = env.clients[0].chain.head().unwrap();
+        let epoch_id = env.clients[0]
+            .runtime_adapter
+            .get_epoch_id_from_prev_block(&head.last_block_hash)
+            .unwrap();
+        let block_producer =
+            env.clients[0].runtime_adapter.get_block_producer(&epoch_id, i).unwrap();
+        let index = if block_producer == "test0".to_string() { 0 } else { 1 };
+        let (encoded_chunk, merkle_paths, receipts) =
+            create_chunk_on_height(&mut env.clients[index], i);
+
+        for j in 0..2 {
+            let mut chain_store =
+                ChainStore::new(env.clients[j].chain.store().owned_store(), genesis_height);
+            env.clients[j]
+                .shards_mgr
+                .distribute_encoded_chunk(
+                    encoded_chunk.clone(),
+                    merkle_paths.clone(),
+                    receipts.clone(),
+                    &mut chain_store,
+                )
+                .unwrap();
+        }
+
+        let mut block = env.clients[index].produce_block(i).unwrap().unwrap();
+        // upgrade to new protocol version but in the second epoch one node vote for the old version.
+        if i != 10 {
+            let validator_signer = InMemoryValidatorSigner::from_seed(
+                &format!("test{}", index),
+                KeyType::ED25519,
+                &format!("test{}", index),
+            );
+
+            block.get_mut().header.get_mut().inner_rest.latest_protocol_version =
+                PROTOCOL_VERSION + 1;
+            block.mut_header().resign(&validator_signer);
+        }
+        for j in 0..2 {
+            let (_, res) = env.clients[j].process_block(block.clone(), Provenance::NONE);
+            assert!(res.is_ok());
+            env.clients[j].run_catchup(&vec![]).unwrap();
+        }
+    }
+    let last_block = env.clients[0].chain.get_block_by_height(16).unwrap().clone();
+    let protocol_version = env.clients[0]
+        .runtime_adapter
+        .get_epoch_protocol_version(last_block.header().epoch_id())
+        .unwrap();
+    assert_eq!(protocol_version, PROTOCOL_VERSION + 1);
+}

chain/epoch_manager/src/lib.rs

@@ -15,7 +15,7 @@ use near_primitives::types::{
     AccountId, ApprovalStake, Balance, BlockChunkValidatorStats, BlockHeight, EpochId, ShardId,
     ValidatorId, ValidatorKickoutReason, ValidatorStake, ValidatorStats,
 };
-use near_primitives::version::ProtocolVersion;
+use near_primitives::version::{ProtocolVersion, UPGRADABILITY_FIX_PROTOCOL_VERSION};
 use near_primitives::views::{
     CurrentEpochValidatorInfo, EpochValidatorInfo, NextEpochValidatorInfo, ValidatorKickoutView,
 };
@@ -236,6 +236,13 @@ impl EpochManager {
             .map(|&id| epoch_info.validators[*id as usize].stake)
             .sum();
 
+        let protocol_version = if epoch_info.protocol_version >= UPGRADABILITY_FIX_PROTOCOL_VERSION
+        {
+            next_epoch_info.protocol_version
+        } else {
+            epoch_info.protocol_version
+        };
+
         let next_version = if let Some((&version, stake)) =
             versions.into_iter().max_by(|left, right| left.1.cmp(&right.1))
         {
@@ -246,10 +253,10 @@ impl EpochManager {
             {
                 version
             } else {
-                epoch_info.protocol_version
+                protocol_version
             }
         } else {
-            epoch_info.protocol_version
+            protocol_version
         };
 
         // Gather slashed validators and add them to kick out first.
@@ -2981,4 +2988,68 @@ mod tests {
             PROTOCOL_VERSION
         );
     }
+
+    #[test]
+    fn test_protocol_version_switch_after_switch() {
+        let store = create_test_store();
+        let config = epoch_config(2, 1, 2, 0, 90, 60, 0);
+        let amount_staked = 1_000_000;
+        let validators = vec![stake("test1", amount_staked), stake("test2", amount_staked)];
+        let mut epoch_manager = EpochManager::new(
+            store.clone(),
+            config.clone(),
+            UPGRADABILITY_FIX_PROTOCOL_VERSION,
+            default_reward_calculator(),
+            validators.clone(),
+        )
+        .unwrap();
+        let h = hash_range(10);
+        record_block(&mut epoch_manager, CryptoHash::default(), h[0], 0, vec![]);
+        for i in 1..5 {
+            let mut block_info = block_info(
+                i as u64,
+                i as u64 - 1,
+                h[i - 1],
+                h[i - 1],
+                h[0],
+                vec![],
+                DEFAULT_TOTAL_SUPPLY,
+            );
+            if i != 4 {
+                block_info.latest_protocol_version = UPGRADABILITY_FIX_PROTOCOL_VERSION + 1;
+            } else {
+                block_info.latest_protocol_version = UPGRADABILITY_FIX_PROTOCOL_VERSION;
+            }
+            epoch_manager.record_block_info(&h[i], block_info, [0; 32]).unwrap();
+        }
+
+        assert_eq!(
+            epoch_manager.get_epoch_info(&EpochId(h[2])).unwrap().protocol_version,
+            UPGRADABILITY_FIX_PROTOCOL_VERSION + 1
+        );
+
+        assert_eq!(
+            epoch_manager.get_epoch_info(&EpochId(h[4])).unwrap().protocol_version,
+            UPGRADABILITY_FIX_PROTOCOL_VERSION + 1
+        );
+
+        // if there are enough votes to use the old version, it should be allowed
+        for i in 5..7 {
+            let mut block_info = block_info(
+                i as u64,
+                i as u64 - 1,
+                h[i - 1],
+                h[i - 1],
+                h[0],
+                vec![],
+                DEFAULT_TOTAL_SUPPLY,
+            );
+            block_info.latest_protocol_version = UPGRADABILITY_FIX_PROTOCOL_VERSION;
+            epoch_manager.record_block_info(&h[i], block_info, [0; 32]).unwrap();
+        }
+        assert_eq!(
+            epoch_manager.get_epoch_info(&EpochId(h[6])).unwrap().protocol_version,
+            UPGRADABILITY_FIX_PROTOCOL_VERSION
+        );
+    }
 }

chain/epoch_manager/src/proposals.rs

@@ -41,7 +41,7 @@ pub(crate) fn find_threshold(
 pub fn proposals_to_epoch_info(
     epoch_config: &EpochConfig,
     rng_seed: RngSeed,
-    epoch_info: &EpochInfo,
+    prev_epoch_info: &EpochInfo,
     proposals: Vec<ValidatorStake>,
     mut validator_kickout: HashMap<AccountId, ValidatorKickoutReason>,
     validator_reward: HashMap<AccountId, Balance>,
@@ -67,7 +67,7 @@ pub fn proposals_to_epoch_info(
             ordered_proposals.insert(p.account_id.clone(), p);
         }
     }
-    for r in epoch_info.validators.iter() {
+    for r in prev_epoch_info.validators.iter() {
         if validator_kickout.contains_key(&r.account_id) {
             stake_change.insert(r.account_id.clone(), 0);
             continue;
@@ -77,7 +77,7 @@ pub fn proposals_to_epoch_info(
         stake_change.insert(p.account_id.clone(), p.stake);
     }
 
-    for r in epoch_info.fishermen.iter() {
+    for r in prev_epoch_info.fishermen.iter() {
         if validator_kickout.contains_key(&r.account_id) {
             stake_change.insert(r.account_id.clone(), 0);
             continue;
@@ -107,8 +107,8 @@ pub fn proposals_to_epoch_info(
             fishermen.push(p);
         } else {
             *stake_change.get_mut(&account_id).unwrap() = 0;
-            if epoch_info.validator_to_index.contains_key(&account_id)
-                || epoch_info.fishermen_to_index.contains_key(&account_id)
+            if prev_epoch_info.validator_to_index.contains_key(&account_id)
+                || prev_epoch_info.fishermen_to_index.contains_key(&account_id)
             {
                 validator_kickout.insert(
                     account_id,
@@ -156,8 +156,8 @@ pub fn proposals_to_epoch_info(
             fishermen.push(p);
         } else {
             stake_change.insert(p.account_id.clone(), 0);
-            if epoch_info.validator_to_index.contains_key(&p.account_id)
-                || epoch_info.fishermen_to_index.contains_key(&p.account_id)
+            if prev_epoch_info.validator_to_index.contains_key(&p.account_id)
+                || prev_epoch_info.fishermen_to_index.contains_key(&p.account_id)
             {
                 validator_kickout.insert(p.account_id, ValidatorKickoutReason::DidNotGetASeat);
             }
@@ -195,7 +195,7 @@ pub fn proposals_to_epoch_info(
         .collect::<HashMap<_, _>>();
 
     Ok(EpochInfo {
-        epoch_height: epoch_info.epoch_height + 1,
+        epoch_height: prev_epoch_info.epoch_height + 1,
         validators: final_proposals,
         fishermen,
         validator_to_index,

core/primitives/src/version.rs

@@ -18,7 +18,7 @@ pub const DB_VERSION: DbVersion = 9;
 pub type ProtocolVersion = u32;
 
 /// Current latest version of the protocol.
-pub const PROTOCOL_VERSION: ProtocolVersion = 36;
+pub const PROTOCOL_VERSION: ProtocolVersion = 37;
 /// TODO: Remove in next release. This is to allow nodes with initial version 34
 /// to be compatible with nodes at version 35
 pub const NETWORK_PROTOCOL_VERSION: ProtocolVersion = 34;
@@ -40,3 +40,7 @@ pub const IMPLICIT_ACCOUNT_CREATION_PROTOCOL_VERSION: ProtocolVersion = 35;
 
 /// The protocol version that enables reward on mainnet.
 pub const ENABLE_INFLATION_PROTOCOL_VERSION: ProtocolVersion = 36;
+
+/// Fix upgrade to use the latest voted protocol version instead of the current epoch protocol
+/// version when there is no new change in protocol version.
+pub const UPGRADABILITY_FIX_PROTOCOL_VERSION: ProtocolVersion = 37;

neard/res/genesis_config.json

@@ -1,5 +1,5 @@
 {
-  "protocol_version": 36,
+  "protocol_version": 37,
   "genesis_time": "1970-01-01T00:00:00.000000000Z",
   "chain_id": "sample",
   "genesis_height": 0,