fix: delete account after unstake should not cause node to crash (#2688)

As described in #2687, there is a critical vulnerability caused by a situation where accounts are deleted after they unstake but we still try to return 0 stake to the account. This PR submits an easy fix to the issue by not requiring account to exist when the returned stake is no more than 0. Please note that this is not the optimal way to fix it. The better way would be to change epoch manager behavior so that it will only record accounts in `stake_change` if it is needed. In the case of unstaking, it means that a stake change should only be recorded if it is a change from nonzero (validator or fishermen) to zero. However, this PR is enough to quickly patch the issue so that we can spend more time ironing out the proper and better fix. Resolves #2687.

neard/src/runtime.rs

@@ -1257,7 +1257,9 @@ mod test {
     use near_chain::{ReceiptResult, Tip};
     use near_crypto::{InMemorySigner, KeyType, Signer};
     use near_logger_utils::init_test_logger;
-    use near_primitives::transaction::{Action, CreateAccountAction, StakeAction};
+    use near_primitives::transaction::{
+        Action, CreateAccountAction, DeleteAccountAction, StakeAction,
+    };
     use near_primitives::types::{BlockHeightDelta, Nonce, ValidatorId, ValidatorKickoutReason};
     use near_primitives::validator_signer::{InMemoryValidatorSigner, ValidatorSigner};
     use near_primitives::views::{
@@ -2478,4 +2480,59 @@ mod test {
             TESTING_INIT_BALANCE + protocol_treasury_reward
         );
     }
+
+    #[test]
+    fn test_delete_account_after_unstake() {
+        init_test_logger();
+        let num_nodes = 2;
+        let validators = (0..num_nodes).map(|i| format!("test{}", i + 1)).collect::<Vec<_>>();
+        let mut env = TestEnv::new(
+            "test_validator_delete_account",
+            vec![validators.clone()],
+            4,
+            vec![],
+            vec![],
+            false,
+        );
+        let block_producers: Vec<_> = validators
+            .iter()
+            .map(|id| InMemoryValidatorSigner::from_seed(id, KeyType::ED25519, id))
+            .collect();
+        let signers: Vec<_> = validators
+            .iter()
+            .map(|id| InMemorySigner::from_seed(id, KeyType::ED25519, id))
+            .collect();
+
+        let staking_transaction1 = stake(1, &signers[1], &block_producers[1], 0);
+        env.step_default(vec![staking_transaction1]);
+        let account = env.view_account(&block_producers[1].validator_id());
+        assert_eq!(account.amount, TESTING_INIT_BALANCE - TESTING_INIT_STAKE);
+        assert_eq!(account.locked, TESTING_INIT_STAKE);
+        for _ in 2..=5 {
+            env.step_default(vec![]);
+        }
+        let staking_transaction2 = stake(2, &signers[1], &block_producers[1], 1);
+        env.step_default(vec![staking_transaction2]);
+        for _ in 7..=13 {
+            env.step_default(vec![]);
+        }
+        let account = env.view_account(&block_producers[1].validator_id());
+        assert_eq!(account.locked, 0);
+
+        let delete_account_transaction = SignedTransaction::from_actions(
+            4,
+            signers[1].account_id.clone(),
+            signers[1].account_id.clone(),
+            &signers[1] as &dyn Signer,
+            vec![Action::DeleteAccount(DeleteAccountAction {
+                beneficiary_id: signers[0].account_id.clone(),
+            })],
+            // runtime does not validate block history
+            CryptoHash::default(),
+        );
+        env.step_default(vec![delete_account_transaction]);
+        for _ in 15..=17 {
+            env.step_default(vec![]);
+        }
+    }
 }

runtime/runtime/src/lib.rs

@@ -854,7 +854,9 @@ impl Runtime {
                     .ok_or_else(|| RuntimeError::UnexpectedIntegerOverflow)?;
 
                 set_account(state_update, account_id.clone(), &account);
-            } else {
+            } else if *max_of_stakes > 0 {
+                // if max_of_stakes > 0, it means that the account must have locked balance
+                // and therefore must exist
                 return Err(StorageError::StorageInconsistentState(format!(
                     "Account {} with max of stakes {} is not found",
                     account_id, max_of_stakes