refactor: move EpochManager functions as default impl for EpochManage…

…rAdapter (#12851)

This PR moves implementation for a bunch of functions from
`EpochManager` to be the default implementation in `EpochManagerAdapter`
trait.
Also `is_slashed` is removed from the API since we do not properly
support slashing.

chain/chain/src/approval_verification.rs

@@ -14,7 +14,7 @@ pub fn verify_approval_with_approvers_info(
     prev_block_height: BlockHeight,
     block_height: BlockHeight,
     approvals: &[Option<Box<near_crypto::Signature>>],
-    info: Vec<(ApprovalStake, bool)>,
+    info: Vec<ApprovalStake>,
 ) -> Result<bool, Error> {
     if approvals.len() > info.len() {
         return Ok(false);
@@ -29,9 +29,9 @@ pub fn verify_approval_with_approvers_info(
         block_height,
     );
 
-    for ((validator, is_slashed), may_be_signature) in info.into_iter().zip(approvals.iter()) {
+    for (validator, may_be_signature) in info.into_iter().zip(approvals.iter()) {
         if let Some(signature) = may_be_signature {
-            if is_slashed || !signature.verify(message_to_sign.as_ref(), &validator.public_key) {
+            if !signature.verify(message_to_sign.as_ref(), &validator.public_key) {
                 return Ok(false);
             }
         }
@@ -43,7 +43,7 @@ pub fn verify_approval_with_approvers_info(
 pub fn verify_approvals_and_threshold_orphan(
     can_approved_block_be_produced: &dyn Fn(
         &[Option<Box<Signature>>],
-        &[(Balance, Balance, bool)],
+        &[(Balance, Balance)],
     ) -> bool,
     prev_block_hash: &CryptoHash,
     prev_block_height: BlockHeight,
@@ -70,7 +70,7 @@ pub fn verify_approvals_and_threshold_orphan(
     }
     let stakes = block_approvers
         .iter()
-        .map(|stake| (stake.stake_this_epoch, stake.stake_next_epoch, false))
+        .map(|stake| (stake.stake_this_epoch, stake.stake_next_epoch))
         .collect::<Vec<_>>();
     if !can_approved_block_be_produced(approvals, &stakes) {
         Err(Error::NotEnoughApprovals)

chain/chain/src/chain.rs

@@ -340,12 +340,7 @@ impl Chain {
             chain_genesis.height,
             chain_genesis.min_gas_price,
             chain_genesis.total_supply,
-            Chain::compute_bp_hash(
-                epoch_manager,
-                EpochId::default(),
-                EpochId::default(),
-                &CryptoHash::default(),
-            )?,
+            Chain::compute_bp_hash(epoch_manager, EpochId::default(), EpochId::default())?,
         );
         Ok((genesis_block, genesis_chunks))
     }
@@ -613,10 +608,8 @@ impl Chain {
         epoch_manager: &dyn EpochManagerAdapter,
         epoch_id: EpochId,
         prev_epoch_id: EpochId,
-        last_known_hash: &CryptoHash,
     ) -> Result<CryptoHash, Error> {
-        let bps = epoch_manager.get_epoch_block_producers_ordered(&epoch_id, last_known_hash)?;
-        let validator_stakes = bps.into_iter().map(|(bp, _)| bp).collect_vec();
+        let validator_stakes = epoch_manager.get_epoch_block_producers_ordered(&epoch_id)?;
         let protocol_version = epoch_manager.get_epoch_protocol_version(&prev_epoch_id)?;
         Self::compute_bp_hash_from_validator_stakes(
             &validator_stakes,
@@ -752,11 +745,8 @@ impl Chain {
             }
         };
 
-        let next_block_producers = get_epoch_block_producers_view(
-            final_block_header.next_epoch_id(),
-            header.prev_hash(),
-            epoch_manager,
-        )?;
+        let next_block_producers =
+            get_epoch_block_producers_view(final_block_header.next_epoch_id(), epoch_manager)?;
 
         create_light_client_block_view(&final_block_header, chain_store, Some(next_block_producers))
     }
@@ -997,7 +987,6 @@ impl Chain {
                     self.epoch_manager.as_ref(),
                     *header.next_epoch_id(),
                     *header.epoch_id(),
-                    header.prev_hash(),
                 )?
             {
                 return Err(Error::InvalidNextBPHash);
@@ -1042,7 +1031,7 @@ impl Chain {
                 .epoch_manager
                 .get_epoch_block_approvers_ordered(header.prev_hash())?
                 .iter()
-                .map(|(x, is_slashed)| (x.stake_this_epoch, x.stake_next_epoch, *is_slashed))
+                .map(|x| (x.stake_this_epoch, x.stake_next_epoch))
                 .collect::<Vec<_>>();
             if !Doomslug::can_approved_block_be_produced(
                 self.doomslug_threshold_mode,

chain/chain/src/doomslug.rs

@@ -280,7 +280,7 @@ impl DoomslugApprovalsTrackersAtHeight {
     fn process_approval(
         &mut self,
         approval: &Approval,
-        stakes: &[(ApprovalStake, bool)],
+        stakes: &[ApprovalStake],
         threshold_mode: DoomslugThresholdMode,
     ) -> DoomslugBlockProductionReadiness {
         if let Some(last_parent) = self.last_approval_per_account.get(&approval.account_id) {
@@ -300,13 +300,7 @@ impl DoomslugApprovalsTrackersAtHeight {
 
         let account_id_to_stakes = stakes
             .iter()
-            .filter_map(|(x, is_slashed)| {
-                if *is_slashed {
-                    None
-                } else {
-                    Some((x.account_id.clone(), (x.stake_this_epoch, x.stake_next_epoch)))
-                }
-            })
+            .map(|x| (x.account_id.clone(), (x.stake_this_epoch, x.stake_next_epoch)))
             .collect::<HashMap<_, _>>();
 
         assert_eq!(account_id_to_stakes.len(), stakes.len());
@@ -559,27 +553,25 @@ impl Doomslug {
     pub fn can_approved_block_be_produced(
         mode: DoomslugThresholdMode,
         approvals: &[Option<Box<Signature>>],
-        stakes: &[(Balance, Balance, bool)],
+        stakes: &[(Balance, Balance)],
     ) -> bool {
         if mode == DoomslugThresholdMode::NoApprovals {
             return true;
         }
 
-        let threshold1 = stakes.iter().map(|(x, _, _)| x).sum::<Balance>() * 2 / 3;
-        let threshold2 = stakes.iter().map(|(_, x, _)| x).sum::<Balance>() * 2 / 3;
+        let threshold1 = stakes.iter().map(|(x, _)| x).sum::<Balance>() * 2 / 3;
+        let threshold2 = stakes.iter().map(|(_, x)| x).sum::<Balance>() * 2 / 3;
 
         let approved_stake1 = approvals
             .iter()
             .zip(stakes.iter())
-            .filter(|(_, (_, _, is_slashed))| !*is_slashed)
-            .map(|(approval, (stake, _, _))| if approval.is_some() { *stake } else { 0 })
+            .map(|(approval, (stake, _))| if approval.is_some() { *stake } else { 0 })
             .sum::<Balance>();
 
         let approved_stake2 = approvals
             .iter()
             .zip(stakes.iter())
-            .filter(|(_, (_, _, is_slashed))| !*is_slashed)
-            .map(|(approval, (_, stake, _))| if approval.is_some() { *stake } else { 0 })
+            .map(|(approval, (_, stake))| if approval.is_some() { *stake } else { 0 })
             .sum::<Balance>();
 
         (approved_stake1 > threshold1 || threshold1 == 0)
@@ -639,7 +631,7 @@ impl Doomslug {
     fn on_approval_message_internal(
         &mut self,
         approval: &Approval,
-        stakes: &[(ApprovalStake, bool)],
+        stakes: &[ApprovalStake],
     ) -> DoomslugBlockProductionReadiness {
         let threshold_mode = self.threshold_mode;
         let ret = self
@@ -662,7 +654,7 @@ impl Doomslug {
     }
 
     /// Processes single approval
-    pub fn on_approval_message(&mut self, approval: &Approval, stakes: &[(ApprovalStake, bool)]) {
+    pub fn on_approval_message(&mut self, approval: &Approval, stakes: &[ApprovalStake]) {
         if approval.target_height < self.tip.height
             || approval.target_height > self.tip.height + MAX_HEIGHTS_AHEAD_TO_STORE_APPROVALS
         {
@@ -935,7 +927,6 @@ mod tests {
                 stake_next_epoch: *stake_next_epoch,
                 public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
             })
-            .map(|stake| (stake, false))
             .collect::<Vec<_>>();
         let signers = accounts
             .iter()
@@ -1058,7 +1049,6 @@ mod tests {
                 stake_next_epoch,
                 public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
             })
-            .map(|stake| (stake, false))
             .collect::<Vec<_>>();
         let clock = FakeClock::new(Utc::UNIX_EPOCH);
         let mut tracker = DoomslugApprovalsTrackersAtHeight::new(clock.clock());

chain/chain/src/lightclient.rs

@@ -1,7 +1,7 @@
 use near_chain_primitives::Error;
 use near_epoch_manager::EpochManagerAdapter;
 use near_primitives::block::BlockHeader;
-use near_primitives::hash::{hash, CryptoHash};
+use near_primitives::hash::hash;
 use near_primitives::types::EpochId;
 use near_primitives::views::validator_stake_view::ValidatorStakeView;
 use near_primitives::views::{BlockHeaderInnerLiteView, LightClientBlockView};
@@ -10,13 +10,12 @@ use crate::ChainStoreAccess;
 
 pub fn get_epoch_block_producers_view(
     epoch_id: &EpochId,
-    prev_hash: &CryptoHash,
     epoch_manager: &dyn EpochManagerAdapter,
 ) -> Result<Vec<ValidatorStakeView>, Error> {
     Ok(epoch_manager
-        .get_epoch_block_producers_ordered(epoch_id, prev_hash)?
+        .get_epoch_block_producers_ordered(epoch_id)?
         .iter()
-        .map(|x| x.0.clone().into())
+        .map(|x| x.clone().into())
         .collect::<Vec<_>>())
 }
 

chain/chain/src/runtime/tests.rs

@@ -1,4 +1,4 @@
-use std::collections::BTreeSet;
+use std::collections::{BTreeSet, HashSet};
 use std::path::PathBuf;
 
 use crate::types::{ChainConfig, RuntimeStorageConfig};
@@ -506,14 +506,14 @@ fn test_validator_rotation() {
         env.epoch_manager.get_epoch_id_from_prev_block(&env.head.last_block_hash).unwrap();
     assert_eq!(
         env.epoch_manager
-            .get_epoch_block_producers_ordered(&epoch_id, &env.head.last_block_hash)
+            .get_epoch_block_producers_ordered(&epoch_id)
             .unwrap()
             .iter()
-            .map(|x| (x.0.account_id().clone(), x.1))
-            .collect::<HashMap<_, _>>(),
-        vec![("test3".parse().unwrap(), false), ("test1".parse().unwrap(), false)]
+            .map(|x| x.account_id().clone())
+            .collect::<HashSet<_>>(),
+        vec!["test3".parse().unwrap(), "test1".parse().unwrap()]
             .into_iter()
-            .collect::<HashMap<_, _>>()
+            .collect::<HashSet<_>>()
     );
 
     let test1_acc = env.view_account(&"test1".parse().unwrap());
@@ -831,7 +831,7 @@ fn test_get_validator_info() {
             let shard_layout = env.epoch_manager.get_shard_layout(&epoch_id).unwrap();
             let shard_id = shard_layout.shard_ids().next().unwrap();
 
-            let em = env.runtime.epoch_manager.read();
+            let em = env.runtime.epoch_manager.clone();
             let bp = em.get_block_producer_info(&epoch_id, height).unwrap();
             let cp_key = ChunkProductionKey { epoch_id, height_created: height, shard_id };
             let cp = em.get_chunk_producer_info(&cp_key).unwrap();
@@ -1029,13 +1029,14 @@ fn test_challenges() {
     assert_eq!(env.view_account(&"test2".parse().unwrap()).locked, 0);
     let mut bps = env
         .epoch_manager
-        .get_epoch_block_producers_ordered(&env.head.epoch_id, &env.head.last_block_hash)
+        .get_epoch_block_producers_ordered(&env.head.epoch_id)
         .unwrap()
         .iter()
-        .map(|x| (x.0.account_id().clone(), x.1))
+        .map(|x| x.account_id().clone())
         .collect::<Vec<_>>();
     bps.sort_unstable();
-    assert_eq!(bps, vec![("test1".parse().unwrap(), false), ("test2".parse().unwrap(), true)]);
+    let expected_bps: Vec<AccountId> = vec!["test1".parse().unwrap(), "test2".parse().unwrap()];
+    assert_eq!(bps, expected_bps);
     let msg = vec![0, 1, 2];
     let signer = InMemorySigner::test_signer(&"test2".parse().unwrap());
     let signature = signer.sign(&msg);
@@ -1079,20 +1080,15 @@ fn test_double_sign_challenge_not_all_slashed() {
     assert_eq!(env.view_account(&"test2".parse().unwrap()).locked, TESTING_INIT_STAKE);
     let mut bps = env
         .epoch_manager
-        .get_epoch_block_producers_ordered(&env.head.epoch_id, &env.head.last_block_hash)
+        .get_epoch_block_producers_ordered(&env.head.epoch_id)
         .unwrap()
         .iter()
-        .map(|x| (x.0.account_id().clone(), x.1))
+        .map(|x| x.account_id().clone())
         .collect::<Vec<_>>();
     bps.sort_unstable();
-    assert_eq!(
-        bps,
-        vec![
-            ("test1".parse().unwrap(), false),
-            ("test2".parse().unwrap(), true),
-            ("test3".parse().unwrap(), false)
-        ]
-    );
+    let expected_bps: Vec<AccountId> =
+        vec!["test1".parse().unwrap(), "test2".parse().unwrap(), "test3".parse().unwrap()];
+    assert_eq!(bps, expected_bps);
     let msg = vec![0, 1, 2];
     let signer = InMemorySigner::test_signer(&"test2".parse().unwrap());
     let signature = signer.sign(&msg);

chain/chain/src/test_utils/kv_runtime.rs

@@ -443,12 +443,11 @@ impl EpochManagerAdapter for MockEpochManager {
     }
 
     fn get_part_owner(&self, epoch_id: &EpochId, part_id: u64) -> Result<AccountId, EpochError> {
-        let validators =
-            &self.get_epoch_block_producers_ordered(epoch_id, &CryptoHash::default())?;
+        let validators = &self.get_epoch_block_producers_ordered(epoch_id)?;
         // if we don't use data_parts and total_parts as part of the formula here, the part owner
         //     would not depend on height, and tests wouldn't catch passing wrong height here
         let idx = part_id as usize + self.num_data_parts() + self.num_total_parts();
-        Ok(validators[idx as usize % validators.len()].0.account_id().clone())
+        Ok(validators[idx as usize % validators.len()].account_id().clone())
     }
 
     fn get_block_info(&self, _hash: &CryptoHash) -> Result<Arc<BlockInfo>, EpochError> {
@@ -659,16 +658,15 @@ impl EpochManagerAdapter for MockEpochManager {
     fn get_epoch_block_producers_ordered(
         &self,
         epoch_id: &EpochId,
-        _last_known_block_hash: &CryptoHash,
-    ) -> Result<Vec<(ValidatorStake, bool)>, EpochError> {
+    ) -> Result<Vec<ValidatorStake>, EpochError> {
         let validators = self.get_block_producers(self.get_valset_for_epoch(epoch_id)?);
-        Ok(validators.iter().map(|x| (x.clone(), false)).collect())
+        Ok(validators.iter().map(|x| x.clone()).collect())
     }
 
     fn get_epoch_block_approvers_ordered(
         &self,
         parent_hash: &CryptoHash,
-    ) -> Result<Vec<(ApprovalStake, bool)>, EpochError> {
+    ) -> Result<Vec<ApprovalStake>, EpochError> {
         let (_cur_epoch, cur_valset, next_epoch) = self.get_epoch_and_valset(*parent_hash)?;
         let mut validators = self
             .get_block_producers(cur_valset)
@@ -686,7 +684,7 @@ impl EpochManagerAdapter for MockEpochManager {
                     .map(|x| x.get_approval_stake(true)),
             );
         }
-        let validators = validators.into_iter().map(|stake| (stake, false)).collect::<Vec<_>>();
+        let validators = validators.into_iter().map(|stake| stake).collect::<Vec<_>>();
         Ok(validators)
     }
 

chain/chain/src/tests/doomslug.rs

@@ -43,7 +43,6 @@ fn one_iter(
             stake_next_epoch: 1,
             public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
         })
-        .map(|stake| (stake, false))
         .collect::<Vec<_>>();
     let signers = account_ids
         .iter()

chain/chain/src/tests/garbage_collection.rs

@@ -55,7 +55,6 @@ fn do_fork(
         let block = if next_epoch_id == *prev_block.header().next_epoch_id() {
             TestBlockBuilder::new(Clock::real(), &prev_block, signer.clone()).build()
         } else {
-            let prev_hash = prev_block.hash();
             let epoch_id = *prev_block.header().next_epoch_id();
             if verbose {
                 println!(
@@ -64,13 +63,9 @@ fn do_fork(
                     prev_block.header().height() + 1
                 );
             }
-            let next_bp_hash = Chain::compute_bp_hash(
-                chain.epoch_manager.as_ref(),
-                next_epoch_id,
-                epoch_id,
-                &prev_hash,
-            )
-            .unwrap();
+            let next_bp_hash =
+                Chain::compute_bp_hash(chain.epoch_manager.as_ref(), next_epoch_id, epoch_id)
+                    .unwrap();
             TestBlockBuilder::new(Clock::real(), &prev_block, signer.clone())
                 .epoch_id(epoch_id)
                 .next_epoch_id(next_epoch_id)
@@ -740,10 +735,8 @@ fn add_block(
     let block = if next_epoch_id == *prev_block.header().next_epoch_id() {
         TestBlockBuilder::new(Clock::real(), &prev_block, signer).height(height).build()
     } else {
-        let prev_hash = prev_block.hash();
         let epoch_id = *prev_block.header().next_epoch_id();
-        let next_bp_hash =
-            Chain::compute_bp_hash(epoch_manager, next_epoch_id, epoch_id, &prev_hash).unwrap();
+        let next_bp_hash = Chain::compute_bp_hash(epoch_manager, next_epoch_id, epoch_id).unwrap();
         TestBlockBuilder::new(Clock::real(), &prev_block, signer)
             .height(height)
             .epoch_id(epoch_id)