From a65bc1a1ddd8d46089320bd70541eee95b9cf69a Mon Sep 17 00:00:00 2001
From: Aleksey Kladov <aleksey.kladov@gmail.com>
Date: Fri, 26 Aug 2022 16:15:19 +0100
Subject: [PATCH] refactor: use EpochError::NotAValidaotor (#7438)

We have `EpochError::NotAValidatora` and chain::Error::NotAValidator.

Surprisingly, the epoch one isn't actually used, and instead epoch
manager uses convention of returning `None`, which is then converted by
RuntimeAdapter to `NotAValidator` manually.

This is inconsistent: we should either remove unused error, or actually
start using it.

This PR chose the second approach, primarily to make sure we don't have
different rules for runtime and epoch_manager.
---
 chain/chain-primitives/src/error.rs |  1 +
 chain/epoch-manager/src/lib.rs      | 12 ++++++---
 nearcore/src/runtime/mod.rs         | 38 +++++++++--------------------
 3 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs
index e12f5f5b568..ff1e6375b59 100644
--- a/chain/chain-primitives/src/error.rs
+++ b/chain/chain-primitives/src/error.rs
@@ -299,6 +299,7 @@ impl From<EpochError> for Error {
         match error {
             EpochError::EpochOutOfBounds(epoch_id) => Error::EpochOutOfBounds(epoch_id),
             EpochError::MissingBlock(h) => Error::DBNotFoundErr(h.to_string()),
+            EpochError::NotAValidator(_account_id, _epoch_id) => Error::NotAValidator,
             err => Error::ValidatorError(err.to_string()),
         }
     }
diff --git a/chain/epoch-manager/src/lib.rs b/chain/epoch-manager/src/lib.rs
index 8df12f0de62..c0dbb5e631b 100644
--- a/chain/epoch-manager/src/lib.rs
+++ b/chain/epoch-manager/src/lib.rs
@@ -955,9 +955,11 @@ impl EpochManager {
         &self,
         epoch_id: &EpochId,
         account_id: &AccountId,
-    ) -> Result<Option<ValidatorStake>, EpochError> {
+    ) -> Result<ValidatorStake, EpochError> {
         let epoch_info = self.get_epoch_info(epoch_id)?;
-        Ok(epoch_info.get_validator_by_account(account_id))
+        epoch_info
+            .get_validator_by_account(account_id)
+            .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), epoch_id.clone()))
     }
 
     /// Returns fisherman for given account id for given epoch.
@@ -965,9 +967,11 @@ impl EpochManager {
         &self,
         epoch_id: &EpochId,
         account_id: &AccountId,
-    ) -> Result<Option<ValidatorStake>, EpochError> {
+    ) -> Result<ValidatorStake, EpochError> {
         let epoch_info = self.get_epoch_info(epoch_id)?;
-        Ok(epoch_info.get_fisherman_by_account(account_id))
+        epoch_info
+            .get_fisherman_by_account(account_id)
+            .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), epoch_id.clone()))
     }
 
     pub fn get_epoch_id(&self, block_hash: &CryptoHash) -> Result<EpochId, EpochError> {
diff --git a/nearcore/src/runtime/mod.rs b/nearcore/src/runtime/mod.rs
index 1fc8d631cae..756d9a5ac25 100644
--- a/nearcore/src/runtime/mod.rs
+++ b/nearcore/src/runtime/mod.rs
@@ -907,17 +907,13 @@ impl RuntimeAdapter for NightshadeRuntime {
         shard_id: ShardId,
     ) -> Result<bool, Error> {
         let epoch_manager = self.epoch_manager.read();
-        if let Ok(chunk_producer) =
-            epoch_manager.get_chunk_producer_info(epoch_id, height_created, shard_id)
-        {
-            let block_info = epoch_manager.get_block_info(last_known_hash)?;
-            if block_info.slashed().contains_key(chunk_producer.account_id()) {
-                return Ok(false);
-            }
-            Ok(signature.verify(chunk_hash.as_ref(), chunk_producer.public_key()))
-        } else {
-            Err(Error::NotAValidator)
+        let chunk_producer =
+            epoch_manager.get_chunk_producer_info(epoch_id, height_created, shard_id)?;
+        let block_info = epoch_manager.get_block_info(last_known_hash)?;
+        if block_info.slashed().contains_key(chunk_producer.account_id()) {
+            return Ok(false);
         }
+        Ok(signature.verify(chunk_hash.as_ref(), chunk_producer.public_key()))
     }
 
     fn verify_approvals_and_threshold_orphan(
@@ -1043,14 +1039,9 @@ impl RuntimeAdapter for NightshadeRuntime {
         account_id: &AccountId,
     ) -> Result<(ValidatorStake, bool), Error> {
         let epoch_manager = self.epoch_manager.read();
-        match epoch_manager.get_validator_by_account_id(epoch_id, account_id) {
-            Ok(Some(validator)) => {
-                let block_info = epoch_manager.get_block_info(last_known_block_hash)?;
-                Ok((validator, block_info.slashed().contains_key(account_id)))
-            }
-            Ok(None) => Err(Error::NotAValidator),
-            Err(e) => Err(e.into()),
-        }
+        let validator = epoch_manager.get_validator_by_account_id(epoch_id, account_id)?;
+        let block_info = epoch_manager.get_block_info(last_known_block_hash)?;
+        Ok((validator, block_info.slashed().contains_key(account_id)))
     }
 
     fn get_fisherman_by_account_id(
@@ -1060,14 +1051,9 @@ impl RuntimeAdapter for NightshadeRuntime {
         account_id: &AccountId,
     ) -> Result<(ValidatorStake, bool), Error> {
         let epoch_manager = self.epoch_manager.read();
-        match epoch_manager.get_fisherman_by_account_id(epoch_id, account_id) {
-            Ok(Some(fisherman)) => {
-                let block_info = epoch_manager.get_block_info(last_known_block_hash)?;
-                Ok((fisherman, block_info.slashed().contains_key(account_id)))
-            }
-            Ok(None) => Err(Error::NotAValidator),
-            Err(e) => Err(e.into()),
-        }
+        let fisherman = epoch_manager.get_fisherman_by_account_id(epoch_id, account_id)?;
+        let block_info = epoch_manager.get_block_info(last_known_block_hash)?;
+        Ok((fisherman, block_info.slashed().contains_key(account_id)))
     }
 
     fn num_shards(&self, epoch_id: &EpochId) -> Result<NumShards, Error> {