Skip to content

Commit

Permalink
Revert "Dont panic for check balance (#1493)"
Browse files Browse the repository at this point in the history
This reverts commit 908daf8.
  • Loading branch information
ilblackdragon authored Oct 21, 2019
1 parent 908daf8 commit 6579fd9
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 118 deletions.
4 changes: 2 additions & 2 deletions chain/chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::types::{
ValidatorSignatureVerificationResult, Weight,
};
use crate::{Chain, ChainGenesis, ValidTransaction};
use near_primitives::errors::RuntimeError;
use near_primitives::errors::InvalidTxErrorOrStorageError;
use near_primitives::merkle::{merklize, verify_path, MerklePath};

pub const DEFAULT_STATE_NUM_PARTS: u64 = 17; /* TODO MOO */
Expand Down Expand Up @@ -441,7 +441,7 @@ impl RuntimeAdapter for KeyValueRuntime {
_gas_price: Balance,
_state_root: StateRoot,
transaction: SignedTransaction,
) -> Result<ValidTransaction, RuntimeError> {
) -> Result<ValidTransaction, InvalidTxErrorOrStorageError> {
Ok(ValidTransaction { transaction })
}

Expand Down
4 changes: 2 additions & 2 deletions chain/chain/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use borsh::{BorshDeserialize, BorshSerialize};

use near_crypto::{Signature, Signer};
pub use near_primitives::block::{Block, BlockHeader, Weight};
use near_primitives::errors::RuntimeError;
use near_primitives::errors::InvalidTxErrorOrStorageError;
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::merkle::{merklize, MerklePath};
use near_primitives::receipt::Receipt;
Expand Down Expand Up @@ -141,7 +141,7 @@ pub trait RuntimeAdapter: Send + Sync {
gas_price: Balance,
state_root: StateRoot,
transaction: SignedTransaction,
) -> Result<ValidTransaction, RuntimeError>;
) -> Result<ValidTransaction, InvalidTxErrorOrStorageError>;

/// Filter transactions by verifying each one by one in the given order. Every successful
/// verification stores the updated account balances to be used by next transactions.
Expand Down
9 changes: 3 additions & 6 deletions chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use near_crypto::Signature;
use near_network::types::{ChunkPartMsg, PeerId, ReasonForBan};
use near_network::{NetworkClientResponses, NetworkRequests};
use near_primitives::block::{Block, BlockHeader};
use near_primitives::errors::{InvalidTxError, RuntimeError};
use near_primitives::errors::InvalidTxErrorOrStorageError;
use near_primitives::hash::CryptoHash;
use near_primitives::merkle::{merklize, MerklePath};
use near_primitives::receipt::Receipt;
Expand Down Expand Up @@ -855,14 +855,11 @@ impl Client {
self.forward_tx(valid_transaction.transaction)
}
}
Err(RuntimeError::InvalidTxError(err)) => {
Err(InvalidTxErrorOrStorageError::InvalidTxError(err)) => {
debug!(target: "client", "Invalid tx: {:?}", err);
NetworkClientResponses::InvalidTx(err)
}
Err(RuntimeError::StorageError(err)) => panic!("{}", err),
Err(RuntimeError::BalanceMismatch(err)) => {
unreachable!("Unexpected BalanceMismatch error in validate_tx: {}", err)
}
Err(InvalidTxErrorOrStorageError::StorageError(err)) => panic!(err),
}
} else {
// We are not tracking this shard, so there is no way to validate this tx. Just rerouting.
Expand Down
77 changes: 5 additions & 72 deletions core/primitives/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,91 +166,24 @@ impl Display for InvalidAccessKeyError {
}
}

/// Happens when the input balance doesn't match the output balance in Runtime apply.
#[derive(BorshSerialize, BorshDeserialize, Debug, Clone, PartialEq, Eq)]
pub struct BalanceMismatchError {
// Input balances
pub initial_accounts_balance: Balance,
pub incoming_receipts_balance: Balance,
pub initial_postponed_receipts_balance: Balance,
// Output balances
pub final_accounts_balance: Balance,
pub outgoing_receipts_balance: Balance,
pub final_postponed_receipts_balance: Balance,
pub total_rent_paid: Balance,
pub total_validator_reward: Balance,
pub total_balance_burnt: Balance,
}

impl Display for BalanceMismatchError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
// Using saturating add to avoid overflow in display
let initial_balance = self
.initial_accounts_balance
.saturating_add(self.incoming_receipts_balance)
.saturating_add(self.initial_postponed_receipts_balance);
let final_balance = self
.final_accounts_balance
.saturating_add(self.outgoing_receipts_balance)
.saturating_add(self.final_postponed_receipts_balance)
.saturating_add(self.total_rent_paid)
.saturating_add(self.total_validator_reward)
.saturating_add(self.total_balance_burnt);
write!(
f,
"Balance Mismatch Error. The input balance {} doesn't match output balance {}\n\
Inputs:\n\
\tInitial accounts balance sum: {}\n\
\tIncoming receipts balance sum: {}\n\
\tInitial postponed receipts balance sum: {}\n\
Outputs:\n\
\tFinal accounts balance sum: {}\n\
\tOutgoing receipts balance sum: {}\n\
\tFinal postponed receipts balance sum: {}\n\
\tTotal rent paid: {}\n\
\tTotal validators reward: {}\n\
\tTotal balance burnt: {}",
initial_balance,
final_balance,
self.initial_accounts_balance,
self.incoming_receipts_balance,
self.initial_postponed_receipts_balance,
self.final_accounts_balance,
self.outgoing_receipts_balance,
self.final_postponed_receipts_balance,
self.total_rent_paid,
self.total_validator_reward,
self.total_balance_burnt
)
}
}

/// Error returned from `Runtime::apply`
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum RuntimeError {
pub enum InvalidTxErrorOrStorageError {
InvalidTxError(InvalidTxError),
StorageError(StorageError),
BalanceMismatch(BalanceMismatchError),
}

impl From<StorageError> for RuntimeError {
impl From<StorageError> for InvalidTxErrorOrStorageError {
fn from(e: StorageError) -> Self {
RuntimeError::StorageError(e)
}
}

impl From<BalanceMismatchError> for RuntimeError {
fn from(e: BalanceMismatchError) -> Self {
RuntimeError::BalanceMismatch(e)
InvalidTxErrorOrStorageError::StorageError(e)
}
}

impl<T> From<T> for RuntimeError
impl<T> From<T> for InvalidTxErrorOrStorageError
where
T: Into<InvalidTxError>,
{
fn from(e: T) -> Self {
RuntimeError::InvalidTxError(e.into())
InvalidTxErrorOrStorageError::InvalidTxError(e.into())
}
}

Expand Down
11 changes: 5 additions & 6 deletions near/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use near_chain::{BlockHeader, Error, ErrorKind, RuntimeAdapter, ValidTransaction
use near_crypto::{PublicKey, Signature};
use near_epoch_manager::{BlockInfo, EpochConfig, EpochManager, RewardCalculator};
use near_primitives::account::{AccessKey, Account};
use near_primitives::errors::RuntimeError;
use near_primitives::errors::InvalidTxErrorOrStorageError;
use near_primitives::hash::CryptoHash;
use near_primitives::receipt::Receipt;
use near_primitives::serialize::from_base64;
Expand Down Expand Up @@ -497,7 +497,7 @@ impl RuntimeAdapter for NightshadeRuntime {
gas_price: Balance,
state_root: StateRoot,
transaction: SignedTransaction,
) -> Result<ValidTransaction, RuntimeError> {
) -> Result<ValidTransaction, InvalidTxErrorOrStorageError> {
let mut state_update = TrieUpdate::new(self.trie.clone(), state_root.hash);
let apply_state = ApplyState {
block_index,
Expand Down Expand Up @@ -641,10 +641,9 @@ impl RuntimeAdapter for NightshadeRuntime {
.runtime
.apply(state_update, &apply_state, &receipts, &transactions)
.map_err(|e| match e {
RuntimeError::InvalidTxError(_) => ErrorKind::InvalidTransactions,
RuntimeError::BalanceMismatch(e) => panic!("{}", e),
RuntimeError::StorageError(e) => {
panic!("Storage error. Corrupted db or invalid state. {}", e);
InvalidTxErrorOrStorageError::InvalidTxError(_) => ErrorKind::InvalidTransactions,
InvalidTxErrorOrStorageError::StorageError(_) => {
panic!("Storage error. Corrupted db or invalid state.");
}
})?;

Expand Down
61 changes: 35 additions & 26 deletions runtime/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ use crate::config::{
};
pub use crate::store::StateRecord;
use near_primitives::errors::{
ActionError, BalanceMismatchError, ExecutionError, InvalidAccessKeyError, InvalidTxError,
RuntimeError,
ActionError, ExecutionError, InvalidAccessKeyError, InvalidTxError,
InvalidTxErrorOrStorageError,
};

mod actions;
Expand Down Expand Up @@ -168,7 +168,7 @@ impl Runtime {
state_update: &mut TrieUpdate,
apply_state: &ApplyState,
signed_transaction: &SignedTransaction,
) -> Result<VerificationResult, RuntimeError> {
) -> Result<VerificationResult, InvalidTxErrorOrStorageError> {
let transaction = &signed_transaction.transaction;
let signer_id = &transaction.signer_id;
if !is_valid_account_id(&signer_id) {
Expand Down Expand Up @@ -321,7 +321,7 @@ impl Runtime {
new_local_receipts: &mut Vec<Receipt>,
new_receipts: &mut Vec<Receipt>,
stats: &mut ApplyStats,
) -> Result<ExecutionOutcomeWithId, RuntimeError> {
) -> Result<ExecutionOutcomeWithId, InvalidTxErrorOrStorageError> {
near_metrics::inc_counter(&metrics::TRANSACTION_PROCESSED_TOTAL);
let outcome =
match self.verify_and_charge_transaction(state_update, apply_state, signed_transaction)
Expand Down Expand Up @@ -850,7 +850,7 @@ impl Runtime {
apply_state: &ApplyState,
prev_receipts: &[Receipt],
transactions: &[SignedTransaction],
) -> Result<ApplyResult, RuntimeError> {
) -> Result<ApplyResult, InvalidTxErrorOrStorageError> {
// TODO(#1481): Remove clone after Runtime bug is fixed.
let initial_state = state_update.clone();

Expand Down Expand Up @@ -914,7 +914,7 @@ impl Runtime {
transactions: &[SignedTransaction],
new_receipts: &[Receipt],
stats: &ApplyStats,
) -> Result<(), RuntimeError> {
) -> Result<(), InvalidTxErrorOrStorageError> {
// Accounts
let all_accounts_ids: HashSet<AccountId> = transactions
.iter()
Expand All @@ -931,8 +931,8 @@ impl Runtime {
.into_iter()
.sum::<u128>())
};
let initial_accounts_balance = total_accounts_balance(&initial_state)?;
let final_accounts_balance = total_accounts_balance(&final_state)?;
let initial_account_balance = total_accounts_balance(&initial_state)?;
let final_account_balance = total_accounts_balance(&final_state)?;
// Receipts
let receipt_cost = |receipt: &Receipt| -> Result<u128, InvalidTxError> {
Ok(match &receipt.receipt {
Expand Down Expand Up @@ -997,45 +997,55 @@ impl Runtime {
.filter_map(|x| x)
.collect();

let total_postponed_receipts_cost = |state| -> Result<u128, RuntimeError> {
let total_postponed_receipts_cost = |state| -> Result<u128, InvalidTxErrorOrStorageError> {
Ok(all_potential_postponed_receipt_ids
.iter()
.map(|(account_id, receipt_id)| {
Ok(get_receipt(state, account_id, &receipt_id)?
.map_or(Ok(0), |r| receipt_cost(&r))?)
})
.collect::<Result<Vec<u128>, RuntimeError>>()?
.collect::<Result<Vec<u128>, InvalidTxErrorOrStorageError>>()?
.into_iter()
.sum::<u128>())
};
let initial_postponed_receipts_balance = total_postponed_receipts_cost(initial_state)?;
let final_postponed_receipts_balance = total_postponed_receipts_cost(final_state)?;
// Sum it up
let initial_balance = initial_accounts_balance
let initial_balance = initial_account_balance
+ incoming_receipts_balance
+ initial_postponed_receipts_balance;
let final_balance = final_accounts_balance
let final_balance = final_account_balance
+ outgoing_receipts_balance
+ final_postponed_receipts_balance
+ stats.total_rent_paid
+ stats.total_validator_reward
+ stats.total_balance_burnt;
if initial_balance != final_balance {
Err(BalanceMismatchError {
initial_accounts_balance,
final_accounts_balance,
panic!(
"Runtime Apply initial balance sum {} doesn't match final balance sum {}\n\
\tInitial account balance sum: {}\n\
\tFinal account balance sum: {}\n\
\tIncoming receipts balance sum: {}\n\
\tOutgoing receipts balance sum: {}\n\
\tInitial postponed receipts balance sum: {}\n\
\tFinal postponed receipts balance sum: {}\n\
\t{:?}\n\
\tIncoming Receipts {:#?}\n\
\tOutgoing Receipts {:#?}",
initial_balance,
final_balance,
initial_account_balance,
final_account_balance,
incoming_receipts_balance,
outgoing_receipts_balance,
initial_postponed_receipts_balance,
final_postponed_receipts_balance,
total_balance_burnt: stats.total_balance_burnt,
total_rent_paid: stats.total_rent_paid,
total_validator_reward: stats.total_validator_reward,
}
.into())
} else {
Ok(())
stats,
prev_receipts,
new_receipts,
);
}
Ok(())
}

pub fn compute_storage_usage(&self, records: &[StateRecord]) -> HashMap<AccountId, u64> {
Expand Down Expand Up @@ -1196,7 +1206,6 @@ mod tests {
use testlib::runtime_utils::{alice_account, bob_account};

use super::*;
use assert_matches::assert_matches;
use near_crypto::{InMemorySigner, KeyType};
use near_primitives::transaction::TransferAction;

Expand Down Expand Up @@ -1243,13 +1252,14 @@ mod tests {
}

#[test]
#[should_panic]
fn test_check_balance_unaccounted_refund() {
let trie = create_trie();
let root = MerkleHash::default();
let initial_state = TrieUpdate::new(trie.clone(), root);
let final_state = TrieUpdate::new(trie.clone(), root);
let runtime = Runtime::new(RuntimeConfig::default());
let err = runtime
runtime
.check_balance(
&initial_state,
&final_state,
Expand All @@ -1258,8 +1268,7 @@ mod tests {
&[],
&ApplyStats::default(),
)
.unwrap_err();
assert_matches!(err, RuntimeError::BalanceMismatch(_));
.unwrap_or_else(|_| ());
}

#[test]
Expand Down
9 changes: 5 additions & 4 deletions test-utils/testlib/src/user/runtime_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use node_runtime::{ApplyState, Runtime};

use crate::user::{User, POISONED_LOCK_ERR};
use near::config::INITIAL_GAS_PRICE;
use near_primitives::errors::RuntimeError;
use near_primitives::errors::InvalidTxErrorOrStorageError;

/// Mock client without chain, used in RuntimeUser and RuntimeNode
pub struct MockClient {
Expand Down Expand Up @@ -74,9 +74,10 @@ impl RuntimeUser {
.runtime
.apply(state_update, &apply_state, &receipts, &txs)
.map_err(|e| match e {
RuntimeError::InvalidTxError(e) => format!("{}", e),
RuntimeError::BalanceMismatch(e) => panic!("{}", e),
RuntimeError::StorageError(e) => panic!("Storage error {:?}", e),
InvalidTxErrorOrStorageError::InvalidTxError(e) => format!("{}", e),
InvalidTxErrorOrStorageError::StorageError(e) => {
panic!("Storage error {:?}", e)
}
})?;
for outcome_with_id in apply_result.tx_result.into_iter() {
self.transaction_results
Expand Down

0 comments on commit 6579fd9

Please sign in to comment.